Skip to content

Commit 5fdd463

Browse files
os-zhuangclaude
andauthored
fix(hooks): guard-main-checkout-bash reads shell comments as text, not commands (#11129)
Both quote-aware passes (`split_segments()` and `tokenize()`) now apply the shell's comment rule: outside quotes and heredoc bodies, an unquoted `#` that starts a WORD begins a comment running to the next newline. The word-start condition is the whole rule — `foo#bar`, `${x#y}`, `curl 'url/#frag'`, `sed 's/#//'` and `grep '#'` are not comments and keep every verdict they had. In `split_segments()` that means `{`/`}` do not reset word state (they are reserved words, not metacharacters), so the `#` of `${#arr[@]}` cannot swallow a real redirect behind it. Nothing changes about what the hook blocks once it has a real command; this is only about what counts as a command. Self-test extended in both directions: 82 -> 100 cases. Fixes #10570 Co-authored-by: Claude <noreply@anthropic.com>
1 parent afe1c4e commit 5fdd463

2 files changed

Lines changed: 74 additions & 5 deletions

File tree

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

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,35 @@ expect allow 'grep -rn "he said \"sed -i\" once" .claude/'
170170
expect block 'node -e "console.log(\"hi\")" > pkg/out.json'
171171
expect block 'sed -i "s/\"a\"/\"b\"/" pkg/x.ts'
172172

173+
echo "== a shell COMMENT is text, not a command (#10570) =="
174+
CWD="$MAIN"
175+
# The measured false blocks: prose in a comment put a real `>` in operator position and the
176+
# next word was named as a write target. Nothing in either command writes anything.
177+
expect allow "$(printf '# rename foo -> bar\necho hello\n')"
178+
expect allow "$(printf '# sitting 1 landed; card stays open for sitting 2 -> pm:dispatched goes, pm:queue returns\ncurl -s https://example.com/a\ncurl -s https://example.com/b\n')"
179+
expect allow 'echo hi # then; tee pkg/x.ts would write it down'
180+
expect allow "$(printf '# step one; then a -> b\n# 2> is not a redirect here either\ngit status\n')"
181+
# Recall is untouched: a real redirect on a LATER line still blocks, and so does one on the
182+
# SAME line ahead of an inline comment.
183+
expect block "$(printf '# rename foo -> bar\necho x > pkg/x.ts\n')"
184+
expect block 'echo x > pkg/x.ts # write it down'
185+
expect block "$(printf '# a comment; with a separator\nsed -i s/a/b/ pkg/x.ts\n')"
186+
# Word start is the whole rule: these `#`s are not comments and must not change a verdict.
187+
expect block 'touch foo#bar'
188+
expect block 'rm -rf pkg/x.ts#old'
189+
expect allow 'curl -s "https://example.com/docs#frag"'
190+
expect allow "curl -s https://example.com/docs#a-real-redirect-would-be > /dev/null"
191+
expect allow "grep -n '#' README.md"
192+
expect allow "sed 's/#//' README.md"
193+
expect allow 'echo "# not a comment > pkg/x.ts"'
194+
# `${x#y}` / `${#arr[@]}` — a parameter expansion, not a comment. Swallowing the line here
195+
# would drop the redirect behind it, so the negative twin is the load-bearing case.
196+
expect allow 'echo ${x#pkg/} '
197+
expect block 'echo ${#TOK[@]} > pkg/x.ts'
198+
expect block 'echo ${x#a} > pkg/x.ts'
199+
# an escaped `\#` outside quotes is a literal, not a comment opener
200+
expect allow 'echo \# not a comment'
201+
173202
echo "== shapes this guard deliberately does NOT claim (documented fail-open) =="
174203
CWD="$MAIN"
175204
expect allow "bash -c \"sed -i s/a/b/ $MAIN/pkg/x.ts\""

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

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,15 @@
6161
# in that tail put a REAL ASCII `>` in operator position, so the guard named the
6262
# following JS fragment as a write "target" and blocked a pure-read command (#10247).
6363
# Single quotes take no escapes: inside '…' a backslash is literal, as in a real shell.
64+
# 4. shell COMMENTS are text, not commands — both quote-aware passes stop at an unquoted
65+
# `#` that starts a WORD and resume at the next newline (#10570). A comment cannot
66+
# write anything, so reading one as a command is a false BLOCK: prose arrows (`->`,
67+
# `=>`) put a real `>` in operator position and the word after it was named as a write
68+
# target, while a `;` in the same comment first split it into its own "segment".
69+
# The word-start condition is the whole rule — `foo#bar`, `${x#y}`, `curl 'url/#frag'`,
70+
# `sed 's/#//'` and `grep '#'` are NOT comments and are left exactly as they were. A
71+
# comment never begins inside '…' or "…" (layer 1 owns those) nor inside a heredoc body
72+
# (layer 2 has already dropped those lines before this pass runs).
6473
#
6574
# Redirection is recognised on ASCII operators ONLY — `>` `>>` `<` `<<` and their fd-prefixed
6675
# forms. No non-ASCII codepoint is ever an operator or an operator boundary, and this is
@@ -138,10 +147,19 @@ strip_heredocs() {
138147

139148
# --- split the command into shell segments, honouring quotes ---------------------------
140149
# A separator inside '…' or "…" does NOT split, so writing *about* the ban is never caught
141-
# by the ban.
150+
# by the ban. The same goes for a separator inside a COMMENT: `word` tracks whether the
151+
# next character would open a new WORD, which is the only position where an unquoted `#`
152+
# begins a comment — the comment then runs to the next newline and never splits, tokenises
153+
# or contributes a target (#10570).
154+
#
155+
# What resets `word` is exactly what delimits a word in the shell: start of input, blank,
156+
# and the metacharacters `; | & ( ) newline > <`. `{` and `}` do NOT — they are reserved
157+
# words rather than metacharacters, so `#` right after one continues the word. That is not
158+
# cosmetic: this pass splits on `{`/`}` anyway, and treating the `#` of `${#arr[@]}` as a
159+
# comment would swallow the rest of the line — including a real redirect behind it.
142160
segments=()
143161
split_segments() {
144-
local s="$1" seg="" q="" ch i n=${#1}
162+
local s="$1" seg="" q="" ch i n=${#1} word=0
145163
for ((i = 0; i < n; i++)); do
146164
ch="${s:i:1}"
147165
if [ -n "$q" ]; then
@@ -156,9 +174,20 @@ split_segments() {
156174
continue
157175
fi
158176
case "$ch" in
159-
"'" | '"') q="$ch" ; seg+="$ch" ;;
160-
';' | '|' | '&' | '(' | ')' | '{' | '}' | $'\n') segments+=("$seg") ; seg="" ;;
161-
*) seg+="$ch" ;;
177+
'#')
178+
if [ "$word" = 0 ]; then
179+
# comment: skip to (not past) the newline, which still separates as usual
180+
while [ $((i + 1)) -lt "$n" ] && [ "${s:i+1:1}" != $'\n' ]; do i=$((i + 1)); done
181+
continue
182+
fi
183+
seg+="$ch" # foo#bar, ${x#y}, url/#frag
184+
;;
185+
"'" | '"') q="$ch" ; seg+="$ch" ; word=1 ;;
186+
';' | '|' | '&' | '(' | ')' | $'\n') segments+=("$seg") ; seg="" ; word=0 ;;
187+
'{' | '}') segments+=("$seg") ; seg="" ; word=1 ;;
188+
' ' | $'\t') seg+="$ch" ; word=0 ;;
189+
'>' | '<') seg+="$ch" ; word=0 ;;
190+
*) seg+="$ch" ; word=1 ;;
162191
esac
163192
done
164193
segments+=("$seg")
@@ -168,6 +197,9 @@ split_segments() {
168197
# The whole argument list matters here, and `read -r -a` would promote a QUOTED ">" or
169198
# "sed -i" to a real operator/command — exactly the false positive that makes a guard get
170199
# disabled. So: quotes are stripped and their contents are inert.
200+
# A comment is inert here too: `have` is already the "a word is open" flag, so an unquoted
201+
# `#` with have=0 is at word start and begins a comment (#10570). `foo#bar` and `${x#y}`
202+
# reach this point with have=1 and stay part of the token.
171203
# TOK[] = token values (unquoted); TOP[] = "op" for an unquoted redirection operator.
172204
TOK=(); TOP=()
173205
tokenize() {
@@ -187,6 +219,14 @@ tokenize() {
187219
continue
188220
fi
189221
case "$ch" in
222+
'#')
223+
if [ "$have" = 0 ]; then
224+
# word-start `#`: comment, inert to the next newline
225+
while [ $((i + 1)) -lt "$n" ] && [ "${s:i+1:1}" != $'\n' ]; do i=$((i + 1)); done
226+
continue
227+
fi
228+
tok+="$ch"
229+
;;
190230
"'" | '"') q="$ch" ; have=1 ;;
191231
'\')
192232
i=$((i + 1))

0 commit comments

Comments
 (0)