fix: the ci-box token guidance says what a Forgejo token actually is
Round finding from @codex-reviewer-andresmgsl, elevated to blocking by
@grok-reviewer-andresmgsl and @kimi-reviewer-andresmgsl. Unanimous, and right.
creds.md called the registration token "short-lived" and said it was "consumed
at registration". Both are GitHub's facts, copied across the forge boundary
with the rest of the sibling's shape. Forgejo's primary source, read rather
than inferred:
models/actions/runner_token.go — ActionRunnerToken has NO expiry field. Only
IsActive, Created, Updated. NewRunnerToken flips IsActive false on prior
tokens at the same scope and only there, so a token dies when somebody mints
its replacement, never on a clock.
routers/api/actions/runner/runner.go — Register reads the token, refuses it
when !IsActive ("please use the latest one"), and returns WITHOUT setting
IsActive = false. Registration does not spend it. One token registers as many
runners as it is shown to.
So it is long-lived and reusable — the precise opposite of the adjective, and
GitHub's really does expire in about an hour, which is why runner-install.sh is
correct to use it.
This is not a wording nit because of where the wording lives. creds.md is
spliced into the ci-box's own CONTEXT.md: it is the paragraph an agent INSIDE
the box reads about its own credentials. Telling that reader the token
self-expires is telling it a leaked one stops mattering on its own, while it is
still registering runners.
Pinned, not merely fixed, per codex's ask — the phrase arrived by copying from
the GitHub sibling, so the same copy can bring it back. Four rows: absence from
both files, and presence of the true claim, so the pin cannot be satisfied by
deleting the sentence instead of correcting it. The first draft of the CIBOX
pin was a phrase match and passed against the exact text it was written to
catch — the old wording wrapped across two comment lines. It is a plain absence
check now, and the file explains the ban without spelling the word.
Mutation-checked: all four go red against the old wording, green after.
This commit is contained in:
parent
25d10b084d
commit
fbdce5284e
3 changed files with 60 additions and 8 deletions
18
docs/templates/ci-box/creds.md
vendored
18
docs/templates/ci-box/creds.md
vendored
|
|
@ -1,9 +1,13 @@
|
||||||
- **Creds-free by default.** The box starts with no Forgejo credentials and no
|
- **Creds-free by default.** The box starts with no Forgejo credentials and no
|
||||||
git credentials. The runner binary is installed but **not registered**:
|
git credentials. The runner binary is installed but **not registered**:
|
||||||
registration needs a short-lived token the operator mints in Forgejo
|
registration needs a token the operator mints in Forgejo (Site
|
||||||
(Site Administration, org, or repo → Actions → Runners) and hands to
|
Administration, org, or repo → Actions → Runners) and hands to
|
||||||
`sudo rig forgejo-runner install --instance <url>`. The token is consumed at
|
`sudo rig forgejo-runner install --instance <url>`. rig never writes that
|
||||||
registration and never written to disk by rig. After that, the runner's own
|
token to disk — but it does **not expire, and registering does not spend
|
||||||
credential lives in `~/forgejo-runner/.runner`, mode 0600 — never copy it,
|
it**. A Forgejo registration token stays valid until somebody mints a
|
||||||
print it, or commit it. Secrets that CI jobs need belong in Forgejo's repo or
|
replacement at that same scope, and it will register as many runners as it is
|
||||||
org secrets, injected per job, not on this box.
|
shown to. Treat a leaked one as live until it has been replaced. (GitHub's
|
||||||
|
equivalent expires in about an hour; do not carry that habit across.) After
|
||||||
|
registration the runner's own credential lives in `~/forgejo-runner/.runner`,
|
||||||
|
mode 0600 — never copy it, print it, or commit it. Secrets that CI jobs need
|
||||||
|
belong in Forgejo's repo or org secrets, injected per job, not on this box.
|
||||||
|
|
|
||||||
12
docs/templates/ci-box/install.sh
vendored
12
docs/templates/ci-box/install.sh
vendored
|
|
@ -3,10 +3,20 @@
|
||||||
# TENANT_USER/TENANT_HOME/TENANT_GROUP/ROLE exported.
|
# TENANT_USER/TENANT_HOME/TENANT_GROUP/ROLE exported.
|
||||||
#
|
#
|
||||||
# This lands the BINARY ONLY. Registration is deliberately not here: it needs a
|
# This lands the BINARY ONLY. Registration is deliberately not here: it needs a
|
||||||
# short-lived token from the Forgejo instance, and a tenant install is
|
# registration token from the Forgejo instance, and a tenant install is
|
||||||
# creds-free by contract — box auto-runs it at mint, holding nothing. The
|
# creds-free by contract — box auto-runs it at mint, holding nothing. The
|
||||||
# operator registers afterwards, out loud:
|
# operator registers afterwards, out loud:
|
||||||
#
|
#
|
||||||
|
# Deliberately NOT described the way the GitHub sibling describes its own
|
||||||
|
# registration token, which really does expire in about an hour. That adjective
|
||||||
|
# must not cross this forge boundary — a test pins its absence from this file
|
||||||
|
# and from creds.md, so do not reintroduce it by copying from `rig runner`.
|
||||||
|
# Forgejo's ActionRunnerToken carries no expiry field at all;
|
||||||
|
# NewRunnerToken invalidates prior tokens only when a replacement is minted at
|
||||||
|
# the same scope, and Register leaves the one it was handed active. It is
|
||||||
|
# reusable until replaced, so a leak stays live. See creds.md, which is the
|
||||||
|
# copy an agent inside the box actually reads.
|
||||||
|
#
|
||||||
# box shell ci-box
|
# box shell ci-box
|
||||||
# sudo rig forgejo-runner install --instance https://forgejo.example.com
|
# sudo rig forgejo-runner install --instance https://forgejo.example.com
|
||||||
#
|
#
|
||||||
|
|
|
||||||
38
test/cli.sh
38
test/cli.sh
|
|
@ -3190,6 +3190,18 @@ check "forgejo-runner: --instance needs a value" 2 "needs a value" "$FR" -
|
||||||
check "forgejo-runner: unknown flag exits 2" 2 "unknown flag" "$FR" --instance https://f.example.com --nope
|
check "forgejo-runner: unknown flag exits 2" 2 "unknown flag" "$FR" --instance https://f.example.com --nope
|
||||||
check "forgejo-runner: empty --labels refused" 2 "must not be empty" "$FR" --instance https://f.example.com --labels ''
|
check "forgejo-runner: empty --labels refused" 2 "must not be empty" "$FR" --instance https://f.example.com --labels ''
|
||||||
check "forgejo-runner: the runner user is never root" 2 "must not be root" "$FR" --instance https://f.example.com --user root
|
check "forgejo-runner: the runner user is never root" 2 "must not be root" "$FR" --instance https://f.example.com --user root
|
||||||
|
# The pin rig RESOLVES was charset-checked; the pin it is HANDED was not, and
|
||||||
|
# that one is the one that reaches a URL path. Refused by name at parse time.
|
||||||
|
check "forgejo-runner: --version refuses a path, not a release number" 2 "release number like" \
|
||||||
|
"$FR" --instance https://f.example.com --version ../../etc/passwd
|
||||||
|
check "forgejo-runner: --version refuses a non-numeric pin" 2 "release number like" \
|
||||||
|
"$FR" --instance https://f.example.com --version latest
|
||||||
|
# Reaching the root check is the proof a good pin got THROUGH validation: this
|
||||||
|
# runs as a normal user in CI, so "must run as root" is the next gate down.
|
||||||
|
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
|
||||||
# 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.
|
||||||
|
|
@ -3384,6 +3396,32 @@ check "ci-box: no warn-and-continue checksum branch either" 1 "" \
|
||||||
check "forgejo-runner: install routes through the shared checksum policy" 0 "fetch_and_verify_sha256" \
|
check "forgejo-runner: install routes through the shared checksum policy" 0 "fetch_and_verify_sha256" \
|
||||||
grep -o "fetch_and_verify_sha256 \"\$URL\"" "$FR"
|
grep -o "fetch_and_verify_sha256 \"\$URL\"" "$FR"
|
||||||
|
|
||||||
|
# --- GitHub's token adjective must not cross the forge boundary -------------
|
||||||
|
# Measured in Forgejo's own source, not inferred: ActionRunnerToken carries NO
|
||||||
|
# expiry field; NewRunnerToken invalidates prior tokens only when a replacement
|
||||||
|
# is minted at the same scope; Register leaves the token it was handed active.
|
||||||
|
# Reusable until replaced — where GitHub's expires in about an hour.
|
||||||
|
#
|
||||||
|
# This is pinned rather than merely fixed because creds.md is spliced into the
|
||||||
|
# ci-box's own CONTEXT.md: it is the text an AGENT INSIDE THE BOX reads about
|
||||||
|
# its own credentials. "short-lived" there makes a leaked token look
|
||||||
|
# self-expiring while it is still registering runners. The wording arrived by
|
||||||
|
# being copied from the GitHub sibling, so the same copy can bring it back.
|
||||||
|
CICREDS="$ROOT/docs/templates/ci-box/creds.md"
|
||||||
|
check "ci-box: creds.md never calls the registration token short-lived" 1 "" \
|
||||||
|
grep -qi "short-lived" "$CICREDS"
|
||||||
|
# Plain absence, not a phrase match: the wording this replaced wrapped across
|
||||||
|
# two comment lines, so a phrase pin would have passed against the very text it
|
||||||
|
# was written to catch. The file explains the ban without spelling the word.
|
||||||
|
check "ci-box: the install header does not call it short-lived either" 1 "" \
|
||||||
|
grep -qi "short-lived" "$CIBOX"
|
||||||
|
# ...and says the true thing, so the pin cannot be satisfied by deleting the
|
||||||
|
# claim rather than correcting it.
|
||||||
|
check "ci-box: creds.md states the token does not expire" 0 "not expire" \
|
||||||
|
grep -o "not expire" "$CICREDS"
|
||||||
|
check "ci-box: creds.md states a leaked token stays live" 0 "leaked" \
|
||||||
|
grep -o "leaked" "$CICREDS"
|
||||||
|
|
||||||
# --- --version must converge, not be swallowed (review !110) ----------------
|
# --- --version must converge, not be swallowed (review !110) ----------------
|
||||||
# forgejo-runner does NOT self-update, and a ci-box's template preinstalls the
|
# forgejo-runner does NOT self-update, and a ci-box's template preinstalls the
|
||||||
# binary at mint — so a bare presence check would make --version dead on the
|
# binary at mint — so a bare presence check would make --version dead on the
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue