Skip to content

fix(hooks): an escaped quote inside a double-quoted word does not close it - #7695

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-7441-stash-guard-in-quote-backslash
Sep 5, 2026
Merged

fix(hooks): an escaped quote inside a double-quoted word does not close it#7695
os-zhuang merged 2 commits into
mainfrom
claude/issue-7441-stash-guard-in-quote-backslash

Conversation

@os-steve

@os-steve os-steve commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7441

Ports the in-quote backslash branch of split_segments() into
.claude/hooks/guard-shared-stash.sh, from objectstack 2b9f5810b (PR objectstack-ai/objectstack#14839).

The rule

Inside "…" a backslash is special only before " \ $ `, so an escaped \" is a
literal quote and the quoted region stays open. This pass read it as closing, went
outside quotes while bash was still inside, and separators behind it split where bash would
not. Two directions, both now pinned:

  • false BLOCK — the tail of a pure read became a segment of its own and was judged on its
    own head word, so a command that touches no stash was blocked. That is the one failure the
    hook's own header promises can never happen ("writing about the ban is never caught by the ban").
  • fail OPEN — once the escapes pair up, the region was left hanging and a real stash
    command behind it rode through as a mere argument.

Inside '…' nothing is special, hence the q = '"' gate.

Premise re-measured on this branch point (A1)

The card measured objectui at 6411def; re-measured here at 51eb515, fed as the PreToolUse
payload {cwd, tool_name:"Bash", tool_input:{command}}:

payload before after
grep -rn "cd x && git stash pop" .claude/ (control) allow allow
grep -rn "he said \"cd x && git stash pop\" once" .claude/ BLOCK allow
echo "he said \"x && git stash pop\" once" BLOCK allow

The control holds on both sides, so the nested escape alone is the difference.

The ported hunk

split_segments(), in-quote path, .claude/hooks/guard-shared-stash.sh:

     if [ -n "$q" ]; then
+      if [ "$q" = '"' ] && [ "$ch" = '\' ] && [ $((i + 1)) -lt "$n" ]; then
+        case "${s:i+1:1}" in
+          '"' | '\' | '$' | '`')
+            seg+="$ch" ; i=$((i + 1)) ; seg+="${s:i:1}" ; continue ;;
+        esac
+      fi
       seg+="$ch"
       [ "$ch" = "$q" ] && q=""
       continue
     fi

Those six lines are byte-identical to the block this repo's own
guard-main-checkout-bash.sh split_segments() already carries, and byte-identical to
upstream 2b9f5810b — verified by diff on the block extracted from each of the three files
(indentation included). Before the port, this repo's split_segments() was code-identical to
objectstack's pre-fix copy; the only delta between them was an in-function comment.

The rest of the diff is the header case count (41 to 48, with its derivation line) and a
comment stating the rule. Upstream's comment block is deliberately not ported verbatim.

Red first, then green

The seven cases went into the self-test first and were run against the unfixed hook:

== an escaped \" INSIDE a double-quoted word does NOT close it ==
  FAIL want=allow got=block  grep -rn "he said \"cd x && git stash pop\" once" .claude/
  FAIL want=allow got=block  echo "he said \"x && git stash pop\" once"
  FAIL want=block got=allow  echo "he said \"x\"" && git stash pop
  FAIL want=block got=allow  echo "he said \"x\"" ; git stash drop
  ok   block  echo "quoted" && git stash pop
  ok   block  echo "a \\" ; git stash pop
  ok   block  echo 'a \' ; git stash pop

44 passed, 4 failed

After the fix, same matrix:

  ok   allow  grep -rn "he said \"cd x && git stash pop\" once" .claude/
  ok   allow  echo "he said \"x && git stash pop\" once"
  ok   block  echo "he said \"x\"" && git stash pop
  ok   block  echo "he said \"x\"" ; git stash drop
  ok   block  echo "quoted" && git stash pop
  ok   block  echo "a \\" ; git stash pop
  ok   block  echo 'a \' ; git stash pop

48 passed, 0 failed

41 to 48 cases; the four that flipped are the two nested-escape reads and the two fail-open
twins. The three precision twins block on both sides — they pin the branch against
degrading into "ignore whatever follows a backslash". The header's declared count is
re-derived by its own recipe: 46 expect lines + 2 inline specials = 48, equal to the run's
own tail.

Non-vacuity of the precision twins

On a throwaway copy, dropping the \ arm of the escapee list (the copy's only change, one
line, confirmed on disk by anchored greps: 0 occurrences of the arm-bearing line, 1 of the
mutated line):

fixed      -> block   echo "a \\" ; git stash pop
ablated    -> allow   echo "a \\" ; git stash pop
full self-test against the ablated copy:  47 passed, 1 failed

So that twin really does pin that arm. The real hook was never mutated — its blob hash equals
HEAD:.claude/hooks/guard-shared-stash.sh (c9b0b59bf1db0a7dd1e57007c1259819ac01d922).

The two main-checkout guards are untouched

They are the in-repo precedent for this port, not its surface:

$ git diff --stat origin/main -- .claude/hooks/guard-main-checkout.sh \
    .claude/hooks/guard-main-checkout-bash.sh \
    .claude/hooks/guard-main-checkout.selftest.sh \
    .claude/hooks/guard-main-checkout-bash.selftest.sh
(no output)

Their matrices still pass unchanged: guard-main-checkout-bash.selftest.sh 121 passed / 0 failed,
guard-main-checkout.selftest.sh 87 passed / 0 failed.

Changeset

node scripts/check-changeset-presence.mjs decides, and this repo has no skip-changeset label:

Compared the working tree with 51eb51558 (merge-base with origin/main): 2 file(s) changed,
0 of them published source of a package the release covers, 0 of them a manifest whose
published contract moved, 0 under a package changesets ignores, 0 changeset(s) added.
✅  No source or published contract of a released package changed in this range, so no changeset is owed.

No changeset added, on the gate's own verdict.

Gates run locally, at c424f18 (the head this PR pushes)

gate verdict
.claude/hooks/guard-shared-stash.selftest.sh exit 0 — 48 passed, 0 failed
.claude/hooks/guard-main-checkout-bash.selftest.sh exit 0 — 121 passed, 0 failed
.claude/hooks/guard-main-checkout.selftest.sh exit 0 — 87 passed, 0 failed
.claude/hooks/guard-tree-enum.selftest.sh exit 0 — 36 passed, 0 failed
check-control-bytes exit 0 — OK (6264 tracked text files scanned, 85 binary skipped)
check-shell-escape-residue exit 0 — OK (5/5 roots resolved)
check-governed-queue-guard --self-test exit 0 — OK
check-skills-paths exit 0
check-skill-eval-tokens exit 0 — every must_contain token is taught by its own skill bundle
check-changeset-presence exit 0 — no changeset owed (quoted above)
check-upstream-port-parity exit 0 — pin unaffected; it pins scripts/pm/check-half-states.mjs and scripts/invoked-as.mjs only, neither touched
check-skill-examples NOT MEASUREDERR_MODULE_NOT_FOUND: typescript (no install in this worktree); its scan roots are skills/, which this diff does not touch. CI runs it with a full install.
pnpm lint not applicable by eslint's own config — every files: glob in eslint.config.js is **/*.{ts,tsx} or narrower; no glob names .sh or .claude, so eslint never selects these files. Not a skip.

The self-test discovery was run the way hook-selftests.yml runs it
(find .claude/hooks -type f -name '*.selftest.sh'): 4 matrices discovered, 4 run, 4 green.

Governance

.claude/** is governed surface: draft PR, human merge. Not marked ready, not enqueued,
no auto-merge, not approved. Reviewer requests left to the dispatching seat — this workflow's
own header records that an MCP update_pull_request call passing only reviewers silently
set draft: false once, so this PR makes no post-creation update call.


🤖 Generated with Claude Code

Generated by Claude Code


Generated by Claude Code

…split_segments

Adds the paired cases to guard-shared-stash.selftest.sh ahead of the fix, so the
matrix is on record failing for the reason the missing branch explains.

Inside "…" a backslash is special only before " \ $ ` , so an escaped \" is a
literal quote and the quoted region stays OPEN. split_segments() reads it as
closing, which breaks in both directions: a pure read whose quoted text carries a
nested escape is falsely BLOCKED, and once the escapes pair up the region is left
hanging and a real stash rides through as a mere argument.

Run against the unfixed hook (41 -> 48 cases, 44 passed / 4 failed):

  == an escaped \" INSIDE a double-quoted word does NOT close it ==
    FAIL want=allow got=block  grep -rn "he said \"cd x && git stash pop\" once" .claude/
    FAIL want=allow got=block  echo "he said \"x && git stash pop\" once"
    FAIL want=block got=allow  echo "he said \"x\"" && git stash pop
    FAIL want=block got=allow  echo "he said \"x\"" ; git stash drop
    ok   block  echo "quoted" && git stash pop
    ok   block  echo "a \\" ; git stash pop
    ok   block  echo 'a \' ; git stash pop

  44 passed, 4 failed

The three ok rows are the precision twins. They hold on both sides of the fix and
pin the branch against degrading into "ignore whatever follows a backslash".

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox
…se it

guard-shared-stash.sh's split_segments() had no backslash branch on its in-quote
path. Inside "…" a backslash is special only before " \ $ ` , so an escaped \" is
a literal quote and the quoted region stays OPEN; the pass read it as CLOSING,
went outside quotes while bash was still inside, and separators behind it split
where bash would not. The tail of a pure READ became a segment of its own, judged
on its own head word — a false BLOCK on a command that touches no stash, which is
the one failure the hook's own header promises can never happen. The same gap also
fails OPEN in the other direction: once the escapes pair up the quoted region is
left hanging and a real stash command behind it rides through as a mere argument.

Ports the in-quote branch this repo's guard-main-checkout-bash.sh split_segments()
already carries, in the same shape and with the same escapee list (" \ $ `), gated
on q='"' because inside '…' nothing is special. That guard and guard-main-checkout.sh
are the precedent here and are untouched.

Self-test: 48 passed, 0 failed. Against the unfixed hook one commit ago the same
matrix read 44 passed, 4 failed — the four that flipped are the two nested-escape
reads (false BLOCK) and the two fail-open twins; the three precision twins blocked
on both sides. Header case count updated 41 -> 48.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019RfFHiRCSs3JXLK4cwcfox

os-steve commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

ACCEPT — governed in-seat review of PR #7695, head c424f18f (skills seat, 2026-09-05T03:5xZ).

Implemented-by: claude/issue-7441-stash-guard-in-quote-backslash (os-dev subagent)
Reviewed-by: session_019RfFHiRCSs3JXLK4cwcfox (skills seat)

Verified in a detached compare worktree at the PR head against merge-base 51eb51558, not from the report: two files. .claude/hooks/guard-shared-stash.sh gains the in-quote backslash branch of split_segments() — inside "…" a \ before " \ $ or ` keeps both characters and does not close the region, gated on q = '"' — and the seat diffed that six-line block against objectstack main's copy: identical; the comment above it states the rule and its two failure directions without card numbers or measurement narrative; the header's self-test count is re-derived 41 → 48. The self-test gains the seven cases (two nested-escape reads now allowed, two paired-escape stashes now blocked, three precision twins) and was run red first against the unfixed hook (44 passed, 4 failed, the four the missing branch explains) and green after (48 passed, 0 failed — the seat's own run on the head agrees). guard-main-checkout.sh, guard-main-checkout-bash.sh and their self-tests are untouched to the byte. The dev's mutation on a throwaway copy (dropping the \ arm) flipped echo "a \\" ; git stash pop to allow, restore proved by blob hash; all four hook matrices the CI runner discovers pass locally (48 / 121 / 87 / 36). No changeset is owed by scripts/check-changeset-presence.mjs's own verdict; lint does not cover shell by eslint's config. CI on c424f18f: 30 check runs, 27 success, 3 skipped, none failing — Hook Self-Tests, Governed Surface Queue Guard and Changeset Declaration among them.

Landing regime: governed (.claude/**) ⇒ the PR stays a draft; reviewers os-zhuang + hotlong requested in this same act; human merge. Closes #7441 (its cross-repo blocker objectstack#11804 closed with PR #14839 on 09-03).


Generated by Claude Code

@os-zhuang
os-zhuang marked this pull request as ready for review September 5, 2026 06:08
@os-zhuang
os-zhuang added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 6eebc54 Sep 5, 2026
32 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-7441-stash-guard-in-quote-backslash branch September 5, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants