Skip to content

Commit 7e40e8a

Browse files
fix(hooks): read a command substitution inside a double-quoted word as its own quoting context (#24886)
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](https://claude.ai/code/session_01M82feuetLzrCiBz41BJb7L)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0091d84 commit 7e40e8a

2 files changed

Lines changed: 78 additions & 1 deletion

File tree

‎.claude/hooks/guard-main-checkout-bash.selftest.sh‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,12 +304,35 @@ expect allow "$(printf "grep '#' README.md <<EOF\nsed -i 's/a/b/' pkg/x.ts\nEOF\
304304
expect block "$(printf "cat > /tmp/n.md <<'EOF'\n# it's a note, don't strip me\nEOF\nsed -i s/a/b/ pkg/x.ts\n")"
305305
expect allow "$(printf "cat > /tmp/n.md <<'EOF'\n# it's a note, don't strip me\nEOF\ngit status\n")"
306306

307+
echo "== a \$(…) inside \"…\" is its own quoting context (#24812) =="
308+
CWD="$MAIN"
309+
# The measured false blocks: a `"` inside the substitution was read as CLOSING the outer string,
310+
# so the `>` in a quoted JSON body became a redirect and the fragment after it a write target.
311+
# Pure reads — nothing is written.
312+
expect allow 'echo "a $(curl -d "{\"c\":\"x > 1\"}" http://h)"'
313+
expect allow "echo \"a \$(curl -d '{\"c\":\"x > 1\"}' http://h)\""
314+
expect allow 'echo "a `curl -d "{\"c\":\"x > 1\"}" http://h`"' # the backtick spelling
315+
# Recall is untouched: a real write beside a quoted substitution still blocks — in the same
316+
# command, behind a separator, and inside a sed -i script — and so does the card's control.
317+
expect block 'echo hi > out.txt'
318+
expect block 'echo "a $(curl -d "{\"c\":\"x > 1\"}" http://h)" > pkg/x.ts'
319+
expect block 'X="$(git rev-parse HEAD)" ; sed -i s/a/b/ pkg/x.ts'
320+
expect block 'sed -i "s/a/$(date +%F)/" pkg/x.ts'
321+
# The fail-OPEN mirror: one `"` in the body used to leave the outer string hanging, so the
322+
# separator behind it went inert and the real write rode through as text.
323+
expect block "echo \"\$(printf '\"')\" ; sed -i s/a/b/ pkg/x.ts"
324+
expect block "echo \"\$(echo \"\$(printf '\"')\")\" ; sed -i s/a/b/ pkg/x.ts" # nested
325+
# The heredoc scan reads the body the same way: the ` #` inside it is no comment, so the
326+
# introducer behind it is found and the body stays prose.
327+
expect allow "$(printf 'echo "$(echo " #")" <<EOF\nsed -i s/a/b/ pkg/x.ts\nEOF\n')"
328+
307329
echo "== shapes this guard deliberately does NOT claim (documented fail-open) =="
308330
CWD="$MAIN"
309331
expect allow "bash -c \"sed -i s/a/b/ $MAIN/pkg/x.ts\""
310332
expect allow 'echo pkg/x.ts | xargs sed -i s/a/b/'
311333
expect allow 'node -e "fs.writeFileSync(\"pkg/x.ts\", \"y\")"'
312334
expect allow 'python3 -c "open(\"pkg/x.ts\",\"w\").write(\"y\")"'
335+
expect allow 'echo "$(sed -i s/a/b/ pkg/x.ts)"' # a QUOTED $(…) body is one inert word
313336
expect allow 'sed -i s/a/b/ $SOMEDIR/x.ts'
314337
expect allow 'sed -i s/a/b/ pkg/*.ts'
315338

‎.claude/hooks/guard-main-checkout-bash.sh‎

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@
3838
#
3939
# Fails open, deliberately, on:
4040
# - wrapped invocations: bash -c '…', xargs, ssh host '…', make, a shell script that
41-
# writes (the target is one indirection away from any token this hook can see);
41+
# writes, a command inside a QUOTED "$(…)" (the target is one indirection away from any
42+
# token this hook can see);
4243
# - programs whose writes live inside their own source text: python -c "open(p,'w')",
4344
# node -e, awk > (the target is not a shell token, so there is nothing to read);
4445
# - any target containing an expansion or glob ($VAR, `…`, $(…), *, ?, ~) — unresolvable
@@ -51,6 +52,8 @@
5152
# Writing ABOUT the ban must never trip the ban (#4890's lesson). Four layers:
5253
# 1. quote-aware segmentation + tokenisation — a `>` or a `sed -i` inside '…' or "…" is
5354
# literal text, so `grep -n "sed -i" .claude/` and `echo "never sed -i in main"` pass;
55+
# a `$(…)` or backtick inside "…" is a quoting context of its own, so a `"` in its body
56+
# nests instead of closing the outer string (subst_end(), shared by every pass);
5457
# 2. heredoc bodies are stripped before analysis — the LINES of a `cat > /tmp/notes <<EOF`
5558
# body are documentation, not commands, and segmentation alone (which splits on
5659
# newlines) would happily read them as such;
@@ -118,6 +121,48 @@ fi
118121

119122
[ -n "$cmd" ] || exit 0
120123

124+
# --- a command substitution inside "…" is its own quoting context ---------------------
125+
# Inside "…", `$(` or a backtick opens a NEW quoting context: a `"` in the body starts a string
126+
# of its own and does not close the outer one. Each scanner below used to read that `"` as the
127+
# closer, so the rest of the body became bare shell, and a `>` in a quoted curl body was taken
128+
# for a redirect whose "target" was the next fragment — a false BLOCK (#24812). All three
129+
# quote-aware scanners hand such a body to this ONE function, so they agree on where it ends.
130+
#
131+
# subst_end S I: S[I] is the `$` of a `$(`, or a backtick, met inside "…". Sets SUBST_END to
132+
# the index of the character that closes the substitution, reading the body with its own
133+
# quotes, escapes and nested `(…)` / `$(…)` / backticks; when the input ends first, to the
134+
# last index — the rest is one inert run (bash refuses such a command anyway). The body is
135+
# kept verbatim as part of the word, so a target holding it still fails open on its `$`.
136+
# Boundary: a `(` or `)` bash would not count (a `case` pattern, a comment) moves the end —
137+
# too early, and the rest is read as it was before this function existed; too late, and the
138+
# text swallowed with the body is inert, i.e. fails open.
139+
SUBST_END=0
140+
subst_end() {
141+
local s="$1" i="$2" n=${#1} st ch top
142+
if [ "${s:i:1}" = '`' ]; then st='`'; else st=')' ; i=$((i + 1)); fi
143+
for ((i = i + 1; i < n; i++)); do
144+
ch="${s:i:1}" ; top="${st:${#st}-1:1}"
145+
if [ "$top" = "'" ]; then [ "$ch" = "'" ] && st="${st%?}"; continue; fi
146+
if [ "$ch" = '\' ]; then i=$((i + 1)); continue; fi
147+
if [ "$top" = '"' ]; then
148+
case "$ch" in
149+
'"') st="${st%?}" ;;
150+
'`') st+='`' ;;
151+
'$') if [ "${s:i+1:1}" = '(' ]; then st+=')' ; i=$((i + 1)); fi ;;
152+
esac
153+
continue
154+
fi
155+
case "$ch" in
156+
"'" | '"') st+="$ch" ;;
157+
'(') st+=')' ;;
158+
')') [ "$top" = ')' ] && st="${st%?}" ;;
159+
'`') if [ "$top" = '`' ]; then st="${st%?}"; else st+='`'; fi ;;
160+
esac
161+
if [ -z "$st" ]; then SUBST_END=$i; return; fi
162+
done
163+
SUBST_END=$((n - 1))
164+
}
165+
121166
# --- heredoc bodies are text, not commands ---------------------------------------------
122167
# `cat > /tmp/notes.md <<EOF` … `EOF` carries prose over several lines. Segmentation splits
123168
# on newlines, so without this pass a documented example inside the body ("sed -i … main
@@ -148,6 +193,9 @@ strip_line_comment() {
148193
ch="${s:i:1}"
149194
if [ -n "$q" ]; then
150195
if [ "$q" = '"' ] && [ "$ch" = '\' ] && [ $((i + 1)) -lt "$n" ]; then i=$((i + 1)); continue; fi
196+
if [ "$q" = '"' ] && { [ "$ch" = '`' ] || [ "${s:i:2}" = '$(' ]; }; then
197+
subst_end "$s" "$i" ; i=$SUBST_END ; continue
198+
fi
151199
[ "$ch" = "$q" ] && q=""
152200
continue
153201
fi
@@ -217,6 +265,9 @@ split_segments() {
217265
seg+="$ch" ; i=$((i + 1)) ; seg+="${s:i:1}" ; continue ;;
218266
esac
219267
fi
268+
if [ "$q" = '"' ] && { [ "$ch" = '`' ] || [ "${s:i:2}" = '$(' ]; }; then
269+
subst_end "$s" "$i" ; seg+="${s:i:SUBST_END-i+1}" ; i=$SUBST_END ; continue
270+
fi
220271
seg+="$ch"
221272
[ "$ch" = "$q" ] && q=""
222273
continue
@@ -274,6 +325,9 @@ tokenize() {
274325
i=$((i + 1)) ; tok+="${s:i:1}" ; have=1 ; continue ;;
275326
esac
276327
fi
328+
if [ "$q" = '"' ] && { [ "$ch" = '`' ] || [ "${s:i:2}" = '$(' ]; }; then
329+
subst_end "$s" "$i" ; tok+="${s:i:SUBST_END-i+1}" ; i=$SUBST_END ; have=1 ; continue
330+
fi
277331
if [ "$ch" = "$q" ]; then q=""; else tok+="$ch"; fi
278332
have=1
279333
continue

0 commit comments

Comments
 (0)