fix: match commands quote-aware in block-cli-workarounds

CLAUDE.md

@@ -17,7 +17,7 @@ These run **when a session begins**:
 These run **before** a tool call is executed:
 
 - `mcp__.*very-good-cli__.*` matcher → `check-vgv-cli.sh` — reads `tool_name` from the payload and exits 0 unless the caller is a Very Good CLI tool, so a host that does not apply the matcher cannot have the decision land on an unrelated tool; for its own tools it auto-approves the call by returning a PreToolUse `allow` decision, so it is always permitted regardless of run mode (interactive, headless, or `skipAutoPermissionPrompt`) and never dead-ends when the tool isn't on `permissions.allow`; denies with an install/upgrade message if the CLI is missing or < 1.3.0, and stands aside when the CLI is present but its version cannot be read (`dart` missing from `PATH`), leaving normal permission handling to apply. The `.*` in the matcher covers both the bare `mcp__very-good-cli__*` server (repo-root `.mcp.json`) and the plugin-namespaced `mcp__plugin_<plugin>_very-good-cli__*` form used when installed from a marketplace
-- `Bash` matcher → `block-cli-workarounds.sh` — prevents direct CLI bypass of VGV CLI commands through the host's shell tool (`Bash`, or `Shell` on other hosts); exits 0 when the payload names a non-shell tool, so an unrelated tool carrying a `command` argument is never inspected; when the CLI is present but cannot run because `dart` is missing from `PATH`, the denial says so rather than redirecting to an MCP server that cannot start; every denial also says the whole shell call was refused, so the agent re-runs any command chained with the blocked one instead of assuming it ran; exits 2 on failure (blocking)
+- `Bash` matcher → `block-cli-workarounds.sh` — denies `flutter`/`dart` `test`/`create` and `very_good test`/`create`/`packages` in the shell and points to the MCP tool. Quoted text is data unless `eval`, `sh -c` or `$( )` runs it. Every adjacent word pair is checked, so wrappers (`fvm`, `melos`, `sudo`, `timeout`) and `/path/to/flutter` need no list. The denial names the match and says the whole call was refused. Pinned gaps: `"flutter" test` and `$F test` pass; unquoted `echo flutter test` is denied
 
 The first two PreToolUse hooks are plugin-level (defined in `hooks.json`) and share common utilities
 from `vgv-cli-common.sh`. The following hook is **agent-scoped** — it is declared in the

README.md

@@ -71,7 +71,7 @@ This plugin includes SessionStart, PreToolUse, and PostToolUse hooks that valida
 | ---- | ------- | -------- |
 | **Warn Missing MCP** (`warn-missing-mcp.sh`) | SessionStart | Warns if the Very Good CLI is missing, older than 1.3.0, or present but unable to run because `dart` is not on the `PATH` hooks inherit; non-blocking |
 | **Check VGV CLI** (`check-vgv-cli.sh`) | PreToolUse (`mcp__.*very-good-cli__.*`) | Verifies from the payload that the caller is a Very Good CLI tool and stands aside otherwise, so the decision never lands on an unrelated tool; for its own tools, auto-approves the call in every run mode via a PreToolUse `allow` decision, so they never dead-end when the tool isn't on `permissions.allow` (including under `skipAutoPermissionPrompt`); denies with an install/upgrade message if the CLI is missing or < 1.3.0 |
