revoke/grant: survive the live-session case — the review's one real hole (#74)

Supplementary groups are read at LOGIN, so 'gpasswd -d' does nothing to a
session the user already holds — and after --purge, a stale-group process
could touch incus-user and lazily RECREATE the project with stock defaults:
the unhardened NAT bridge, un-narrowed, strictly worse than the granted
state. Adversarial review caught it; verified live, then closed:

- revoke --purge terminates the user's sessions first (loginctl, then
  pkill), and refuses to purge under processes it cannot kill
- bare revoke says out loud that held sessions keep the socket until they
  end, and names the loginctl command — instead of claiming a lockout it
  did not deliver (help/README/design doc reworded to match)
- a failed grant backs out its own group-add on exit (trap, disarmed on
  success): no half-granted user holding an un-narrowed socket while the
  admin reads the error. Verified by injecting a bad profile YAML
- the rehearsal now holds a session open across the purge and demands it
  dies with the tier (criterion l, 42nd check)

Smaller review findings, same pass: the escape-hatch probes assert the
refusal's REASON instead of any nonzero exit (an image hiccup must not read
as 'the escape is closed'); probe_from maps an outer-timeout kill to
dropped, not reachable; the rehearsal cleanup keeps the account when a purge
fails so doctor can name the leftovers; the purge asserts the trust
certificate's absence; cmd_new distinguishes a dead daemon from a missing
stack before prescribing setup-host; grant's success message names the
user-<uid> bridge variant correctly on big-uid hosts.

Rehearsal after: 42/42 (containers). test/cli.sh: 76 checks.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

README.md

@@ -123,8 +123,10 @@ An admin hands the tier out per user, and takes it back:
 
 ```sh
 box grant dev1              # dev1 can now: box new / list / shell / snapshot / rm — their boxes only
-box revoke dev1             # locked out; their boxes survive (grant again restores)
+box revoke dev1             # tier removed; their boxes survive (grant again restores).
-box revoke dev1 --purge     # ...or delete everything they had
+                            #   a session they already hold keeps the socket until it
+                            #   ends — revoke warns and names the loginctl command
+box revoke dev1 --purge     # ...or end their sessions and delete everything they had
 ```
 
 `grant` is an idempotent convergence, not a flag flip, because incus-user's

bin/box

@@ -448,14 +448,18 @@ existing box-to-box drop means even their instances cannot reach a sibling.
 EOF
 ;;
     revoke) cat <<'EOF'
-Take the restricted tier back from a user. Without --purge, this only removes
+Take the restricted tier back from a user. Without --purge, this removes
-them from the 'incus' group: they lose the socket, but their project and
+them from the 'incus' group: their project and boxes stay (still running!)
-boxes stay (still running!) and 'box grant' restores access untouched. With
+and 'box grant' restores access untouched. Group membership is read at
---purge, their boxes, images, project, private bridge and trust-store
+LOGIN, so a session they already hold keeps the socket until it ends —
-certificate are removed too — irreversible, so it asks first.
+revoke says so and names the loginctl command when it happens. With
+--purge, their sessions are terminated first (a stale session could quietly
+recreate the project, unhardened, afterwards — measured, not theoretical),
+then their boxes, images, project, private bridge and trust-store
+certificate are removed — irreversible, so it asks first.
 
-  box revoke dev1            # lock them out, keep their boxes
+  box revoke dev1            # take the tier; their boxes keep running
-  box revoke dev1 --purge    # ...and delete everything they had
+  box revoke dev1 --purge    # ...or end their sessions and delete everything
 EOF
 ;;
     teardown-host) cat <<'EOF'
