chore: unify secret-redaction key matching in one shared helper#290
Merged
Colin Francis (colifran) merged 2 commits intoJul 13, 2026
Merged
Conversation
Three independent secret-redaction implementations had drifted apart: - cli.tsx: api-key|authorization|bearer|token|secret|password - agent/index.ts: api-key|authorization|bearer|password|secret|token|user_id - mcp-runtime.ts: token|secret|password|authorization|api-key|cookie Because the term lists differed, a key redacted on one path (e.g. user_id in an OpenRouter body, or cookie in MCP output) could leak through another. Redaction is a security boundary; it must not depend on which code path happens to handle a value. Consolidate into a single exported isSecretLikeKey plus a shared SECRET_KEY_PATTERN_SOURCE in diagnostics.ts (the documented, well-tested redaction module), using the union of all previous terms. Route cli.tsx, mcp-runtime.ts, and the sanitizeOpenRouterResponseBody JSON-key regex through it. Add tests asserting the unified predicate catches every term (including the ones previously matched by only one path) and that sanitizeOpenRouterResponseBody redacts user_id/cookie while preserving non-secret fields. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jul 12, 2026
Colin Francis (colifran)
approved these changes
Jul 13, 2026
Colin Francis (colifran)
left a comment
Contributor
There was a problem hiding this comment.
Nice Greg Land (@bikeusaland)! This was on my radar as well. Thanks for the PR!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Three independent secret-redaction implementations had drifted apart, each matching a different set of secret-bearing key terms:
cli.tsxisSecretLikeKeyapi-key,authorization,bearer,token,secret,passwordagent/index.tssanitizeOpenRouterResponseBody(inline)api-key,authorization,bearer,password,secret,token,user_idmcp-runtime.tsisSecretLikeKeytoken,secret,password,authorization,api-key,cookieBecause the lists differed, a value redacted on one path could leak through another — e.g. a
user_idfield is redacted in an OpenRouter response body but not in MCP tool output, and acookiefield is redacted in MCP output but not in the CLI diagnostics path. Redaction is a security boundary; whether a credential gets hidden should not depend on which code path happens to handle it.Fix
Consolidate into a single exported
isSecretLikeKeyplus a sharedSECRET_KEY_PATTERN_SOURCEindiagnostics.ts— already the documented, well-tested redaction module (test/redaction.test.ts). The unified term list is the union of all three:All three consumers now route through it:
cli.tsx— imports the shared predicate, local copy removed.connectors/mcp-runtime.ts— imports the shared predicate, local copy removed.agent/index.tssanitizeOpenRouterResponseBody— builds its JSON-key regex fromSECRET_KEY_PATTERN_SOURCE, so the body sanitizer and the key predicate can never drift again.Net effect: every path now redacts the strict superset of what any path did before — no key that was previously hidden is now exposed.
Tests
isSecretLikeKeycatches every term (includinguser_idandcookie, previously matched by only one path each), case-insensitively, and does not flag benign keys.sanitizeOpenRouterResponseBodyredactsuser_id/cookievalues while preserving non-secret fields.Full gate passes:
typecheck,lint:check,format:check, test suite (213 tests, +9 new).Scope note
This PR intentionally unifies only the secret-key predicate. The related
isRecordduplication (and the array-vs-record nuance informatToolArgs) touches ~40 stream-parsing call sites and is left for a separate change to keep this security fix small and low-risk.🤖 Generated with Claude Code
Related PRs
Part of a batch from a single codebase-review pass. Independent,
main-based branches that can merge in any order.Security
Correctness
Feature
(This PR is #290.)