From 69ace118b308ae034db720751a5cb4d51e5ad175 Mon Sep 17 00:00:00 2001 From: Tym Rabchuk Date: Fri, 10 Apr 2026 20:01:18 -0400 Subject: [PATCH 1/3] fix(hooks): extend devkit-guard to prompt steps + orphan recovery (#63) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the drift hole where a workflow prompt step returns its prompt and the agent can run arbitrary unrelated tools without ever calling devkit_advance. The engine correctly holds step ordering inside a workflow; this closes the outer "force the agent to actually use it" loop at the hook layer. - devkit-guard.sh: under enforce=hard, prompt steps now allow only Read/Grep/Glob/TodoWrite/NotebookRead + devkit MCP tools. Write, Edit, Bash, Task, WebFetch, and other MCP tools are blocked with a veto naming workflow + step N/M. Soft enforce emits a stderr nudge instead of blocking. - hooks/lib/read-session.sh: shared python3 parser sourced by both devkit-guard and devkit-stop-guard (eliminates drift risk from duplicate parsing). - SessionState.UpdatedAt: bumped on every WriteSessionJSON; hooks treat sessions idle past 30min TTL as orphaned and stop enforcing. - devkit_start: reclaims stale sessions instead of rejecting, so a crashed engine no longer wedges the slot forever. - hooks.json: devkit-guard timeout 2s → 5s to hedge python3 cold-start on Windows (proper fix is a native Go guard — follow-up issue). - Tests: Go coverage for UpdatedAt bump + stale/fresh reclaim cutoff; new hooks/devkit-guard_test.sh fixture matrix (21 cases) covering command/prompt × hard/soft × fresh/stale. --- README.md | 2 +- hooks/devkit-guard.sh | 117 ++++++++++++++++---------- hooks/devkit-guard_test.sh | 167 +++++++++++++++++++++++++++++++++++++ hooks/devkit-stop-guard.sh | 67 ++++++++------- hooks/hooks.json | 2 +- hooks/lib/read-session.sh | 89 ++++++++++++++++++++ src/lib/state_json.go | 35 ++++---- src/lib/state_test.go | 46 ++++++++++ src/mcp/tools.go | 24 +++++- src/mcp/tools_test.go | 164 ++++++++++++++++++++++++++++++++++++ 10 files changed, 626 insertions(+), 87 deletions(-) create mode 100755 hooks/devkit-guard_test.sh create mode 100644 hooks/lib/read-session.sh diff --git a/README.md b/README.md index 1c47758..174a42d 100644 --- a/README.md +++ b/README.md @@ -197,7 +197,7 @@ Coding principles (`clean-code`, `dry`, `yagni`, `dont-reinvent`, `executing`, ` | PreToolUse | **audit-trail** | Logs every command to `.devkit/audit.log` | | PreToolUse | **pr-gate** | Prompts to run the pr-ready skill before `gh pr create` | | PreToolUse | **rtk-rewrite** | Compresses Bash output via RTK (no-op if not installed) | -| PreToolUse | **devkit-guard** | Blocks out-of-step tools during workflow command steps | +| PreToolUse | **devkit-guard** | Blocks out-of-step tools during workflow command AND prompt steps (hard enforce); soft enforce emits a reminder. Skills are intentionally unguarded. | | PostToolUse | **post-validate** | Suppressed errors, leaked secrets, writes outside repo | | PostToolUse | **slop-detect** | AI code patterns — doc/code imbalance, restating comments | | PostToolUse | **lang-review** | Language-aware checks: Go, TypeScript, Rust, Python, Shell | diff --git a/hooks/devkit-guard.sh b/hooks/devkit-guard.sh index 5419f6e..ea7c281 100755 --- a/hooks/devkit-guard.sh +++ b/hooks/devkit-guard.sh @@ -5,81 +5,114 @@ set -euo pipefail # Reads $CLAUDE_PLUGIN_DATA/session.json. Blocks out-of-step actions. # Exit 0 = allow, Exit 2 + stderr = hard block. # -# Policy: during a command step (workflow.yml `command:`), the engine — -# not Claude — executes the shell. The only tool Claude is allowed to -# call is devkit_advance (which triggers execution and returns the next -# step). Everything else is blocked so Claude cannot observe or -# interfere with the state of the step. +# Policy matrix: +# step_type=command, enforce=hard → allow only devkit MCP + TodoWrite +# (engine runs the command, not Claude) +# step_type=prompt, enforce=hard → allow evidence-gathering + devkit MCP +# (Read/Grep/Glob/TodoWrite) — force +# the agent to advance before any +# write/bash/dispatch. Closes issue #63 +# drift hole. +# step_type=prompt, enforce=soft → allow everything, emit stderr nudge +# step_type=parallel → allow everything (engine dispatches) +# stale session (see lib/read-session.sh) → allow + warn; do not enforce +# against an orphaned state file. # # This hook uses an ALLOWLIST rather than a blocklist because the # Claude Code tool surface evolves — Task, SlashCommand, ExitPlanMode, # BashOutput, KillBash, TodoWrite, any mcp__* tool, and future names # would silently bypass a blocklist of hardcoded names. +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=lib/read-session.sh +source "${SCRIPT_DIR}/lib/read-session.sh" + DATA_DIR="${CLAUDE_PLUGIN_DATA:-}" if [[ -z "$DATA_DIR" ]]; then printf 'devkit-guard: CLAUDE_PLUGIN_DATA unset — enforcement disabled\n' >&2 - exit 0 # not in plugin context + exit 0 fi SESSION_FILE="${DATA_DIR}/session.json" -if [[ ! -f "$SESSION_FILE" ]]; then - exit 0 # no active workflow -fi -# Parse all session fields in a single python3 call (no jq dependency). -# Outputs tab-separated: status, step_type, enforce, current_step. -# Passes file path via sys.argv to prevent shell injection. Handles -# FileNotFoundError so a TOCTOU race (file cleared between -f and open) -# is treated as "no session," matching the pre-check intent. -SESSION_DATA=$(python3 -c " -import json, sys -try: - d = json.load(open(sys.argv[1])) -except FileNotFoundError: - print('\t'.join(['', '', '', ''])) - sys.exit(0) -print('\t'.join([ - d.get('status', ''), - d.get('step_type', ''), - d.get('enforce', 'hard'), - d.get('current_step', '') -])) -" "$SESSION_FILE" 2>/dev/null) || { - # python3 unavailable or JSON corrupt — fail closed if session file exists - printf 'BLOCKED: Cannot parse session state (python3 required or JSON corrupt). Remove %s to clear.\n' "$SESSION_FILE" >&2 - exit 2 -} +if ! parse_session_fields "$SESSION_FILE"; then + # python3 unavailable or JSON corrupt — fail closed if session file + # exists, otherwise fall through (no session = nothing to guard). + if [[ -f "$SESSION_FILE" ]]; then + printf 'BLOCKED: Cannot parse session state (python3 required or JSON corrupt). Remove %s to clear.\n' "$SESSION_FILE" >&2 + exit 2 + fi + exit 0 +fi -IFS=$'\t' read -r STATUS STEP_TYPE ENFORCE CURRENT_STEP <<< "$SESSION_DATA" +if [[ "$SESSION_STATUS" != "running" ]]; then + exit 0 +fi -if [[ "$STATUS" != "running" ]]; then +if [[ "$SESSION_STALE" == "1" ]]; then + printf 'devkit-guard: session %s idle past TTL — treating as orphaned (run devkit_start to reclaim)\n' "$SESSION_WORKFLOW" >&2 exit 0 fi -# Read tool name from stdin +# Read tool name from stdin. Matches PreToolUse payload format. INPUT=$(cat) TOOL_NAME=$(printf '%s' "$INPUT" | python3 -c "import json,sys; print(json.load(sys.stdin).get('tool_name',''))" 2>/dev/null || echo "") +# Build a progress label for veto messages so the agent always sees +# workflow + position without another devkit_status round trip. +step_label() { + if [[ -n "$SESSION_CURRENT_INDEX" && -n "$SESSION_TOTAL_STEPS" ]]; then + local human_index=$((SESSION_CURRENT_INDEX + 1)) + printf '%s step %d/%d (%s)' "$SESSION_WORKFLOW" "$human_index" "$SESSION_TOTAL_STEPS" "$SESSION_CURRENT_STEP" + else + printf '%s (%s)' "$SESSION_WORKFLOW" "$SESSION_CURRENT_STEP" + fi +} + # Command steps: allow ONLY the MCP tools needed to progress the -# workflow. Everything else is blocked, including future tools the -# hook author hasn't heard of. -if [[ "$STEP_TYPE" == "command" && "$ENFORCE" == "hard" ]]; then +# workflow. Everything else is blocked, including future tools. +if [[ "$SESSION_STEP_TYPE" == "command" && "$SESSION_ENFORCE" == "hard" ]]; then case "$TOOL_NAME" in - # MCP devkit tools — Claude uses these to drive the engine. mcp__*devkit*|devkit_advance|devkit_status|devkit_list|devkit_start) exit 0 ;; - # TodoWrite is a pure in-memory tracker with no side effects, allowed. TodoWrite) exit 0 ;; *) - printf 'BLOCKED: Command step "%s" in progress — the engine runs this step. Call devkit_advance to execute it. (attempted tool: %s)\n' "$CURRENT_STEP" "$TOOL_NAME" >&2 + printf 'BLOCKED: Command step "%s" in progress — the engine runs this step. Call devkit_advance to execute it. (attempted tool: %s)\n' "$(step_label)" "$TOOL_NAME" >&2 exit 2 ;; esac fi -# Prompt/parallel steps: allow everything (Claude needs full tool access). +# Prompt steps under hard enforcement: allow read-only evidence tools +# plus devkit MCP. Blocks Write/Edit/Bash/Task/WebFetch/other MCP so +# the agent cannot drift into unrelated work between step 1 and +# devkit_advance. See issue #63. +if [[ "$SESSION_STEP_TYPE" == "prompt" && "$SESSION_ENFORCE" == "hard" ]]; then + case "$TOOL_NAME" in + mcp__*devkit*|devkit_advance|devkit_status|devkit_list|devkit_start) + exit 0 + ;; + Read|Grep|Glob|TodoWrite|NotebookRead) + exit 0 + ;; + *) + printf 'BLOCKED: devkit workflow %s is at a prompt step — gather evidence with Read/Grep/Glob then call devkit_advance. (attempted tool: %s)\n' "$(step_label)" "$TOOL_NAME" >&2 + exit 2 + ;; + esac +fi + +# Prompt steps under soft enforcement: allow everything, but inject a +# stderr nudge so the transcript shows the agent that a step is open. +# Soft nudge is idempotent — if the agent ignores it, Stop gate still +# blocks via devkit-stop-guard.sh. +if [[ "$SESSION_STEP_TYPE" == "prompt" && "$SESSION_ENFORCE" != "hard" ]]; then + printf 'devkit-guard: %s is open — call devkit_advance when the step is complete.\n' "$(step_label)" >&2 + exit 0 +fi + +# Parallel steps: engine is dispatching, agent needs full tool access. exit 0 diff --git a/hooks/devkit-guard_test.sh b/hooks/devkit-guard_test.sh new file mode 100755 index 0000000..ade37ab --- /dev/null +++ b/hooks/devkit-guard_test.sh @@ -0,0 +1,167 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Fixture matrix test for devkit-guard.sh. +# Seeds CLAUDE_PLUGIN_DATA with a crafted session.json and pipes a +# synthetic PreToolUse payload on stdin. Asserts exit code and the +# substring of whatever stderr diagnostic the guard emitted. +# +# Run: bash hooks/devkit-guard_test.sh + +SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +GUARD="${SCRIPT_DIR}/devkit-guard.sh" + +if [[ ! -x "$GUARD" ]]; then + chmod +x "$GUARD" || true +fi + +PASS=0 +FAIL=0 +FAILED_CASES=() + +# run_case