perf(secrets): gate built-in credential regexes behind literal prefix probes - #12666
Open
DarkPhilosophy wants to merge 4 commits into
Open
DarkPhilosophy wants to merge 4 commits into
DarkPhilosophy wants to merge 4 commits into
Conversation
…prefix probe The block-scoped profiler named the freeze: 45 of the first 58 stalls in the user's session spent 60-80% of their time in #collectRegexSecretValues, under obfuscateProviderContext, under prepareProviderCall. Every request to the model rescans the entire conversation - hundreds of messages, 700 KB and growing - with twelve lookbehind credential regexes, three to four times over. The phase breadcrumbs (compose, emit, append) only named whatever happened to be open around it. Each built-in pattern now declares the literal prefixes every one of its matches contains (every alternative spelled out: gho_/ghp_/ghu_/ghs_/ghr_, xoxa-..xoxs-, sk_live_/sk_test_/rk_live_/rk_test_, ...), and the obfuscator skips the regex over text containing none of them. The probe honours the regex's case-insensitivity by folding the text once per scan. It is fail-safe: a regex without metadata - every custom secrets.yml pattern - always runs, and an empty list counts as no metadata. A test redacts a specimen of every alternative and case variant of every built-in pattern through both the gated and the unfiltered scan and requires byte-identical output. On a 716 KB, 700-message context, one request's obfuscation drops from 1018 ms to 19 ms. (cherry picked from commit 6afdb21)
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
I hit constant multi-second UI freezes in long sessions, profiled where the time actually went, and found the credential scanner re-reading the whole conversation on every request. Built-in credential patterns now declare the literal prefixes that every one of their matches contains, and the obfuscator skips a regex entirely when the text contains none of them.
Details:
gho_/ghp_/ghu_/ghs_/ghr_/github_pat_/glpat-/sk-,AKIA/ASIA,xoxa-..xoxs-,sk_live_/sk_test_/rk_live_/rk_test_, and so on.SK-...andBEARER ...are still caught.secrets.ymlregex, is always scanned. An empty list counts as no metadata.obfuscate().Why
Secret redaction runs on the outbound provider context, and it runs on the whole context, not just on the new turn. So every request re-scanned the entire conversation with twelve lookbehind credential regexes, three to four times over. The cost is linear in transcript size, which means it grows for the whole life of a session: at ~700 messages the scan alone took about a second of straight CPU on the UI thread, every single turn.
What this looks like in use is a UI that freezes right after the agent stops, not while it streams. That timing is what made it hard to place: the loop-phase breadcrumbs blamed
ui:render:compose,ui:render:emitandsession:append, because those were simply what happened to be open when the block landed.To get real attribution I ran the engine's sampling profiler scoped to event-loop stalls and aggregated the samples per block. The result was unambiguous: of the first 58 blocks in one session, 45 spent 60-80% of their samples in
#collectRegexSecretValues, underobfuscateProviderContext, underprepareProviderCall. Ruled out along the way, with measurements rather than guesses: garbage collection (a full collection over the 700 MB heap takes 44 ms), session persistence (7 ms per append), and the transcript walk (13,000 peeks, 2.9 s total across a 400 s run).The prefix probe works because these patterns are anchored vendor shapes. A match of
AWSAccessKeyalways containsAKIAorASIA, so text without either cannot match, andString.includesover a few short literals costs a rounding error next to a lookbehind regex over 700 KB. Ordinary source code contains none of the prefixes, so nearly every scan now exits immediately.The gain, concretely:
Testing
packages/coding-agent/test/secrets-prefilter.test.ts(new): for every built-in pattern, a specimen of every alternative and every case variant is redacted through both the gated obfuscator and one built with the metadata stripped, and the outputs must be byte-identical. Also covers a custom regex with no metadata and one with an empty list, both of which must still be scanned.secrets-obfuscator.test.tsplus the new file: 183 pass, 0 fail.bun checkclean.bun checkpasses