forked from heavy-duty/box
fix: make the 'pristine' path's never-fatal contract structural, not incidental
Both from claude-bot's review of #128. storage_driver's two driver probes had no '|| true' while the pool probe one line up did. They were safe only via a bash subtlety: command substitution strips errexit, so a failing 'incus storage show' fell through to the fallback instead of aborting. Add 'shopt -s inherit_errexit' to bin/box — the class of robustness tweak #107 documents sailing through review — and under pipefail a restricted tier's probe refusal becomes a fatal abort mid-mint, inside the function whose contract is NEVER fatal. All three probes now read alike and depend on nothing. Driven test: refuse both probes with inherit_errexit on, assert the caller survives and the driver reads empty (the unreadable-pool case, which already takes the mark anyway). The clone's inheritance narration piped a multi-line 'incus snapshot list' straight into 'grep -q' — #124's racing-reader class in a 'set -euo pipefail' file. A 141 there reads as "no pristine" and narrates the WRONG inheritance shape on a clone that does carry one. Capture first, then read. Pinned by shape (grep|head|sed|awk|read), not by instance spelling. bin/box has ~10 more sites of that class and no sweep covers the file; one of them unpins an exposure's static address on a fail-open read. Filed as #134 rather than widened into this PR. Refs #104, #107, #124, #134. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
parent
a091c6e496
commit
24a05c3020
3 changed files with 62 additions and 3 deletions
|
|
@ -104,6 +104,13 @@ which records not just what changed but what each drill run proved.
|
|||
`box export` remains the durable path — and that it cannot reach off-box
|
||||
state such as a tailnet join or a runner registration. `BOX_SNAPSHOT_PRISTINE=0`
|
||||
opts out, in the `BOX_LAUNCH_TIMEOUT` shape rather than a new flag.
|
||||
The never-fatal contract is now structural rather than incidental: all three
|
||||
storage probes carry `|| true`, so a restricted tier's refusal stays an
|
||||
answer even if `inherit_errexit` is ever switched on in `bin/box` (#107's
|
||||
class), and the clone's inheritance read captures before it greps rather
|
||||
than piping a multi-line `incus` writer into an early-exit reader (#124's
|
||||
class), which under `pipefail` could have narrated the wrong inheritance
|
||||
shape on a clone that does carry a `pristine`.
|
||||
|
||||
### Changed
|
||||
|
||||
|
|
|
|||
22
bin/box
22
bin/box
|
|
@ -1103,9 +1103,18 @@ storage_driver() {
|
|||
local pool driver
|
||||
pool="$(incus profile device get box-net root pool 2>/dev/null || true)"
|
||||
[ -n "$pool" ] || pool=default
|
||||
driver="$(incus storage show "$pool" 2>/dev/null | awk '/^driver:/ {print $2; exit}')"
|
||||
# '|| true' on all three probes, not just the first. A refusal here is an
|
||||
# ANSWER ("this tier cannot read the pool"), never an error — this function
|
||||
# must never be the thing that kills a mint. Today the two pipelines are
|
||||
# safe only by accident: command substitution strips errexit, so a failing
|
||||
# 'incus storage show' under pipefail falls through to the fallback instead
|
||||
# of aborting. Add 'shopt -s inherit_errexit' to this file — exactly the
|
||||
# robustness tweak #107 describes sailing through review — and that accident
|
||||
# reverses into a fatal abort mid-mint, inside the function whose contract
|
||||
# is NEVER fatal. Make the three probes read alike and depend on nothing.
|
||||
driver="$(incus storage show "$pool" 2>/dev/null | awk '/^driver:/ {print $2; exit}' || true)"
|
||||
[ -n "$driver" ] || driver="$(incus storage list --format csv 2>/dev/null \
|
||||
| awk -F, -v p="$pool" '$1 == p {print $2; exit}')"
|
||||
| awk -F, -v p="$pool" '$1 == p {print $2; exit}' || true)"
|
||||
printf '%s' "$driver"
|
||||
}
|
||||
|
||||
|
|
@ -1222,7 +1231,14 @@ cmd_new() {
|
|||
# carries no snapshot list, so that clone starts with none. Both are
|
||||
# honest; neither is invented. Say which one this is rather than leaving
|
||||
# the operator to run 'box info' to find out.
|
||||
if incus snapshot list "$instance" --format csv 2>/dev/null | grep -q '^pristine,'; then
|
||||
# Capture first, THEN read (#124's class). Piping a multi-line incus
|
||||
# writer straight into an early-exit reader lets grep close the pipe on
|
||||
# the first match, SIGPIPE incus, and hand pipefail a 141 — which here
|
||||
# would read as "no pristine" and narrate the WRONG inheritance shape on a
|
||||
# clone that actually has one. Un-racy today at this writer's size, but
|
||||
# the assignment costs nothing and does not depend on that staying true.
|
||||
local snaps; snaps="$(incus snapshot list "$instance" --format csv 2>/dev/null || true)"
|
||||
if printf '%s\n' "$snaps" | grep -q '^pristine,'; then
|
||||
echo "box: it inherited the source's snapshots, 'pristine' among them (box restore $name pristine)."
|
||||
else
|
||||
echo "box: no 'pristine' mark here — a clone has no pristine moment to capture, and box"
|
||||
|
|
|
|||
36
test/cli.sh
36
test/cli.sh
|
|
@ -510,6 +510,14 @@ check "pristine: nothing on the clone path creates a snapshot at all" 1 "" \
|
|||
# 'box info' to find out which world they are in.
|
||||
check "pristine: the clone narrates whether a pristine rode along" 0 "" \
|
||||
grep -q "no 'pristine' mark here" "$CLONEBR"
|
||||
# ...and it reads the snapshot list CAPTURE-FIRST (#124's class). Piping a
|
||||
# multi-line incus writer into an early-exit reader lets the reader close the
|
||||
# pipe, SIGPIPE incus, and hand pipefail a 141 — which on THIS line reads as
|
||||
# "no pristine" and narrates the wrong inheritance shape on a clone that has
|
||||
# one. Pin the shape, not the instance spelling: no 'incus snapshot list'
|
||||
# feeding grep/head/sed/awk/read directly.
|
||||
check "pristine: the clone's inheritance read is capture-first, not a piped early-exit reader" 1 "" \
|
||||
grep -Eq 'incus snapshot list[^|]*\| *(grep|head|sed|awk|read)' "$CLONEBR"
|
||||
rm -f "$CLONEBR"
|
||||
|
||||
# The policy half, DRIVEN not grepped: extract storage_driver +
|
||||
|
|
@ -520,6 +528,34 @@ awk '/^storage_driver\(\) \{/,/^\}/;/^snapshot_pristine\(\) \{/,/^\}/' "$ROOT/bi
|
|||
check "pristine: the functions extracted from bin/box (guards the awk)" 0 "BOX_SNAPSHOT_PRISTINE" cat "$PRISFN"
|
||||
check "pristine: the extracted functions are valid bash" 0 "" bash -n "$PRISFN"
|
||||
|
||||
# storage_driver's probes must survive a REFUSAL, and not by accident. Today
|
||||
# command substitution strips errexit, so a failing probe falls through to the
|
||||
# fallback; add 'shopt -s inherit_errexit' to bin/box — the robustness tweak
|
||||
# #107 describes sailing through review — and under pipefail that same refusal
|
||||
# becomes a fatal abort mid-mint, inside the function whose contract is NEVER
|
||||
# fatal. Drive it with inherit_errexit ON and a tier that refuses both probes:
|
||||
# the function must return empty (the unreadable-pool case) and the caller
|
||||
# must still be alive afterwards.
|
||||
driver_under_inherit_errexit() {
|
||||
# shellcheck disable=SC2016 # the body is the stub's source, expanded by the
|
||||
# inner bash, never by this shell.
|
||||
env PRISFN="$PRISFN" bash -c '
|
||||
set -euo pipefail
|
||||
shopt -s inherit_errexit
|
||||
incus() {
|
||||
case "$*" in
|
||||
"profile device get box-net root pool") printf "boxpool\n" ;;
|
||||
*) printf "incus: not authorized\n" >&2; return 1 ;;
|
||||
esac
|
||||
}
|
||||
. "$PRISFN"
|
||||
d="$(storage_driver)"
|
||||
printf "SURVIVED driver=[%s]\n" "$d"
|
||||
' 2>&1
|
||||
}
|
||||
check "pristine: a refused storage probe is an answer, not a fatal (survives inherit_errexit)" 0 "SURVIVED driver=[]" \
|
||||
driver_under_inherit_errexit
|
||||
|
||||
# pris <driver> [env...] — drive snapshot_pristine against a fake pool of
|
||||
# <driver>. 'none' makes both probes answer nothing (the unreadable-pool
|
||||
# case). Every incus call the function can make is stubbed and echoed, so the
|
||||
|
|
|
|||
Loading…
Reference in a new issue