diff --git a/src/commands/gateway-status.e2e.test.ts b/src/commands/gateway-status.e2e.test.ts index 1e532a752c..0746bac5f3 100644 --- a/src/commands/gateway-status.e2e.test.ts +++ b/src/commands/gateway-status.e2e.test.ts @@ -3,7 +3,7 @@ import { describe, expect, it, vi } from "vitest"; const loadConfig = vi.fn(() => ({ gateway: { mode: "remote", - remote: { url: "ws://remote.example:18789", token: "rtok" }, + remote: { url: "wss://remote.example:18789", token: "rtok" }, auth: { token: "ltok" }, }, })); @@ -272,7 +272,7 @@ describe("gateway-status command", () => { loadConfig.mockReturnValueOnce({ gateway: { mode: "remote", - remote: { url: "ws://studio.example:18789", token: "rtok" }, + remote: { url: "wss://studio.example:18789", token: "rtok" }, auth: { token: "ltok" }, }, }); @@ -298,7 +298,7 @@ describe("gateway-status command", () => { loadConfig.mockReturnValueOnce({ gateway: { mode: "remote", - remote: { url: "ws://studio.example:18789", token: "rtok" }, + remote: { url: "wss://studio.example:18789", token: "rtok" }, auth: { token: "ltok" }, }, }); diff --git a/src/gateway/call.test.ts b/src/gateway/call.test.ts index 9beb849390..8ff455a3a8 100644 --- a/src/gateway/call.test.ts +++ b/src/gateway/call.test.ts @@ -31,9 +31,13 @@ vi.mock("../infra/tailnet.js", () => ({ pickPrimaryTailnetIPv4, })); -vi.mock("./net.js", () => ({ - pickPrimaryLanIPv4, -})); +vi.mock("./net.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + pickPrimaryLanIPv4, + }; +}); vi.mock("./client.js", () => ({ describeGatewayCloseCode: (code: number) => { @@ -91,7 +95,7 @@ function makeRemotePasswordGatewayConfig(remotePassword: string, localPassword = return { gateway: { mode: "remote", - remote: { url: "ws://remote.example:18789", password: remotePassword }, + remote: { url: "wss://remote.example:18789", password: remotePassword }, auth: { password: localPassword }, }, }; @@ -122,25 +126,46 @@ describe("callGateway url resolution", () => { expect(lastClientOptions?.url).toBe("ws://127.0.0.1:18800"); }); - it("uses tailnet IP when local bind is tailnet and tailnet is present", async () => { - loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "tailnet" } }); + it("uses tailnet IP with TLS when local bind is tailnet", async () => { + loadConfig.mockReturnValue({ + gateway: { mode: "local", bind: "tailnet", tls: { enabled: true } }, + }); resolveGatewayPort.mockReturnValue(18800); pickPrimaryTailnetIPv4.mockReturnValue("100.64.0.1"); await callGateway({ method: "health" }); - expect(lastClientOptions?.url).toBe("ws://100.64.0.1:18800"); + expect(lastClientOptions?.url).toBe("wss://100.64.0.1:18800"); }); - it("uses LAN IP when bind is lan and LAN IP is available", async () => { - loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "lan" } }); + it("blocks ws:// to tailnet IP without TLS (CWE-319)", async () => { + loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "tailnet" } }); + resolveGatewayPort.mockReturnValue(18800); + pickPrimaryTailnetIPv4.mockReturnValue("100.64.0.1"); + + await expect(callGateway({ method: "health" })).rejects.toThrow("SECURITY ERROR"); + }); + + it("uses LAN IP with TLS when bind is lan", async () => { + loadConfig.mockReturnValue({ + gateway: { mode: "local", bind: "lan", tls: { enabled: true } }, + }); resolveGatewayPort.mockReturnValue(18800); pickPrimaryTailnetIPv4.mockReturnValue(undefined); pickPrimaryLanIPv4.mockReturnValue("192.168.1.42"); await callGateway({ method: "health" }); - expect(lastClientOptions?.url).toBe("ws://192.168.1.42:18800"); + expect(lastClientOptions?.url).toBe("wss://192.168.1.42:18800"); + }); + + it("blocks ws:// to LAN IP without TLS (CWE-319)", async () => { + loadConfig.mockReturnValue({ gateway: { mode: "local", bind: "lan" } }); + resolveGatewayPort.mockReturnValue(18800); + pickPrimaryTailnetIPv4.mockReturnValue(undefined); + pickPrimaryLanIPv4.mockReturnValue("192.168.1.42"); + + await expect(callGateway({ method: "health" })).rejects.toThrow("SECURITY ERROR"); }); it("falls back to loopback when bind is lan but no LAN IP found", async () => { @@ -214,9 +239,9 @@ describe("buildGatewayConnectionDetails", () => { expect(details.message).toContain("Gateway target: ws://127.0.0.1:18789"); }); - it("uses LAN IP and reports lan source when bind is lan", () => { + it("uses LAN IP with TLS and reports lan source when bind is lan", () => { loadConfig.mockReturnValue({ - gateway: { mode: "local", bind: "lan" }, + gateway: { mode: "local", bind: "lan", tls: { enabled: true } }, }); resolveGatewayPort.mockReturnValue(18800); pickPrimaryTailnetIPv4.mockReturnValue(undefined); @@ -224,11 +249,22 @@ describe("buildGatewayConnectionDetails", () => { const details = buildGatewayConnectionDetails(); - expect(details.url).toBe("ws://10.0.0.5:18800"); + expect(details.url).toBe("wss://10.0.0.5:18800"); expect(details.urlSource).toBe("local lan 10.0.0.5"); expect(details.bindDetail).toBe("Bind: lan"); }); + it("throws for ws:// to LAN IP without TLS (CWE-319)", () => { + loadConfig.mockReturnValue({ + gateway: { mode: "local", bind: "lan" }, + }); + resolveGatewayPort.mockReturnValue(18800); + pickPrimaryTailnetIPv4.mockReturnValue(undefined); + pickPrimaryLanIPv4.mockReturnValue("10.0.0.5"); + + expect(() => buildGatewayConnectionDetails()).toThrow("SECURITY ERROR"); + }); + it("prefers remote url when configured", () => { loadConfig.mockReturnValue({ gateway: { @@ -247,6 +283,34 @@ describe("buildGatewayConnectionDetails", () => { expect(details.bindDetail).toBeUndefined(); expect(details.remoteFallbackNote).toBeUndefined(); }); + + it("throws for insecure ws:// remote URLs (CWE-319)", () => { + loadConfig.mockReturnValue({ + gateway: { + mode: "remote", + bind: "loopback", + remote: { url: "ws://remote.example.com:18789" }, + }, + }); + resolveGatewayPort.mockReturnValue(18789); + pickPrimaryTailnetIPv4.mockReturnValue(undefined); + + expect(() => buildGatewayConnectionDetails()).toThrow("SECURITY ERROR"); + expect(() => buildGatewayConnectionDetails()).toThrow("plaintext ws://"); + expect(() => buildGatewayConnectionDetails()).toThrow("wss://"); + }); + + it("allows ws:// for loopback addresses in local mode", () => { + loadConfig.mockReturnValue({ + gateway: { mode: "local", bind: "loopback" }, + }); + resolveGatewayPort.mockReturnValue(18789); + pickPrimaryTailnetIPv4.mockReturnValue(undefined); + + const details = buildGatewayConnectionDetails(); + + expect(details.url).toBe("ws://127.0.0.1:18789"); + }); }); describe("callGateway error details", () => { diff --git a/src/gateway/call.ts b/src/gateway/call.ts index 41dc07b53b..193433d8a2 100644 --- a/src/gateway/call.ts +++ b/src/gateway/call.ts @@ -16,7 +16,7 @@ import { type GatewayClientName, } from "../utils/message-channel.js"; import { GatewayClient } from "./client.js"; -import { pickPrimaryLanIPv4 } from "./net.js"; +import { isSecureWebSocketUrl, pickPrimaryLanIPv4 } from "./net.js"; import { PROTOCOL_VERSION } from "./protocol/index.js"; export type CallGatewayOptions = { @@ -134,6 +134,22 @@ export function buildGatewayConnectionDetails( ? "Warn: gateway.mode=remote but gateway.remote.url is missing; set gateway.remote.url or switch gateway.mode=local." : undefined; const bindDetail = !urlOverride && !remoteUrl ? `Bind: ${bindMode}` : undefined; + + // Security check: block ALL insecure ws:// to non-loopback addresses (CWE-319, CVSS 9.8) + // This applies to the FINAL resolved URL, regardless of source (config, CLI override, etc). + // Both credentials and chat/conversation data must not be transmitted over plaintext to remote hosts. + if (!isSecureWebSocketUrl(url)) { + throw new Error( + [ + `SECURITY ERROR: Gateway URL "${url}" uses plaintext ws:// to a non-loopback address.`, + "Both credentials and chat data would be exposed to network interception.", + `Source: ${urlSource}`, + `Config: ${configPath}`, + "Fix: Use wss:// for the gateway URL, or connect via SSH tunnel to localhost.", + ].join("\n"), + ); + } + const message = [ `Gateway target: ${url}`, `Source: ${urlSource}`, diff --git a/src/gateway/client.test.ts b/src/gateway/client.test.ts index d3e02850b4..4481d94645 100644 --- a/src/gateway/client.test.ts +++ b/src/gateway/client.test.ts @@ -86,6 +86,77 @@ function getLatestWs(): MockWebSocket { return ws; } +describe("GatewayClient security checks", () => { + beforeEach(() => { + wsInstances.length = 0; + }); + + it("blocks ws:// to non-loopback addresses (CWE-319)", () => { + const onConnectError = vi.fn(); + const client = new GatewayClient({ + url: "ws://remote.example.com:18789", + onConnectError, + }); + + client.start(); + + expect(onConnectError).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining("SECURITY ERROR"), + }), + ); + expect(wsInstances.length).toBe(0); // No WebSocket created + client.stop(); + }); + + it("handles malformed URLs gracefully without crashing", () => { + const onConnectError = vi.fn(); + const client = new GatewayClient({ + url: "not-a-valid-url", + onConnectError, + }); + + // Should not throw + expect(() => client.start()).not.toThrow(); + + expect(onConnectError).toHaveBeenCalledWith( + expect.objectContaining({ + message: expect.stringContaining("SECURITY ERROR"), + }), + ); + expect(wsInstances.length).toBe(0); // No WebSocket created + client.stop(); + }); + + it("allows ws:// to loopback addresses", () => { + const onConnectError = vi.fn(); + const client = new GatewayClient({ + url: "ws://127.0.0.1:18789", + onConnectError, + }); + + client.start(); + + expect(onConnectError).not.toHaveBeenCalled(); + expect(wsInstances.length).toBe(1); // WebSocket created + client.stop(); + }); + + it("allows wss:// to any address", () => { + const onConnectError = vi.fn(); + const client = new GatewayClient({ + url: "wss://remote.example.com:18789", + onConnectError, + }); + + client.start(); + + expect(onConnectError).not.toHaveBeenCalled(); + expect(wsInstances.length).toBe(1); // WebSocket created + client.stop(); + }); +}); + describe("GatewayClient close handling", () => { beforeEach(() => { wsInstances.length = 0; diff --git a/src/gateway/client.ts b/src/gateway/client.ts index dd5d4f1763..270ce2633e 100644 --- a/src/gateway/client.ts +++ b/src/gateway/client.ts @@ -21,6 +21,7 @@ import { type GatewayClientName, } from "../utils/message-channel.js"; import { buildDeviceAuthPayload } from "./device-auth.js"; +import { isSecureWebSocketUrl } from "./net.js"; import { type ConnectParams, type EventFrame, @@ -109,6 +110,26 @@ export class GatewayClient { this.opts.onConnectError?.(new Error("gateway tls fingerprint requires wss:// gateway url")); return; } + + // Security check: block ALL plaintext ws:// to non-loopback addresses (CWE-319, CVSS 9.8) + // This protects both credentials AND chat/conversation data from MITM attacks. + // Device tokens may be loaded later in sendConnect(), so we block regardless of hasCredentials. + if (!isSecureWebSocketUrl(url)) { + // Safe hostname extraction - avoid throwing on malformed URLs in error path + let displayHost = url; + try { + displayHost = new URL(url).hostname || url; + } catch { + // Use raw URL if parsing fails + } + const error = new Error( + `SECURITY ERROR: Cannot connect to "${displayHost}" over plaintext ws://. ` + + "Both credentials and chat data would be exposed to network interception. " + + "Use wss:// for the gateway URL, or connect via SSH tunnel to localhost.", + ); + this.opts.onConnectError?.(error); + return; + } // Allow node screen snapshots and other large responses. const wsOptions: ClientOptions = { maxPayload: 25 * 1024 * 1024, diff --git a/src/gateway/net.test.ts b/src/gateway/net.test.ts index 005f4aadcb..18d5fc14ef 100644 --- a/src/gateway/net.test.ts +++ b/src/gateway/net.test.ts @@ -2,6 +2,7 @@ import os from "node:os"; import { afterEach, describe, expect, it, vi } from "vitest"; import { isPrivateOrLoopbackAddress, + isSecureWebSocketUrl, isTrustedProxyAddress, pickPrimaryLanIPv4, resolveGatewayListenHosts, @@ -237,3 +238,42 @@ describe("isPrivateOrLoopbackAddress", () => { } }); }); + +describe("isSecureWebSocketUrl", () => { + describe("wss:// (TLS) URLs", () => { + it("returns true for wss:// regardless of host", () => { + expect(isSecureWebSocketUrl("wss://127.0.0.1:18789")).toBe(true); + expect(isSecureWebSocketUrl("wss://localhost:18789")).toBe(true); + expect(isSecureWebSocketUrl("wss://remote.example.com:18789")).toBe(true); + expect(isSecureWebSocketUrl("wss://192.168.1.100:18789")).toBe(true); + }); + }); + + describe("ws:// (plaintext) URLs", () => { + it("returns true for ws:// to loopback addresses", () => { + expect(isSecureWebSocketUrl("ws://127.0.0.1:18789")).toBe(true); + expect(isSecureWebSocketUrl("ws://localhost:18789")).toBe(true); + expect(isSecureWebSocketUrl("ws://[::1]:18789")).toBe(true); + expect(isSecureWebSocketUrl("ws://127.0.0.42:18789")).toBe(true); + }); + + it("returns false for ws:// to non-loopback addresses (CWE-319)", () => { + expect(isSecureWebSocketUrl("ws://remote.example.com:18789")).toBe(false); + expect(isSecureWebSocketUrl("ws://192.168.1.100:18789")).toBe(false); + expect(isSecureWebSocketUrl("ws://10.0.0.5:18789")).toBe(false); + expect(isSecureWebSocketUrl("ws://100.64.0.1:18789")).toBe(false); + }); + }); + + describe("invalid URLs", () => { + it("returns false for invalid URLs", () => { + expect(isSecureWebSocketUrl("not-a-url")).toBe(false); + expect(isSecureWebSocketUrl("")).toBe(false); + }); + + it("returns false for non-WebSocket protocols", () => { + expect(isSecureWebSocketUrl("http://127.0.0.1:18789")).toBe(false); + expect(isSecureWebSocketUrl("https://127.0.0.1:18789")).toBe(false); + }); + }); +}); diff --git a/src/gateway/net.ts b/src/gateway/net.ts index ee3c762d53..edeeb83547 100644 --- a/src/gateway/net.ts +++ b/src/gateway/net.ts @@ -396,3 +396,33 @@ export function isLoopbackHost(host: string): boolean { const unbracket = h.startsWith("[") && h.endsWith("]") ? h.slice(1, -1) : h; return isLoopbackAddress(unbracket); } + +/** + * Security check for WebSocket URLs (CWE-319: Cleartext Transmission of Sensitive Information). + * + * Returns true if the URL is secure for transmitting data: + * - wss:// (TLS) is always secure + * - ws:// is only secure for loopback addresses (localhost, 127.x.x.x, ::1) + * + * All other ws:// URLs are considered insecure because both credentials + * AND chat/conversation data would be exposed to network interception. + */ +export function isSecureWebSocketUrl(url: string): boolean { + let parsed: URL; + try { + parsed = new URL(url); + } catch { + return false; + } + + if (parsed.protocol === "wss:") { + return true; + } + + if (parsed.protocol !== "ws:") { + return false; + } + + // ws:// is only secure for loopback addresses + return isLoopbackHost(parsed.hostname); +} diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index c9e8143d39..04a1ae9e4d 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -2018,7 +2018,7 @@ description: test skill mode: "remote", auth: { token: "local-token-should-not-use" }, remote: { - url: "ws://remote.example.com:18789", + url: "wss://remote.example.com:18789", token: "remote-token-xyz789", }, }, @@ -2057,7 +2057,7 @@ description: test skill mode: "remote", auth: { token: "local-token-should-not-use" }, remote: { - url: "ws://remote.example.com:18789", + url: "wss://remote.example.com:18789", token: "remote-token", }, }, @@ -2094,7 +2094,7 @@ description: test skill gateway: { mode: "remote", remote: { - url: "ws://remote.example.com:18789", + url: "wss://remote.example.com:18789", password: "remote-pass", }, }, @@ -2132,7 +2132,7 @@ description: test skill gateway: { mode: "remote", remote: { - url: "ws://remote.example.com:18789", + url: "wss://remote.example.com:18789", password: "remote-pass", }, }, diff --git a/src/tui/gateway-chat.test.ts b/src/tui/gateway-chat.test.ts index 01707ffbe6..21aa681828 100644 --- a/src/tui/gateway-chat.test.ts +++ b/src/tui/gateway-chat.test.ts @@ -21,9 +21,15 @@ vi.mock("../infra/tailnet.js", () => ({ pickPrimaryTailnetIPv4, })); -vi.mock("../gateway/net.js", () => ({ - pickPrimaryLanIPv4, -})); +vi.mock("../gateway/net.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + pickPrimaryLanIPv4, + // Allow all URLs in tests - security validation is tested separately + isSecureWebSocketUrl: () => true, + }; +}); const { resolveGatewayConnection } = await import("./gateway-chat.js");