From 4d80aa0a8fdf4f42c7e54db68427c5483701b2bd Mon Sep 17 00:00:00 2001 From: Dave Jong Date: Wed, 12 Aug 2026 15:33:43 +0200 Subject: [PATCH] egress: stop screening WebSocket egress (hostname-only check over-promised) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WebSocket constructor is synchronous, so the only screen possible inline is a textual hostname match — which can't offer the DNS-resolution guarantee the fetch and node:http/https egress paths provide. A connection-pinning dispatcher could close that, but the server-side attacker-controlled-WebSocket sink is rare, and a partial hostname-only check over-promises the SSRF control. Remove the WebSocket wrapper rather than ship a screen weaker than the one the docs imply. Outbound SSRF screening remains on fetch + node:http/https (node path pinned, fetch resolve-and-screen). The former WS test is replaced with one that pins the decision so a half-guard can't be reintroduced silently. Co-Authored-By: Claude Opus 4.8 --- src/protect/egress.js | 31 +++++--------------------- tests/protect/egress-followups.test.ts | 26 ++++++++++----------- 2 files changed, 18 insertions(+), 39 deletions(-) diff --git a/src/protect/egress.js b/src/protect/egress.js index 62eb006..fc0336e 100644 --- a/src/protect/egress.js +++ b/src/protect/egress.js @@ -145,31 +145,12 @@ export async function installEgressGuard({ shouldBlock, onBlock, dnsScreen = tru } } - // 3. global WebSocket — ws:// / wss:// egress that never touches fetch or node:http. - const OriginalWS = globalThis.WebSocket; - if (typeof OriginalWS === 'function' && !OriginalWS.__patchstackGuarded) { - const GuardedWS = new Proxy(OriginalWS, { - construct(target, args, newTarget) { - const url = String(args?.[0] ?? ''); - let host = null; - try { - host = new URL(url).hostname; - } catch { - host = null; - } - if (block(url, host, 'WEBSOCKET')) { - throw new Error(`Patchstack blocked an outbound WebSocket to a disallowed address: ${host ?? url}`); - } - return Reflect.construct(target, args, newTarget); - }, - }); - OriginalWS.__patchstackGuarded = true; // marker on the original guards against double-wrap - globalThis.WebSocket = GuardedWS; - restores.push(() => { - if (globalThis.WebSocket === GuardedWS) globalThis.WebSocket = OriginalWS; - delete OriginalWS.__patchstackGuarded; - }); - } + // WebSocket egress is intentionally NOT screened. The WebSocket constructor is synchronous, so + // the only check possible inline is a textual hostname match — which can't offer the + // DNS-resolution guarantee the fetch and node:http/https paths give (a name that resolves to an + // internal address would pass). A connection-pinning dispatcher could close that, but the + // server-side, attacker-controlled-WebSocket sink is rare, and a partial hostname-only check + // over-promises the control. Outbound SSRF screening covers fetch + node:http/https. return () => { for (const restore of restores) { diff --git a/tests/protect/egress-followups.test.ts b/tests/protect/egress-followups.test.ts index 1c1aafc..507e47c 100644 --- a/tests/protect/egress-followups.test.ts +++ b/tests/protect/egress-followups.test.ts @@ -1,8 +1,8 @@ import { describe, expect, it } from 'vitest'; import { createProtection } from '../../src/protect/runtime.js'; -// Egress hardening follow-ups: IPv6 host handling on the node:http path, WebSocket screening, -// and the allowHosts allowlist overriding an internal-host block. +// Egress hardening follow-ups: IPv6 host handling on the node:http path, the deliberate +// non-screening of WebSocket egress, and the allowHosts allowlist overriding an internal-host block. async function withEgress(opts: any, fn: (p: any) => Promise) { const origFetch = globalThis.fetch; @@ -40,21 +40,19 @@ describe('egress — node:http IPv6 hosts', () => { }); }); -describe('egress — WebSocket screening', () => { - it('blocks a ws:// connection to an internal host and restores the global on uninstall', async () => { +describe('egress — WebSocket is intentionally not screened', () => { + // The WebSocket constructor is synchronous, so only a textual hostname check is possible inline — + // which can't match the DNS-resolution guarantee the fetch / node:http paths give. Rather than ship + // a partial hostname-only guard that over-promises the control, WebSocket egress is left unwrapped. + // This test pins that decision so a half-guard can't be reintroduced silently. + it('does not wrap the global WebSocket', async () => { if (typeof globalThis.WebSocket !== 'function') return; // runtime without global WebSocket + const original = globalThis.WebSocket; await withEgress({ allowHosts: [] }, async () => { - let blocked = false; - try { - // eslint-disable-next-line no-new - new WebSocket('ws://169.254.169.254/'); - } catch (e) { - blocked = /Patchstack blocked/.test(String(e)); - } - expect(blocked).toBe(true); + expect(globalThis.WebSocket).toBe(original); // unchanged while the guard is active + expect((globalThis.WebSocket as any).__patchstackGuarded).toBeUndefined(); }); - // After uninstall the guard marker is gone (global restored). - expect((globalThis.WebSocket as any)?.__patchstackGuarded).toBeUndefined(); + expect(globalThis.WebSocket).toBe(original); }); });