Skip to content

Commit 5295de1

Browse files
committed
fix(hooks): read a command substitution inside "..." as its own quoting 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>
1 parent b167644 commit 5295de1

2 files changed

Lines changed: 77 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+
expect allow 'echo "$(echo "$(echo "x > 1")")"' # nested
316+
# Recall is untouched: a real write beside a quoted substitution still blocks — in the same
317+
# command, behind a separator, and inside a sed -i script — and so does the card's control.
318+
expect block 'echo hi > out.txt'
319+
expect block 'echo "a $(curl -d "{\"c\":\"x > 1\"}" http://h)" > pkg/x.ts'
320+
expect block 'X="$(git rev-parse HEAD)" ; sed -i s/a/b/ pkg/x.ts'
321+
expect block 'sed -i "s/a/$(date +%F)/" pkg/x.ts'
322+
# The fail-OPEN mirror: one `"` in the body used to leave the outer string hanging, so the
323+
# separator behind it went inert and the real write rode through as text.
324+
expect block "echo \"\$(printf '\"')\" ; sed -i s/a/b/ pkg/x.ts"
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: 54 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,47 @@ 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, and
137+
# the text after it is then read exactly as before this function existed.
138+
SUBST_END=0
139+
subst_end() {
140+
local s="$1" i="$2" n=${#1} st ch top
141+
if [ "${s:i:1}" = '`' ]; then st='`'; else st=')' ; i=$((i + 1)); fi
142+
for ((i = i + 1; i < n; i++)); do
143+
ch="${s:i:1}" ; top="${st:${#st}-1:1}"
144+
if [ "$top" = "'" ]; then [ "$ch" = "'" ] && st="${st%?}"; continue; fi
145+
if [ "$ch" = '\' ]; then i=$((i + 1)); continue; fi
146+
if [ "$top" = '"' ]; then
147+
case "$ch" in
148+
'"') st="${st%?}" ;;
149+
'`') st+='`' ;;
150+
'$') if [ "${s:i+1:1}" = '(' ]; then st+=')' ; i=$((i + 1)); fi ;;
151+
esac
152+
continue
153+
fi
154+
case "$ch" in
155+
"'" | '"') st+="$ch" ;;
156+
'(') st+=')' ;;
157+
')') [ "$top" = ')' ] && st="${st%?}" ;;
158+
'`') if [ "$top" = '`' ]; then st="${st%?}"; else st+='`'; fi ;;
159+
esac
160+
if [ -z "$st" ]; then SUBST_END=$i; return; fi
161+
done
162+
SUBST_END=$((n - 1))
163+
}
164+
121165
# --- heredoc bodies are text, not commands ---------------------------------------------
122166
# `cat > /tmp/notes.md <<EOF` … `EOF` carries prose over several lines. Segmentation splits
123167
# on newlines, so without this pass a documented example inside the body ("sed -i … main
@@ -148,6 +192,9 @@ strip_line_comment() {
148192
ch="${s:i:1}"
149193
if [ -n "$q" ]; then
150194
if [ "$q" = '"' ] && [ "$ch" = '\' ] && [ $((i + 1)) -lt "$n" ]; then i=$((i + 1)); continue; fi
195+
if [ "$q" = '"' ] && { [ "$ch" = '`' ] || [ "${s:i:2}" = '$(' ]; }; then
196+
subst_end "$s" "$i" ; i=$SUBST_END ; continue
197+
fi
151198
[ "$ch" = "$q" ] && q=""
152199
continue
153200
fi
@@ -217,6 +264,9 @@ split_segments() {
217264
seg+="$ch" ; i=$((i + 1)) ; seg+="${s:i:1}" ; continue ;;
218265
esac
219266
fi
267+
if [ "$q" = '"' ] && { [ "$ch" = '`' ] || [ "${s:i:2}" = '$(' ]; }; then
268+
subst_end "$s" "$i" ; seg+="${s:i:SUBST_END-i+1}" ; i=$SUBST_END ; continue
269+
fi
220270
seg+="$ch"
221271
[ "$ch" = "$q" ] && q=""
222272
continue
@@ -274,6 +324,9 @@ tokenize() {
274324
i=$((i + 1)) ; tok+="${s:i:1}" ; have=1 ; continue ;;
275325
esac
276326
fi
327+
if [ "$q" = '"' ] && { [ "$ch" = '`' ] || [ "${s:i:2}" = '$(' ]; }; then
328+
subst_end "$s" "$i" ; tok+="${s:i:SUBST_END-i+1}" ; i=$SUBST_END ; have=1 ; continue
329+
fi
277330
if [ "$ch" = "$q" ]; then q=""; else tok+="$ch"; fi
278331
have=1
279332
continue

0 commit comments

Comments
 (0)