fix(coding-agent): compare friendlyName collision against the untruncated label

Codex P2 finding on commit 9b2e14d: #friendlyNameCollidesWithSecret
compared a secret's full sanitized value against the already 32-char-capped
(sanitizeSecretFriendlyName) friendly name, so a secret whose sanitized
form exceeds MAX_FRIENDLY_NAME_LEN could never be fully contained in the
truncated label -- the collision went undetected and the secret's first
32 sanitized characters leaked as an accepted placeholder prefix. The
collision check now runs against the full, untruncated sanitized label
(sanitizeForCollisionCheck(friendlyName)); the 32-char cap is applied
only afterward, to the label actually used for display.
This commit is contained in:
Mathews-Tom
2026-07-06 06:06:06 +05:30
parent 9b2e14d91d
commit 9632ed504e
3 changed files with 34 additions and 2 deletions
+1
View File
@@ -33,6 +33,7 @@
- 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.
- Fixed `secrets.yml`-loaded regex `friendlyName` entries pre-sanitizing the label before it reached `#friendlyNameCollidesWithSecret`'s raw-label regex check (the fix above), silently defeating it for every config-file-loaded entry — only entries constructed programmatically with a raw string were actually protected. The loader now preserves the original, unsanitized `friendlyName` string (still validating that it sanitizes to something non-empty), deferring sanitization to the obfuscator as before.
- Fixed the forged friendly-name-alias guard comparing a normalized (alnum-only, uppercased) dropped prefix against RAW plain-secret values, so a lowercase or punctuated configured secret's normalized rendering (e.g. `GITHUBPATABC123` for `github_pat_abc123`) slipped past both the obfuscate-direction guard and, more severely, an equivalent unguarded fallback in `deobfuscate()` — restoring a forged `#GITHUBPATABC123_<suffix-copied-from-any-real-placeholder>#` straight to that OTHER secret's raw value on live provider-output/tool-call-argument paths, with no check at all. Both guards now normalize the compared secret values the same way before accepting the alias fallback, and `deobfuscate()` gained the same secret-shaped-prefix check `obfuscate()` already had.
- Fixed the friendly-name self-collision check comparing an already 32-char-capped, sanitized label against a configured secret's full sanitized value: a secret longer than the cap (or a label set to a long secret's value) could never have its full sanitized form contained in the truncated label, so the collision went undetected and the secret's first 32 sanitized characters were accepted and stamped into the placeholder. The check now runs against the full, uncapped sanitized label; the 32-char cap is applied only afterward, for display.
## [16.3.8] - 2026-07-05
@@ -1116,12 +1116,18 @@ export class SecretObfuscator {
// whole token as already-generated on an EXACT match, before the
// alias-fallback prefix check above ever runs, so the embedded secret
// would never be scanned. Drop the label for this mint rather than risk
// it; the secret still gets a bare (unprefixed) placeholder.
// it; the secret still gets a bare (unprefixed) placeholder. The collision
// check runs against the FULL normalized label — `sanitizeForCollisionCheck`,
// not yet capped at `MAX_FRIENDLY_NAME_LEN` — so a secret longer than the
// 32-char display cap (or one whose sanitized form exceeds it) still gets
// caught: a truncated `requestedFriendlyName` can never contain a longer
// secret's full sanitized form, so checking the truncated label would let
// the secret's first 32 (post-cap) characters leak as an accepted prefix.
const requestedFriendlyName = friendlyName ? sanitizeSecretFriendlyName(friendlyName) : undefined;
const sanitizedFriendlyName =
requestedFriendlyName !== undefined &&
friendlyName !== undefined &&
!this.#friendlyNameCollidesWithSecret(requestedFriendlyName, friendlyName)
!this.#friendlyNameCollidesWithSecret(sanitizeForCollisionCheck(friendlyName), friendlyName)
? requestedFriendlyName
: undefined;
const preferredBase = this.#resolvePreferredPlaceholderBase(baseKey);
@@ -347,6 +347,31 @@ describe("SecretObfuscator friendlyName placeholders", () => {
expect(distinctObfuscator.deobfuscate(distinctObfuscated)).toBe(distinctSecret);
});
it("rejects a friendlyName equal to its own secret even when the secret's sanitized form exceeds the 32-char display cap", () => {
// `#friendlyNameCollidesWithSecret` used to compare the friendly name
// against each secret's sanitized value using the ALREADY-CAPPED,
// display-length friendly name (sliced to `MAX_FRIENDLY_NAME_LEN` = 32
// chars by `sanitizeSecretFriendlyName`) rather than the full,
// un-truncated sanitized form. For a secret whose sanitized (alnum-only,
// uppercased) form is longer than 32 characters, the 32-char-capped
// label being checked could never `.includes()` the longer sanitized
// secret, so a friendlyName set to the secret's own value slipped past
// the collision guard entirely — and the secret's first 32 sanitized
// characters were accepted and baked into the placeholder as a
// visible prefix (e.g. "#GITHUBPATABCDEFGHIJKLMNOPQRSTUV_<hash>:L#"),
// leaking part of the secret. The check now runs against the full,
// un-truncated sanitized label before the 32-char cap is applied for
// display, so secrets longer than the cap are still fully compared
// and caught.
const longSecret = "github_pat_abcdefghijklmnopqrstuvwxyz0123456789";
const obfuscator = new SecretObfuscator([{ type: "plain", content: longSecret, friendlyName: longSecret }]);
const obfuscated = obfuscator.obfuscate(longSecret);
expect(obfuscated).not.toMatch(/GITHUBPAT/);
expect(obfuscated).toMatch(/^#[A-Z0-9]+:L#$/);
expect(obfuscator.deobfuscate(obfuscated)).toBe(longSecret);
});
it("uses regex entry friendly names for discovered matches", () => {
const secret = "tok_abc123";
const obfuscator = new SecretObfuscator([{ type: "regex", content: "tok_[a-z0-9]+", friendlyName: "API Key" }]);