fix: refuse a PATH without /usr/sbin, before the token prompt
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>
This commit is contained in:
parent
9cb81c9f6b
commit
7f2501d0fe
6 changed files with 90 additions and 0 deletions
3
changelog.d/139.md
Normal file
3
changelog.d/139.md
Normal file
|
|
@ -0,0 +1,3 @@
|
|||
### Fixed
|
||||
|
||||
- `rig forgejo-runner install`, `rig runner install` and `rig users apply` 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)
|
||||
|
|
@ -17,6 +17,8 @@ set -euo pipefail
|
|||
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
|
||||
# shellcheck source=SCRIPTDIR/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' "$*"; }
|
||||
warn() { printf 'rig-forgejo-runner: WARNING: %s\n' "$*" >&2; }
|
||||
|
|
@ -202,6 +204,10 @@ fi
|
|||
|
||||
# --- guards ----------------------------------------------------------------
|
||||
[ "$(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
|
||||
# Sourced in a subshell: os-release defines VERSION (e.g. "13 (trixie)"),
|
||||
# which would clobber this script's $VERSION.
|
||||
|
|
|
|||
29
commands/lib/admin-path.sh
Normal file
29
commands/lib/admin-path.sh
Normal file
|
|
@ -0,0 +1,29 @@
|
|||
#!/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).
|
||||
#
|
||||
# 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"
|
||||
}
|
||||
|
|
@ -9,6 +9,8 @@ set -euo pipefail
|
|||
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
|
||||
# shellcheck source=SCRIPTDIR/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' "$*"; }
|
||||
warn() { printf 'rig-runner: WARNING: %s\n' "$*" >&2; }
|
||||
|
|
@ -85,6 +87,10 @@ VERSION="${VERSION#v}"
|
|||
|
||||
# --- guards ----------------------------------------------------------------
|
||||
[ "$(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
|
||||
# Sourced in a subshell: os-release defines VERSION (e.g. "13 (trixie)"),
|
||||
# which would clobber this script's $VERSION.
|
||||
|
|
|
|||
|
|
@ -11,6 +11,8 @@ set -euo pipefail
|
|||
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
|
||||
# shellcheck source=SCRIPTDIR/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' "$*"; }
|
||||
warn() { printf 'rig-users: WARNING: %s\n' "$*" >&2; }
|
||||
|
|
@ -136,6 +138,10 @@ done <<< "$PARSED"
|
|||
|
||||
# --- guards ------------------------------------------------------------------
|
||||
[ "$(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
|
||||
|
||||
# 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
|
||||
|
|
|
|||
40
test/cli.sh
40
test/cli.sh
|
|
@ -3334,6 +3334,46 @@ check "ci-box: its install.sh does not register" 1 "" \
|
|||
check "ci-box: bootstrap-tenant does not read the staging dir" 1 "" \
|
||||
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
|
||||
rm -rf "$ADMPATH_DIR"
|
||||
|
||||
# --- rig forgejo-runner (#109) ----------------------------------------------
|
||||
FR="$ROOT/commands/forgejo-runner-install.sh"
|
||||
check "forgejo-runner: bare subcommand shows usage, exit 2" 2 "usage:" "$ROOT/bin/rig" forgejo-runner
|
||||
|
|
|
|||
Loading…
Reference in a new issue