diff --git a/changelog.d/139.md b/changelog.d/139.md new file mode 100644 index 0000000..2384383 --- /dev/null +++ b/changelog.d/139.md @@ -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) diff --git a/commands/forgejo-runner-install.sh b/commands/forgejo-runner-install.sh index 902bff6..ea217dd 100755 --- a/commands/forgejo-runner-install.sh +++ b/commands/forgejo-runner-install.sh @@ -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. diff --git a/commands/lib/admin-path.sh b/commands/lib/admin-path.sh new file mode 100644 index 0000000..6d70991 --- /dev/null +++ b/commands/lib/admin-path.sh @@ -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 ... — 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" +} diff --git a/commands/runner-install.sh b/commands/runner-install.sh index 6dcd97f..e8800f6 100755 --- a/commands/runner-install.sh +++ b/commands/runner-install.sh @@ -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. diff --git a/commands/users-apply.sh b/commands/users-apply.sh index 67fca52..263f8d4 100755 --- a/commands/users-apply.sh +++ b/commands/users-apply.sh @@ -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 diff --git a/test/cli.sh b/test/cli.sh index 704b94b..0f0cd3c 100644 --- a/test/cli.sh +++ b/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