fix(coding-agent): address secrets review feedback
- Fix #generateRegexReplacement's pathological (match-everything) fallback emitting a 1-2 byte matched value unchanged when it was exactly `Z`/`ZZ`, the shared sentinel #generateReplacement uses for such short values. Falls back to a same-length, key-derived run instead, which stays a fixed point under re-obfuscation without being a public, guessable constant. - Default getSecretPlaceholderKey()/getExistingSecretPlaceholderKey() to getAgentDir() instead of getConfigRootDir(), matching the directory createAgentSession() actually passes. - Isolate the getSecretPlaceholderKey test suite under a fresh $HOME/temp agent dir instead of the real homedir, fixing an EACCES failure in sandboxed review environments. - Revert an unrelated gc session-ordering tie-breaker bundled into this branch's history; out of scope for the secrets/friendly-name feature. - Relocate this PR's CHANGELOG.md entries out of already-released sections (16.3.0, 16.3.5) into [Unreleased], where a stale merge had left them, and drop a duplicate blank line and duplicate serverSideFallback/ softRequestBudgetNotice entries the same merge introduced.
This commit is contained in:
Mathews-Tom
@@ -2,6 +2,32 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Added
|
||||
|
||||
- Added `friendlyName` support for hidden secrets so model-visible placeholders can carry sanitized semantic labels, content-derived hashes, and case hints while preserving exact deobfuscation ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
|
||||
### Fixed
|
||||
|
||||
- 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 same-length whitespace markers — a full space/tab run, then a single whitespace byte among non-whitespace filler (` AAAA`) — so `\S`-class patterns and ones that also match all-space/all-tab runs (e.g. `(?:\S{n}| {n}|\t{n})`) are redacted to a stable nonmatching value instead of leaking to the provider; a regex that matches every non-line-terminator stays in the existing `.`/`[\s\S]` sentinel-keeping case ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed the placeholder key-need check over-requiring the persisted `secret-placeholder.key` for an effectively non-placeholding `mode: "replace"` config when a later (shorter-content) replacement erases the placeholder content before the plain-obfuscate pass. Each replacement output is now tested in the form it survives the rest of the replace phase, and the obfuscate `content` it tiles into must also survive those later replacements — covering both a fragment a later replacement rewrites (`AA -> SEC` then `S -> X` turning every `SEC` into `XEC`) and surrounding passthrough bytes a later replacement rewrites (`AA -> SEC` forming `SEC`+`RET12`, then `R -> X` turning the freshly formed `SECRET12` into `SECXET12`). Such configs no longer create the key or fail startup in an unwritable agent config dir, while a fragment whose formed content genuinely survives still requires the key ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed an obfuscate-mode secret regex whose sub-threshold match straddles a previously generated `#…#` placeholder re-obfuscating across the token — corrupting reversible deobfuscation (e.g. a plain `SECRETUV` secret plus `[A-Z]{6}` turning `XXSECRETUVYY` into a single placeholder that restored as `XXSECRETUV`) and, on a re-obfuscation pass, rewriting the surrounding context into fresh placeholders so the `obfuscate()` fixed point and provider-visible history/prompt-cache prefixes drifted. The short-match guard now measures the regex's own match length in the placeholder-expanded scan view (not the rewritten source span) and runs before the placeholder-preservation branch, so a match shorter than `MIN_OBFUSCATE_SECRET_LEN` is skipped, surrounding literals round-trip intact, and re-obfuscation stays a fixed point ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex whose match boundary falls inside a previously generated `#…#` placeholder's expanded value mishandling the cut. In obfuscate mode the boundary was snapped out to the whole token, so two such matches around one placeholder mapped to overlapping source ranges that clobbered on apply and dropped bytes from reversible deobfuscation (e.g. a plain `ABCDEFGH` secret plus `[A-Z]{8}` turned `YYBBABCDEFGHSECRETUV` into a placeholder that restored as `YYBBABCDEFGHETUV`, dropping `SECR`); in replace mode the same cut redacted only the bytes outside the snapped token with a deterministic scramble that drifted across re-obfuscation passes (`ZZgK#…#` → `ZZgZ#…#`). The regex scan now resumes just past the cut placeholder rather than consuming the straddled span, so the cut secret stays hidden as its existing placeholder, no bytes are lost, any trailing wholly-outside content (e.g. an adjacent 8-char run) is still obfuscated or redacted on its own, and re-obfuscation is a fixed point ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a `mode: "replace"` regex that depends on surrounding context (lookbehind/lookahead/`\b`) leaking the raw matched value on alternating turns. The deterministic-replacement collision search tested candidate redactions in isolation, so for a pattern like `(?<=api=)[AZ]` it accepted `api=A` for `api=Z` (a bare `A` does not match the lookbehind) — but the next obfuscate pass re-matched `A` in context and redacted it back to `api=Z`, shipping the secret every other turn. Candidate redactions are now evaluated in their surrounding text, and the deterministic replacement itself is verified to be a fixed point in context (not just against the `Z`/`ZZ` sentinel), so context-sensitive replace regexes resolve to a value the pattern never re-matches in place ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a multi-character `mode: "replace"` regex remainder (the bytes of a match outside a preserved `#…#` placeholder) drifting across an obfuscator restart, which invalidated provider prompt-cache prefixes even with a stable key. The remainder was redacted to a content-derived `ZZ`+hash marker that was only recognized as already-redacted within the generating session (via an in-memory set), so a fresh obfuscator reprocessing persisted text re-redacted it to a different value (`ZZPL#…#` → `ZZ7f#…#`). The remainder marker now derives from a keyed run of the per-install key and the remainder length, so any instance sharing the key reproduces it byte-identically (idempotent across restart) while staying unpredictable enough that raw sentinel-shaped bytes (`ZZZZ`) still differ from it and are redacted rather than passed through ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex match that starts in outside text and ends inside a previously generated `#…#` placeholder's expanded value leaving an independently-matching outside prefix provider-visible. Resuming the scan past the cut placeholder skipped the whole straddling span, so a pattern like `[A-Z0-9]{8,12}` greedily spanning `SECRETUV` into an `ABCDEFGH` placeholder returned `SECRETUV#…#` even though `SECRETUV` satisfies the regex on its own. The cut handling now re-runs the regex bounded to just before the placeholder (full left context kept, so lookbehind still evaluates) and redacts the standalone prefix match — to its own reversible placeholder in obfuscate mode, or a one-way redaction in replace mode — while the cut secret stays as its existing placeholder. The replace-mode redaction's fixed point is verified against the placeholder-expanded view re-obfuscation actually scans, so it does not drift when the adjacent placeholder expands ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex whose match straddles a previously generated `#…#` placeholder still rewriting short surrounding raw bytes the regex never needed, drifting the `obfuscate()` fixed point and provider-visible history/prompt-cache prefixes across re-obfuscation passes. When a greedy match (e.g. `[A-Z0-9]{8,12}`) reaches across a prior-call placeholder whose own value already satisfies the pattern, a trailing/leading raw chunk that does not independently match is now left verbatim instead of being rewritten on the next pass — in obfuscate mode the chunk was minted into a fresh placeholder (`…SECRETUV→#…#A`), and in default replace mode its deterministic scramble drifted (`…#…#ZZJ5sotJ` → `…#…#ZZpvsotJ`). Surrounding bytes are still redacted when the placeholder value alone cannot satisfy the regex (e.g. a required `api_key=` prefix) or when they independently match it ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex match straddling a prior-call placeholder with independently-matching raw bytes on one or both sides leaking those bytes unredacted, in two ways. First, the spillover check concatenated the outside-placeholder chunks before testing whether they independently satisfy the regex, which erased the placeholder-token boundary between them — e.g. with `\b[A-Z]{8}\b|[A-Z]{17}` and a placeholder for `SECRETUV` flanked by prefix `ABCDEFGH` (matches on its own) and suffix `I` (does not), the concatenated `ABCDEFGHI` matched neither alternative, so `ABCDEFGH` was treated as spillover and left verbatim. Second, testing each chunk in isolation (an out-of-context substring) broke context-sensitive patterns — lookbehind, lookahead, and `\b` — that depend on bytes actually adjacent to the chunk in the source text but outside its own range: `(?<=api=)[0-9]{8}` over `api=12345678` plus a trailing placeholder failed when `12345678` was tested standalone, since the isolated slice has no `api=` immediately before it. Each outside chunk is now tested at its real position in the source text, so lookbehind/lookahead see the actual surrounding bytes while a match spanning into the placeholder itself still doesn't count as independent.
|
||||
- Fixed a default (no custom `replacement`) `mode: "replace"` regex whose deterministic redaction has no same-length candidate able to escape the regex (a pathological match-everything config such as `[\s\S]{8}`) churning its marker across every re-obfuscation pass and across obfuscator restarts, drifting provider prompt-cache prefixes. The fallback kept the content-hash-derived replacement, which the regex itself re-matches on the next pass; since that replacement is hashed from its own bytes (not the original secret), each pass rehashed it into a different value. The fallback now reuses the same key+length-only marker already used for per-chunk remainder redactions, which depends on nothing but the per-install key and the value's length, so re-matching and re-redacting it always reproduces the identical marker. A config with only this kind of regex (no other entry needing a persisted key) now also gets a persisted placeholder key created/read for it, so the marker's key input itself stays stable across a process restart instead of falling back to a process-random key.
|
||||
- Fixed `findNonMatchingReplacement` exhaustively enumerating every `90^length` candidate (up to 729,000 for a 3-character match) before falling back, when a default `mode: "replace"` regex of length <= 3 matches every candidate (e.g. `[\s\S]{3}`). Each such match could burn tens of milliseconds; a modest tool output could stall provider requests. All lengths now use the same bounded single-position-substitution search already used for longer values (O(length * 90) instead of O(90^length)).
|
||||
- Fixed a bounded default-mode regex (e.g. `[A-Z]{9}`) whose greedy reach spans a placeholdered secret plus a short trailing raw chunk (or a second adjacent secret) leaving that chunk unredacted on the first `obfuscate()` call but sweeping it into a new placeholder starting from the second call, churning provider-visible history and prompt-cache prefixes. A cut-resolution resume point that landed exactly on the start of another already-generated placeholder handed it straight to a fresh regex attempt instead of skipping past it, so a leading run of secrets resolved differently depending on whether its first member was still raw text (this call is about to placeholder it) or was already a placeholder from a prior call. Resume points are now chained past every immediately-adjacent placeholder before a new match attempt, so both calls land on the same next scan position and agree on the same (conservative) redaction from the first pass onward.
|
||||
- Fixed friendly-name secret placeholders (`#PREFIX_HASH:HINT#`) being forgeable: untrusted text could wrap a real secret's plaintext in a fabricated friendly-name prefix around a hash suffix borrowed from any OTHER already-obfuscated secret, and `obfuscate()` would treat the whole token — including the exposed secret literal standing in for the prefix — as already redacted, letting it reach the provider untouched. `obfuscate()` now refuses that friendly-name-independent alias fallback whenever the dropped prefix contains a configured secret's literal value, while still accepting a stale prefix left over from a friendly-name rename.
|
||||
- Fixed the friendly-name placeholder forgery check above missing regex-discovered secrets (only statically configured plain secrets were checked), and a regex match's short-match guard undercounting its length when clamped to a wholly-outside prefix before an already-generated placeholder — a full-size match whose kept prefix was under the 8-byte floor was wrongly skipped as noise, leaving that prefix provider-visible.
|
||||
- Fixed a configured secret's `friendlyName` being able to bake another live secret's literal value straight into every placeholder minted for it (e.g. `{ content: "ABCDEFGH", friendlyName: "LEAKTOKEN" }` alongside a secret covering `LEAKTOKEN`), which then read as an already-generated placeholder on an exact match and was never scanned. `obfuscate()` now drops the friendly-name prefix for a given secret whenever the sanitized name contains a configured plain secret's literal or is matched by a configured regex, independent of `entries[]` order (regex patterns are now compiled before any placeholder is minted).
|
||||
- Fixed a default (no custom `replacement`) `mode: "replace"` regex with no same-length candidate able to escape it (a pathological match-everything config such as `.`/`[\s\S]`) emitting a 1–2 byte matched value unchanged when it was exactly `Z` or `ZZ`: the fallback reused `#generateReplacement`'s shared `Z`/`ZZ` sentinel for such short values, and a raw match of exactly that sentinel round-tripped to the same bytes, reaching the provider unredacted (plain `mode: "replace"` secrets already avoided this via `ensureDistinctReplacement`, but the regex fallback did not). The fallback now uses a same-length, key-derived run instead of the sentinel for <=2 char values — still a fixed point under re-obfuscation (depends only on the per-install key and the value's length, never its content, so re-matching and re-redacting it reproduces the identical marker), but no longer a public, install-independent constant a regex config could be tuned to bypass.
|
||||
- Fixed `getSecretPlaceholderKey()`/`getExistingSecretPlaceholderKey()` defaulting their `keyDir` parameter to `getConfigRootDir()` (`~/.omp`) while `createAgentSession()` always passes the profile-scoped `agentDir` (`~/.omp/agent`, per `docs/secrets.md`) explicitly. A caller relying on the default read or minted a key file at a different path than live sessions use, so placeholders it created were never stable against SDK sessions. Both helpers now default to `getAgentDir()`.
|
||||
|
||||
## [16.3.8] - 2026-07-05
|
||||
|
||||
### Fixed
|
||||
@@ -90,15 +116,6 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed a default (no custom `replacement`) `mode: "replace"` regex whose deterministic redaction has no same-length candidate able to escape the regex (a pathological match-everything config such as `[\s\S]{8}`) churning its marker across every re-obfuscation pass and across obfuscator restarts, drifting provider prompt-cache prefixes. The fallback kept the content-hash-derived replacement, which the regex itself re-matches on the next pass; since that replacement is hashed from its own bytes (not the original secret), each pass rehashed it into a different value. The fallback now reuses the same key+length-only marker already used for per-chunk remainder redactions, which depends on nothing but the per-install key and the value's length, so re-matching and re-redacting it always reproduces the identical marker. A config with only this kind of regex (no other entry needing a persisted key) now also gets a persisted placeholder key created/read for it, so the marker's key input itself stays stable across a process restart instead of falling back to a process-random key.
|
||||
- Fixed `findNonMatchingReplacement` exhaustively enumerating every `90^length` candidate (up to 729,000 for a 3-character match) before falling back, when a default `mode: "replace"` regex of length <= 3 matches every candidate (e.g. `[\s\S]{3}`). Each such match could burn tens of milliseconds; a modest tool output could stall provider requests. All lengths now use the same bounded single-position-substitution search already used for longer values (O(length * 90) instead of O(90^length)).
|
||||
- Fixed a bounded default-mode regex (e.g. `[A-Z]{9}`) whose greedy reach spans a placeholdered secret plus a short trailing raw chunk (or a second adjacent secret) leaving that chunk unredacted on the first `obfuscate()` call but sweeping it into a new placeholder starting from the second call, churning provider-visible history and prompt-cache prefixes. A cut-resolution resume point that landed exactly on the start of another already-generated placeholder handed it straight to a fresh regex attempt instead of skipping past it, so a leading run of secrets resolved differently depending on whether its first member was still raw text (this call is about to placeholder it) or was already a placeholder from a prior call. Resume points are now chained past every immediately-adjacent placeholder before a new match attempt, so both calls land on the same next scan position and agree on the same (conservative) redaction from the first pass onward.
|
||||
- Fixed friendly-name secret placeholders (`#PREFIX_HASH:HINT#`) being forgeable: untrusted text could wrap a real secret's plaintext in a fabricated friendly-name prefix around a hash suffix borrowed from any OTHER already-obfuscated secret, and `obfuscate()` would treat the whole token — including the exposed secret literal standing in for the prefix — as already redacted, letting it reach the provider untouched. `obfuscate()` now refuses that friendly-name-independent alias fallback whenever the dropped prefix contains a configured secret's literal value, while still accepting a stale prefix left over from a friendly-name rename.
|
||||
- Fixed the friendly-name placeholder forgery check above missing regex-discovered secrets (only statically configured plain secrets were checked), and a regex match's short-match guard undercounting its length when clamped to a wholly-outside prefix before an already-generated placeholder — a full-size match whose kept prefix was under the 8-byte floor was wrongly skipped as noise, leaving that prefix provider-visible.
|
||||
- Fixed a configured secret's `friendlyName` being able to bake another live secret's literal value straight into every placeholder minted for it (e.g. `{ content: "ABCDEFGH", friendlyName: "LEAKTOKEN" }` alongside a secret covering `LEAKTOKEN`), which then read as an already-generated placeholder on an exact match and was never scanned. `obfuscate()` now drops the friendly-name prefix for a given secret whenever the sanitized name contains a configured plain secret's literal or is matched by a configured regex, independent of `entries[]` order (regex patterns are now compiled before any placeholder is minted).
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed Mnemopi auto-retention so protocol markers are stripped from embedding and FTS projections while stored transcripts remain readable. ([#4395](https://github.com/can1357/oh-my-pi/issues/4395))
|
||||
- Fixed mnemopi auto-retain storing cumulative full-session transcripts on every retention interval; subsequent retains now store only newly completed user-turn suffixes. ([#4396](https://github.com/can1357/oh-my-pi/issues/4396))
|
||||
- Fixed large `artifact://` reads materializing entire MCP/tool artifacts before selector paging, preventing OOM crashes on unbounded raw reads and surfacing bounded read/search/copy guidance ([#4482](https://github.com/can1357/oh-my-pi/issues/4482)).
|
||||
@@ -214,27 +231,6 @@
|
||||
|
||||
### Added
|
||||
|
||||
- Added `friendlyName` support for hidden secrets so model-visible placeholders can carry sanitized semantic labels, content-derived hashes, and case hints while preserving exact deobfuscation ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
|
||||
### Fixed
|
||||
|
||||
- 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 same-length whitespace markers — a full space/tab run, then a single whitespace byte among non-whitespace filler (` AAAA`) — so `\S`-class patterns and ones that also match all-space/all-tab runs (e.g. `(?:\S{n}| {n}|\t{n})`) are redacted to a stable nonmatching value instead of leaking to the provider; a regex that matches every non-line-terminator stays in the existing `.`/`[\s\S]` sentinel-keeping case ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed the placeholder key-need check over-requiring the persisted `secret-placeholder.key` for an effectively non-placeholding `mode: "replace"` config when a later (shorter-content) replacement erases the placeholder content before the plain-obfuscate pass. Each replacement output is now tested in the form it survives the rest of the replace phase, and the obfuscate `content` it tiles into must also survive those later replacements — covering both a fragment a later replacement rewrites (`AA -> SEC` then `S -> X` turning every `SEC` into `XEC`) and surrounding passthrough bytes a later replacement rewrites (`AA -> SEC` forming `SEC`+`RET12`, then `R -> X` turning the freshly formed `SECRET12` into `SECXET12`). Such configs no longer create the key or fail startup in an unwritable agent config dir, while a fragment whose formed content genuinely survives still requires the key ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed an obfuscate-mode secret regex whose sub-threshold match straddles a previously generated `#…#` placeholder re-obfuscating across the token — corrupting reversible deobfuscation (e.g. a plain `SECRETUV` secret plus `[A-Z]{6}` turning `XXSECRETUVYY` into a single placeholder that restored as `XXSECRETUV`) and, on a re-obfuscation pass, rewriting the surrounding context into fresh placeholders so the `obfuscate()` fixed point and provider-visible history/prompt-cache prefixes drifted. The short-match guard now measures the regex's own match length in the placeholder-expanded scan view (not the rewritten source span) and runs before the placeholder-preservation branch, so a match shorter than `MIN_OBFUSCATE_SECRET_LEN` is skipped, surrounding literals round-trip intact, and re-obfuscation stays a fixed point ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex whose match boundary falls inside a previously generated `#…#` placeholder's expanded value mishandling the cut. In obfuscate mode the boundary was snapped out to the whole token, so two such matches around one placeholder mapped to overlapping source ranges that clobbered on apply and dropped bytes from reversible deobfuscation (e.g. a plain `ABCDEFGH` secret plus `[A-Z]{8}` turned `YYBBABCDEFGHSECRETUV` into a placeholder that restored as `YYBBABCDEFGHETUV`, dropping `SECR`); in replace mode the same cut redacted only the bytes outside the snapped token with a deterministic scramble that drifted across re-obfuscation passes (`ZZgK#…#` → `ZZgZ#…#`). The regex scan now resumes just past the cut placeholder rather than consuming the straddled span, so the cut secret stays hidden as its existing placeholder, no bytes are lost, any trailing wholly-outside content (e.g. an adjacent 8-char run) is still obfuscated or redacted on its own, and re-obfuscation is a fixed point ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a `mode: "replace"` regex that depends on surrounding context (lookbehind/lookahead/`\b`) leaking the raw matched value on alternating turns. The deterministic-replacement collision search tested candidate redactions in isolation, so for a pattern like `(?<=api=)[AZ]` it accepted `api=A` for `api=Z` (a bare `A` does not match the lookbehind) — but the next obfuscate pass re-matched `A` in context and redacted it back to `api=Z`, shipping the secret every other turn. Candidate redactions are now evaluated in their surrounding text, and the deterministic replacement itself is verified to be a fixed point in context (not just against the `Z`/`ZZ` sentinel), so context-sensitive replace regexes resolve to a value the pattern never re-matches in place ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a multi-character `mode: "replace"` regex remainder (the bytes of a match outside a preserved `#…#` placeholder) drifting across an obfuscator restart, which invalidated provider prompt-cache prefixes even with a stable key. The remainder was redacted to a content-derived `ZZ`+hash marker that was only recognized as already-redacted within the generating session (via an in-memory set), so a fresh obfuscator reprocessing persisted text re-redacted it to a different value (`ZZPL#…#` → `ZZ7f#…#`). The remainder marker now derives from a keyed run of the per-install key and the remainder length, so any instance sharing the key reproduces it byte-identically (idempotent across restart) while staying unpredictable enough that raw sentinel-shaped bytes (`ZZZZ`) still differ from it and are redacted rather than passed through ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex match that starts in outside text and ends inside a previously generated `#…#` placeholder's expanded value leaving an independently-matching outside prefix provider-visible. Resuming the scan past the cut placeholder skipped the whole straddling span, so a pattern like `[A-Z0-9]{8,12}` greedily spanning `SECRETUV` into an `ABCDEFGH` placeholder returned `SECRETUV#…#` even though `SECRETUV` satisfies the regex on its own. The cut handling now re-runs the regex bounded to just before the placeholder (full left context kept, so lookbehind still evaluates) and redacts the standalone prefix match — to its own reversible placeholder in obfuscate mode, or a one-way redaction in replace mode — while the cut secret stays as its existing placeholder. The replace-mode redaction's fixed point is verified against the placeholder-expanded view re-obfuscation actually scans, so it does not drift when the adjacent placeholder expands ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex whose match straddles a previously generated `#…#` placeholder still rewriting short surrounding raw bytes the regex never needed, drifting the `obfuscate()` fixed point and provider-visible history/prompt-cache prefixes across re-obfuscation passes. When a greedy match (e.g. `[A-Z0-9]{8,12}`) reaches across a prior-call placeholder whose own value already satisfies the pattern, a trailing/leading raw chunk that does not independently match is now left verbatim instead of being rewritten on the next pass — in obfuscate mode the chunk was minted into a fresh placeholder (`…SECRETUV→#…#A`), and in default replace mode its deterministic scramble drifted (`…#…#ZZJ5sotJ` → `…#…#ZZpvsotJ`). Surrounding bytes are still redacted when the placeholder value alone cannot satisfy the regex (e.g. a required `api_key=` prefix) or when they independently match it ([#2465](https://github.com/can1357/oh-my-pi/issues/2465)).
|
||||
- Fixed a secret regex match straddling a prior-call placeholder with independently-matching raw bytes on one or both sides leaking those bytes unredacted, in two ways. First, the spillover check concatenated the outside-placeholder chunks before testing whether they independently satisfy the regex, which erased the placeholder-token boundary between them — e.g. with `\b[A-Z]{8}\b|[A-Z]{17}` and a placeholder for `SECRETUV` flanked by prefix `ABCDEFGH` (matches on its own) and suffix `I` (does not), the concatenated `ABCDEFGHI` matched neither alternative, so `ABCDEFGH` was treated as spillover and left verbatim. Second, testing each chunk in isolation (an out-of-context substring) broke context-sensitive patterns — lookbehind, lookahead, and `\b` — that depend on bytes actually adjacent to the chunk in the source text but outside its own range: `(?<=api=)[0-9]{8}` over `api=12345678` plus a trailing placeholder failed when `12345678` was tested standalone, since the isolated slice has no `api=` immediately before it. Each outside chunk is now tested at its real position in the source text, so lookbehind/lookahead see the actual surrounding bytes while a match spanning into the placeholder itself still doesn't count as independent.
|
||||
|
||||
### Added
|
||||
|
||||
- Added `providers.anthropic.serverSideFallback` (default off; UI in the "Model → Retry & Fallback" group). When enabled, Claude Fable 5 / Mythos 5 requests carry `fallbacks: [{ model: "claude-opus-4-8" }]` via Anthropic's server-side-fallback beta chain so classifier-blocked turns are retried on Opus 4.8 without breaking the current call. Opt-in only — leaving it off preserves the pre-fallback behavior. ([#4177](https://github.com/can1357/oh-my-pi/issues/4177))
|
||||
- Added `task.softRequestBudgetNotice` (default off) to opt into the subagent soft-budget wrap-up steering notice while keeping the 1.5x graceful abort guard active.
|
||||
- Added `providers.anthropic.serverSideFallback` configuration option to opt into Anthropic's server-side-fallback beta chain, allowing Claude Fable 5 / Mythos 5 requests to automatically retry on Opus 4.8 when blocked by classifiers.
|
||||
- Added `providers.anthropic.serverSideFallback` configuration option to opt into Anthropic's server-side-fallback beta chain, allowing Claude requests to automatically retry on alternative models when blocked by classifiers.
|
||||
- Added `task.softRequestBudgetNotice` configuration option to enable subagent soft-budget wrap-up steering notices while keeping the graceful abort guard active.
|
||||
|
||||
@@ -619,7 +615,6 @@
|
||||
|
||||
## [16.1.23] - 2026-06-26
|
||||
|
||||
|
||||
### Added
|
||||
|
||||
- Added `compaction.midTurnEnabled` for mid-turn threshold auto-compaction before the next tool-loop provider request. ([#3525](https://github.com/can1357/oh-my-pi/issues/3525))
|
||||
|
||||
@@ -351,7 +351,7 @@ async function listActiveSessions(sessionsRoot: string): Promise<SessionInfo[]>
|
||||
if (!entry.isDirectory()) continue;
|
||||
sessions.push(...(await listSessionsReadOnly(path.join(sessionsRoot, entry.name), storage)));
|
||||
}
|
||||
sessions.sort((a, b) => b.modified.getTime() - a.modified.getTime() || b.path.localeCompare(a.path));
|
||||
sessions.sort((a, b) => b.modified.getTime() - a.modified.getTime());
|
||||
return sessions;
|
||||
}
|
||||
|
||||
@@ -361,7 +361,7 @@ async function listNestedSessionsReadOnly(artifactsRoot: string): Promise<Sessio
|
||||
const storage = new FileSessionStorage();
|
||||
const sessions: SessionInfo[] = [];
|
||||
for (const dir of dirs) sessions.push(...(await listSessionsReadOnly(dir, storage)));
|
||||
sessions.sort((a, b) => b.modified.getTime() - a.modified.getTime() || b.path.localeCompare(a.path));
|
||||
sessions.sort((a, b) => b.modified.getTime() - a.modified.getTime());
|
||||
return sessions;
|
||||
}
|
||||
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import * as crypto from "node:crypto";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as path from "node:path";
|
||||
import { getConfigRootDir, isEnoent, logger } from "@oh-my-pi/pi-utils";
|
||||
import { getAgentDir, isEnoent, logger } from "@oh-my-pi/pi-utils";
|
||||
import { YAML } from "bun";
|
||||
import { type SecretEntry, sanitizeSecretFriendlyName } from "./obfuscator";
|
||||
import { compileSecretRegex } from "./regex";
|
||||
@@ -10,12 +10,16 @@ const PLACEHOLDER_KEY_RE = /^[A-Za-z0-9_-]{43}$/;
|
||||
const cachedPlaceholderKeys = new Map<string, string>();
|
||||
|
||||
/**
|
||||
* Per-install secret key for the placeholder digest. Persisted under the config
|
||||
* root and never sent to a provider, so model-visible placeholders cannot be
|
||||
* reversed by dictionary-hashing candidate secrets. Stable across sessions so
|
||||
* persisted transcripts deobfuscate consistently.
|
||||
* Per-install secret key for the placeholder digest. Persisted under the agent
|
||||
* config directory and never sent to a provider, so model-visible placeholders
|
||||
* cannot be reversed by dictionary-hashing candidate secrets. Stable across
|
||||
* sessions so persisted transcripts deobfuscate consistently. Defaults to
|
||||
* `getAgentDir()` — the same directory `createAgentSession()` passes as
|
||||
* `agentDir` — so a caller relying on the default reads/writes the identical
|
||||
* key file live sessions use, per `~/.omp/agent/secret-placeholder.key` in
|
||||
* docs/secrets.md.
|
||||
*/
|
||||
export async function getSecretPlaceholderKey(keyDir: string = getConfigRootDir()): Promise<string> {
|
||||
export async function getSecretPlaceholderKey(keyDir: string = getAgentDir()): Promise<string> {
|
||||
const keyPath = path.join(keyDir, "secret-placeholder.key");
|
||||
const cached = cachedPlaceholderKeys.get(keyPath);
|
||||
if (cached !== undefined) return cached;
|
||||
@@ -48,9 +52,7 @@ export async function getSecretPlaceholderKey(keyDir: string = getConfigRootDir(
|
||||
}
|
||||
|
||||
/** Return an existing placeholder key for redaction without creating a new key file. */
|
||||
export async function getExistingSecretPlaceholderKey(
|
||||
keyDir: string = getConfigRootDir(),
|
||||
): Promise<string | undefined> {
|
||||
export async function getExistingSecretPlaceholderKey(keyDir: string = getAgentDir()): Promise<string | undefined> {
|
||||
const keyPath = path.join(keyDir, "secret-placeholder.key");
|
||||
const cached = cachedPlaceholderKeys.get(keyPath);
|
||||
if (cached !== undefined) return cached;
|
||||
|
||||
@@ -913,11 +913,31 @@ export class SecretObfuscator {
|
||||
* the next pass, and because it is derived from the bytes being replaced, rehashing
|
||||
* those bytes (the marker itself, not the original secret) produces a DIFFERENT
|
||||
* value — churning the redaction, and the provider prompt-cache prefix it anchors,
|
||||
* across every re-obfuscation. Fall back instead to the same key+length-only marker
|
||||
* `#generateReplacement` uses for chunk redactions: it depends on nothing but
|
||||
* `this.#key` and the value's length, so re-matching and re-redacting it reproduces
|
||||
* the IDENTICAL marker every time, which the pathological case requires since no
|
||||
* value can escape the regex at all.
|
||||
* across every re-obfuscation. Fall back instead to a marker that depends only
|
||||
* on `this.#key` and the value's length, not its content, so re-matching and
|
||||
* re-redacting it reproduces the IDENTICAL marker every time, which the
|
||||
* pathological case requires since no value can escape the regex at all. This
|
||||
* cannot reuse `#generateReplacement`'s own <=2-char branch directly: that
|
||||
* branch is the fixed `Z`/`ZZ` sentinel, which is itself a value this fallback
|
||||
* could be asked to replace (an input of exactly `Z` or `ZZ`), and returning it
|
||||
* unchanged would ship the raw secret to the provider — the exact failure plain
|
||||
* replace-mode secrets avoid via `ensureDistinctReplacement`. Neither
|
||||
* `ensureDistinctReplacement`'s single-char flip NOR a length-changing marker is
|
||||
* usable here: a pathological regex re-matches ANY same-length value, including
|
||||
* a flipped one, so a value-dependent flip oscillates between the two forever;
|
||||
* and a regex with no quantifier (matching one input character per match, e.g.
|
||||
* `.`) re-scans a LONGER marker as several independent same-regex matches on the
|
||||
* next pass, re-expanding each one — unbounded growth, not a fixed point. Use a
|
||||
* SAME-LENGTH keyed run instead of the sentinel for <=2 chars: content-independent
|
||||
* (so it is trivially its own fixed point once emitted) and no longer a public,
|
||||
* install-independent constant, closing the specific guessable collision
|
||||
* (`Z`/`ZZ`) the sentinel had. A same-length, content-independent marker cannot
|
||||
* mathematically rule out equaling some pathological input by construction (the
|
||||
* marker is itself a same-length string a match-everything regex also matches),
|
||||
* but that residual case now requires guessing this install's private key rather
|
||||
* than a universal constant — the same class of accepted risk
|
||||
* `generateDeterministicReplacement`'s hash collision already carries for longer
|
||||
* values.
|
||||
*/
|
||||
#generateRegexReplacement(value: string, regex: RegExp, context: RegexMatchContext): string {
|
||||
let replacement = generateDeterministicReplacement(value);
|
||||
@@ -928,7 +948,12 @@ export class SecretObfuscator {
|
||||
// secret. Search for a candidate the regex does not re-match in place.
|
||||
if (replacement === value || regexRematchesInContext(replacement, regex, context)) {
|
||||
const stable = findNonMatchingReplacement(value, regex, context);
|
||||
replacement = stable ?? this.#generateReplacement(value);
|
||||
// See docstring above: same-length keyed run for <=2 chars (never the
|
||||
// `Z`/`ZZ` sentinel, which a <=2 char value could itself be), otherwise
|
||||
// the ordinary keyed-run fallback #generateReplacement already uses.
|
||||
replacement =
|
||||
stable ??
|
||||
(value.length <= 2 ? buildKeyedReplacementRun(this.#key, value.length) : this.#generateReplacement(value));
|
||||
regex.lastIndex = 0;
|
||||
}
|
||||
this.#generatedReplaceChunks.add(replacement);
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
* Tests for secrets regex parsing, compilation, and obfuscation.
|
||||
*/
|
||||
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { describe, expect, it, spyOn } from "bun:test";
|
||||
import * as fs from "node:fs/promises";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
@@ -27,7 +27,7 @@ import {
|
||||
stripPendingSecretPlaceholderSuffix,
|
||||
} from "@oh-my-pi/pi-coding-agent/secrets/obfuscator";
|
||||
import { compileSecretRegex } from "@oh-my-pi/pi-coding-agent/secrets/regex";
|
||||
import { getActiveProfile, getConfigRootDir, setProfile } from "@oh-my-pi/pi-utils/dirs";
|
||||
import { getActiveProfile, getAgentDir, setProfile } from "@oh-my-pi/pi-utils/dirs";
|
||||
import { type } from "arktype";
|
||||
|
||||
describe("compileSecretRegex", () => {
|
||||
@@ -222,66 +222,67 @@ describe("SecretObfuscator regex behavior", () => {
|
||||
});
|
||||
|
||||
describe("getSecretPlaceholderKey", () => {
|
||||
async function withTempConfigRoot(run: () => Promise<void>): Promise<void> {
|
||||
// Isolate under a fresh $HOME (never the real homedir) so mkdir/writeFile
|
||||
// below cannot hit EACCES in a sandboxed CI/review environment where the
|
||||
// real home is read-only, and so the default-arg path under test resolves
|
||||
// through the SAME `getAgentDir()` the runtime uses (see getSecretPlaceholderKey's
|
||||
// docstring) rather than the unrelated `getConfigRootDir()`.
|
||||
async function withTempAgentHome(run: () => Promise<void>): Promise<void> {
|
||||
const originalProfile = getActiveProfile();
|
||||
const originalConfigDir = process.env.PI_CONFIG_DIR;
|
||||
const originalAgentDir = process.env.PI_CODING_AGENT_DIR;
|
||||
const configDirName = `.omp-secret-key-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}`;
|
||||
const configRoot = path.join(os.homedir(), configDirName);
|
||||
const originalHome = process.env.HOME;
|
||||
const tempHomeDir = await fs.mkdtemp(path.join(os.tmpdir(), "omp-secret-key-"));
|
||||
process.env.HOME = tempHomeDir;
|
||||
const homedirSpy = spyOn(os, "homedir").mockReturnValue(tempHomeDir);
|
||||
try {
|
||||
process.env.PI_CONFIG_DIR = configDirName;
|
||||
setProfile(undefined);
|
||||
await run();
|
||||
} finally {
|
||||
setProfile(undefined);
|
||||
if (originalConfigDir === undefined) {
|
||||
delete process.env.PI_CONFIG_DIR;
|
||||
} else {
|
||||
process.env.PI_CONFIG_DIR = originalConfigDir;
|
||||
}
|
||||
if (originalAgentDir === undefined) {
|
||||
delete process.env.PI_CODING_AGENT_DIR;
|
||||
} else {
|
||||
process.env.PI_CODING_AGENT_DIR = originalAgentDir;
|
||||
}
|
||||
setProfile(originalProfile);
|
||||
await fs.rm(configRoot, { recursive: true, force: true });
|
||||
homedirSpy.mockRestore();
|
||||
if (originalHome === undefined) {
|
||||
delete process.env.HOME;
|
||||
} else {
|
||||
process.env.HOME = originalHome;
|
||||
}
|
||||
await fs.rm(tempHomeDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
it("caches placeholder keys per profile config root", async () => {
|
||||
await withTempConfigRoot(async () => {
|
||||
it("caches placeholder keys per profile agent dir", async () => {
|
||||
await withTempAgentHome(async () => {
|
||||
const alphaKey = "A".repeat(43);
|
||||
const betaKey = "B".repeat(43);
|
||||
setProfile("alpha");
|
||||
await fs.mkdir(getConfigRootDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getConfigRootDir(), "secret-placeholder.key"), alphaKey);
|
||||
await fs.mkdir(getAgentDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getAgentDir(), "secret-placeholder.key"), alphaKey);
|
||||
expect(await getSecretPlaceholderKey()).toBe(alphaKey);
|
||||
|
||||
setProfile("beta");
|
||||
await fs.mkdir(getConfigRootDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getConfigRootDir(), "secret-placeholder.key"), betaKey);
|
||||
await fs.mkdir(getAgentDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getAgentDir(), "secret-placeholder.key"), betaKey);
|
||||
expect(await getSecretPlaceholderKey()).toBe(betaKey);
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects truncated placeholder key files", async () => {
|
||||
await withTempConfigRoot(async () => {
|
||||
await withTempAgentHome(async () => {
|
||||
setProfile("truncated");
|
||||
await fs.mkdir(getConfigRootDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getConfigRootDir(), "secret-placeholder.key"), "abc123");
|
||||
await fs.mkdir(getAgentDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getAgentDir(), "secret-placeholder.key"), "abc123");
|
||||
|
||||
await expect(getSecretPlaceholderKey()).rejects.toThrow("secret placeholder key");
|
||||
});
|
||||
});
|
||||
|
||||
it("retries empty existing placeholder key files without creating a new one", async () => {
|
||||
await withTempConfigRoot(async () => {
|
||||
await withTempAgentHome(async () => {
|
||||
setProfile("race");
|
||||
await fs.mkdir(getConfigRootDir(), { recursive: true });
|
||||
const keyPath = path.join(getConfigRootDir(), "secret-placeholder.key");
|
||||
await fs.mkdir(getAgentDir(), { recursive: true });
|
||||
const keyPath = path.join(getAgentDir(), "secret-placeholder.key");
|
||||
await fs.writeFile(keyPath, "");
|
||||
const eventualKey = "C".repeat(43);
|
||||
// Real delay is intentional: this exercises readPlaceholderKeyFile's retry
|
||||
// loop against an actual concurrent filesystem write, not a mockable timer.
|
||||
const writer = Bun.sleep(25).then(() => fs.writeFile(keyPath, eventualKey));
|
||||
|
||||
await expect(getExistingSecretPlaceholderKey()).resolves.toBe(eventualKey);
|
||||
@@ -290,10 +291,10 @@ describe("getSecretPlaceholderKey", () => {
|
||||
});
|
||||
|
||||
it("treats an invalid existing placeholder key as absent for redaction", async () => {
|
||||
await withTempConfigRoot(async () => {
|
||||
await withTempAgentHome(async () => {
|
||||
setProfile("invalid-existing");
|
||||
await fs.mkdir(getConfigRootDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getConfigRootDir(), "secret-placeholder.key"), "abc123");
|
||||
await fs.mkdir(getAgentDir(), { recursive: true });
|
||||
await fs.writeFile(path.join(getAgentDir(), "secret-placeholder.key"), "abc123");
|
||||
|
||||
// Replace-only/no-secret sessions load the key only to redact it from tool
|
||||
// output; a corrupt key must not block startup, so the existing-key probe
|
||||
@@ -1412,20 +1413,36 @@ describe("SecretObfuscator friendlyName placeholders", () => {
|
||||
expect(obf.obfuscate(out)).toBe(out);
|
||||
});
|
||||
|
||||
it("keeps the sentinel only when no same-length value avoids the regex", () => {
|
||||
it("never emits a <=2 char match unchanged 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
|
||||
// config redacts every character and is pathological by construction.
|
||||
// search exhausts. The fallback must still differ from a <=2 char value that
|
||||
// happens to equal `#generateReplacement`'s own `Z`/`ZZ` sentinel (else the
|
||||
// raw secret would ship unchanged) while staying a fixed point under
|
||||
// re-obfuscation. Such a config redacts every character and is pathological
|
||||
// by construction.
|
||||
for (const content of [".", "[\\s\\S]"]) {
|
||||
const obf = new SecretObfuscator([{ type: "regex", mode: "replace", content }], "Q".repeat(43));
|
||||
|
||||
const out = obf.obfuscate("Z");
|
||||
|
||||
expect(out).toBe("Z");
|
||||
expect(out).not.toBe("Z");
|
||||
expect(out).toHaveLength(1);
|
||||
expect(obf.obfuscate(out)).toBe(out);
|
||||
}
|
||||
});
|
||||
|
||||
it("never emits a 2-char match unchanged when no same-length value avoids the regex", () => {
|
||||
// Same pathology as above, sized to hit the `ZZ` half of `#generateReplacement`'s
|
||||
// sentinel rather than the `Z` half.
|
||||
const obf = new SecretObfuscator([{ type: "regex", mode: "replace", content: "[\\s\\S]{2}" }], "Q".repeat(43));
|
||||
|
||||
const out = obf.obfuscate("ZZ");
|
||||
|
||||
expect(out).not.toBe("ZZ");
|
||||
expect(out).toHaveLength(2);
|
||||
expect(obf.obfuscate(out)).toBe(out);
|
||||
});
|
||||
|
||||
it("resolves a three-character match-everything regex without the removed exhaustive sweep", () => {
|
||||
// Regression: `findNonMatchingReplacement` used to special-case `len <= 3`
|
||||
// with a fully exhaustive search over every `base**length` candidate
|
||||
|
||||
Reference in New Issue
Block a user