fix(coding-agent): reject forged alias prefixes matching a regex pattern

Codex P2 finding on commit dff2a8d: #isGeneratedPlaceholder's forged-alias guard only checked a dropped friendly-name prefix against exact previously- discovered secret strings, recorded in whatever casing they first turned up in. A case-insensitive (or other flag-variant) regex only ever records the one casing it actually discovered, so a forged token wrapping a differently-cased occurrence of that secret-shaped text around a real bare-alias suffix matched neither exact-string check and sailed through as an already-redacted placeholder, leaking the secret-shaped text verbatim. The guard now also tests the dropped prefix directly against every configured regex pattern.
2026-07-06 05:30:09 +05:30
parent dff2a8dc88
commit 029e838bf0
3 changed files with 49 additions and 4 deletions
@@ -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_<observed-suffix>#` 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

@@ -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_<observed-suffix>#` 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]}#`);
 	}

@@ -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