fix(runner): install refuses a box registered to another repo

`rig runner install --repo <B>` on a box already registered to repo A
treated the mere existence of .runner as "already registered", skipped
configure, restarted the service still pointed at A, and reported success.
--repo was accepted, validated, and then ignored — leaving B with zero
runners and its `runs-on` jobs queued against one that will never come.

This is the natural next command after a partial `repoint`, and the failure
is worse than a no-op: moving a runner between repos is a trust-boundary
act, so quietly putting it back on the old one defeats the point of the move.

Gate install on the repo .runner actually names. Convergence — the property
worth keeping — is untouched: re-running against the repo the box is already
on still skips registration, never prompts for a token, and exits 0.
Skipping when the repo *differs* was never convergence, only a silently
ignored argument, so it now fails and names both repos, pointing at
`runner repoint` (move) or `runner remove` (start over). An unreadable
.runner is refused too — it is no licence to assume a match.

The .runner reader that `status` and `repoint` each carried is lifted into
commands/lib/runner-config.sh, which now also holds the guard. Its json_field
no longer dies bare under `set -o pipefail` when a key is missing, which is
what `status`'s own ${REPO_URL:-unknown} fallback always assumed.

Tests: the guard is exercised against a fixture .runner (refuses another repo
naming both, points at repoint, no-ops on the same repo, passes an
unregistered box, refuses an unreadable one) plus an ordering assertion that
it precedes svc.sh start — reaching it through the CLI would need root and a
really-registered runner, which the dependency-free harness cannot fabricate.
All three mutants (guard deleted, guard comparing nothing, guard moved below
the service start) go red.

Closes #13
This commit is contained in:
claude-hdb 2026-07-13 14:57:28 +00:00
parent deb6f653e3
commit d4ab362964
6 changed files with 161 additions and 24 deletions

View file

