From b8549171867104f4bca6cc6f9cd1ec7e3ffed7ec Mon Sep 17 00:00:00 2001 From: A <258483684+la14-1@users.noreply.github.com> Date: Mon, 16 Mar 2026 15:22:29 -0700 Subject: [PATCH] fix(security): validate tunnel URL and port from history before openBrowser() (#2697) Add validateTunnelUrl() and validateTunnelPort() in security.ts to prevent phishing attacks via tampered ~/.spawn/history.json. Apply both validations in cmdEnterAgent() and cmdOpenDashboard() in connect.ts before any tunnel data is used. - validateTunnelUrl: enforce URL starts with http://localhost: or http://127.0.0.1: only (blocks external/phishing URLs) - validateTunnelPort: enforce numeric value in range 1-65535 - Add comprehensive test cases for both validators Fixes #2696 Agent: security-auditor Co-authored-by: B <6723574+louisgv@users.noreply.github.com> Co-authored-by: Claude Sonnet 4.6 --- packages/cli/package.json | 2 +- .../security-connection-validation.test.ts | 107 ++++++++++++++++++ packages/cli/src/commands/connect.ts | 33 ++++++ packages/cli/src/security.ts | 72 ++++++++++++ 4 files changed, 213 insertions(+), 1 deletion(-) diff --git a/packages/cli/package.json b/packages/cli/package.json index 2bfb1dab..ef5a3899 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -1,6 +1,6 @@ { "name": "@openrouter/spawn", - "version": "0.20.5", + "version": "0.20.6", "type": "module", "bin": { "spawn": "cli.js" diff --git a/packages/cli/src/__tests__/security-connection-validation.test.ts b/packages/cli/src/__tests__/security-connection-validation.test.ts index 5830b1fc..16e41940 100644 --- a/packages/cli/src/__tests__/security-connection-validation.test.ts +++ b/packages/cli/src/__tests__/security-connection-validation.test.ts @@ -10,6 +10,8 @@ import { validateMetadataValue, validatePreLaunchCmd, validateServerIdentifier, + validateTunnelPort, + validateTunnelUrl, validateUsername, } from "../security.js"; @@ -442,3 +444,108 @@ describe("validateMetadataValue", () => { }); }); }); + +describe("validateTunnelUrl", () => { + describe("valid inputs", () => { + it("should accept localhost URLs with __PORT__ placeholder", () => { + expect(() => validateTunnelUrl("http://localhost:__PORT__")).not.toThrow(); + expect(() => validateTunnelUrl("http://127.0.0.1:__PORT__")).not.toThrow(); + }); + + it("should accept localhost URLs with numeric ports", () => { + expect(() => validateTunnelUrl("http://localhost:8080")).not.toThrow(); + expect(() => validateTunnelUrl("http://127.0.0.1:3000")).not.toThrow(); + }); + + it("should accept localhost URLs with path components", () => { + expect(() => validateTunnelUrl("http://localhost:__PORT__/dashboard")).not.toThrow(); + expect(() => validateTunnelUrl("http://127.0.0.1:__PORT__/app/ui")).not.toThrow(); + expect(() => validateTunnelUrl("http://localhost:8080/?token=abc")).not.toThrow(); + }); + + it("should accept empty or missing values", () => { + expect(() => validateTunnelUrl("")).not.toThrow(); + expect(() => validateTunnelUrl(" ")).not.toThrow(); + }); + }); + + describe("invalid inputs — phishing prevention", () => { + it("should reject external URLs", () => { + expect(() => validateTunnelUrl("https://evil.com")).toThrow(/Invalid tunnel URL/); + expect(() => validateTunnelUrl("http://attacker.com:8080")).toThrow(/Invalid tunnel URL/); + }); + + it("should reject https localhost (tunnel is always http)", () => { + expect(() => validateTunnelUrl("https://localhost:__PORT__")).toThrow(/Invalid tunnel URL/); + }); + + it("should reject URLs without port", () => { + expect(() => validateTunnelUrl("http://localhost")).toThrow(/Invalid tunnel URL/); + expect(() => validateTunnelUrl("http://localhost/")).toThrow(/Invalid tunnel URL/); + }); + + it("should reject non-HTTP schemes", () => { + expect(() => validateTunnelUrl("javascript:alert(1)")).toThrow(/Invalid tunnel URL/); + expect(() => validateTunnelUrl("file:///etc/passwd")).toThrow(/Invalid tunnel URL/); + expect(() => validateTunnelUrl("ftp://localhost:21")).toThrow(/Invalid tunnel URL/); + }); + + it("should reject URLs that are too long", () => { + const longUrl = "http://localhost:__PORT__/" + "a".repeat(2048); + expect(() => validateTunnelUrl(longUrl)).toThrow(/too long/); + }); + + it("should reject URLs with credentials", () => { + expect(() => validateTunnelUrl("http://user:pass@localhost:8080")).toThrow(/Invalid tunnel URL/); + }); + + it("should reject lookalike hosts", () => { + expect(() => validateTunnelUrl("http://localhost.evil.com:8080")).toThrow(/Invalid tunnel URL/); + expect(() => validateTunnelUrl("http://127.0.0.2:8080")).toThrow(/Invalid tunnel URL/); + }); + }); +}); + +describe("validateTunnelPort", () => { + describe("valid inputs", () => { + it("should accept valid port numbers", () => { + expect(() => validateTunnelPort("1")).not.toThrow(); + expect(() => validateTunnelPort("80")).not.toThrow(); + expect(() => validateTunnelPort("443")).not.toThrow(); + expect(() => validateTunnelPort("8080")).not.toThrow(); + expect(() => validateTunnelPort("65535")).not.toThrow(); + }); + + it("should accept empty or missing values", () => { + expect(() => validateTunnelPort("")).not.toThrow(); + expect(() => validateTunnelPort(" ")).not.toThrow(); + }); + }); + + describe("invalid inputs", () => { + it("should reject non-numeric values", () => { + expect(() => validateTunnelPort("abc")).toThrow(/Invalid tunnel port/); + expect(() => validateTunnelPort("80abc")).toThrow(/Invalid tunnel port/); + expect(() => validateTunnelPort("80; rm -rf /")).toThrow(/Invalid tunnel port/); + }); + + it("should reject port 0", () => { + expect(() => validateTunnelPort("0")).toThrow(/Invalid tunnel port/); + }); + + it("should reject ports above 65535", () => { + expect(() => validateTunnelPort("65536")).toThrow(/Invalid tunnel port/); + expect(() => validateTunnelPort("99999")).toThrow(/Invalid tunnel port/); + }); + + it("should reject negative ports", () => { + expect(() => validateTunnelPort("-1")).toThrow(/Invalid tunnel port/); + }); + + it("should reject shell metacharacters", () => { + expect(() => validateTunnelPort("$(whoami)")).toThrow(/Invalid tunnel port/); + expect(() => validateTunnelPort("`id`")).toThrow(/Invalid tunnel port/); + expect(() => validateTunnelPort("8080|cat")).toThrow(/Invalid tunnel port/); + }); + }); +}); diff --git a/packages/cli/src/commands/connect.ts b/packages/cli/src/commands/connect.ts index 1beffd95..a273256d 100644 --- a/packages/cli/src/commands/connect.ts +++ b/packages/cli/src/commands/connect.ts @@ -9,6 +9,8 @@ import { validateLaunchCmd, validatePreLaunchCmd, validateServerIdentifier, + validateTunnelPort, + validateTunnelUrl, validateUsername, } from "../security.js"; import { getHistoryPath } from "../shared/paths.js"; @@ -184,6 +186,22 @@ export async function cmdEnterAgent( let tunnelHandle: SshTunnelHandle | undefined; const tunnelPort = connection.metadata?.tunnel_remote_port; if (tunnelPort && connection.ip !== "sprite-console") { + // SECURITY: Validate tunnel metadata before use (prevent phishing via tampered history) + const tunnelValidation = tryCatch(() => { + validateTunnelPort(tunnelPort); + const tpl = connection.metadata?.tunnel_browser_url_template; + if (tpl) { + validateTunnelUrl(tpl); + } + }); + if (!tunnelValidation.ok) { + p.log.error(`Security validation failed: ${getErrorMessage(tunnelValidation.error)}`); + p.log.info("Your spawn history file may be corrupted or tampered with."); + p.log.info(`Location: ${getHistoryPath()}`); + p.log.info("To fix: edit the file and remove the invalid entry, or run 'spawn list --clear'"); + process.exit(1); + } + const tunnelResult = await asyncTryCatchIf(isOperationalError, async () => { const keys = await ensureSshKeys(); tunnelHandle = await startSshTunnel({ @@ -243,6 +261,21 @@ export async function cmdOpenDashboard(connection: VMConnection): Promise return; } + // SECURITY: Validate tunnel metadata before use (prevent phishing via tampered history) + const tunnelValidation = tryCatch(() => { + validateTunnelPort(tunnelPort); + if (urlTemplate) { + validateTunnelUrl(urlTemplate); + } + }); + if (!tunnelValidation.ok) { + p.log.error(`Security validation failed: ${getErrorMessage(tunnelValidation.error)}`); + p.log.info("Your spawn history file may be corrupted or tampered with."); + p.log.info(`Location: ${getHistoryPath()}`); + p.log.info("To fix: edit the file and remove the invalid entry, or run 'spawn list --clear'"); + return; + } + p.log.step("Opening SSH tunnel to dashboard..."); const keys = await ensureSshKeys(); const tunnelResult = await asyncTryCatchIf(isOperationalError, () => diff --git a/packages/cli/src/security.ts b/packages/cli/src/security.ts index cdaa8f21..a7c052ff 100644 --- a/packages/cli/src/security.ts +++ b/packages/cli/src/security.ts @@ -480,6 +480,78 @@ export function validateMetadataValue(value: string, fieldName: string): void { } } +/** + * Validates a tunnel browser URL template from connection history metadata. + * SECURITY-CRITICAL: This URL is passed to openBrowser() — a malicious URL + * could direct the user to a phishing site. + * + * Only allows URLs that point to localhost (http://localhost: or http://127.0.0.1:) + * with a __PORT__ placeholder or a numeric port. + * + * @param url - The tunnel_browser_url_template value to validate + * @throws Error if the URL is not a safe localhost URL + */ +export function validateTunnelUrl(url: string): void { + if (!url || url.trim() === "") { + return; // Empty/missing is fine — caller skips browser open + } + + if (url.length > 2048) { + throw new Error( + `Tunnel URL template is too long (${url.length} characters, maximum is 2048)\n\n` + + "Your spawn history file may be corrupted or tampered with.\n" + + `To fix: run 'spawn list --clear' to reset history`, + ); + } + + // Only allow http://localhost: or http://127.0.0.1: + // The __PORT__ placeholder gets replaced at runtime with the actual local tunnel port. + const SAFE_TUNNEL_URL = + /^http:\/\/(?:localhost|127\.0\.0\.1):(?:__PORT__|\d{1,5})(?:\/[a-zA-Z0-9._~:/?#[\]@!$&'()*+,;=%-]*)?$/; + if (!SAFE_TUNNEL_URL.test(url)) { + throw new Error( + `Invalid tunnel URL template: "${url}"\n\n` + + "Tunnel URLs must start with http://localhost: or http://127.0.0.1:\n" + + "followed by a port number or __PORT__ placeholder.\n\n" + + "Your spawn history file may be corrupted or tampered with.\n" + + `To fix: run 'spawn list --clear' to reset history`, + ); + } +} + +/** + * Validates a tunnel remote port from connection history metadata. + * SECURITY-CRITICAL: This port is passed to startSshTunnel() — an out-of-range + * value could cause unexpected behavior. + * + * @param port - The tunnel_remote_port value to validate (string from metadata) + * @throws Error if the port is not a valid number in range 1-65535 + */ +export function validateTunnelPort(port: string): void { + if (!port || port.trim() === "") { + return; // Empty/missing is fine — caller skips tunnel setup + } + + // Must be purely numeric (no shell metacharacters) + if (!/^\d+$/.test(port)) { + throw new Error( + `Invalid tunnel port: "${port}"\n\n` + + "Tunnel port must be a numeric value between 1 and 65535.\n\n" + + "Your spawn history file may be corrupted or tampered with.\n" + + `To fix: run 'spawn list --clear' to reset history`, + ); + } + + const num = Number.parseInt(port, 10); + if (num < 1 || num > 65535) { + throw new Error( + `Invalid tunnel port: ${num} (must be between 1 and 65535)\n\n` + + "Your spawn history file may be corrupted or tampered with.\n" + + `To fix: run 'spawn list --clear' to reset history`, + ); + } +} + // Sensitive path patterns that should never be read as prompt files // These protect credentials and system files from accidental exfiltration const SENSITIVE_PATH_PATTERNS: ReadonlyArray<{