drill/wipe.sh carries the #102 SIGPIPE shape — safe only because it lacks pipefail #107
Labels
No labels
blocked
blocker:ci-red
blocker:conflict
blocker:drill-pending
blocker:unrequested
bug
claimed
documentation
enhancement
epic
merge-next
needs-triage
ready
release
scope:cli
scope:drill
scope:host
scope:installer
scope:templates
scope:tiers
stale
state:addressing
state:bots-reviewing
state:building
state:needs-human
No milestone
No project
No assignees
1 participant
Notifications
Due date
No due date set.
Dependencies
No dependencies set.
Reference: heavy-duty/box#107
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
drill/wipe.shcarries the #102 SIGPIPE shape — safe only because it lackspipefailBackground
#102 (fixed in #106) was a production firewall bug with a subtle cause:
Status: activeis the first line ufw prints.grep -qmatches it and exitsimmediately, closing the pipe while ufw is still writing the table, so ufw dies of
SIGPIPE.
grepreturns 0 — but underset -o pipefailthe pipeline returns141. Any
ifbuilt on it reads false, and a host with UFW plainly active istreated as having no UFW. It is a branch condition, so
errexitnever fires: noerror, 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.shandhost/teardown-host.sh.What is left
drill/wipe.sh:120still has the identical shape:…plus the same early-exit reader as a loop condition at
:122, and a secondun-captured read at
:123.It is correct today.
drill/wipe.sh:27isset -u— nopipefail— so theSIGPIPE 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 pipefailto 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.shalongside this as a latentsite. That was wrong:
drill/doctor.sh:285already readsufw_out="$(sudo ufw status 2>/dev/null)"and matches the captured text. It issafe by construction, not by the absence of
pipefail, and needs nothing.drill/wipe.shis the only remaining site.The decision
with
[[ ]], and read one capture per iteration in the delete loop. Removesthe hazard by construction and makes the file safe under any future
setline. Cost: churn in a file that has no bug today.
safe without
pipefail, so the next person addingpipefailis warned atthe point of change. Cost: relies on someone reading it.
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.shgrep pin asserting noufw statusis pipedinto an early-exit reader anywhere under
host/ordrill/, with comment linesstripped 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.