Skip to content

fix: keep credential headers out of connection log metadata - #440

Open
sgzrov wants to merge 1 commit into
replit:mainfrom
sgzrov:fix/separate-logging-extras
Open

sgzrov wants to merge 1 commit into
replit:mainfrom
sgzrov:fix/separate-logging-extras

Conversation

@sgzrov

@sgzrov sgzrov commented Oct 31, 2026 •

Copy link
Copy Markdown

Why

Session cookies and bearer tokens end up in server logs.

WebSocketServerTransport stores each connection's upgrade headers in connection.extras, and WebSocketConnection.loggingMetadata attaches those extras to every log line for the connection (added in #246 so headers are visible in logs). The headers are only filtered for sec-*, so cookie and authorization are logged as-is. This has been the case since #246.

The obvious fix, dropping those headers from the extras, breaks something else: #406 passes the same extras to handshake validation so an app can authenticate from the upgrade headers, and it promised the default extras stay unchanged.

So the extras have to keep the real values, and only what gets logged can change. This PR does exactly that: the connection keeps extras as-is for validation and gets a separate loggingExtras, a copy with the cookie, authorization, and proxy-authorization values replaced by [redacted], and only loggingExtras is attached to log lines.

What changed

  • WebSocketConnection gets a private loggingExtras value, and loggingMetadata attaches only that. The connection extras are never logged directly and are untouched for validation (transport/server.ts still passes session.conn.extras at both validation sites).
  • loggingExtras is computed once in the WebSocketConnection constructor: a copy of the extras with the cookie, authorization, and proxy-authorization header values (the credential request headers per the Fetch spec) replaced by [redacted], every other key unchanged. A headers value that is not a plain object is passed through as-is. Every caller constructing a WebSocketConnection gets this; no signature changes.
  • WebSocketServerTransport is unchanged.
  • README: two sentences under the existing extras factory section.
  • Adds one test to ws.test.ts, next to the Expose WebSocket upgrade context to handshake validation #406 test (details under Tests).

Caddy takes the same approach: its log marshaler redacts these headers while the request headers stay intact.

Versioning

  • Breaking protocol change
  • Breaking ts/js API change

No protocol or API change. The one behavior change is in the default log output: cookie, authorization, and proxy-authorization values now show [redacted]. Anyone correlating logs by those values loses that; all other headers are still logged, as in #246.

Tests

Ran locally on Node 22.16.0, on top of main (b154136):

  • Full suite (npm run test:single): 816 passed, 1 skipped, across 37 files. Clean main is 815 passed.
  • npm run check passes: TypeScript, Prettier, and ESLint.
  • New test in transport/impls/ws/ws.test.ts: a client connects with cookie, authorization, and user-agent upgrade headers. The handshake validator receives the real values through connectionExtras, the connection's log metadata shows [redacted] for the two credential headers and keeps user-agent, and conn.extras is untouched.
  • Red proof: with connection.ts reverted to main, the new test fails with the real cookie in log metadata; the existing Expose WebSocket upgrade context to handshake validation #406 test keeps passing both ways.

Scope

  • Only cookie, authorization, and proxy-authorization are redacted. Any other header, and any other key in custom extras, is logged as before.
  • These values have been in server logs since add conn extras to logging metadata #246, so existing logs may need purging.

@sgzrov
sgzrov requested a review from a team as a code owner October 31, 2026 06:30
@sgzrov
sgzrov requested review from pr1m8 and removed request for a team October 31, 2026 06:30
@sgzrov
sgzrov force-pushed the fix/separate-logging-extras branch 2 times, most recently from 88a865e to d22b42b Compare October 31, 2026 06:58
The WebSocket connection extras do two jobs: they are passed to handshake validation (so an app can authenticate from the upgrade headers, see replit#406) and they are attached to every log line for the connection (since replit#246). The default extras contain the upgrade headers, so a client's session cookie or bearer token was written to the server logs.

Give WebSocketConnection a separate loggingExtras value that is the only thing attached to log metadata: a copy of the extras with the cookie, authorization, and proxy-authorization headers replaced by [redacted], computed once in the constructor. The extras themselves are untouched, so validation still sees the real headers. No signature changes. Adds a test in ws.test.ts for both halves and a README note.

This branch has not been deployed

No deployments
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.

1 participant