-| **Block CLI Workarounds** (`block-cli-workarounds.sh`) | PreToolUse (`Bash`) | Blocks direct CLI bypass of Very Good CLI commands through the host's shell tool; inspects the command only when the payload identifies a shell call, so an unrelated tool carrying a `command` argument is left alone; when the CLI is present but cannot run because `dart` is missing from `PATH`, the denial says so instead of redirecting to an MCP server that cannot start; exits 2 on failure (blocking) |
+| **Block CLI Workarounds** (`block-cli-workarounds.sh`) | PreToolUse (`Bash`) | Denies `flutter`/`dart` `test`/`create` and `very_good test`/`create`/`packages` in the shell and points to the MCP tool. Quoted text is data unless `eval`, `sh -c` or `$( )` runs it. Every adjacent word pair is checked, so wrappers need no list. The denial names the match and says the whole call was refused |
 | **Allow Read-only Git** (`allow-readonly-git.sh`) | PreToolUse (`Bash`, `flutter-reviewer` agent only) | Restricts the `flutter-reviewer` agent's Bash to single-line `git diff`/`git status`, without the `--output` or `--ext-diff` options, and denies anything else (blocking). Scoped via the agent's frontmatter, not `hooks.json` |
 | **Analyze** (`analyze.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart analyze` on the modified `.dart` file; exits 2 on failure (blocking — Claude must fix issues before continuing) |
 | **Format** (`format.sh`) | PostToolUse (`Edit`/`Write`) | Runs `dart format` on the modified `.dart` file; always exits 0 (non-blocking — formatting is applied silently) |

config/cspell.json

@@ -33,6 +33,8 @@
     "GHSA",
     "goldens",
     "gradeable",
+    "gsub",
+    "heredocs",
     "hoverable",
     "icontains",
     "idgetbook",
@@ -49,6 +51,7 @@
     "mocktail",
     "monorepo",
     "Mundo",
+    "nohup",
     "opencode",
     "pasteable",
     "pipefail",
@@ -65,6 +68,7 @@
     "subagent",
     "subagents",
     "subclassing",
+    "subshell",
     "tappable",
     "tooltipped",
     "unmirrored",

hooks/scripts/block-cli-workarounds.sh

@@ -1,7 +1,6 @@
 #!/bin/bash
-# PreToolUse hook: block Bash commands that bypass MCP tools.
-# Denies flutter create, dart create, very_good create, very_good test,
-# very_good packages, flutter test, dart test.
+# PreToolUse hook: deny shell calls to CLI commands the MCP tools cover.
+# To block another command, add a hint line below and a test case.
 
 if ! command -v jq &>/dev/null; then
   echo "jq is required for block-cli-workarounds hook but not found" >&2
@@ -34,54 +33,71 @@ fi
 # assumes the chained command's side effect happened and carries on without it.
 WHOLE_CALL_REFUSED="This whole shell call was refused, so none of it ran: run any other commands it chained in a call of their own."
 
-# Deny with an install/upgrade message when the CLI is missing or outdated, with a PATH
-# message when it is present but cannot run, and otherwise redirect to the MCP tool.
+# Deny with a reason that fits the CLI status.
 deny_with_cli_check() {
-  local mcp_hint="$1"
-  local cli_status
+  local mcp_hint="$1" matched="$2"
+  local cli_status reason
   cli_status=$(check_vgv_cli)
   case "$cli_status" in
     not_installed)
-      deny "Very Good CLI is required but was not found. Install with: dart pub global activate very_good_cli. $WHOLE_CALL_REFUSED"
+      reason="Very Good CLI is required but was not found. Install with: dart pub global activate very_good_cli."
       ;;
     outdated:*)
-      local version="${cli_status#outdated:}"
-      deny "Very Good CLI ${version} is too old (requires >= ${MIN_VERSION}). Update with: dart pub global activate very_good_cli. $WHOLE_CALL_REFUSED"
+      reason="Very Good CLI ${cli_status#outdated:} is too old (requires >= ${MIN_VERSION}). Update with: dart pub global activate very_good_cli."
       ;;
     unverifiable)
       # Redirecting to the MCP tool here would be a dead end: the server starts through
       # the same very_good shim, which cannot exec dart from this PATH either.
-      deny "Very Good CLI was found but could not run: dart is not on the PATH available to hooks, so the very_good_cli MCP server cannot start either. Add the Dart SDK bin directory to PATH for non-interactive shells (e.g. in ~/.zprofile) and start a new session. $WHOLE_CALL_REFUSED"
+      reason="Very Good CLI was found but could not run: dart is not on the PATH available to hooks, so the very_good_cli MCP server cannot start either. Add the Dart SDK bin directory to PATH for non-interactive shells (e.g. in ~/.zprofile) and start a new session."
       ;;
     *)
-      deny "$mcp_hint $WHOLE_CALL_REFUSED"
+      reason="$mcp_hint"
       ;;
   esac
+  deny "$reason $WHOLE_CALL_REFUSED Matched: $matched"
 }
 
