[mostafaseyedan]

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.

[Sanjay Singh (san360)]

Thank you mostafaseyedan for this one, and for the very clear write-up! The \b-between-_-and-a-letter explanation and the doubled-quote example made the problem easy to follow, and it's a real gap: compound names like client_secret, refresh_token and AZURE_CLIENT_SECRET= went to the model unredacted. We appreciate the false-positive checks in the description and the prose test too.

What we verified

• Your three new behavioural tests fail on current main and pass with the PR; the full unit suite passes on main + this PR (typecheck, lint and knip are clean, and the bundle stays within budget).
• On a corpus of ~35 strings, the PR redacts every new case (client_secret, refresh_token, app_token, db_password, AZURE_CLIENT_SECRET=…, id_token: …, ctx7sk-…, doubled-quote JSON) and leaves the prose, tokenizer, max_tokens, camelCase inputTokens, token_url and secret_name cases unchanged.
• With real Claude Code and Copilot Chat sessions in throwaway repos (fake credentials in the prompts), transcript text that goes through spotlight() no longer carries any of the fake values; on main all of them got through. The dashboard is unchanged on every page, as expected for an egress-only change.

Suggestions (details and a ready-to-apply version inline)

1. The motivating settings.local.json case still gets through. Inside a JSON file the doubled quotes are themselves backslash-escaped, so the file contains \"\"client_secret\"\":\"\"…\"\", not ""client_secret"":""…"". Context Health sends the raw file text (getContextReviewPayload in src/core/analyzer-config.ts reads it with readFileSync), so neither pattern matches there. We reproduced it with a .claude/settings.local.json holding two Bash(curl …) allow rules: both secret values still reached the review prompt with the PR. The same shape appears in chat tool results, which are JSON.stringify'd before redaction (textResult in src/mcp/tools.ts), so \"client_secret\":\"…\" inside a string isn't caught either. Allowing an optional backslash before each quote fixes both.

