fix: drill/wipe.sh reads ufw into a capture, not into an early-exit reader

wipe.sh piped `ufw status` straight into `grep -q "Status: active"`. That is
closes the pipe, ufw takes SIGPIPE, and the pipeline yields 141.

It was correct today, and only by accident — the file is `set -u` with no
`pipefail`, so the 141 was discarded and grep's 0 carried the branch. It was
one line from wrong: adding `set -o pipefail` for unrelated robustness 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.

Transplant #106's pattern verbatim from host/teardown-host.sh: capture
ufw_status once and match with `[[ ]]`; rewrite the numbered-delete loop —
whose condition was itself an early-exit reader, plus an un-captured re-read
to get the number — as a `while :` that reads one capture per iteration and
breaks on absence. The re-scan stays per-delete, since numbers shift after
each removal; it just no longer races. Removals keep the file's
`cmd && say "did X"` idiom.

Generalize the test/cli.sh pin from the one site to the class: sweep every
host/*.sh and drill/*.sh for the racing shape and name 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 comment documenting why it exists. The positive pins (the
capture, the break-on-absence) now run per file over both, so the sweep
cannot be satisfied by deleting a block instead of fixing it.

drill/doctor.sh was checked and needs nothing: it already reads into
`ufw_out` and is safe by construction, not by absent pipefail.

Refs #107
This commit is contained in:
dan-claude-bot 2026-07-19 23:37:03 +00:00
parent 239428bf88
commit aec22277cd
3 changed files with 90 additions and 20 deletions

View file

@ -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

View file

@ -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

View file

@ -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" </dev/null
# #102's race, in the one other file that sets pipefail. A daemon-free run
# cannot exercise a UFW teardown, so the shape is pinned instead: no `ufw
# status` may be piped into an early-exit reader here, because under this
# file's pipefail the reader's match closes the pipe, ufw takes SIGPIPE, and
# the branch silently reads false — skipping crumb removal on a host the
# operator was told is clean. Both directions: the racing shape absent, the
# capture present.
# Comment lines are stripped before matching: the fix's own commentary quotes
# #102's race, pinned as a CLASS rather than at the one site that had it
# (#107). A daemon-free run cannot exercise a UFW teardown, so the shape is
# pinned instead: nowhere under host/ or drill/ may `ufw status` be piped into
# a reader that exits on its first match. `Status: active` is ufw's FIRST line,
# so the reader matches, closes the pipe, ufw takes SIGPIPE, and the pipeline
# yields 141 — under pipefail the branch silently reads false and the whole
# firewall block is skipped on a host the operator was told is clean.
#
# Swept, not per-file, because absence of pipefail is what made drill/wipe.sh
# survive the same shape: a file is only ever one `set -o pipefail` — the kind
# of robustness tweak that sails through review — from being #102 again. The
# sweep closes the class, so a new host/ or drill/ script inherits the pin for
# free instead of being one more site someone has to remember.
# Comment lines are stripped before matching: each fix's own commentary quotes
# the racing shape to explain it, and a pin that cannot tell prose from code
# would fail on the very comment documenting why it exists.
# shellcheck disable=SC2016 # "$1" is the subshell's positional, passed below
check "teardown-host: no 'ufw status' piped into an early-exit reader" 0 "" \
bash -c 'grep -vE "^[[:space:]]*#" "$1" | grep -qE "ufw status[^|]*\| *grep" && exit 1; exit 0' \
_ "$ROOT/host/teardown-host.sh"
# shellcheck disable=SC2016 # the $-strings are literals in the target file
check "teardown-host: the UFW branch reads a captured snapshot" 0 "" \
grep -qF 'if [[ "$ufw_status" == *"Status: active"* ]]; then' "$ROOT/host/teardown-host.sh"
# shellcheck disable=SC2016 # ditto
check "teardown-host: the numbered-delete loop breaks on absence, not on a pipe" 0 "" \
grep -qF '[ -n "$line" ] || break' "$ROOT/host/teardown-host.sh"
check "no 'ufw status' is piped into an early-exit reader under host/ or drill/" 0 "" \
bash -c 'bad=""
for f in "$1"/host/*.sh "$1"/drill/*.sh; do
grep -vE "^[[:space:]]*#" "$f" | grep -qE "ufw status[^|]*\| *grep" && bad="$bad ${f#"$1"/}"
done
[ -z "$bad" ] || { printf "racing ufw reads in:%s\n" "$bad"; exit 1; }' \
_ "$ROOT"
# The other direction, per file that removes UFW rules: the capture present and
# the delete loop breaking on absence, so the sweep above cannot be satisfied by
# deleting the block instead of fixing it.
for f in host/teardown-host.sh drill/wipe.sh; do
# shellcheck disable=SC2016 # the $-strings are literals in the target files
check "$f: the UFW branch reads a captured snapshot" 0 "" \
grep -qF 'if [[ "$ufw_status" == *"Status: active"* ]]; then' "$ROOT/$f"
# shellcheck disable=SC2016 # ditto
check "$f: the numbered-delete loop breaks on absence, not on a pipe" 0 "" \
grep -qF '[ -n "$line" ] || break' "$ROOT/$f"
done
check "drill: reads the installed tree through current/" 0 "" \
grep -qF '.local/share/box/current/VERSION' "$ROOT/drill/drill.sh"