fix: version names die at one shared gate, and --purge-host hears --force
Round-1 convergence from all three reviewers, both findings real: A version string used to be a path fragment: 'box uninstall ../../../.ssh' resolved below versions/ and rm -rf'd wherever it landed, 'box use' could point current outside the root, and a hostile flat-tree VERSION could steer the migration's mv the same way. One strict validator now gates every caller — only [A-Za-z0-9._+-], no leading '.' or '-' — byte-identical in install.sh and bin/box like existing_boxes, diff-guarded in the tests, with traversal regressions on use, uninstall and the migration (which now refuses BEFORE the tree moves anywhere). --force is uninstall's installer-family consent, and --purge-host now forwards it: teardown-host.sh gets --yes under --force/BOX_YES, so the combined non-interactive uninstall no longer dies at teardown's own prompt. CI's drill now runs the combined verb with --force alone (no BOX_YES, no TTY) — the exact invocation that used to abort. Also grok's polish, taken: current flips by rename (ln to a side name, mv -Tf over — no window with no current) in both install.sh and 'box use'; BOX_REINSTALL swaps by two renames and deletes LAST; and the single-version path refuses while current is dangling (readlink -f resolves a missing last component, so the guard checks the DIRECTORY, not just the string). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
dan-claude-bot
parent
34bf39be86
commit
68a4996f0b
4 changed files with 122 additions and 12 deletions
7
.github/workflows/ci.yml
vendored
7
.github/workflows/ci.yml
vendored
|
|
@ -74,8 +74,11 @@ jobs:
|
|||
sudo test ! -e "/var/lib/incus/users/$uid"
|
||||
! sudo incus project show "user-$uid"
|
||||
! sudo incus config trust list --format csv | grep -q "incus-user-$uid"
|
||||
sudo BOX_YES=1 /usr/local/bin/box teardown-host
|
||||
sudo BOX_YES=1 /usr/local/bin/box uninstall --all
|
||||
# The COMBINED verb, --force only, deliberately no BOX_YES and no
|
||||
# TTY: this is the exact invocation that used to die at teardown's
|
||||
# own prompt when consent was not forwarded (--purge-host now
|
||||
# passes --yes through under --force/BOX_YES).
|
||||
sudo /usr/local/bin/box uninstall --all --purge-host --force
|
||||
# zero residue: the daemon's state...
|
||||
! sudo incus network show boxnet
|
||||
! sudo incus profile show box-net
|
||||
|
|
|
|||
40
bin/box
40
bin/box
|
|
@ -1112,6 +1112,21 @@ install_root() {
|
|||
dirname "$vdir"
|
||||
}
|
||||
|
||||
# A version is a DIRECTORY NAME under versions/ — nothing else. One strict
|
||||
# gate for every caller that builds a path from one (the installer's new_ver,
|
||||
# migration's flat_ver, and bin/box's 'use'/single-version uninstall): only
|
||||
# [A-Za-z0-9._+-], no leading '.' or '-'. That forbids '/', '..'-escapes,
|
||||
# spaces and option-lookalikes by construction — a crafted version dies HERE,
|
||||
# never in an rm -rf or an ln. install.sh carries a byte-identical copy;
|
||||
# test/cli.sh diffs the two so the gates cannot drift.
|
||||
valid_version() {
|
||||
case "$1" in
|
||||
''|.*|-*) return 1 ;;
|
||||
*[!A-Za-z0-9._+-]*) return 1 ;;
|
||||
esac
|
||||
return 0
|
||||
}
|
||||
|
||||
# Which boxes exist on this host, at THIS caller's tier? Prints their names
|
||||
# (both tag generations) and succeeds when at least one exists; fails when
|
||||
# none are visible — including when incus is absent or not answering, because
|
||||
|
|
@ -1188,9 +1203,12 @@ cmd_use() {
|
|||
local v="${args[0]:-}" ir eff expect out
|
||||
[ -n "$v" ] || usage_error "usage: $(synopsis_of use)"
|
||||
ir="$(install_root)" || die "this box runs from a working tree ($root), not a versioned install — nothing to switch"
|
||||
valid_version "$v" || die "not a sane version name: '$v' (a version is a directory name under versions/ — see 'box versions')"
|
||||
[ -d "$ir/versions/$v" ] || die "no such version: $v (see 'box versions')"
|
||||
die_under_boxes "switch the default box version" "box use $v"
|
||||
ln -sfn "versions/$v" "$ir/current"
|
||||
# An atomic flip, not unlink+create: ln -sfn leaves a window where current
|
||||
# is missing; a rename over it does not.
|
||||
ln -sfn "versions/$v" "$ir/current.new.$$" && mv -Tf "$ir/current.new.$$" "$ir/current"
|
||||
converge_bin_links "$ir"
|
||||
# Assert the EFFECTIVE result, not the intent: current must resolve to the
|
||||
# version asked for, and the chain's own binary must answer that version —
|
||||
|
|
@ -1246,8 +1264,15 @@ cmd_uninstall() {
|
|||
# -- one version -----------------------------------------------------------
|
||||
if [ -n "$ver" ] && [ "$all" -eq 0 ]; then
|
||||
[ "$purge_host" -eq 0 ] || usage_error "--purge-host goes with the full uninstall, not a single version"
|
||||
valid_version "$ver" || die "not a sane version name: '$ver' (a version is a directory name under versions/ — see 'box versions')"
|
||||
[ -d "$ir/versions/$ver" ] || die "no such version: $ver (see 'box versions')"
|
||||
cur="$(basename "$(readlink -f "$ir/current" 2>/dev/null || true)")"
|
||||
# A broken current makes the CURRENT guard below unfireable (cur empty
|
||||
# when the link is missing; cur naming a non-directory when it dangles —
|
||||
# readlink -f resolves a link whose last component does not exist). Heal
|
||||
# first, then decide; never delete around a broken default.
|
||||
{ [ -n "$cur" ] && [ -d "$ir/versions/$cur" ]; } \
|
||||
|| die "current is dangling — 'box use <version>' to repoint the default first (refusing to remove versions while it is broken)"
|
||||
[ "$ver" != "$cur" ] || die "$ver is the CURRENT version — 'box use <other>' first, or 'box uninstall --all' for everything"
|
||||
uninstall_confirm "remove box version $ver from $ir"
|
||||
# rm's exit code is not the verdict — the absence re-check below is (a
|
||||
|
|
@ -1268,8 +1293,17 @@ cmd_uninstall() {
|
|||
# 'box revoke <user> --purge' is the clean path (and asserts its absence).
|
||||
granted="$(timeout 10 incus project list --format csv 2>/dev/null </dev/null | cut -d, -f1 | grep '^user-' | tr '\n' ' ' || true)"
|
||||
[ -n "${granted% }" ] && echo "box: NOTE — granted users still have projects (${granted% }) — 'box revoke <user> --purge' removes each world cleanly first" >&2
|
||||
bash "$root/host/teardown-host.sh" \
|
||||
|| die "teardown-host did not complete — stopping BEFORE removing the install (the tree is untouched; fix the error and re-run)"
|
||||
# Consent forwards: --force and BOX_YES are this verb's installer-family
|
||||
# yes, and teardown-host must hear it too — otherwise a non-interactive
|
||||
# 'uninstall --all --purge-host --force' dies at teardown's own prompt
|
||||
# (EOF on read) with the tree untouched but the promise broken.
|
||||
if [ "$force" -eq 1 ] || [ -n "${BOX_YES:-}" ]; then
|
||||
bash "$root/host/teardown-host.sh" --yes \
|
||||
|| die "teardown-host did not complete — stopping BEFORE removing the install (the tree is untouched; fix the error and re-run)"
|
||||
else
|
||||
bash "$root/host/teardown-host.sh" \
|
||||
|| die "teardown-host did not complete — stopping BEFORE removing the install (the tree is untouched; fix the error and re-run)"
|
||||
fi
|
||||
else
|
||||
die_under_boxes "uninstall box" "box uninstall (or 'box uninstall --purge-host' to tear the host stack down with them)"
|
||||
fi
|
||||
|
|
|
|||
45
install.sh
45
install.sh
|
|
@ -62,6 +62,21 @@ confirm() { # $1 = question
|
|||
case "$reply" in y|Y|yes|YES) return 0 ;; *) return 1 ;; esac
|
||||
}
|
||||
|
||||
# A version is a DIRECTORY NAME under versions/ — nothing else. One strict
|
||||
# gate for every caller that builds a path from one (the installer's new_ver,
|
||||
# migration's flat_ver, and bin/box's 'use'/single-version uninstall): only
|
||||
# [A-Za-z0-9._+-], no leading '.' or '-'. That forbids '/', '..'-escapes,
|
||||
# spaces and option-lookalikes by construction — a crafted version dies HERE,
|
||||
# never in an rm -rf or an ln. bin/box carries a byte-identical copy;
|
||||
# test/cli.sh diffs the two so the gates cannot drift.
|
||||
valid_version() {
|
||||
case "$1" in
|
||||
''|.*|-*) return 1 ;;
|
||||
*[!A-Za-z0-9._+-]*) return 1 ;;
|
||||
esac
|
||||
return 0
|
||||
}
|
||||
|
||||
# Which boxes exist on this host, at THIS caller's tier? Prints their names
|
||||
# (both tag generations) and succeeds when at least one exists; fails when
|
||||
# none are visible — including when incus is absent or not answering, because
|
||||
|
|
@ -98,6 +113,15 @@ fi
|
|||
# under my boxes" class of failures (#66).
|
||||
confirm "Install box from $SRCDESC?" || die "cancelled — nothing was changed."
|
||||
|
||||
# Flip $DEST/current to versions/<v> atomically: build the new link beside it,
|
||||
# rename over. Plain ln -sfn is unlink+create — a window where current names
|
||||
# nothing and a concurrent 'box' invocation dies mid-chain. bin/box's cmd_use
|
||||
# flips with the same pattern.
|
||||
flip_current() {
|
||||
ln -sfn "versions/$1" "$DEST/current.new.$$"
|
||||
mv -Tf "$DEST/current.new.$$" "$DEST/current"
|
||||
}
|
||||
|
||||
# --- migrate a pre-0.7.0 flat install --------------------------------------
|
||||
# 0.6.0 and earlier installed the tree FLAT at $DEST (bin/box directly under
|
||||
# it). Move such a tree to versions/<its-VERSION> BEFORE anything else, so an
|
||||
|
|
@ -106,12 +130,16 @@ confirm "Install box from $SRCDESC?" || die "cancelled — nothing was changed."
|
|||
# window with no install — and the operator's tree is preserved bit for bit.
|
||||
if [ -e "$DEST/bin/box" ] && [ ! -d "$DEST/versions" ]; then
|
||||
flat_ver="$(cat "$DEST/VERSION" 2>/dev/null || echo 0.0.0-unknown)"
|
||||
# The flat tree's VERSION is data from disk, not from this installer — the
|
||||
# same trust boundary as the new_ver check, so the same gate: a corrupted
|
||||
# (or hostile) VERSION must not steer the mv/ln below out of versions/.
|
||||
valid_version "$flat_ver" || die "the flat install's VERSION is not a sane directory name: '$flat_ver' — fix $DEST/VERSION (one line, e.g. 0.6.0), then re-run"
|
||||
log "found a pre-0.7.0 flat install at $DEST (version $flat_ver) — migrating it into the versioned layout"
|
||||
staging="$DEST.migrating.$$"
|
||||
mv "$DEST" "$staging"
|
||||
mkdir -p "$DEST/versions"
|
||||
mv "$staging" "$DEST/versions/$flat_ver"
|
||||
ln -sfn "versions/$flat_ver" "$DEST/current"
|
||||
flip_current "$flat_ver"
|
||||
mkdir -p "$BINDIR"
|
||||
ln -sfn "$DEST/current/bin/box" "$BINDIR/box"
|
||||
log "migrated: it now lives at $DEST/versions/$flat_ver (still current; your boxes are untouched)"
|
||||
|
|
@ -176,7 +204,7 @@ fi
|
|||
# the identity of what is being installed, and 'box versions' lists these names.
|
||||
new_ver="$(cat "$EXTRACTED/VERSION" 2>/dev/null || true)"
|
||||
[ -n "$new_ver" ] || die "source has no VERSION file — cannot install it as a version"
|
||||
case "$new_ver" in */* | *' '* | .*) die "the source's VERSION is not a sane directory name: '$new_ver'" ;; esac
|
||||
valid_version "$new_ver" || die "the source's VERSION is not a sane directory name: '$new_ver'"
|
||||
|
||||
# --- install into $DEST/versions/<version> ---------------------------------
|
||||
VDIR="$DEST/versions/$new_ver"
|
||||
|
|
@ -186,12 +214,15 @@ if [ -d "$VDIR" ]; then
|
|||
# Replace THIS version's tree, as atomically as two renames allow — never
|
||||
# a partial overlay of new files onto an old tree.
|
||||
log "BOX_REINSTALL=1 — replacing the installed $new_ver tree"
|
||||
stage="$VDIR.new.$$"
|
||||
rm -rf "$stage"
|
||||
stage="$VDIR.new.$$"; old="$VDIR.old.$$"
|
||||
rm -rf "$stage" "$old"
|
||||
chmod +x "$EXTRACTED/bin/box"
|
||||
mv "$EXTRACTED" "$stage"
|
||||
rm -rf "$VDIR"
|
||||
# Swap by renames, delete LAST: rm-then-move leaves a hole the whole
|
||||
# length of the delete where current -> this version resolves to nothing.
|
||||
mv "$VDIR" "$old"
|
||||
mv "$stage" "$VDIR"
|
||||
rm -rf "$old"
|
||||
printf '%s\n' "$INSTALLED_FROM" > "$VDIR/INSTALLED_FROM"
|
||||
log "reinstalled $new_ver"
|
||||
else
|
||||
|
|
@ -221,7 +252,7 @@ fi
|
|||
cur="$(readlink -f "$DEST/current" 2>/dev/null || true)"
|
||||
want="$(readlink -f "$VDIR")"
|
||||
if [ -z "$cur" ] || [ ! -d "$cur" ]; then
|
||||
ln -sfn "versions/$new_ver" "$DEST/current"
|
||||
flip_current "$new_ver"
|
||||
log "default version: $new_ver"
|
||||
elif [ "$cur" = "$want" ]; then
|
||||
: # already the default — nothing to flip
|
||||
|
|
@ -241,7 +272,7 @@ else
|
|||
log " (a portable 'box export' is #70), then 'box rm <box>' when you are done"
|
||||
log " · then flip the default: box use $new_ver"
|
||||
else
|
||||
ln -sfn "versions/$new_ver" "$DEST/current"
|
||||
flip_current "$new_ver"
|
||||
log "default version switched: $old_ver -> $new_ver ('box use $old_ver' switches back)"
|
||||
fi
|
||||
fi
|
||||
|
|
|
|||
42
test/cli.sh
42
test/cli.sh
|
|
@ -368,6 +368,10 @@ check "versions: marks the running one" 0 "(running)" ibox "$B1/box" versions
|
|||
# --- box use ----------------------------------------------------------------
|
||||
check "use: no argument is a usage error" 2 "usage: box use" ibox "$B1/box" use
|
||||
check "use: an unknown version is refused by name" 1 "no such version" ibox "$B1/box" use 1.2.3
|
||||
# A version is a directory NAME — a crafted one must die at the gate, never
|
||||
# reach the ln (current pointing outside the root) or an rm -rf.
|
||||
check "use: a path-traversal version dies at the gate" 1 "not a sane version name" \
|
||||
ibox "$B1/box" use '../../tmp/evil'
|
||||
check "use: refuses under existing boxes, naming them (#66)" 1 "wedged" \
|
||||
ibox FAKE_BOXES="wedged stuck" "$B1/box" use "$VER"
|
||||
check "use: the refusal points at the remedy (box rm, then re-run)" 1 "box rm" \
|
||||
|
|
@ -417,6 +421,26 @@ check "migrate+upgrade: both versions present" 0 "" \
|
|||
check "migrate+upgrade: no boxes → the new version is the default" 0 "box 9.9.9-drill" \
|
||||
ibox "$B4/box" --version
|
||||
|
||||
# A broken current must halt the single-version path BEFORE any decision: the
|
||||
# CURRENT guard keys off what current resolves to, and a dangling link makes
|
||||
# that answer a lie. Drive the version tree's own binary — the current chain
|
||||
# is exactly what is broken. H4 has two versions; heal current afterwards.
|
||||
ln -sfn "versions/gone" "$H4/current"
|
||||
check "uninstall: refuses while current is dangling (heal before delete)" 1 "dangling" \
|
||||
ibox "$H4/versions/$VER/bin/box" uninstall 9.9.9-drill --force
|
||||
check "uninstall: ...and both version trees survived the refusal" 0 "" \
|
||||
bash -c "[ -d '$H4/versions/$VER' ] && [ -d '$H4/versions/9.9.9-drill' ]"
|
||||
ln -sfn "versions/9.9.9-drill" "$H4/current"
|
||||
|
||||
# The migration reads VERSION off the old tree — disk data, not installer
|
||||
# data. A hostile value must refuse BEFORE the tree moves anywhere.
|
||||
H9="$WORK/h9"; B9="$WORK/b9"; mkdir -p "$H9/bin" "$B9"
|
||||
cp "$ROOT/bin/box" "$H9/bin/box"; chmod +x "$H9/bin/box"
|
||||
printf '%s\n' '../pwn' > "$H9/VERSION"
|
||||
check "migrate: a hostile flat VERSION refuses to migrate" 1 "not a sane directory name" \
|
||||
inst "$H9" "$B9"
|
||||
check "migrate: ...with the flat tree untouched where it was" 0 "" test -x "$H9/bin/box"
|
||||
|
||||
# --- healing: a wedged \$BINDIR/box must never block an install -------------
|
||||
H5="$WORK/h5"; B5="$WORK/b5"; mkdir -p "$B5"
|
||||
ln -s "$WORK/nowhere/box" "$B5/box" # dangling
|
||||
|
|
@ -433,6 +457,8 @@ check "uninstall: refuses to remove the CURRENT version" 1 "CURRENT" \
|
|||
ibox "$B1/box" uninstall "$VER" --force
|
||||
check "uninstall: an unknown version is refused by name" 1 "no such version" \
|
||||
ibox "$B1/box" uninstall 5.5.5 --force
|
||||
check "uninstall: a path-traversal version dies at the gate (never an rm -rf)" 1 "not a sane version name" \
|
||||
ibox "$B1/box" uninstall '../../../../etc' --force
|
||||
check "uninstall: a version plus --all is ambiguous (usage error)" 2 "" \
|
||||
ibox "$B1/box" uninstall 9.9.9-drill --all --force
|
||||
check "uninstall: removes a non-current version" 0 "removed version" \
|
||||
|
|
@ -485,6 +511,22 @@ check "existing_boxes: extracted from bin/box (guards the awk)" 0 "user.box=1" c
|
|||
check "existing_boxes: bin/box and install.sh copies are byte-identical" 0 "" diff "$EBBIN" "$EBINST"
|
||||
rm -f "$EBBIN" "$EBINST"
|
||||
|
||||
# Same discipline for the version-name gate: one policy, two copies, no drift
|
||||
# — a version that install.sh would refuse must not be one 'box use' accepts.
|
||||
VVBIN="$(mktemp)"; VVINST="$(mktemp)"
|
||||
awk '/^valid_version\(\) \{/,/^\}/' "$ROOT/bin/box" > "$VVBIN"
|
||||
awk '/^valid_version\(\) \{/,/^\}/' "$ROOT/install.sh" > "$VVINST"
|
||||
check "valid_version: extracted from bin/box (guards the awk)" 0 "A-Za-z0-9" cat "$VVBIN"
|
||||
check "valid_version: bin/box and install.sh copies are byte-identical" 0 "" diff "$VVBIN" "$VVINST"
|
||||
rm -f "$VVBIN" "$VVINST"
|
||||
|
||||
# --purge-host must FORWARD installer-family consent: under --force/BOX_YES
|
||||
# the teardown call carries --yes, or a non-interactive combined uninstall
|
||||
# dies at teardown's own prompt with the flag's promise broken.
|
||||
# shellcheck disable=SC2016 # the $-string is a literal in the target file
|
||||
check "uninstall: --purge-host forwards consent to teardown-host (--yes)" 0 "" \
|
||||
grep -qF -- 'bash "$root/host/teardown-host.sh" --yes' "$ROOT/bin/box"
|
||||
|
||||
# --- the help keeps its promises --------------------------------------------
|
||||
check "help: the table lists 'versions'" 0 "versions" "$BOX" help
|
||||
check "help use: names the #66 stance" 0 "boxes" "$BOX" help use
|
||||
|
|
|
|||
Loading…
Reference in a new issue