heavy-duty/rig

Fork 1

feat!: bootstrap takes the users file #54

Merged

dan-claude-bot merged 1 commit from feat/bootstrap-takes-users into main

2026-07-19 19:17:45 +00:00

dan-claude-bot commented

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

(Migrated from github.com)

One command, box ready

rig bootstrap already knew everything else about what a box is — class,
host, join, hostname — and wrote /etc/rig/role to say so. The users file was
the last piece of that answer it did not take, so bring-up was two commands and
the second one was the forgettable one:

rig bootstrap dev --hostname foo          # …and then remember this:
rig users apply --file ./users

Now:

rig bootstrap dev --hostname foo --users ./users   # one command, people included

--users <path> runs exactly what rig users apply --file <path> runs, as
bootstrap's final phase.

Required, with an explicit opt-out

--users is required on every machine role; --no-users is the deliberate
opt-out. Omitting both is a usage error naming both flags; passing both is a
usage error too (rig refuses to pick a winner — a precedence rule deciding who
may enter a box would be wrong half the time).

Required on class=server as well as class=human, per the issue: a machine
nobody logs into routinely is exactly where shared-root access rots, and
per-human accounts keep attribution intact for the times someone does go in. So
the complete path is the default path, and skipping it is deliberate rather
than an omission that looks identical to forgetting — which is precisely what
the old optional flag could not distinguish.

Ordering — and why `host=yes` is smaller than it looks

The phase order is a correctness property, not taste:

traits → tailnet join (verified) → /etc/rig/role → box install (host=yes) → users apply

users apply reads the marker (class= picks its root-SSH note, host=
decides what a missing incus group means), so it must run after the marker is
written. And on host=yes it must run after the box install, because
box's own installer already runs setup-host — the incus group, boxnet,
ACL and firewall are built by installing the box CLI. There is no phase to wait
on: the prerequisite is simply "box installed".

So this PR does not resolve the issue's open question ("should rig install
box on host=yes?"). It does not call box setup-host, and adds no box
install of its own; a test grep-pins that. What it does add is an early
refusal, but only where the outcome is already proven: with
RIG_SKIP_BOX_INSTALL=1 the run has been told it will not touch box, so a
missing incus group can no longer be rescued and the run is doomed a hundred
lines before it would notice. Every other failure shape (no network, box's
installer breaking) is unknowable that early and lands in users apply's
existing refusal at the end — the same message, one phase later.

The precondition fires only when someone in the file actually carries role
box; an admins-only file converges fine on a host that never saw Incus, and
refusing it there would be rig inventing a prerequisite apply does not have.

Decisions the issue left open

The tenant roles take no --users. rig bootstrap <claude|codex|grok| staging> is a different family and stays that way. A tenant is a box-minted
guest: box auto-runs its bootstrap at mint, non-interactively, with no file
to hand it — requiring --users would break the mint-time path outright. The
guest never joins the tailnet and has no SSH door of its own; entry is box shell, gated by the host's incus grants, which the host's own users file
already converged. A fleet-wide operator file has nothing to converge in there.
Mechanically this is free: tenant dispatch happens before the flag parser, so
--users lands in the tenant script's own unknown-flag refusal — pinned by a
test, and explained in both usage() and the README.

--users - is refused. apply's stdin convenience cannot survive the trip:
bootstrap's stdin belongs to the pre-auth key prompt, and the two
credentials-shaped reads would fight over one pipe. The refusal names the split
(--no-users, then apply by hand).

The file is pre-flighted, not just passed through. Bootstrap sources
lib/users-config.sh and runs the same parser apply uses, before apt, before
the hostname change, before a single-use pre-auth key is spent. A typo in the
users file must not be discovered after all of that was already paid for. The
lib is sourced for validation only — users apply stays the single owner of
what a users file does to a box.

Not persisted. rig takes the path, reads it once through apply, and copies
it nowhere; README.md's "rig never persists it" still holds. A negative grep
test enforces it.

apply is invoked as a child, not exec'd. Bootstrap keeps the last word,
a failing apply fails the bootstrap under set -e, and apply's invoker gate
stays intact: SUDO_USER rides through, so sudo rig bootstrap --users <file-naming-me-admin> by a role-rig user refuses exactly as sudo rig users apply would. Bootstrap must not become a laundering path around the one gate
that keeps rig's scoped sudo from being root-equivalent.

Breaking change — the migration

Every existing rig bootstrap invocation must add --users <path> or
--no-users. Scripted bring-up that already ran rig users apply as a
separate step can either fold it in (--users ./users, and drop the separate
call) or keep the old shape verbatim by adding --no-users. Named in the
CHANGELOG under ## Unreleased.

Verification

shellcheck -x over the full CI sweep (bin/* **/*.sh, 22 files, the exact
command from .github/workflows/ci.yml) — clean.
bash test/cli.sh — 352 passed, 0 failed (up from 345; the delta is this
PR's coverage, plus the pre-existing bootstrap invocations updated to state
their users answer).
bash test/release.sh — 41 passed, 0 failed.

New coverage, following the existing patterns in test/cli.sh:

Required-flag enforcement — omitting both exits 2 and the message names
both flags; asserted on class=server too, and --users with no value.
Mutual exclusion — both orders, because a "last flag wins" implementation
would pass one of them silently.
Phase ordering — line-number asserts (the repo's marker/box-install
idiom, fail-closed defaults) that the apply call sits after both the role
marker write and the box install.
The host=yes precondition, both directions — driven with a shim
getent (precedent: the install.sh getent shim) so it is provable on any
machine, not skipped where the runner happens to have an incus group. Four
negative cases prove it does not over-refuse: no box-role user, an existing
group, no RIG_SKIP_BOX_INSTALL, and host=no.
Pre-flight — unreadable file, invalid file (carrying the parser's own
per-line error), --users -.
Never persisted — negative grep for any copy of $USERS_FILE.
Never resolves the box question — negative grep for a box setup-host
call.
Tenant exclusion — --users on a tenant role exits 2, and usage explains
why.

Closes #51

🤖 Generated with Claude Code

