[isaacslee]

Why

The independent review after the ARM64 acceptance (#18) found gaps in #15–#17. One is medium: support for systemd below 255 was claimed without verification. The rest are low or informational:

• a fail-open process check;
• retirement that a failure could leave unfinishable;
• parser exceptions escaping;
• O_NOFOLLOW described more strongly than it worked;
• the token's lifetime in the operator's shell;
• an inaccurate unpack comment.

Closes #18.

Stacked on #17 (fix/vm-findings). Do not merge before #17, #16 and #15; merge order stays top-down.

What changed

A. The process check fails closed (f7670f1)

• pgrep -u is gone. A missing or failing pgrep printed nothing, read as "no process", and let retirement proceed.
• host_account_processes reads /proc/<pid>/status and matches the account's uid against the real, effective, saved and filesystem uids.
• Each result is distinct: an unreadable process table, a missing Uid: line, or no status file at all is "cannot tell" (2), never "none" (1). Retirement refuses on 0 or 2.

B. Partial retirement and recovery (f7670f1)

• Removal order: the account first, then the runtime root, then the unit. While the account exists, the unit and the marked root still identify the runner, so a retry continues from where it stopped.

• The root marker is removed last. A drill found the old rm -rf removed the marker first, so a failing removal left a root that no retry would ever prove again.

• userdel's outcome is read back, not inferred from its exit code. For example, exit 12 means the account is gone but the home is not.

  • Account still there: stop. Nothing else is removed.
  • Home left behind: reported, never removed.

• A group is removed only when it is provably the account's own:

  • its gid is recorded before userdel;
  • it has no members;
  • no other account uses it as its primary group.

  This covers USERGROUPS_ENAB no. A group found on a later run, without its account, is reported and never removed, because the proof is gone.

• Recovery: every residual message has a row in the recovery table in docs/operations.md, and there is a procedure for a runtime root without its marker.

C. systemd range (887d873)

• Supported: 255 only (verified).
• Accepted: 249–254 and 256 or later. Preflight still accepts them, and warns that the version is accepted but unverified.
• Why accepted, not refused: the RestrictNamespaces printing that check depends on was read in the v249, v252 and v255 sources (src/shared/bus-print-properties.c) and is identical. That is a source review, not verification, and the documentation says so. Narrowing to 255 would refuse hosts nothing shows are broken.
• Documents updated: compatibility, installation, README and contract now separate "accepted" from "supported".

D. Other items

• JSON reads (bbea655):
  • safe_load opens the runner directory, which sits in a root-owned runtime root, then walks each component with O_NOFOLLOW, so a symlinked bin/ is refused. Before, only the last component was protected.
  • Any exception, including RecursionError, exits 2.
  • A manifest whose libraries is not an object is refused. The old code returned a version from a list.
  • The threat model now describes this accurately.
• Token (97f2218): registration and deregistration print as one ( ... ) block. The token lives only in that subshell, and a failing gh ends it before config.sh runs.
• Unpack (930798a): tar --no-overwrite-dir keeps the runner directory 0700 for the whole unpack. The comment is corrected.
• Test gaps (7f84c01, and inside the commits above):
  • the fresh unpack mode;
  • markerless, wrong-profile and symlinked markers on retire;
  • a person's account at apply level;
  • an inherited TMPDIR;
  • .runner and manifest that are symlinks, FIFOs, oversized, deeply nested, the wrong shape, or carry control characters;
  • the --replace assertion, now checked against the whole printed block.

How it was verified

• shellcheck 0.10.0: 20 scripts clean, on the final tree e68c8a1 (tree 201295e).

• Drills, final tree: 514 cases, 0 failed.

  apply	check	cli	docs	fabric	render	host	release	status
  187	45	21	7	84	40	105	10	15

• Each commit on its own: every intermediate commit passes shellcheck and the whole suite. The release drill skipped in those runs, because they ran outside a git checkout.

• Failing before: the final tests against the code of 01ca427 give 41 failures (apply 22, host 19); after the fix, 0.

• Mutation check: 30 mutations of the new guards, each run against its drills. 29 were caught. One survived and is equivalent: removing the st_size fast path changes nothing, because the read loop enforces the same limit, and removing the limit itself is caught.

• ARM64 VM (rs-arm64, Ubuntu 24.04, systemd 255), commit 7f84c01:

  • All 49 archive files match the commit, and root owns them.
  • A test-only profile probe was used and never registered.
  • Results are in docs/acceptance/2026-10-arm64.md (follow-up section). Summary:
    • real and saved-uid leftover processes refused; pgrep -u found the saved-uid one: nothing;
    • userdel failing under a held /etc/passwd lock removed nothing else;
    • an immutable file in the root: the unit and marker were kept, and the retry finished;
    • USERGROUPS_ENAB no: the account's own group was removed;
    • a group with a member was kept; a foreign-owned home was reported and kept;
    • a symlinked bin/ was refused;
    • the unpack mode went 750, then 700, then 750; the archive's 0775 never appeared;
    • the drills in the VM as nobody: 504 cases, 0 failed.
  • Afterwards, /etc/passwd, /etc/group and /etc/login.defs matched their checksums from before, byte for byte.

• Hosted CI: BLOCKED. The organisation has used 2000/2000 Linux minutes this month, with a $0 budget and prevent_further_usage. No merge exception is requested.

Evidence level

• RUNTIME VERIFIED on one ARM64 VM: A, B, the unpack mode and the symlinked bin/.
• SYNTHETICALLY TESTED only:
  • an unreadable /proc;
  • FIFO, oversized, deeply nested and wrong-shape runner files;
  • the token block (run with a stand-in gh and sudo);
  • the systemd warning, since the VM is on 255.
• Source review only, not verification: systemd 249–254.

Limitations

• No real registration was used, so the token block was not run against GitHub in this PR. Its predecessor was, in fix: correct what the real arm64 host contradicted #17.
• x86_64 is not verified.

Risks

• Retirement order changed. A host whose userdel succeeds but whose root cannot be removed now keeps the unit until a retry. That is intended and documented.
• A group that existed before the account and was adopted by hand is removed only if it passes all three proofs.

Compatibility impact

• Preflight now prints a warning on systemd other than 255. The exit code is unchanged.
• The printed registration and deregistration commands have a new shape.
• Retirement exits 1 when something is left behind, where before it could exit 0.

Remaining work

• Hosted CI once the free minutes reset (1 November 00:00 UTC).
• runner-slice status with a real gh; a real misregistration; unknown job state on a real host; self-update; x86_64.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

[isaacslee]

Independent review, and what changed after it

The review of e68c8a1 found one High, which it reproduced. Removing the runtime root's children one at a time let rm --one-file-system empty a filesystem mounted at work/ or runner/. This was a regression in this PR.

e2e58f8 fixes it, together with the Medium and Low findings:

• Mounts: retirement refuses, removing nothing, while anything is mounted in the runtime root or the mount table cannot be read. rm also gets --preserve-root=all.
• Marker: it is written back if the final rmdir fails.
• Accounts: a failed getent lookup is no longer read as "absent". A group is kept when accounts also come from a directory service.
• Unit: a unit that cannot be removed is reported, and the run is not called retired.

The re-review of e2e58f8 found no Critical or High. 90f7ec8 closes its three Lows:

• nsswitch parsing;
• compat with NIS +/- lines;
• the canonical path for the mount check.

Evidence for 90f7ec8 (tree 84a0cec):

• Hosted gate: run 38923067922, success, on a GitHub-hosted ubuntu-24.04 runner. shellcheck: 20 scripts clean. 545 drill cases, 0 failed.
• Failing first: the new tests fail on the previous code. The mount case removed the data on the old code.
• Mutations: all 11 new ones are caught.
• Real ARM64 VM:
  • all 49 archive files match the commit;
  • a tmpfs at work/: retire refused, removed nothing, and the data was kept;
  • after unmounting, retire exited 0 and left nothing;
  • /etc/passwd, /etc/group and /etc/login.defs were unchanged.