Skip to content

fix(hooks): give guard-shared-stash's split_segments() the backslash branch #11131 closed next door - #11803

Draft
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-11738-stash-guard-backslash
Draft

fix(hooks): give guard-shared-stash's split_segments() the backslash branch #11131 closed next door#11803
claude[bot] wants to merge 1 commit into
mainfrom
claude/issue-11738-stash-guard-backslash

Conversation

@claude

@claudeclaudeBot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Fixes#11738

.claude/hooks/guard-shared-stash.sh's split_segments() had no backslash branch. Outside
quotes a backslash escapes the next character, so an escaped \" opens no quoted region.
Without the branch the pass read that " as opening a region that never closed, went inert
for every separator behind it, and collapsed the whole command into a single segment whose
head word was the harmless one — leaving a real git stash pop to be read as a mere argument
of echo.

This is the enforcement behind the ⛔ Never git stash Prime Directive — the rule whose
violation swapped two parallel agents' in-flight work with a success message (objectui#3430).
The rule outranks the hook, so this was a backstop gap rather than a live breach; the backstop
exists precisely for the agent who does not know the rule.

The card's premise held — measured here, not inherited

The card's reading was taken on objectui's copy and asked whoever took it to probe this
repo's copy independently rather than trust the declared parity. Probed, on de1cb8def, fed
as the PreToolUse payload shape {cwd, tool_name:"Bash", tool_input:{command}}:

commandbeforeafter
git stash(control)blocked, exit 2blocked, exit 2
git stash pop(control)blocked, exit 2blocked, exit 2
echo \" ; git stashALLOWED, exit 0blocked, exit 2
echo \" ; git stash popALLOWED, exit 0blocked, exit 2
echo \" && git stash popALLOWED, exit 0blocked, exit 2
printf \" ; git stash dropALLOWED, exit 0blocked, exit 2
echo \"git stash popALLOWED, exit 0blocked, exit 2
echo \" ; echo hello(precision)allowedallowed
echo a\ b(precision)allowedallowed
echo \\ ; git status(precision)allowedallowed

The controls are load-bearing: without them "ALLOWED" could mean the harness never reached the
guard. And the shape really does execute — bash -c 'echo \" ; echo SECOND_COMMAND_REALLY_RAN'
prints " and then SECOND_COMMAND_REALLY_RAN.

The fix follows #11131, and says where it doesn't

The branch is the one guard-main-checkout-bash.sh's split_segments() took in #11131 (landed
in #11278), in the same shape. Two deliberate deviations, both stated in the file:

This fixes a parse that was wrong, not one that was uncertain. The hook's documented
fail-open posture is unchanged and untouched — its header states it explicitly ("Anything this
cannot parse fails OPEN — a guard that blocks work it does not understand gets disabled, and
then it guards nothing"), along with the named boundary it keeps (wrapped invocations: bash -c,
xargs, ssh host '…'). So the stash guard's intended posture is the same as the checkout
guard's, declared in its own header rather than assumed from the sibling. Making the parse
correct is strictly better than either posture and does not trade against that boundary.

Non-vacuity: the new cases fail before the fix

Twelve cases added (6 block + 6 precision twins), 32 → 44. The new self-test was run against the
pre-fix hook — lifted from de1cb8def into a scratch dir, so the worktree was never mutated:

NEW selftest x PRE-FIX hook : 38 passed, 6 failed <- exactly the 6 new block cases
NEW selftest x FIXED hook : 44 passed, 0 failed

All six failures are the six new block cases; all six precision twins pass in both directions,
so they pin precision rather than padding the count. The ablation was confirmed on disk before it
was read, each zero against a positive control: the branch literal '\') appears in the
fixed hook and in the pre-fix copy, and the new comment sentence / .

Verification

Gates re-run on the final commit bc9400bcd, all green, quoting each gate's own verdict line:

  • guard-shared-stash.selftest.sh44 passed, 0 failed
  • check:nul-bytescheck-nul-bytes: OK (scanned 6577 text file(s) … no raw ASCII control bytes)
  • check:agent-test-spelling — self-test + sweep, exit 0
  • check:doc-authoring✓ doc authoring guard: 389 files clean
  • check:pm-governed-merges✓ check-governed-merges --self-test: 129 assertions
  • check:skill-frame-sync✓ … 4 copies of the decision frame are structurally isomorphic