@ -168,7 +168,14 @@ from stale runners, so freezing it would just make it silently stop taking
work. The install-time version is a starting point either way; `--version` work. The install-time version is a starting point either way; `--version`
exists for when you want that starting point deterministic and auditable. exists for when you want that starting point deterministic and auditable.
Convergent — safe to re-run; an already-registered runner is left alone. Convergent **toward `--repo`** — re-running against the repo this box is
already on re-uses the binary, skips registration, and never asks for a token.
Pointed at a *different* repo it **refuses**, and names both: skipping there
would not be convergence, it would be ignoring the argument — restarting the
runner on the **old** repo while reporting success, leaving the repo you asked
for with no runner and its `runs-on` jobs queued forever. Moving a runner
between repos is a trust-boundary act; that verb is
[`rig runner repoint`](#rig-runner-repoint---repo-ownerrepo).
### `rig runner status` ### `rig runner status`
@ -225,10 +232,10 @@ Moves an installed runner from one repository to another: deregister,
re-register, reusing the binary already on the box. It keeps the runner's re-register, reusing the binary already on the box. It keeps the runner's
existing name unless you pass `--name`. existing name unless you pass `--name`.
This is the verb that was missing. `runner install` is convergent *by This is the verb that was missing. `runner install` can create a runner but
skipping* — it sees a registered runner and leaves it alone — so it can never move one — pointed at a repo the box is not on, it fails and sends you
create a runner but never move one, and re-pointing a box meant hand-rolled here — and re-pointing a box otherwise meant hand-rolled `config.sh`/`svc.sh`
`config.sh`/`svc.sh` incantations against an install path only rig knew. incantations against an install path only rig knew.
Two short-lived tokens, each minted from **its own** repo — `RUNNER_REMOVE_TOKEN` Two short-lived tokens, each minted from **its own** repo — `RUNNER_REMOVE_TOKEN`
for the one it's leaving, `RUNNER_TOKEN` for the one it's joining. Both are for the one it's leaving, `RUNNER_TOKEN` for the one it's joining. Both are

View file

@ -0,0 +1,71 @@
#!/usr/bin/env bash
# Shared reader for the runner's own on-disk config ($RUNNER_DIR/.runner).
# Sourced by the runner-* commands; never executed on its own.
# .runner is JSON, parsed here with grep/sed on purpose: a rig-bootstrapped box
# has no jq, and installing one to read two fields would be a poor trade.
#
# json_field <file> <key> — the first string value for <key>, empty if absent.
# Never fails: callers run under `set -e` with pipefail, where a grep that
# matches nothing would otherwise kill the script with no message. A missing
# key is a fact to test for, not an error to die on.
json_field() {
grep -o "\"$2\"[[:space:]]*:[[:space:]]*\"[^\"]*\"" "$1" 2>/dev/null \
| head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//' || true
}
# runner_repo_url <runner_dir> — the repository this box's runner is registered
# to, empty when nothing is registered there.
runner_repo_url() {
[ -e "$1/.runner" ] || return 0
json_field "$1/.runner" gitHubUrl
}
# runner_agent_name <runner_dir> — the runner's name, empty when unregistered.
runner_agent_name() {
[ -e "$1/.runner" ] || return 0
json_field "$1/.runner" agentName
}
# assert_runner_repo <runner_dir> <owner/repo>
#
# Returns 0 when the box has no runner, or has one already registered to
# <owner/repo>: re-running `install` against the repo the box is already on is
# real convergence — it re-uses the binary, skips registration, exits 0.
#
# Returns 1, explaining itself on stderr, when the runner is registered to a
# DIFFERENT repo. Skipping *that* is not convergence, it is ignoring the
# argument: `install` would skip its configure step, restart the service on the
# OLD repo, and report success — leaving the repo you asked for with no runner
# and its jobs queued against one that will never come. Moving a runner between
# repos is a trust-boundary act, so it belongs to `repoint`, out loud.
assert_runner_repo() {
local dir="$1" repo="$2" current wanted
[ -e "$dir/.runner" ] || return 0
current="$(runner_repo_url "$dir")"
wanted="https://github.com/${repo}"
if [ -z "$current" ]; then
printf 'rig-runner: ERROR: %s\n' \
"${dir}/.runner exists but names no repository — this box's registration cannot
be read, so rig cannot tell whether it is already on ${wanted}.
Wipe the local registration and install again:
rig runner remove --local" >&2
return 1
fi
if [ "$current" = "$wanted" ]; then
return 0
fi
printf 'rig-runner: ERROR: %s\n' \
"this box's runner is already registered to ${current}, not ${wanted}.
install will not move a runner between repositories: it would leave the service
running against the OLD repo and report success. To move it in one act:
rig runner repoint --repo ${repo}
or take it off the old repo first, then install:
rig runner remove (deregisters from ${current}; needs a removal token)
rig runner remove --local (when you cannot mint one)" >&2
return 1
}

View file

@ -1,9 +1,15 @@
#!/usr/bin/env bash #!/usr/bin/env bash
# rig runner install — GitHub Actions self-hosted runner as a systemd service # rig runner install — GitHub Actions self-hosted runner as a systemd service
# under an unprivileged user. Outbound-only (long-poll to GitHub), no Docker. # under an unprivileged user. Outbound-only (long-poll to GitHub), no Docker.
# Convergent: safe to re-run; an already-registered runner is left alone. # Convergent toward --repo: re-running against the repo the box is already on
# leaves it alone; a box registered to a DIFFERENT repo is refused, never
# silently restarted on the old one (that is `repoint`'s job).
set -euo pipefail set -euo pipefail
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
# shellcheck source=SCRIPTDIR/lib/runner-config.sh
. "$HERE/lib/runner-config.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; }
die() { printf 'rig-runner: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; } die() { printf 'rig-runner: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; }
@ -33,6 +39,11 @@ the interactive prompt (get one from the repo's Settings > Actions >
Runners > "New self-hosted runner", or: Runners > "New self-hosted runner", or:
gh api -X POST repos/<owner/repo>/actions/runners/registration-token). gh api -X POST repos/<owner/repo>/actions/runners/registration-token).
It is consumed at registration and never written to disk by rig. It is consumed at registration and never written to disk by rig.
Convergent toward --repo: re-running against the repo this box is already on
re-uses the binary, skips registration, and never asks for a token. A box
registered to a DIFFERENT repo is refused — moving a runner is
`rig runner repoint --repo <owner/repo>`.
EOF EOF
} }
@ -88,17 +99,25 @@ else
fi fi
command -v curl >/dev/null || die "curl is required (run rig bootstrap first)" command -v curl >/dev/null || die "curl is required (run rig bootstrap first)"
# --- registration token — only when registration is actually pending ------- # --- is this box already registered somewhere else? --------------------------
# Pending unless the runner user already exists AND $RUNNER_DIR/.runner # Before anything is prompted for, downloaded, or started: --repo must agree
# exists (user absent => nothing can be registered => pending). # with what is already on the box. Everything below this point treats an
# existing .runner as "nothing to do" — which is right for the repo the box is
# already on, and silently wrong for any other. See assert_runner_repo.
#
# Registration is pending unless the runner user already exists AND
# $RUNNER_DIR/.runner exists (user absent => nothing can be registered).
REG_PENDING=1 REG_PENDING=1
if id -u "$RUNNER_USER" >/dev/null 2>&1; then if id -u "$RUNNER_USER" >/dev/null 2>&1; then
USER_HOME="$(getent passwd "$RUNNER_USER" | cut -d: -f6)" USER_HOME="$(getent passwd "$RUNNER_USER" | cut -d: -f6)"
RUNNER_DIR="$USER_HOME/actions-runner" RUNNER_DIR="$USER_HOME/actions-runner"
assert_runner_repo "$RUNNER_DIR" "$REPO" || exit 1
if [ -e "$RUNNER_DIR/.runner" ]; then if [ -e "$RUNNER_DIR/.runner" ]; then
REG_PENDING=0 REG_PENDING=0
fi fi
fi fi
# --- registration token — only when registration is actually pending -------
if [ "$REG_PENDING" -eq 1 ]; then if [ "$REG_PENDING" -eq 1 ]; then
RUNNER_TOKEN="${RUNNER_TOKEN:-}" RUNNER_TOKEN="${RUNNER_TOKEN:-}"
if [ -z "$RUNNER_TOKEN" ]; then if [ -z "$RUNNER_TOKEN" ]; then