2. An empty value can swallow the fields after it. With ["']{1,2}, {"token": "", "user": "bob", "note": ""} becomes {"token": ""[REDACTED]""}: the empty "" is taken as a doubled opening quote and everything up to the next "" is masked. It only over-redacts, but it removes non-secret context and breaks the JSON shape. Giving the doubled-quote form its own pattern (doubled quotes around the key and the value) avoids this.

3. A small one: a lookbehind instead of the compound prefix. \b(?:[A-Za-z0-9]+[_-])* is retried from every word boundary inside a long hyphen- or underscore-joined run, so the cost grows quadratically with the run length (about 2.3 s for 50 KB of a-a_ on our machine versus a few ms on main). (?<![A-Za-z0-9]) in front of the key name matches the same cases (the client_ part simply stays outside the match, so the replacement output is identical) and stays linear. Real text rarely has such long runs, so this is minor.

With all three applied, our runs give identical output to the PR on every case above, catch the settings-file and stringified forms, and keep the empty-value JSON intact. Since redactSecrets applies the patterns as a .replaceAll chain, the new doubled-quote pattern also needs one line in it, just before QUOTED_SECRET_ASSIGNMENT (that line isn't part of the diff, so here it is):

    .replaceAll(DOUBLED_QUOTE_SECRET_ASSIGNMENT, '$1$2$3[REDACTED]$3')
    .replaceAll(QUOTED_SECRET_ASSIGNMENT, '$1$2$3[REDACTED]$3')

And two tests that pin the new behaviour (they fail on the current PR and pass with the changes):

  it('keeps the fields after an empty secret value', () => {
    const text = '{"token": "", "user": "bob", "note": ""}';
    expect(redactSecrets(text)).toBe(text);
  });

  it('redacts a doubled-quote rule as it is stored in settings.local.json', () => {
    const settings = JSON.stringify({ permissions: { allow: [
      'Bash(curl -d \'{""client_secret"":""8bavjPwL9uoXDp""}\')',
    ] } }, null, 2);
    const out = redactSecrets(settings);
    expect(out).not.toContain('8bavjPwL9uoXDp');
    expect(JSON.parse(out)).toBeTruthy();
  });

Next steps: the CI runs on this branch didn't start (the runs from September show no jobs), so a maintainer will re-run them once the branch is updated. Separately, for workspaces opened only in Claude Code, Context Health reaches these files once the workspace root resolves, which is what your #273 addresses.

Thanks again for tightening this up. Redaction is one of those places where every caught shape matters, and this is a clear improvement.

[Sanjay Singh (san360)]

Inside a JSON file the doubled quotes are escaped again, so .claude/settings.local.json contains \"\"client_secret\"\":\"\"…\"\". Context Health sends that raw text, and neither pattern matches it, so the two curl-rule secrets in our test file still reached the review prompt. A related case: {"token": "", "user": "bob", "note": ""} becomes {"token": ""[REDACTED]""} because the empty "" is read as a doubled opening quote.

One option that handles both: a lookbehind for the key name (same matches, and it stays linear on long a-b-c… runs), optional backslashes before quotes, and a separate pattern for the doubled form. It needs one extra .replaceAll(DOUBLED_QUOTE_SECRET_ASSIGNMENT, '$1$2$3[REDACTED]$3') before the QUOTED_SECRET_ASSIGNMENT line (see the review body).

Suggested change

	const SECRET_KEY_NAME = String.raw`(?:[A-Za-z0-9]+[_-])*(?:api[_-]?key|access[_-]?key|secret|token|password|passwd|credentials?)`;
	const QUOTED_SECRET_ASSIGNMENT = new RegExp(
	String.raw`\b(${SECRET_KEY_NAME})(["']{0,2}\s*[:=]\s*)(["']{1,2})((?:(?!\3).){4,512})\3`, 'gi');
	const JWT = /\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b/g;
	const AUTH_HEADER = /\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{16,512}/gi;
	const KEY_VALUE_ASSIGNMENT = /\b(api[_-]?key|access[_-]?key|secret|token|password|passwd|credentials?)(["']?\s*[:=]\s*)(["']?)[^\s"'`;,]{8,512}\3/gi;
	const KEY_VALUE_ASSIGNMENT = new RegExp(
	String.raw`\b(${SECRET_KEY_NAME})(["']{0,2}\s*[:=]\s*)(["']{0,2})[^\s"'\`;,]{8,512}\3`, 'gi');
	// The key name may follow `_` or `-` (client_secret, refresh_token) but not a letter or digit.
	// Quotes may be backslash-escaped, as in raw JSON files and JSON.stringify output.
	const SECRET_KEY_NAME = String.raw`(?<![A-Za-z0-9])(api[_-]?key|access[_-]?key|secret|token|password|passwd|credentials?)`;
	// JSON escaped by doubling its quotes inside a shell command: ""client_secret"":""value"".
	const DOUBLED_QUOTE_SECRET_ASSIGNMENT = new RegExp(
	String.raw`${SECRET_KEY_NAME}((?:\\?"){2}\s*:\s*)((?:\\?"){2})((?:(?!\3).){4,512})\3`, 'gi');
	const QUOTED_SECRET_ASSIGNMENT = new RegExp(
	String.raw`${SECRET_KEY_NAME}((?:\\?["'])?\s*[:=]\s*)(\\?["'])((?:(?!\3).){4,512})\3`, 'gi');
	const JWT = /\beyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]{8,}\b/g;
	const AUTH_HEADER = /\b(Bearer|Basic)\s+[A-Za-z0-9._~+/=-]{16,512}/gi;
	const KEY_VALUE_ASSIGNMENT = new RegExp(
	String.raw`${SECRET_KEY_NAME}(["']?\s*[:=]\s*)(["']?)[^\s"'\`;,]{8,512}\3`, 'gi');

[Sanjay Singh (san360)]

If the patterns change as suggested, the header could describe them like this:

Suggested change

	* Key names are matched with an optional compound prefix, so `client_secret`
	* and `refresh_token` are caught as well as bare `secret` and `token`. Quote
	* runs of one or two characters are accepted, because JSON embedded in a
	* shell command is commonly escaped by doubling its quotes.
	* Key names may follow `_` or `-`, so `client_secret` and `refresh_token` are
	* caught as well as bare `secret` and `token`. JSON embedded in a shell command
	* is commonly escaped by doubling its quotes, so that form has its own pattern.

[mostafaseyedan]

Thank you so much, Sanjay Singh (@san360), for this very careful review! The settings.local.json escaping and the empty-value case were great catches, and I really appreciate the ready-to-apply version.

I applied all three suggestions, the extra .replaceAll line, both tests and the new header text. I also moved the "Quoted assignment whose value may contain spaces" comment down so it sits above QUOTED_SECRET_ASSIGNMENT again.

Checked locally against the previous commit of this branch:

• .claude/settings.local.json shape (JSON.stringify of a doubled-quote curl rule): the secret leaked before, now it is redacted and the output still parses as JSON.
• {"token": "", "user": "bob", "note": ""}: was {"token": ""[REDACTED]""}, now unchanged.
• 50 KB of a-a_ followed by token=: 1159 ms before, 1 ms now.
• redact-secrets.test.ts 27/27, typecheck clean, lint has no new warnings.

Thanks again for the time you put into this!

[Sanjay Singh (san360)]

Thank you mostafaseyedan for the quick and careful update, and for the kind words!

We re-checked 438f20c on top of current main:

• The change matches what we suggested; the only difference is the moved "Quoted assignment" comment, which reads better where you put it.
• redact-secrets.test.ts passes 27/27, and the full unit suite, typecheck, lint (no new warnings), knip, build and bundle size are all green.
• The raw settings.local.json form of the doubled-quote curl rules, JSON.stringify'd text and the stringified \"password\": \"…\" form are all redacted now. {"token": "", "user": "bob", "note": ""} is left intact. Our ~35-string corpus gives the expected output on every case, and nothing that main redacts is lost.
• Timing matches your numbers: a-a_×12 500 followed by token=… takes about 2 ms, and long hyphen-joined runs are now as fast as on main.
• With fresh Claude Code and Copilot Chat sessions holding fake credentials, nothing reaches the prompt any more, either through transcript snippets or through the Context Health review of a settings file. The dashboard is unchanged on every page.

This looks good to merge from our side once CI passes. The workflow runs for the new commit are waiting for a maintainer to approve them, and that's the only remaining step.

Thanks again for finding this and seeing it through!