Skip to content

WebSocket upgrade requests are not IP-stamped (clientIp reads spoofable header) #778

Description

@vivek7405

Problem

A WebSocket WS(ws, req, ...) handler that calls clientIp(req) (e.g. to rate-limit a socket by IP) reads the spoofable inbound x-webjs-remote-ip header on BOTH runtimes. The WS upgrade path builds the handler's Request by copying all inbound headers and never (a) strips the inbound x-webjs-remote-ip copy nor (b) stamps the framework-trusted socket IP. On Bun it additionally never sets the out-of-band trusted-IP WeakMap for the WS path. So a client can send x-webjs-remote-ip: <fake> on the upgrade and clientIp trusts it.

This is pre-existing (the WS upgrade check runs before the fetch handler's stampRemoteIp, and was never stamped before), so it is NOT introduced or regressed by #756 / #773. It is the same spoof class #773 closed everywhere else (the SSR fetch path, the page-action body rebuild, the basePath rebuild), left open on the WS seam. Surfaced during the #773 round-3 security review of the listener IP-trust model.

Design / approach

Mirror the fetch path's IP-trust seam on the WS upgrade. The HTTP path already does the right thing: the node shell's toWebRequest strips the inbound header then stamps the real socket IP; the Bun shell's stampRemoteIp(req, srv) calls setTrustedRemoteIp(req, srv.requestIP(req)?.address || '') out of band (no Request clone, #756). The WS upgrade needs the equivalent on the request object it hands the WS handler.

Implementation notes (for the implementing agent)

  • Where to edit:
    • Bun: packages/server/src/listener-bun.js upgradeRequest(req, url) (around L254, called from the upgrade branch around L239). After building handlerReq, call handlerReq.headers.delete('x-webjs-remote-ip') then setTrustedRemoteIp(handlerReq, srv.requestIP(req)?.address || '') (setTrustedRemoteIp is already imported at L48; the upgrade branch has srv in scope, the same value stampRemoteIp at L310 uses).
    • Node: packages/server/src/websocket.js buildRequestFromUpgrade(req, url) (around L103). Strip the inbound x-webjs-remote-ip header from the built request and stamp req.socket.remoteAddress (the node upgrade has the raw http.IncomingMessage). Prefer setting the x-webjs-remote-ip header (the node convention) OR setTrustedRemoteIp (the IMPORTANT: Bun listener per-request overhead (Request clone + zlib bridge) erodes the win #756 WeakMap convention), matching whichever clientIp reads first (the WeakMap is authoritative on both).
  • Landmines:
    • clientIp (rate-limit.js) reads the WeakMap first (trustedRemoteIp), then the header. Stamping the WeakMap is the robust fix; if you only set the header, also strip any inbound copy or a client can still spoof.
    • The Bun WS adapter re-imports the route module per connection in dev; the stamp must happen per upgrade, not once.
    • Do NOT trust x-forwarded-for here unless trustProxy is set (same rule as the HTTP path).
  • Invariants: AGENTS.md invariant Corrected backtick errors #1 (the server-only trust boundary) and the IMPORTANT: Bun listener per-request overhead (Request clone + zlib bridge) erodes the win #756/perf: cut Bun listener per-request overhead (IP clone + compress bridge) #773 IP-trust model (a client-supplied x-webjs-remote-ip is NEVER trusted).
  • Tests + docs: add a cross-runtime assertion to test/bun/listener.mjs (it already exercises a WS echo over a real socket) that a WS upgrade carrying a spoofed x-webjs-remote-ip does not make clientIp return it; run on node AND bun. Update the listener-bun.js / websocket.js rows in packages/server/AGENTS.md.

Acceptance criteria

  • A WS upgrade with a spoofed x-webjs-remote-ip header does NOT make clientIp(req) inside the WS handler return the spoofed value, on node AND Bun.
  • The framework-trusted socket IP reaches clientIp inside a WS handler (not _anon_) on both runtimes.
  • A counterfactual proves the test fires (revert the strip/stamp, expect red).
  • Cross-runtime test in test/bun/listener.mjs; docs/AGENTS.md updated.

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions