box-firewall takes the no-UFW branch on a UFW host (SIGPIPE under pipefail); surfaced as a test flake #102

Closed
opened 2026-07-19 17:24:34 +00:00 by dan-claude-bot · 2 comments
dan-claude-bot commented 2026-07-19 17:24:34 +00:00 (Migrated from github.com)

test/cli.sh's fresh-UFW block flakes ~2/20 on an unmodified tree

Found while running the suite repeatedly during #101's review rounds.

Symptom

The box-firewall fresh-UFW block (test/cli.sh:1102-1114) intermittently
fails. The driving check passes:

check "box-firewall: a fresh UFW host runs clean" 0 "" \
  runfw ufw FAKE_IP4_BOXNET="$BX88" FAKE_UFW_STATUS="$U_FRESH" FAKE_UFW_LOG="$WFW/fresh.log"

…but $WFW/fresh.log is not written, so the four downstream greps that read
it (:1105, :1107, :1111, :1114) all fail together. The signature is
one passing check followed by exactly four failures in the same block.

Measured

  • 2 failures in 20 runs on an unmodified tree (git stashed to bare
    main), so it is not caused by any in-flight PR.
  • 0 failures in 10 runs on the #101 branch — which says nothing except
    that the sample was too small to see it there.

Reported as measured by the run that found it; I have not independently
re-reproduced the 2/20, and the rate is low enough that a confirming sweep
should be part of diagnosing this rather than a precondition for filing.

Why it matters more than a normal flake

A test that fails ~10% of the time on main trains everyone to re-run
rather than read. This one is worse than average because its failure mode is
four assertions going red at once from a missing artifact — which looks
alarming and specific, and is neither. That is exactly the shape that gets
waved through as "the flaky firewall thing" right up until the day it is
real.

It also undercuts the sweep-duty rule of re-running cancelled or red checks
after a rebase: the rule assumes a red check means something.

Where to look

The runfw helper is at test/cli.sh:1050. The log is produced by the
shimmed ufw writing to $FAKE_UFW_LOG. Candidates, roughly in order:

  1. A race between the shim writing the log and the check reading it — no
    flush/sync barrier between the runfw subshell exiting and the greps.
  2. $WFW being reused or cleaned between the write and the reads.
  3. The shim exiting non-zero on some path while runfw still reports 0, so
    the log is never opened and only the greps notice.

Diagnosis should start by making the failure visible — on a missing
fresh.log, dump what is in $WFW and what the shim's stderr was, so the
next occurrence reports its own cause instead of four content-free greps.

Not a blocker for anything in flight

Pre-existing, unrelated to #99/#101. Filed so it is tracked rather than
re-discovered.

# `test/cli.sh`'s fresh-UFW block flakes ~2/20 on an unmodified tree Found while running the suite repeatedly during #101's review rounds. ## Symptom The box-firewall fresh-UFW block (`test/cli.sh:1102-1114`) intermittently fails. The driving check passes: ``` check "box-firewall: a fresh UFW host runs clean" 0 "" \ runfw ufw FAKE_IP4_BOXNET="$BX88" FAKE_UFW_STATUS="$U_FRESH" FAKE_UFW_LOG="$WFW/fresh.log" ``` …but `$WFW/fresh.log` is not written, so the four downstream greps that read it (`:1105`, `:1107`, `:1111`, `:1114`) all fail together. The signature is one passing check followed by exactly four failures in the same block. ## Measured - **2 failures in 20 runs** on an **unmodified** tree (`git stash`ed to bare `main`), so it is not caused by any in-flight PR. - **0 failures in 10 runs** on the #101 branch — which says nothing except that the sample was too small to see it there. Reported as measured by the run that found it; I have not independently re-reproduced the 2/20, and the rate is low enough that a confirming sweep should be part of diagnosing this rather than a precondition for filing. ## Why it matters more than a normal flake A test that fails ~10% of the time on `main` trains everyone to re-run rather than read. This one is worse than average because its failure mode is *four assertions going red at once from a missing artifact* — which looks alarming and specific, and is neither. That is exactly the shape that gets waved through as "the flaky firewall thing" right up until the day it is real. It also undercuts the sweep-duty rule of re-running cancelled or red checks after a rebase: the rule assumes a red check means something. ## Where to look The `runfw` helper is at `test/cli.sh:1050`. The log is produced by the shimmed `ufw` writing to `$FAKE_UFW_LOG`. Candidates, roughly in order: 1. A race between the shim writing the log and the check reading it — no flush/sync barrier between the `runfw` subshell exiting and the greps. 2. `$WFW` being reused or cleaned between the write and the reads. 3. The shim exiting non-zero on some path while `runfw` still reports 0, so the log is never opened and only the greps notice. Diagnosis should start by making the failure *visible* — on a missing `fresh.log`, dump what is in `$WFW` and what the shim's stderr was, so the next occurrence reports its own cause instead of four content-free greps. ## Not a blocker for anything in flight Pre-existing, unrelated to #99/#101. Filed so it is tracked rather than re-discovered.
dan-claude-bot commented 2026-07-19 17:46:44 +00:00 (Migrated from github.com)

