Merge pull request #14 from claude-hdb/fix/runner-install-repo-guard

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

.github/workflows/ci.yml

@@ -9,6 +9,14 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - name: shellcheck
-        run: shellcheck install.sh bin/rig commands/*.sh test/cli.sh
+        # -x follows the `source=SCRIPTDIR/...` directives into commands/lib/.
+        # globstar so a script in a new subdirectory is linted without anyone
+        # remembering to edit this list; bin/* covers the extensionless entrypoints.
+        # The file list is printed so under-coverage shows up in the log.
+        run: |
+          shopt -s globstar
+          files=(bin/* **/*.sh)
+          printf 'shellcheck: %s\n' "${files[@]}"
+          shellcheck -x "${files[@]}"
       - name: cli tests
         run: bash test/cli.sh

README.md

@@ -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`
 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`
 
@@ -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
 existing name unless you pass `--name`.
 
-This is the verb that was missing. `runner install` is convergent *by
-skipping* — it sees a registered runner and leaves it alone — so it can
-create a runner but never move one, and re-pointing a box meant hand-rolled
-`config.sh`/`svc.sh` incantations against an install path only rig knew.
+This is the verb that was missing. `runner install` can create a runner but
+never move one — pointed at a repo the box is not on, it fails and sends you
+here — and re-pointing a box otherwise meant hand-rolled `config.sh`/`svc.sh`
+incantations against an install path only rig knew.
 
 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

commands/lib/runner-config.sh

@@ -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
+}

commands/runner-install.sh

@@ -1,9 +1,15 @@
 #!/usr/bin/env bash
 # rig runner install — GitHub Actions self-hosted runner as a systemd service
 # 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
 
+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' "$*"; }
 warn() { printf 'rig-runner: WARNING: %s\n' "$*" >&2; }
 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:
   gh api -X POST repos/<owner/repo>/actions/runners/registration-token).
 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
 }
 
@@ -88,17 +99,25 @@ else
 fi
 command -v curl >/dev/null || die "curl is required (run rig bootstrap first)"
 
-# --- registration token — only when registration is actually pending -------
-# Pending unless the runner user already exists AND $RUNNER_DIR/.runner
-# exists (user absent => nothing can be registered => pending).
+# --- is this box already registered somewhere else? --------------------------
+# Before anything is prompted for, downloaded, or started: --repo must agree
+# 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
 if id -u "$RUNNER_USER" >/dev/null 2>&1; then
   USER_HOME="$(getent passwd "$RUNNER_USER" | cut -d: -f6)"
   RUNNER_DIR="$USER_HOME/actions-runner"
+  assert_runner_repo "$RUNNER_DIR" "$REPO" || exit 1
   if [ -e "$RUNNER_DIR/.runner" ]; then
     REG_PENDING=0
   fi
 fi
+
+# --- registration token — only when registration is actually pending -------
 if [ "$REG_PENDING" -eq 1 ]; then
   RUNNER_TOKEN="${RUNNER_TOKEN:-}"
   if [ -z "$RUNNER_TOKEN" ]; then

commands/runner-repoint.sh

@@ -5,6 +5,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"
 
 log()  { printf 'rig-runner: %s\n' "$*"; }
 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"
 
 # --- what is it registered to now? ------------------------------------------
-json_field() {
-  grep -o "\"$2\"[[:space:]]*:[[:space:]]*\"[^\"]*\"" "$1" \
-    | head -n1 | sed 's/.*:[[:space:]]*"//; s/"$//'
-}
-CURRENT_URL="$(json_field "$RUNNER_DIR/.runner" gitHubUrl)"
+CURRENT_URL="$(runner_repo_url "$RUNNER_DIR")"
 TARGET_URL="https://github.com/${REPO}"
 
 if [ "$CURRENT_URL" = "$TARGET_URL" ]; then
@@ -108,7 +106,7 @@ fi
 
 # Keep the runner's identity across the move unless told otherwise.
 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"
 fi
 if [ -z "$LABELS" ]; then

commands/runner-status.sh

@@ -3,6 +3,10 @@
 # Read-only: reports what is already on the box. No credential, no network call.
 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' "$*"; }
 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}"
 
 # --- read the runner's own config -------------------------------------------
-# .runner is JSON. Kept dependency-free on purpose: a rig-bootstrapped box has
-# no jq, and installing one to read five fields would be a poor trade.
-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)"
+REPO_URL="$(runner_repo_url "$RUNNER_DIR")"
+RUNNER_NAME="$(runner_agent_name "$RUNNER_DIR")"
 
 # 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

test/cli.sh

@@ -82,6 +82,51 @@ fi
 
 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: 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