diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7ab80c751..748de0bf3 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -30,6 +30,7 @@ - Fixed a default (no custom `replacement`) `mode: "replace"` regex that cannot escape a 1–2 byte match (e.g. `.`, `[\s\S]`, `[\s\S]{2}`) still risking an unredacted round-trip: the previous key-derived same-length fallback marker was returned without checking it against the matched value, and since that marker is drawn from an alphabet the regex has already proven to match exhaustively, a real 1–2 byte secret that happened to equal it would ship to the provider unchanged. Such regex entries are now rejected outright — dropped with a warning when loaded from `secrets.yml`, dropped silently as a construction-time backstop otherwise — since no same-length marker can be guaranteed distinct from every possible match once the regex is proven to match every candidate in that alphabet. - Fixed a plain secret's `friendlyName` being accepted as a placeholder prefix when it was a lowercase or punctuated variant of the secret's own value (e.g. `friendlyName: "github_pat_abc123"` for a secret of the same content): the collision check compared the already-sanitized (uppercased, alphanumeric-only) friendly name against the secret's raw, case-sensitive value, so a case/punctuation variant slipped through and stamped most of the secret into every placeholder (`#GITHUBPATABC123_…#`). The secret value is now sanitized the same way before the comparison. - Fixed a regex-discovered secret's `friendlyName` collision check testing the already-sanitized (uppercased, separator-stripped) label against the configured regex instead of the label as written, so a case-sensitive or punctuated pattern (e.g. `tok_[a-z0-9]+`) missed a `friendlyName` that was itself a live match for that regex (e.g. `"tok_abc123"`), stamping the matched token — minus separators — into every placeholder (`#TOKABC123_…#`). The regex check now runs against the raw, pre-sanitization label, matching how the regex would actually encounter that text verbatim. +- Fixed the forged friendly-name-alias placeholder guard missing a case- or flag-variant occurrence of a regex-discovered secret: it only checked the dropped alias prefix against exact previously-discovered secret strings, so a differently-cased match of a case-insensitive pattern (e.g. `content: "tok[a-z0-9]+", flags: "i"` discovering lowercase `tokabc123`) never landed in the exact-match set under its uppercase form, letting a forged `#TOKABC123_#` be waved through as already-redacted and leave `TOKABC123` provider-visible. The guard now also tests the dropped prefix directly against every configured regex pattern. ## [16.3.8] - 2026-07-05 diff --git a/packages/coding-agent/src/secrets/obfuscator.ts b/packages/coding-agent/src/secrets/obfuscator.ts index 123edc58f..9f6821c1a 100644 --- a/packages/coding-agent/src/secrets/obfuscator.ts +++ b/packages/coding-agent/src/secrets/obfuscator.ts @@ -1212,12 +1212,17 @@ export class SecretObfuscator { // suffix elsewhere in the transcript could wrap it around a DIFFERENT // real secret's plaintext to make the whole token look pre-redacted and // smuggle that secret through untouched. Refuse the alias fallback when the - // dropped prefix contains a configured plain secret's literal value OR any + // dropped prefix contains a configured plain secret's literal value, any // regex-discovered secret's value this instance has ever minted a // placeholder for (`#obfuscateMappings` — regex secrets are found - // dynamically, so they are never in `#configuredSecretValues`), so the - // plain-secret/regex pass still catches it instead of skipping the whole - // span. + // dynamically, so they are never in `#configuredSecretValues`), OR is + // itself matched by a configured regex pattern directly — a case- or + // flag-variant occurrence (e.g. `content: "tok[a-z0-9]+", flags: "i"` + // discovering lowercase `tokabc123`) never lands in `#obfuscateMappings` + // under its differently-cased form, but the pattern itself still flags it, + // so a forged `#TOKABC123_#` must not be waved through as + // already-redacted. Either check means the plain-secret/regex pass still + // catches it instead of skipping the whole span. #isGeneratedPlaceholder(placeholder: string): boolean { if (this.#deobfuscateMap.has(placeholder)) return true; const match = /^#([A-Z0-9]+)_([A-Z0-9]{4,}(?::[ULCM])?)#$/.exec(placeholder); @@ -1229,6 +1234,12 @@ export class SecretObfuscator { for (const { secret } of this.#obfuscateMappings.values()) { if (secret.length > 0 && prefix.includes(secret)) return false; } + for (const entry of this.#regexEntries) { + entry.regex.lastIndex = 0; + const matches = entry.regex.test(prefix); + entry.regex.lastIndex = 0; + if (matches) return false; + } return this.#deobfuscateMap.has(`#${match[2]}#`); } diff --git a/packages/coding-agent/test/secrets-obfuscator.test.ts b/packages/coding-agent/test/secrets-obfuscator.test.ts index 4b40c1de8..edd9a0127 100644 --- a/packages/coding-agent/test/secrets-obfuscator.test.ts +++ b/packages/coding-agent/test/secrets-obfuscator.test.ts @@ -2211,6 +2211,39 @@ describe("SecretObfuscator friendlyName placeholders", () => { expect(obfuscator.deobfuscate(bravoPlaceholder)).toBe(secretB); }); + it("rejects a forged alias wrapper whose prefix only matches a case-variant regex-discovered secret", () => { + // Regression: the forged-prefix checks above only ever scanned EXACT + // previously-DISCOVERED secret strings (`#configuredSecretValues` and + // `#obfuscateMappings`), recorded in whatever casing they first turned + // up in. A case-insensitive (or other flag-variant) regex — e.g. + // `{ type: "regex", content: "tok[a-z0-9]+", flags: "i" }` — only ever + // records the ONE casing it actually discovered (lowercase + // `tokabc123`) in `#obfuscateMappings`. A forged token wrapping a + // DIFFERENTLY-cased occurrence of that same secret-shaped text + // (`TOKABC123`) around a real bare-alias suffix matched neither exact- + // string check, so it sailed through #isGeneratedPlaceholder as an + // already-generated placeholder and the uppercase secret text leaked + // verbatim. The fix also tests the dropped prefix directly against the + // configured regex's own pattern, which matches regardless of the + // casing the secret was first discovered under. + const obf = new SecretObfuscator([{ type: "regex", content: "tok[a-z0-9]+", flags: "i" }], "Q".repeat(43)); + + // Discover the secret in lowercase, minting a real bare placeholder and + // registering `tokabc123` (not `TOKABC123`) in `#obfuscateMappings`. + const real = obf.obfuscate("use tokabc123 now"); + expect(real).not.toContain("tokabc123"); + const suffix = /#([A-Z0-9]{4,}(?::[ULCM])?)#/.exec(real)?.[1]; + expect(suffix).toBeDefined(); + + // Forge a token wrapping an UPPERCASE variant of the secret around the + // real bare-alias suffix — differently cased from what was actually + // discovered, so it cannot match either exact-string check, only the + // regex pattern itself. + const forged = `see #TOKABC123_${suffix}# here`; + const out = obf.obfuscate(forged); + expect(out).not.toContain("TOKABC123"); + }); + it("drops a friendly name that contains another configured secret's literal value", () => { // Regression: a friendlyName is baked verbatim into every placeholder // minted for its secret (`#NAME_hash:hint#`) via an EXACT