@@ -754,7 +758,12 @@ cmd_new() {
   # for an admin (setup-host builds it), in YOUR project for a restricted user
   # (box grant converges it). Its absence has a different fix per tier, and
   # incus's own "Profile not found" at launch time names neither.
-  if [ -z "$remote" ] && ! incus profile show box-net >/dev/null 2>&1 </dev/null; then
+  if [ -z "$remote" ] && ! timeout 10 incus profile show box-net >/dev/null 2>&1 </dev/null; then
+    # A missing profile and a daemon that is not answering are different
+    # faults with different fixes — "run setup-host" at a wedged daemon
+    # (the #26 shape) is wrong advice. Separate them before diagnosing.
+    timeout 10 incus list >/dev/null 2>&1 </dev/null \
+      || die "the incus daemon is not answering — diagnose it: box doctor"
     if [ "$(box_tier)" = restricted ]; then
       die "your project has no box-net profile — the restricted tier is granted per user, by an admin: box grant $(id -un)"
     fi

docs/box-design.md

@@ -169,11 +169,17 @@ in their own project — which is why the nft bridge drop, which they cannot
 touch, exists as the second, host-owned layer. Defense in depth, both layers
 measured (`drill/multiuser.sh`, criteria a–l).
 
-`box revoke` is two strengths: bare, it removes the group — the socket
+`box revoke` is two strengths: bare, it removes the group — their boxes keep
-closes, their boxes keep *running* (revoking a person does not kill their
+*running* (revoking a person does not kill their workloads), `grant` restores
-workloads) and `grant` restores everything; `--purge` deletes their world
+everything, and because supplementary groups are read at login, revoke warns
-(boxes, images, project, private bridge, trust-store certificate) and asserts
+when live sessions keep the socket until they end (and names the `loginctl`
-the absence afterwards.
+command). `--purge` terminates those sessions *first* — a stale-group process
+could otherwise touch incus-user after the purge and lazily recreate the
+project with stock, unhardened defaults, undoing the grant's whole point —
+then deletes their world (boxes, images, project, private bridge, trust-store
+certificate) and asserts the absence afterwards. A failed `grant` backs its
+own group-add out on exit for the same reason: no half-granted user holding
+an un-narrowed socket.
 
 ## Non-goals
 

drill/multiuser.sh

@@ -79,10 +79,13 @@ as_u() { local u="$1"; shift; runuser -u "$u" -- "$@" </dev/null; }
 # down): "refused" means a packet ARRIVED and was answered — which, for an
 # isolation probe, is a failure wearing polite clothes. Silence is the pass.
 probe_from() {  # probe_from <user> <box> <url>
-  local u="$1" b="$2" url="$3" out
+  local u="$1" b="$2" url="$3" out rc
-  out="$(as_u "$u" timeout -k 5 30 incus exec "$b" -- curl -sS -m 15 -o /dev/null "$url" 2>&1)"
+  out="$(as_u "$u" timeout -k 5 30 incus exec "$b" -- curl -sS -m 15 -o /dev/null "$url" 2>&1)"; rc=$?
-  case "$out" in
+  # Silence + success is the only 'reachable': when the OUTER timeout kills a
-    "") echo reachable ;;
+  # wedged exec (the #26 shape), curl never spoke — empty output with rc 124
+  # must not read as an open door.
+  case "$rc:$out" in
+    0:) echo reachable ;;
     *Connection\ refused*) echo refused ;;
     *) echo dropped ;;
   esac
@@ -104,9 +107,15 @@ cleanup() {
   echo "── cleanup"
   for u in "$U1" "$U2"; do
     id "$u" >/dev/null 2>&1 || continue
-    BOX_YES=1 box revoke "$u" --purge >/dev/null 2>&1
+    # A half-failed purge followed by userdel leaves a project owned by
+    # nobody — and doctor's leftover check keys on the USER existing. Keep
+    # the user when the purge fails, and name what survived.
+    if BOX_YES=1 box revoke "$u" --purge >/dev/null 2>&1; then
       userdel -r "$u" >/dev/null 2>&1
       id "$u" >/dev/null 2>&1 && echo "  WARNING: user $u still exists" || echo "  removed $u (tier, boxes, account)"
+    else
+      echo "  WARNING: purge FAILED for $u — kept the account so 'box doctor' can name it; project user-$(id -u "$u") may survive"
+    fi
   done
 }
 trap cleanup EXIT
