box restore destroys without asking #105

New issue

Closed

opened 2026-07-19 17:35:39 +00:00 by dan-claude-bot · 0 comments

dan-claude-bot commented

2026-07-19 17:35:39 +00:00

(Migrated from github.com)

`box restore` destroys without asking

Child of heavy-duty/rig#62, but it is a live bug independent of that work.

The bug

restore is a thin passthrough row (bin/box:67):

"restore^<box> <snapshot>^box,arg2^Roll a box back to one of its snapshots^incus:snapshot restore^restored {} to {1}"

Preconditions are box,arg2 — the instance must be ours, and a snapshot
name must be present. That is all. Compare rm on the very next lines
(bin/box:73), which carries confirm, wired at bin/box:1896:

case ",$pre," in *,confirm,*) confirm "delete $inst and all its snapshots" ;;

Both verbs irreversibly destroy user state. Only one asks. The help text
does warn — "Anything in the box since that snapshot is lost"
(bin/box:356-361) — but a warning in --help is not a gate, and restore
is about to become a routine operation rather than a rare one if the
pristine snapshot in heavy-duty/rig#62 lands.

Fix

Add confirm to the precondition field. Given the table-driven design this
is a one-token change, and confirm() (bin/box:767-775) already handles
--force, the TTY check, and refusing rather than assuming consent without
a terminal.

Also worth deciding: the `stopped` precondition

restore has no stopped guard either, so it hands a running instance
straight to incus. Snapshots here are stateless — cmd_snapshot
(bin/box:1295-1299) passes no --stateful, and nothing in the tree
mentions migration.stateful — so restoring a running box is
crash-consistent at best: dirty page cache, live docker containers,
half-written files. export already takes the stricter line via
require_stopped (bin/box:1314, "#70 settled on require-down over
snapshot-then-export").

Requiring stopped is probably right, but it is a UX cost on the common case
and worth its own call rather than being smuggled in with the confirm.

Non-issue, stated so nobody adds it

box restore does not need reset_identity (bin/box:846-864). That
exists because a clone shares /etc/machine-id with its source and
therefore its DHCP DUID and lease. A restore is the same instance keeping
its own identity — correct as-is, and adding a reset would cost a pointless
reboot.

# `box restore` destroys without asking Child of heavy-duty/rig#62, but it is a live bug independent of that work. ## The bug `restore` is a thin passthrough row (`bin/box:67`): ``` "restore^<box> <snapshot>^box,arg2^Roll a box back to one of its snapshots^incus:snapshot restore^restored {} to {1}" ``` Preconditions are `box,arg2` — the instance must be ours, and a snapshot name must be present. That is all. Compare `rm` on the very next lines (`bin/box:73`), which carries `confirm`, wired at `bin/box:1896`: ```bash case ",$pre," in *,confirm,*) confirm "delete $inst and all its snapshots" ;; ``` Both verbs irreversibly destroy user state. Only one asks. The help text does warn — "Anything in the box since that snapshot is lost" (`bin/box:356-361`) — but a warning in `--help` is not a gate, and restore is about to become a *routine* operation rather than a rare one if the pristine snapshot in heavy-duty/rig#62 lands. ## Fix Add `confirm` to the precondition field. Given the table-driven design this is a one-token change, and `confirm()` (`bin/box:767-775`) already handles `--force`, the TTY check, and refusing rather than assuming consent without a terminal. ## Also worth deciding: the `stopped` precondition `restore` has no `stopped` guard either, so it hands a running instance straight to incus. Snapshots here are **stateless** — `cmd_snapshot` (`bin/box:1295-1299`) passes no `--stateful`, and nothing in the tree mentions `migration.stateful` — so restoring a running box is crash-consistent at best: dirty page cache, live docker containers, half-written files. `export` already takes the stricter line via `require_stopped` (`bin/box:1314`, "#70 settled on require-down over snapshot-then-export"). Requiring stopped is probably right, but it is a UX cost on the common case and worth its own call rather than being smuggled in with the confirm. ## Non-issue, stated so nobody adds it `box restore` does **not** need `reset_identity` (`bin/box:846-864`). That exists because a *clone* shares `/etc/machine-id` with its source and therefore its DHCP DUID and lease. A restore is the same instance keeping its own identity — correct as-is, and adding a reset would cost a pointless reboot.

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#105

No description provided.

box restore destroys without asking #105

box restore destroys without asking

The bug

Fix

Also worth deciding: the stopped precondition

Non-issue, stated so nobody adds it

`box restore` destroys without asking

Also worth deciding: the `stopped` precondition