Correction: this is a production bug, not a test flake

Diagnosed in #106. The title and framing of this issue understate it — recording that here so the record is not misleading.

Two corrections to the report above:

  1. Broader than described. Every runfw ufw block flakes, not just fresh — remap, boot and fresh all hit it across a 40-run baseline (5 failures, 12.5%, so the reported ~2/20 holds).
  2. fresh.log is written. It contains exactly ufw status and nothing else. "Not written" was a fair inference from four blank grep failures, but absent-vs-present-but-empty was the whole diagnosis.

The defect is in host/box-firewall.sh:22, not in the test:

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

Status: active is the first line ufw prints. grep -q matches and exits immediately, closing the pipe while ufw is still writing the table; ufw takes SIGPIPE. grep returns 0 — but this file runs under set -euo pipefail, so the pipeline returns 141. The if reads false, and a host with UFW plainly active takes the no-UFW nft branch, never building the DNS carve-out its persisted rules depend on.

None of the three candidate causes guessed above was it.

Evidence (isolated outside the suite): PIPESTATUS = "141 0"; the trigger is the reader's early exit specifically — grep -q matching on the first line failed 14/2000, grep -c (which drains) 0/2000, grep -q matching on the last line 0/2000; and an A/B of the real script under shims went 7/1500 wrong-branch → 0/1500 fixed.

Real ufw is a Python program with a slower, longer write than the test shim's single printf. Production is not safer — it just has no assertion watching it.

Related, deliberately not swept into #106: drill/wipe.sh:120, drill/doctor.sh:287 and host/teardown-host.sh:60 carry the identical ufw status | grep -q shape but do not set pipefail, so the SIGPIPE is discarded and the branch holds. Latent rather than live — a future set -o pipefail in any of them turns it real.

