fix: Ctrl-D at a confirm prompt aborts out loud, not in silence (#111) #112
5 changed files with 144 additions and 4 deletions
42
CHANGELOG.md
42
CHANGELOG.md
|
|
@ -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
|
||||
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. 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`
|
||||
|
|
|
|||
7
bin/box
7
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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
||||
|
|
|
|||
89
test/cli.sh
89
test/cli.sh
|
|
@ -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.
|
||||
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" <<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"
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
Loading…
Reference in a new issue