Skip to content

fix(hooks): read a command substitution inside a double-quoted word as its own quoting context - #24886

Merged
objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24812-guard-hook-cmdsubst-quotes
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 3 commits into
mainfrom
claude/issue-24812-guard-hook-cmdsubst-quotes

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Fixes #24812

Clause-②: no

What

.claude/hooks/guard-main-checkout-bash.sh read a " inside a $(…) (or a backtick) that sits inside a double-quoted word as the END of the outer string. The rest of the substitution body was then scanned as bare shell, so the > inside a quoted curl JSON body became a redirect and the next fragment a write "target", and a read-only command run from the primary checkout was blocked.

One helper, subst_end(), now answers where such a substitution ends, reading its body with its own quotes, escapes and nesting ((…), $(…), backticks). All three quote-aware scanners (the heredoc comment scan strip_line_comment(), segmentation split_segments(), tokenisation tokenize()) delegate to it from their in-double-quote branch, so they agree on the extent by construction. The body is kept verbatim as part of the word, so a target holding it still fails open on its $, which is the header's existing promise for $(…) targets.

Diff: 2 files, +77 / -1 (.claude/** only, Tier S).

Approach: why the substitution is tracked rather than failed open

The card offers two routes, and the dispatch preferred failing open on any double-quoted word containing $( for its smaller parser surface. I measured what that route gives up before choosing. The hook cannot know where an unreadable word ends, so failing open on it means claiming nothing from that segment onward. On the pre-fix hook these three writes into the primary checkout are all blocked, correctly, and that route would let all three through: sed -i "s/a/$(date +%F)/" pkg/x.ts (the reflexive in-place edit this guard exists for), X="$(git rev-parse HEAD)" ; sed -i s/a/b/ pkg/x.ts (any write behind a quoted-substitution preamble), and echo "$(date)" > pkg/x.ts. Tracking the substitution costs one ~30-line helper that answers a single question (where the body ends) and never classifies anything. It is called from all three scanners' existing in-quote branch, so the passes cannot drift apart on it. Measured against the pre-fix hook, it never reads a command worse, with one exception: a ( or ) that bash would not count (a case pattern, a comment inside the substitution) moves the end. Too early, and the rest is read exactly as before. Too late, and the swallowed text is inert, i.e. fails open. That boundary is written in the helper's header. The selftest pins the recall boundary in both directions (the three writes above stay blocked), so the alternative route would go red on it.

Measurement (cwd = primary checkout; payload {"tool_input":{"command":…},"cwd":"/home/user/objectstack"})

command pre-fix b1676441b this PR e4ba00dc9
echo "a $(curl -d "{\"c\":\"x > 1\"}" http://h)" 2, target: 1" 0
echo "a $(curl -d '{"c":"x > 1"}' http://h)" 2, target: 1}' http://h) 0
curl -s -d "{\"c\":\"x > 1\"}" http://h 0 0
control echo hi > out.txt 2, target: out.txt 2, target: out.txt
original probe echo "PUT $1 -> $(curl … -d "{…\"condition\":\"int(record.title) > 1\"…}" …)" 2, target: 1" 0
backtick spelling of row 1 2, target: 1" 0
echo "$(date)" > /home/user/objectstack/f 2 2
X="$(git rev-parse HEAD)" && echo "$X" > /home/user/objectstack/f 2 2

H2 (both passes share the defect): confirmed by per-site ablation. Each mutation went through scripts/ablation-replace.mjs in WRAP mode at 0963684e5: one call site replaced by a no-op, the hook self-test run as the child, then a restore proven by blob equality with HEAD and an empty git diff HEAD. Results:

  • segmentation only reverted: 3 red (card form 1, the backtick form, the fail-open mirror);
  • tokenisation only reverted: 2 red (both card forms);
  • heredoc comment scan only reverted: 1 red (the heredoc pin);
  • the helper's nested-$( arm removed: 1 red (the nested pin).

So no single pass fixes it alone. The whole pre-fix hook (b1676441b, blob 1fd91d0a8) against the new self-test: 136 passed, 5 failed. The 5 are both card forms, the backtick form, the fail-open mirror and the heredoc pin.

The first A2/A3 attempts were refused by the tool itself, not run: a bare : replacement collided with the : already in the anchor. They were re-run with unique markers. e4ba00dc9 after the ablations is comment-only.

H3 (a real redirect inside a quoted $(…)). Pre-fix: echo "$(cat x > /home/user/objectstack/f)" exit 0, and x="$(sed -i s/a/b/ /home/user/objectstack/README.md)" exit 0. The write sat inside the string, by accident of the quote reading. After: the same, exit 0. The answer is unchanged, but it is now a deliberate fail-open, listed among the wrapped invocations in the header and pinned in the self-test's "deliberately NOT claim" section. The UNQUOTED echo $(cat x > /home/user/objectstack/f) is still blocked (exit 2), unchanged.

Tests and gates, all at HEAD e4ba00dc9

  • bash .claude/hooks/guard-main-checkout-bash.selftest.sh: 141 passed, 0 failed (130 on main; +11 cases).
  • The other hook matrices in .claude/hooks/: guard-main-checkout 120/0, guard-process-kill 69/0, guard-shared-stash 53/0 and guard-tree-enum 38/0.
  • node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack derived 16 families. All 16 ran with exit 0, captured before any pipe. --ran reconciliation: 16 derived, 16 run, 0 NOT-MEASURED, 0 UNRUN.
  • pnpm check:bash32-floor reports 19 constructs checked, floor bash 3.2. The helper uses only ${s:i:n}, +=, ${st%?} and arithmetic for. There is no recursion, no mapfile and no negative substring offset.
  • check:doc-formula-expressions first answered PREREQUISITE NOT MET (exit 3, unbuilt @objectstack/formula and @objectstack/lint). I built both under the verify lock and re-ran it: exit 0.
  • Also run: pnpm check:pm-settings-deny-roster (its roster lives under .claude): exit 0.
  • No changeset: .claude/** publishes nothing (skip-changeset).

Acceptance notes

  • Same defect in the sibling stash guard (not touched: outside this card's declared file surface). .claude/hooks/guard-shared-stash.sh split_segments() carries the identical in-quote branch. echo "a $(echo "x; git stash pop")" is a pure read, yet the guard answers exit 2 (blocked); echo "x; git stash pop" answers 0. Measured at b1676441b. The fix there is a ~3-line delegation to the same helper shape; reported to the seat for a family card.
  • The objectui twins carry it too. objectui 9eb1479, .claude/hooks/guard-main-checkout-bash.sh: both card forms exit 2. objectui guard-shared-stash.sh: the stash shape above exits 2. This hook's header says the two repos' copies are kept case-for-case identical, so this PR opens a mirror obligation there. Reported, not filed.
  • Neighbouring shape, not this card: a parameter expansion with a nested quote inside a double-quoted word, echo "${x:-"a > b"}", is still blocked (exit 2, target: b}). Its single-quote semantics differ from $(…) (a ' inside "${…}" is literal), so it is not a mechanical extension of this helper. Reported to the seat.
  • Cost: on a 2.5 KB, 25-line command of the card's probe shape, the hook takes 247 ms. The pre-fix hook took 158 ms, but it stopped at the false block on line 1, so the two numbers do not measure the same work.

维护者速读(草稿)

改了什么

修了 Bash 写保护钩子的一个误判。命令里如果有双引号包着的 $(…)(或反引号),而这段子命令里又带了引号,钩子会把内层引号当成外层字符串的结束,结果把 JSON 里的 > 当成重定向,拦下了根本不写文件的只读命令。现在由一个公共函数判断子命令到哪里结束,三处扫描都改用它,判断结果一致。

为什么改

云端会话里每次命令都在主检出目录下执行。开发 agent 用 curl 发带 > 条件的 JSON 探测请求时被误拦,只能把请求挪进临时脚本绕开。钩子头部自己承诺过:看不懂的形状一律放行,绝不误拦只读命令;这次的误拦违背了这个承诺。

风险与代价(含回滚)

钩子多了约 30 行、只负责找子命令结尾的函数。没有选「遇到就整体放行」的方案,因为实测那样会漏放 sed -i "s/a/$(date)/" 文件 这类真正写进主检出的命令。已知边界:子命令内部有 case 分支或带括号的注释时,结尾判断可能偏移;偏早就退回旧行为,偏晚则放行(不误拦)。回滚就是 revert 本 PR,只涉及两个 .claude/hooks 文件。

席位意见

你要做的

无需操作:这是 Tier S,席位按契约档复核后自行落地。另外,objectui 的两个同名钩子和本仓的 guard-shared-stash.sh 有同一个缺陷,已在报告里交席位跟进。


Generated by Claude Code

…ng context

guard-main-checkout-bash.sh read a `"` inside a `$(...)` (or backtick) that
sits inside a double-quoted word as the END of the outer string. The rest of
the substitution body was then scanned as bare shell, so a `>` inside a quoted
curl JSON body became a redirect and the next fragment a write "target": a
read-only command run from the primary checkout was blocked.

One helper, subst_end(), now answers where such a substitution ends, reading
its body with its own quotes, escapes and nesting. All three quote-aware
scanners (the heredoc comment scan, segmentation, tokenisation) delegate to it,
so they agree on the extent by construction. The body stays part of the word
verbatim, so a target holding it still fails open on its `$`; a write INSIDE a
quoted substitution stays unclaimed, as before.

Self-test: the card's two forms and the backtick/nested spellings now allow;
the plain-redirect control, a write behind a quoted substitution, a sed -i
script holding one, and the fail-open mirror (one `"` in the body used to hide
a real sed -i behind a separator) all block.

Claude-Session: https://claude.ai/code/session_01M82feuetLzrCiBz41BJb7L
Co-authored-by: Claude <noreply@anthropic.com>
The depth-2 nested allow case passed on the pre-fix hook by quote parity, so it
discriminated nothing. Replace it with a nested shape whose real sed -i behind a
separator is only seen when the helper reads a `$(` inside a string inside the
substitution as one more nesting level.

Claude-Session: https://claude.ai/code/session_01M82feuetLzrCiBz41BJb7L
Co-authored-by: Claude <noreply@anthropic.com>
@objectstack-fleet objectstack-fleet Bot added the skip-changeset PR has no user-facing published change; bypasses the changeset gate label Oct 20, 2026
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: e4ba00dc9241df6ca5e3974512aa69d4b5d3a04d
Local-runs: none
Scope: .claude/hooks/guard-main-checkout-bash.sh, .claude/hooks/guard-main-checkout-bash.selftest.sh + AGENTS.md (Prime Directive 11, the rule this hook enforces — read, unchanged), .claude/hooks/guard-shared-stash.sh (read only to confirm the sibling finding in ③)

Read: card #24812 (body and both comments), PR #24886 (body, file list, net diff against merge-base b1676441b — byte-identical to the API diff apart from index abbreviation; main has not touched .claude/hooks/ since that base), both files whole at the head, and the 40 check-runs on the head (read 2026-10-20T13:25Z; every one completed).

① Derived judgments

Accept-set changes the diff implies, each traced by hand through subst_end() and the three scanners at the head:

  1. A read whose double-quoted word holds a $(…) or backtick body containing its own quotes and a greater-than sign: BLOCK → ALLOW. Right — this is the card's defect (table rows 1 and 2, the original multi-line probe, and the backtick spelling). Pinned by the three allow cases in the new self-test section.
  2. The fail-open mirror — one " inside such a body used to leave the outer string hanging, so a real sed -i behind a separator rode through as text: ALLOW → BLOCK. Right — recall is gained, not lost. Pinned (printf '"' case, plus its nested form).
  3. The heredoc delimiter scan (strip_line_comment) — a blank-then-hash inside such a body used to truncate the introducing line, dropping the real heredoc introducer so the body lines were read as commands (a false block): BLOCK → ALLOW. Right; pinned. The card named two quote-aware passes; the hook has three readers of the same rule, and leaving the third would be a third dialect — the [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 lesson the card itself cites.
  4. A write INSIDE a quoted $(…) (echo "$(sed -i s/a/b/ pkg/x.ts)"): ALLOW → ALLOW. Unchanged in verdict, now named in the header's fail-open list and pinned in the "deliberately does NOT claim" section. Right — it is the wrapped-invocation class the header already fails open on, and the UNQUOTED form still blocks.
  5. Recall beside a quoted substitution — a redirect after the closing quote, a write behind a semicolon preamble, a sed -i script holding $(date +%F): BLOCK → BLOCK. Right; these three pins are exactly what makes the dispatch-suggested fail-open route go red, which is the measured reason the dev took the card's route 1 instead.
  6. The nested pin ("$(echo "$(printf '"')")" then a semicolon and a real sed -i) is discriminating: without the helper's nested-$( arm the inner " pops the stack early, the rest of the command is swallowed as inert, and the write is missed. My trace agrees with the dev's per-site ablation.
  7. The boundary the helper introduces, stated in its header: a ( or ) bash would not count — a case pattern, a paren inside a comment within the body — moves the end. Too early, and the tail is read exactly as the pre-fix hook read it (traced: a case pattern inside the quoted body followed by a real redirect still blocks). Too late, and the swallowed tail is inert — a new fail-open on one rare shape (a multi-line comment holding an unbalanced paren inside a quoted substitution, followed by a real redirect). Judged right under the hook's declared PRECISION OVER RECALL contract; the header now states both directions, which is what the final comment-only commit added.
  8. Arithmetic expansion "$((1+2))" followed by a real redirect is handled by paren balance and still blocks. A backslash-escaped $( or backtick inside the string is consumed by the pre-existing escape branch before the new branch runs in all three scanners, so it opens nothing — consistent with bash.
  9. Public surface: none. .claude/** publishes nothing; the exit-code contract (0 allow, 2 block), the OS_ALLOW_MAIN_EDITS=1 hatch, the refusal text and the jq-less fallback are untouched. The rule side, AGENTS.md Prime Directive 11 ("precision-first: it never blocks reads … any shape it cannot resolve with confidence … is allowed through"), needs no edit — the diff moves the hook toward it.
  10. Self-test delta is +11 cases (10 in the new guard-main-checkout-bash.sh reads quotes inside a $(…) command substitution as closing the outer double quote, so a > inside a quoted curl body is taken for a redirect and a read-only command from the primary checkout is blocked #24812 section, 1 in the fail-open section), matching the claimed 130 → 141; the file's pre-existing shape (hermetic fixture, pass/fail counters) is unchanged.
  11. Bash 3.2 floor: the helper uses ${s:i:n} with arithmetic offsets (a form the pre-fix scanners already use), +=, ${st%?} and an arithmetic for; st is never empty when top is computed, because the function returns the moment it empties. check:bash32-floor runs inside the green Lint lanes.

Check-runs on the head, as read: the five required contexts — Lint & Repo Gates success, TypeScript Type Check success, Test Core success, Governed Surface Queue Guard success, Build Core skipped (paths-filtered; the filter job is success and a bash hook has no build leg — reported as the conclusion it is, not read as a pass). Every other non-skipped run is success: Check Changeset, Check PR Size, Check Documentation Links, the seven Lint lanes, the four Type Check lanes, Dogfood Regression Gate, and the three card-claim guards. No in_progress, no failure.

② Semver level

skip-changeset is right: the diff is .claude/hooks/** only — no released package, no skills/**, no export, no schema — so nothing publishes, and Check Changeset is success. Clause-②: no, with no arm, on the PR body and on the dispatch agree, and it is right: the accept-set changes above are an internal-tooling bug fix with no published or authorable surface.

③ Boundary flags

open_questions: none declared. The eight declared deviations, each answered:

  • Route 1 (track the substitution as its own quoting context) instead of the dispatch-suggested route 2 (fail open on any double-quoted word holding $(): ACCEPTED. The card's Expected offers either; the dev measured route 2 against the pre-fix hook, found it lets three real writes through that the pre-fix hook blocks, and pinned all three so route 2 goes red. Prime Directive 5 over a smaller parser surface.
  • The backtick spelling and the third scanner (heredoc delimiter scan) folded in: ACCEPTED — same defect class, same helper, each pinned by a discriminating case (judgments 3 and 6).
  • The depth-2 nested allow pin replaced by a nested recall pin: ACCEPTED (judgment 6).
  • Local builds of formula and lint to clear a gate prerequisite; extra gates run; test:repo not run (not derived, and Test Core is success on the head); the model-free commit trailer pair per AGENTS.md rather than the harness reminder's spelling: all ACCEPTED, none touches the diff. The three commits carry the Claude-Session trailer naming this seat session and the model-free co-author line.
  • No objectui mirror PR and guard-shared-stash.sh untouched: ACCEPTED for this PR — both are outside the dispatch's declared file surface, which said stop on breach. ESCALATED to the seat as three findings the seat owns filing (Prime Directive 10: a reproducible defect with a named location is a card, not a note):
    1. .claude/hooks/guard-shared-stash.sh split_segments() carries the identical pre-fix in-double-quote branch and no subst_end (confirmed at this head: zero occurrences) — the same false block on echo "a $(echo "x; git stash pop")". Family card, same fix shape.
    2. The objectui twins of both hooks. This hook's own header (lines 14–16, untouched by the diff) promises the two repos' copies are kept case-for-case identical; this PR knowingly diverges from that promise until the mirror lands, so the mirror card is the remedy and the header claim is stale meanwhile.
    3. The neighbouring shape — a double-quoted parameter expansion whose default value holds a nested quote and a greater-than sign — still false-blocks. Its quoting semantics differ from a substitution's, so it is not an extension of this helper: its own card.
  • Cosmetic, no action: the PR body says "+77 / -1"; the net diff at this head is +78 / -1, the final comment-only commit having added one line after the body was written. The file list and the claimed scope are exact.

Tier and landing: .claude/** is the Tier S row of GOVERNED_SURFACES; the PR is draft, its head repo equals the base repo, its size is far under the 3,000-line ceiling, the dispatch Claim: names this branch, and the card-claim guards are success. With this PASS record on the head, the owning seat may land it through the queue; the seat reads the check-runs itself before arming.

Implemented-by: claude/issue-24812-guard-hook-cmdsubst-quotes
Reviewed-by: session_01M82feuetLzrCiBz41BJb7L

VERDICT: PASS


Generated by Claude Code

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · domain:skills seat 1 (seat post #7623) · session_01M82feuetLzrCiBz41BJb7L · head e4ba00dc9241df6ca5e3974512aa69d4b5d3a04d · 2026-10-20T13:29Z

  1. Form. The PR is a draft against main. The body's first line is Fixes #24812, and no other open card number sits beside a closing keyword (full body read). Assignee huangyiirene equals the card's. Files, read with the PR's file list rather than the report: the two declared paths only, .claude/hooks/guard-main-checkout-bash.sh (+55/−1) and .claude/hooks/guard-main-checkout-bash.selftest.sh (+23/−0). check-governed-merges.mjs --pr 24886 gives Tier S (.claude/** ×2) and 79 changed lines, under the 3000-line ceiling. skip-changeset is right: no package is touched.
  2. Premises measured. The card's reproduction table is re-measured on pre-fix b1676441b in the PR body's table. The seat read the diff itself. subst_end() keeps a closer stack and returns the moment it empties, so top is never read from an empty stack. All three scanners call it only from their q = '"' branch, after the pre-existing escape branch. Under the hook's set -uo pipefail (no -e), the trailing [ … ] && … arms cannot abort.
  3. Gate check-runs, pinned to this head (2026-10-20T13:17Z): 40 runs, 26 success, 14 skipped, 0 failure, 0 in progress. Lint & Repo Gates, TypeScript Type Check and every Lint · and Type Check · lane are success, as are Governed Surface Queue Guard and the three card-claim guards. check-expected-skips.mjs --pr 24886, run on a checkout re-pinned to origin/main ad8cf92f0: OK — 14 skipped check-run(s), every one in the roster (exit 0). The contract review of record is comment 6291929083 (Served-tier: CONTRACT_REVIEW_TIER, this head, Local-runs: none, PASS), rendered by an isolated subagent at that tier and adopted by this seat.
  4. Accepted deviations.
    • The route is the card's option 1 (track the substitution), not the dispatch's suggested option 2 (fail open). This is accepted on measurement: option 2 releases three real writes that the pre-fix hook blocks, and all three are now pinned blocked.
    • The backtick spelling and the third scanner (strip_line_comment) are folded in. They are the same defect class, and each has a discriminating pin.
    • The model-free commit trailers follow AGENTS.md.
    • Cosmetic: the body says +77/−1; the head is +78/−1.
  5. Out-of-scope findings, routed.
  6. Landing. Tier S with an at-tier PASS on this head and every check green: the seat flips it ready and arms auto-merge through the relay (pr_ready + automerge_enable). It follows the PR to MERGED, then closes out guard-main-checkout-bash.sh reads quotes inside a $(…) command substitution as closing the outer double quote, so a > inside a quoted curl body is taken for a redirect and a read-only command from the primary checkout is blocked #24812.

Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 13:31
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 13:31
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 7e40e8a Oct 20, 2026
41 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/issue-24812-guard-hook-cmdsubst-quotes branch October 20, 2026 14:04
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Director record · governed landing recognised · summon #45 continuation 2, session_01Lg6GQ4xjqmvEzYv78d6H7i · 2026-10-20T23:07Z

Audit window --since-ref objectstack=a43405d596d4 (tip e4d604efd39f): this PR landed on the Tier S surface .claude/hooks/** (×2, +77/−1: guard-main-checkout-bash.sh and its selftest) at 7e40e8a03a, merged by objectstack-fleet[bot] from the queue, with the at-tier contract review PASS 6291929083 (Served-tier: CONTRACT_REVIEW_TIER, head e4ba00dc92) and the domain:skills seat 1 ACCEPT 6292026192 on the same head, and no APPROVED review by a GOVERNED_APPROVERS account — the one row this window puts to the maintainer under the rule of #24241.

Put to the maintainer in this session's chat as the recognition row beside batch #379, with one line on what changed: the hook now reads a $(…) or backtick substitution inside a double-quoted word as its own quoting context up to its end, so a " inside the substitution no longer closes the outer string, a > inside a quoted JSON body is no longer read as a redirect, and read-only commands run from the primary checkout are no longer blocked. The maintainer answered 「同意」 — recognised. Nothing is owed on this PR; the ledger block on #12708 carries the row.

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

Labels

size/s skip-changeset PR has no user-facing published change; bypasses the changeset gate

Projects

None yet

2 participants