-# Split on shell operators and check the first two tokens of each subcommand.
-# This avoids false positives from file paths (.dart) or quoted strings.
-BLOCKED=$(echo "$COMMAND" | awk '{
-  n = split($0, parts, /[;&|]+/)
-  for (i = 1; i <= n; i++) {
-    gsub(/^[[:space:]]+/, "", parts[i])
-    split(parts[i], w, /[[:space:]]+/)
-    b = w[1]; s = w[2]
-    if ((b == "flutter" || b == "dart") && s == "create")   { print "create";      exit }
-    if ((b == "flutter" || b == "dart") && s == "test")     { print "test";         exit }
-    if (b == "very_good" && s == "create")                  { print "vg_create";    exit }
-    if (b == "very_good" && s == "test")                    { print "vg_test";      exit }
-    if (b == "very_good" && s == "packages")                { print "vg_packages";  exit }
+# Quoted text is data unless eval, sh -c or $( ) runs it. Every adjacent word pair is
+# checked, so wrappers (fvm, melos, sudo, timeout) and /path/to/flutter need no list.
+read -r -d '' find_invocation <<'AWK' || true   # read, not $(cat): unbalanced parens inside
+BEGIN {
+  RS = "\001"   # whole command is one record
+  hint["flutter test"]       = "Do not use 'flutter test' or 'dart test'. Use the very_good_cli MCP 'test' tool instead."
+  hint["dart test"]          = hint["flutter test"]
+  hint["flutter create"]     = "Do not use 'flutter create' or 'dart create'. Use the very_good_cli MCP 'create' tool instead."
+  hint["dart create"]        = hint["flutter create"]
+  hint["very_good test"]     = "Do not use 'very_good test' via shell. Use the very_good_cli MCP 'test' tool instead."
+  hint["very_good create"]   = "Do not use 'very_good create' via shell. Use the very_good_cli MCP 'create' tool instead."
+  hint["very_good packages"] = "Do not use 'very_good packages' via shell. Use the very_good_cli MCP 'packages_get' or 'packages_check_licenses' tool instead."
+}
+# First blocked "cmd sub" pair in s, or "". Extra params are awk locals.
+function scan(s,   n, w, i, b, pair) {
+  gsub(/[;&|(){}`\n]+/, " ; ", s)                # operators end a command
+  n = split(s, w, /[[:space:]]+/)
+  for (i = 1; i < n; i++) {
+    b = w[i]; sub(/^.*\//, "", b)                 # basename
+    pair = b " " w[i + 1]
+    if (pair in hint) return pair
   }
-}')
+  return ""
+}
+{
+  gsub(/\\["'$`]/, "_")                                               # escaped: literal
+  text  = $0; gsub(/'[^']*'/, "_", text); gsub(/"[^"]*"/, "_", text)  # quotes are data
+  subst = $0; gsub(/'[^']*'/, "_", subst); gsub(/"/, "", subst)       # but "$( )" runs
+  bare  = $0; gsub(/["']/, "", bare)                                  # and eval "" runs
+
+  pair = scan(text)
+  if (pair == "" && $0 ~ /\$\(|`/)                                pair = scan(subst)
+  if (pair == "" && $0 ~ /(^|[^[:alnum:]_])(eval|sh|bash|zsh) /)  pair = scan(bare)
+  if (pair != "") print hint[pair] "\t" pair
+}
+AWK
+
+RESULT=$(printf '%s\n' "$COMMAND" | awk "$find_invocation")   # printf: echo eats -n
 
-case "$BLOCKED" in
-  create)      deny_with_cli_check "Do not use 'flutter create' or 'dart create'. Use the very_good_cli MCP 'create' tool instead." ;;
-  test)        deny_with_cli_check "Do not use 'flutter test' or 'dart test'. Use the very_good_cli MCP 'test' tool instead." ;;
-  vg_create)   deny_with_cli_check "Do not use 'very_good create' via shell. Use the very_good_cli MCP 'create' tool instead." ;;
-  vg_test)     deny_with_cli_check "Do not use 'very_good test' via shell. Use the very_good_cli MCP 'test' tool instead." ;;
-  vg_packages) deny_with_cli_check "Do not use 'very_good packages' via shell. Use the very_good_cli MCP 'packages_get' or 'packages_check_licenses' tool instead." ;;
-esac
+if [ -n "$RESULT" ]; then
+  deny_with_cli_check "${RESULT%%$'\t'*}" "${RESULT#*$'\t'}"
+fi
 
-# Not a blocked command — allow
 exit 0

hooks/scripts/block-cli-workarounds_test.sh

@@ -66,14 +66,17 @@ run_hook() {
   run_hook_payload "$(jq -n --arg c "$1" '{"tool_input":{"command":$c}}')"
 }
 
+# Show newlines as ~ so multi-line commands keep the columns aligned.
+label_of() { printf '%s' "$1" | tr '\n' '~'; }
+
 assert_blocked() {
   local cmd="$1"
   run_hook "$cmd"
   if [ "$LAST_RESULT" = "deny" ]; then
-    printf "  \033[32mPASS\033[0m  blocked:  %s\n" "$cmd"
+    printf "  \033[32mPASS\033[0m  blocked:  %s\n" "$(label_of "$cmd")"
     PASSED=$((PASSED + 1))
   else
-    printf "  \033[31mFAIL\033[0m  expected deny but got %s:  %s\n" "$LAST_RESULT" "$cmd"
+    printf "  \033[31mFAIL\033[0m  expected deny but got %s:  %s\n" "$LAST_RESULT" "$(label_of "$cmd")"
     FAILED=$((FAILED + 1))
   fi
 }
@@ -82,10 +85,10 @@ assert_allowed() {
   local cmd="$1"
   run_hook "$cmd"
   if [ "$LAST_RESULT" = "aside" ]; then
-    printf "  \033[32mPASS\033[0m  allowed:  %s\n" "$cmd"
+    printf "  \033[32mPASS\033[0m  allowed:  %s\n" "$(label_of "$cmd")"
     PASSED=$((PASSED + 1))
   else
-    printf "  \033[31mFAIL\033[0m  expected aside but got %s:  %s\n" "$LAST_RESULT" "$cmd"
+    printf "  \033[31mFAIL\033[0m  expected aside but got %s:  %s\n" "$LAST_RESULT" "$(label_of "$cmd")"
     FAILED=$((FAILED + 1))
   fi
 }
@@ -104,6 +107,8 @@ assert_reason_contains() {
   fi
 }
 
+# Edit with Write/Edit, not a heredoc: the hook reads the heredoc body and denies it.
+
 echo "=== block-cli-workarounds tests ==="
 stub_cli 1.5.0
 echo ""
@@ -136,6 +141,136 @@ assert_allowed "gh pr create --body 'use dart test instead'"
 assert_allowed "git log --grep='dart test'"
 assert_allowed "ls"
 assert_allowed "pwd"
+assert_allowed ""
+assert_allowed "   "
+
+echo ""
+echo "--- Quoted text is data (issue #147) ---"
+
+# A quoted `|` is not a pipe. Both positions, since the bug only fired mid-alternation.
+assert_allowed "grep -nE \"very_good|flutter test|foo\" CLAUDE.md"
+assert_allowed "grep -nE \"very_good|flutter test\" CLAUDE.md"
+assert_allowed "grep -nE \"flutter test|very_good|foo\" AGENTS.md"
+assert_allowed "rg 'flutter create|dart create' docs/"
+
+# A quoted phrase is a single word and runs nothing.
+assert_allowed "echo \"flutter test\""
+assert_allowed "echo 'flutter test'"
+assert_allowed "git commit -m \"ban flutter test | dart test\""
+
+# One quote type is inert inside the other.
+assert_allowed "echo 'say \"flutter test\" now'"
+assert_allowed "echo \"say 'flutter test' now\""
+
+# Escapes and unbalanced quotes.
+assert_allowed "echo \\\"flutter test\\\""
+assert_allowed "grep \"flutter test file.md"
+assert_blocked "flutter test --name \"my app\""
+
+# A quoted string may span newlines and is still one word.
+assert_allowed "$(printf 'echo "hello\nflutter test\nworld"')"
+
+echo ""
+echo "--- Command position ---"
+
+# Unquoted separators open a command position.
+assert_blocked "echo hi; flutter test"
+assert_blocked "echo hi | flutter test"
+assert_blocked "echo hi & flutter test"
+assert_blocked "test -d lib || flutter test"
+
+# A prefix is still an invocation.
+assert_blocked "ENV=1 flutter test"
+assert_blocked "CI=true COVERAGE=1 dart test"
+assert_blocked "(flutter test)"
+assert_blocked "\$(flutter test)"
+assert_blocked "\`flutter test\`"
+assert_blocked "echo start && (very_good test)"
+
+# Wrappers need no list: every adjacent word pair is checked.
+assert_blocked "fvm flutter test"
+assert_blocked "command flutter test"
+assert_blocked "env flutter test"
+assert_blocked "env FOO=1 flutter test"
+assert_blocked "env -i flutter test"
+assert_blocked "sudo flutter test"
+assert_blocked "sudo -u ci flutter test"
+assert_blocked "nohup flutter test"
+assert_blocked "exec flutter test"
+assert_blocked "time flutter test"
+assert_blocked "timeout 60 flutter test"
+assert_blocked "nice -n 10 flutter test"
+assert_blocked "xargs flutter test"
+
+# melos runs commands across a VGV monorepo.
+assert_blocked "melos exec -- flutter test"
+assert_blocked "melos exec --concurrency 1 -- dart test"
+
+# Shell keywords and brace groups are separators.
+assert_blocked "if true; then flutter test; fi"
+assert_blocked "for f in a; do flutter test; done"
+assert_blocked "{ flutter test; }"
+assert_blocked "while :; do dart test; done"
+
+# Match on the basename.
+assert_blocked "/usr/local/bin/flutter test"
+assert_blocked "./flutter test"
+assert_blocked "\$FLUTTER_ROOT/bin/flutter test"
+assert_blocked "../sdk/bin/dart test"
+
+# ...but only when the wrapped command is itself blocked.
+assert_allowed "fvm flutter pub get"
+assert_allowed "command -v flutter"
+assert_allowed "env | grep PATH"
+assert_allowed "melos exec -- dart analyze"
+assert_allowed "timeout 60 dart pub get"
+assert_allowed "/usr/local/bin/flutter analyze"
+
+# A wrapper with nothing after it has no pair to match.
+assert_allowed "fvm"
+assert_allowed "env -i"
+assert_allowed "ENV=1"
+
+# A basename that only resembles the command.
+assert_allowed "git add lib/router.dart test/router_test.dart"
+assert_allowed "cp foo.dart test/"
+assert_allowed "ls bin/flutter_tools"
+
+# Comments are not modelled. `; dart test` inside one is denied: accepted, since the
+# agent does not write comments in tool calls.
+assert_blocked "$(printf '# a comment\nflutter test')"
+assert_blocked "$(printf 'ls # note\ncd pkg\nflutter test')"
+assert_allowed "$(printf '# it%ss broken\nls -la' "'")"
+assert_blocked "ls # fix; dart test"
+
+# `$( )` and backticks run inside double quotes. Single quotes and `\$` are inert.
+assert_blocked "OUT=\"\$(flutter test)\""
+assert_blocked "echo \"\$(flutter test)\""
+assert_blocked "echo \"\`flutter test\`\""
+assert_blocked "if [ -z \"\$(dart test)\" ]; then echo x; fi"
+assert_allowed "echo \"\\\$(flutter test)\""
+assert_allowed "echo '\$(flutter test)'"
+assert_allowed "echo \"\$(date) building\""
+assert_allowed "VAR=\"\$(ls)\"; dart analyze"
+
+# eval and sh -c run their string.
+assert_blocked "eval \"flutter test\""
+assert_blocked "bash -c 'flutter test'"
+assert_blocked "sh -c \"flutter test\""
+assert_blocked "zsh -c \"cd pkg && dart test\""
+
+echo ""
+echo "--- Documented non-goals ---"
+#
+# Allowed on purpose and pinned. A variable needs execution to resolve; the rest are
+# forms nobody types.
+assert_allowed "F=flutter; \$F test"
+assert_allowed "\"flutter\" test"
+assert_allowed "'dart' test"
+assert_allowed "$(printf 'flutter \\\ntest --coverage')"
+
+# Heredoc bodies are not modelled; denied as a known limitation.
+assert_blocked "$(printf 'cat <<EOF\nflutter test\nEOF')"
 
 echo ""
 echo "--- Tool scoping ---"
@@ -179,6 +314,16 @@ stub_cli 1.5.0
 assert_blocked "flutter test"
 assert_reason_contains "MCP 'test' tool" "current CLI redirects to the MCP tool"
 
+# The reason names the match.
+assert_reason_contains "Matched: flutter test" "deny reason quotes the matched command"
+
+assert_blocked "fvm dart create my_app"
+assert_reason_contains "Matched: dart create" "wrapper is skipped in the matched command"
+
+# very_good hits name the match too.
+assert_blocked "very_good packages check licenses"
+assert_reason_contains "Matched: very_good packages" "very_good hits name the matched command"
+
 stub_cli 1.2.9
 assert_blocked "flutter test"
 assert_reason_contains "too old" "outdated CLI asks for an update"