Repository navigation
fix(hooks): read a command substitution inside a double-quoted word as its own quoting context - #24886
Conversation
…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>
Claude-Session: https://claude.ai/code/session_01M82feuetLzrCiBz41BJb7L Co-authored-by: Claude <noreply@anthropic.com>
Contract reviewServed-tier: Read: card #24812 (body and both comments), PR #24886 (body, file list, net diff against merge-base ① Derived judgmentsAccept-set changes the diff implies, each traced by hand through
Check-runs on the head, as read: the five required contexts — ② Semver level
③ Boundary flags
Tier and landing: Implemented-by: VERDICT: PASS Generated by Claude Code |
席位核对ACCEPT ·
Generated by Claude Code |
|
Director record · governed landing recognised · summon #45 continuation 2, Audit window 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 |
Fixes #24812
Clause-②: no
What
.claude/hooks/guard-main-checkout-bash.shread 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 scanstrip_line_comment(), segmentationsplit_segments(), tokenisationtokenize()) 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), andecho "$(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 (acasepattern, 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"})b1676441be4ba00dc9echo "a $(curl -d "{\"c\":\"x > 1\"}" http://h)"target: 1"echo "a $(curl -d '{"c":"x > 1"}' http://h)"target: 1}' http://h)curl -s -d "{\"c\":\"x > 1\"}" http://hecho hi > out.txttarget: out.txttarget: out.txtecho "PUT $1 -> $(curl … -d "{…\"condition\":\"int(record.title) > 1\"…}" …)"target: 1"target: 1"echo "$(date)" > /home/user/objectstack/fX="$(git rev-parse HEAD)" && echo "$X" > /home/user/objectstack/fH2 (both passes share the defect): confirmed by per-site ablation. Each mutation went through
scripts/ablation-replace.mjsin WRAP mode at0963684e5: 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 emptygit diff HEAD. Results:$(arm removed: 1 red (the nested pin).So no single pass fixes it alone. The whole pre-fix hook (
b1676441b, blob1fd91d0a8) 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.e4ba00dc9after the ablations is comment-only.H3 (a real redirect inside a quoted
$(…)). Pre-fix:echo "$(cat x > /home/user/objectstack/f)"exit 0, andx="$(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 UNQUOTEDecho $(cat x > /home/user/objectstack/f)is still blocked (exit 2), unchanged.Tests and gates, all at HEAD
e4ba00dc9bash .claude/hooks/guard-main-checkout-bash.selftest.sh:141 passed, 0 failed(130 onmain; +11 cases)..claude/hooks/:guard-main-checkout120/0,guard-process-kill69/0,guard-shared-stash53/0 andguard-tree-enum38/0.node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstackderived 16 families. All 16 ran with exit 0, captured before any pipe.--ranreconciliation:16 derived, 16 run, 0 NOT-MEASURED, 0 UNRUN.pnpm check:bash32-floorreports19 constructs checked, floor bash 3.2. The helper uses only${s:i:n},+=,${st%?}and arithmeticfor. There is no recursion, nomapfileand no negative substring offset.check:doc-formula-expressionsfirst answeredPREREQUISITE NOT MET(exit 3, unbuilt@objectstack/formulaand@objectstack/lint). I built both under the verify lock and re-ran it: exit 0.pnpm check:pm-settings-deny-roster(its roster lives under.claude): exit 0..claude/**publishes nothing (skip-changeset).Acceptance notes
.claude/hooks/guard-shared-stash.shsplit_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 atb1676441b. The fix there is a ~3-line delegation to the same helper shape; reported to the seat for a family card.9eb1479,.claude/hooks/guard-main-checkout-bash.sh: both card forms exit 2. objectuiguard-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.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.维护者速读(草稿)
改了什么
修了 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