diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index bd2740f15..4a0ede1a8 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -10,6 +10,7 @@ - Fixed reversible secret placeholders sharing a case-folded hash base across ASCII case variants, which let a prompt-injected model synthesize a never-provider-visible sibling secret's keyed token by swapping the case hint (`#…:L#` → `#…:U#`) in a tool-call argument. Placeholder bases are now keyed on the exact secret value, so each casing variant gets an independent base and a synthesized sibling token deobfuscates to nothing on live provider/tool-call paths ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)). - Fixed an auto-collected environment secret that is also declared as a plain `mode: "replace"` entry with the same content still forcing creation of the persisted `secret-placeholder.key`. Replace mappings run before obfuscate mappings, so the value is one-way replaced and the obfuscate entry never emits a reversible placeholder; the key-need check now ignores such replace-shadowed obfuscate entries, so an effectively replace-only secret set no longer requires (or writes) the key file and no longer fails startup when the agent config dir is unwritable ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)). +- Fixed a default `mode: "replace"` regex that matches every non-whitespace candidate (e.g. `\S{n}`) shipping the raw secret unchanged when its deterministic replacement collided with the secret value, since every alphanumeric/punctuation candidate still matched. The redaction search now falls back to a same-length whitespace run (space/tab), which `\S`-style patterns never re-match, so the secret is redacted to a stable nonmatching value instead of leaking to the provider ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)). ## [16.2.3] - 2026-06-28 diff --git a/packages/coding-agent/src/secrets/obfuscator.ts b/packages/coding-agent/src/secrets/obfuscator.ts index d6ebb29b6..cbe43baf2 100644 --- a/packages/coding-agent/src/secrets/obfuscator.ts +++ b/packages/coding-agent/src/secrets/obfuscator.ts @@ -26,6 +26,14 @@ export type JsonRecord = { [key: string]: JsonValue | undefined }; const REPLACEMENT_CHARS = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789"; const NONMATCHING_REPLACEMENT_CHARS = `${REPLACEMENT_CHARS}!#$%&()*+,-./:;<=>?@[]^_{|}~`; +// Last-resort redaction bytes for a default replace regex that matches every +// non-whitespace candidate (e.g. `\S{n}`): a same-length run of a single +// whitespace byte is still a stable nonmatching redaction, so the secret is +// replaced rather than shipped raw. Only `space`/`tab` are used — never a line +// terminator — so a `.`-style match-everything regex (which matches space and +// tab but not `\n`) still exhausts to the sentinel instead of redacting to a +// newline run. +const WHITESPACE_REPLACEMENT_CHARS = " \t"; /** Generate a deterministic same-length replacement string from a secret value. */ function generateDeterministicReplacement(secret: string): string { @@ -72,9 +80,12 @@ function ensureDistinctReplacement(replacement: string, secret: string): string * over a stable ASCII alphabet: alphanumerics first (usually enough), then * punctuation fallback bytes when the regex covers every alphanumeric candidate. * This keeps the common case readable while still finding a nonmatching - * same-length redaction for patterns such as `[A-Za-z0-9]{2}`. The sweep is - * bounded so a match-everything regex (`.`/`[\s\S]`) terminates, returning - * undefined to let the caller keep the sentinel as the only available fixed point. + * same-length redaction for patterns such as `[A-Za-z0-9]{2}`. When the regex + * covers every non-whitespace candidate (e.g. `\S{n}`), a same-length run of a + * single whitespace byte (space/tab) is tried as a last resort. The sweep is + * bounded so a match-everything regex (`.`/`[\s\S]`, which also matches space and + * tab) terminates, returning undefined to let the caller keep the sentinel as the + * only available fixed point. */ function findNonMatchingReplacement(value: string, regex: RegExp): string | undefined { const len = value.length; @@ -95,7 +106,7 @@ function findNonMatchingReplacement(value: string, regex: RegExp): string | unde regex.lastIndex = 0; if (!regex.test(candidate)) return candidate; } - return undefined; + return findWhitespaceFallbackReplacement(value, regex); } // Longer collisions stay bounded. First exhaust every single-position // substitution against the deterministic baseline (`AAAA…`, then `!AAA…`, @@ -119,6 +130,24 @@ function findNonMatchingReplacement(value: string, regex: RegExp): string | unde regex.lastIndex = 0; if (!regex.test(candidate)) return candidate; } + return findWhitespaceFallbackReplacement(value, regex); +} + +/** + * Last-resort fallback for a default replace regex that matches every + * non-whitespace candidate: a same-length run of one whitespace byte (space, then + * tab) is a stable nonmatching redaction. `space` already covers the common + * `\S`-style case; `tab` extends it to space-only exclusions. A match-everything + * regex (`.`/`[\s\S]`) also matches both, so this still returns undefined there, + * keeping the caller's sentinel as the sole fixed point. + */ +function findWhitespaceFallbackReplacement(value: string, regex: RegExp): string | undefined { + for (const ch of WHITESPACE_REPLACEMENT_CHARS) { + const candidate = ch.repeat(value.length); + if (candidate === value) continue; + regex.lastIndex = 0; + if (!regex.test(candidate)) return candidate; + } return undefined; } diff --git a/packages/coding-agent/test/secrets-obfuscator.test.ts b/packages/coding-agent/test/secrets-obfuscator.test.ts index 80b916d62..981d3d29f 100644 --- a/packages/coding-agent/test/secrets-obfuscator.test.ts +++ b/packages/coding-agent/test/secrets-obfuscator.test.ts @@ -842,6 +842,21 @@ describe("SecretObfuscator friendlyName placeholders", () => { expect(obf.obfuscate(out)).toBe(out); }); + it("redacts a default replace regex that matches every non-whitespace candidate", () => { + // `\S{5}` matches every non-whitespace value, so the alphanumeric and + // punctuation candidates are all exhausted. A same-length whitespace run is a + // stable nonmatching redaction, so the colliding sentinel value is replaced + // rather than shipped raw to the provider. + const obf = new SecretObfuscator([{ type: "regex", mode: "replace", content: "\\S{5}" }], "Q".repeat(43)); + + const out = obf.obfuscate("ZZLB6"); + + expect(out).not.toBe("ZZLB6"); + expect(out).toHaveLength(5); + expect(/\S{5}/.test(out)).toBe(false); + expect(obf.obfuscate(out)).toBe(out); + }); + it("keeps the sentinel only when no same-length value avoids the regex", () => { // A match-everything regex has no nonmatching same-length redaction, so the // search exhausts and the sentinel is kept as the sole fixed point. Such a