View file

@ -5,6 +5,8 @@
set -euo pipefail 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
. "$HERE/lib/runner-config.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; }
@ -94,11 +96,7 @@ RUNNER_DIR="$USER_HOME/actions-runner"
|| die "no runner registered in ${RUNNER_DIR} — use: rig runner install" || die "no runner registered in ${RUNNER_DIR} — use: rig runner install"
# --- what is it registered to now? ------------------------------------------ # --- what is it registered to now? ------------------------------------------
json_field() { CURRENT_URL="$(runner_repo_url "$RUNNER_DIR")"
grep -o "\"$2\"[[:space:]]*:[[:space:]]*\"[^\"]*\"" "$1" \
| head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//'
}
CURRENT_URL="$(json_field "$RUNNER_DIR/.runner" gitHubUrl)"
TARGET_URL="https://github.com/${REPO}" TARGET_URL="https://github.com/${REPO}"
if [ "$CURRENT_URL" = "$TARGET_URL" ]; then if [ "$CURRENT_URL" = "$TARGET_URL" ]; then
@ -108,7 +106,7 @@ fi
# Keep the runner's identity across the move unless told otherwise. # Keep the runner's identity across the move unless told otherwise.
if [ -z "$RUNNER_NAME" ]; then if [ -z "$RUNNER_NAME" ]; then
RUNNER_NAME="$(json_field "$RUNNER_DIR/.runner" agentName)" RUNNER_NAME="$(runner_agent_name "$RUNNER_DIR")"
[ -n "$RUNNER_NAME" ] || die "could not read the current runner name from ${RUNNER_DIR}/.runner" [ -n "$RUNNER_NAME" ] || die "could not read the current runner name from ${RUNNER_DIR}/.runner"
fi fi
if [ -z "$LABELS" ]; then if [ -z "$LABELS" ]; then

View file

