diff --git a/plugins/guardrails/.claude-plugin/plugin.json b/plugins/guardrails/.claude-plugin/plugin.json index 68e9dad03..584483bdf 100644 --- a/plugins/guardrails/.claude-plugin/plugin.json +++ b/plugins/guardrails/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "$schema": "https://json.schemastore.org/claude-code-plugin-manifest.json", "name": "guardrails", - "version": "0.9.3", + "version": "0.9.4", "description": "Eight safety guards that block secret/credential writes, hardcoded machine-specific paths, git hook-bypass attempts, irreversible git operations (force-push, reset --hard, worktree-wide checkout/restore discards), Bash file-write workarounds that circumvent Write/Edit hooks, (advisory) hallucinated CLI flags, (advisory) un-throttled Workflow fan-out that risks burst 529s, and (advisory) direct git commit/gh pr create calls bypassing this marketplace's own commit/pull-request skills — each independently toggleable.", "author": { "name": "Melodic Software", diff --git a/plugins/guardrails/CHANGELOG.md b/plugins/guardrails/CHANGELOG.md index bf12d1ee1..41dcebd52 100644 --- a/plugins/guardrails/CHANGELOG.md +++ b/plugins/guardrails/CHANGELOG.md @@ -3,6 +3,25 @@ All notable changes to the `guardrails` plugin are documented here. Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); this plugin uses semantic versioning. +## [0.9.4] + +### Fixed + +- **`cli-flag-verify` scans only the content the tool call wrote, never the whole file + from disk.** The PostToolUse check re-read the entire edited file, so any edit to a + file already containing an unrecognized flag elsewhere re-fired the advisory about + lines the edit never touched. The hook now scans the tool payload — an Edit's + changed hunk, a Write's full content (a PostToolUse Write payload cannot distinguish + a new file from an overwrite, so whole-content is the closest the payload allows) — + per the hook-precision convention's diff-scoping rule. Repro-first: the + pre-existing-flag stay-quiet case fails against the prior hook and passes now, with + a hunk-introduced-flag MUST-FIRE counterpart. Markdown fence state is derived from + the hunk alone — a fence-straddling edit can misclassify in either direction, the + accepted trade of hunk scoping. A partial-replacement edit whose hunk is a bare + flag fragment (no binary in the changed region) reconstructs bounded on-disk + context — the lines carrying the hunk's flag tokens — so a swapped-in unknown flag + still fires, while a pre-existing unrelated flag sharing that line stays quiet. + ## [0.9.3] ### Changed diff --git a/plugins/guardrails/hooks/cli-flag-verify.sh b/plugins/guardrails/hooks/cli-flag-verify.sh index 3d196b06d..fa1047028 100755 --- a/plugins/guardrails/hooks/cli-flag-verify.sh +++ b/plugins/guardrails/hooks/cli-flag-verify.sh @@ -59,6 +59,22 @@ case "$FILE" in *) exit 0 ;; esac +# Diff-scope: verify only the content THIS tool call wrote, never re-read the +# whole file from disk. An Edit's pre-existing lines outside the changed hunk are +# not this call's claims; scanning the whole file re-flags an unknown flag on +# every later unrelated edit. Edit -> the changed hunk (new_string); Write -> the +# full payload content (a PostToolUse Write payload cannot distinguish a new file +# from an overwrite, so "whole-file only for new files" degrades to whole-content +# here — still the payload, never disk). The live matcher is Write|Edit; any +# other tool carries nothing this call wrote that we can scope a scan to. +TOOL=$(printf '%s' "$INPUT" | jq -r '.tool_name // empty' 2>/dev/null | tr -d '\r') +case "$TOOL" in +Edit) SCAN_CONTENT=$(printf '%s' "$INPUT" | jq -r '.tool_input.new_string // empty' 2>/dev/null | tr -d '\r') ;; +Write) SCAN_CONTENT=$(printf '%s' "$INPUT" | jq -r '.tool_input.content // empty' 2>/dev/null | tr -d '\r') ;; +*) exit 0 ;; +esac +[[ -n "$SCAN_CONTENT" ]] || exit 0 + [[ -x "$VERIFIER" ]] || exit 0 # Verifier not present — fail open, don't block # Repo root for telemetry data.file + relative sink resolution (file-anchored). @@ -93,30 +109,40 @@ is_skipped() { return 1 } -# Emit candidate command fragments (one per line) from the file. +# Emit candidate command fragments (one per line) from the scanned payload +# content ($SCAN_CONTENT — the changed hunk for Edit, full content for Write). # - Markdown: scan CODE only — inline-code spans (one combined grep, then # strip the delimiting backticks) + fenced code-block bodies. Fenced content # carries no inline backticks, so the inline grep naturally excludes it. # - Shell/PowerShell: every line is code. +# Fence state is derived from the hunk alone, so a fence-straddling markdown edit +# can misclassify in EITHER direction (a hunk inside a block whose ``` markers +# fall outside it reads as prose and is under-scanned; a hunk that opens with a +# closing ``` can over-scan). The convention trades this whole-file context away +# for diff-scope precision; this fence-straddling residual is accepted, not +# reconstructed. A distinct residual — an Edit hunk that is a bare flag fragment +# with no binary in the changed region — IS recovered downstream by +# reconstruct_partial_edit, which pulls bounded on-disk context so a swapped-in +# unknown flag is not missed. emit_fragments() { if [[ "$IS_MD" == "true" ]]; then # Inline-code spans (strip the delimiting backticks). # shellcheck disable=SC2016 # backticks are literal ERE/sed data, not expansions - grep -oE '`[^`]+`' "$FILE" 2>/dev/null | sed -E 's/^`+//; s/`+$//' + printf '%s' "$SCAN_CONTENT" | grep -oE '`[^`]+`' 2>/dev/null | sed -E 's/^`+//; s/`+$//' # Fenced code-block bodies, skipping in-fence comment lines (a backtick- # wrapped example inside a `#` comment would otherwise split into a segment). - awk ' + printf '%s' "$SCAN_CONTENT" | awk ' /^[[:space:]]*```/ { f = !f; next } /^[[:space:]]*~~~/ { f = !f; next } f && $0 !~ /^[[:space:]]*#/ { print } - ' "$FILE" 2>/dev/null + ' 2>/dev/null else # Shell/PowerShell: every non-comment line is code. Drop full-line comments # BEFORE separator-splitting — otherwise a backtick-wrapped CLI example in a # comment is split on its backticks into a spurious bin-led command segment # (the comment's leading `#` lands in a different segment). `|| true`: grep # exits 1 when every line is a comment. - grep -vE '^[[:space:]]*#' "$FILE" 2>/dev/null || true + printf '%s' "$SCAN_CONTENT" | grep -vE '^[[:space:]]*#' 2>/dev/null || true fi } @@ -226,6 +252,55 @@ extract_candidates() { extract_candidates +# Partial-replacement context reconstruction (Edit only). When an Edit's hunk is +# a bare flag fragment — a flag token swapped in with no binary/subcommand in the +# changed region — the diff-scoped scan finds no command candidate and would +# silently miss a genuinely-introduced unknown flag. Recover bounded context: +# pull from the on-disk file only the lines carrying one of the hunk's flag +# tokens (the edit has already been applied by PostToolUse time, so the swapped-in +# flag is on disk), scan those, and keep only candidates whose flag appears in the +# hunk. The flag-token filter is what preserves the diff-scope contract: a +# pre-existing unrelated flag sharing one of those lines never re-fires. The +# anchor is the flag token, not the line — a bare-flag hunk carries no positional +# information — so a hunk flag that also occurs on an untouched line is scanned +# there too; only a DIFFERENT pre-existing flag is excluded. +reconstruct_partial_edit() { + [[ "$TOOL" == "Edit" && -f "$FILE" ]] || return 0 + # Trigger only when the hunk produced no command candidate; a full-command hunk + # already scans correctly and must not re-scan from disk. + ((${#CANDIDATES[@]} == 0)) || return 0 + local -a hunk_flags=() + mapfile -t hunk_flags < <( + printf '%s' "$SCAN_CONTENT" | + grep -oE '(^|[[:space:]])--?[A-Za-z0-9][A-Za-z0-9-]*' 2>/dev/null | + sed -E 's/^[[:space:]]*//' | sort -u + ) + ((${#hunk_flags[@]})) || return 0 + local tok ctx="" lines + for tok in "${hunk_flags[@]}"; do + lines=$(grep -F -- "$tok" "$FILE" 2>/dev/null) + [[ -n "$lines" ]] && ctx+="$lines"$'\n' + done + ctx=$(printf '%s' "$ctx" | grep -vE '^[[:space:]]*$' | head -20) + [[ -n "$ctx" ]] || return 0 + SCAN_CONTENT="$ctx" + extract_candidates + local key flag t keep + for key in "${!CANDIDATES[@]}"; do + flag="${key##*|}" + keep=0 + for t in "${hunk_flags[@]}"; do + [[ "$flag" == "$t" ]] && { + keep=1 + break + } + done + ((keep)) || unset 'CANDIDATES[$key]' + done +} + +reconstruct_partial_edit + # Verify each unique (bin, chain, flag). Collect failures (keys, formatted later). FAILURES=() for key in "${!CANDIDATES[@]}"; do diff --git a/plugins/guardrails/hooks/cli-flag-verify.test.sh b/plugins/guardrails/hooks/cli-flag-verify.test.sh index 05ba0e8a1..3bff511fc 100755 --- a/plugins/guardrails/hooks/cli-flag-verify.test.sh +++ b/plugins/guardrails/hooks/cli-flag-verify.test.sh @@ -46,6 +46,11 @@ chmod +x "$FAKE_BIN_DIR/faketool" # run_fake: invoke the hook with faketool as the only scanned bin and an # isolated per-case verifier cache so a stale 24h --help cache never leaks. +# Models a Write: the content goes into the payload's `content` (what the hook +# now scans) AND to disk (so the file existence + extension checks pass). Keeping +# disk == payload means the pre-fix hook — which read the file — scans the same +# bytes, so this conversion is behavior-preserving and every existing case stays +# green across the diff-scope fix. run_fake() { local content="$1" ext="${2:-sh}" local case_dir="$TEST_TMPDIR/fake-$((PASS + FAIL + 1))" @@ -56,7 +61,24 @@ run_fake() { CLAUDE_PLUGIN_OPTION_CLI_FLAG_VERIFY_BINS=faketool \ LOCALAPPDATA="$case_dir/cache" \ XDG_CACHE_HOME="$case_dir/cache" \ - bash "$HOOK" <<<"$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$target" '{tool_input:{file_path:$fp}}')" 2>&1 + bash "$HOOK" <<<"$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$target" --arg c "$content" '{tool_name:"Write",tool_input:{file_path:$fp,content:$c}}')" 2>&1 +} + +# run_edit [ext]: models an Edit — the file on disk +# carries (pre-existing lines), the payload's new_string is the +# changed hunk. The hook must scan ONLY the hunk, never the disk body. Isolates +# the diff-scope contract: disk and payload deliberately differ. +run_edit() { + local disk="$1" hunk="$2" ext="${3:-sh}" + local case_dir="$TEST_TMPDIR/edit-$((PASS + FAIL + 1))" + mkdir -p "$case_dir/cache" + local target="$case_dir/target.$ext" + printf '%s\n' "$disk" >"$target" + PATH="$FAKE_BIN_DIR:$PATH" \ + CLAUDE_PLUGIN_OPTION_CLI_FLAG_VERIFY_BINS=faketool \ + LOCALAPPDATA="$case_dir/cache" \ + XDG_CACHE_HOME="$case_dir/cache" \ + bash "$HOOK" <<<"$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$target" --arg s "$hunk" '{tool_name:"Edit",tool_input:{file_path:$fp,new_string:$s}}')" 2>&1 } OUT=$(run_fake 'faketool sub --real'); RC=$? @@ -122,11 +144,44 @@ assert_exit "--version universally skipped → exit 0" 0 "$RC" OUT=$(run_fake 'faketool sub --fake' cs); RC=$? assert_exit "non-target ext (.cs) → exit 0" 0 "$RC" +# ======================= DIFF-SCOPE (edit hunk only) ======================= +# An Edit must verify only its changed hunk, never the whole file on disk. Repro +# shape: a file already carrying an unknown flag, edited elsewhere with a clean +# hunk, must stay quiet — the pre-fix whole-file scan re-flagged the untouched +# line on every unrelated edit. +OUT=$(run_edit 'faketool sub --fake' 'faketool sub --real'); RC=$? +assert_exit "diff-scope: unknown flag on disk, clean hunk → exit 0" 0 "$RC" +assert_silent "diff-scope: pre-existing flag outside the hunk not re-flagged" "$OUT" + +# Counterpart MUST-fire: an unknown flag introduced BY the edit's hunk fires. +OUT=$(run_edit 'faketool sub --real' 'faketool sub --fake'); RC=$? +assert_exit "diff-scope: unknown flag in the hunk → exit 0" 0 "$RC" +ctx_contains "diff-scope: hunk flag reported" "$OUT" "UNKNOWN_FLAG: faketool sub --fake" + +# --------------- PARTIAL-REPLACEMENT (bare-flag hunk reconstruction) --------- +# An Edit whose new_string is ONLY the swapped-in flag carries no binary or +# subcommand, so the hunk yields no command candidate and a genuinely-introduced +# unknown flag would be missed. The disk lines model POST-edit state (PostToolUse +# runs after the edit applies), so the flag token is already on disk. Bounded +# context reconstruction pulls the on-disk line carrying the hunk's flag token +# and scans it. MUST-FIRE: fails against the pre-fix hook, passes after. +OUT=$(run_edit 'faketool sub --fake' '--fake'); RC=$? +assert_exit "partial-edit: bare-flag hunk reconstructs context → exit 0" 0 "$RC" +ctx_contains "partial-edit: reconstructed hunk flag reported" "$OUT" "UNKNOWN_FLAG: faketool sub --fake" + +# MUST-STAY-QUIET: the same bare-flag shape where the disk line ALSO carries a +# different pre-existing unknown flag NOT in new_string. Reconstruction keeps +# only candidates whose flag appears in the hunk, so the unrelated --otherbogus +# never re-fires — the diff-scope contract holds through reconstruction. +OUT=$(run_edit 'faketool sub --real --otherbogus' '--real'); RC=$? +assert_exit "partial-edit: unrelated pre-existing flag not re-fired → exit 0" 0 "$RC" +assert_silent "partial-edit: only hunk-flag verified, --otherbogus stays quiet" "$OUT" + # Kill switch — disabled path is a clean no-op despite a hallucinated flag. dis_dir="$TEST_TMPDIR/fake-disabled" mkdir -p "$dis_dir/cache" printf 'faketool sub --fake\n' >"$dis_dir/target.sh" -dis_input=$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$dis_dir/target.sh" '{tool_input:{file_path:$fp}}') +dis_input=$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$dis_dir/target.sh" --arg c 'faketool sub --fake' '{tool_name:"Write",tool_input:{file_path:$fp,content:$c}}') OUT=$(PATH="$FAKE_BIN_DIR:$PATH" CLAUDE_PLUGIN_OPTION_CLI_FLAG_VERIFY_BINS=faketool \ LOCALAPPDATA="$dis_dir/cache" XDG_CACHE_HOME="$dis_dir/cache" \ CLAUDE_PLUGIN_OPTION_CLI_FLAG_VERIFY_ENABLED=false bash "$HOOK" <<<"$dis_input" 2>&1); RC=$? @@ -155,7 +210,7 @@ SINK="$(make_sink "cat >\"$TEL\"")" tel_dir="$TEST_TMPDIR/fake-tel" mkdir -p "$tel_dir/cache" printf 'faketool sub --fake\n' >"$tel_dir/target.sh" -tel_input=$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$tel_dir/target.sh" '{tool_input:{file_path:$fp}}') +tel_input=$(MSYS_NO_PATHCONV=1 jq -n --arg fp "$tel_dir/target.sh" --arg c 'faketool sub --fake' '{tool_name:"Write",tool_input:{file_path:$fp,content:$c}}') PATH="$FAKE_BIN_DIR:$PATH" CLAUDE_PLUGIN_OPTION_CLI_FLAG_VERIFY_BINS=faketool \ LOCALAPPDATA="$tel_dir/cache" XDG_CACHE_HOME="$tel_dir/cache" \ HOOK_TELEMETRY_SINK="$SINK" bash "$HOOK" <<<"$tel_input" >/dev/null 2>&1 || true