From 0302b29ca4afb543b6cdf5788a952a30679ecb95 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 5 Oct 2026 16:06:19 +0200 Subject: [PATCH] swarm-ci-runner: give the registration token only to the register step MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The runner got its registration token through an `EnvironmentFile` (`TOKEN=…`), so the daemon carried it for its whole life, and for a `host` label act_runner copies its environment into every job step: any workflow could read a token that registers runners on the forge. The host unit now writes the bare token, and the runner unit loads it with `LoadCredential=`. The upstream pre-start step, which registered from `$TOKEN`, is replaced by one register step that reads the token from `$CREDENTIALS_DIRECTORY` and runs `act_runner register` only when `.runner` is missing, with the instance URL, name, labels and config file upstream used. The precond now also removes `.runner` when its labels differ from the configured ones, which upstream's step covered. A changed token alone no longer re-registers. Host jobs can still read `.runner` itself (#4979). Closes #4974 --- docs/scheduler/ci.md | 31 ++++-- docs/swarm/credentials.md | 2 +- nix/host-modules/swarm-ci-runner.nix | 158 +++++++++++++++++---------- nix/module-eval/forge-placement.nix | 34 ++++++ 4 files changed, 156 insertions(+), 69 deletions(-) diff --git a/docs/scheduler/ci.md b/docs/scheduler/ci.md index 2c58dcb6..bc9909b6 100644 --- a/docs/scheduler/ci.md +++ b/docs/scheduler/ci.md @@ -130,7 +130,7 @@ can't hold the runner's single slot indefinitely). - **Private netns, bridge-attached**: the container runs in its own network namespace (`privateNetwork = true`, `hostBridge`), takes its address from the gateway's DHCP, and resolves through the gateway's dnsmasq on the bridge IP. It can't reach host-loopback services — the core dashboard at `127.0.0.1:7000` and the raw forge port are unreachable from CI. - **Non-ephemeral**: runner credentials (`.runner`) persist across restarts, written to the container's state dir on registration and reused after. - **Sandbox fallback**: nspawn containers can't create user-namespaces, so nix's sandboxing would always fail. Module sets `nix.settings.sandbox-fallback = true` in the container — nix builds run unsandboxed (safe because the container is already isolated). See `docs/process/gotchas.md`. -- **Credential isolation**: no forge admin credential enters the container. swarm-controller holds the forge admin token; the runner holds only the registration token and its own `.runner`, and the host unit that fetches the token holds a store identity that reads that one path. +- **Credential isolation**: no forge admin credential enters the container. swarm-controller holds the forge admin token; the runner holds only the registration token and its own `.runner`, and the host unit that fetches the token holds a store identity that reads that one path. Only the register step reads the registration token; the daemon and the jobs it runs never have it in their environment. ## Registration @@ -143,13 +143,16 @@ can't hold the runner's single slot indefinitely). 2. **`swarm-bao-ci-runner.service`** on the runner's host logs in to the store with the `ci-runner` leaf, whose `swarm-ci-runner` role reads that one path and nothing else, retrying every 30 seconds until the token is there. It - writes `TOKEN=` to the runner's `EnvironmentFile` inside the - container, and restarts the runner when the value changes. It runs at boot - and on deploy, not on a timer. -3. **The runner unit** (`gitea-runner-swarm` in the container) registers in - the upstream pre-start step whenever `.runner` is missing or the token or - labels changed. A pre-start step of this module's, ahead of it, removes - `.runner` in two cases upstream doesn't check: + writes the token to a root-only file inside the container, and restarts + the runner when the value changes. It runs at boot and on deploy, not on a + timer. +3. **The runner unit** (`gitea-runner-swarm` in the container) loads that + file with `LoadCredential=`, not as an environment variable: for a `host` + label, act_runner hands its whole environment to every job step. A + register step before the daemon reads the token from + `$CREDENTIALS_DIRECTORY` and runs `act_runner register` only when `.runner` + is missing; with a `.runner` in place the daemon starts without the token. + A pre-start step ahead of it removes `.runner` in three cases: - The forge has no record of the runner. Forgejo answers every call from an unknown runner with `unregistered runner`, and the daemon exits 1 when its startup `Declare` gets that answer. On the next start the step finds the @@ -158,9 +161,15 @@ can't hold the runner's single slot indefinitely). and logging the error without exiting; the next restart of the unit or container registers it again. - `.runner` names another forge address than the configured one. + - `.runner` names other labels than the configured ones. - Until a token arrives the step fails, and `Restart=on-failure` retries every - 15 seconds with no start limit. + A changed token alone doesn't register again: the daemon authenticates with + the per-runner token in `.runner`. Without `.runner` and with no token yet + the register step fails, and `Restart=on-failure` retries every 15 seconds + with no start limit. + + Host jobs run as the runner's own user, so they can still read `.runner` + and act as this runner; see the security section below. A new registration is a new runner on the forge: the previous entry stays in the forge admin panel as offline until an admin deletes it. @@ -222,7 +231,7 @@ A malicious `default.nix` or build script in a PR can therefore: - **Make arbitrary network requests** to any address reachable from the container. The container runs in its own netns behind the hive bridge, so it reaches the forge by name at `https://` (no admin credentials) and **can't** reach host-loopback services: the unauthenticated core dashboard at `127.0.0.1:7000` and the raw forge port are off-limits (a firewall rule drops bridge→127.0.0.0/8 traffic). - **Write to the container filesystem**, including corrupting the runner's state dir or `.runner` credentials. -No forge admin token enters the container: swarm-controller holds it and publishes only the registration token, which reaches the container's env-file. A build process can still reach the forge over the network, and it runs as the runner's own user, so it can read `.runner` and act as this runner; it can't issue admin API calls. +No forge admin token enters the container: swarm-controller holds it and publishes only the registration token, which only the runner's register step reads, so a job step doesn't inherit it. A build process can still reach the forge over the network, and it runs as the runner's own user, so it can read `.runner` and act as this runner; it can't issue admin API calls. Note: `nix flake check --no-build` (eval-only) reduces the attack surface but doesn't eliminate it — `builtins.fetchGit`, `builtins.fetchurl`, and import-from-derivation can reach the network and filesystem during evaluation. The default CI workflow runs full `nix flake check` (builds derivations), which is the higher-risk path. diff --git a/docs/swarm/credentials.md b/docs/swarm/credentials.md index d2aadec2..95820eff 100644 --- a/docs/swarm/credentials.md +++ b/docs/swarm/credentials.md @@ -69,7 +69,7 @@ of the cell says how. | `swarm/hives//matrix/sender-token` | `swarm-controller`, with the swarm's appservice token, for every hive in its directory in a five-minute pass | `swarm-controller` under its own certificate, before it decides whether to mint, and hive-c0re's `stored_sender_token()`, under the hive's own certificate | ✅ the controller's pass re-mints when the stored token is missing, unknown to the homeserver, or someone else's | ✅ hive-c0re's matrix sweep reads the store every run and overwrites its token file when the store's token differs | | `swarm/hives//queue/agent` | authelia | `swarm-bao-queue-agent` on the hive's host, under its own per-hive certificate; no agent's policy reaches it | must be stated | must be stated | | `swarm/services//oidc/client` | authelia | that service's own reader, under its own certificate — the queue responder's under `swarm-nats-auth`'s, the homeserver's under `swarm-matrix-ctl`'s; the bao UI's by the store's granter | must be stated | must be stated | -| `swarm/services/ci-runner/registration-token` | `swarm-controller`, from the forge's admin API, at start and in a five-minute pass; its grant on the path is write-only | `swarm-bao-ci-runner` on the runner's host, under the runner's own certificate, into the runner's `EnvironmentFile`; no other principal's policy reads it | ❌ the forge hands out one token until an admin resets it; the controller writes a reset token within five minutes | ❌ `swarm-bao-ci-runner` reads it when it starts, at boot or deploy, and restarts the runner when the value changed | +| `swarm/services/ci-runner/registration-token` | `swarm-controller`, from the forge's admin API, at start and in a five-minute pass; its grant on the path is write-only | `swarm-bao-ci-runner` on the runner's host, under the runner's own certificate, into a file only the runner's register step loads; no other principal's policy reads it | ❌ the forge hands out one token until an admin resets it; the controller writes a reset token within five minutes | ❌ `swarm-bao-ci-runner` reads it when it starts, at boot or deploy, and restarts the runner when the value changed | | _(not in the store)_ a hive's mTLS leaf | the store's own PKI, or an operator placing it by hand | its own client, off disk — the exception above, because it's what makes every other row's pull possible | must be stated | must be stated | diff --git a/nix/host-modules/swarm-ci-runner.nix b/nix/host-modules/swarm-ci-runner.nix index 9739b80e..279adb45 100644 --- a/nix/host-modules/swarm-ci-runner.nix +++ b/nix/host-modules/swarm-ci-runner.nix @@ -57,11 +57,13 @@ let # it, which is the spelling a step run as root (`+`) can follow. runnerFile = "/var/lib/gitea-runner/${instance}/.runner"; - # Inside the container, root-only. The token file is the runner's - # `EnvironmentFile` (`TOKEN=`); the host unit below writes it through - # the container's root, at `hostTokenFile`. + # Inside the container, root-only. The token file holds the bare token and + # reaches the runner unit only as a `LoadCredential=`, which only the + # register step reads; the host unit below writes it through the + # container's root, at `hostTokenFile`. stateDir = "/var/lib/swarm-ci-runner"; - tokenFile = "${stateDir}/registration-token.env"; + tokenFile = "${stateDir}/registration-token"; + tokenCredential = "registration-token"; hostTokenFile = "/var/lib/nixos-containers/${machine}${tokenFile}"; # The epoch second the daemon last started. The precond reads the daemon's # journal from there. @@ -246,15 +248,15 @@ in exit 1 fi - # Root-owned: systemd reads an `EnvironmentFile` as root, before the - # runner drops to its dynamic user. + # Root-owned: systemd reads a `LoadCredential=` source as root, before + # the runner drops to its dynamic user. install -d -m 0700 ${lib.escapeShellArg (builtins.dirOf hostTokenFile)} - if secret_differs ${lib.escapeShellArg hostTokenFile} "TOKEN=$token"; then - atomic_write_secret 0600 "" ${lib.escapeShellArg hostTokenFile} "TOKEN=$token" + if secret_differs ${lib.escapeShellArg hostTokenFile} "$token"; then + atomic_write_secret 0600 "" ${lib.escapeShellArg hostTokenFile} "$token" fi - # `EnvironmentFile=` is read once, at start. A changed token makes the - # upstream pre-start step register again. + # `LoadCredential=` copies the file at start, so a runner still waiting + # to register sees a new token only once it restarts. refresh_consumer ${lib.escapeShellArg machine} ${runnerUnit}.service ${lib.escapeShellArg hostTokenFile} ''; }; @@ -282,7 +284,20 @@ in // caTrust.bindMount; config = - { pkgs, lib, ... }: + { + pkgs, + lib, + config, + ... + }: + let + runnerCfg = config.services.gitea-actions-runner; + instanceCfg = runnerCfg.instances.${instance}; + # The same file upstream generates and hands the daemon. + runnerConfigFile = (pkgs.formats.yaml { }).generate "config.yaml" instanceCfg.settings; + # In the form the precond reads back from `.runner`. + registeredLabels = lib.concatStringsSep "," (lib.sort lib.lessThan instanceCfg.labels); + in { imports = [ ../container-modules/swarm-container.nix @@ -321,8 +336,9 @@ in systemd.tmpfiles.rules = [ "d ${stateDir} 0700 root root -" - # `EnvironmentFile=` fails the unit when the file is missing; an - # empty one leaves `TOKEN` unset for the precond to report. + # `LoadCredential=` fails the unit when the file is missing; an + # empty one lets a registered runner start before any token has + # arrived, and the register step report the missing token. "f ${tokenFile} 0600 root root -" ]; @@ -332,6 +348,8 @@ in enable = true; inherit (cfg) name labels; url = forgeUrl; + # Upstream refuses an instance without a token source, and turns + # this one into the `EnvironmentFile=` the unit below drops. inherit tokenFile; settings = { runner.capacity = cfg.concurrency; @@ -360,54 +378,80 @@ in environment = lib.mkIf caTrust.useSelfSigned { NODE_EXTRA_CA_CERTS = caTrust.caContainerPath; }; - # Runs before upstream's pre-start step, which registers whenever - # `.runner` is absent or the token or labels changed. This one - # removes `.runner` in the two cases upstream cannot see, and holds - # the unit down until there is a token. - # - # `+`: root, because `.runner`, the token and the system journal - # are all out of the dynamic user's reach. - serviceConfig.ExecStartPre = lib.mkMerge [ - (lib.mkBefore [ - "+${pkgs.writeShellScript "swarm-ci-runner-precond" '' - set -eu - runner=${runnerFile} - started=${daemonStartedFile} + # The registration token reaches the unit only as a credential, + # which the register step reads. A host job inherits the daemon's + # whole environment, so neither carries it. + serviceConfig.EnvironmentFile = lib.mkForce [ ]; + serviceConfig.LoadCredential = [ "${tokenCredential}:${tokenFile}" ]; + # One registration path: upstream's pre-start step registers from + # `$TOKEN`, so this list replaces it. + serviceConfig.ExecStartPre = lib.mkForce [ + # Removes `.runner` in the cases where reusing it would fail, so + # the register step after it registers again. + # + # `+`: root, because the system journal is out of the dynamic + # user's reach. + "+${pkgs.writeShellScript "swarm-ci-runner-precond" '' + set -eu + runner=${runnerFile} + started=${daemonStartedFile} - # The forge refused the registration the daemon last started - # with. Read on the next start rather than when the daemon - # exits, so journald has its last lines by then. - if [ -f "$started" ]; then - since="$(< "$started")" - ${pkgs.coreutils}/bin/rm -f "$started" - if ${pkgs.systemd}/bin/journalctl -u ${runnerUnit}.service --since "@$since" -o cat --no-pager \ - | ${pkgs.gnugrep}/bin/grep -qF ${lib.escapeShellArg rejectedMessage}; then - echo "the forge has no record of this runner's registration; registering again" >&2 - ${pkgs.coreutils}/bin/rm -f "$runner" - fi - fi - - # `.runner` records the address it registered at, and the - # daemon dials that one, not the configured one. - if [ -f "$runner" ] \ - && [ "$(${pkgs.jq}/bin/jq -r .address "$runner")" != ${lib.escapeShellArg forgeUrl} ]; then - echo "$runner names another forge address; registering again at ${forgeUrl}" >&2 + # The forge refused the registration the daemon last started + # with. Read on the next start rather than when the daemon + # exits, so journald has its last lines by then. + if [ -f "$started" ]; then + since="$(< "$started")" + ${pkgs.coreutils}/bin/rm -f "$started" + if ${pkgs.systemd}/bin/journalctl -u ${runnerUnit}.service --since "@$since" -o cat --no-pager \ + | ${pkgs.gnugrep}/bin/grep -qF ${lib.escapeShellArg rejectedMessage}; then + echo "the forge has no record of this runner's registration; registering again" >&2 ${pkgs.coreutils}/bin/rm -f "$runner" fi + fi - # Upstream registers on a changed token hash, an empty token - # included, and a registration with no token fails. - if [ -z "''${TOKEN:-}" ]; then - echo "no registration token yet; swarm-bao-ci-runner on the host delivers it" >&2 - exit 1 - fi - ''}" - ]) - (lib.mkAfter [ - "+${pkgs.writeShellScript "swarm-ci-runner-mark-start" '' - ${pkgs.coreutils}/bin/date +%s > ${daemonStartedFile} - ''}" - ]) + # `.runner` records the address it registered at, and the + # daemon dials that one, not the configured one. + if [ -f "$runner" ] \ + && [ "$(${pkgs.jq}/bin/jq -r .address "$runner")" != ${lib.escapeShellArg forgeUrl} ]; then + echo "$runner names another forge address; registering again at ${forgeUrl}" >&2 + ${pkgs.coreutils}/bin/rm -f "$runner" + fi + + # The daemon declares the labels `.runner` records, not the + # configured ones. + if [ -f "$runner" ] \ + && [ "$(${pkgs.jq}/bin/jq -r '.labels | sort | join(",")' "$runner")" != ${lib.escapeShellArg registeredLabels} ]; then + echo "$runner names other labels; registering again with ${registeredLabels}" >&2 + ${pkgs.coreutils}/bin/rm -f "$runner" + fi + ''}" + # Registers only when `.runner` is absent. As the dynamic user, + # so `.runner` is the daemon's own. The derivation itself rather + # than its path, so ../module-eval reads its text without building. + (pkgs.writeShellScript "swarm-ci-runner-register" '' + set -eu + dir="$STATE_DIRECTORY/${instance}" + ${pkgs.coreutils}/bin/mkdir -p "$dir" + cd "$dir" + if [ -e .runner ]; then + exit 0 + fi + + token="$(< "$CREDENTIALS_DIRECTORY/${tokenCredential}")" + if [ -z "$token" ]; then + echo "no registration token yet; swarm-bao-ci-runner on the host delivers it" >&2 + exit 1 + fi + exec ${lib.getExe runnerCfg.package} register --no-interactive \ + --instance ${lib.escapeShellArg instanceCfg.url} \ + --token "$token" \ + --name ${lib.escapeShellArg instanceCfg.name} \ + --labels ${lib.escapeShellArg (lib.concatStringsSep "," instanceCfg.labels)} \ + --config ${runnerConfigFile} + '') + "+${pkgs.writeShellScript "swarm-ci-runner-mark-start" '' + ${pkgs.coreutils}/bin/date +%s > ${daemonStartedFile} + ''}" ]; # Restarted until the token arrives, however long the store and # the controller take. `mkForce`: upstream sets both. diff --git a/nix/module-eval/forge-placement.nix b/nix/module-eval/forge-placement.nix index d82a7da3..e610bc71 100644 --- a/nix/module-eval/forge-placement.nix +++ b/nix/module-eval/forge-placement.nix @@ -81,6 +81,9 @@ let forgeSettings = forgeHere.containers.hive-forge.config.services.forgejo.settings; + runnerService = ciAlone.containers.swarm-ci-runner.config.systemd.services.gitea-runner-swarm; + registerStep = lib.findFirst lib.isDerivation null runnerService.serviceConfig.ExecStartPre; + cases = [ { # The absence the whole option exists for: a second forge in a swarm @@ -150,6 +153,37 @@ let name = "the CI runner is refused without its store identity"; ok = refusedOver ciNoIdentity "deploy.bao.ciRunnerClientCertFile"; } + { + # A host job inherits the daemon's whole environment, and a token there + # lets any workflow register runners against the forge. + name = "the CI runner daemon carries no registration token in its environment"; + ok = + let + sc = runnerService.serviceConfig; + in + !(runnerService.environment ? TOKEN) + && lib.toList (sc.EnvironmentFile or [ ]) == [ ] + && !(lib.any (lib.hasPrefix "TOKEN=") (lib.toList (sc.Environment or [ ]))) + && sc.LoadCredential == [ "registration-token:/var/lib/swarm-ci-runner/registration-token" ]; + } + { + # The only reader of the credential, and the only registration path. + name = "the CI runner registers from its credential, and only without a .runner"; + ok = + let + text = registerStep.text; + beforeRegister = lib.head (lib.splitString "register --no-interactive" text); + in + registerStep != null + && !(lib.any ( + s: lib.hasInfix "gitea-register-runner" (toString s) + ) runnerService.serviceConfig.ExecStartPre) + && lib.hasInfix "if [ -e .runner ]; then\n exit 0\nfi" beforeRegister + && lib.hasInfix ''token="$(< "$CREDENTIALS_DIRECTORY/registration-token")"'' beforeRegister + && lib.hasInfix ''--token "$token"'' text + && !(lib.hasInfix "export" text) + && !(lib.hasInfix "TOKEN" text); + } { # A removed option evaluates to an assertion naming the replacement. name = "a CI option under deploy.forgejo names its replacement under deploy.ci-runner";