@ -3,6 +3,10 @@
# Read-only: reports what is already on the box. No credential, no network call. # Read-only: reports what is already on the box. No credential, no network call.
set -euo pipefail set -euo pipefail
HERE="$(cd "$(dirname "$(readlink -f "${BASH_SOURCE[0]}")")" && pwd)"
# shellcheck source=SCRIPTDIR/lib/runner-config.sh
. "$HERE/lib/runner-config.sh"
log() { printf 'rig-runner: %s\n' "$*"; } log() { printf 'rig-runner: %s\n' "$*"; }
die() { printf 'rig-runner: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; } die() { printf 'rig-runner: ERROR: %s\n' "$1" >&2; exit "${2:-1}"; }
@ -47,15 +51,8 @@ RUNNER_DIR="$USER_HOME/actions-runner"
|| die "no runner registered in ${RUNNER_DIR}" || die "no runner registered in ${RUNNER_DIR}"
# --- read the runner's own config ------------------------------------------- # --- read the runner's own config -------------------------------------------
# .runner is JSON. Kept dependency-free on purpose: a rig-bootstrapped box has REPO_URL="$(runner_repo_url "$RUNNER_DIR")"
# no jq, and installing one to read five fields would be a poor trade. RUNNER_NAME="$(runner_agent_name "$RUNNER_DIR")"
json_field() {
grep -o "\"$2\"[[:space:]]*:[[:space:]]*\"[^\"]*\"" "$1" \
| head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//'
}
REPO_URL="$(json_field "$RUNNER_DIR/.runner" gitHubUrl)"
RUNNER_NAME="$(json_field "$RUNNER_DIR/.runner" agentName)"
# GitHub owns the labels; the runner does not persist them locally. rig records # GitHub owns the labels; the runner does not persist them locally. rig records
# what it registered with, so a box installed before this existed reports the # what it registered with, so a box installed before this existed reports the

View file

@ -82,6 +82,51 @@ fi
check "runner: bad subcommand exits 2" 2 "usage:" "$ROOT/bin/rig" runner frobnicate check "runner: bad subcommand exits 2" 2 "usage:" "$ROOT/bin/rig" runner frobnicate
# --- runner install: --repo must agree with what the box is already on -------
# The bug: `install --repo B` on a box registered to repo A skipped configure,
# restarted the service on A, and reported success — --repo accepted, validated,
# then ignored. The guard is exercised here through the shared lib, against a
# fixture .runner: reaching it via the CLI needs root AND a really-registered
# runner, neither of which this harness can fabricate.
guard() { # guard <runner_dir> <owner/repo>
bash -c 'set -euo pipefail
. "$1/commands/lib/runner-config.sh"
assert_runner_repo "$2" "$3"' _ "$ROOT" "$1" "$2"
}
REG_DIR="$(mktemp -d)" # a box registered to acme/alpha
EMPTY_DIR="$(mktemp -d)" # a box with no runner at all
printf '%s\n' '{"agentId":7,"agentName":"ci-box","gitHubUrl":"https://github.com/acme/alpha","workFolder":"_work"}' \
> "$REG_DIR/.runner"
check "runner install: refuses a repo the box is not registered to" \
1 "already registered to https://github.com/acme/alpha" guard "$REG_DIR" acme/beta
check "runner install: the refusal names the repo that was asked for" \
1 "not https://github.com/acme/beta" guard "$REG_DIR" acme/beta
check "runner install: the refusal points at repoint" \
1 "rig runner repoint --repo acme/beta" guard "$REG_DIR" acme/beta
# Convergence is the property worth keeping: same repo stays a clean no-op.
check "runner install: the repo it is already on is a no-op" \
0 "" guard "$REG_DIR" acme/alpha
check "runner install: an unregistered box passes the guard" \
0 "" guard "$EMPTY_DIR" acme/beta
# A .runner rig cannot read is not a licence to assume it matches.
printf '%s\n' '{"agentName":"ci-box"}' > "$REG_DIR/.runner"
check "runner install: refuses an unreadable registration" \
1 "names no repository" guard "$REG_DIR" acme/alpha
rm -rf "$REG_DIR" "$EMPTY_DIR"
# The guard is only worth something if it runs BEFORE the box is touched: the
# token prompt, the download, configure and svc.sh start all come after it.
# Ordering is the whole fix, so assert it rather than trust it.
# Matches the CALL, not the word: the comment above it mentions assert_runner_repo
# too, and a plain grep would keep finding that after the call itself was deleted.
# The defaults fail closed, so a guard that is gone cannot read as one that merely
# sits early in the file.
guard_at="$(grep -nE '^[[:space:]]*assert_runner_repo ' "$ROOT/commands/runner-install.sh" | head -n1 | cut -d: -f1)"
start_at="$(grep -n 'svc.sh start' "$ROOT/commands/runner-install.sh" | head -n1 | cut -d: -f1)"
check "runner install: the repo guard precedes svc.sh start" \
0 "" test "${guard_at:-999999}" -lt "${start_at:-0}"
check "runner status: --help exits 0" 0 "usage:" "$ROOT/commands/runner-status.sh" --help check "runner status: --help exits 0" 0 "usage:" "$ROOT/commands/runner-status.sh" --help
check "runner status: user needs value" 2 "needs a value" "$ROOT/commands/runner-status.sh" --user check "runner status: user needs value" 2 "needs a value" "$ROOT/commands/runner-status.sh" --user
check "runner status: refuses --user root" 2 "must not be root" "$ROOT/commands/runner-status.sh" --user root check "runner status: refuses --user root" 2 "must not be root" "$ROOT/commands/runner-status.sh" --user root