diff --git a/CHANGELOG.md b/CHANGELOG.md index d5bb6dc..c3d8866 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -374,6 +374,31 @@ which records not just what changed but what each drill run proved. and *above* the first `incus` call, so the refusal costs no daemon and `test/cli.sh` drives it for real rather than grepping for it. +- **`drill/wipe.sh` no longer carries #102's SIGPIPE shape, and the pin now + sweeps the class** (#107) — the file piped `ufw status` straight into + `grep -q "Status: active"`. `Status: active` is ufw's FIRST line, so the + reader matches, closes the pipe, ufw dies of SIGPIPE, and the pipeline + yields 141. This was **correct today and only by accident**: `wipe.sh` is + `set -u` with no `pipefail`, so the 141 was discarded and grep's 0 carried + the branch. It was also one line from wrong — adding `set -o pipefail` for + unrelated robustness, the kind of tweak that reads as an obvious + improvement, would have silently skipped every UFW removal on a host the + operator was told is wiped, with no error and no red X (measured on a shim: + 5/5 runs took the wrong branch under `pipefail`, 3/3 the right one + without). Now it captures `ufw_status` once and matches with `[[ ]]`, and + the numbered-delete loop — whose condition was itself an early-exit reader, + plus an un-captured re-read to get the number — reads one capture per + iteration and breaks on absence. That is #106's pattern transplanted + verbatim from `host/teardown-host.sh`, so both files now read alike. + The `test/cli.sh` pin is **generalized from the single site to the class**: + it sweeps every `host/*.sh` and `drill/*.sh` for the racing shape and names + the offenders, so a new script in either directory inherits the pin instead + of being one more site to remember. Comment lines are stripped before + matching — each fix's own commentary quotes the racing shape to explain it, + and a prose-blind pin would fail on the very comment documenting why it + exists. (`drill/doctor.sh` was checked and needs nothing: it already reads + into `ufw_out` and is safe by construction, not by absent `pipefail`.) + ## 0.8.0 — 2026-07-19 ### Added diff --git a/drill/wipe.sh b/drill/wipe.sh index a6c4ed0..b17092b 100644 --- a/drill/wipe.sh +++ b/drill/wipe.sh @@ -117,10 +117,39 @@ for t in "inet box" "bridge box" "inet claudebox" "bridge claudebox"; do # shellcheck disable=SC2086 # the table spec is two words by design sudo nft delete table $t >/dev/null 2>&1 && say "deleted nft table $t" done -if command -v ufw >/dev/null && sudo ufw status 2>/dev/null | grep -q "Status: active"; then +# Every ufw read is CAPTURED before it is matched, never piped into a reader +# that exits on its first match (#102, #107). +# +# `ufw status | grep -q "Status: active"` returns the WRITER's exit: grep +# matches on the first line ufw prints, closes the pipe, ufw takes SIGPIPE, +# and the pipeline yields 141. This file is 'set -u' with no pipefail, so +# that 141 is discarded, grep's 0 is the pipeline's result, and the branch +# held — the defect was latent here, never live. It was also one line from +# live: adding 'set -o pipefail' for unrelated robustness would silently turn +# this into #102, skipping every UFW removal on a host the operator was told +# is wiped, with no error and no red X to see. Captured and matched with +# [[ ]], it is correct under any future 'set' line. +# +# The numbered loop had the same defect for a different reason: its condition +# was also an early-exit reader, so it could end while rules remained, and it +# re-read un-captured to get the number. It now reads one capture per +# iteration and breaks on absence — the re-scan is still per-delete (numbers +# shift after each removal), just no longer racing. +ufw_status="" +if command -v ufw >/dev/null; then + # '|| true': ufw exits non-zero when it cannot read its config, and "no + # usable ufw here" is nothing-to-clean, not a failure to report. + ufw_status="$(sudo ufw status 2>/dev/null || true)" +fi + +if [[ "$ufw_status" == *"Status: active"* ]]; then for net in boxnet claudenet; do - while sudo ufw status numbered | grep -q "on $net"; do - n="$(sudo ufw status numbered | grep -m1 "on $net" | sed -E 's/^\[ *([0-9]+)\].*/\1/')" + while :; do + numbered="$(sudo ufw status numbered 2>/dev/null || true)" + line="$(printf '%s\n' "$numbered" | grep -m1 "on $net" || true)" + [ -n "$line" ] || break + n="$(printf '%s\n' "$line" | sed -E 's/^\[ *([0-9]+)\].*/\1/')" + [ -n "$n" ] || break sudo ufw --force delete "$n" >/dev/null && say "deleted UFW rule on $net" done done diff --git a/test/cli.sh b/test/cli.sh index 7983e43..fd641a7 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -2006,26 +2006,42 @@ check "teardown-host: points at box uninstall when done" 0 "" \ check "teardown-host: refuses without a TTY and names the override (#113)" 2 \ "--yes (or BOX_YES=1) means yes" env -u BOX_YES bash "$ROOT/host/teardown-host.sh"