From 119a68faa2e97e77a10c7d641a17c1ec860d27ae Mon Sep 17 00:00:00 2001 From: mark-wint Date: Fri, 2 Oct 2026 09:51:54 -0400 Subject: [PATCH] fix: say a blocked shell call ran none of its chained commands block-cli-workarounds refuses the whole shell call, so a command chained with the blocked one (`edit-a-file && dart test`) never runs either. The deny reason only said to use the MCP tool, so an agent could assume the chained command's side effect happened and carry on without it. Every deny reason from this hook now says the whole call was refused. Co-Authored-By: Claude Opus 5.5 --- CLAUDE.md | 2 +- hooks/scripts/block-cli-workarounds.sh | 13 +++++++++---- hooks/scripts/block-cli-workarounds_test.sh | 21 +++++++++++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index ca67553..e2a900a 100644 --- a/CLAUDE.md +++ b/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__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; exits 2 on failure (blocking) +- `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) 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 diff --git a/hooks/scripts/block-cli-workarounds.sh b/hooks/scripts/block-cli-workarounds.sh index d422d99..98f2985 100644 --- a/hooks/scripts/block-cli-workarounds.sh +++ b/hooks/scripts/block-cli-workarounds.sh @@ -29,6 +29,11 @@ if [ -z "$COMMAND" ]; then exit 0 fi +# A deny refuses the whole shell call, so a command chained before or after the blocked +# one (`edit-a-file && dart test`) never runs either. Without saying so, the agent +# 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_cli_check() { @@ -37,19 +42,19 @@ deny_with_cli_check() { 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" + deny "Very Good CLI is required but was not found. Install with: dart pub global activate very_good_cli. $WHOLE_CALL_REFUSED" ;; 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" + deny "Very Good CLI ${version} is too old (requires >= ${MIN_VERSION}). Update with: dart pub global activate very_good_cli. $WHOLE_CALL_REFUSED" ;; 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." + 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" ;; *) - deny "$mcp_hint" + deny "$mcp_hint $WHOLE_CALL_REFUSED" ;; esac } diff --git a/hooks/scripts/block-cli-workarounds_test.sh b/hooks/scripts/block-cli-workarounds_test.sh index 128c6d9..d5d182e 100755 --- a/hooks/scripts/block-cli-workarounds_test.sh +++ b/hooks/scripts/block-cli-workarounds_test.sh @@ -193,6 +193,27 @@ stub_cli assert_blocked "flutter test" assert_reason_contains "dart is not on the PATH" "CLI that cannot run points at PATH, not the MCP tool" +echo "" +echo "--- Deny reason says the whole call was refused ---" + +# A command chained with the blocked one is refused with it. Every reason must say so, +# or the agent assumes the chained command's side effect happened. +stub_cli 1.5.0 +assert_blocked "python3 edit_pubspec.py && dart test" +assert_reason_contains "none of it ran" "chained command, current CLI" + +stub_cli 1.2.9 +assert_blocked "python3 edit_pubspec.py && dart test" +assert_reason_contains "none of it ran" "chained command, outdated CLI" + +no_cli +assert_blocked "python3 edit_pubspec.py && dart test" +assert_reason_contains "none of it ran" "chained command, missing CLI" + +stub_cli +assert_blocked "python3 edit_pubspec.py && dart test" +assert_reason_contains "none of it ran" "chained command, CLI that cannot run" + echo "" echo "=== Results: $PASSED passed, $FAILED failed ==="