Merge pull request #55 from dan-claude-bot/fix/incus-revoke-via-box
fix: dropping the box role revokes through box, not behind its back
This commit is contained in:
Daniel Marin
GitHub
commit
71fd0513ab
4 changed files with 264 additions and 5 deletions
25
CHANGELOG.md
25
CHANGELOG.md
|
|
@ -4,7 +4,7 @@ History before 0.1.0 lives in git — rig grew its version surface (`VERSION`,
|
|||
`rig --version`, the side-by-side `versions/<v>` install layout; #35/#36)
|
||||
on the way to cutting its first release, and this file starts there.
|
||||
|
||||
## 0.1.0 — 2026-07-19
|
||||
## Unreleased
|
||||
|
||||
### Fixed
|
||||
|
||||
|
|
@ -27,6 +27,29 @@ on the way to cutting its first release, and this file starts there.
|
|||
exact-membership convergence now strips box-role users out of `incus`, on
|
||||
the same reasoning: a membership inherited from a previous life is the same
|
||||
half-grant as a freshly added one.
|
||||
- **Dropping the box role revokes through `box`, not behind its back**
|
||||
(#50) — `users apply` converged group `incus` with a bare `gpasswd -d`,
|
||||
the same move it makes for `rig-admin` and `rig`. Those two are rig's;
|
||||
`incus` is box's, and `box revoke` does strictly more with it: it says
|
||||
out loud that supplementary groups are read at LOGIN, so a session the
|
||||
dropped operator already holds keeps the Incus socket until it dies, and
|
||||
hands over `loginctl terminate-user <user>` as the remedy. rig logged
|
||||
`removed <user> from incus` and moved on, so an operator who dropped
|
||||
someone from the users file and watched apply succeed believed the VM
|
||||
access was gone — and was wrong for as long as that user held a session.
|
||||
Both removal paths (the per-user convergence and the dropped-user sweep)
|
||||
now call `box revoke`, which keeps one owner for the group. Never
|
||||
`--purge`: that deletes the user's boxes, images and project, and
|
||||
destroying someone's running machines is not a convergence step — it
|
||||
stays an explicit admin act. The exit code is not trusted (#12's lesson):
|
||||
a revoke that returns 0 with the membership still standing has not closed
|
||||
the socket, and rig falls back to removing the group itself, as it also
|
||||
does where box is not installed. Every fallback path carries the session
|
||||
warning, because the silence was the bug.
|
||||
|
||||
## 0.1.0 — 2026-07-19
|
||||
|
||||
### Fixed
|
||||
|
||||
- **The release suite accepts the ceremony's own tree** (#44) —
|
||||
`test/release.sh` demanded a literal `## Unreleased` heading in the real
|
||||
|
|
|
|||
14
README.md
14
README.md
|
|
@ -784,6 +784,20 @@ apply dies pointing at `box setup-host` rather than conjure a group nothing
|
|||
would consult. `incus-admin` is deliberately **not** a role: that group is
|
||||
host-root-equivalent, break-glass by hand only.
|
||||
|
||||
Losing the `box` role goes back through box, too. `rig-admin` and `rig` are
|
||||
rig's groups and a `gpasswd -d` is the whole story for them; `incus` is box's,
|
||||
and `box revoke` does more with it than remove a membership — it says out loud
|
||||
that supplementary groups are read **at login**, so a session the dropped
|
||||
operator already holds keeps the Incus socket until it dies, and names
|
||||
`loginctl terminate-user <user>` as the way to end it now. So apply calls
|
||||
`box revoke` and lets box speak. Never `--purge`: that deletes the user's
|
||||
boxes, images and project, and destroying someone's running machines is not a
|
||||
convergence step — `box revoke <user> --purge` stays a deliberate admin act.
|
||||
Where box is not installed (or the revoke returns success with the membership
|
||||
still standing — an exit code is not effective state) rig removes the group
|
||||
itself **and carries the session warning**, because a silent removal is what
|
||||
lets an operator believe access ended when it has not.
|
||||
|
||||
**All passwords stay locked, always** — created or found. The SSH key at the
|
||||
door is the authentication, and NOPASSWD sudo does not weaken it: there was
|
||||
never a password to guess or rotate.
|
||||
|
|
|
|||
|
|
@ -47,7 +47,10 @@ roles:
|
|||
admin group rig-admin — full NOPASSWD sudo
|
||||
rig group rig — NOPASSWD sudo for /usr/local/bin/rig only
|
||||
box group incus — Incus restricted tier, no sudo (box's setup-host
|
||||
owns the Incus install; rig only asserts it)
|
||||
owns the Incus install; rig only asserts it).
|
||||
Dropping this role hands the group back through
|
||||
'box revoke' — never --purge: convergence removes
|
||||
access, never someone's running boxes.
|
||||
|
||||
All passwords stay locked, always — the SSH key at the door is the
|
||||
authentication, and NOPASSWD sudo does not weaken it. Convergent: membership
|
||||
|
|
@ -222,6 +225,60 @@ fi
|
|||
|
||||
in_group() { id -nG "$1" 2>/dev/null | tr ' ' '\n' | grep -qx "$2"; }
|
||||
|
||||
# --- dropping role box: give the group back to the tool that owns it (#50) ---
|
||||
# rig-admin and rig are rig's own groups, and `gpasswd -d` is the whole story
|
||||
# for them. incus is not rig's: box's setup-host creates it, `box grant` hands
|
||||
# it out, and `box revoke` takes it back — doing strictly MORE than removing
|
||||
# the membership. It says out loud what removing the membership does not do:
|
||||
# supplementary groups are read at LOGIN, so a session the dropped operator
|
||||
# already holds keeps the Incus socket until that session dies, and the remedy
|
||||
# is `loginctl terminate-user <user>`. rig's bare `gpasswd -d` logged "removed
|
||||
# <user> from incus" and left it there, so an operator who dropped someone from
|
||||
# the users file and watched apply succeed believed the VM access was gone —
|
||||
# and was wrong for as long as that user held a session.
|
||||
#
|
||||
# So box takes its own group back. The fallback below exists for the host where
|
||||
# box is not installed (someone else built the Incus stack, or box was removed
|
||||
# from under it), and it carries the session warning ITSELF: the silence is the
|
||||
# bug being fixed, not the gpasswd call.
|
||||
#
|
||||
# NEVER --purge from here. A bare revoke ends access and leaves the user's
|
||||
# project and boxes intact — still RUNNING, because revoking a person does not
|
||||
# kill their workloads. Deleting them is irreversible and is not a convergence
|
||||
# step: an edit to the users file must never destroy someone's machines behind
|
||||
# an admin's back. `box revoke <user> --purge`, run deliberately, is where that
|
||||
# lives.
|
||||
#
|
||||
# The absent-group case needs no guard of its own: `id -nG` cannot report a
|
||||
# group that does not exist, so every caller's `in_group` test is already false
|
||||
# on a host=no box or one where `box setup-host` never ran — there is nothing
|
||||
# to revoke, and apply says nothing and moves on.
|
||||
drop_incus() { # drop_incus <user> — hand group 'incus' back to box
|
||||
local u="$1"
|
||||
if command -v box >/dev/null 2>&1; then
|
||||
# Don't trust the exit code — prove the effective state (the #12 lesson,
|
||||
# the same one bootstrap applies to box's installer). A revoke that exits
|
||||
# 0 having left the membership in place would otherwise be reported as a
|
||||
# removal; the membership is what closes the socket, so it gets checked.
|
||||
if box revoke "$u" && ! in_group "$u" incus; then
|
||||
log "removed $u from incus (via 'box revoke' — box owns that group)"
|
||||
return 0
|
||||
fi
|
||||
warn "'box revoke $u' did not remove the incus group — removing it directly; check box on this host"
|
||||
fi
|
||||
if in_group "$u" incus; then
|
||||
gpasswd -d "$u" incus >/dev/null
|
||||
log "removed $u from incus"
|
||||
fi
|
||||
# box's warning, in rig's voice, because rig is the one that took the group
|
||||
# here. pgrep decides whether it applies; if pgrep is absent rig cannot tell,
|
||||
# and an unnecessary warning costs an operator one command while a missing
|
||||
# one costs them a wrong belief about who can reach the daemon.
|
||||
if ! command -v pgrep >/dev/null 2>&1 || pgrep -u "$u" >/dev/null 2>&1; then
|
||||
warn "$u may hold live sessions, and group membership is read at login — those sessions keep the Incus socket until they end. To end them now: loginctl terminate-user $u"
|
||||
fi
|
||||
}
|
||||
|
||||
# --- converge each user ------------------------------------------------------
|
||||
for u in "${USERS[@]}"; do
|
||||
if ! id -u "$u" >/dev/null 2>&1; then
|
||||
|
|
@ -263,8 +320,12 @@ for u in "${USERS[@]}"; do
|
|||
fi ;;
|
||||
*)
|
||||
if in_group "$u" "$g"; then
|
||||
gpasswd -d "$u" "$g" >/dev/null
|
||||
log "removed $u from $g"
|
||||
if [ "$g" = incus ]; then
|
||||
drop_incus "$u"
|
||||
else
|
||||
gpasswd -d "$u" "$g" >/dev/null
|
||||
log "removed $u from $g"
|
||||
fi
|
||||
CHANGED=1
|
||||
fi ;;
|
||||
esac
|
||||
|
|
@ -324,8 +385,19 @@ if [ -r "$LEDGER" ]; then
|
|||
if [ -f "$prevhome/.ssh/authorized_keys" ]; then
|
||||
mv "$prevhome/.ssh/authorized_keys" "$prevhome/.ssh/authorized_keys.revoked-by-rig"
|
||||
fi
|
||||
# incus goes back through box here too — a user dropped from the file
|
||||
# entirely is exactly the offboarding this warning was written for, and it
|
||||
# would be perverse for the full revocation to be the quiet one. It fires
|
||||
# once, on the transition: the membership is gone on the next run, so
|
||||
# in_group is false and an already-revoked user stays a clean no-op.
|
||||
for g in rig-admin rig incus; do
|
||||
if in_group "$prev" "$g"; then gpasswd -d "$prev" "$g" >/dev/null; fi
|
||||
if in_group "$prev" "$g"; then
|
||||
if [ "$g" = incus ]; then
|
||||
drop_incus "$prev"
|
||||
else
|
||||
gpasswd -d "$prev" "$g" >/dev/null
|
||||
fi
|
||||
fi
|
||||
done
|
||||
REVOKED+=("$prev")
|
||||
# Warn on the TRANSITION only: an already-revoked user is converged above
|
||||
|
|
|
|||
150
test/cli.sh
150
test/cli.sh
|
|
@ -814,6 +814,156 @@ check "users apply: the incus want is gated on the host= verdict, not just the g
|
|||
check "users apply: a marker/reality mismatch warns and names the repair" 0 "" \
|
||||
grep -q "marker and this box's reality disagree" "$ROOT/commands/users-apply.sh"
|
||||
|
||||
# --- dropping role box goes through box, not behind its back (#50) -----------
|
||||
# The 'incus' group is box's: box's setup-host creates it and `box revoke`
|
||||
# takes it back, warning that supplementary groups are read at LOGIN so a
|
||||
# session the user already holds keeps the socket until it dies. rig's old bare
|
||||
# `gpasswd -d` took the group and said nothing, so an operator watching apply
|
||||
# succeed believed VM access had ended when it had not.
|
||||
#
|
||||
# Both removal paths route through one function, so both are covered by the one
|
||||
# set of assertions below.
|
||||
check "users apply: both removal paths route incus through drop_incus" 0 "" \
|
||||
test "$(grep -c 'drop_incus "\$' "$ROOT/commands/users-apply.sh")" -eq 2
|
||||
# Convergence removes access, never someone's running machines: '--purge'
|
||||
# deletes the user's boxes, images and project, and must stay an explicit admin
|
||||
# act. Asserted by the SHAPE of the one line that invokes box — a bare revoke
|
||||
# whose effective state is then checked — rather than by grepping the file for
|
||||
# '--purge', which the rationale comments and --help text mention on purpose,
|
||||
# to say where it does belong. The captures below prove the same thing at
|
||||
# runtime, on every path.
|
||||
# shellcheck disable=SC2016 # the literal source line is the pattern, unexpanded
|
||||
check "users apply: the box invocation is a bare revoke" 0 "" \
|
||||
grep -qF 'if box revoke "$u" && ! in_group "$u" incus; then' \
|
||||
"$ROOT/commands/users-apply.sh"
|
||||
|
||||
# drop_incus is exercised, not argued about. users-apply.sh EXECUTES when
|
||||
# sourced (and dies at the root check), so the function is lifted out of the
|
||||
# real file verbatim — column-0 'drop_incus() {' through column-0 '}' — and
|
||||
# driven against stub log/warn/in_group and a stub PATH holding only box,
|
||||
# gpasswd and pgrep. The extraction is asserted first: if that shape ever
|
||||
# changes the lift comes back empty and every case below fails loudly rather
|
||||
# than passing vacuously.
|
||||
DROP_FN="$(sed -n '/^drop_incus() {/,/^}/p' "$ROOT/commands/users-apply.sh")"
|
||||
# shellcheck disable=SC2016 # $1 is the inner bash -c's positional, deliberately
|
||||
check "users apply: drop_incus lifts out of the real file whole" 0 "" \
|
||||
bash -c '[ -n "$1" ] && printf %s "$1" | grep -q "^}$"' _ "$DROP_FN"
|
||||
|
||||
# The driving shell is named by absolute path: PATH below is REPLACED by the
|
||||
# stub directory (not prefixed) so that 'absent' means absent even on a host
|
||||
# that really has box installed — which also puts bash itself out of reach of
|
||||
# a PATH lookup.
|
||||
BASH_BIN="$(command -v bash)"
|
||||
# drive_drop <ok|hollow|fail|absent> — run the real drop_incus against stubs.
|
||||
# 'ok': box revokes and the membership is gone afterwards. 'hollow': box exits
|
||||
# 0 and leaves the membership standing (the #12 lesson — an exit code is not
|
||||
# effective state). 'fail': box exits non-zero. 'absent': no box on the host.
|
||||
drive_drop() {
|
||||
local mode="$1" d
|
||||
d="$(mktemp -d)"
|
||||
mkdir -p "$d/bin"
|
||||
: > "$d/member" # 'dan is in incus' — removed when taken
|
||||
# The stub reports on STDERR: the real call is 'gpasswd -d ... >/dev/null',
|
||||
# so a stub that spoke on stdout would be silenced by the code under test and
|
||||
# every fallback assertion below would pass vacuously.
|
||||
# Each stub restores a real PATH for itself: the caller's PATH is REPLACED by
|
||||
# the stub dir (that is what makes 'absent' mean absent), which would other-
|
||||
# wise leave the stubs unable to find 'rm'.
|
||||
cat > "$d/bin/gpasswd" <<EOF
|
||||
#!/bin/sh
|
||||
PATH=/usr/bin:/bin
|
||||
echo "CALL: gpasswd \$*" >&2
|
||||
rm -f "$d/member"
|
||||
EOF
|
||||
printf '%s\n' '#!/bin/sh' 'exit 0' > "$d/bin/pgrep" # dan holds a session
|
||||
if [ "$mode" != absent ]; then
|
||||
cat > "$d/bin/box" <<EOF
|
||||
#!/bin/sh
|
||||
PATH=/usr/bin:/bin
|
||||
echo "CALL: box \$*"
|
||||
case "$mode" in
|
||||
ok) rm -f "$d/member" ;;
|
||||
hollow) : ;;
|
||||
fail) exit 1 ;;
|
||||
esac
|
||||
EOF
|
||||
fi
|
||||
chmod +x "$d"/bin/*
|
||||
# shellcheck disable=SC2016 # $MEMBER/$* resolve inside the driving shell
|
||||
MEMBER="$d/member" PATH="$d/bin" "$BASH_BIN" -c '
|
||||
set -euo pipefail
|
||||
log() { printf "rig-users: %s\n" "$*"; }
|
||||
warn() { printf "rig-users: WARNING: %s\n" "$*" >&2; }
|
||||
in_group() { [ -e "$MEMBER" ]; }
|
||||
'"$DROP_FN"'
|
||||
drop_incus dan' 2>&1
|
||||
rm -rf "$d"
|
||||
}
|
||||
DROP_OK="$(drive_drop ok)"
|
||||
DROP_HOLLOW="$(drive_drop hollow)"
|
||||
DROP_FAIL="$(drive_drop fail)"
|
||||
DROP_ABSENT="$(drive_drop absent)"
|
||||
in_out() { printf '%s' "$1" | grep -qF -e "$2"; } # in_out <captured> <substr>
|
||||
|
||||
# The happy path: box takes its own group back, bare, and rig does not reach
|
||||
# for gpasswd behind it.
|
||||
check "drop_incus: calls 'box revoke <user>'" 0 "" in_out "$DROP_OK" "CALL: box revoke dan"
|
||||
# Bare on EVERY path box is reached on, not just the one that works: a retry
|
||||
# or a fallback must never escalate to the destructive verb.
|
||||
check "drop_incus: the box call is bare — no --purge" 1 "" in_out "$DROP_OK" "--purge"
|
||||
check "drop_incus: a hollow success never retries with --purge" 1 "" in_out "$DROP_HOLLOW" "--purge"
|
||||
check "drop_incus: a failed revoke never retries with --purge" 1 "" in_out "$DROP_FAIL" "--purge"
|
||||
check "drop_incus: box's success needs no gpasswd" 1 "" in_out "$DROP_OK" "CALL: gpasswd"
|
||||
check "drop_incus: names box as the one that revoked" 0 "" in_out "$DROP_OK" "via 'box revoke'"
|
||||
|
||||
# Effective state, not exit codes: a revoke that returns 0 with the membership
|
||||
# still standing has not closed the socket, and apply must not report that it
|
||||
# has.
|
||||
check "drop_incus: a hollow box success is caught" 0 "" \
|
||||
in_out "$DROP_HOLLOW" "did not remove the incus group"
|
||||
check "drop_incus: a hollow box success falls back to gpasswd" 0 "" \
|
||||
in_out "$DROP_HOLLOW" "CALL: gpasswd -d dan incus"
|
||||
check "drop_incus: a hollow box success never claims box did it" 1 "" \
|
||||
in_out "$DROP_HOLLOW" "via 'box revoke'"
|
||||
check "drop_incus: a failing box revoke falls back to gpasswd" 0 "" \
|
||||
in_out "$DROP_FAIL" "CALL: gpasswd -d dan incus"
|
||||
|
||||
# No box on the host: rig takes the group itself and carries box's warning,
|
||||
# because the silence is the bug — an operator must not read "removed" as
|
||||
# "their sessions are gone too".
|
||||
check "drop_incus: no box on PATH still removes the group" 0 "" \
|
||||
in_out "$DROP_ABSENT" "CALL: gpasswd -d dan incus"
|
||||
check "drop_incus: the fallback warns that groups are read at login" 0 "" \
|
||||
in_out "$DROP_ABSENT" "group membership is read at login"
|
||||
check "drop_incus: the fallback hands over the remedy" 0 "" \
|
||||
in_out "$DROP_ABSENT" "loginctl terminate-user dan"
|
||||
# Every fallback path carries it, not just the box-less one.
|
||||
check "drop_incus: the hollow-success fallback warns too" 0 "" \
|
||||
in_out "$DROP_HOLLOW" "loginctl terminate-user dan"
|
||||
check "drop_incus: the failed-revoke fallback warns too" 0 "" \
|
||||
in_out "$DROP_FAIL" "loginctl terminate-user dan"
|
||||
# box only warns when the user has live processes, and rig mirrors that — but
|
||||
# an absent pgrep means rig cannot tell, and a wrong belief about who reaches
|
||||
# the daemon costs more than one unnecessary command. Proven by running the
|
||||
# fallback with a PATH that has no pgrep at all.
|
||||
NOPGREP_D="$(mktemp -d)"
|
||||
mkdir -p "$NOPGREP_D/bin"
|
||||
printf '%s\n' '#!/bin/sh' 'PATH=/usr/bin:/bin' 'echo "CALL: gpasswd $*" >&2' \
|
||||
"rm -f $NOPGREP_D/member" > "$NOPGREP_D/bin/gpasswd"
|
||||
chmod +x "$NOPGREP_D/bin/gpasswd"
|
||||
: > "$NOPGREP_D/member"
|
||||
# shellcheck disable=SC2016 # $MEMBER/$* resolve inside the driving shell
|
||||
DROP_NOPGREP="$(MEMBER="$NOPGREP_D/member" PATH="$NOPGREP_D/bin" "$BASH_BIN" -c '
|
||||
set -euo pipefail
|
||||
log() { printf "rig-users: %s\n" "$*"; }
|
||||
warn() { printf "rig-users: WARNING: %s\n" "$*" >&2; }
|
||||
in_group() { [ -e "$MEMBER" ]; }
|
||||
'"$DROP_FN"'
|
||||
drop_incus dan' 2>&1)"
|
||||
check "drop_incus: an absent pgrep warns rather than guessing" 0 "" \
|
||||
in_out "$DROP_NOPGREP" "loginctl terminate-user dan"
|
||||
rm -rf "$NOPGREP_D"
|
||||
|
||||
# --- users close-root: the human-class root-door shutter ---------------------
|
||||
check "users close-root: --help exits 0" 0 "usage:" "$ROOT/commands/users-close-root.sh" --help
|
||||
check "users close-root: unknown flag exits 2" 2 "unknown flag" "$ROOT/commands/users-close-root.sh" --nope
|
||||
|
|
|
|||
Loading…
Reference in a new issue