From 7306c166934af3217d583620ca2ee6de4386d027 Mon Sep 17 00:00:00 2001 From: Han Ngo Date: Fri, 17 Jul 2026 20:41:06 +0700 Subject: [PATCH 1/2] fix(safety-gate): close the find delete-verb gap The gate keys each rule on its segment's binary and had arms for rm, git and kubectl but none for find, so every delete expressed through find passed unchallenged. On 2026-07-08 a cleanup batch ran `find -mindepth 1 -delete` against a path that merely looked like a cache and wiped a bun global install (the omp cockpit). The rm rule never saw it: that segment's binary was find, not rm. Adds a find arm covering -delete and -exec/-execdir/-ok/-okdir with rm, and extracts the build-artifact allowlist into targets_all_safe() so rm and find share one definition of "regenerable". Fail-closed: a find with no path operand blocks. Pinned as D1-D5 in tests/test-hooks.sh. Red before: 469/471 with D1+D2 failing. Green after: 471/471. --- hooks/safety-gate.sh | 65 +++++++++++++++++++++++++++++++------------- tests/test-hooks.sh | 14 ++++++++++ 2 files changed, 60 insertions(+), 19 deletions(-) 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" From 22d4c525c411aa8d999a888b160838eb31c42c90 Mon Sep 17 00:00:00 2001 From: Han Ngo Date: Fri, 17 Jul 2026 20:43:35 +0700 Subject: [PATCH 2/2] docs(verification): proof of done for the find delete-verb gap Red baseline 469/471 (D1+D2 fail), green 471/471, plus a negative control that reverts the hook to its pre-fix blob and shows the incident command flip 2 -> 0 -> 2 while the rm control stays blocked throughout. Ships the control as a runnable script: its payloads live in the file rather than on the command line, because the live gate otherwise fires on the harness testing it. The per-step sha print is load-bearing, a first attempt stashed an already-clean file, silently did not revert, and produced a false PASS. --- docs/verification/find-delete-gate-negctl.sh | 42 +++++++ docs/verification/find-delete-gate.md | 110 +++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100755 docs/verification/find-delete-gate-negctl.sh create mode 100644 docs/verification/find-delete-gate.md 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.