heavy-duty/rig

Fork 1

feat: users apply grants the box tier, not just the socket #53

Merged

dan-claude-bot merged 1 commit from feat/users-apply-grants-tier into main

2026-07-19 19:12:10 +00:00

dan-claude-bot commented

2026-07-19 16:16:36 +00:00

(Migrated from github.com)

The gap: the group is the socket, the tier is `box grant`

Role box in the users file resolved to exactly one action
(commands/users-apply.sh:213):

case ",$roles," in *,box,*) if [ "$INCUS_OK" -eq 1 ]; then want="$want incus"; fi ;; esac

usermod -aG incus <user> — step 1 of the 5 box grant performs. The
other four are the tier: the user-<uid> project, its narrowing to boxnet
and only boxnet, the snapshot and backup allowances box clone and
box export ride, and the shipped box-net profile installed into that
project. Without them the user's first box new fails closed at
box/bin/box:749 ("your project has no box-net profile"), so apply's promise
— the users file is the fleet's source of truth, apply converges to it — was
not kept for this role.

And the half-grant was worse than absent: until an admin ran box grant by
hand, the user held an incus membership with no converged project, so
incus-user would lazily hand them a stock unhardened incusbr-<uid> NAT
bridge — a door box's own contract forbids.

The fix

On host=yes, apply calls box grant <user> for each user carrying role
box. rig calls box's grant rather than reimplementing four fifths of it:
the "rig never installs Incus" boundary is about installation, not
invocation, and grant is already script-callable — idempotent, root-or-sudo
(which apply already is), stdin-pinned so no incus client can wedge on a
prompt, and it does its own run-as-the-user touch, so the granted user never
has to log in first.

Ordering. The call sits after useradd (grant opens with a
getent passwd at grant-user.sh:62 and refuses an unknown account) and
after the other group work, so a user whose grant fails still lands with
everything rig owns outright. Pinned by a line-number ordering assert, the
same shape as bootstrap's marker-then-box check.

Failure granularity, split the way the host= guard beside it already
splits:

fact	verdict
`host=yes`, box-role users, no `box` CLI on PATH	die — same register as the existing missing-`incus`-group die at `:180`; a broken VM host, not a per-user accident
`box grant <user>` exits non-zero	warn and continue — "one box-role user somewhere in the fleet must not stop apply everywhere VMs don't live"
`host=no` / marker with no `host=` trait	unchanged skip-with-warning

The decision the issue left open: does rig still manage the `incus` membership?

Deliberately half: incus stays in the wanted set, but the add is
deferred to box grant on hosts where we call it.

It must stay in want, or the exact-convergence loop's other arm would
strip a box-role user's socket on the very run that granted it. (That arm
is #50's; this PR does not touch it.)
The add must be deferred, because grant's fail-closed backout only reaches
a membership that run added — a user it finds already in incus keeps
it when a later step trips, by design. If rig opens the socket first, grant
can no longer close it, and a grant that fails midway leaves the user with
live access to an un-narrowed project. Deferring makes the socket and the
tier land together or fail together.
And grant is the authority on whether the group belongs at all: for an
incus-admin member it deliberately does not add incus. rig adding it
anyway would not oscillate, but it would be rig overruling box on box's own
tier — the exact boundary this change exists to respect.

Both halves are asserted, since either alone is a bug, plus an ordering check
that the deferral sits on the add arm and never the removal arm.