## One command, box ready `rig bootstrap` already knew everything else about what a box **is** — class, host, join, hostname — and wrote `/etc/rig/role` to say so. The users file was the last piece of that answer it did not take, so bring-up was two commands and the second one was the forgettable one: ```sh rig bootstrap dev --hostname foo # …and then remember this: rig users apply --file ./users ``` Now: ```sh rig bootstrap dev --hostname foo --users ./users # one command, people included ``` `--users <path>` runs exactly what `rig users apply --file <path>` runs, as bootstrap's **final phase**. ## Required, with an explicit opt-out `--users` is **required** on every machine role; `--no-users` is the deliberate opt-out. Omitting both is a usage error naming both flags; passing both is a usage error too (rig refuses to pick a winner — a precedence rule deciding who may enter a box would be wrong half the time). Required on `class=server` as well as `class=human`, per the issue: a machine nobody logs into routinely is exactly where shared-root access rots, and per-human accounts keep attribution intact for the times someone does go in. So the complete path is the default path, and skipping it is deliberate rather than an omission that looks identical to forgetting — which is precisely what the old optional flag could not distinguish. ## Ordering — and why `host=yes` is smaller than it looks The phase order is a correctness property, not taste: ``` traits → tailnet join (verified) → /etc/rig/role → box install (host=yes) → users apply ``` `users apply` **reads** the marker (`class=` picks its root-SSH note, `host=` decides what a missing `incus` group means), so it must run after the marker is written. And on `host=yes` it must run after the box install, because **box's own installer already runs `setup-host`** — the `incus` group, boxnet, ACL and firewall are built by installing the box CLI. There is no phase to wait on: the prerequisite is simply *"box installed"*. So this PR does **not** resolve the issue's open question ("should rig install box on `host=yes`?"). It does not call `box setup-host`, and adds no box install of its own; a test grep-pins that. What it does add is an **early** refusal, but only where the outcome is already proven: with `RIG_SKIP_BOX_INSTALL=1` the run has been told it will not touch box, so a missing `incus` group can no longer be rescued and the run is doomed a hundred lines before it would notice. Every other failure shape (no network, box's installer breaking) is unknowable that early and lands in `users apply`'s existing refusal at the end — the same message, one phase later. The precondition fires only when someone in the file actually carries role `box`; an admins-only file converges fine on a host that never saw Incus, and refusing it there would be rig inventing a prerequisite apply does not have. ## Decisions the issue left open **The tenant roles take no `--users`.** `rig bootstrap <claude|codex|grok| staging>` is a different family and stays that way. A tenant is a box-minted *guest*: box auto-runs its bootstrap at mint, non-interactively, with no file to hand it — requiring `--users` would break the mint-time path outright. The guest never joins the tailnet and has no SSH door of its own; entry is `box shell`, gated by the **host's** `incus` grants, which the host's own users file already converged. A fleet-wide operator file has nothing to converge in there. Mechanically this is free: tenant dispatch happens before the flag parser, so `--users` lands in the tenant script's own unknown-flag refusal — pinned by a test, and explained in both `usage()` and the README. **`--users -` is refused.** apply's stdin convenience cannot survive the trip: bootstrap's stdin belongs to the pre-auth key prompt, and the two credentials-shaped reads would fight over one pipe. The refusal names the split (`--no-users`, then apply by hand). **The file is pre-flighted, not just passed through.** Bootstrap sources `lib/users-config.sh` and runs the same parser apply uses, before `apt`, before the hostname change, before a single-use pre-auth key is spent. A typo in the users file must not be discovered after all of that was already paid for. The lib is sourced for validation only — `users apply` stays the single owner of what a users file *does* to a box. **Not persisted.** rig takes the path, reads it once through apply, and copies it nowhere; `README.md`'s "rig never persists it" still holds. A negative grep test enforces it. **apply is invoked as a child, not `exec`'d.** Bootstrap keeps the last word, a failing apply fails the bootstrap under `set -e`, and apply's **invoker gate** stays intact: `SUDO_USER` rides through, so `sudo rig bootstrap --users <file-naming-me-admin>` by a role-`rig` user refuses exactly as `sudo rig users apply` would. Bootstrap must not become a laundering path around the one gate that keeps rig's scoped sudo from being root-equivalent. ## Breaking change — the migration **Every existing `rig bootstrap` invocation must add `--users <path>` or `--no-users`.** Scripted bring-up that already ran `rig users apply` as a separate step can either fold it in (`--users ./users`, and drop the separate call) or keep the old shape verbatim by adding `--no-users`. Named in the CHANGELOG under `## Unreleased`. ## Verification - `shellcheck -x` over the full CI sweep (`bin/* **/*.sh`, 22 files, the exact command from `.github/workflows/ci.yml`) — **clean**. - `bash test/cli.sh` — **352 passed, 0 failed** (up from 345; the delta is this PR's coverage, plus the pre-existing bootstrap invocations updated to state their users answer). - `bash test/release.sh` — **41 passed, 0 failed**. New coverage, following the existing patterns in `test/cli.sh`: - **Required-flag enforcement** — omitting both exits 2 and the message names both flags; asserted on `class=server` too, and `--users` with no value. - **Mutual exclusion** — both orders, because a "last flag wins" implementation would pass one of them silently. - **Phase ordering** — line-number asserts (the repo's marker/box-install idiom, fail-closed defaults) that the apply call sits after *both* the role marker write and the box install. - **The `host=yes` precondition, both directions** — driven with a shim `getent` (precedent: the `install.sh` getent shim) so it is provable on any machine, not skipped where the runner happens to have an `incus` group. Four negative cases prove it does *not* over-refuse: no box-role user, an existing group, no `RIG_SKIP_BOX_INSTALL`, and `host=no`. - **Pre-flight** — unreadable file, invalid file (carrying the parser's own per-line error), `--users -`. - **Never persisted** — negative grep for any copy of `$USERS_FILE`. - **Never resolves the box question** — negative grep for a `box setup-host` call. - **Tenant exclusion** — `--users` on a tenant role exits 2, and usage explains why. Closes #51 🤖 Generated with [Claude Code](https://claude.com/claude-code)

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

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

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

Verdict: approve

Verified at head b4fdfbb: bash test/cli.sh — 352 passed, 0 failed; bash test/release.sh — 41 passed, 0 failed; shellcheck -x over the CI sweep (22 files) — clean. All three match the PR body's claims.

Breaking change — properly carried

The feat! is fully documented and fails loudly, never silently:

CHANGELOG.md names the break under ## Unreleased with an explicit migration line (add --users <path> or --no-users to every invocation).
The old invocation shape (no flag) exits 2 with a message naming both ways out (commands/bootstrap.sh:209-211), so a stale script cannot misbehave quietly — it gets told exactly what changed. Asserted at test/cli.sh:437-442.
README, bin/rig --help, and bootstrap --help all updated; usage documents both flags and the tenant exclusion (tests at test/cli.sh:543-554).

Design points that hold up under scrutiny

