Make setup-host privilege-aware; make the drill prove the new contract
Review found two real problems, both confirmed by reproducing them. setup-host hardcoded 'sudo' for every privileged call, so install.sh's deliberate root branch — the one that proceeds when id -u is 0 even with no sudo installed — handed off to a script that died on 'sudo: command not found' before doing anything (exit 127, reproduced with env -i and a minimal PATH). The root path was nominal, not real. Privilege is now resolved once: nothing at UID 0, sudo otherwise, a clear error if neither is possible. Two things fell out of that. Root does not need incus-admin at all (UID 0 opens the socket regardless), so adding root to the group was a no-op that also missed the human — under 'sudo install.sh' that is SUDO_USER, who is now the one granted the group. And apt must not hang: install.sh runs setup-host with nobody watching, while a fresh cloud image holds the dpkg lock in apt-daily for its first minutes, so the calls are now bounded and non-interactive. The drill did not exercise any of this. It ran setup-host immediately after install.sh, so the stack existed by the drill's own hand and a run passed identically whether or not install.sh had done a thing — a fresh run converged three times while its messages still described the pre-#63 "first pass may only add you to the group" behaviour. It now asserts the post-install stack in-group, before the clean or anything else mutates the host, which is the assertion that actually proves #64. setup-host then runs exactly once more, after the clean — that one is load-bearing, since the clean deliberately unsets dns.mode and something has to converge it back. DRILL_OWNS_SETUP=1 hands sequencing back to the drill. Pre-setup tripwires now read before install.sh, because install.sh is what triggers setup now; read afterwards they said nothing. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
claude-bot-andresmgsl
parent
aad576a86a
commit
ce3a0c5076
3 changed files with 166 additions and 26 deletions
26
CHANGELOG.md
26
CHANGELOG.md
|
|
@ -18,8 +18,34 @@ which records not just what changed but what each drill run proved.
|
|||
group and died further down on a bare permission error from `incus`. Argless
|
||||
`id -nG` asks the process what it actually holds.
|
||||
|
||||
- **`setup-host` works as root, with or without `sudo`** — every privileged
|
||||
call was a hardcoded `sudo`, so on a minimal root image (no `sudo` package)
|
||||
it died on `sudo: command not found` before doing anything. Privilege is now
|
||||
resolved once: nothing at UID 0, `sudo` otherwise, and a clear error if
|
||||
neither is possible. This is what made `install.sh`'s root path real rather
|
||||
than nominal.
|
||||
- **`setup-host` grants `incus-admin` to the human, not to root** — under
|
||||
`sudo install.sh` it would have added `root` to the group: a no-op (UID 0
|
||||
opens the socket regardless) that also left the actual user locked out of
|
||||
their own boxes. It now derives the login user from `SUDO_USER`.
|
||||
- **`setup-host`'s apt calls can no longer hang** — a fresh cloud image has
|
||||
`apt-daily`/`unattended-upgrades` holding the dpkg lock, and a plain
|
||||
`apt-get install` waits on it silently and indefinitely. Now bounded
|
||||
(`DPkg::Lock::Timeout=300`) and non-interactive, which matters because
|
||||
`install.sh` runs it with nobody watching.
|
||||
|
||||
### Changed
|
||||
|
||||
- **`drill.sh` proves the new contract instead of masking it** — the drill ran
|
||||
`setup-host` itself right after installing, so the stack existed by its own
|
||||
hand and a run passed identically whether or not `install.sh` had done a
|
||||
thing; a fresh run converged the stack three times, while the messages still
|
||||
described the pre-#63 "first pass may only add you to the group" behaviour.
|
||||
It now asserts the post-install stack in-group before touching the host, and
|
||||
runs `setup-host` exactly once more — after the clean, which deliberately
|
||||
unsets `dns.mode` and so has to be converged back. `DRILL_OWNS_SETUP=1`
|
||||
hands sequencing back to the drill. Pre-setup tripwires now read *before*
|
||||
`install.sh`, since that is what triggers setup now.
|
||||
- **`install.sh` runs the host setup itself** (#64) — it printed a warning and
|
||||
left you a command to run, so the install reported success and `box new`
|
||||
failed on a host with no Incus. Since `setup-host` is idempotent, doing this
|
||||
|
|
|
|||
|
|
@ -172,6 +172,36 @@ EOF
|
|||
fi
|
||||
|
||||
phase "Installing box ($REPO@$REF)"
|
||||
|
||||
# Sudo, up front and out loud. Later calls run unattended, and a password
|
||||
# prompt swallowed by a '-qq' redirect looks exactly like a hang. This now
|
||||
# has to precede the install too: install.sh runs the host setup itself, so
|
||||
# the first thing needing root is no longer further down — it is inside the
|
||||
# very next command.
|
||||
sudo -v || { echo "drill: need sudo (the host setup installs packages and firewall rules)"; exit 1; }
|
||||
|
||||
# Pre-setup observations must be READ BEFORE install.sh, because install.sh
|
||||
# is now what runs setup-host. Read after it and setup has already had its
|
||||
# chance to act, so the observation says nothing.
|
||||
# setup-host.sh installs nftables itself when neither nft nor UFW exists
|
||||
# (a stock Debian 13 cloud image ships neither). This is a tripwire: if it
|
||||
# fires, that fix regressed.
|
||||
fw_absent_pre=0
|
||||
if ! command -v nft >/dev/null 2>&1 && ! command -v ufw >/dev/null 2>&1; then
|
||||
fw_absent_pre=1
|
||||
fi
|
||||
|
||||
# DRILL_OWNS_SETUP=1 opts out of the installer's automatic setup and puts the
|
||||
# drill back in charge of sequencing it (install, then clean, then converge).
|
||||
# The DEFAULT deliberately does not: a drill that runs setup-host itself right
|
||||
# after installing cannot tell you whether install.sh did its job, because the
|
||||
# drill's own call would build the stack either way. The default path exercises
|
||||
# what a user actually runs, and asserts the result in-group below.
|
||||
OWNS="${DRILL_OWNS_SETUP:-0}"
|
||||
if [ "$OWNS" = 1 ]; then
|
||||
export BOX_SKIP_SETUP_HOST=1
|
||||
fi
|
||||
|
||||
BOX_REPO="$REPO" BOX_REF="$REF" \
|
||||
bash -c "$(curl -fsSL "https://raw.githubusercontent.com/$REPO/$REF/install.sh")" \
|
||||
|| { echo "install failed"; exit 1; }
|
||||
|
|
@ -195,17 +225,10 @@ EOF
|
|||
inf "installed tree confirms: $got"
|
||||
|
||||
phase "Host setup (Incus, boxnet, ACL, profile, firewall)"
|
||||
# setup-host.sh installs nftables itself when neither nft nor UFW exists
|
||||
# (a stock Debian 13 cloud image ships neither). This guard is a tripwire:
|
||||
# if it fires, that fix regressed.
|
||||
if ! command -v nft >/dev/null 2>&1 && ! command -v ufw >/dev/null 2>&1; then
|
||||
if [ "$fw_absent_pre" = 1 ]; then
|
||||
note "neither nft nor ufw present pre-setup — setup-host.sh must install nftables itself (it fixed this once; watch that it still does)"
|
||||
fi
|
||||
|
||||
# Sudo, up front and out loud. Later calls run unattended, and a password
|
||||
# prompt swallowed by a '-qq' redirect looks exactly like a hang.
|
||||
sudo -v || { echo "drill: need sudo (the host setup installs packages and firewall rules)"; exit 1; }
|
||||
|
||||
# apt's lock is held by apt-daily / unattended-upgrades on a fresh cloud
|
||||
# image, and 'apt-get -qq >/dev/null' waits for it in COMPLETE SILENCE —
|
||||
# which is how run 5 looked stuck for minutes right after this header.
|
||||
|
|
@ -224,16 +247,57 @@ EOF
|
|||
inf "incus already installed — skipping apt"
|
||||
fi
|
||||
|
||||
inf "running setup-host.sh (first pass: may only add you to incus-admin)…"
|
||||
~/.local/share/box/host/setup-host.sh || true
|
||||
# The group we were just added to isn't in this shell's credentials yet.
|
||||
# No setup-host call here any more. It used to run a full "first pass" that
|
||||
# the comment described as "may only add you to incus-admin" — behaviour that
|
||||
# no longer exists (setup-host converges in one run now, #63) and that, since
|
||||
# install.sh runs setup itself (#64), was simply the stack being built a
|
||||
# second time before the drill had asserted the first.
|
||||
if [ "$OWNS" = 1 ]; then
|
||||
# We opted out of the installer's setup, so nobody has joined us to the
|
||||
# group yet. usermod ONLY: the stack build waits until after the clean
|
||||
# below, which is the entire reason for owning the sequence.
|
||||
inf "DRILL_OWNS_SETUP=1 — the drill owns the host setup"
|
||||
id -nG | grep -qw incus-admin || sudo usermod -aG incus-admin "$USER"
|
||||
else
|
||||
inf "install.sh ran the host setup — asserting what it left, in-group, next"
|
||||
fi
|
||||
# setup-host's own sg re-exec was a CHILD of install.sh; this shell's
|
||||
# credentials are untouched, so we still have to enter the group ourselves —
|
||||
# once, for the remainder of the drill.
|
||||
inf "re-entering inside the incus-admin group…"
|
||||
exec sg incus-admin -c "IN_GROUP=1 BOX_REPO='$REPO' BOX_REF='$REF' KEEP=$KEEP bash '$SELF' --in-group"
|
||||
exec sg incus-admin -c "IN_GROUP=1 DRILL_OWNS_SETUP='$OWNS' BOX_REPO='$REPO' BOX_REF='$REF' KEEP=$KEEP bash '$SELF' --in-group"
|
||||
fi
|
||||
|
||||
export PATH="$HOME/.local/bin:$PATH"
|
||||
KEEP="${KEEP:-0}"
|
||||
|
||||
# PROVE THE INSTALLER'S CONTRACT (#64) — first, before the clean or anything
|
||||
# else on this host mutates the stack, and before the drill runs setup-host
|
||||
# itself further down. That ordering is the whole point: the old flow ran
|
||||
# setup-host immediately after installing, so the stack existed by the drill's
|
||||
# own hand and the run passed identically whether or not install.sh had done a
|
||||
# thing. This is read-only, so it is safe with a previous run's boxes still
|
||||
# attached.
|
||||
if [ "${DRILL_OWNS_SETUP:-0}" != 1 ]; then
|
||||
phase "Asserting the stack that install.sh built"
|
||||
missing=""
|
||||
incus network show boxnet >/dev/null 2>&1 || missing="$missing boxnet"
|
||||
incus network acl show box-isolate >/dev/null 2>&1 || missing="$missing box-isolate"
|
||||
incus profile show box-net >/dev/null 2>&1 || missing="$missing box-net"
|
||||
# Last thing setup-host does, so it doubles as "it ran to the end".
|
||||
sudo nft list table bridge box >/dev/null 2>&1 || missing="$missing nft-bridge-box"
|
||||
if [ -n "$missing" ]; then
|
||||
echo "drill: FATAL — install.sh reported success but left an INCOMPLETE stack:$missing" >&2
|
||||
echo " install.sh is supposed to run the host setup itself (#64), and setup-host" >&2
|
||||
echo " is supposed to converge in one run (#63). One of those did not happen." >&2
|
||||
echo " reproduce with the output visible:" >&2
|
||||
echo " ~/.local/share/box/host/setup-host.sh" >&2
|
||||
echo " or hand setup back to the drill: DRILL_OWNS_SETUP=1 $SELF" >&2
|
||||
exit 1
|
||||
fi
|
||||
ok "install.sh left a complete host stack (boxnet, box-isolate, box-net, nft bridge drop) — no second setup needed"
|
||||
fi
|
||||
|
||||
# CLEAN BEFORE SETUP, not after. setup-host.sh reconfigures the network's ACLs,
|
||||
# and a previous run's boxes are still ATTACHED to that network — 'incus network
|
||||
# set' then has to push the change onto every live NIC, which is how run 6
|
||||
|
|
@ -277,7 +341,15 @@ done
|
|||
left="$(incus list --format csv --columns n 2>/dev/null | tr '\n' ' ')"
|
||||
[ -n "$left" ] && inf "instances still on this host (not ours, left alone): $left"
|
||||
|
||||
inf "running setup-host.sh (in-group pass: network, ACL, profile, firewall)…"
|
||||
# This call stays, and it is NOT the install's setup repeated for its own sake:
|
||||
# the clean above deliberately unsets dns.mode, which is part of the SHIPPED
|
||||
# stack, and drops a previous run's phase-D mutations. Something has to put the
|
||||
# host back together afterwards, and setup-host is that something — this is the
|
||||
# "converge against a clean slate" the block above is ordered for. On the
|
||||
# default path the install's setup has already been asserted, so what this
|
||||
# proves is idempotency: a second run over a cleaned host is a no-op that
|
||||
# restores the stack rather than a fresh build.
|
||||
inf "running setup-host.sh (post-clean convergence: restores dns.mode and any reverted mutations)…"
|
||||
if ! timeout -k 10 300 ~/.local/share/box/host/setup-host.sh; then
|
||||
echo "drill: setup-host.sh failed or timed out (>5 min)." >&2
|
||||
echo " it should take seconds on a host that already has incus. usual causes:" >&2
|
||||
|
|
|
|||
|
|
@ -6,11 +6,53 @@ set -euo pipefail
|
|||
self="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)/$(basename "${BASH_SOURCE[0]}")"
|
||||
here="$(dirname "$(dirname "$self")")"
|
||||
|
||||
if ! command -v incus >/dev/null; then
|
||||
sudo apt-get update
|
||||
sudo apt-get install -y incus
|
||||
# How we reach root, decided once. 'sudo' cannot be hardcoded: at UID 0 it is
|
||||
# unnecessary, and on a minimal root image it is not installed at all — this
|
||||
# script died on 'sudo: command not found' before doing anything, which made
|
||||
# install.sh's deliberate root path unusable on exactly the hosts it was for.
|
||||
if [ "$(id -u)" -eq 0 ]; then
|
||||
SUDO=""
|
||||
elif command -v sudo >/dev/null 2>&1; then
|
||||
SUDO="sudo"
|
||||
else
|
||||
echo "ERROR: host setup needs root and 'sudo' was not found." >&2
|
||||
echo " re-run this as root: $self" >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
# apt, unattended-safe. install.sh now runs us without a human watching, and
|
||||
# a fresh cloud image has apt-daily/unattended-upgrades holding the dpkg lock
|
||||
# for the first minutes of its life — plain 'apt-get install' then waits on it
|
||||
# in complete silence, indefinitely. Bound the wait and never prompt.
|
||||
# 'env', not a bare VAR=val prefix: bash recognises assignments at PARSE time,
|
||||
# so with $SUDO empty (we are root) 'DEBIAN_FRONTEND=x apt-get' would have
|
||||
# already been parsed as a plain word and bash would try to EXECUTE it —
|
||||
# 'DEBIAN_FRONTEND=noninteractive: command not found'. env is immune.
|
||||
apt_get() {
|
||||
$SUDO env DEBIAN_FRONTEND=noninteractive apt-get -o DPkg::Lock::Timeout=300 "$@"
|
||||
}
|
||||
|
||||
if ! command -v incus >/dev/null; then
|
||||
apt_get update
|
||||
apt_get install -y incus
|
||||
fi
|
||||
|
||||
if [ "$(id -u)" -eq 0 ]; then
|
||||
# Root needs no group: UID 0 opens /var/lib/incus/unix.socket regardless of
|
||||
# who owns it, and there is nothing to re-exec into. The HUMAN needs it — and
|
||||
# under 'sudo install.sh' that is SUDO_USER, not the root we are running as.
|
||||
# Adding root to incus-admin would be a no-op that also left the actual user
|
||||
# locked out of their own boxes.
|
||||
# NOTE: 'id -nG "$name"' here is deliberate and NOT the bug fixed below. That
|
||||
# bug was asking the DATABASE about our own process; this asks the database
|
||||
# about someone else's account, which is the only thing it can be asked.
|
||||
login_user="${SUDO_USER:-}"
|
||||
if [ -n "$login_user" ] && [ "$login_user" != root ]; then
|
||||
if ! id -nG "$login_user" | grep -qw incus-admin; then
|
||||
usermod -aG incus-admin "$login_user"
|
||||
echo "added $login_user to incus-admin — log out and back in for your shell to pick it up"
|
||||
fi
|
||||
fi
|
||||
# Group membership is a property of THIS PROCESS's credentials, not of the group
|
||||
# database — and the two disagree for exactly as long as it matters here.
|
||||
# 'id -nG "$USER"' names a user, so it reads /etc/group and reports incus-admin
|
||||
|
|
@ -20,8 +62,8 @@ fi
|
|||
# permission error that named neither the group nor the re-login. Argless
|
||||
# 'id -nG' asks the process what it actually holds, which is what incus checks
|
||||
# when it opens /var/lib/incus/unix.socket.
|
||||
if ! id -nG | grep -qw incus-admin; then
|
||||
sudo usermod -aG incus-admin "$USER"
|
||||
elif ! id -nG | grep -qw incus-admin; then
|
||||
$SUDO usermod -aG incus-admin "$USER"
|
||||
# Then finish the job rather than adjourning it. Exiting 0 here was a
|
||||
# success-shaped no-op: no boxnet, no ACL, no box-net profile, no firewall —
|
||||
# and the burden of knowing that on the reader of a NOTE (#63). 'sg' runs us
|
||||
|
|
@ -50,7 +92,7 @@ fi
|
|||
# says so.
|
||||
if ! incus storage show default >/dev/null 2>&1; then
|
||||
driver=btrfs
|
||||
command -v mkfs.btrfs >/dev/null 2>&1 || sudo apt-get install -y btrfs-progs || driver=dir
|
||||
command -v mkfs.btrfs >/dev/null 2>&1 || apt_get install -y btrfs-progs || driver=dir
|
||||
if ! incus admin init --preseed <<PRESEED
|
||||
storage_pools:
|
||||
- name: default
|
||||
|
|
@ -139,19 +181,19 @@ incus network set boxnet raw.dnsmasq \
|
|||
# The no-UFW path drives nft directly, and a stock Debian 13 cloud image ships
|
||||
# neither nftables nor UFW — install the dependency we are about to use.
|
||||
if ! command -v ufw >/dev/null 2>&1 && ! command -v nft >/dev/null 2>&1; then
|
||||
sudo apt-get install -y nftables
|
||||
apt_get install -y nftables
|
||||
fi
|
||||
sudo install -m 755 "$here/host/box-firewall.sh" /usr/local/sbin/box-firewall
|
||||
sudo install -m 644 "$here/host/box-firewall.service" /etc/systemd/system/
|
||||
sudo systemctl daemon-reload
|
||||
sudo systemctl enable box-firewall.service
|
||||
$SUDO install -m 755 "$here/host/box-firewall.sh" /usr/local/sbin/box-firewall
|
||||
$SUDO install -m 644 "$here/host/box-firewall.service" /etc/systemd/system/
|
||||
$SUDO systemctl daemon-reload
|
||||
$SUDO systemctl enable box-firewall.service
|
||||
# RESTART, not 'enable --now'. The unit is RemainAfterExit, so once it has run
|
||||
# it stays "active" forever — and 'enable --now' does nothing to an active unit.
|
||||
# Re-running setup-host after upgrading the tool therefore installed the new
|
||||
# rules to /usr/local/sbin and never applied them: the host kept the old
|
||||
# firewall, silently, and the box→box hole stayed open through a release that
|
||||
# claimed to close it. Restart re-runs the script, which is idempotent by design.
|
||||
sudo systemctl restart box-firewall.service
|
||||
$SUDO systemctl restart box-firewall.service
|
||||
|
||||
# Profile — box-net, the placement contract: the isolated NIC and the root
|
||||
# disk, nothing a template controls (resources are stamped per-instance from
|
||||
|
|
@ -165,7 +207,7 @@ incus profile edit box-net < "$here/profiles/box-net.yaml"
|
|||
|
||||
# The sibling drop is the one rule whose absence is invisible: everything keeps
|
||||
# working, and boxes can simply reach each other. Assert it landed.
|
||||
if sudo nft list table bridge box >/dev/null 2>&1; then
|
||||
if $SUDO nft list table bridge box >/dev/null 2>&1; then
|
||||
echo "Isolation: box-to-box drop is live (nft bridge table 'box')."
|
||||
else
|
||||
echo "WARNING: the box-to-box drop is NOT active — boxes can reach each other." >&2
|
||||
|
|
|
|||
Loading…
Reference in a new issue