Compare commits

...

9 commits

Author SHA1 Message Date
cb4de44bf4 Merge pull request 'fix: refuse a PATH without /usr/sbin, before the token prompt' (#140) from build/139-sbin-path-preflight into main
All checks were successful
ci / check (push) Successful in 56s
ci / install (push) Successful in 3s
ci / db-integration (push) Successful in 3s
release / release (push) Successful in 6s
Reviewed-on: #140
Reviewed-by: grok-reviewer-andresmgsl <andres+3@heavyduty.builders>
Reviewed-by: kimi-reviewer-andresmgsl <andres+4@heavyduty.builders>
Reviewed-by: codex-reviewer-andresmgsl <andres+2@heavyduty.builders>
2026-08-02 07:57:52 +00:00
7aed6ea098 fix: preflight every admin binary a command uses, not only useradd
Some checks failed
ci / check (pull_request) Successful in 55s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 7s
Addresses codex (1538) and kimi (1539): the sweep caught the reported
incident and not the class. Both are right, and there were four sites, not
three.

  forgejo-runner-install  useradd -> useradd usermod   (usermod -aG docker,
                          reached only after the token has been spent)
  users-apply             useradd usermod -> + groupadd (called two lines
                          into convergence), and visudo when a role needs it
  bootstrap-tenant        NEW site (kimi) — usermod -aG docker runs AFTER
                          docker and node are installed, so an unguarded
                          PATH fails it mid-convergence on a changed machine
  runner-install          unchanged: useradd is the only admin binary it
                          calls, and declaring more would refuse boxes that
                          are fine

visudo is checked after the sudo-install block rather than beside the root
check, because until sudo is installed its absence has an innocent cause.
Below that block it does not: sudo is present, so a missing visudo means
/usr/sbin is off PATH. That case is the quiet one — the sudoers block reads
`command -v visudo` as "no sudo on the box means no role needed it", so
apply reported success having granted roles without the escalation those
roles exist for. The other three sites at least crash.

Measured which binaries this covers (Debian 13): useradd, usermod, groupadd,
userdel, groupdel and visudo are /usr/sbin; gpasswd is /usr/bin and so is
NOT affected and deliberately not preflighted. visudo shares the directory
but ships in `sudo`, not `passwd` — which is why it needs its own treatment.

Tests: the sbin-less fixtures could only ever prove the FIRST binary is
named, since useradd wins every race. Six new checks use partial PATHs that
resolve the earlier binaries and withhold exactly one, plus the ordering
assertions (no token prompt, no group created) and the negative case — a
users file needing no sudo must NOT be refused for a missing visudo.

Refs #139
2026-08-02 00:05:02 +00:00
5187b74fa0 Merge remote-tracking branch 'origin/main' into pr140 2026-08-01 23:58:21 +00:00
0d36b4dc95 Merge pull request 'fix: slim ubuntu-latest default; install shellcheck in ci.yml (#144)' (#146) from build/144-default-labels-option-b into main
All checks were successful
ci / check (push) Successful in 55s
ci / install (push) Successful in 3s
ci / db-integration (push) Successful in 3s
release / release (push) Successful in 6s
Reviewed-on: #146
Reviewed-by: codex-reviewer-andresmgsl <andres+2@heavyduty.builders>
Reviewed-by: cluade-reviewer-andresmgsl <andres+1@heavyduty.builders>
Reviewed-by: kimi-reviewer-andresmgsl <andres+4@heavyduty.builders>
2026-08-01 23:23:41 +00:00
6f92b9eaa6 test: drive the retired-default label recogniser (#144)
Some checks failed
ci / check (pull_request) Successful in 55s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 7s
Extract labels_are_a_retired_default and assert the four cases a grep pin
cannot: pre-#144 matches, current default does not, custom --labels does
not, near-miss does not. Addresses the remaining REQUEST_CHANGES on !146.

Refs #144
2026-08-01 21:38:26 +00:00
f0f17ad2ff fix: warn only on retired default labels, not custom maps (#144)
Some checks failed
ci / check (pull_request) Successful in 54s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 6s
Plain converge comparing RECORDED to the current default nags every runner
registered with intentional --labels (including drill Leg 3). Match known
past DEFAULT_LABELS strings instead — same intent, no noise. Changelog
split per review.

Refs #144
2026-08-01 21:33:11 +00:00
23965bebca test: forgejo-runner version pins work when CI runs as root (#144)
Some checks failed
ci / check (pull_request) Successful in 55s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 6s
act-22.04 jobs are uid 0, so "must run as root" is never the next gate after
--version validation. Accept the unattended-token refuse when already root;
keep the non-root arm for GitHub-hosted runners.

Refs #144
2026-08-01 21:18:33 +00:00
ad3133d1c0 fix: slim ubuntu-latest default; install shellcheck in ci.yml (#144)
Some checks failed
ci / check (pull_request) Failing after 52s
ci / install (pull_request) Successful in 3s
ci / db-integration (pull_request) Successful in 3s
labels / labels (pull_request) Failing after 6s
Option B (andres ruling): keep act-22.04 for ubuntu-latest so box-class
ci tenants can hold the image; workflows supply tools the slim image
lacks. Opt-in ubuntu-latest-full for operators who need GH parity.
Plain converge warns when recorded labels lag the current default map.

Refs #144
2026-08-01 21:13:46 +00:00
7f2501d0fe fix: refuse a PATH without /usr/sbin, before the token prompt
Some checks failed
ci / check (pull_request) Failing after 7s
ci / install (pull_request) Successful in 4s
ci / db-integration (pull_request) Successful in 4s
labels / labels (pull_request) Failing after 7s
Reported from a real ci-box: `rig forgejo-runner install` read a registration
token off the operator's terminal and then died with

  …/forgejo-runner-install.sh: line 250: useradd: command not found

rig checked `id -u` and concluded it could administer the machine. Being root
and being able to FIND the admin binaries are different facts, and only the
first was asserted. `su` without `-`, sudo with a sanitised secure_path, and
several container images all produce a root shell with no /usr/sbin on PATH,
which is where useradd lives.

Three call sites had it: both runner installers and users apply. The last is
the worst — it runs mid-convergence, so a PATH-shorn root could fail partway
through a user sweep rather than before it starts.

require_admin_bins refuses rather than repairing PATH itself: a command that
quietly prepends /usr/sbin teaches the operator nothing and leaves a
misconfigured host misconfigured. The message names the remedy and,
deliberately, not this script — echoing an internal path back at someone who
typed `rig forgejo-runner install` is the unhelpful half of the original error.

It sits beside each root check, so identity and capability are asserted
together and before anything is spent. A secret typed for a run that could
never succeed is the avoidable half of this bug, and there is a test for
exactly that ordering.

Closes #139

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-08-01 18:39:03 +00:00
9 changed files with 349 additions and 15 deletions

View file

@ -26,7 +26,21 @@ jobs:
# The file list is printed so under-coverage shows up in the log, and # The file list is printed so under-coverage shows up in the log, and
# the comm below turns under-coverage into a failure rather than a # the comm below turns under-coverage into a failure rather than a
# thing someone has to notice: every tracked `.sh` must be in the set. # thing someone has to notice: every tracked `.sh` must be in the set.
#
# Install shellcheck when missing (#144). rig's default Forgejo label
# maps ubuntu-latest to catthehacker's act-22.04 (slim), which does not
# ship shellcheck; GitHub-hosted ubuntu-latest does. The conditional
# keeps each forge from paying for the other.
#
# sudo: load-bearing on GitHub (job runs as `runner` with passwordless
# sudo) and a no-op on act-22.04 (jobs run as uid 0; the image has no
# `runner` account). Do not delete it as "dead weight" — that breaks
# the GitHub half the day that image stops preinstalling shellcheck.
run: | run: |
if ! command -v shellcheck >/dev/null 2>&1; then
sudo apt-get update
sudo apt-get install -y shellcheck
fi
shopt -s globstar dotglob shopt -s globstar dotglob
files=(bin/* **/*.sh) files=(bin/* **/*.sh)
printf 'shellcheck: %s\n' "${files[@]}" printf 'shellcheck: %s\n' "${files[@]}"

4
changelog.d/139.md Normal file
View file

@ -0,0 +1,4 @@
### Fixed
- `rig forgejo-runner install`, `rig runner install`, `rig users apply` and `rig bootstrap <tenant>` refuse with a named remedy when root's `PATH` carries no `/usr/sbin`, instead of dying on `useradd: command not found` after prompting for a token (#139)
- `rig users apply` no longer reports success having silently skipped the sudoers drop-in when `visudo` is off `PATH` (#139)

8
changelog.d/144.md Normal file
View file

@ -0,0 +1,8 @@
### Added
- Default Forgejo runner labels include opt-in `ubuntu-latest-full` for the GitHub-parity image (#144)
### Fixed
- `ci.yml` installs `shellcheck` when the runner image lacks it, so Forgejo's slim `ubuntu-latest` can run `check` (#144)
- Plain `rig forgejo-runner install` warns when recorded labels are a retired rig default, without nagging custom `--labels` (#144)

View file

@ -31,6 +31,8 @@ HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
. "$HERE/lib/templates.sh" # templates_resolve / template_parse_env / render_tenant_context . "$HERE/lib/templates.sh" # templates_resolve / template_parse_env / render_tenant_context
# shellcheck source=SCRIPTDIR/lib/users-config.sh # shellcheck source=SCRIPTDIR/lib/users-config.sh
. "$HERE/lib/users-config.sh" # read_role_marker / root_door_of . "$HERE/lib/users-config.sh" # read_role_marker / root_door_of
# shellcheck source=SCRIPTDIR/lib/admin-path.sh
. "$HERE/lib/admin-path.sh" # require_admin_bins
# shellcheck source=SCRIPTDIR/lib/sshd.sh # shellcheck source=SCRIPTDIR/lib/sshd.sh
. "$HERE/lib/sshd.sh" # harden_sshd (the staging-box tenant) . "$HERE/lib/sshd.sh" # harden_sshd (the staging-box tenant)
# shellcheck source=SCRIPTDIR/lib/manifest.sh # shellcheck source=SCRIPTDIR/lib/manifest.sh
@ -200,6 +202,15 @@ else
fi fi
[ "$(id -u)" -eq 0 ] || die "must run as root" [ "$(id -u)" -eq 0 ] || die "must run as root"
# Root is not enough here either (#139). This mint adds the tenant user to the
# docker group with `usermod` far below — AFTER installing docker and node,
# which is what makes it the worst-placed of the four call sites: on a
# PATH-shorn root it dies mid-convergence with a bare `usermod: command not
# found`, having already changed the machine, rather than before touching it.
#
# Unconditional because the docker block below is unconditional — "every tenant
# gets docker" is the stated rule there, so every tenant reaches the usermod.
require_admin_bins usermod
if [ -r /etc/os-release ]; then if [ -r /etc/os-release ]; then
# Sourced in a subshell: os-release defines VERSION, NAME, ID, etc. — # Sourced in a subshell: os-release defines VERSION, NAME, ID, etc. —
# sourcing it in the main shell silently clobbers same-named script vars. # sourcing it in the main shell silently clobbers same-named script vars.

View file

@ -17,6 +17,8 @@ set -euo pipefail
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
# shellcheck source=SCRIPTDIR/lib/forgejo-runner-config.sh # shellcheck source=SCRIPTDIR/lib/forgejo-runner-config.sh
. "$HERE/lib/forgejo-runner-config.sh" . "$HERE/lib/forgejo-runner-config.sh"
# shellcheck source=SCRIPTDIR/lib/admin-path.sh
. "$HERE/lib/admin-path.sh"
log() { printf 'rig-forgejo-runner: %s\n' "$*"; } log() { printf 'rig-forgejo-runner: %s\n' "$*"; }
warn() { printf 'rig-forgejo-runner: WARNING: %s\n' "$*" >&2; } warn() { printf 'rig-forgejo-runner: WARNING: %s\n' "$*" >&2; }
@ -31,7 +33,32 @@ die() { printf 'rig-forgejo-runner: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; }
# on the box itself. No docker-in-docker: the guide this came from stacks a # on the box itself. No docker-in-docker: the guide this came from stacks a
# privileged dind sidecar with a plaintext tcp://…:2375 daemon to isolate jobs # privileged dind sidecar with a plaintext tcp://…:2375 daemon to isolate jobs
# from a shared CI server, and inside a box that boundary is already paid for. # from a shared CI server, and inside a box that boundary is already paid for.
DEFAULT_LABELS='ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:act-22.04,docker:docker://node:22-bookworm' #
# WHY act-22.04 (slim) for ubuntu-latest, not full-22.04 — measured 2026-08-01
# against ghcr manifests (#144):
# act-22.04: ~0.55 GB compressed / ~2.2 GB on disk — no shellcheck
# full-22.04: ~18.67 GB compressed / ~54.5 GB on disk — has shellcheck 0.8.0
# A normal box-class ci tenant cannot hold 54.5 GB (typical free space ~34 GB).
# So ubuntu-latest stays slim, and workflows must not assume GitHub-image tools
# (rig's own ci.yml installs shellcheck when missing). Operators who need the
# full tool surface opt in with runs-on: ubuntu-latest-full — that label is
# inert until matched, so boxes that never ask pay nothing.
DEFAULT_LABELS='ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:act-22.04,ubuntu-latest-full:docker://ghcr.io/catthehacker/ubuntu:full-22.04,docker:docker://node:22-bookworm'
# labels_are_a_retired_default <recorded>
#
# True when <recorded> is a past DEFAULT_LABELS value rig has shipped — the
# only plain-converge case that should warn about re-registration (#144).
# Custom operator maps (drill's Leg 3, any --labels) must return false so a
# bare re-run stays quiet. One pattern per past default; add when the string
# changes. Extracted and driven by test/cli.sh — a grep pin alone cannot prove
# the match is exact.
labels_are_a_retired_default() {
case "${1:-}" in
'ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:act-22.04,docker:docker://node:22-bookworm') return 0 ;;
*) return 1 ;;
esac
}
# fetch_and_verify_sha256 <asset-url> <file> <sumfile> <label> # fetch_and_verify_sha256 <asset-url> <file> <sumfile> <label>
# #
@ -93,8 +120,10 @@ usage: rig forgejo-runner install --instance <url> [options]
time). Pin it for a deterministic, auditable install. time). Pin it for a deterministic, auditable install.
--name <name> runner name (default: this host's hostname) --name <name> runner name (default: this host's hostname)
--labels <csv> runner labels; replaces the default. The default maps --labels <csv> runner labels; replaces the default. The default maps
ubuntu-latest and docker onto container images, so a ubuntu-latest (slim act image), ubuntu-latest-full
workflow written for GitHub runs unchanged. (opt-in parity image), and docker onto containers so
a workflow written for GitHub runs; full tools need
runs-on: ubuntu-latest-full or an install step.
--user <name> unprivileged service user (default: the tenant user --user <name> unprivileged service user (default: the tenant user
`ci` when it exists, else forgejo-runner; created if `ci` when it exists, else forgejo-runner; created if
absent; never root) absent; never root)
@ -202,6 +231,17 @@ fi
# --- guards ---------------------------------------------------------------- # --- guards ----------------------------------------------------------------
[ "$(id -u)" -eq 0 ] || die "must run as root" [ "$(id -u)" -eq 0 ] || die "must run as root"
# Root is not enough: the admin binaries must also be reachable (#139). This
# sits beside the root check so identity and capability are asserted together,
# and BEFORE any prompt or download — a token typed for a doomed run is waste.
#
# BOTH binaries this command goes on to call: useradd at the user-create below,
# usermod at the docker-group add further down. Naming only the first would
# still consume the token on a PATH that happened to resolve useradd but not
# usermod — the same failure one step later, which is the shape #75 exists to
# refuse (a sweep that covers most of its call sites is the hole the next bug
# arrives through).
require_admin_bins useradd usermod
if [ -r /etc/os-release ]; then if [ -r /etc/os-release ]; then
# Sourced in a subshell: os-release defines VERSION (e.g. "13 (trixie)"), # Sourced in a subshell: os-release defines VERSION (e.g. "13 (trixie)"),
# which would clobber this script's $VERSION. # which would clobber this script's $VERSION.
@ -408,14 +448,29 @@ install -d -m 0755 -o "$RUNNER_USER" -g "$RUNNER_GROUP" "$USER_HOME/.cache"
if [ -e "$RUNNER_DIR/.runner" ]; then if [ -e "$RUNNER_DIR/.runner" ]; then
log "already registered; skipping registration" log "already registered; skipping registration"
# Registration was skipped, so the labels on the instance are the ones it was # Registration was skipped, so the labels on the instance are the ones it was
# registered with — NOT whatever this invocation was passed. Say so when the # registered with — NOT whatever this invocation was passed. Forgejo owns
# operator explicitly asked for different ones, rather than letting the # labels from registration time; a re-run never rewrites them.
# request evaporate. Only when EXPLICIT: comparing the default against a #
# runner registered with custom labels would warn on every plain converge. # Two warn paths (#144):
if [ "$LABELS_EXPLICIT" -eq 1 ] && [ -r "$RUNNER_DIR/.rig-labels" ]; then # EXPLICIT --labels that differs → operator asked and it was not applied.
# Plain converge whose recorded labels match a known *retired* default →
# rig's default map moved (e.g. added ubuntu-latest-full). Without this
# the operator re-runs install, sees "already registered", and believes
# they have the new default while Forgejo still holds the old set.
#
# Do NOT warn on every RECORDED != current default: that fires forever for
# any runner the operator deliberately gave custom --labels (drill's Leg 3
# registers drill:docker://node:22-bookworm). The old LABELS_EXPLICIT-only
# gate existed to avoid that noise; retired-default matching keeps the
# silence for intentional maps and still catches silent drift off a past
# rig default. Re-register only to pick up labels the old set never had —
# nothing matching the recorded set is broken by the map change alone.
if [ -r "$RUNNER_DIR/.rig-labels" ]; then
RECORDED="$(cat "$RUNNER_DIR/.rig-labels")" RECORDED="$(cat "$RUNNER_DIR/.rig-labels")"
if [ "$RECORDED" != "$LABELS" ]; then if [ "$LABELS_EXPLICIT" -eq 1 ] && [ "$RECORDED" != "$LABELS" ]; then
warn "--labels was not applied: this runner is already registered, and Forgejo owns its labels from registration time. It still has: ${RECORDED}. Labels are what 'runs-on' matches, so changing them means re-registering: 'rig forgejo-runner remove' then install again with the labels you want." warn "--labels was not applied: this runner is already registered, and Forgejo owns its labels from registration time. It still has: ${RECORDED}. Labels are what 'runs-on' matches, so changing them means re-registering: 'rig forgejo-runner remove' then install again with the labels you want."
elif [ "$LABELS_EXPLICIT" -eq 0 ] && labels_are_a_retired_default "$RECORDED"; then
warn "this runner was registered with an older rig default label set. The current default adds ubuntu-latest-full (the GitHub-parity image). Labels are fixed at registration, so picking it up means re-registering: 'rig forgejo-runner remove' then install again. Nothing you run today is affected — re-register only if you want the new label."
fi fi
fi fi
else else

View file

@ -0,0 +1,47 @@
#!/usr/bin/env bash
# admin-path.sh — assert the admin binaries are REACHABLE, not merely that we
# are root.
#
# Being uid 0 and being able to find useradd are different facts, and rig
# asserted only the first. `su` without `-`, sudo with a sanitised secure_path,
# and several container images all hand you a root shell whose PATH carries no
# /usr/sbin — which is where useradd, usermod and groupadd live on Debian. The
# result was a bare `useradd: command not found` naming a line number inside a
# versioned install root, emitted AFTER a registration token had been read off
# the operator's terminal (#139).
#
# Which binaries this covers is measured, not assumed (Debian 13, 2026-08-01):
#
# useradd usermod groupadd userdel groupdel /usr/sbin package: passwd
# visudo /usr/sbin package: sudo
# gpasswd /usr/bin package: passwd
#
# Two consequences worth keeping written down. `gpasswd` is in the same PACKAGE
# as useradd but a different DIRECTORY, so it is reachable on a PATH-shorn root
# and does not belong in any of these preflights — do not add it for symmetry.
# And `visudo` shares the directory but not the package, so its absence has a
# second, innocent cause (sudo simply not installed) that the others do not, so
# `rig users apply` checks it separately, after the point where that cause is
# ruled out. See the comment there.
#
# (Spelled without the `.sh` on purpose: test/cli.sh pins that exactly one file
# under commands/ names that script, to catch a second caller appearing. A
# comment is not a caller, but the pin is deliberately blunt and cheap.)
#
# It REFUSES rather than repairing PATH itself. A command that quietly prepends
# /usr/sbin teaches the operator nothing and leaves a misconfigured host
# misconfigured; the same reason bootstrap refuses rather than guessing. The
# message carries the fix so the refusal costs one paste, not an investigation.
# require_admin_bins <bin>... — die unless every one resolves on PATH.
require_admin_bins() {
local missing=() b
for b in "$@"; do
command -v "$b" >/dev/null 2>&1 || missing+=("$b")
done
[ "${#missing[@]}" -eq 0 ] && return 0
# Names the REMEDY, not this script: the operator typed a `rig ...` command,
# and echoing the internal path back at them is the unhelpful half of the
# original `useradd: command not found`.
die "cannot find ${missing[*]} on PATH — it lives in /usr/sbin, which this root shell does not carry (a 'su' without '-' does this, and so do some container images). Re-run the same rig command with: PATH=/usr/sbin:/sbin:\$PATH"
}

View file

@ -9,6 +9,8 @@ set -euo pipefail
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
# shellcheck source=SCRIPTDIR/lib/runner-config.sh # shellcheck source=SCRIPTDIR/lib/runner-config.sh
. "$HERE/lib/runner-config.sh" . "$HERE/lib/runner-config.sh"
# shellcheck source=SCRIPTDIR/lib/admin-path.sh
. "$HERE/lib/admin-path.sh"
log() { printf 'rig-runner: %s\n' "$*"; } log() { printf 'rig-runner: %s\n' "$*"; }
warn() { printf 'rig-runner: WARNING: %s\n' "$*" >&2; } warn() { printf 'rig-runner: WARNING: %s\n' "$*" >&2; }
@ -85,6 +87,10 @@ VERSION="${VERSION#v}"
# --- guards ---------------------------------------------------------------- # --- guards ----------------------------------------------------------------
[ "$(id -u)" -eq 0 ] || die "must run as root" [ "$(id -u)" -eq 0 ] || die "must run as root"
# Root is not enough: the admin binaries must also be reachable (#139). This
# sits beside the root check so identity and capability are asserted together,
# and BEFORE any prompt or download — a token typed for a doomed run is waste.
require_admin_bins useradd
if [ -r /etc/os-release ]; then if [ -r /etc/os-release ]; then
# Sourced in a subshell: os-release defines VERSION (e.g. "13 (trixie)"), # Sourced in a subshell: os-release defines VERSION (e.g. "13 (trixie)"),
# which would clobber this script's $VERSION. # which would clobber this script's $VERSION.

View file

@ -11,6 +11,8 @@ set -euo pipefail
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)" HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
# shellcheck source=SCRIPTDIR/lib/users-config.sh # shellcheck source=SCRIPTDIR/lib/users-config.sh
. "$HERE/lib/users-config.sh" . "$HERE/lib/users-config.sh"
# shellcheck source=SCRIPTDIR/lib/admin-path.sh
. "$HERE/lib/admin-path.sh"
log() { printf 'rig-users: %s\n' "$*"; } log() { printf 'rig-users: %s\n' "$*"; }
warn() { printf 'rig-users: WARNING: %s\n' "$*" >&2; } warn() { printf 'rig-users: WARNING: %s\n' "$*" >&2; }
@ -136,6 +138,10 @@ done <<< "$PARSED"
# --- guards ------------------------------------------------------------------ # --- guards ------------------------------------------------------------------
[ "$(id -u)" -eq 0 ] || die "must run as root" [ "$(id -u)" -eq 0 ] || die "must run as root"
# Root is not enough: the admin binaries must also be reachable (#139). This
# sits beside the root check so identity and capability are asserted together,
# and BEFORE any prompt or download — a token typed for a doomed run is waste.
require_admin_bins useradd usermod groupadd
# Identity management gates its INVOKER, not just its uid: %rig's sudoers rule # Identity management gates its INVOKER, not just its uid: %rig's sudoers rule
# is binary-scoped but not argument-scoped, so without this gate a rig-role # is binary-scoped but not argument-scoped, so without this gate a rig-role
@ -195,6 +201,26 @@ if [ "$NEED_SUDO" -eq 1 ] && ! command -v sudo >/dev/null 2>&1; then
DEBIAN_FRONTEND=noninteractive apt-get install -y -qq sudo DEBIAN_FRONTEND=noninteractive apt-get install -y -qq sudo
CHANGED=1 CHANGED=1
fi fi
# visudo is checked HERE and not beside the root check, because until the block
# above has run there is a legitimate reason for it to be absent: sudo is not
# installed yet, and apply is what installs it. Above, a missing visudo would
# be indistinguishable from that, so the refusal would fire on a healthy box.
#
# Below, it is unambiguous. sudo is present, so `visudo` missing means only one
# thing: /usr/sbin is off PATH. And it MUST refuse here rather than be left to
# the sudoers block further down, because that block asks `command -v visudo`
# and treats false as "no sudo on the box means no role needed it" — which on a
# PATH-shorn root is FALSE TWICE. A role does need it, sudo is installed, and
# apply would finish reporting success having silently never written the
# sudoers drop-in: the users get their roles and not the escalation the roles
# are FOR. That is the failure this whole issue is about (a wrong effective
# state reported as success, #12), in its quietest form — the other three sites
# at least crash. Refusing before the first mutation is what keeps it loud.
#
# It sits before `groupadd` below, so nothing has been converged when it fires.
if [ "$NEED_SUDO" -eq 1 ]; then
require_admin_bins visudo
fi
# --- groups ------------------------------------------------------------------ # --- groups ------------------------------------------------------------------
groupadd -f rig-admin groupadd -f rig-admin

View file

@ -3334,6 +3334,121 @@ check "ci-box: its install.sh does not register" 1 "" \
check "ci-box: bootstrap-tenant does not read the staging dir" 1 "" \ check "ci-box: bootstrap-tenant does not read the staging dir" 1 "" \
grep -q 'docs/templates' "$ROOT/commands/bootstrap-tenant.sh" grep -q 'docs/templates' "$ROOT/commands/bootstrap-tenant.sh"
# --- admin binaries must be reachable, not just root (#139) ------------------
# Being root and being able to FIND the admin binaries are different facts, and
# rig asserted only the first. `su` without `-`, sudo with a sanitised
# secure_path, and several container images all give a root shell whose PATH
# carries no /usr/sbin — where useradd lives. Reported from a real ci-box:
#
# root@ci-forgejo-box:/home/dev# rig forgejo-runner install --instance …
# forgejo runner registration token:
# …/forgejo-runner-install.sh: line 250: useradd: command not found
#
# Note where it died: AFTER reading a registration token off the operator's
# terminal. A secret typed for a run that could never succeed is the avoidable
# half of the bug, so the refusal has to come before the prompt.
#
# The root check fires first and correctly, so these stub `id -u` to 0 — the
# idiom the bootstrap --undo block above already uses — to reach the preflight.
ADMPATH_DIR="$(mktemp -d)"
mkdir -p "$ADMPATH_DIR/bin"
# shellcheck disable=SC2016 # the body is shell source being written, not expanded
printf '#!/usr/bin/env bash\nif [ "${1:-}" = -u ]; then printf "0\\n"; else exec /usr/bin/id "$@"; fi\n' \
> "$ADMPATH_DIR/bin/id"
chmod +x "$ADMPATH_DIR/bin/id"
# A PATH with the stub and the ordinary bindirs, but deliberately no /usr/sbin.
SBINLESS="$ADMPATH_DIR/bin:/usr/local/bin:/usr/bin:/bin"
adm_run() { env PATH="$SBINLESS" "$@" 2>&1; }
adm_prompted() { # did it read a token before refusing?
adm_run "$@" | grep -qi 'registration token:'
}
check "preflight: forgejo-runner install refuses a PATH with no /usr/sbin" 1 "useradd" \
adm_run "$ROOT/commands/forgejo-runner-install.sh" --instance https://f.example.com
check "preflight: …and names PATH as the cause, not just the missing binary" 1 "PATH" \
adm_run "$ROOT/commands/forgejo-runner-install.sh" --instance https://f.example.com
check "preflight: …and refuses BEFORE prompting for a token" 1 "" \
adm_prompted "$ROOT/commands/forgejo-runner-install.sh" --instance https://f.example.com
check "preflight: the GitHub runner installer refuses too" 1 "useradd" \
adm_run "$ROOT/commands/runner-install.sh" --repo o/r
check "preflight: users apply refuses before it converges anything" 1 "useradd" \
adm_run "$ROOT/commands/users-apply.sh" --file /dev/null
# A sbin-less PATH loses ALL of /usr/sbin at once, so the checks above can only
# ever prove the FIRST binary is named — `useradd` wins every race and would
# hide a preflight that forgot the others. These fixtures resolve the earlier
# binaries and withhold exactly one, which is the only way to show the sweep
# covers what each command actually calls (the #75 lesson: a sweep that misses
# one call site is how the next bug gets in).
#
# The withheld binary is real: these are PATHs, not stubs of the tools
# themselves — a stub that silently succeeded would let convergence run on.
admstub() { # admstub <dir> <bin>... — a bindir resolving id(0) plus <bin>...
local d="$1"; shift
mkdir -p "$d"
# shellcheck disable=SC2016 # shell source being written, not expanded
printf '#!/usr/bin/env bash\nif [ "${1:-}" = -u ]; then printf "0\\n"; else exec /usr/bin/id "$@"; fi\n' > "$d/id"
chmod +x "$d/id"
local b
for b in "$@"; do printf '#!/usr/bin/env bash\nexit 0\n' > "$d/$b"; chmod +x "$d/$b"; done
}
adm_with() { # adm_with <bindir> <cmd>...
local d="$1"; shift
env PATH="$d:/usr/local/bin:/usr/bin:/bin" "$@" 2>&1
}
# adm_saw <bindir> <pattern> <cmd>... — exit 0 if the run printed <pattern>.
# Used with `check ... 1 ""` to assert a thing did NOT happen (a token prompt,
# a group creation), the same shape as adm_prompted above.
adm_saw() {
local d="$1" pat="$2"; shift 2
adm_with "$d" "$@" | grep -qi -- "$pat"
}
# useradd resolves, usermod does not — the forgejo installer calls both, and
# only reaches usermod after the token has been spent.
admstub "$ADMPATH_DIR/no-usermod" useradd
check "preflight: forgejo-runner install names usermod when only useradd resolves" 1 "usermod" \
adm_with "$ADMPATH_DIR/no-usermod" "$ROOT/commands/forgejo-runner-install.sh" --instance https://f.example.com
check "preflight: …and still refuses before the token prompt" 1 "" \
adm_saw "$ADMPATH_DIR/no-usermod" 'registration token:' \
"$ROOT/commands/forgejo-runner-install.sh" --instance https://f.example.com
# users apply calls groupadd unconditionally, two lines into its convergence.
admstub "$ADMPATH_DIR/no-groupadd" useradd usermod
check "preflight: users apply names groupadd when useradd and usermod resolve" 1 "groupadd" \
adm_with "$ADMPATH_DIR/no-groupadd" "$ROOT/commands/users-apply.sh" --file /dev/null
# visudo is the quiet one. With sudo INSTALLED and /usr/sbin off PATH, the
# sudoers block reads `command -v visudo` as "no sudo on the box" and skips the
# drop-in — apply then reports success having granted roles without the
# escalation those roles exist for. So this fixture needs a role that wants
# sudo, and asserts the refusal by name rather than a silent success.
ADM_USERS="$ADMPATH_DIR/users"
printf '%s\n' 'dan admin ssh-ed25519 AAAAC3fixture dan@laptop' > "$ADM_USERS"
admstub "$ADMPATH_DIR/no-visudo" useradd usermod groupadd
check "preflight: users apply refuses a sudo-needing role when visudo is unreachable" 1 "visudo" \
adm_with "$ADMPATH_DIR/no-visudo" "$ROOT/commands/users-apply.sh" --file "$ADM_USERS" --yes
# …and the refusal must land BEFORE the groups are converged, or the machine is
# already half-changed when the operator reads it.
check "preflight: …before any group is created" 1 "" \
adm_saw "$ADMPATH_DIR/no-visudo" 'rig-admin' \
"$ROOT/commands/users-apply.sh" --file "$ADM_USERS" --yes
# A users file needing no sudo must NOT be refused for a missing visudo: the
# guard has to be as narrow as the need, or it refuses healthy boxes.
printf '%s\n' 'maria ops ssh-ed25519 AAAAC3fixture maria@mac' > "$ADMPATH_DIR/users-nosudo"
check "preflight: …but a file with no sudo-backed role is not refused for visudo" 1 "" \
adm_saw "$ADMPATH_DIR/no-visudo" 'visudo' \
"$ROOT/commands/users-apply.sh" --file "$ADMPATH_DIR/users-nosudo" --yes
# The fourth call site (#139 review): bootstrap-tenant adds the tenant user to
# the docker group AFTER installing docker and node, so an unguarded PATH fails
# it mid-convergence on a machine it has already changed.
# staging-box because it is the one tenant role defined in rig's own tree: the
# others resolve through the template registry, and a preflight test must not
# depend on the network to reach the guard it is testing.
admstub "$ADMPATH_DIR/no-any"
check "preflight: bootstrap-tenant refuses a sbin-less PATH before it converges" 1 "usermod" \
adm_with "$ADMPATH_DIR/no-any" "$ROOT/commands/bootstrap-tenant.sh" staging-box
rm -rf "$ADMPATH_DIR"
# --- rig forgejo-runner (#109) ---------------------------------------------- # --- rig forgejo-runner (#109) ----------------------------------------------
FR="$ROOT/commands/forgejo-runner-install.sh" FR="$ROOT/commands/forgejo-runner-install.sh"
check "forgejo-runner: bare subcommand shows usage, exit 2" 2 "usage:" "$ROOT/bin/rig" forgejo-runner check "forgejo-runner: bare subcommand shows usage, exit 2" 2 "usage:" "$ROOT/bin/rig" forgejo-runner
@ -3349,12 +3464,22 @@ check "forgejo-runner: --version refuses a path, not a release number" 2 "releas
"$FR" --instance https://f.example.com --version ../../etc/passwd "$FR" --instance https://f.example.com --version ../../etc/passwd
check "forgejo-runner: --version refuses a non-numeric pin" 2 "release number like" \ check "forgejo-runner: --version refuses a non-numeric pin" 2 "release number like" \
"$FR" --instance https://f.example.com --version latest "$FR" --instance https://f.example.com --version latest
# Reaching the root check is the proof a good pin got THROUGH validation: this # Reaching a gate AFTER --version parsing is the proof a good pin got THROUGH
# runs as a normal user in CI, so "must run as root" is the next gate down. # validation. Which gate depends on the uid: non-root hits "must run as root";
check "forgejo-runner: a plain release number passes validation" 1 "must run as root" \ # act/Forgejo jobs run as uid 0 (no `runner` account — #144), so they sail past
"$FR" --instance https://f.example.com --version 12.13.2 # the root check and hit the unattended-token refuse instead. Both prove the
check "forgejo-runner: a leading v is stripped before that check" 1 "must run as root" \ # same thing. GitHub-hosted ubuntu-latest is non-root and takes the first arm.
"$FR" --instance https://f.example.com --version v12.13.2 if [ "$(id -u)" -ne 0 ]; then
check "forgejo-runner: a plain release number passes validation" 1 "must run as root" \
"$FR" --instance https://f.example.com --version 12.13.2
check "forgejo-runner: a leading v is stripped before that check" 1 "must run as root" \
"$FR" --instance https://f.example.com --version v12.13.2
else
check "forgejo-runner: a plain release number passes validation" 1 "FORGEJO_RUNNER_TOKEN is unset" \
env -u FORGEJO_RUNNER_TOKEN "$FR" --instance https://f.example.com --version 12.13.2
check "forgejo-runner: a leading v is stripped before that check" 1 "FORGEJO_RUNNER_TOKEN is unset" \
env -u FORGEJO_RUNNER_TOKEN "$FR" --instance https://f.example.com --version v12.13.2
fi
# A schemeless host and a repo URL are the two ways an operator mis-states the # A schemeless host and a repo URL are the two ways an operator mis-states the
# instance, and only one of them would fail loudly on its own — a repo URL # instance, and only one of them would fail loudly on its own — a repo URL
# registers somewhere subtly wrong instead. Both refuse by name. # registers somewhere subtly wrong instead. Both refuse by name.
@ -3704,6 +3829,44 @@ check "forgejo-runner: an explicit --labels on a rerun warns it was not applied"
grep -o -- "--labels was not applied" "$FR" grep -o -- "--labels was not applied" "$FR"
check "forgejo-runner: that warning is gated on --labels being EXPLICIT" 0 "LABELS_EXPLICIT" \ check "forgejo-runner: that warning is gated on --labels being EXPLICIT" 0 "LABELS_EXPLICIT" \
grep -o "LABELS_EXPLICIT" "$FR" grep -o "LABELS_EXPLICIT" "$FR"
# #144 option B: plain converge warns only for known *retired* defaults — not
# for every RECORDED != current default (that would noise custom --labels
# forever, including drill's Leg 3). Drive the recogniser against fixtures
# (test/drill.sh extraction pattern) so a dead matcher cannot greppen green.
check "forgejo-runner: plain converge warns on a known retired default label set" 0 "registered with an older rig default label set" \
grep -o "registered with an older rig default label set" "$FR"
check "forgejo-runner: that warn says re-register only if you want the new label" 0 "re-register only if you want the new label" \
grep -o "re-register only if you want the new label" "$FR"
PRE_144_LABELS='ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:act-22.04,docker:docker://node:22-bookworm'
CURRENT_LABELS="$(sed -n "s/^DEFAULT_LABELS='\\(.*\\)'$/\\1/p" "$FR")"
RETIRED_FNS="$(mktemp)"
awk '/^labels_are_a_retired_default\(\) \{/,/^\}/' "$FR" > "$RETIRED_FNS"
check "extraction guards the awk: labels_are_a_retired_default() landed" 0 "labels_are_a_retired_default() {" \
grep -F 'labels_are_a_retired_default() {' "$RETIRED_FNS"
# shellcheck source=/dev/null
. "$RETIRED_FNS"
check "retired-default: the pre-#144 default is recognised" 0 "" \
labels_are_a_retired_default "$PRE_144_LABELS"
check "retired-default: the CURRENT default is not drift" 1 "" \
labels_are_a_retired_default "$CURRENT_LABELS"
check "retired-default: an operator's own --labels map is never drift" 1 "" \
labels_are_a_retired_default 'drill:docker://node:22-bookworm'
check "retired-default: a near-miss of a past default is not a match" 1 "" \
labels_are_a_retired_default "${PRE_144_LABELS} "
rm -f "$RETIRED_FNS"
# Default map: slim ubuntu-latest (act), opt-in full, docker — pin the three
# so a silent drop of the full rider or a flip back to full-as-default fails.
check "forgejo-runner: DEFAULT_LABELS maps ubuntu-latest to act-22.04 (slim)" 0 "ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:act-22.04" \
grep -o "ubuntu-latest:docker://ghcr.io/catthehacker/ubuntu:act-22.04" "$FR"
check "forgejo-runner: DEFAULT_LABELS offers ubuntu-latest-full as opt-in" 0 "ubuntu-latest-full:docker://ghcr.io/catthehacker/ubuntu:full-22.04" \
grep -o "ubuntu-latest-full:docker://ghcr.io/catthehacker/ubuntu:full-22.04" "$FR"
check "forgejo-runner: DEFAULT_LABELS comment records the slim/full size measurement" 0 "54.5 GB" \
grep -o "54.5 GB" "$FR"
# ci.yml must install shellcheck when the image lacks it (#144 option B).
check "ci.yml: shellcheck step installs the tool when missing" 0 "command -v shellcheck" \
grep -o "command -v shellcheck" "$ROOT/.github/workflows/ci.yml"
check "ci.yml: shellcheck install uses sudo (GitHub path; no-op on act as root)" 0 "sudo apt-get install -y shellcheck" \
grep -o "sudo apt-get install -y shellcheck" "$ROOT/.github/workflows/ci.yml"
# The GitHub sibling is the precedent this restores — pin that it still scopes # The GitHub sibling is the precedent this restores — pin that it still scopes
# its own write, so the two cannot drift apart again. # its own write, so the two cannot drift apart again.
gh_labels_write_is_scoped() { gh_labels_write_is_scoped() {