From e27ab239f16fc5e5dbe5c4f2bc28a5e4478a4b8b Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sun, 19 Jul 2026 20:33:06 +0000 Subject: [PATCH 1/2] fix: Ctrl-D at a confirm prompt aborts out loud, not in silence (#111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit confirm() and uninstall_confirm() read the operator's answer with a bare 'read -r reply'. Ctrl-D makes read return non-zero, set -euo pipefail ends the run on that line, and the case below — the only thing that ever says "aborted." — is never reached. box exits 1 having printed nothing after the question it just asked. The cure is one token in each, 'read -r reply || die "aborted."', the same one heavy-duty/rig#43 applied to rig's credential prompts. The three answers a human can give (y, n, Ctrl-D) are now driven on a real pty via util-linux 'script'. They were structurally untested before — '[ -t 0 ]' sends a terminal-less suite to the refusal branch, so every existing check stopped there, which is how this survived four releases. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 25 +++++++++++++++++++++++++ bin/box | 7 +++++-- test/cli.sh | 48 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed4ee00..5edaa9c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,31 @@ which records not just what changed but what each drill run proved. misnumbered one, and an operator sent to correct a version number that is already right will not find the real problem. Matches heavy-duty/rig#67, so the three repos agree. +- **Ctrl-D at a confirmation prompt aborts out loud, instead of exiting + in silence** (#111) — `confirm()` and `uninstall_confirm()` both took + the operator's answer with a bare `read -r reply`. Every answer a + human can type routes through the `case` below it and ends at a + `return` or at `die "aborted."` — every answer except EOF. Ctrl-D + makes `read` return non-zero, `set -euo pipefail` ends the run on that + line, and the `case` is never reached: box exits 1 having printed + nothing at all after the question it just asked. It fails closed, + which is why this is a small fix and not an incident — nothing is + destroyed, the abort is real. The damage is that the tool goes mute at + the one moment it had the operator's full attention, and someone who + Ctrl-Ds out of `box rm work` cannot tell from the output whether the + box is still there. The cure is one token in each function, + `read -r reply || die "aborted."`, the same one heavy-duty/rig#43 + applied to rig's credential prompts so the two repos read alike. The + bug predates everything it touches — `rm` has carried a confirm gate + for as long as the verb has existed — but #105 took the number of + verbs reaching that line from one to two, and both are irreversible, + which is the argument for closing it now rather than the next time + someone notices. The three answers a human can actually give (`y`, + `n`, and Ctrl-D) are now driven for real on a pty via util-linux + `script`: they were structurally untested before, because `[ -t 0 ]` + sends a terminal-less suite to the refusal branch and every existing + check stopped there — which is exactly how this survived four + releases. - **`box restore` asks before it destroys — and the confirmation prompt is now the row's, not rm's** (#105) — `restore` and `rm` both irreversibly discard user state, and only one of them asked. The table gave `restore` diff --git a/bin/box b/bin/box index c1ba897..344dd65 100755 --- a/bin/box +++ b/bin/box @@ -823,7 +823,9 @@ confirm() { # $1 = prompt. --force, or a TTY to ask on, or we refuse. [ -t 0 ] || usage_error "refusing to $1 without --force (no terminal to confirm on)" local reply printf 'box: %s? this cannot be undone. [y/N] ' "$1" - read -r reply + # EOF (Ctrl-D) is an answer, and it means no. Unguarded, 'read' returns + # non-zero and 'set -e' ends the run in silence — heavy-duty/rig#43. + read -r reply || die "aborted." case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac } @@ -1651,7 +1653,8 @@ uninstall_confirm() { # $1 = question. --force, or BOX_YES=1, or a TTY. [ -t 0 ] || usage_error "refusing to $1 without --force (no terminal to confirm on; BOX_YES=1 also means yes)" local reply printf 'box: %s? [y/N] ' "$1" - read -r reply + # Same EOF cure as confirm() above — Ctrl-D must abort out loud. + read -r reply || die "aborted." case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac } diff --git a/test/cli.sh b/test/cli.sh index bfd8ad6..d6da980 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -890,6 +890,54 @@ check "dispatch: the confirm prompt comes from the row, not a constant (#105)" 0 # Pinned here because the rehearsal itself needs a daemon and this suite has none. check "rehearsal: the unattended restore passes --force (#105)" 0 "" \ grep -qF 'box restore mine s1 --force' "$ROOT/drill/multiuser.sh" +# --- the three answers a human can give — DRIVEN ON A REAL PTY (#111) ------- +# Everything above stops at the no-TTY refusal, because confirm() branches on +# '[ -t 0 ]' and this suite has no terminal. So the interactive half — 'y', +# 'n', and Ctrl-D — had never been executed here at all, which is precisely +# how #111 survived: an unguarded 'read' returns non-zero on EOF, 'set -e' +# ends the run before the 'case', and the abort happens in total silence. +# +# 'script' from util-linux gives the child a pty, so box takes the interactive +# branch for real and reads the answer we write to the master side. This does +# NOT hang a suite run from a terminal: script's own stdin is a file or +# /dev/null on every run below, never the developer's tty, so the answer (or +# the EOF) is always already waiting. +if command -v script >/dev/null 2>&1 && script --version 2>/dev/null | grep -q util-linux; then + PWORK="$(mktemp -d)"; PLOG="$PWORK/pty.log" + printf 'y\n' > "$PWORK/yes"; printf 'n\n' > "$PWORK/no" + # Invoked through a file so 'script -c' needs no quoting of its own; the log + # path and the shim PATH ride the environment script hands to the child. + cat > "$PWORK/run" < — 'box rm work' on a pty, answered + : > "$PLOG" + FAKE_INCUS_LOG="$PLOG" script -qec "$PWORK/run" /dev/null < "$1" + } + # The load-bearing assertion is the MESSAGE, not the exit code: before the + # fix Ctrl-D also exited 1, just without ever saying why. Asserting on the + # code alone would pass against the bug. + check "rm: Ctrl-D at the prompt aborts OUT LOUD, not in silence (#111)" \ + 1 "aborted." ptybox /dev/null + check "rm: ...and the Ctrl-D abort really deleted nothing (#111)" 1 "" \ + grep -qF 'incus delete' "$PLOG" + check "rm: 'n' at the prompt aborts (#111)" 1 "aborted." ptybox "$PWORK/no" + check "rm: ...and 'n' really deleted nothing (#111)" 1 "" \ + grep -qF 'incus delete' "$PLOG" + # The accept path, so the pty rig is proven to be able to reach the work — + # three checks that can only ever refuse would pass against a box that + # refuses everything. + check "rm: 'y' at the prompt goes through (#111)" 0 "removed work" \ + ptybox "$PWORK/yes" + check "rm: ...and 'y' really reached 'incus delete -f' (#111)" 0 "" \ + grep -qF 'incus delete -f work' "$PLOG" + rm -rf "$PWORK" +else + echo "skip: the interactive confirm answers (no util-linux 'script' here; CI has it)" +fi + rm -rf "$CSHIM" "$CWORK" # --------------------------------------------------------------------------- -- 2.45.2 From bb51aeeca9ed695b0056e1549e223f4a6c1705a8 Mon Sep 17 00:00:00 2001 From: dan-claude-bot Date: Sun, 19 Jul 2026 21:06:44 +0000 Subject: [PATCH 2/2] fix: close the silent-EOF class at the two host/ gates it also reached (#111) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first pass fixed #111 where it was reported — confirm() and uninstall_confirm() in bin/box — and stopped there, while the same defect sat at two more destructive gates in this repo: · host/revoke-user.sh:50 guards 'box revoke --purge', the prompt whose own text says "this cannot be undone"; · host/teardown-host.sh:31 guards a full host teardown. Both run under 'set -euo pipefail', so an unguarded 'read' returning non-zero on EOF ends the run before the 'case' that names the abort — exit 1, in silence, at the moment the script asked. Each now carries the guard in its own script's wording rather than importing bin/box's die(). The three drill/ prompts are deliberately untouched: they run under 'set -u' only, so EOF falls through to the '*)' arm and already aborts out loud. install.sh:65 was already guarded. What keeps the class closed is a repo-wide sweep in test/cli.sh: every statement-initial 'read' fed from stdin, in any file that turns on errexit, must carry a '||' guard — 'while read' loops and '<<<' herestrings excluded, since neither is a prompt. Removing any of the four guards makes it fail and name the file:line; it flags nothing else across the tree's fifteen shell files. The absence of exactly this check is why the host/ pair was missed the first time. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 19 ++++++++++++++++++- host/revoke-user.sh | 6 +++++- host/teardown-host.sh | 4 +++- test/cli.sh | 41 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5edaa9c..75f4e91 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,7 +103,24 @@ which records not just what changed but what each drill run proved. `script`: they were structurally untested before, because `[ -t 0 ]` sends a terminal-less suite to the refusal branch and every existing check stopped there — which is exactly how this survived four - releases. + releases. Review caught that the first pass fixed the bug where it was + reported and stopped there, while the same defect sat at two more + destructive gates in this repo: `host/revoke-user.sh:50`, the prompt + guarding `box revoke --purge` — the one whose own text says "this + cannot be undone" — and `host/teardown-host.sh:31`, guarding a full + host teardown. Both run under `set -euo pipefail`, both died mute on + EOF with their `aborted` line never reached; both now carry the guard + in their own script's wording. The three `drill/` prompts are + deliberately left alone — they run under `set -u` only, so EOF falls + through to the `*)` arm and already aborts out loud — and + `install.sh:65` was already guarded. What keeps the class closed is a + repo-wide sweep in `test/cli.sh`: every statement-initial `read` fed + from stdin, in any file that turns on errexit, must carry a `||` + guard, with `while read` loops and `<<<` herestrings excluded because + neither is a prompt. The sweep flags all four sites when their guards + are removed and nothing else across the tree's fifteen shell files — + the absence of exactly this check is why the `host/` pair was missed + in the first place. - **`box restore` asks before it destroys — and the confirmation prompt is now the row's, not rm's** (#105) — `restore` and `rm` both irreversibly discard user state, and only one of them asked. The table gave `restore` diff --git a/host/revoke-user.sh b/host/revoke-user.sh index 147a05f..9533899 100644 --- a/host/revoke-user.sh +++ b/host/revoke-user.sh @@ -47,7 +47,11 @@ if [ "$purge" -eq 1 ]; then if [ -z "${BOX_YES:-}" ]; then if [ -t 0 ]; then printf 'box revoke: delete ALL of %s'\''s boxes, images and their project %s? this cannot be undone. [y/N] ' "$user" "$project" - read -r reply + # EOF (Ctrl-D) is an answer too, and it is a refusal. Unguarded, 'read' + # returns non-zero and errexit ends the run right here — before the + # 'case' below can name the abort, so the most destructive prompt box + # has would go silent at the moment it asked (#111). + read -r reply || { echo "box revoke: aborted." >&2; exit 1; } case "$reply" in y|Y|yes|YES|Yes) : ;; *) echo "box revoke: aborted." >&2; exit 1 ;; esac else echo "box revoke: refusing to --purge without a terminal to confirm on. BOX_YES=1 means yes." >&2 diff --git a/host/teardown-host.sh b/host/teardown-host.sh index b957ed0..a3fdce0 100755 --- a/host/teardown-host.sh +++ b/host/teardown-host.sh @@ -28,7 +28,9 @@ $purge && echo "Incus itself will also be uninstalled (--purge-incus)." if [ "$yes" -eq 1 ]; then echo "(confirmed non-interactively: --yes/BOX_YES)" else - read -rp "Continue? [y/N] " a + # EOF (Ctrl-D) refuses, out loud: unguarded, errexit would end the run on + # this line and the 'aborted' below would never print (#111). + read -rp "Continue? [y/N] " a || { echo "aborted"; exit 1; } case "$a" in y|Y) ;; *) echo "aborted"; exit 1 ;; esac fi diff --git a/test/cli.sh b/test/cli.sh index d6da980..2ca1d2e 100644 --- a/test/cli.sh +++ b/test/cli.sh @@ -938,6 +938,47 @@ else echo "skip: the interactive confirm answers (no util-linux 'script' here; CI has it)" fi +# --- the sweep: no prompt-shaped 'read' under 'set -e' may go unguarded (#111) +# The pty checks above prove the two 'bin/box' gates. This proves the CLASS, +# repo-wide, and it exists because the class is exactly what the first pass at +# #111 missed: 'host/revoke-user.sh' and 'host/teardown-host.sh' carried the +# identical defect and survived, because nothing here was looking for the shape. +# +# The shape: a 'read' at the start of a statement, fed from the script's own +# stdin (so a human, or an EOF), inside a file that turns on errexit. On EOF +# 'read' returns non-zero and 'set -e' ends the run BEFORE the 'case' that was +# going to name the abort — the tool goes mute at the moment it asked. +# +# What is deliberately NOT flagged, because it is not the shape: +# · 'while IFS= read -r' loops — fed by a redirect at 'done', and a non-zero +# read is how the loop is supposed to end; +# · '<<<' herestring reads — fed from a string, never from a human; +# · files without errexit ('drill/wipe.sh', 'drill/drill.sh', +# 'drill/multiuser.sh' run under 'set -u' only, wipe.sh documents why), where +# EOF simply falls through to the '*)' arm and aborts out loud on its own. +# A guard is any '||' on the read's own line: '|| die', '|| reply=""', +# '|| { echo …; exit 1; }' — the spelling is each script's to choose, the +# guard is not. +eof_guard_sweep() { + local f n line bad=0 files + files="$(cd "$ROOT" && shopt -s globstar && printf '%s\n' bin/* ./**/*.sh | sed 's|^\./||' | sort -u)" + while IFS= read -r f; do + [ -f "$ROOT/$f" ] || continue + grep -qE '^[[:space:]]*set[[:space:]]+-[a-zA-Z]*e' "$ROOT/$f" || continue + while IFS=: read -r n line; do + case "$line" in + *'<<<'*) continue ;; # herestring, not a prompt + *'||'*) continue ;; # guarded — the whole point + esac + echo "$f:$n: prompt-shaped 'read' under 'set -e' with no '||' guard:$line" + bad=1 + done < <(grep -nE '^[[:space:]]*(IFS=[^[:space:]]+[[:space:]]+)?read([[:space:]]|$)' "$ROOT/$f") + done <<<"$files" + return "$bad" +} +check "no prompt-shaped 'read' under 'set -e' goes unguarded, repo-wide (#111)" \ + 0 "" eof_guard_sweep + rm -rf "$CSHIM" "$CWORK" # --------------------------------------------------------------------------- -- 2.45.2