fix: Ctrl-D at a confirm prompt aborts out loud, not in silence (#111) #112

Merged
dan-claude-bot merged 2 commits from fix/confirm-eof-abort into main 2026-07-19 21:27:57 +00:00
5 changed files with 144 additions and 4 deletions

View file

@ -79,6 +79,48 @@ 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 misnumbered one, and an operator sent to correct a version number that is
already right will not find the real problem. Matches already right will not find the real problem. Matches
heavy-duty/rig#67, so the three repos agree. 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. 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 - **`box restore` asks before it destroys — and the confirmation prompt is
now the row's, not rm's** (#105) — `restore` and `rm` both irreversibly 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` discard user state, and only one of them asked. The table gave `restore`

View file

@ -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)" [ -t 0 ] || usage_error "refusing to $1 without --force (no terminal to confirm on)"
local reply local reply
printf 'box: %s? this cannot be undone. [y/N] ' "$1" 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 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)" [ -t 0 ] || usage_error "refusing to $1 without --force (no terminal to confirm on; BOX_YES=1 also means yes)"
local reply local reply
printf 'box: %s? [y/N] ' "$1" 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 case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac
} }

View file

@ -47,7 +47,11 @@ if [ "$purge" -eq 1 ]; then
if [ -z "${BOX_YES:-}" ]; then if [ -z "${BOX_YES:-}" ]; then
if [ -t 0 ]; 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" 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 case "$reply" in y|Y|yes|YES|Yes) : ;; *) echo "box revoke: aborted." >&2; exit 1 ;; esac
else else
echo "box revoke: refusing to --purge without a terminal to confirm on. BOX_YES=1 means yes." >&2 echo "box revoke: refusing to --purge without a terminal to confirm on. BOX_YES=1 means yes." >&2

View file

@ -28,7 +28,9 @@ $purge && echo "Incus itself will also be uninstalled (--purge-incus)."
if [ "$yes" -eq 1 ]; then if [ "$yes" -eq 1 ]; then
echo "(confirmed non-interactively: --yes/BOX_YES)" echo "(confirmed non-interactively: --yes/BOX_YES)"
else 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 case "$a" in y|Y) ;; *) echo "aborted"; exit 1 ;; esac
fi fi

View file

@ -890,6 +890,95 @@ 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. # Pinned here because the rehearsal itself needs a daemon and this suite has none.
check "rehearsal: the unattended restore passes --force (#105)" 0 "" \ check "rehearsal: the unattended restore passes --force (#105)" 0 "" \
grep -qF 'box restore mine s1 --force' "$ROOT/drill/multiuser.sh" 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" <<RUNNER
#!/usr/bin/env bash
exec env PATH="$CSHIM:\$PATH" "$BOX" rm work
RUNNER
chmod +x "$PWORK/run"
ptybox() { # ptybox <answers-file> — '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
# --- 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" rm -rf "$CSHIM" "$CWORK"
# --------------------------------------------------------------------------- # ---------------------------------------------------------------------------