Skip to content

feat(core): redact sensitive headers in the fetch log#1585

Open
cliffhall wants to merge 1 commit into
v2/mainfrom
v2/1561-redact-headers
Open

feat(core): redact sensitive headers in the fetch log#1585
cliffhall wants to merge 1 commit into
v2/mainfrom
v2/1561-redact-headers

Conversation

@cliffhall

Copy link
Copy Markdown
Member

Closes #1561

Summary

Redacts sensitive header values in createFetchTracker before any request/response entry is recorded. Authorization, Cookie, Set-Cookie, Proxy-Authorization, X-Api-Key, and x-mcp-remote-auth are replaced with a [REDACTED] marker on both request and response headers.

Recorded fetch entries flow to the in-memory log (visible in the Network UI), the pino logger, and persisted session storage — none of those sinks should ever see a live bearer token or session cookie, especially once OAuth tokens ride these requests.

Details

  • New SENSITIVE_HEADERS set + REDACTED_HEADER_VALUE constant + exported redactSensitiveHeaders() in core/mcp/fetchTracking.ts.
  • Applied to request headers (success and error paths) and response headers, before the entry reaches any sink.
  • Comparison is case-insensitive (HTTP header names are case-insensitive); original key casing is preserved so the entry still shows what the client sent.
  • The live outbound request is untouched — redaction affects only the recorded entry.

Re-implements the fetch-log redaction slice of PR #1510 (reference head 33fac3f), scoped per tracking issue #1579 Wave 1.

Testing

  • clients/web unit tests for fetchTracking pass (redaction on request, error path, response; redactSensitiveHeaders case-insensitivity + non-mutation).
  • Per-file coverage gate (test:coverage) passes ≥90 on all four dimensions.
  • validate:web and test:integration (799/799) green.

🤖 Generated with Claude Code

https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX

Redact Authorization, Cookie, Set-Cookie, Proxy-Authorization, X-Api-Key,
and x-mcp-remote-auth to [REDACTED] in createFetchTracker before any
request/response entry reaches the in-memory log, pino logger, or
persisted session storage. Comparison is case-insensitive; original key
casing is preserved and the live outbound request is untouched.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01S3fTN8H3R8YV4yUGvZjYnX
@cliffhall cliffhall added the v2 Issues and PRs for v2 label Jul 1, 2026
@cliffhall cliffhall linked an issue Jul 1, 2026 that may be closed by this pull request
@cliffhall

Copy link
Copy Markdown
Member Author

@claude review

@claude

claude Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @cliffhall's task in 1m 0s —— View job


Code Review: redact sensitive headers in the fetch log

Reviewed core/mcp/fetchTracking.ts and its tests against origin/v2/main. This is a clean, well-targeted change — nice work. No blocking issues.

Strengths

  • Correct placement. redactSensitiveHeaders() is applied to the raw maps before the entry is constructed, on all three code paths — request headers (success + error), and response headers. Since every sink (in-memory list, pino logger, session storage) reads the same FetchRequestEntryBase, redacting at the entry boundary covers them all in one spot.
  • Live request untouched. Redaction operates on a separate rawRequestHeaders copy, not init/input, so the outbound bearer/cookie is preserved. The test at fetchTracking.test.ts:311-315 asserts exactly this — good defensive coverage.
  • Case-insensitive + non-mutating + casing-preserving. key.toLowerCase() lookup against a lowercased set, returning a fresh object with original keys. Both properties are directly tested (:363-382).
  • Header set is well chosen and the inline comment explaining x-mcp-remote-auth (the inspector backend's own bearer from createRemoteFetch) is a helpful pointer.
  • Tests are thorough and coverage-gate-clean.

Consideration (scope, not a bug)

Request/response bodies are still recorded verbatim. requestBody / responseBody (types.ts:254,258) flow to the same sinks unredacted. This matters specifically for OAuth, which the PR body calls out as the motivation:

  • The token endpoint request body carries client_secret, code, and refresh_token (form-encoded).
  • The token endpoint response body carries access_token and refresh_token (JSON).

So a bearer redacted from the Authorization header is still fully present in the body of the exchange that minted it. I assume this is deliberately out of scope for this slice (Wave 1 of #1579 is headers) — just flagging so body redaction is explicitly tracked as a follow-up rather than assumed covered. If there isn't an issue for it yet, worth filing one.

Minor, related: tokens that ride in the URL query string (url is stored verbatim) are also unredacted — same follow-up bucket.

Nits

  • None worth changing. The SENSITIVE_HEADERS list is reasonable; www-authenticate is a challenge rather than a secret, so its omission is fine.

Verdict: LGTM for the header-redaction scope. Recommend confirming a tracking issue exists for body/URL redaction so the "OAuth tokens ride these requests" motivation is fully closed out later.

@cliffhall

Copy link
Copy Markdown
Member Author

📋 Smoke test & AGENTS.md audit for this PR: #1561 (comment) (posted on the linked issue #1561).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

v2 Issues and PRs for v2

Projects

None yet

Development

Successfully merging this pull request may close these issues.

core: redact sensitive headers in the fetch log

1 participant