Mutual exclusion over precedence (bootstrap.sh:206-208), tested in both flag orders (test/cli.sh:448-451) — the both-orders test genuinely kills a "last flag wins" implementation.
Pre-flight before cost (bootstrap.sh:218-237): unreadable file, invalid file (with the parser's own per-line errors), and --users - all die exit 2 before the root check, before apt/hostname/key-spend. The - refusal is correct — bootstrap's stdin really is the pre-auth key prompt's on the authkey path (bootstrap.sh:513-516).
Ordering: apply runs after the marker write and the box install (bootstrap.sh:658-661), and both orderings are pinned by fail-closed line-number asserts (test/cli.sh:528-534). The rationale is real: apply reads class=/host= from the marker (users-apply.sh:150-154, 176-184), and on host=yes the incus group arrives via box's installer.
Idempotence preserved: re-running bootstrap with --users re-runs apply, which is convergent by construction (cmp-guarded writes, exact group membership, "already converged; no changes"). Re-running with --no-users on a box that has users touches nothing. The already-joined tailnet path skips the key prompt, so a repair re-run stays cheap.
The invoker gate is not laundered: apply is invoked as a child, so SUDO_USER rides through and apply's rig-admin gate (users-apply.sh:125-128) fires identically under sudo rig bootstrap --users <file-naming-me-admin>. I checked the escalation shape: a role-rig user's scoped sudo reaches rig bootstrap, but the file naming them admin still dies at apply's gate. (That refusal lands late — end of the run — but on any box where a rig-role user exists the tailnet is already joined, so no key is spent; acceptable.)
The host=yes precondition is correctly narrow (bootstrap.sh:255-259): only box-role-in-file + host=yes + RIG_SKIP_BOX_INSTALL=1 + no group. The four negative tests (test/cli.sh:492-514) prove it does not over-refuse, and the getent shim makes it machine-independent. The role-box grep on the parsed user|roles|key stream (bootstrap.sh:234) handles comma lists correctly.
Not persisted / no setup-host: both enforced as negative greps (test/cli.sh:518-519, 539-540).

Series context

#53 and #55 both target main independently; this PR is a single commit atop main (219b0fb) — no stacking, no hidden inclusion. #53 rewrites apply's box-role internals, but bootstrap only invokes apply as a child, so there is no semantic conflict; expect at most textual merge friction in test/cli.sh.

Non-blocking observations

An empty (or comments-only) users file passes pre-flight silently and converges a root-only box under --users — re-creating exactly the "looks identical to forgetting" ambiguity the required flag was built to kill. Worse on a re-run: a truncated file against a box with existing operators revokes them all (apply's documented drop-semantics, so it warns per user — but bootstrap could catch the zero-user parse at bootstrap.sh:227-229 and require --no-users to say root-only out loud). Worth a follow-up.
--users <directory> dies exit 2 but noisily: [ -r ] passes for a directory (bootstrap.sh:226), so the parser aborts with bash noise (read: Is a directory, line: unbound variable) before the die message. An [ -f ] check would clean it up. Same pre-existing shape in users-apply.sh:85, so this is parity, not a regression.
Post-#53 the early precondition becomes a weaker proxy: #53 adds a "box CLI not on PATH" die on host=yes that the early check (group only) does not mirror. That is consistent with this PR's "early only where proven" rule, but whoever merges second may want to extend the precondition to command -v box under the same skip condition.

None of these block: 1 and 2 are edge polish on top of behavior that still fails safe or loud, and 3 belongs to whichever PR lands second.

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

## Verdict: approve Verified at head b4fdfbb: `bash test/cli.sh` — **352 passed, 0 failed**; `bash test/release.sh` — **41 passed, 0 failed**; `shellcheck -x` over the CI sweep (22 files) — clean. All three match the PR body's claims. ### Breaking change — properly carried The `feat!` is fully documented and fails loudly, never silently: - `CHANGELOG.md` names the break under `## Unreleased` with an explicit migration line (add `--users <path>` or `--no-users` to every invocation). - The old invocation shape (no flag) exits 2 with a message naming both ways out (`commands/bootstrap.sh:209-211`), so a stale script cannot misbehave quietly — it gets told exactly what changed. Asserted at `test/cli.sh:437-442`. - README, `bin/rig --help`, and `bootstrap --help` all updated; usage documents both flags and the tenant exclusion (tests at `test/cli.sh:543-554`). ### Design points that hold up under scrutiny - **Mutual exclusion over precedence** (`bootstrap.sh:206-208`), tested in both flag orders (`test/cli.sh:448-451`) — the both-orders test genuinely kills a "last flag wins" implementation. - **Pre-flight before cost** (`bootstrap.sh:218-237`): unreadable file, invalid file (with the parser's own per-line errors), and `--users -` all die exit 2 before the root check, before apt/hostname/key-spend. The `-` refusal is correct — bootstrap's stdin really is the pre-auth key prompt's on the authkey path (`bootstrap.sh:513-516`). - **Ordering**: apply runs after the marker write and the box install (`bootstrap.sh:658-661`), and both orderings are pinned by fail-closed line-number asserts (`test/cli.sh:528-534`). The rationale is real: apply reads `class=`/`host=` from the marker (`users-apply.sh:150-154, 176-184`), and on `host=yes` the incus group arrives via box's installer. - **Idempotence preserved**: re-running bootstrap with `--users` re-runs apply, which is convergent by construction (cmp-guarded writes, exact group membership, "already converged; no changes"). Re-running with `--no-users` on a box that has users touches nothing. The already-joined tailnet path skips the key prompt, so a repair re-run stays cheap. - **The invoker gate is not laundered**: apply is invoked as a child, so `SUDO_USER` rides through and apply's rig-admin gate (`users-apply.sh:125-128`) fires identically under `sudo rig bootstrap --users <file-naming-me-admin>`. I checked the escalation shape: a role-`rig` user's scoped sudo reaches `rig bootstrap`, but the file naming them admin still dies at apply's gate. (That refusal lands late — end of the run — but on any box where a rig-role user exists the tailnet is already joined, so no key is spent; acceptable.) - **The `host=yes` precondition is correctly narrow** (`bootstrap.sh:255-259`): only box-role-in-file + `host=yes` + `RIG_SKIP_BOX_INSTALL=1` + no group. The four negative tests (`test/cli.sh:492-514`) prove it does not over-refuse, and the getent shim makes it machine-independent. The role-box grep on the parsed `user|roles|key` stream (`bootstrap.sh:234`) handles comma lists correctly. - **Not persisted / no setup-host**: both enforced as negative greps (`test/cli.sh:518-519, 539-540`). ### Series context #53 and #55 both target `main` independently; this PR is a single commit atop `main` (219b0fb) — no stacking, no hidden inclusion. #53 rewrites apply's box-role internals, but bootstrap only invokes apply as a child, so there is no semantic conflict; expect at most textual merge friction in `test/cli.sh`. ### Non-blocking observations 1. **An empty (or comments-only) users file passes pre-flight silently** and converges a root-only box under `--users` — re-creating exactly the "looks identical to forgetting" ambiguity the required flag was built to kill. Worse on a re-run: a truncated file against a box with existing operators revokes them all (apply's documented drop-semantics, so it warns per user — but bootstrap could catch the zero-user parse at `bootstrap.sh:227-229` and require `--no-users` to say root-only out loud). Worth a follow-up. 2. **`--users <directory>` dies exit 2 but noisily**: `[ -r ]` passes for a directory (`bootstrap.sh:226`), so the parser aborts with bash noise (`read: Is a directory`, `line: unbound variable`) before the `die` message. An `[ -f ]` check would clean it up. Same pre-existing shape in `users-apply.sh:85`, so this is parity, not a regression. 3. **Post-#53 the early precondition becomes a weaker proxy**: #53 adds a "box CLI not on PATH" die on `host=yes` that the early check (group only) does not mirror. That is consistent with this PR's "early only where proven" rule, but whoever merges second may want to extend the precondition to `command -v box` under the same skip condition. None of these block: 1 and 2 are edge polish on top of behavior that still fails safe or loud, and 3 belongs to whichever PR lands second. — _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:04 +00:00

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

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

Why

BREAKING but deliberate: --users / --no-users required; omission and both-flags are usage errors (exit 2). Migration path clear.
Users phase is last (after marker + host=yes box install); pre-flight uses apply’s parser before apt/key spend; --users - refused (stdin is authkey); tenants excluded correctly.
Convergence stays owned by users apply (exec, not reimplemented). CI check/db green; labels reconcile cancelled (not a product fail).

No blockers.

**Verdict: Approve** — I agree with this as-is. ### Why - BREAKING but deliberate: `--users` / `--no-users` required; omission and both-flags are usage errors (exit 2). Migration path clear. - Users phase is last (after marker + host=yes box install); pre-flight uses apply’s parser before apt/key spend; `--users -` refused (stdin is authkey); tenants excluded correctly. - Convergence stays owned by `users apply` (exec, not reimplemented). CI check/db green; labels reconcile cancelled (not a product fail). No blockers.

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

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

Verdict: I agree with everything and have no additional feedback. The required --users/--no-users contract is validated before costly bootstrap work, tenant roles remain separate, and users convergence runs after the role marker and host-side box installation while preserving users apply as the single convergence owner. The migration is documented and the current CI checks are green.

Verdict: I agree with everything and have no additional feedback. The required `--users`/`--no-users` contract is validated before costly bootstrap work, tenant roles remain separate, and users convergence runs after the role marker and host-side box installation while preserving `users apply` as the single convergence owner. The migration is documented and the current CI checks are green.

dan-claude-bot commented

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

(Migrated from github.com)

@danmt — handoff summary. This is the breaking one (feat!) — read the migration note before merging.

What this does

rig bootstrap took --hostname, --class, --host, --join — everything about what a box is except its people. It now takes --users <path>, required, with an explicit --no-users opt-out. Closes #51.

The rationale is yours, recorded on the issue: even on class=server boxes nobody logs into routinely, named human accounts beat shared root — that is exactly where root access rots, and per-human users keep attribution intact for the times someone does go in. Required-with-opt-out makes the complete path the default path, and skipping deliberate.

Migration

Every existing rig bootstrap invocation gains a required flag. Omitting both exits 2 naming both ways out, so a stale script fails loudly rather than quietly bootstrapping a root-only box. CHANGELOG carries the migration line; README, bin/rig --help and bootstrap --help are all updated.

Review history

One round, no fixes needed. 3/3 approved at head b4fdfbb — claude-bot, codex-bot, grok-bot.

Verification

test/cli.sh 352 passed / 0 failed (baseline 345 — 7 net new, after updating pre-existing bootstrap invocations to state a users answer)
test/release.sh 41 passed / 0 failed
shellcheck -x over the CI sweep, 22 files: clean
CI: check, scope, db-integration green

Decisions made along the way

Tenant roles take no --users. rig bootstrap <claude|codex|grok|staging> are box-minted guests: box auto-runs their bootstrap at mint, non-interactively, with no file to hand it — requiring the flag would break the mint-time path outright. The guest never joins the tailnet and has no SSH door of its own; entry is box shell, gated by the host's incus grants that the host's own users file already converged. Mechanically free, since tenant dispatch precedes the flag parser.

Pre-flight before cost. Unreadable file, invalid file, and --users - all die exit 2 before the root check, before apt / hostname / key spend. The - refusal is deliberate: bootstrap's stdin belongs to the pre-auth key prompt.

apply is invoked as a child, not exec'd, so SUDO_USER rides through and apply's rig-admin invoker gate still fires. claude-bot specifically checked the escalation shape: a role-rig user's scoped sudo reaches rig bootstrap, but a file naming them admin still dies at apply's gate. Bootstrap cannot become a laundering path around it.

A correction to the issue, for the record

I filed #51 claiming the open question was "should rig install box on host=yes" and called it the last thing between us and one-command bring-up. That was wrong — commands/bootstrap.sh:437-512 already installs box and lets box's own installer run setup-host, with the delegation boundary reasoned out in place. I've corrected the issue body and left a comment there. Consequence: the new early host=yes precondition fires only under RIG_SKIP_BOX_INSTALL=1, and it is gated on the file actually containing a role-box user so an admins-only file isn't refused on an Incus-less host.

Flagged for your judgment

The one I'd genuinely like your call on, from claude-bot: an empty or comments-only users file passes pre-flight silently and converges a root-only box under --users — re-creating the "looks identical to forgetting" ambiguity that requiring the flag was built to kill. Sharper on a re-run: a truncated file against a box with existing operators revokes them all. Apply's documented drop-semantics warn per user, so it is loud rather than silent — but bootstrap could catch the zero-user parse and require --no-users to say root-only out loud. Given that this PR's entire premise is "make the complete path the default path", I lean toward tightening it. Not done here because it changes the contract the bots just approved. Say the word and it's a small follow-up.

Two smaller ones, neither blocking: --users <directory> dies exit 2 but noisily ([ -r ] passes for a directory, so the parser emits bash noise before the die) — [ -f ] would clean it up, and the same shape pre-exists in users-apply.sh:85, so it's parity not regression. And post-#53 the early precondition becomes a weaker proxy, since #53 adds a "box CLI not on PATH" die that the group-only early check doesn't mirror; whoever merges second may want to extend it under the same skip condition.

Merge order

This PR only invokes apply and does not modify it, so there is no semantic conflict with #53 or #55 — expect at most textual friction in test/cli.sh and CHANGELOG's ## Unreleased.

🤖 Generated with Claude Code

@danmt — handoff summary. **This is the breaking one** (`feat!`) — read the migration note before merging. ## What this does `rig bootstrap` took `--hostname`, `--class`, `--host`, `--join` — everything about what a box *is* except its people. It now takes `--users <path>`, **required**, with an explicit `--no-users` opt-out. Closes #51. The rationale is yours, recorded on the issue: even on `class=server` boxes nobody logs into routinely, named human accounts beat shared root — that is exactly where root access rots, and per-human users keep attribution intact for the times someone does go in. Required-with-opt-out makes the complete path the default path, and skipping deliberate. ## Migration Every existing `rig bootstrap` invocation gains a required flag. Omitting both exits 2 naming both ways out, so a stale script fails loudly rather than quietly bootstrapping a root-only box. CHANGELOG carries the migration line; README, `bin/rig --help` and `bootstrap --help` are all updated. ## Review history One round, no fixes needed. **3/3 approved at head `b4fdfbb`** — claude-bot, codex-bot, grok-bot. ## Verification - `test/cli.sh` **352 passed / 0 failed** (baseline 345 — 7 net new, after updating pre-existing bootstrap invocations to state a users answer) - `test/release.sh` **41 passed / 0 failed** - `shellcheck -x` over the CI sweep, 22 files: clean - CI: `check`, `scope`, `db-integration` green ## Decisions made along the way **Tenant roles take no `--users`.** `rig bootstrap <claude|codex|grok|staging>` are box-minted guests: box auto-runs their bootstrap at mint, non-interactively, with no file to hand it — requiring the flag would break the mint-time path outright. The guest never joins the tailnet and has no SSH door of its own; entry is `box shell`, gated by the *host's* `incus` grants that the host's own users file already converged. Mechanically free, since tenant dispatch precedes the flag parser. **Pre-flight before cost.** Unreadable file, invalid file, and `--users -` all die exit 2 before the root check, before apt / hostname / key spend. The `-` refusal is deliberate: bootstrap's stdin belongs to the pre-auth key prompt. **apply is invoked as a child, not `exec`'d**, so `SUDO_USER` rides through and apply's rig-admin invoker gate still fires. claude-bot specifically checked the escalation shape: a role-`rig` user's scoped sudo reaches `rig bootstrap`, but a file naming them admin still dies at apply's gate. Bootstrap cannot become a laundering path around it. ## A correction to the issue, for the record I filed #51 claiming the open question was "should rig install box on `host=yes`" and called it the last thing between us and one-command bring-up. **That was wrong** — `commands/bootstrap.sh:437-512` already installs box and lets box's own installer run `setup-host`, with the delegation boundary reasoned out in place. I've corrected the issue body and left a comment there. Consequence: the new early `host=yes` precondition fires only under `RIG_SKIP_BOX_INSTALL=1`, and it is gated on the file actually containing a role-`box` user so an admins-only file isn't refused on an Incus-less host. ## Flagged for your judgment **The one I'd genuinely like your call on**, from claude-bot: an **empty or comments-only users file passes pre-flight silently** and converges a root-only box under `--users` — re-creating the "looks identical to forgetting" ambiguity that requiring the flag was built to kill. Sharper on a re-run: a truncated file against a box with existing operators revokes them all. Apply's documented drop-semantics warn per user, so it is loud rather than silent — but bootstrap could catch the zero-user parse and require `--no-users` to say root-only out loud. Given that this PR's entire premise is "make the complete path the default path", I lean toward tightening it. Not done here because it changes the contract the bots just approved. Say the word and it's a small follow-up. **Two smaller ones, neither blocking:** `--users <directory>` dies exit 2 but noisily (`[ -r ]` passes for a directory, so the parser emits bash noise before the `die`) — `[ -f ]` would clean it up, and the same shape pre-exists in `users-apply.sh:85`, so it's parity not regression. And post-#53 the early precondition becomes a weaker proxy, since #53 adds a "box CLI not on PATH" die that the group-only early check doesn't mirror; whoever merges second may want to extend it under the same skip condition. ## Merge order This PR only *invokes* apply and does not modify it, so there is no semantic conflict with #53 or #55 — expect at most textual friction in `test/cli.sh` and CHANGELOG's `## Unreleased`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

dan-claude-bot commented

2026-07-19 19:08:15 +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:17:17 +00:00

(Migrated from github.com)

Rebased after #53 merged — and picked up the deferred precondition work

@danmt — at b8dc115, mergeable: true. This is the last of the batch, and it does one thing beyond the rebase, deliberately.

The changelog conflict

Only CHANGELOG.md conflicted. Resolved so ## Unreleased now holds all four entries under correct headings — ### Added (#49), ### Changed (this PR's BREAKING entry), ### Fixed (#58, #50) — with ## 0.1.0 left holding only what it actually shipped. A duplicated ## 0.1.0 heading from an earlier replay was also removed. See #66; this batch has hit that trap on every single rebase.

The deferred item, now due

@claude-bot-andresmgsl's third non-blocking note on this PR said:

post-#53 the early precondition becomes a weaker proxy: #53 adds a "box CLI not on PATH" die on host=yes that the early check (group only) does not mirror. Whoever merges second may want to extend the precondition.

#53 has merged and this PR is second, so that is now. The early check tested only for the incus group; under RIG_SKIP_BOX_INSTALL=1 a missing box CLI is just as final and just as knowable at that point, and would otherwise fail a hundred lines later in apply. It now mirrors both refusals — either one alone dooms the run.

A machine-dependence bug I introduced and then fixed

Adding that check quietly made an existing test depend on the runner. bootstrap: an existing incus group satisfies the precondition passed here only because this machine happens to have box on PATH; on a CI runner without it, the new die would fire and the test would fail for a reason that has nothing to do with what it asserts.

Fixed the same way this block already handles getent: INCUS_SHIM_YES now ships a stub box, so the satisfied case is proven on its own terms rather than on the runner's luck. The two new negative tests build PATH without the real one, so the absence they test is the test's, not the machine's.

Verified by running the whole suite with box stripped from PATH — command -v box returns nothing, suite still 393 passed, 0 failed.

Verification at `b8dc115`

test/cli.sh — 393 passed, 0 failed (+2 new)
Same suite with box hidden from PATH — 393 passed, 0 failed
test/release.sh — 61 / 0
shellcheck -x over the CI sweep — clean
Mutation check: deleting only the new box-CLI die makes exactly the 2 new tests fail

Once this merges, #59 unblocks — I will rebase it onto main, drop blocked, and put it in front of the bots.

## Rebased after #53 merged — and picked up the deferred precondition work @danmt — at `b8dc115`, `mergeable: true`. This is the last of the batch, and it does one thing beyond the rebase, deliberately. ### The changelog conflict Only `CHANGELOG.md` conflicted. Resolved so `## Unreleased` now holds all four entries under correct headings — `### Added` (#49), `### Changed` (this PR's BREAKING entry), `### Fixed` (#58, #50) — with `## 0.1.0` left holding only what it actually shipped. A duplicated `## 0.1.0` heading from an earlier replay was also removed. See #66; this batch has hit that trap on every single rebase. ### The deferred item, now due @claude-bot-andresmgsl's third non-blocking note on this PR said: > post-#53 the early precondition becomes a weaker proxy: #53 adds a "box CLI not on PATH" die on `host=yes` that the early check (group only) does not mirror. Whoever merges second may want to extend the precondition. #53 has merged and this PR is second, so that is now. The early check tested only for the `incus` group; under `RIG_SKIP_BOX_INSTALL=1` a missing **box CLI** is just as final and just as knowable at that point, and would otherwise fail a hundred lines later in apply. It now mirrors both refusals — either one alone dooms the run. ### A machine-dependence bug I introduced and then fixed Adding that check quietly made an existing test depend on the runner. `bootstrap: an existing incus group satisfies the precondition` passed here only because **this machine happens to have `box` on PATH**; on a CI runner without it, the new die would fire and the test would fail for a reason that has nothing to do with what it asserts. Fixed the same way this block already handles `getent`: `INCUS_SHIM_YES` now ships a stub `box`, so the satisfied case is proven on its own terms rather than on the runner's luck. The two new negative tests build `PATH` **without** the real one, so the absence they test is the test's, not the machine's. Verified by running the whole suite with `box` stripped from `PATH` — `command -v box` returns nothing, suite still **393 passed, 0 failed**. ### Verification at `b8dc115` - `test/cli.sh` — **393 passed, 0 failed** (+2 new) - Same suite with `box` hidden from `PATH` — **393 passed, 0 failed** - `test/release.sh` — 61 / 0 - `shellcheck -x` over the CI sweep — clean - **Mutation check**: deleting only the new box-CLI die makes exactly the 2 new tests fail Once this merges, #59 unblocks — I will rebase it onto `main`, drop `blocked`, and put it in front of the bots.

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#54

No description provided.

feat!: bootstrap takes the users file #54

One command, box ready

Required, with an explicit opt-out

Ordering — and why host=yes is smaller than it looks

Decisions the issue left open

Breaking change — the migration

Verification

Verdict: approve

Breaking change — properly carried

Design points that hold up under scrutiny

Series context

Non-blocking observations

Why

What this does

Migration

Review history

Verification

Decisions made along the way

A correction to the issue, for the record

Flagged for your judgment

Merge order

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

Verification at the rebased head

Rebased after #53 merged — and picked up the deferred precondition work

The changelog conflict

The deferred item, now due

A machine-dependence bug I introduced and then fixed

Verification at b8dc115

Ordering — and why `host=yes` is smaller than it looks

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

Verification at `b8dc115`