fix(hooks): an escaped quote inside a double-quoted word does not close it (guard-shared-stash split_segments) - #14839
Merged
Conversation
…se it
guard-shared-stash.sh's split_segments() had no backslash branch on its in-quote
path, so a `\"` inside a `"…"` word was read as CLOSING the quoted region. The
pass then went "outside quotes" while bash was still inside, separators behind it
split where bash would not, and the tail of a pure READ became a segment of its
own — judged on its own head word. That is a false BLOCK on a command that
touches no stash, which is exactly the failure the hook's own header promises can
never happen ("writing *about* the ban is never caught by the ban").
Port the in-quote branch guard-main-checkout-bash.sh's split_segments() already
carries, in the same shape and with the same escapee list (" \ $ `), gated on
q='"' because inside '…' nothing is special. This is the second half of the
backslash rule; the out-of-quotes half landed earlier at the `'\')` case.
The same missing rule also failed OPEN in the other direction: once the escapes
pair up the region was left hanging, and a real `git stash pop` behind a closed
quote rode through as a mere argument. Both directions are now pinned.
Self-test grows 44 -> 51 cases: two allow-cases for the nested-escape reads, two
precision twins that were falsely allowed, and three that blocked before and
still block (a plain closed quote, an escaped backslash before the closing quote,
and the single-quote asymmetry that pins the q='"' gate). All seven were run
against the pre-fix hook first — four red, three green — then all 51 green after.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01LAwHpn4uVuf4N1geBcD5i3
Contributor
Author
|
PM status ( Generated by Claude Code |
os-zhuang
approved these changes
Sep 3, 2026
os-zhuang
marked this pull request as ready for review
September 3, 2026 04:54
os-zhuang
enabled auto-merge
September 3, 2026 04:54
This was referenced Sep 3, 2026
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 #11804
guard-shared-stash.sh'ssplit_segments()had no backslash branch on its in-quote path, so an escaped quote inside a double-quoted word was read as closing the quoted region. The pass then went "outside quotes" while bash was still inside, separators behind it split where bash would not, and the tail of a pure read became a segment of its own — judged on its own head word. That is a false BLOCK on a command that touches no stash, which is exactly what the hook's own header promises can never happen ("writing about the ban is never caught by the ban", objectstack#4890's lesson).This is the
#10406half of the backslash rule. PR #11803 landed the out-of-quotes half at the'\')case; this card is the other half only, as that PR's author and the triage seat both recorded.The fix
Ported from
guard-main-checkout-bash.sh'ssplit_segments(), which already carries this branch — same shape, same escapee list (double quote, backslash, dollar, backtick), gated onqbeing a double quote because inside single quotes nothing is special. Seven lines; no other quoting rule touched, no tokenizer refactor, andguard-main-checkout-bash.sh/guard-main-checkout.share untouched precedent.I verified the branch against bash semantics rather than copying blind: inside a double-quoted region a backslash is special only before those four characters, and inside single quotes nothing is. The two guards'
split_segments()were compared on the current tree; they had not diverged in shape since #11803, so the port applies as-is minus thewordbookkeeping, which has no analogue here (this pass has no comment rule).Measured — the card's three rows, on
2d40f9146Fed as the PreToolUse payload shape
{cwd, tool_name:"Bash", tool_input:{command}}, exit code captured before any pipe.grep -rn "cd x && git stash pop" .claude/grep -rn "he said \"cd x && git stash pop\" once" .claude/echo "he said \"x && git stash pop\" once"The single-level control passes throughout, so the guard is reached and the nested escape alone is the difference — the change is not a loosening.
The defect reproduced on me while I worked: the live pre-fix hook in the shared checkout blocked the
Bashcall that was writing the repro harness, because the harness quoted the example. That is the failure mode the header names, observed rather than argued.Self-test — 44 to 51 cases, run against the pre-fix hook first
Per the triage instruction, the paired cases were written and run before the fix existed. Against the pre-fix hook: 47 passed, 4 failed. After: 51 passed, 0 failed.
The four that were red pre-fix, in both directions:
expect allow 'grep -rn "he said \"cd x && git stash pop\" once" .claude/'— was blockedexpect allow 'echo "he said \"x && git stash pop\" once"'— was blockedexpect block 'echo "he said \"x\"" && git stash pop'— was allowedexpect block 'echo "he said \"x\"" ; git stash drop'— was allowedThat second pair is worth flagging: the same missing rule also failed open. Once the escapes pair up the region was left hanging, every later separator went inert, and a real
git stash popbehind a properly closed quote rode through as a mere argument. The card scoped itself to the false block; the false allow fell out of the same branch and is now pinned too.Three precision twins blocked before and still block, so the branch cannot have become a blanket "ignore whatever follows a backslash":
echo "quoted" && git stash pop— a plain closed quoteecho "a \\" ; git stash pop— an escaped backslash, so the next quote still closes; without the backslash arm of the case list that closing quote is eaten and the stash rides throughecho 'a \' ; git stash pop— the single-quote asymmetry, which pins theqgate; applied to single quotes the branch would swallow the closing quoteAblation
Run from the committed state, deleting only the new branch. Mutation confirmed on disk before measuring — the gate line count went 1 to 0 and the blob hash moved off the HEAD blob (
086c457atoec18b884); the mutation script aborts non-zero on a shape mismatch rather than writing nothing (it did abort twice on a bad anchor, and those runs were discarded, not silently retried). Mutated: exit=1, 47 passed / 4 failed — the four above. Restore proven by observation, not by exit code:git diff HEADempty and the blob hash back to086c457a, after which the self-test reads 51 passed, 0 failed.No rebuild leg applies here and none is claimed: the self-test execs the hook file by path, so there is no compiled artifact between the edit and the measurement.
Gates — union re-run after the final commit, at
573be135All six families derived by
node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstackfrom the real change set (2 paths, working tree), plus the always-runs hook self-test step andcheck:nul-bytes. Exit codes captured before any pipe.pnpm check:agent-test-spellingexit=0pnpm check:bash32-floorexit=0 — "26 tracked shell file(s) ... name no bash 4+ construct"pnpm check:doc-authoringexit=0 — "393 files clean"pnpm check:pm-governed-mergesexit=0 — "243 assertions"pnpm check:skill-frame-syncexit=0 — "12 cases pass"pnpm --filter @objectstack/lint run check:doc-formula-expressionsexit=0 — "58 cases passed"pnpm check:nul-bytesexit=0 — "scanned 8070 text file(s) ... no raw ASCII control bytes".claude/hooks/*.selftest.sh(CI's always-runs step): 49 / 121 / 87 / 51 / 36 passed, 0 failedcheck:doc-formula-expressionsfirst answered exit=3, PREREQUISITE NOT MET twice (@objectstack/formula, then@objectstack/lintunbuilt). That is the gate's own "nothing was measured" code, not a finding; both packages were built throughscripts/pm/os-verify-lock.shand the gate then ran green.skip-changesetapplies:.claude/**publishes nothing from any released package.Out of scope, filed as a finding for the PM — objectui carries the same defect
The card asked for this to be probed rather than trusted. Probed, not assumed: on
objectstack-ai/objectuiat6411def,.claude/hooks/guard-shared-stash.sh'ssplit_segments()carries the out-of-quotes branch and not the in-quote one, and the same three commands measureallow / block / blockthere — identical to this repo before the fix. That repo's ownguard-main-checkout-bash.shalready has the in-quote branch, so objectui is internally divergent in exactly the way this PR just repaired here. Cross-repo work is out of this card's scope and this session has read-only access to objectui, so it goes to the PM rather than into this PR.Governed surface: this stays a draft for human merge. Not flipped ready, no auto-merge armed.
🤖 Generated with Claude Code
Generated by Claude Code
Generated by Claude Code