The #43 read-guard sweep does not cover bin/rig, which is how #68 slipped through #75

New issue

Closed

opened 2026-07-19 23:49:59 +00:00 by dan-claude-bot · 0 comments

dan-claude-bot commented

2026-07-19 23:49:59 +00:00

(Migrated from github.com)

Non-blocking suggestion from claude-bot-andresmgsl's review of #72, filed so it does not rot in a PR thread.

The gap

test/cli.sh:705 is the sweep #43 added to keep unguarded read prompts out of the tree. It matches read -rsp[^|]*$ and scans commands/ only.

#68 was a read -r prompt (not -rsp), in bin/rig (not commands/). It missed the net on both axes — which is why the bug survived until a drill hit it, and why the audit clearing the remaining sites currently lives in #72's PR body rather than in a check.

Why it matters

The class is nastier than it looks: under set -euo pipefail, an unguarded read called as a plain statement kills the shell at EOF before the case that would have printed the abort. The failure is silent — the operator sees nothing, and the exit code (1) is identical to a normal refusal, so an exit-code assertion passes against the bug. #68 is the second instance of this class in the repo (#43 was the first).

Suggested check

Extend the sweep to plain-statement prompt reads across both trees:

grep -RE '^[[:space:]]*read -r[a-z]* [A-Za-z_]+ *$' bin/rig commands/   # expect exit 1

The trailing $ is what makes it usable: it matches bare prompt reads while naturally excluding while read heads (which are errexit-exempt) and <<< here-string reads (which cannot fail). That is exactly the distinction #72's audit had to make by hand.

Current state

Clean — #72 audited every remaining site and no bare prompt read exists in bin/rig or commands/ today:

the five -rsp token prompts are || { echo; die … } (#43)
commands/db.sh:152 is || reply=""
bin/rig:168/:314 and the commands/users-*.sh reads are while conditions
commands/lib/users-config.sh:50/:78 read from here-strings

So this is a guard against reintroduction, not a live defect — the same "catch the state, not the instance" shape as heavy-duty/box#112's eof_guard_sweep.

Refs

#72 (where it was suggested), #43 (the original sweep), #68 (the site that escaped it), heavy-duty/box#112 (the sibling class-check precedent)

Non-blocking suggestion from `claude-bot-andresmgsl`'s review of #72, filed so it does not rot in a PR thread. ## The gap `test/cli.sh:705` is the sweep #43 added to keep unguarded `read` prompts out of the tree. It matches `read -rsp[^|]*$` and scans **`commands/` only**. #68 was a `read -r` prompt (not `-rsp`), in **`bin/rig`** (not `commands/`). It missed the net on both axes — which is why the bug survived until a drill hit it, and why the audit clearing the remaining sites currently lives in #72's PR body rather than in a check. ## Why it matters The class is nastier than it looks: under `set -euo pipefail`, an unguarded `read` called as a plain statement kills the shell at EOF *before* the `case` that would have printed the abort. The failure is silent — the operator sees nothing, and the exit code (1) is identical to a normal refusal, so an exit-code assertion passes against the bug. #68 is the second instance of this class in the repo (#43 was the first). ## Suggested check Extend the sweep to plain-statement prompt reads across both trees: ```bash grep -RE '^[[:space:]]*read -r[a-z]* [A-Za-z_]+ *$' bin/rig commands/ # expect exit 1 ``` The trailing `$` is what makes it usable: it matches bare prompt reads while naturally excluding `while read` heads (which are errexit-exempt) and `<<<` here-string reads (which cannot fail). That is exactly the distinction #72's audit had to make by hand. ## Current state Clean — #72 audited every remaining site and no bare prompt `read` exists in `bin/rig` or `commands/` today: - the five `-rsp` token prompts are `|| { echo; die … }` (#43) - `commands/db.sh:152` is `|| reply=""` - `bin/rig:168`/`:314` and the `commands/users-*.sh` reads are `while` conditions - `commands/lib/users-config.sh:50`/`:78` read from here-strings So this is a guard against reintroduction, not a live defect — the same "catch the state, not the instance" shape as heavy-duty/box#112's `eof_guard_sweep`. ## Refs #72 (where it was suggested), #43 (the original sweep), #68 (the site that escaped it), heavy-duty/box#112 (the sibling class-check precedent)