Skip to content

egress: stop screening WebSocket egress (hostname-only check over-promised the control) - #113

Merged
patchstackdave merged 1 commit into
mainfrom
egress/drop-websocket-screening
Aug 12, 2026
Merged

egress: stop screening WebSocket egress (hostname-only check over-promised the control)#113
patchstackdave merged 1 commit into
mainfrom
egress/drop-websocket-screening

Conversation

@patchstackdave

Copy link
Copy Markdown
Contributor

What

Removes the WebSocket egress wrapper. Outbound SSRF screening now covers fetch + node:http/https only (node path pinned, fetch resolve-and-screen).

Why

The WebSocket constructor is synchronous, so the only screen possible inline is a textual hostname match — it can't perform the DNS-resolution check the fetch and node:http paths do (a hostname that resolves to an internal address would pass a textual check). Bringing WebSocket to parity would require a connection-pinning dispatcher, but:

  • the server-side, attacker-controlled-WebSocket URL sink is rare (real attacker-URL SSRF sinks — webhook senders, URL unfurlers, image proxies, OIDC discovery — live on fetch/node:http), and
  • a partial, hostname-only guard over-promises the SSRF control relative to what the other transports guarantee.

So rather than keep a half-guard (or ship a runtime-dependent dispatcher for a rare sink), we remove it and screen only the transports we can screen properly. Docs updated to match (no transport claimed that isn't screened).

Tests

The former WebSocket test (which asserted a literal internal host was blocked) is replaced by one that pins the decision — it asserts the guard leaves the global WebSocket unwrapped — so a half-guard can't be reintroduced silently. Full suite green (647 passing), typecheck clean.

…mised)

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 <noreply@anthropic.com>
@coderbuds

coderbuds Bot commented Aug 12, 2026

Copy link
Copy Markdown

Intentionally removes WebSocket screening to avoid misleading partial security guarantees

🎯 Quality: 100% Elite · 📦 Size: Small

📈 This month: Your 49th PR — above team average · Averaging Elite

See how your team is trending →

@patchstackdave

Copy link
Copy Markdown
Contributor Author

/review

1 similar comment
@patchstackdave

Copy link
Copy Markdown
Contributor Author

/review

@patchstackdave
patchstackdave merged commit 0389aa9 into main Aug 12, 2026
4 checks passed
@patchstackdave
patchstackdave deleted the egress/drop-websocket-screening branch August 12, 2026 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants