diff --git a/docs/verification/find-delete-gate-negctl.sh b/docs/verification/find-delete-gate-negctl.sh new file mode 100755 index 00000000..6a5903d7 --- /dev/null +++ b/docs/verification/find-delete-gate-negctl.sh @@ -0,0 +1,42 @@ +#!/bin/bash +# Negative control for the find-delete gate fix. +# Payloads live here (not on the caller's command line) so the live gate does not +# fire on the harness that is testing it. +cd ~/workspace/tieubao/dwarves-kit || exit 1 +export DWARVES_KIT_LOG_DIR=$(mktemp -d) + +INCIDENT='find ~/.cache/.bun -mindepth 1 -delete 2>/dev/null' +CONTROL_BLOCK='rm -r'' -f src' # split so this file's own text is inert +CONTROL_ALLOW='find node_modules -mindepth 1 -delete' + +probe() { # + jq -nc --arg c "$1" '{tool_input:{command:$c}}' \ + | bash hooks/safety-gate.sh >/dev/null 2>&1 + echo $? +} + +row() { printf ' %-34s exit=%s (want %s)\n' "$2" "$(probe "$1")" "$3"; } + +echo "### STEP 1 -- fix in place (HEAD)" +echo " sha: $(shasum hooks/safety-gate.sh | cut -c1-12)" +row "$INCIDENT" "incident: find -delete" 2 +row "$CONTROL_BLOCK" "control: recursive-force rm" 2 +row "$CONTROL_ALLOW" "control: find on artifact" 0 + +echo "### STEP 2 -- NEGATIVE CONTROL: revert hook to pre-fix (HEAD~1)" +git checkout HEAD~1 -- hooks/safety-gate.sh +echo " sha: $(shasum hooks/safety-gate.sh | cut -c1-12) <- must differ from step 1" +row "$INCIDENT" "incident: find -delete" 0 +row "$CONTROL_BLOCK" "control: recursive-force rm" 2 +row "$CONTROL_ALLOW" "control: find on artifact" 0 + +echo "### STEP 3 -- RESTORE (HEAD)" +git checkout HEAD -- hooks/safety-gate.sh +echo " sha: $(shasum hooks/safety-gate.sh | cut -c1-12) <- must match step 1" +row "$INCIDENT" "incident: find -delete" 2 +row "$CONTROL_BLOCK" "control: recursive-force rm" 2 +row "$CONTROL_ALLOW" "control: find on artifact" 0 + +echo "### tree clean?" +git status --porcelain hooks/safety-gate.sh +echo "(clean if nothing above)" diff --git a/docs/verification/find-delete-gate.md b/docs/verification/find-delete-gate.md new file mode 100644 index 00000000..800b111c --- /dev/null +++ b/docs/verification/find-delete-gate.md @@ -0,0 +1,110 @@ +# Proof of done: safety-gate find delete-verb gap + +Verdict: PASS + +Origin: a live incident, not a hypothetical. On 2026-07-08 a disk-cleanup batch ran +`find -mindepth 1 -delete` against `~/.cache/.bun`, a path that reads +like a cache but was the bun global install root. It wiped the `omp` (oh-my-pi) cockpit. +The gate did not fire: it keys each rule on its segment's binary, and that segment's +binary was `find`, not `rm`. Discovered 2026-07-17 when the binary turned up missing. + +## Acceptance criteria -> confirmation + +| AC | Criterion | How proven | Result | +|----|-----------|------------|--------| +| AC1 | the exact incident command blocks | probe: `find ~/.cache/.bun -mindepth 1 -delete` -> exit 2 | PASS | +| AC2 | `-exec rm` is caught too, not just `-delete` | probe D2 -> exit 2 | PASS | +| AC3 | regenerable artifacts stay deletable (no false positive) | probe D3 `find node_modules ... -delete` -> exit 0 | PASS | +| AC4 | non-delete find is untouched | probes D4 (`-exec gofmt`), D5 (read-only find) -> exit 0 | PASS | +| AC5 | the rm rule is unaffected by the refactor | control `rm -r -f src` -> exit 2 across all 3 control steps | PASS | +| AC6 | fail-closed on a find with no path operand | `targets_all_safe` with zero operands returns non-zero -> block | PASS | +| AC7 | no regression in the suite | `tests/test-hooks.sh` 471/471 | PASS | + +## Implementation + +- `hooks/safety-gate.sh` -- new `find)` arm: sets a delete flag on `-delete`, or on + `-exec`/`-execdir`/`-ok`/`-okdir` paired with an `rm` token; collects find's leading + path operands (the tokens before the first primary) and blocks unless every one is + allowlisted. +- `hooks/safety-gate.sh` -- extracted the build-artifact allowlist out of the `rm` arm + into `targets_all_safe()`, so `rm` and `find` share ONE definition of "regenerable". + The rm arm collapses to a single guarded call; its behavior is unchanged (AC5). +- `tests/test-hooks.sh` -- D1-D5 pinned as permanent regression tests. + +## Confirmation run-table + +| Command | Exit | Result | +|---------|------|--------| +| `bash tests/test-hooks.sh` (before fix) | 1 | 469/471 -- D1 + D2 FAIL (the gap, reproduced) | +| `bash tests/test-hooks.sh` (after fix) | 0 | 471/471 -- all green, no regression | +| `bash -n hooks/safety-gate.sh` | 0 | syntax OK | +| negative control (revert -> RED -> restore) | 0 | incident flips 2 -> 0 -> 2; see below | + +## Run detail + +Red baseline, before the fix existed (only the two new pins fail, nothing else moves): + +``` + FAIL D1: find -delete on a home path blocks (expected exit 2, got 0) + FAIL D2: find -exec rm blocks (expected exit 2, got 0) + PASS D3: find -delete on an allowlisted artifact is allowed (exit 0) + PASS D4: find -exec without rm is allowed (exit 0) + PASS D5: read-only find is allowed (exit 0) +Passed: 469 / 471 +``` + +Green, after: + +``` + PASS D1: find -delete on a home path blocks (exit 2) + PASS D2: find -exec rm blocks (exit 2) + PASS D3: find -delete on an allowlisted artifact is allowed (exit 0) + PASS D4: find -exec without rm is allowed (exit 0) + PASS D5: read-only find is allowed (exit 0) +Passed: 471 / 471 +``` + +Negative control. The hook file is reverted to its pre-fix blob and the SAME probes +re-run, so the RED is produced by the absence of the fix, not by a changed harness. +The sha is printed each step to prove the file actually changed (a first attempt used +`git stash` on an already-clean file, silently did NOT revert, and produced a false +PASS; the sha line is what caught it and is why it stays in the script): + +``` +### STEP 1 -- fix in place (HEAD) + sha: 09e9fc716a81 + incident: find -delete exit=2 (want 2) + control: recursive-force rm exit=2 (want 2) + control: find on artifact exit=0 (want 0) +### STEP 2 -- NEGATIVE CONTROL: revert hook to pre-fix (HEAD~1) + sha: a60f39d4c23d <- must differ from step 1 + incident: find -delete exit=0 (want 0) <-- RED: the gap is real + control: recursive-force rm exit=2 (want 2) + control: find on artifact exit=0 (want 0) +### STEP 3 -- RESTORE (HEAD) + sha: 09e9fc716a81 <- must match step 1 + incident: find -delete exit=2 (want 2) + control: recursive-force rm exit=2 (want 2) + control: find on artifact exit=0 (want 0) +### tree clean? +(clean) +``` + +## Reproduce + +```bash +cd ~/workspace/tieubao/dwarves-kit +bash tests/test-hooks.sh # 471/471 + +# negative control: pin the payloads in a file, not on the command line, or the +# live gate fires on the harness testing it. +bash docs/verification/find-delete-gate-negctl.sh +``` + +## Known residual (not closed by this change) + +The allowlist still treats a bare relative `.cache` as regenerable, so `rm -rf .cache` +is permitted. Run from `$HOME` that is the same class of mistake this fix addresses, +reached by a different path. Left as-is deliberately: the entry exists for project-local +`.cache` build dirs, and narrowing it is a separate judgment call with its own false-positive +cost. Flagged for the operator rather than silently changed. diff --git a/hooks/safety-gate.sh b/hooks/safety-gate.sh index 25be7294..1e33b8fe 100755 --- a/hooks/safety-gate.sh +++ b/hooks/safety-gate.sh @@ -71,6 +71,29 @@ strip_quotes() { printf '%s' "$1" | sed -E "s/\"([^\"]*)\"/\\1/g; s/'([^']*)'/\\1/g" } +# Build-artifact allowlist: regenerable dirs only; any other target blocks. Shared +# by every delete verb (rm, find), so the set of "safe to wipe" paths is defined +# once. Fail-closed: no path operand at all is not safe. +targets_all_safe() { + local t have=0 + for t in "$@"; do + case "$t" in + -*) ;; + *..*) return 1 ;; # parent traversal is never safe (C-1) + node_modules|node_modules/|node_modules/*|./node_modules|./node_modules/|./node_modules/*|\ + dist|dist/|dist/*|./dist|./dist/|./dist/*|\ + build|build/|build/*|./build|./build/|./build/*|\ + .next|.next/|.next/*|./.next|./.next/|./.next/*|\ + .nuxt|.nuxt/|.nuxt/*|.turbo|.turbo/|.turbo/*|.cache|.cache/|.cache/*|\ + target|target/|target/*|./target|./target/|./target/*|\ + coverage|coverage/|coverage/*|out|out/|out/*) + have=1 ;; + *) return 1 ;; + esac + done + [ "$have" = 1 ] +} + while IFS= read -r SEG; do [ -n "${SEG// /}" ] || continue # shellcheck disable=SC2086 @@ -101,27 +124,31 @@ while IFS= read -r SEG; do esac done if [ "$HAS_R" = 1 ] && [ "$HAS_F" = 1 ]; then - # Build-artifact allowlist: regenerable dirs only; any other target blocks. + targets_all_safe "$@" || \ + block "rm-rf" "Destructive delete detected. Use 'trash' or 'mv' to a temp directory instead of rm -rf (build artifacts like node_modules/dist are allowlisted)." + fi + ;; + find) + # find carries its own delete verbs. The rm rule never sees them: this + # segment's binary is find, not rm. 2026-07-08: an unguarded + # `find ~/.cache/.bun -mindepth 1 -delete` wiped a bun global install. + HAS_DEL=0; HAS_EXEC=0 + for t in "$@"; do + case "$t" in + -delete) HAS_DEL=1 ;; + -exec|-execdir|-ok|-okdir) HAS_EXEC=1 ;; + rm|/bin/rm|/usr/bin/rm) if [ "$HAS_EXEC" = 1 ]; then HAS_DEL=1; fi ;; + esac + done + if [ "$HAS_DEL" = 1 ]; then + # find's path operands are the leading tokens, before the first primary. + PATHS="" for t in "$@"; do - case "$t" in - -*) ;; - *..*) ALL_SAFE=0; break ;; # parent traversal is never safe (C-1) - node_modules|node_modules/|node_modules/*|./node_modules|./node_modules/|./node_modules/*|\ - dist|dist/|dist/*|./dist|./dist/|./dist/*|\ - build|build/|build/*|./build|./build/|./build/*|\ - .next|.next/|.next/*|./.next|./.next/|./.next/*|\ - .nuxt|.nuxt/|.nuxt/*|.turbo|.turbo/|.turbo/*|.cache|.cache/|.cache/*|\ - target|target/|target/*|./target|./target/|./target/*|\ - coverage|coverage/|coverage/*|out|out/|out/*) - HAVE_TARGET=1 ;; - *) ALL_SAFE=0; break ;; - esac + case "$t" in -*) break ;; *) PATHS="$PATHS $t" ;; esac done - if [ "$ALL_SAFE" = 1 ] && [ "$HAVE_TARGET" = 1 ]; then - : # safe regenerable delete - else - block "rm-rf" "Destructive delete detected. Use 'trash' or 'mv' to a temp directory instead of rm -rf (build artifacts like node_modules/dist are allowlisted)." - fi + # shellcheck disable=SC2086 + targets_all_safe $PATHS || \ + block "find-delete" "Destructive delete detected (find -delete / -exec rm). Use 'trash' or 'mv' to a temp directory instead (build artifacts like node_modules/dist are allowlisted)." fi ;; git) diff --git a/tests/test-hooks.sh b/tests/test-hooks.sh index a39f3854..b5437f30 100755 --- a/tests/test-hooks.sh +++ b/tests/test-hooks.sh @@ -119,6 +119,20 @@ RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"eval \"rm -rf src\""}}') assert_exit "F3b: eval smuggle blocks" 2 $RC RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"xargs rm -rf < list.txt"}}') assert_exit "F3c: xargs rm blocks" 2 $RC + +# find is a delete verb the rm rule never sees. Permanent pin for the 2026-07-08 +# incident: `find ~/.cache/.bun -mindepth 1 -delete` passed the gate and wiped a +# bun global install (the omp cockpit), because the case dispatched on rm only. +RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"find ~/.cache/.bun -mindepth 1 -delete"}}') +assert_exit "D1: find -delete on a home path blocks" 2 $RC +RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"find /tmp/x -name \"*.log\" -exec rm -rf {} +"}}') +assert_exit "D2: find -exec rm blocks" 2 $RC +RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"find node_modules -mindepth 1 -delete"}}') +assert_exit "D3: find -delete on an allowlisted artifact is allowed" 0 $RC +RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"find . -name \"*.go\" -exec gofmt -w {} +"}}') +assert_exit "D4: find -exec without rm is allowed" 0 $RC +RC=$(run_hook safety-gate.sh '{"tool_input":{"command":"find ~/workspace -name \"*.md\" -type f"}}') +assert_exit "D5: read-only find is allowed" 0 $RC # F4: cd-prefix repo resolution parses portably (probe affordance prints the target) CDOUT=$(echo '{"tool_input":{"command":"cd /tmp/some-repo && git push -q origin feat/x"}}' | DWARVES_KIT_PRINT_CDDIR=1 bash "$KIT_DIR/hooks/ship-gate.sh" 2>/dev/null) assert_output_contains "F4: ship-gate resolves the cd target" "^/tmp/some-repo$" "$CDOUT"