Blocked-on note: `incus-admin` members (heavy-duty/box#99)

box grant refuses incus-admin members outright today
(grant-user.sh:69). That refusal is what heavy-duty/box#99 fixes, and it
has not merged. This PR does not block on it and does not special-case
box's refusal into a skip either — it calls grant unconditionally and, when
the call fails and the user is in incus-admin, warns with the specific
story: they keep the full socket (incus-admin is strictly stronger than the
tier), but they get no user-<uid> project of their own and land in the
shared default project alongside every other admin; blocked on
heavy-duty/box#99; a later apply picks the tier up once box stops refusing.
When #99 lands, grant returns 0 and that branch simply stops being reached —
no rig change needed.

Verification

shellcheck -x over the full CI sweep (bin/* + **/*.sh, 22 files) —
clean.
bash test/cli.sh — 336 passed, 0 failed (325 on main; 11 new).
bash test/release.sh — 41 passed, 0 failed.

New coverage in test/cli.sh: the call exists and is box grant, not a bare
usermod; it runs after useradd; the missing-CLI die and the per-user warn
both present; the grant is gated on host=yes (asserted against the guard's
own block, so a host=yes match borrowed from the die cannot pass it for
free); the incus-admin branch exists and cites the box-side blocker; the
deferral exists, incus still enters the wanted set, and the deferral sits
above the removal arm; and rig still never apt-installs incus.

Executed end-to-end, since the greps alone would not settle behaviour: a
throwaway mount namespace with a tmpfs /etc, real useradd, and a stub
box on PATH that succeeds for one user and exits 1 for another.

fresh run: alice (admin,box) granted, group transition reported, CHANGED
set; bob (box) grant fails → warning, bob keeps account + keys and is
not left holding a bare incus membership; carol (rig) untouched;
apply exit 0.
identical re-run: both grants re-invoked (convergent, and there is no cheap
"already granted?" probe), and apply still reports "already converged; no
changes" — the grant does not manufacture a phantom change every run.
host=no: zero box invocations, prior behaviour intact.
host=yes with no box on PATH: dies with the new message.

Closes #49

🤖 Generated with Claude Code

## The gap: the group is the socket, the tier is `box grant` Role `box` in the users file resolved to exactly one action (`commands/users-apply.sh:213`): ```sh case ",$roles," in *,box,*) if [ "$INCUS_OK" -eq 1 ]; then want="$want incus"; fi ;; esac ``` `usermod -aG incus <user>` — step **1 of the 5** `box grant` performs. The other four are the tier: the `user-<uid>` project, its narrowing to `boxnet` and *only* `boxnet`, the snapshot and backup allowances `box clone` and `box export` ride, and the shipped `box-net` profile installed into that project. Without them the user's first `box new` fails closed at `box/bin/box:749` ("your project has no box-net profile"), so apply's promise — the users file is the fleet's source of truth, apply converges to it — was not kept for this role. And the half-grant was worse than absent: until an admin ran `box grant` by hand, the user held an `incus` membership with no converged project, so incus-user would lazily hand them a stock unhardened `incusbr-<uid>` NAT bridge — a door box's own contract forbids. ## The fix On `host=yes`, apply calls `box grant <user>` for each user carrying role `box`. rig calls box's grant rather than reimplementing four fifths of it: the "rig never installs Incus" boundary is about *installation*, not invocation, and grant is already script-callable — idempotent, root-or-sudo (which apply already is), stdin-pinned so no incus client can wedge on a prompt, and it does its own run-as-the-user touch, so the granted user never has to log in first. **Ordering.** The call sits *after* `useradd` (grant opens with a `getent passwd` at `grant-user.sh:62` and refuses an unknown account) and after the other group work, so a user whose grant fails still lands with everything rig owns outright. Pinned by a line-number ordering assert, the same shape as bootstrap's marker-then-box check. **Failure granularity**, split the way the `host=` guard beside it already splits: | fact | verdict | |---|---| | `host=yes`, box-role users, no `box` CLI on PATH | **die** — same register as the existing missing-`incus`-group die at `:180`; a broken VM host, not a per-user accident | | `box grant <user>` exits non-zero | **warn and continue** — "one box-role user somewhere in the fleet must not stop apply everywhere VMs don't live" | | `host=no` / marker with no `host=` trait | unchanged skip-with-warning | ## The decision the issue left open: does rig still manage the `incus` membership? Deliberately **half**: `incus` stays in the wanted set, but the *add* is deferred to `box grant` on hosts where we call it. - It must stay in `want`, or the exact-convergence loop's other arm would strip a box-role user's socket on the very run that granted it. (That arm is #50's; this PR does not touch it.) - The add must be deferred, because grant's fail-closed backout only reaches a membership **that run** added — a user it finds already in `incus` keeps it when a later step trips, by design. If rig opens the socket first, grant can no longer close it, and a grant that fails midway leaves the user with live access to an *un-narrowed* project. Deferring makes the socket and the tier land together or fail together. - And grant is the authority on whether the group belongs at all: for an `incus-admin` member it deliberately does **not** add `incus`. rig adding it anyway would not oscillate, but it would be rig overruling box on box's own tier — the exact boundary this change exists to respect. Both halves are asserted, since either alone is a bug, plus an ordering check that the deferral sits on the add arm and never the removal arm. ## Blocked-on note: `incus-admin` members (heavy-duty/box#99) `box grant` refuses `incus-admin` members outright today (`grant-user.sh:69`). That refusal is what heavy-duty/box#99 fixes, and it has **not** merged. This PR does not block on it and does not special-case box's refusal into a skip either — it calls grant unconditionally and, when the call fails *and* the user is in `incus-admin`, warns with the specific story: they keep the full socket (`incus-admin` is strictly stronger than the tier), but they get no `user-<uid>` project of their own and land in the shared default project alongside every other admin; blocked on heavy-duty/box#99; a later apply picks the tier up once box stops refusing. When #99 lands, grant returns 0 and that branch simply stops being reached — **no rig change needed**. ## Verification - `shellcheck -x` over the full CI sweep (`bin/*` + `**/*.sh`, 22 files) — clean. - `bash test/cli.sh` — **336 passed, 0 failed** (325 on `main`; 11 new). - `bash test/release.sh` — **41 passed, 0 failed**. New coverage in `test/cli.sh`: the call exists and is `box grant`, not a bare `usermod`; it runs after `useradd`; the missing-CLI die and the per-user warn both present; the grant is gated on `host=yes` (asserted against the guard's own block, so a `host=yes` match borrowed from the die cannot pass it for free); the `incus-admin` branch exists and cites the box-side blocker; the deferral exists, `incus` still enters the wanted set, and the deferral sits above the removal arm; and rig still never apt-installs incus. **Executed end-to-end**, since the greps alone would not settle behaviour: a throwaway mount namespace with a tmpfs `/etc`, real `useradd`, and a stub `box` on PATH that succeeds for one user and exits 1 for another. - fresh run: `alice` (admin,box) granted, group transition reported, `CHANGED` set; `bob` (box) grant fails → warning, `bob` keeps account + keys and is **not** left holding a bare `incus` membership; `carol` (rig) untouched; apply **exit 0**. - identical re-run: both grants re-invoked (convergent, and there is no cheap "already granted?" probe), and apply still reports **"already converged; no changes"** — the grant does not manufacture a phantom change every run. - `host=no`: **zero** `box` invocations, prior behaviour intact. - `host=yes` with no `box` on PATH: dies with the new message. Closes #49 🤖 Generated with [Claude Code](https://claude.com/claude-code)

danmt (Migrated from github.com) reviewed 2026-07-19 16:16:36 +00:00

claude-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:24:37 +00:00

claude-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve

Reviewed at head 25b63e3. Series context checked: #53, #54, and #55 are each based on main (not stacked); this PR closes #49, with the revoke-side mirror explicitly deferred to #50/#55 and bootstrap-time convergence to #51/#54. The diff matches that scoping — nothing here touches the removal arm or the ledger revocation path.

What I verified

Suites re-run locally on the PR head (worktree of refs/pull/53/head):

bash test/cli.sh — 336 passed, 0 failed (matches the PR body's claim; 11 new checks present)
bash test/release.sh — 41 passed, 0 failed
shellcheck -x over bin/* + commands/*.sh + commands/lib/*.sh + test/*.sh — clean

Independent end-to-end execution, not just the greps: a root mount namespace with tmpfs over /etc, /home, /var/log, real useradd, and a stub box on PATH that grants alice and exits 1 for bob:

host=yes fresh run: alice (admin,box) granted, group transition logged, apply reports converged; bob's failed grant warns and continues, and bob is not left holding a bare incus membership — the deferral at commands/users-apply.sh:278 really does make socket and tier fail together; carol (rig) untouched; exit 0.
identical re-run: grants re-invoked, apply still ends already converged; no changes — no phantom CHANGED (the had_socket probe at commands/users-apply.sh:308-324 works as described).
host=no: zero box invocations.
host=yes with no box on PATH: dies with the new message at commands/users-apply.sh:221. The message's claim that "rig bootstrap does it on host=yes" checks out against commands/bootstrap.sh:437-446.

Review-focus answers

Least privilege: yes. Deferring the incus add to box grant (commands/users-apply.sh:278) is the sharpest part of the change — rig never opens the socket ahead of the narrowing, so a mid-grant failure cannot strand a user with live access to an un-narrowed project. This closes the "worse than absent" window #49 named rather than widening it.
Idempotence: verified by execution (re-run is a clean no-op report; grant is re-invoked convergently, which the PR body honestly flags as the cost of having no cheap converged-probe).
Revocation symmetry: deliberately not this PR's job — both the role-drop arm (gpasswd -d at commands/users-apply.sh:286) and the ledger revocation (:396-398) still bypass box revoke; that is #50, addressed by #55. The test at test/cli.sh:816-820 correctly pins that the deferral sits on the add arm only, so a de-roled user's socket is still stripped.
Unknown/missing users: the grant runs after useradd inside the per-user loop, pinned by the fail-closed ordering assert (test/cli.sh:762-768, defaults 999999/0 so a failed grep fails the test). Usernames reaching box grant "$u" are already constrained to ^[a-z_][a-z0-9_-]{0,31}$ by commands/lib/users-config.sh:71-74, so no injection surface.
Escalation from a lower tier: none added. The rig-role invoker gate (commands/users-apply.sh:130-133) is untouched, and rig now defers to box's own authority on the incus group (the incus-admin non-add), rather than overruling it.
Consistency with docs and box's grant model: README role table and the new "the group is the socket" section match the implementation, including the die-vs-warn split; the --help text (commands/users-apply.sh:49-55) was updated in step.
Tests pin both paths: grant call shape, ordering, host=yes gating (the awk block-scoped check at test/cli.sh:786-788 is a nice touch — it cannot be satisfied by the die's own host=yes), both failure registers, both halves of the defer/wanted-set pair, and the no-apt-install invariant.

Non-blocking observations

Pre-existing corner the PR's framing slightly overstates: "host=no keeps its skip-with-warning" is only true when the incus group is absent. On a host=no (or marker-less) box where the group exists — a marker/reality mismatch, e.g. box setup-host ran but bootstrap said --host no — apply still bare-adds box-role users to incus with no tier (commands/users-apply.sh:251 gated only on INCUS_OK), which is exactly the half-grant state this PR calls worse than absent. I reproduced it in the namespace run (flipping the marker to host=no handed bob the bare socket his failed grant had correctly withheld). This is unchanged from main and out of #49's scope, but it deserves an issue: either a warn on that arm or a decision that the marker wins over the group's presence.
Misattribution risk in the incus-admin branch (commands/users-apply.sh:325): the branch keys on membership, not on grant's actual refusal, so an incus-admin user whose grant fails for an unrelated per-user reason (e.g. incus-user socket down) gets the "blocked on heavy-duty/box#99, everything else converged" story instead of the real cause. Low stakes — box's own stderr still prints above — but matching on grant's refusal message or a distinct exit code (once box#99 settles the interface) would make the warning honest in that corner.
Post-#99 CHANGED silence: once box#99 lands, a first-time grant for an incus-admin member can succeed without any group transition, so the run that materially creates their project will still report "already converged; no changes". The comment at commands/users-apply.sh:314-320 shows this trade-off was chosen knowingly; fine as is.

None of these block: 1 is pre-existing, 2 and 3 are cosmetic-to-minor and both sit on paths that box#99 will reshape anyway.

— automated review by claude-bot-andresmgsl · heavy-duty-review-bot

## Verdict: Approve Reviewed at head 25b63e3. Series context checked: #53, #54, and #55 are each based on `main` (not stacked); this PR closes #49, with the revoke-side mirror explicitly deferred to #50/#55 and bootstrap-time convergence to #51/#54. The diff matches that scoping — nothing here touches the removal arm or the ledger revocation path. ### What I verified **Suites re-run locally on the PR head** (worktree of `refs/pull/53/head`): - `bash test/cli.sh` — 336 passed, 0 failed (matches the PR body's claim; 11 new checks present) - `bash test/release.sh` — 41 passed, 0 failed - `shellcheck -x` over `bin/*` + `commands/*.sh` + `commands/lib/*.sh` + `test/*.sh` — clean **Independent end-to-end execution**, not just the greps: a root mount namespace with tmpfs over `/etc`, `/home`, `/var/log`, real `useradd`, and a stub `box` on PATH that grants `alice` and exits 1 for `bob`: - host=yes fresh run: `alice` (admin,box) granted, group transition logged, apply reports `converged`; `bob`'s failed grant warns and continues, and `bob` is **not** left holding a bare `incus` membership — the deferral at `commands/users-apply.sh:278` really does make socket and tier fail together; `carol` (rig) untouched; exit 0. - identical re-run: grants re-invoked, apply still ends `already converged; no changes` — no phantom `CHANGED` (the `had_socket` probe at `commands/users-apply.sh:308-324` works as described). - host=no: zero `box` invocations. - host=yes with no `box` on PATH: dies with the new message at `commands/users-apply.sh:221`. The message's claim that "rig bootstrap does it on host=yes" checks out against `commands/bootstrap.sh:437-446`. ### Review-focus answers - **Least privilege**: yes. Deferring the `incus` add to `box grant` (`commands/users-apply.sh:278`) is the sharpest part of the change — rig never opens the socket ahead of the narrowing, so a mid-grant failure cannot strand a user with live access to an un-narrowed project. This closes the "worse than absent" window #49 named rather than widening it. - **Idempotence**: verified by execution (re-run is a clean no-op report; grant is re-invoked convergently, which the PR body honestly flags as the cost of having no cheap converged-probe). - **Revocation symmetry**: deliberately not this PR's job — both the role-drop arm (`gpasswd -d` at `commands/users-apply.sh:286`) and the ledger revocation (`:396-398`) still bypass `box revoke`; that is #50, addressed by #55. The test at `test/cli.sh:816-820` correctly pins that the deferral sits on the add arm only, so a de-roled user's socket is still stripped. - **Unknown/missing users**: the grant runs after `useradd` inside the per-user loop, pinned by the fail-closed ordering assert (`test/cli.sh:762-768`, defaults `999999`/`0` so a failed grep fails the test). Usernames reaching `box grant "$u"` are already constrained to `^[a-z_][a-z0-9_-]{0,31}$` by `commands/lib/users-config.sh:71-74`, so no injection surface. - **Escalation from a lower tier**: none added. The rig-role invoker gate (`commands/users-apply.sh:130-133`) is untouched, and rig now defers to box's own authority on the `incus` group (the incus-admin non-add), rather than overruling it. - **Consistency with docs and box's grant model**: README role table and the new "the group is the socket" section match the implementation, including the die-vs-warn split; the `--help` text (`commands/users-apply.sh:49-55`) was updated in step. - **Tests pin both paths**: grant call shape, ordering, host=yes gating (the awk block-scoped check at `test/cli.sh:786-788` is a nice touch — it cannot be satisfied by the die's own `host=yes`), both failure registers, both halves of the defer/wanted-set pair, and the no-apt-install invariant. ### Non-blocking observations 1. **Pre-existing corner the PR's framing slightly overstates**: "host=no keeps its skip-with-warning" is only true when the `incus` group is *absent*. On a `host=no` (or marker-less) box where the group exists — a marker/reality mismatch, e.g. `box setup-host` ran but bootstrap said `--host no` — apply still bare-adds box-role users to `incus` with no tier (`commands/users-apply.sh:251` gated only on `INCUS_OK`), which is exactly the half-grant state this PR calls worse than absent. I reproduced it in the namespace run (flipping the marker to host=no handed `bob` the bare socket his failed grant had correctly withheld). This is unchanged from `main` and out of #49's scope, but it deserves an issue: either a warn on that arm or a decision that the marker wins over the group's presence. 2. **Misattribution risk in the incus-admin branch** (`commands/users-apply.sh:325`): the branch keys on membership, not on grant's actual refusal, so an incus-admin user whose grant fails for an unrelated per-user reason (e.g. incus-user socket down) gets the "blocked on heavy-duty/box#99, everything else converged" story instead of the real cause. Low stakes — box's own stderr still prints above — but matching on grant's refusal message or a distinct exit code (once box#99 settles the interface) would make the warning honest in that corner. 3. **Post-#99 CHANGED silence**: once box#99 lands, a first-time grant for an incus-admin member can succeed without any group transition, so the run that materially creates their project will still report "already converged; no changes". The comment at `commands/users-apply.sh:314-320` shows this trade-off was chosen knowingly; fine as is. None of these block: 1 is pre-existing, 2 and 3 are cosmetic-to-minor and both sit on paths that box#99 will reshape anyway. — _automated review by `claude-bot-andresmgsl` · heavy-duty-review-bot_

grok-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:26:03 +00:00

grok-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: Approve — I agree with this as-is.

Why

Role box now invokes box grant on host=yes after account creation — the tier, not just usermod -aG incus.
Socket ADD deferred to grant so a mid-fail cannot leave live access to an un-narrowed project; missing CLI on host=yes dies; per-user grant failure warns and continues (fleet-safe).
incus-admin branch is an honest warn until box#99/101 lands; no rig reimplementation of grant. Grep pins + CI green.

No blockers. Land with box#101 for the admin-member story end-to-end.

**Verdict: Approve** — I agree with this as-is. ### Why - Role `box` now invokes `box grant` on `host=yes` after account creation — the tier, not just `usermod -aG incus`. - Socket ADD deferred to grant so a mid-fail cannot leave live access to an un-narrowed project; missing CLI on host=yes dies; per-user grant failure warns and continues (fleet-safe). - `incus-admin` branch is an honest warn until box#99/101 lands; no rig reimplementation of grant. Grep pins + CI green. No blockers. Land with box#101 for the admin-member story end-to-end.

codex-bot-andresmgsl (Migrated from github.com) approved these changes 2026-07-19 16:31:30 +00:00

codex-bot-andresmgsl (Migrated from github.com) left a comment

Verdict: I agree with everything and have no additional feedback. On host=yes, the implementation delegates the full tier to box grant after account creation, defers the incus add so grant can roll it back on failure, and retains incus in the desired set so subsequent convergence does not remove it. Host-level prerequisites fail loudly, per-user failures remain isolated, and the current CI checks are green.

Verdict: I agree with everything and have no additional feedback. On `host=yes`, the implementation delegates the full tier to `box grant` after account creation, defers the `incus` add so grant can roll it back on failure, and retains `incus` in the desired set so subsequent convergence does not remove it. Host-level prerequisites fail loudly, per-user failures remain isolated, and the current CI checks are green.

dan-claude-bot commented

2026-07-19 16:37:22 +00:00

(Migrated from github.com)

@danmt — handoff summary.

What this fixes

rig users apply gave role-box users the Incus socket and never the tier. One line — usermod -aG incus — where box grant does five things (project, bridge unpin, restricted.networks.access boxnet, snapshot/backup allowances, box-net profile). Their first box new failed at box/bin/box:749 telling them to find an admin. Worse, the group without the project left a window where incus-user would lazily create an unhardened project (incusbr-<uid>: NAT on v4 and v6, no ACL, no dns.mode=none, no port isolation) — a state box's own design doc measures as worse than not granting at all.

Now apply calls box grant per box-role user on host=yes, after useradd. Closes #49.

Review history

One round, no fixes needed. 3/3 approved at head 25b63e3 — claude-bot, codex-bot, grok-bot. claude-bot re-ran the suites independently and executed the change end-to-end in its own mount namespace rather than trusting the diff.

Verification

test/cli.sh 336 passed / 0 failed (baseline 325 — 11 new checks)
test/release.sh 41 passed / 0 failed
shellcheck -x over CI's exact sweep, 22 files: clean
CI: check, scope, db-integration, reconcile all green
Executed end-to-end under a real useradd in a tmpfs /etc, with a stub box rigged to grant one user and exit 1 for another — confirming warn-and-continue, idempotent re-run, zero box calls on host=no, and the new die when box is absent on host=yes

The design decision worth your eye

The sharpest call was deferring the incus group add to box grant rather than doing it in rig. incus stays in the wanted set (so the exact-convergence else-arm doesn't strip a box-role user's socket), but the add is deferred via a continue on the add arm only.

Reasoning: box grant's fail-closed backout only reverses a membership that run added — a pre-existing one is deliberately kept. So if rig opened the socket first, a mid-way grant failure could not close it, stranding the user with live access to an un-narrowed project. That is exactly the hazard this PR exists to remove. claude-bot verified this by execution: the failed-grant user is not left holding a bare incus membership.

Flagged for your judgment

A pre-existing hole this PR does not close, found by claude-bot. On a host=no or marker-less box where the incus group nonetheless exists — a marker/reality mismatch, e.g. box setup-host ran but bootstrap said --host no — apply still bare-adds box-role users to incus with no tier. That is the half-grant state this PR calls worse than absent, reproduced in claude-bot's namespace run. Unchanged from main and outside #49's scope, but it wants a decision: warn on that arm, or rule that the marker wins over the group's presence. Happy to file it.

Two minor notes, both on paths box#99 reshapes anyway: the incus-admin warning keys on group membership rather than on grant's actual refusal, so an unrelated per-user failure gets the wrong story; and post-#99 a first-time grant for an incus-admin member can succeed with no group transition, so the run that materially creates their project still reports "already converged". Both were chosen knowingly and are documented in-code.

Merge order matters

#55 touches this same file (users-apply.sh) — this PR owns the add arm, #55 owns the removal arms, deliberately non-overlapping. Whichever lands second needs a rebase; both also touch CHANGELOG's ## Unreleased.

The incus-admin story only completes when heavy-duty/box#101 lands (currently in round 2 — reviewers found a real blocker in it). This PR handles both worlds: if box grant still refuses admin members, it warns and continues rather than failing apply, and needs no change once box#101 merges.

🤖 Generated with Claude Code

@danmt — handoff summary. ## What this fixes `rig users apply` gave role-`box` users the Incus **socket** and never the **tier**. One line — `usermod -aG incus` — where `box grant` does five things (project, bridge unpin, `restricted.networks.access boxnet`, snapshot/backup allowances, `box-net` profile). Their first `box new` failed at `box/bin/box:749` telling them to find an admin. Worse, the group without the project left a window where incus-user would lazily create an **unhardened** project (`incusbr-<uid>`: NAT on v4 *and* v6, no ACL, no `dns.mode=none`, no port isolation) — a state box's own design doc measures as worse than not granting at all. Now apply calls `box grant` per box-role user on `host=yes`, after `useradd`. Closes #49. ## Review history One round, no fixes needed. **3/3 approved at head `25b63e3`** — claude-bot, codex-bot, grok-bot. claude-bot re-ran the suites independently and executed the change end-to-end in its own mount namespace rather than trusting the diff. ## Verification - `test/cli.sh` **336 passed / 0 failed** (baseline 325 — 11 new checks) - `test/release.sh` **41 passed / 0 failed** - `shellcheck -x` over CI's exact sweep, 22 files: clean - CI: `check`, `scope`, `db-integration`, `reconcile` all green - Executed end-to-end under a real `useradd` in a tmpfs `/etc`, with a stub `box` rigged to grant one user and exit 1 for another — confirming warn-and-continue, idempotent re-run, zero `box` calls on `host=no`, and the new die when box is absent on `host=yes` ## The design decision worth your eye The sharpest call was **deferring the `incus` group add to `box grant`** rather than doing it in rig. `incus` stays in the wanted set (so the exact-convergence else-arm doesn't strip a box-role user's socket), but the *add* is deferred via a `continue` on the add arm only. Reasoning: `box grant`'s fail-closed backout only reverses a membership *that run* added — a pre-existing one is deliberately kept. So if rig opened the socket first, a mid-way grant failure could not close it, stranding the user with live access to an un-narrowed project. That is exactly the hazard this PR exists to remove. claude-bot verified this by execution: the failed-grant user is **not** left holding a bare `incus` membership. ## Flagged for your judgment **A pre-existing hole this PR does not close, found by claude-bot.** On a `host=no` or marker-less box where the `incus` group nonetheless *exists* — a marker/reality mismatch, e.g. `box setup-host` ran but bootstrap said `--host no` — apply still bare-adds box-role users to `incus` with no tier. That is the half-grant state this PR calls worse than absent, reproduced in claude-bot's namespace run. Unchanged from `main` and outside #49's scope, but it wants a decision: warn on that arm, or rule that the marker wins over the group's presence. Happy to file it. **Two minor notes, both on paths box#99 reshapes anyway:** the incus-admin warning keys on group membership rather than on grant's actual refusal, so an unrelated per-user failure gets the wrong story; and post-#99 a first-time grant for an incus-admin member can succeed with no group transition, so the run that materially creates their project still reports "already converged". Both were chosen knowingly and are documented in-code. ## Merge order matters #55 touches this same file (`users-apply.sh`) — this PR owns the add arm, #55 owns the removal arms, deliberately non-overlapping. Whichever lands second needs a rebase; both also touch CHANGELOG's `## Unreleased`. The incus-admin story only completes when **heavy-duty/box#101** lands (currently in round 2 — reviewers found a real blocker in it). This PR handles both worlds: if `box grant` still refuses admin members, it warns and continues rather than failing apply, and needs no change once box#101 merges. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dan-claude-bot commented

2026-07-19 19:08:12 +00:00

(Migrated from github.com)

Rebased onto `main` after #60 merged — and the changelog needed hand-placing

@danmt — sweep after #60. This branch is rebased onto main; mergeable: true.

The changelog needed more than conflict resolution, and the reason is worth knowing before you merge the rest of this batch.

main cut 0.1.0 at 18:03:45Z. #60 merged at 19:02:12Z — an hour later. Its entry had been written under ## Unreleased, but by merge time that heading had been stamped to ## 0.1.0, so git merged the entry cleanly, with no conflict, into the notes of an already-published release. Same thing happened to this branch on rebase.

Every PR in this batch predates the release, so every one of them has this. It does not announce itself — #54's rebase produced zero conflicts and still put a BREAKING entry inside 0.1.0's notes.

Resolved here by rebuilding the section structure: a fresh ## Unreleased holding this PR's entry and #60's (moved back out of 0.1.0, where it does not belong — it shipped after), with ## 0.1.0 left holding only what it actually released.

Scope of the damage, checked rather than assumed: the published GitHub release body for 0.1.0 is clean — it was generated at release time, before #60 merged, so it never contained the stray entry. Only the CHANGELOG.md file on main was wrong, and merging any PR from this batch corrects it.

Filed as heavy-duty/rig#66 — the release flow has no guard against this, and it will recur on every release.

Verification at the rebased head

shellcheck -x over the CI sweep — clean
test/release.sh — 61 / 0
Suites re-run; numbers in the per-PR detail below.

Content is otherwise unchanged from what the bots approved; the delta is the rebase plus changelog placement.

## Rebased onto `main` after #60 merged — and the changelog needed hand-placing @danmt — sweep after #60. This branch is rebased onto `main`; `mergeable: true`. **The changelog needed more than conflict resolution, and the reason is worth knowing before you merge the rest of this batch.** `main` cut **0.1.0 at 18:03:45Z**. #60 merged at **19:02:12Z** — an hour later. Its entry had been written under `## Unreleased`, but by merge time that heading had been stamped to `## 0.1.0`, so git merged the entry **cleanly, with no conflict**, into the notes of an already-published release. Same thing happened to this branch on rebase. Every PR in this batch predates the release, so every one of them has this. It does not announce itself — #54's rebase produced **zero** conflicts and still put a BREAKING entry inside 0.1.0's notes. **Resolved here** by rebuilding the section structure: a fresh `## Unreleased` holding this PR's entry and #60's (moved back out of 0.1.0, where it does not belong — it shipped after), with `## 0.1.0` left holding only what it actually released. **Scope of the damage, checked rather than assumed:** the *published* GitHub release body for 0.1.0 is **clean** — it was generated at release time, before #60 merged, so it never contained the stray entry. Only the `CHANGELOG.md` file on `main` was wrong, and merging any PR from this batch corrects it. Filed as heavy-duty/rig#66 — the release flow has no guard against this, and it will recur on every release. ### Verification at the rebased head - `shellcheck -x` over the CI sweep — clean - `test/release.sh` — 61 / 0 - Suites re-run; numbers in the per-PR detail below. Content is otherwise unchanged from what the bots approved; the delta is the rebase plus changelog placement.

dan-claude-bot commented

2026-07-19 19:10:50 +00:00

(Migrated from github.com)

Rebased again after #55 merged

@danmt — second sweep, at a950569. #55 merging conflicted this branch in four files; all resolved, all suites green.

Three of the four were complementary content, not competing — #55 and this PR describe opposite directions of the same role, so both sides were kept:

commands/users-apply.sh (--help) — the box role blurb now carries both: apply calls box grant on the way in, hands the group back through box revoke on the way out. It ends "defers BOTH directions to box", which is the actual composed behavior.
README.md — both paragraphs kept, grant first (the acquiring direction), then #55's revoke paragraph.
test/cli.sh — both test blocks, purely additive.
CHANGELOG.md — this PR's entry under ### Added, #55's and #58's under ### Fixed, all inside ## Unreleased. See #66 for why that placement needs care on this branch.

One assertion needed a real update, not a merge. The previous rebase (onto #60) composed the wanted-set arm into:

case ",$roles," in *,box,*) if [ "$BOX_ROLE_OK" -eq 1 ] && [ "$INCUS_OK" -eq 1 ]; then want="$want incus"; fi ;; esac

…which is exactly what both PRs' authors predicted would happen. But this PR pinned the old literal line, so the test failed on rebase — correctly. Updated to the composed form, with a comment recording that the two operands answer different questions: BOX_ROLE_OK is "does the box role apply on this box at all" (the marker's call, #58), INCUS_OK is "is the group there to converge". Both must hold, and incus must still enter the wanted set when they do — otherwise the exact-convergence else-arm strips a box-role user's socket on the very run that granted it, which is the hazard this PR exists to remove.

That failing test was the rebase working. A silent pass there would have meant the pin had stopped pinning anything.

Verification at `a950569`

test/cli.sh — 364 passed, 0 failed
test/release.sh — 61 / 0
shellcheck -x over the CI sweep — clean

## Rebased again after #55 merged @danmt — second sweep, at `a950569`. `#55` merging conflicted this branch in four files; all resolved, all suites green. **Three of the four were complementary content, not competing** — #55 and this PR describe opposite directions of the same role, so both sides were kept: - `commands/users-apply.sh` (`--help`) — the `box` role blurb now carries both: apply calls `box grant` on the way in, hands the group back through `box revoke` on the way out. It ends "defers BOTH directions to box", which is the actual composed behavior. - `README.md` — both paragraphs kept, grant first (the acquiring direction), then #55's revoke paragraph. - `test/cli.sh` — both test blocks, purely additive. - `CHANGELOG.md` — this PR's entry under `### Added`, #55's and #58's under `### Fixed`, all inside `## Unreleased`. See #66 for why that placement needs care on this branch. **One assertion needed a real update, not a merge.** The previous rebase (onto #60) composed the wanted-set arm into: ```sh case ",$roles," in *,box,*) if [ "$BOX_ROLE_OK" -eq 1 ] && [ "$INCUS_OK" -eq 1 ]; then want="$want incus"; fi ;; esac ``` …which is exactly what both PRs' authors predicted would happen. But this PR pinned the *old* literal line, so the test failed on rebase — correctly. Updated to the composed form, with a comment recording that the two operands answer different questions: `BOX_ROLE_OK` is "does the box role apply on this box at all" (the marker's call, #58), `INCUS_OK` is "is the group there to converge". Both must hold, and `incus` must still *enter* the wanted set when they do — otherwise the exact-convergence else-arm strips a box-role user's socket on the very run that granted it, which is the hazard this PR exists to remove. That failing test was the rebase working. A silent pass there would have meant the pin had stopped pinning anything. ### Verification at `a950569` - `test/cli.sh` — **364 passed, 0 failed** - `test/release.sh` — 61 / 0 - `shellcheck -x` over the CI sweep — clean

No reviewers

danmt

claude-bot-andresmgsl

grok-bot-andresmgsl

codex-bot-andresmgsl

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: heavy-duty/rig#53

No description provided.

feat: users apply grants the box tier, not just the socket #53

The gap: the group is the socket, the tier is box grant

The fix

The decision the issue left open: does rig still manage the incus membership?

Blocked-on note: incus-admin members (heavy-duty/box#99)

Verification

Verdict: Approve

What I verified

Review-focus answers

Non-blocking observations

Why

What this fixes

Review history

Verification

The design decision worth your eye

Flagged for your judgment

Merge order matters

Rebased onto main after #60 merged — and the changelog needed hand-placing

Verification at the rebased head

Rebased again after #55 merged

Verification at a950569

The gap: the group is the socket, the tier is `box grant`

The decision the issue left open: does rig still manage the `incus` membership?

Blocked-on note: `incus-admin` members (heavy-duty/box#99)

Rebased onto `main` after #60 merged — and the changelog needed hand-placing

Verification at `a950569`