Skip to content

Redact compound secret key names and doubled-quote JSON - #274

Open
mostafaseyedan wants to merge 1 commit into
microsoft:mainfrom
mostafaseyedan:fix/redact-compound-secret-keys
Open

mostafaseyedan wants to merge 1 commit into
microsoft:mainfrom
mostafaseyedan:fix/redact-compound-secret-keys

Conversation

@mostafaseyedan

Copy link
Copy Markdown

Problem

redactSecrets masks credential-shaped text before it is sent to a language model — by callLlm/callLlmJson via redactMessages, so it guards every AI feature. Two common shapes pass through it unchanged.

1. Compound key names. Both assignment patterns anchor the key with \b:

/\b(api[_-]?key|access[_-]?key|secret|token|password|passwd|credentials?)/gi

\b does not match between _ and a letter, so client_secret never matches while bare secret does:

MISS   "client_secret":"abcdefgh12345678"
match  "secret":"abcdefgh12345678"
MISS   "app_token":"abcdefgh12345678"
match  "x-api-key":"abcdefgh12345678"     <- hyphen gives a boundary, underscore does not
MISS   "refresh_token":"abcdefgh12345678"

2. Doubled-quote JSON. The quote group accepts exactly one quote character. JSON embedded in a shell command is routinely escaped by doubling its quotes — which is how Claude Code records a curl invocation in a permission allow rule:

Bash(curl -d '{""client_id"":""…"",""client_secret"":""…""}')

(["']) captures one ", the content class then immediately hits the second ", and the match fails. Neither pattern fires.

These combine badly in practice. A .claude/settings.local.json accumulates Bash(curl …) allow rules verbatim, so OAuth client secrets end up stored in exactly the shape that defeats both patterns, and Context Health reads those files and sends them to the model.

3. Separately, a Context7 key (ctx7sk-…) is not reachable by the existing sk- pattern, because the preceding digit suppresses the word boundary.

Fix

  • Factor the key-name alternation into SECRET_KEY_NAME with an optional (?:[A-Za-z0-9]+[_-])* prefix, so qualified names are covered.
  • Accept quote runs of one or two characters in both patterns; the backreference keeps the closing run symmetrical.
  • Add ctx7sk- alongside the other vendor prefixes.

client_id is deliberately still not redacted — it is an identifier rather than a credential, and adding it would start redacting ordinary config.

Testing

Four tests added. Verified against a real 122-rule settings.local.json: two credential-shaped strings survived redaction before, none do after, 18 redaction markers inserted.

False-positive checked — these are still returned unchanged:

The token bucket algorithm smooths bursty traffic.
Set a secret in your CI provider, then reference it.
const tokenizer = new Tokenizer();
// TODO: rotate the access_key next sprint

All 25 tests in redact-secrets.test.ts pass (21 pre-existing, unmodified). npm run typecheck and npm run lint clean (0 errors). The 7 github-app-analytics failures are pre-existing on main.

redactSecrets masks transcript text before it is sent to a language
model, but two common shapes slipped through it unchanged.

Both assignment patterns anchor the key name with \b, which does not
match between an underscore and a letter. `client_secret`,
`refresh_token`, `app_token` and `db_password` therefore never matched,
while the bare `secret` and `token` forms did. Match an optional
compound prefix so the qualified names are covered too.

The quote group also accepted a single quote character only. JSON
embedded in a shell command is routinely escaped by doubling its quotes,
as Claude Code does when recording a curl invocation in a permission
allow rule, so `""client_secret"":""…""` matched neither pattern.
Accept quote runs of one or two characters.

Add the Context7 key prefix alongside the other vendor tokens; its
`ctx7sk-` form is not reachable by the existing `sk-` pattern because
the leading digit suppresses the word boundary.

Checked against a real settings file: two credential-shaped strings
survived redaction before, none do now, and prose mentioning "token",
"secret" or "password" is still left intact.
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