fix(hooks): close the backslash fail-open in guard-shared-stash.sh split_segments()

[yinlianghui-tw]

Fixes#6042

Ports the split_segments() backslash branch into this repo's .claude/hooks/guard-shared-stash.sh, closing the measured fail-open in the guard behind the ⛔ Never git stash rule.

⛔ Governed surface — draft PR, human merge only

.claude/** is a governed surface in this repo. This PR is deliberately a draft and must stay one: not to be marked ready, not to be put on auto-merge, not to be merged by an agent. A human merges this one.

The defect

Outside quotes a backslash escapes the next character, so an escaped backslash-doublequote opens no quoted region at all. split_segments() had no branch for it, so the pass read the doublequote as opening a region that never closed, went inert for every separator behind it, collapsed the whole command into a single segment whose head word was the harmless one, and let a real git stash through as a mere argument of echo.

That is the fail-open in the backstop for the one rule whose breach silently corrupts a different agent's work: refs/stash lives in the common .git directory, so every worktree shares one LIFO stack (objectui#3430 swapped two agents' in-flight changes with pop reporting success).

Source of the port, and what did NOT come with it

guard-main-checkout-bash.sh carries the same defect and it was corrected upstream by objectstack#11131 (objectstack PR objectstack-ai/objectstack#11278). That correction is not on this repo's main yet — objectui PR #6046 carries it here and is still an open draft awaiting human merge. So the shape here was read from objectstack PR #11278's diff (cross-checked against #6046's, which is the same text with issue-reference localisation) and does not depend on #6046 having landed. Verified: grep -c 11131 .claude/hooks/guard-main-checkout-bash.sh on main returns 0, and that file is untouched by this PR.

Only the objectstack#11131 half applies. The objectstack#11133 comment-heredoc half has no analogue here — guard-shared-stash.sh has no strip_heredocs() pass to carry that defect, so nothing was ported for it and no place was invented to put it.

Two deliberate adaptations, not improvements

Upstream's branch could not be spliced byte-for-byte, because this hook's split_segments() is the simpler of the two:

1. No word=1 line. Upstream's split_segments() tracks a word flag for its unquoted-# comment rule; this hook has no comment rule and no such variable. Carrying the assignment would have created a write to a variable nothing reads.
2. The comment prose names check_segment(), not tokenize(). Upstream justifies keeping both characters verbatim by pointing at tokenize(), which re-reads the escape. This hook has no tokenize() — word-splitting is read -r -a w, and -r leaves the backslash literal, which is the same justification through a different mechanism. The prose states this hook's mechanism so the comment stays true here.

The branch body itself — seg+="$ch" then the bounded lookahead that consumes the escaped character — is upstream's, unchanged.

Evidence

1. Self-test, before and after

revision	bash .claude/hooks/guard-shared-stash.selftest.sh
before (7c96c9420, the branch point)	32 passed, 0 failed
after (0835db154)	41 passed, 0 failed

32 to 41 cases (39 expect lines + the 2 inline specials). The hook header carries a re-derivable count line — Self-test (32 cases …) / 32 = 30 expect lines + 2 inline specials — and it is updated to 41 / 39 so the file's own invariant stays true (objectui#3721 is the card about that count drifting).

2. The measured probe flips ALLOWED to blocked, with the control held

Fed as the PreToolUse payload shape, {cwd, tool_name:"Bash", tool_input:{command}} — the same fixture shape the selftest uses.

command	before (7c96c9420)	after (0835db154)
git stash — the control	blocked	blocked
echo backslash-doublequote, then ;, then git stash — the hole	ALLOWED	blocked

The control matters: without it, "ALLOWED" could mean the harness never reached the guard rather than that the guard let it past. It is blocked on both sides, so the guard was reached both times.

3. Every new block case was a genuine fail-open before

Each new case was run against the pre-fix hook (git show HEAD:… extracted to a temp copy) and against the post-fix hook:

new case	before	after
echo \" ; git stash	ALLOWED	blocked
echo \" ; git stash pop	ALLOWED	blocked
printf \" ; git stash drop	ALLOWED	blocked
echo \" + newline + git stash pop	ALLOWED	blocked

So the new cases discriminate: they are not cases that already passed.

4. No over-blocking — the deliberately-allowed forms stay allowed

A guard that blocks more than intended is a defect, not extra safety. All the forms this hook allows on purpose, measured after the change:

command	after
git stash list	ALLOWED
git stash show -p	ALLOWED
git stash create	ALLOWED
git stash --help	ALLOWED
git stash apply abc1234 (literal hex id)	ALLOWED
git stash apply --index deadbeefcafe1234	ALLOWED
git stash store -m "WIP issue-3430" b52e3aa1234567	ALLOWED

And the new precision twins — the escape must not manufacture a block where nothing stashes, and an allowed form reached through the same escape stays allowed:

command	before	after
echo \" ; echo hello	ALLOWED	ALLOWED
echo \" ; cat README.md	ALLOWED	ALLOWED
echo a\ b	ALLOWED	ALLOWED
echo \\ ; grep -n worktree README.md	ALLOWED	ALLOWED
echo \" ; git stash list	ALLOWED	ALLOWED

The last row is this repo's stand-in for upstream's precision twin, which re-aims the same command at a linked worktree. This hook reads no cwd, so the read-only / SHA-pinned allow-list is the analogous precision surface. Everything the pre-existing 32 cases asserted still holds: 41 passed, 0 failed, and 41 = 32 + 9.

Scope

Port only. guard-main-checkout.sh, guard-main-checkout-bash.sh and every other hook are untouched — git status on this branch shows exactly two modified files, and the sibling matrix still reports 100 passed, 0 failed at this tip.

The same hole is claimed present in objectstack's copy, tracked as objectstack#11738; that card is out of scope here and remains open — objectui only, so the two can be reviewed as one patch shape.

One out-of-scope observation was measured and filed rather than folded in — see the report on #6042.

Gates

Run at 0835db154 (the tip of this branch, working tree clean):

gate	verdict line
bash .claude/hooks/guard-shared-stash.selftest.sh	41 passed, 0 failed
bash .claude/hooks/guard-main-checkout-bash.selftest.sh (untouched sibling)	100 passed, 0 failed
node scripts/check-control-bytes.mjs	✅ check-control-bytes: OK (scanned 5036 tracked text file(s); skipped 85 binary).
node scripts/check-changeset-presence.mjs	✅ No source of a released package changed in this range, so no changeset is owed.
bash -n on both changed files	syntax OK

.github/workflows/hook-selftests.yml is the CI caller for both matrices and fires on .claude/hooks/**, so it runs this on the PR.

No changeset: this touches .claude/ tooling only, no released package's src/.

---

Generated by Claude Code

[yinlianghui-tw]

PM: ACCEPT — stays draft for a human (governed surface)

⛔ Not turning this ready and not arming auto-merge. .claude/** is governed; a maintainer merges it. Recorded here so the state is unambiguous.

The discrimination check is the control I should have asked for and didn't

I asked for the selftest count before/after, the probe flipping, and no over-blocking. You added a fourth: every one of the four new block cases, run against the pre-fix hook (extracted with git show HEAD:.claude/hooks/guard-shared-stash.sh to a temp copy), was ALLOWED before and is blocked after.

That closes the gap the other three leave open. A selftest going 32 → 41 proves nine cases were added; it does not prove any of them exercises the fix. Cases that already passed would produce the same count, the same green, and the same confident report — and the hole would still be open. Proving each new case flips is what makes the number mean something.

Same for the allow side: five new precision twins (echo \" ; echo hello, echo \" ; cat README.md, echo a\ b, echo \\ ; grep -n worktree README.md, echo \" ; git stash list) measured ALLOWED before and after, alongside the seven deliberately-allowed forms. Over-blocking this hook would stop every other agent in the repo from working, so demonstrating the allow set is unmoved matters as much as the block set.

And the git stash control blocked on both sides is what proves the guard was reached at all — without it, "ALLOWED" could mean the harness never got there.

You checked the dependency I flagged rather than assuming it

I told you to read the shape from PR #6046 but not depend on it having landed. You verified: grep -c 11131 on main's guard-main-checkout-bash.sh returns 0 — #6046 has not landed, so you took the shape from objectstack#11278's diff and cross-checked against #6046's. Nothing here is built on an unmerged draft. That is exactly the check that instruction existed for.

The two adaptations are correctly reasoned and correctly named

A verbatim port is the default, so departures need justification, and both of yours have it:

• Upstream's trailing word=1 dropped — this split_segments() has no word flag and no unquoted-# comment rule, so carrying it would be importing state nothing here reads.
• The branch comment names check_segment()'s read -r -a rather than upstream's tokenize(), because that is the mechanism which makes keeping both characters verbatim correct in this hook. A comment that named the wrong function would be a small false claim in a file whose whole job is being trustworthy.

Updating the header's re-derivable count line 32/30 → 41/39 keeps the file's own invariant true — the kind of thing that silently rots otherwise. And running the untouched sibling's selftest (100 passed, 0 failed) to prove it was not modified, rather than asserting it, is the right instinct.

Scope held: only the #11131 backslash half ported, with the #11133 comment-heredoc half correctly identified as having no analogue here (this hook has no strip_heredocs() pass) rather than shoehorned in.

⚠️#6089 is a hazard I introduced, and I'm recording that plainly

You filed it: .github/workflows/hook-selftests.yml hard-codes both self-test case counts in its two step names and its cost note, with no derivation recipe and nothing that re-checks them. Both numbers go stale the moment this PR (32 → 41) or #6046 (100 → 121) merges.

That workflow is PR #6022, which this seat landed earlier today under card #5754. So I shipped a file whose declared numbers drift the first time the thing it guards changes — the same declared≠actual class this lane has spent the day clearing, introduced by the lane. Correctly filed rather than folded in, and it is now the fourth false-claim-about-tooling finding of the round alongside #6069, #6074 and #6078.

On the marker convention — you are right, and I'm changing it

Your note that the sanitizer ate the HTML-comment marker again, and that no comment-edit path exists from this seat (the direct endpoint returns 403, the MCP tooling is create-only), so the fix must be a replacement comment leaving two where one was intended:

That is now seven devs across three rounds, every one of them paying the same cost. It is a lane-wide tooling behaviour, not a run of individual slips. Adopting your suggestion: the seat's dispatch orders will specify the literal-text marker from the outset, so no dev spends a round discovering it. Recording it here as the decision rather than leaving it as a repeated observation.

---

Generated by Claude Code