Merge pull request #127 from dan-claude-bot/test/racing-reader-sweep-class

fix: the racing-reader sweep guards the class, and revoke-user captures the trust store

CHANGELOG.md

@@ -468,6 +468,33 @@ which records not just what changed but what each drill run proved.
   exists. (`drill/doctor.sh` was checked and needs nothing: it already reads
   into `ufw_out` and is safe by construction, not by absent `pipefail`.)
 
+- **The racing-reader sweep guards the class, not one spelling — and now
+  names a second writer** (#124) — the sweep #107 added matched
+  `ufw status[^|]*\| *grep`, which caught every historical instance and none
+  of the equivalent spellings. `head -n1`, `sed -n '1p;q'`,
+  `awk '/x/ {print; exit}'` and `read` all close the pipe early, SIGPIPE the
+  writer, and yield the identical 141 under `pipefail`; the pin guarded the
+  instance spelling of the very thing it existed to generalise. Both halves
+  of the matcher are alternations now. The reader half is deliberately not
+  narrowed to the early-exit spellings — separating `grep -q` from `grep -c`
+  by regex is precision that rots, and every `ufw status` site in the tree
+  already captures first, so banning the pipe outright costs nothing.
+  The writer half gains `incus config trust list`, because
+  **`host/revoke-user.sh:206` piped it into `grep -q` as the `--purge`
+  leftover assert** — under `set -euo pipefail`, so unlike `drill/wipe.sh`
+  nothing but the writer's size was holding it, and sitting left of `&&` a
+  141 is `set -e`-exempt too. It would have read as "no leftover cert" on a
+  host that still trusts the revoked user's certificate and reported the
+  purge complete: fail-open, on the path whose whole job is to prove access
+  is gone. Now captured into `trust_csv` and matched with `[[ ]]` (leading
+  newline so the first CSV row still anchors like the `^` it replaces —
+  verified equivalent to the old `grep` across nine anchoring cases). Writers
+  are enumerated rather than generalised: ~150 legitimate `| grep` sites
+  exist under `host/` and `drill/`, nearly all re-reading an already-captured
+  string, so the sweep claims only what it can check and grows one named
+  writer at a time. The `id -nG | tr | grep -qx` shapes in grant/revoke/setup
+  are deliberately untouched — single tiny writes, not realistically racy.
+
 ## 0.8.0 — 2026-07-19
 
 ### Added

host/revoke-user.sh

@@ -200,10 +200,23 @@ fi
 # and a promise the header makes is a promise this block checks. The
 # incus-user state directory too — it was purged for releases without being
 # re-checked, which is exactly the gap this block exists to close.
+# The trust store is read into a capture rather than piped into a reader that
+# stops at its first match — #102's shape, and this file is `set -euo pipefail`
+# already, so unlike drill/wipe.sh (#107) nothing but the writer's size is
+# holding it. A reader that exits early SIGPIPEs incus mid-table and the
+# pipeline yields 141; sitting left of `&&` that is also set -e-exempt, so it
+# would read as "no leftover cert" on a host that still trusts the revoked
+# user's certificate and the purge would report success. Fail-open, on the
+# cleanup path whose entire job is to prove access is gone.
+# Un-racy in practice today — the trust store is small and likely one write —
+# so this is defensive, not a live defect. Captured so it cannot become one.
+trust_csv="$(incus config trust list --format csv --columns nf 2>/dev/null || true)"
+
 leftover=""
 incus project show "$project" >/dev/null 2>&1 </dev/null && leftover="$leftover $project"
 incus network show "$bridge" >/dev/null 2>&1 </dev/null && leftover="$leftover $bridge"
-incus config trust list --format csv --columns nf 2>/dev/null | grep -q "^incus-user-$uid," \
+# Leading newline so the first CSV row anchors like the `^` this replaces.
+[[ $'\n'"$trust_csv" == *$'\n'"incus-user-$uid,"* ]] \
   && leftover="$leftover cert:incus-user-$uid"
 $SUDO test -d "/var/lib/incus/users/$uid" 2>/dev/null \
   && leftover="$leftover /var/lib/incus/users/$uid"

test/cli.sh

@@ -2333,9 +2333,9 @@ check "teardown-host: refuses without a TTY and names the override (#113)" 2 \
 
 # #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
+# pinned instead: nowhere under host/ or drill/ may a known multi-line writer
-# a reader that exits on its first match. `Status: active` is ufw's FIRST line,
+# be piped into a line reader. `Status: active` is ufw's FIRST line, so the
-# so the reader matches, closes the pipe, ufw takes SIGPIPE, and the pipeline
+# 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.
 #
@@ -2347,13 +2347,36 @@ check "teardown-host: refuses without a TTY and names the override (#113)" 2 \
 # 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.
+#
+# BOTH halves of the matcher are alternations, and both were widened in #124:
+#
+#   · READERS. Pinning `| grep` guarded the instance spelling, not the class.
+#     `head -n1`, `sed -n '1p;q'` and `awk '/x/ {print; exit}'` all close the
+#     pipe early and produce the identical wrong answer under pipefail. The
+#     alternation is deliberately NOT restricted to the early-exit spellings
+#     (`grep -q` but not `grep -c`, `sed …q` but not `sed s///`): telling
+#     those apart by regex is exactly the kind of precision that rots, and
+#     the house idiom is to capture first anyway — all six `ufw status` sites
+#     in the tree already do. Banning the pipe outright costs nothing real
+#     and cannot be defeated by a spelling nobody enumerated.
+#
+#   · WRITERS. Enumerated, not generalised. `incus config trust list` joins
+#     `ufw status` because host/revoke-user.sh used it as a leftover-detection
+#     condition under `set -euo pipefail` (#124). A generic "no multi-line
+#     writer feeds a reader" matcher is unwritable here: ~150 legitimate
+#     `| grep` sites exist across host/ and drill/, nearly all reading an
+#     already-captured string back out of `printf '%s\n' "$var"`. So the
+#     sweep claims exactly what it can check — THESE writers are never piped
+#     — and grows one named writer at a time.
 # shellcheck disable=SC2016  # "$1" is the subshell's positional, passed below
-check "no 'ufw status' is piped into an early-exit reader under host/ or drill/" 0 "" \
+check "no multi-line writer is piped into a line 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"/}"
+      grep -vE "^[[:space:]]*#" "$f" \
+        | grep -qE "(ufw status|incus config trust list)[^|]*\| *(grep|head|sed|awk|read)" \
+        && bad="$bad ${f#"$1"/}"
     done
