Repository navigation
Conversation
01bbcad to
89e7e17
Compare
|
Heads up — I'm auto-updating this PR by merging the latest |
|
Heads up — I'm auto-updating this PR by merging the latest |
|
I tried to auto-update this PR with |
The matcher split on shell operators with no notion of quoting or command position, so it denied read-only commands whose quoted arguments contained the governed strings, and missed real invocations behind any prefix. Two awk passes replace it. Pass 1 makes quoting inert, treating $( ) and backticks as command positions even inside double quotes and ending a line at an unquoted #. Pass 2 tests every adjacent token pair against the basename, which drops the wrapper list and covers melos, timeout, sudo, xargs, nice, shell keywords, brace groups and path-qualified binaries in one rule. Closes #147 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Read the whole command as one awk record, which removes the accumulator, the END flush and all cross-record quote-state carrying, and makes backslash line continuation fall out of the escape rule instead of needing its own branch. A kind[] lookup replaces the five-branch conditional, and the two awk programs are named so the pipeline reads in one line. Behavior is unchanged: the existing 131 cases pass untouched. Two cases cover a comment followed by a blocked command on a later line, which one record made load-bearing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The matcher emitted one of five tokens that the caller translated straight back into a message, so the tokens existed only to be undone. The lookup table now holds the redirect itself and the five-branch case statement is gone. Reading each awk program from a quoted heredoc rather than a single-quoted string also retires the -v SQ/-v DQ workaround, since the program can now contain quote characters directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The character-level lexer existed to handle forms the agent never writes: quoting the command name, a backslash-continued line, a # comment inside a tool call. Three regex substitutions collapse quoted spans instead, and a second scan with the quotes deleted runs only when something executes the string -- eval, sh -c, or $( ) and backticks inside double quotes. That catches eval "flutter test" and bash -c '...', which the lexer did not, and drops "flutter" test and line continuation, which nobody types. The non-goal tests flip accordingly; everything else passes untouched. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
scan() turns operators into a ";" word and walks one flat word list instead of a nested fragment loop. The main block names its three views of the command up front, so the decision reads as three gated scans. Comments say what, not why. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The README row and CLAUDE.md bullet each said the same thing three ways. The file header now points at the hint table instead of duplicating its contents, and notes why the program is read with read rather than a command substitution. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The trigger words were assembled from fragments so the pair never appeared in this file, protecting shell edits of it from the hook under test. Single words are harmless and only a heredoc write was ever at risk, so the file now reads plainly and a header note covers that one case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
400db6f to
a078868
Compare
| } | ||
| { | ||
| gsub(/\\["'$`]/, "_") # escaped: literal | ||
| text = $0; gsub(/'[^']*'/, "_", text); gsub(/"[^"]*"/, "_", text) # quotes are data |
There was a problem hiding this comment.
The single-quote and double-quote passes run one after the other, not left to right, so an apostrophe inside a double-quoted string opens a fake '…' span. By hand, echo "don't" && flutter test && echo 'x' comes out of this line as echo "don_x', and the hook allows it. One alternation regex consumed left to right would fix this and stays lexer-free, e.g. a loop with match($0, /'[^']*'|"[^"]*"/) that replaces each match in order.
Could you add this case (and git commit -m "it's" && flutter test && git commit -m "don't") to the tests?
There was a problem hiding this comment.
Fixed in 3cf6c24. The two gsub passes are now a single left-to-right pass: dequote() walks the string with match(s, /'[^']*'|"[^"]*"/) and takes the leftmost span each time, so a quote inside another quote can never open one of its own.
Both of your cases are pinned, plus a control that must stay allowed:
echo "don't" && flutter test && echo 'x' -> denied
git commit -m "it's" && flutter test && git commit -m "..." -> denied
echo "don't" && ls && echo 'x' -> allowed
I checked they fail against the pre-fix script, so they are real regression tests rather than cases that always passed.
| bare = $0; gsub(/["']/, "", bare) # and eval "" runs | ||
|
|
||
| pair = scan(text) | ||
| if (pair == "" && $0 ~ /\$\(|`/) pair = scan(subst) |
There was a problem hiding this comment.
Once $( appears anywhere, this pass makes all double-quoted text live, not just the text inside the substitution. That denies the standard git commit -m "$(cat <<'EOF' … flutter test … EOF)" and gh pr create --body "$(cat <<'EOF' …)" forms whenever the message mentions flutter test, which is common in a Flutter repo.
Could you either (a) drop quoted-delimiter heredoc bodies (<<'X' / <<"X" … X) before scanning, or (b) scan only the text between $( and its matching )? At minimum, please add this case to the pinned non-goals and to the CLAUDE.md/README gap list so it's a known trade-off rather than a surprise.
There was a problem hiding this comment.
Went with (a). strip_heredocs() drops heredoc bodies whose delimiter is quoted (<<'X', <<-"X") before anything is scanned -- the shell expands nothing inside them, so they are data. An unquoted <<EOF does expand $( ), so its body stays in place and is still scanned; that case remains denied and stays pinned.
(b) on its own would not have fixed this: in git commit -m "$(cat <<'EOF' ... )" the heredoc body sits inside the $( ) region, so scanning only that region still reaches it.
git commit -m "$(cat <<'EOF' ... flutter test ... EOF)" -> allowed
gh pr create --body "$(cat <<'EOF' ... )" -> allowed
cat <<EOF / flutter test / EOF -> denied, unchanged
CLAUDE.md and the README now spell out the quoted/unquoted delimiter split. Tests 134 -> 140.
Two bugs found in review. An apostrophe inside "..." opened a single-quote span that swallowed the rest of the line, because the two quote passes ran one after the other rather than left to right. `echo "don't" && flutter test` was allowed. One pass now consumes each span in order, so a quote inside another quote cannot open its own. The $( ) pass stripped every double quote, which made all double-quoted text live as soon as `$(` appeared anywhere. That denied the ordinary `git commit -m "$(cat <<'EOF' ... EOF)"` whenever the message mentioned a governed command. A heredoc body with a quoted delimiter expands nothing, so it is dropped before scanning; an unquoted <<EOF still expands and is still scanned. Both forms are pinned by tests.
2 participants
Closes #147.
The hook split on
[;&|]+with no quote awareness, sogrep "a|flutter test|b"was denied andfvm flutter testwas not.What it does now
eval,sh -cor$( )runs it.73 lines of code, no lexer.
Before / after
grep -nE "very_good|flutter test|foo" CLAUDE.mdfvm flutter test,melos exec -- flutter test,sudo -u ci flutter testif true; then flutter test; fi,/usr/local/bin/flutter test,OUT="$(flutter test)"eval "flutter test",bash -c 'flutter test'Pinned gaps
echo flutter testis denied. Quote it."flutter" testandF=flutter; $F testpass.Also
Matched: <cmd>.🤖 Generated with Claude Code