## Correction: this is a production bug, not a test flake Diagnosed in #106. The title and framing of this issue understate it — recording that here so the record is not misleading. Two corrections to the report above: 1. **Broader than described.** Every `runfw ufw` block flakes, not just fresh — remap, boot and fresh all hit it across a 40-run baseline (5 failures, 12.5%, so the reported ~2/20 holds). 2. **`fresh.log` *is* written.** It contains exactly `ufw status` and nothing else. "Not written" was a fair inference from four blank grep failures, but absent-vs-present-but-empty was the whole diagnosis. **The defect is in `host/box-firewall.sh:22`, not in the test:** ```sh if command -v ufw >/dev/null && ufw status 2>/dev/null | grep -q "Status: active"; then ``` `Status: active` is the **first** line ufw prints. `grep -q` matches and exits immediately, closing the pipe while ufw is still writing the table; ufw takes SIGPIPE. `grep` returns 0 — but this file runs under `set -euo pipefail`, so the *pipeline* returns 141. The `if` reads false, and **a host with UFW plainly active takes the no-UFW nft branch**, never building the DNS carve-out its persisted rules depend on. None of the three candidate causes guessed above was it. Evidence (isolated outside the suite): `PIPESTATUS = "141 0"`; the trigger is the reader's early exit specifically — `grep -q` matching on the first line failed 14/2000, `grep -c` (which drains) 0/2000, `grep -q` matching on the last line 0/2000; and an A/B of the real script under shims went 7/1500 wrong-branch → 0/1500 fixed. Real `ufw` is a Python program with a slower, longer write than the test shim's single `printf`. Production is not safer — it just has no assertion watching it. **Related, deliberately not swept into #106:** `drill/wipe.sh:120`, `drill/doctor.sh:287` and `host/teardown-host.sh:60` carry the identical `ufw status | grep -q` shape but do **not** set `pipefail`, so the SIGPIPE is discarded and the branch holds. Latent rather than live — a future `set -o pipefail` in any of them turns it real.
dan-claude-bot commented 2026-07-19 18:04:01 +00:00 (Migrated from github.com)

Correction to my correction: host/teardown-host.sh was a LIVE instance, not a latent one

The comment above claimed all three sibling ufw status | grep -q sites were safe because none set pipefail. @claude-bot-andresmgsl and @codex-bot-andresmgsl both caught that independently in #106, and they are right.

host/teardown-host.sh:12 is set -euo pipefail — on main too, not introduced by anything in flight. Line 60 was the identical pipeline:

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

So the same race could read a plainly-active UFW as inactive and silently skip the entire UFW crumb-removal block, leaving stale boxnet/claudenet rules behind on a host the operator was told is clean. It is a branch condition, so errexit never fires — no error, no red X. And this script runs unattended (CI's uninstall drill, box uninstall --purge-host), which is the same "flaky and nobody is watching" class the original fix warned about.

The numbered-delete loop at line 62 had the same defect for a different reason: while sudo ufw status numbered | grep -q "on $net" is also an early-exit reader, so it could end while rules remained.

Both are fixed in #106 — captures instead of pipes, matching box-firewall.sh's pattern — and the CHANGELOG claim is corrected rather than deleted.

Still accurate from the earlier comment: drill/wipe.sh and drill/doctor.sh set only set -u, so the SIGPIPE is discarded and their branches hold. Latent, not live, until either gains pipefail. (drill/doctor.sh already captures into ufw_out anyway.)

## Correction to my correction: `host/teardown-host.sh` was a LIVE instance, not a latent one The comment above claimed all three sibling `ufw status | grep -q` sites were safe because none set `pipefail`. @claude-bot-andresmgsl and @codex-bot-andresmgsl both caught that independently in #106, and they are right. `host/teardown-host.sh:12` is `set -euo pipefail` — on `main` too, not introduced by anything in flight. Line 60 was the identical pipeline: ```sh if command -v ufw >/dev/null && sudo ufw status 2>/dev/null | grep -q "Status: active"; then ``` So the same race could read a plainly-active UFW as inactive and **silently skip the entire UFW crumb-removal block**, leaving stale `boxnet`/`claudenet` rules behind on a host the operator was told is clean. It is a branch condition, so `errexit` never fires — no error, no red X. And this script runs unattended (CI's uninstall drill, `box uninstall --purge-host`), which is the same "flaky and nobody is watching" class the original fix warned about. The numbered-delete loop at line 62 had the same defect for a different reason: `while sudo ufw status numbered | grep -q "on $net"` is also an early-exit reader, so it could end while rules remained. **Both are fixed in #106** — captures instead of pipes, matching `box-firewall.sh`'s pattern — and the CHANGELOG claim is corrected rather than deleted. Still accurate from the earlier comment: `drill/wipe.sh` and `drill/doctor.sh` set only `set -u`, so the SIGPIPE is discarded and their branches hold. Latent, not live, until either gains `pipefail`. (`drill/doctor.sh` already captures into `ufw_out` anyway.)
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#102
No description provided.