@@ -238,15 +247,29 @@ v6="$(as_u "$U1" timeout -k 5 20 incus exec mine -- sh -c 'ip -6 addr show dev e
              || no "(g) the box holds a global IPv6 address — an uncovered egress path"
 
 phase "h. the escape hatches, tried and refused"
-as_u "$U1" incus launch images:debian/13 esc --network "incusbr-$uid1" >/dev/null 2>&1 \
+# Each probe asserts the refusal's REASON, not just a nonzero exit — an image
-  && { no "(h) $U1 attached the unhardened private bridge"; as_u "$U1" incus delete -f esc >/dev/null 2>&1; } \
+# server hiccup or a name collision also exits nonzero, and reading that as
-  || ok "(h) attaching the private incusbr-$uid1 is refused (not in restricted.networks.access)"
+# "the escape is closed" is a false verdict wearing a green light (the drill
-as_u "$U1" incus project set "$p1" restricted.networks.access "boxnet,incusbr-$uid1" >/dev/null 2>&1 \
+# has relearned this enough times to earn a rule).
-  && no "(h) $U1 widened their OWN project's network access" \
+out="$(as_u "$U1" incus launch images:debian/13 esc --network "incusbr-$uid1" 2>&1)"; rc=$?
-  || ok "(h) a restricted certificate cannot widen its own project"
+if [ "$rc" -ne 0 ] && printf '%s' "$out" | grep -qiE 'not found|not allowed'; then
-as_u "$U1" incus network set boxnet dns.mode=managed >/dev/null 2>&1 \
+  ok "(h) attaching the private incusbr-$uid1 is refused (not in restricted.networks.access)"
-  && no "(h) $U1 edited boxnet itself" \
+else
-  || ok "(h) boxnet's config refuses a restricted certificate"
+  no "(h) private-bridge attach: rc=$rc, said: $(printf '%s' "$out" | head -1)"
+  as_u "$U1" incus delete -f esc >/dev/null 2>&1
+fi
+out="$(as_u "$U1" incus project set "$p1" restricted.networks.access "boxnet,incusbr-$uid1" 2>&1)"; rc=$?
+if [ "$rc" -ne 0 ] && printf '%s' "$out" | grep -qi 'restricted'; then
+  ok "(h) a restricted certificate cannot widen its own project"
+else
+  no "(h) project-widen attempt: rc=$rc, said: $(printf '%s' "$out" | head -1)"
+fi
+out="$(as_u "$U1" incus network set boxnet dns.mode=managed 2>&1)"; rc=$?
+if [ "$rc" -ne 0 ] && printf '%s' "$out" | grep -qiE 'permission|restricted'; then
+  ok "(h) boxnet's config refuses a restricted certificate"
+else
+  no "(h) boxnet edit attempt: rc=$rc, said: $(printf '%s' "$out" | head -1)"
+fi
 
 phase "e/f. the honest refusals — expose, setup-host, doctor"
 out="$(as_u "$U1" box expose mine 3000 2>&1)"; rc=$?
@@ -275,7 +298,16 @@ acc="$(incus project get "$p1" restricted.networks.access 2>/dev/null)"
 aud "k. incus-user re-sync: convergence intact (matches its source: setup runs only at project creation)"
 
 phase "l. revoke — one user out, the other untouched"
+# Revocation's hard case is a user who is LOGGED IN: groups are read at
+# login, so a held session keeps the socket — and after a purge it could
+# touch incus-user and recreate the project with stock, unhardened defaults.
+# Hold a session open across the purge and demand it dies with the tier.
+runuser -u "$U2" -- sleep 300 </dev/null >/dev/null 2>&1 &
+sleep 1
 BOX_YES=1 box revoke "$U2" --purge >/dev/null 2>&1 && ok "(l) box revoke $U2 --purge exits 0" || no "(l) revoke failed"
+pgrep -u "$U2" >/dev/null 2>&1 \
+  && no "(l) $U2 still has live processes after the purge — a stale session could recreate their project, unhardened" \
+  || ok "(l) the purge terminated $U2's held session (no stale-group path back in)"
 as_u "$U2" incus list >/dev/null 2>&1 \
   && no "(l) $U2 still reaches the daemon after revoke" \
   || ok "(l) $U2 is locked out"

