Repository navigation
fix(hooks): anchor the secret-scanner capability guard to end of line, not end of input (#737) - #774
Merged
Conversation
…, not end of input (#737) v6.8.0 exempted a value that is exactly read/write/none/true/false/null from the five reference-guarded key-value rules, so that `id-token: write` would stop being read as credential material. The exemption was anchored with `$`, but all nine rules are built with the `gi` flags and no `m`, so `$` matched end of input rather than end of line. The guard fired only when the permission line was the final content in the string. secret-file-scanner.mjs runs on whole file contents, where a permissions block is followed by the jobs it authorises, so the OIDC case the fix was written for stayed blocked in that hook throughout. secret-detection.mjs blocked any prompt or subagent report quoting the line mid-paragraph, which is what halted a long /arckit:repo-audit run part-way through. Trailing spaces after the value defeated the guard too. Both copies now spell end of line out as `[^\S\r\n]*(?:\r?\n|$)`, fixing it in one place per file rather than adding `m` to nine constructors. Literal credentials, provider token formats and values that merely begin with a level word are still blocked, mid-file as well as at the end. Six regression tests cover the multi-line and trailing-whitespace cases against both hooks; every capability vector in that suite had been a single line with nothing after it, which is why `$` looked correct. The parity test now compares the REF and LEVEL guard constants as well as the pattern block, because the block interpolates them and this bug lived in a line the old comparison never read. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VKbHCujejpxkgvh47BndDE
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.
Fixes #737.
The bug
v6.8.0 exempted a value that is exactly
read,write,none,true,falseornullfrom the five reference-guarded key-value rules, so thatid-token: writewould stop being read as credential material:All nine rules that interpolate
LEVELare built with the flags'gi'. Withoutm, JavaScript's$anchors to end of input, not end of line, so the exemption fired only when the permission line was the last content in the string:Why it matters more than the issue says
The same guard is duplicated in
secret-file-scanner.mjs, thePreToolUse Edit|Writehook, where input is multi-line by definition. The OIDC case the v6.8.0 changelog entry was written for never worked there at all. Onmaintoday, a realistic release workflow is still blocked:For
secret-detection.mjs(UserPromptSubmit) the effect is the one reported: any prompt or subagent report quoting the line mid-paragraph is blocked, which halted a long/arckit:repo-auditrun part-way through.Trailing whitespace after the value was a second latent false positive from the same anchor.
The fix
End of line is spelled out rather than relying on
$, which is one line per file instead of addingmto nine regex constructors:token: writeKeyABC123Why CI did not catch it
All four capability vectors in
tests/plugin/scanner-reference-guard.test.mjswere single-line strings with nothing after them, so$coincided with end of line by accident.This PR adds six regression tests covering the multi-line and trailing-whitespace cases against both hooks, plus two vectors asserting literal credentials are still caught when they are not the last line. Verified they fail on the old guard and pass on the new one:
The parity test now also compares the
REFandLEVELconstants, not just theSECRET_PATTERNSblock. The block interpolates both, so the copies could drift on the guards undetected, which is exactly where this bug lived.Note for the reporter
@johnfelipe your trace was right on the mechanism. One correction:
CLAUDE_CODE_OAUTH_TOKENis not the trigger. The underscore is a word character, so\b(token|auth_?token|...)never matches inside..._OAUTH_TOKEN. The trigger isid-token: write, where the hyphen supplies the boundary, which you identified in your second note.Your workaround reasoning was also correct: the CLI loads hooks from
~/.claude/plugins/cache/, so patching a local checkout has no runtime effect and a cache patch is overwritten on the next plugin update. This release removes the need for either.Verification
node --test tests/plugin/*.test.mjs tests/plugin/test_*.mjs— 296 pass, 0 failpython -m pytest tests/codex -q— 35 passpython scripts/converter.py— exit 0, fix propagated toarckit-kimi(the only extension shipping these hooks verbatim); no hand-maintained Codex/Gemini hook carries a duplicate guardpython scripts/sync-shared-assets.py --check— all 9 shared assets present in 14 community pluginsnpx markdownlint-cli2 CHANGELOG.md— 0 issues🤖 Generated with Claude Code
https://claude.ai/code/session_01VKbHCujejpxkgvh47BndDE