confirm() swallows Ctrl-D: an interactive abort exits 1 in silence, never reaching 'aborted.' #111

New issue

Closed

opened 2026-07-19 20:25:58 +00:00 by dan-claude-bot · 0 comments

dan-claude-bot commented

2026-07-19 20:25:58 +00:00

(Migrated from github.com)

Surfaced by claude-bot-andresmgsl during review of #109, as an explicitly non-blocking note. Filing it so it is not lost with the PR.

The defect

confirm() at bin/box:826 reads the operator's answer unguarded:

confirm() {   # $1 = prompt. --force, or a TTY to ask on, or we refuse.
  if [ "$force" -eq 1 ]; then return 0; fi
  [ -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
  case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac
}

Ctrl-D at a real interactive prompt makes read return non-zero. Under set -euo pipefail the run ends silently with exit 1 — the case is never reached, so die "aborted." never fires. The operator sees the question, presses Ctrl-D, and gets nothing: no confirmation of the abort, no message, no explanation of the exit code.

uninstall_confirm() at bin/box:1654 has the identical shape.

This is rig#43, again

heavy-duty/rig#43 already diagnosed and cured exactly this pattern with read ... || die. box has the same gap in the same construct. Worth pointing at that issue for the reasoning rather than re-deriving it here.

Severity: low, and honest about why

It fails closed — nothing is destroyed, the abort is real. The damage is purely that the tool goes silent at the exact moment it asked the operator a question, which is the worst moment to say nothing. An operator who Ctrl-Ds out of a box rm prompt cannot tell from the output whether the box survived.

It also predates #109 — rm has always had it, and box rm has carried a confirm prompt for as long as the verb has existed. This is not a regression.

Why file it now rather than let it ride

#109 takes the number of verbs reaching that line from one to two (rm, and now restore). The exposure doubled, and both verbs are irreversible. That is the argument for closing it now rather than the next time someone notices.

It was deliberately kept out of #109 — a different defect with a different blast radius, and folding an unrelated pre-existing fix into a destructive-gate PR would have muddied a diff that reviewers were checking line by line.

The fix

One token in two places:

bin/box:826 — read -r reply || die "aborted."
bin/box:1654 — same, in uninstall_confirm()

Mirror rig#43's cure so the two repos read alike.

Testing note, stated honestly

The interactive accept (y) and abort (n) paths are currently untested, structurally — they need a pty, which the fake-incus harness does not provide. test/cli.sh covers the --force bypass and the no-TTY refusal (both driven, both green) but cannot reach the branch this issue is about.

So the EOF path is testable only through a script-based check. That is worth building — it would close the whole family at once rather than this one line — but it is a bigger lift than the fix, and the fix should not wait for it. If the pty harness is out of scope, say so in the PR rather than claiming coverage that is not there.

Refs

#109 (where it was found; the review comment has the full analysis) · #105 (the gate that doubled the exposure) · heavy-duty/rig#43 (the same defect, already cured there)

Surfaced by `claude-bot-andresmgsl` during review of #109, as an explicitly non-blocking note. Filing it so it is not lost with the PR. ## The defect `confirm()` at `bin/box:826` reads the operator's answer unguarded: ```bash confirm() { # $1 = prompt. --force, or a TTY to ask on, or we refuse. if [ "$force" -eq 1 ]; then return 0; fi [ -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 case "$reply" in y|Y|yes|YES|Yes) return 0 ;; *) die "aborted." ;; esac } ``` Ctrl-D at a real interactive prompt makes `read` return non-zero. Under `set -euo pipefail` the run ends **silently** with exit 1 — the `case` is never reached, so `die "aborted."` never fires. The operator sees the question, presses Ctrl-D, and gets nothing: no confirmation of the abort, no message, no explanation of the exit code. `uninstall_confirm()` at `bin/box:1654` has the identical shape. ## This is rig#43, again `heavy-duty/rig#43` already diagnosed and cured exactly this pattern with `read ... || die`. box has the same gap in the same construct. Worth pointing at that issue for the reasoning rather than re-deriving it here. ## Severity: low, and honest about why It **fails closed** — nothing is destroyed, the abort is real. The damage is purely that the tool goes silent at the exact moment it asked the operator a question, which is the worst moment to say nothing. An operator who Ctrl-Ds out of a `box rm` prompt cannot tell from the output whether the box survived. It also **predates #109** — `rm` has always had it, and `box rm` has carried a confirm prompt for as long as the verb has existed. This is not a regression. ## Why file it now rather than let it ride #109 takes the number of verbs reaching that line from **one to two** (`rm`, and now `restore`). The exposure doubled, and both verbs are irreversible. That is the argument for closing it now rather than the next time someone notices. It was deliberately kept out of #109 — a different defect with a different blast radius, and folding an unrelated pre-existing fix into a destructive-gate PR would have muddied a diff that reviewers were checking line by line. ## The fix One token in two places: - `bin/box:826` — `read -r reply || die "aborted."` - `bin/box:1654` — same, in `uninstall_confirm()` Mirror rig#43's cure so the two repos read alike. ## Testing note, stated honestly The interactive accept (`y`) and abort (`n`) paths are **currently untested, structurally** — they need a pty, which the fake-incus harness does not provide. `test/cli.sh` covers the `--force` bypass and the no-TTY refusal (both driven, both green) but cannot reach the branch this issue is about. So the EOF path is testable only through a `script`-based check. That is worth building — it would close the whole family at once rather than this one line — but it is a bigger lift than the fix, and the fix should not wait for it. If the pty harness is out of scope, say so in the PR rather than claiming coverage that is not there. ## Refs #109 (where it was found; the review comment has the full analysis) · #105 (the gate that doubled the exposure) · `heavy-duty/rig#43` (the same defect, already cured there)

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/box#111

No description provided.