host/grant-user.sh

@@ -85,10 +85,28 @@ fi
 # 1. The group. 'incus' is the restricted socket; membership takes effect at
 # the user's next login, but run_as below starts a fresh process with the
 # database's groups, so the grant itself never waits on a re-login.
+#
+# If THIS run granted the group and a later step fails, take it back on the
+# way out: a half-granted user would otherwise hold live socket access to an
+# UN-NARROWED project — the stock unhardened bridge attachable — until an
+# admin re-runs. Backing out the group closes that window completely for a
+# fresh grant (their existing sessions predate the membership, so no process
+# holds it yet). A user who was already in the group keeps it: not ours to
+# take on a re-run's failure.
+added_group=0
+backout() {
+  if [ "$added_group" -eq 1 ]; then
+    $SUDO gpasswd -d "$user" incus >/dev/null 2>&1 || true
+    echo "box grant: FAILED — removed $user from 'incus' again (no half-granted access left behind); fix the cause and re-run" >&2
+  fi
+}
+trap backout EXIT
+
 if id -nG "$user" | tr ' ' '\n' | grep -qx incus; then
   echo "group: $user already in 'incus'"
 else
   $SUDO usermod -aG incus "$user"
+  added_group=1
   echo "group: added $user to 'incus' (their next login picks it up; the grant does not wait)"
 fi
 
@@ -129,7 +147,12 @@ if ! err="$(incus project set "$project" restricted.networks.access boxnet 2>&1
   echo "  (an instance still on the private bridge blocks this — move or delete it, then re-run)" >&2
   exit 1
 fi
-echo "network: $project restricted to boxnet (the private incusbr-$uid is unreferenced and unreachable)"
+# The private bridge's name follows incus-user's own rule (revoke-user.sh
+# mirrors it too): incusbr-<uid>, or user-<uid> when that would not fit an
+# interface name — naming the wrong one here would be a true claim with the
+# wrong noun on big-uid (SSSD/AD) hosts.
+bridge="incusbr-$uid"; [ "${#bridge}" -gt 15 ] && bridge="user-$uid"
+echo "network: $project restricted to boxnet (the private $bridge is unreferenced and unreachable)"
 
 # 5. Snapshots. incus-user projects block them by default, and box's whole
 # reuse story — log in once, snapshot, clone forever — is snapshots.
@@ -150,5 +173,6 @@ echo "profile: box-net installed in $project"
 run_as "$user" timeout 30 incus profile show box-net >/dev/null 2>&1 \
   || { echo "box grant: converged, but $user cannot see the box-net profile through incus-user — check journalctl -u incus-user" >&2; exit 1; }
 
+trap - EXIT   # converged and verified: the grant stands
 echo "granted: $user has the restricted tier — their 'box new' lands on the hardened boxnet."
 echo "         (their boxes are theirs alone; 'box revoke $user' takes the tier back)"

host/revoke-user.sh

@@ -59,11 +59,39 @@ fi
 # The group, first — access ends even if a purge step below trips.
 if id -nG "$user" | tr ' ' '\n' | grep -qx incus; then
   $SUDO gpasswd -d "$user" incus >/dev/null
-  echo "group: removed $user from 'incus' — the socket closes with their next login"
+  echo "group: removed $user from 'incus'"
 else
   echo "group: $user was not in 'incus'"
 fi
 