-    [ -z "$bad" ] || { printf "racing ufw reads in:%s\n" "$bad"; exit 1; }' \
+    [ -z "$bad" ] || { printf "racing reads in:%s\n" "$bad"; exit 1; }' \
     _ "$ROOT"
 
 # The other direction, per file that removes UFW rules: the capture present and
@@ -2367,6 +2390,18 @@ for f in host/teardown-host.sh drill/wipe.sh; do
   check "$f: the numbered-delete loop breaks on absence, not on a pipe" 0 "" \
     grep -qF '[ -n "$line" ] || break' "$ROOT/$f"
 done
+
+# Same other-direction pin for the non-ufw writer the sweep now names: the
+# --purge leftover assert must match a captured trust store, so the sweep
+# cannot be satisfied by deleting the assert instead of fixing it. That assert
+# is the last thing standing between "purge INCOMPLETE" and a silent claim of
+# success on a host that still trusts the revoked user's certificate.
+# shellcheck disable=SC2016  # the $-strings are literals in the target file
+check "revoke-user: the purge leftover assert reads a captured trust store" 0 "" \
+  grep -qF 'trust_csv="$(incus config trust list' "$ROOT/host/revoke-user.sh"
+# shellcheck disable=SC2016  # ditto
+check "revoke-user: the cert leftover check matches the capture, not a pipe" 0 "" \
+  grep -qF '"$trust_csv" == *$' "$ROOT/host/revoke-user.sh"
 check "drill: reads the installed tree through current/" 0 "" \
   grep -qF '.local/share/box/current/VERSION' "$ROOT/drill/drill.sh"