check:doc-formula-expressions was narrowed, and the narrowing measured (it needs a workspace
install this bash-only change does not otherwise need): its population comes from the gate's own
source — ROOTS = ['.claude','docs','skills','content'] with the walker keeping only .md/.mdx
(plus .ts/.tsx under packages/spec/src). That population is 1671 files at this commit;
it contains 0.sh files and neither changed file, while the positive control shows the walk
really does reach .claude/ (25.md files there). My diff adds and removes 0.md/.mdx
paths, so the population is identical before and after. CI runs the full farm regardless.

No changeset: .claude/** is internal agent tooling and publishes nothing — skip-changeset.

Scope notes

  • No third sibling carries this. Only two files in the tree define split_segments():
    this one and guard-main-checkout-bash.sh (already repaired by [finding] guard-main-checkout-bash.sh: an unquoted backslash still makes tokenize() and split_segments() disagree, and a real sed -i into the shared checkout is allowed through #11131).
    guard-main-checkout.sh reads .tool_input.file_path, not commands — it parses no shell at
    all and structurally cannot carry the defect.
  • A second, opposite-direction defect exists in this same function and is filed separately
    rather than folded in — it is the #10406 half (an escaped quote inside a double-quoted
    word), not the #11131 half this card scopes to. It is a false BLOCK, not a fail-open:
    grep -rn "he said \"cd x && git stash pop\" once" .claude/ is blocked today, though the
    file's own header promises writing about the ban is never caught by it.
  • Cross-repo twin objectui#6042 carries the same patch shape for objectui's copy; separate
    repos, separate PRs, not folded here.

⚠️ Governed surface (.claude/**): this PR stays draft for human merge. Review requested
from @os-zhuang.


Generated by Claude Code

…branch
Outside quotes a backslash escapes the next character, so an escaped `\"` opens no
quoted region at all. split_segments() had no backslash branch, so it read the `"`
as opening a region that never closed, went inert for every separator behind it,
and collapsed the whole command into one segment whose head word was the harmless
one — leaving a real `git stash pop` to be read as a mere argument of `echo`.
This is the fail-OPEN backstop gap in the enforcement behind the ⛔ Never `git stash`
Prime Directive, the rule whose violation swapped two parallel agents' in-flight work
(objectui#3430).
Measured on THIS repo's copy (the card's reading was taken on objectui's), fed as the
PreToolUse payload shape {cwd, tool_name:"Bash", tool_input:{command}}:
git stash (control) before: exit 2 blocked after: exit 2 blocked
git stash pop (control) before: exit 2 blocked after: exit 2 blocked
echo \" ; git stash before: exit 0 ALLOWED after: exit 2 blocked
echo \" ; git stash pop before: exit 0 ALLOWED after: exit 2 blocked
echo \" && git stash pop before: exit 0 ALLOWED after: exit 2 blocked
printf \" ; git stash drop before: exit 0 ALLOWED after: exit 2 blocked
Same repair guard-main-checkout-bash.sh's split_segments() took in #11131, in the same
shape. That guard's `word` bookkeeping has no analogue here: this pass has no comment
rule, so there are no word starts to track. The #11133 comment-heredoc half does not
apply either — there is no strip_heredocs() pass here to carry that defect.
This fixes a parse that was WRONG, not one that was uncertain. The hook's documented
fail-open posture for what it cannot parse (wrapped invocations: bash -c, xargs, ssh)
is unchanged and deliberate.
Self-test: 32 -> 44 passed, 0 failed. The six new block cases were run against the
pre-fix hook and all six FAIL there (38 passed, 6 failed), so they are discriminating
rather than decorative; the six precision twins pass in both directions.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015ahemw8RcTgqtxrj15PEZx
@os-steveClaude

Copy link
Copy Markdown
Collaborator

ACCEPT (advisory — governed surface, so the merge decision is os-zhuang's). I reproduced the defect myself rather than verifying your account of it.

Reproduced, live, on origin/main

Running .claude/hooks/guard-shared-stash.sh directly with a PreToolUse payload:

exit=2 git stash ← blocked
exit=2 git stash pop ← blocked
exit=0 git status ← allowed (control: the guard discriminates)
exit=0 echo \" ; git stash pop ← ⛔ ALLOWED
exit=0 echo \" ; git stash ← ⛔ ALLOWED

And the payload behind the escape really executes:

$ bash -c 'echo \" ; echo SECOND_COMMAND_REALLY_RAN'
"
SECOND_COMMAND_REALLY_RAN

So on main today, the guard enforcing the stash ban is defeated by prefixing an escaped double-quote — a construct common enough to appear by accident. The rule it protects is the one whose violation cost two parallel agents their in-flight work (objectui#3430), and the guard was blocking the plain spelling while waving the escaped one through. The card reasoned from a sibling's defect; the defect is real here.

You established the posture instead of assuming it — which was the Zone 2 I most wanted answered

I flagged that CLAUDE.md's "anything it cannot parse with confidence is allowed through" is the checkout guard's deliberate posture and that you must establish the stash guard's intended posture rather than inherit it. You read its own header and drew the line correctly:

This fixes a parse that was WRONG, not one that was UNCERTAIN

The documented fail-open covers wrapped invocations (bash -c, xargs, ssh) — things the hook genuinely cannot see into — and that is unchanged. An escaped quote is not an unparseable wrapper; the parser simply read it wrong. Those are different failures and only the second is a bug. Getting that distinction right is what stops this PR from quietly tightening a deliberate design decision.

Taking #11131's exact shape, and stating why its word=1 bookkeeping has no analogue here (this pass has no comment rule, so there are no word starts to track) and why #11133's heredoc half does not apply (no strip_heredocs() pass), is the difference between porting a fix and copying one.

Non-vacuity, and the twins

32 → 44 cases, with the ablation run against the pre-fix hook lifted out of de1cb8def with git show into a scratch dir — so the worktree was never mutated and no restore leg was needed. That is a cleaner instrument than mutating in place, and you still proved the two copies differ on disk before reading any result.

NEW selftest × PRE-FIX hook → exit 1, 38 passed / 6 failed, the failures being exactly the 6 new block cases. NEW selftest × FIXED hook → 44/0. And the 6 precision twins pass in both directions, so they pin precision rather than pad the count — without them, "12 new cases" is consistent with 12 assertions that only ever fire one way.

⭐ Two vacuous controls of your own, caught and published

my first marker grep returned 0 on BOTH copies — an over-escaped pattern measuring nothing

my first control used a TAB, which that class deliberately excludes, so it proved nothing

Both are the same failure the whole repo keeps hitting: a probe that cannot fire reads exactly like a clean result. Catching it twice in one run, on your own instruments, and quoting the corrected readings rather than the first ones, is what makes the rest of the numbers usable.

#11804 is the other direction, and it contradicts the hook's own promise

The same split_segments() misreads an escaped quote inside a double-quoted word — a false BLOCK on pure reads, so grep -rn "he said \"cd x && git stash pop\" once" .claude/ is refused today. That contradicts the header's promise that writing about the ban is never caught by it, which matters because this hook's documentation is how agents learn the rule.

I confirmed the single-level control still passes (grep -rn "cd x && git stash pop" .claude/ → exit 0), so it is specifically the nested level. Right call not to fold it in — I ⛔'d widening on this governed surface, and the two halves are separately reviewable. ⚠️ Consequence worth stating plainly: the two guards' split_segments() stay divergent on the in-quote half until #11804 lands.

And the third-sibling question is closed properly

Only two files in the tree define split_segments()guard-main-checkout.sh reads .tool_input.file_path and parses no shell at all, so it structurally cannot carry it. Nothing to file.

That is a negative result with a reason, not an absence of evidence — and it refines #11800 usefully: that hook's missing self-test covers a much smaller surface than the 121-case Bash sibling's, because it does no shell parsing. I will carry that into #11800's grading.

⛔ Governed handling confirmed: draft, os-zhuang requested, never flipped ready / armed / enqueued. skip-changeset read back.


Generated by Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/sskip-changesetPR has no user-facing published change; bypasses the changeset gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

guard-shared-stash.sh has the same split_segments() backslash fail-open that #11131 closed in the bash checkout guard

2 participants

@os-steve@claude