+# Supplementary groups are fixed AT LOGIN: the database change above does
+# nothing to a session the user already holds — a leftover tmux keeps the
+# socket until it dies. For a bare revoke that is an honest warning. For
+# --purge it is a hole: a stale-group process can touch incus-user AFTER the
+# purge and lazily recreate the project with incus-user's stock defaults —
+# the unhardened NAT bridge, un-narrowed — which is strictly worse than the
+# granted state this script is unwinding. So --purge terminates the user's
+# sessions first (it is already the destructive, confirmed path), and a bare
+# revoke says out loud what it did not do.
+if pgrep -u "$user" >/dev/null 2>&1; then
+  if [ "$purge" -eq 1 ]; then
+    echo "sessions: $user has live processes — terminating them (a stale session could recreate the project, unhardened, after the purge)"
+    $SUDO loginctl terminate-user "$user" 2>/dev/null || true
+    $SUDO pkill -u "$user" 2>/dev/null || true
+    sleep 1
+    $SUDO pkill -9 -u "$user" 2>/dev/null || true
+    if pgrep -u "$user" >/dev/null 2>&1; then
+      echo "box revoke: could not terminate $user's processes — refusing to purge under them" >&2
+      echo "            (they retain the socket until those sessions end, and could recreate the project)" >&2
+      exit 1
+    fi
+  else
+    echo "WARNING: $user has live sessions, and group membership is read at login —"
+    echo "         those sessions keep the socket until they end. To end them now:"
+    echo "         sudo loginctl terminate-user $user"
+  fi
+fi
+
 if [ "$purge" -eq 0 ]; then
   if incus project show "$project" >/dev/null 2>&1 </dev/null; then
     echo "kept: project $project and its boxes (still running — revoking a person does not kill their workloads)"
@@ -113,9 +141,13 @@ if [ -d "/var/lib/incus/users/$uid" ]; then
 fi
 
 # Assert absence rather than trusting exit codes — the wipe.sh discipline.
+# The certificate included: its removal above is set -e-exempt (left of &&),
+# and a promise the header makes is a promise this block checks.
 leftover=""
 incus project show "$project" >/dev/null 2>&1 </dev/null && leftover="$leftover $project"
 incus network show "$bridge" >/dev/null 2>&1 </dev/null && leftover="$leftover $bridge"
+incus config trust list --format csv --columns nf 2>/dev/null | grep -q "^incus-user-$uid," \
+  && leftover="$leftover cert:incus-user-$uid"
 if [ -n "$leftover" ]; then
   echo "box revoke: purge INCOMPLETE — still present:$leftover" >&2
   exit 1

test/cli.sh

@@ -217,6 +217,23 @@ check "grant: refuses an incus-admin member (nothing tighter to grant)" 0 "" \
   grep -qF 'incus-admin' "$ROOT/host/grant-user.sh"
 check "revoke: group removal is the lockout" 0 "" \
   grep -qF 'gpasswd -d' "$ROOT/host/revoke-user.sh"
+# Group membership is read at login: purge must terminate live sessions (a
+# stale-group process could recreate the project unhardened AFTER the purge),
+# and a bare revoke must say the socket survives in held sessions.
+check "revoke: purge terminates live sessions first" 0 "" \
+  grep -qF 'loginctl terminate-user' "$ROOT/host/revoke-user.sh"
+check "revoke: purge refuses under unkillable sessions" 0 "" \
+  grep -qF 'refusing to purge under them' "$ROOT/host/revoke-user.sh"
+check "revoke: bare revoke warns about held sessions" 0 "" \
+  grep -qF 'live sessions' "$ROOT/host/revoke-user.sh"
+check "revoke: the purge asserts the certificate's absence too" 0 "" \
+  bash -c 'grep -A6 "Assert absence" "'"$ROOT"'/host/revoke-user.sh" | grep -q "config trust list"'
+# A failed grant must not leave a half-granted user: if THIS run added the
+# group, the exit path takes it back (and the trap disarms only on success).
+check "grant: backs out its own group-add on failure" 0 "" \
+  grep -qF 'trap backout EXIT' "$ROOT/host/grant-user.sh"
+check "grant: the back-out disarms on success" 0 "" \
+  grep -qF 'trap - EXIT' "$ROOT/host/grant-user.sh"
 # shellcheck disable=SC2016  # the $-strings are literals in the target file
 check "revoke: purge deletes instances one at a time" 0 "" \
   grep -qF 'delete -f "$inst"' "$ROOT/host/revoke-user.sh"