drill/wipe.sh carries the #102 SIGPIPE shape — safe only because it lacks pipefail #107

Closed
opened 2026-07-19 18:58:28 +00:00 by dan-claude-bot · 0 comments
dan-claude-bot commented 2026-07-19 18:58:28 +00:00 (Migrated from github.com)

drill/wipe.sh carries the #102 SIGPIPE shape — safe only because it lacks pipefail

Background

#102 (fixed in #106) was a production firewall bug with a subtle cause:

ufw status 2>/dev/null | grep -q "Status: active"

Status: active is the first line ufw prints. grep -q matches it and exits
immediately, closing the pipe while ufw is still writing the table, so ufw dies of
SIGPIPE. grep returns 0 — but under set -o pipefail the pipeline returns
141. Any if built on it reads false, and a host with UFW plainly active is
treated as having no UFW. It is a branch condition, so errexit never fires: no
error, no red X, just a silently wrong outcome at roughly 2% per invocation
(measured: 5 failures in 40 runs).

#106 fixed the two files where that shape was live under pipefail:
host/box-firewall.sh and host/teardown-host.sh.

What is left

drill/wipe.sh:120 still has the identical shape:

if command -v ufw >/dev/null && sudo ufw status 2>/dev/null | grep -q "Status: active"; then

…plus the same early-exit reader as a loop condition at :122, and a second
un-captured read at :123.

It is correct today. drill/wipe.sh:27 is set -u — no pipefail — so the
SIGPIPE is discarded, grep's 0 is the pipeline's result, and the branch holds.
There is no bug to observe right now.

It is one line away from being wrong. Adding set -o pipefail to that file —
a change anyone would reasonably make for unrelated robustness reasons, and which
would look like an obvious improvement in review — silently converts it into
#102. Nothing in the file says so.

Correction to the earlier scoping

An earlier comment on #102 listed drill/doctor.sh alongside this as a latent
site. That was wrong: drill/doctor.sh:285 already reads
ufw_out="$(sudo ufw status 2>/dev/null)" and matches the captured text. It is
safe by construction, not by the absence of pipefail, and needs nothing.

drill/wipe.sh is the only remaining site.

The decision

  1. Convert it now. Apply #106's pattern — read once into a variable, match
    with [[ ]], and read one capture per iteration in the delete loop. Removes
    the hazard by construction and makes the file safe under any future set
    line. Cost: churn in a file that has no bug today.
  2. Leave it and mark it. A comment at the shape explaining that it is only
    safe without pipefail, so the next person adding pipefail is warned at
    the point of change. Cost: relies on someone reading it.
  3. Leave it silently. Accept the tripwire.

Option 1 is cheap and this repo already has the pattern and the pins for it.
Option 3 seems clearly wrong — the whole reason #102 took a full diagnosis round
is that the failure mode is invisible.

Suggested pin

Whichever option: a test/cli.sh grep pin asserting no ufw status is piped
into an early-exit reader anywhere under host/ or drill/, with comment lines
stripped before matching. #106 established both halves of that discipline —
including the wrinkle that the fix's own explanatory comment quotes the racing
shape, so a prose-blind pin fails on the comment documenting why it exists.

# `drill/wipe.sh` carries the #102 SIGPIPE shape — safe only because it lacks `pipefail` ## Background #102 (fixed in #106) was a production firewall bug with a subtle cause: ```sh ufw status 2>/dev/null | grep -q "Status: active" ``` `Status: active` is the **first** line ufw prints. `grep -q` matches it and exits immediately, closing the pipe while ufw is still writing the table, so ufw dies of SIGPIPE. `grep` returns 0 — but under `set -o pipefail` the **pipeline** returns 141. Any `if` built on it reads **false**, and a host with UFW plainly active is treated as having no UFW. It is a branch condition, so `errexit` never fires: no error, no red X, just a silently wrong outcome at roughly 2% per invocation (measured: 5 failures in 40 runs). #106 fixed the two files where that shape was live under `pipefail`: `host/box-firewall.sh` and `host/teardown-host.sh`. ## What is left `drill/wipe.sh:120` still has the identical shape: ```sh if command -v ufw >/dev/null && sudo ufw status 2>/dev/null | grep -q "Status: active"; then ``` …plus the same early-exit reader as a loop condition at `:122`, and a second un-captured read at `:123`. **It is correct today.** `drill/wipe.sh:27` is `set -u` — no `pipefail` — so the SIGPIPE is discarded, `grep`'s 0 is the pipeline's result, and the branch holds. There is no bug to observe right now. **It is one line away from being wrong.** Adding `set -o pipefail` to that file — a change anyone would reasonably make for unrelated robustness reasons, and which would look like an obvious improvement in review — silently converts it into #102. Nothing in the file says so. ## Correction to the earlier scoping An earlier comment on #102 listed `drill/doctor.sh` alongside this as a latent site. That was wrong: `drill/doctor.sh:285` already reads `ufw_out="$(sudo ufw status 2>/dev/null)"` and matches the captured text. It is safe **by construction**, not by the absence of `pipefail`, and needs nothing. `drill/wipe.sh` is the only remaining site. ## The decision 1. **Convert it now.** Apply #106's pattern — read once into a variable, match with `[[ ]]`, and read one capture per iteration in the delete loop. Removes the hazard by construction and makes the file safe under any future `set` line. Cost: churn in a file that has no bug today. 2. **Leave it and mark it.** A comment at the shape explaining that it is only safe without `pipefail`, so the next person adding `pipefail` is warned at the point of change. Cost: relies on someone reading it. 3. **Leave it silently.** Accept the tripwire. Option 1 is cheap and this repo already has the pattern and the pins for it. Option 3 seems clearly wrong — the whole reason #102 took a full diagnosis round is that the failure mode is invisible. ## Suggested pin Whichever option: a `test/cli.sh` grep pin asserting no `ufw status` is piped into an early-exit reader anywhere under `host/` or `drill/`, with comment lines stripped before matching. #106 established both halves of that discipline — including the wrinkle that the fix's own explanatory comment quotes the racing shape, so a prose-blind pin fails on the comment documenting why it exists.
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/box#107
No description provided.