From 524401e1d8cb9ea7cfd83a55137213f6395dadb0 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Fri, 11 Sep 2026 17:46:14 -0400 Subject: [PATCH 1/2] feat(runner): add nself runner provision/verify for CI runner hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI runner hosts were hand-built, so required system dependencies were discovered only when a job failed mid-run, and two hosts advertising the same GitHub Actions labels (self-hosted,Linux,X64) could silently drift apart depending on which one had gh/zip/unzip installed. A Playwright --with-deps sudo failure also produced a Chromium binary missing shared libs, which then surfaced hours later as a misleading generic error. Adds internal/runner: a declarative dependency manifest (manifest.yaml, embedded) covering git/jq/yq/gh/make/zip/unzip/curl/docker/build tooling plus the Playwright/Chromium runtime libs, an Executor seam (local or SSH, reusing internal/deploy's SSH primitives) so every check/step is a plain command string asserted in tests without touching a real host, a verify path that runs the manifest against N hosts and renders a cross-host parity matrix plus explicit drift findings, a dedicated ldd check against any cached Chromium binary, and a provision path that installs packages, creates the runner user with passwordless sudo, and registers runner instances as systemd services — refusing outright if the shared _work directory is a symlink, since git resolves symlinks before matching includeIf.gitdir and that silently breaks actions/checkout's credential injection. Wires `nself runner` (provision, verify) into cmd/commands, adds it to the command group table and error harness, and regenerates the CLI command inventory (SPORT F02, .github/wiki/Commands.md, cmd-runner.md). --- .github/command-inventory.json | 35 ++++++ .github/wiki/Commands.md | 3 +- .github/wiki/_Sidebar.md | 4 +- .github/wiki/cmd-runner.md | 84 +++++++++++++ .github/wiki/llms.txt | 17 ++- cmd/commands/error_harness_test.go | 6 + cmd/commands/groups.go | 1 + cmd/commands/runner.go | 70 +++++++++++ cmd/commands/runner_provision.go | 102 +++++++++++++++ cmd/commands/runner_test.go | 123 ++++++++++++++++++ cmd/commands/runner_verify.go | 100 +++++++++++++++ internal/runner/chromium.go | 93 ++++++++++++++ internal/runner/chromium_test.go | 71 +++++++++++ internal/runner/exec.go | 92 ++++++++++++++ internal/runner/exec_test.go | 79 ++++++++++++ internal/runner/fake_test.go | 49 ++++++++ internal/runner/manifest.go | 99 +++++++++++++++ internal/runner/manifest.yaml | 175 ++++++++++++++++++++++++++ internal/runner/manifest_test.go | 66 ++++++++++ internal/runner/provision.go | 196 +++++++++++++++++++++++++++++ internal/runner/provision_test.go | 123 ++++++++++++++++++ internal/runner/report.go | 193 ++++++++++++++++++++++++++++ internal/runner/report_test.go | 116 +++++++++++++++++ internal/runner/verify.go | 121 ++++++++++++++++++ internal/runner/verify_test.go | 119 ++++++++++++++++++ 25 files changed, 2133 insertions(+), 4 deletions(-) create mode 100644 .github/wiki/cmd-runner.md create mode 100644 cmd/commands/runner.go create mode 100644 cmd/commands/runner_provision.go create mode 100644 cmd/commands/runner_test.go create mode 100644 cmd/commands/runner_verify.go create mode 100644 internal/runner/chromium.go create mode 100644 internal/runner/chromium_test.go create mode 100644 internal/runner/exec.go create mode 100644 internal/runner/exec_test.go create mode 100644 internal/runner/fake_test.go create mode 100644 internal/runner/manifest.go create mode 100644 internal/runner/manifest.yaml create mode 100644 internal/runner/manifest_test.go create mode 100644 internal/runner/provision.go create mode 100644 internal/runner/provision_test.go create mode 100644 internal/runner/report.go create mode 100644 internal/runner/report_test.go create mode 100644 internal/runner/verify.go create mode 100644 internal/runner/verify_test.go diff --git a/.github/command-inventory.json b/.github/command-inventory.json index 949b2139..38037dbb 100644 --- a/.github/command-inventory.json +++ b/.github/command-inventory.json @@ -1899,6 +1899,41 @@ "--verbose" ] }, + { + "name": "runner", + "path": "nself runner", + "short": "Provision and audit self-hosted GitHub Actions CI runner hosts", + "hidden": false, + "group_id": "advanced", + "subcommands": [ + { + "name": "provision", + "path": "nself runner provision", + "short": "Install runner dependencies, user, sudoers, and N runner instances", + "hidden": false, + "flags": [ + "--github-url", + "--host", + "--install-root", + "--instances", + "--labels", + "--ssh-key", + "--token" + ] + }, + { + "name": "verify", + "path": "nself runner verify", + "short": "Check host(s) against the dependency manifest and report drift", + "hidden": false, + "flags": [ + "--host", + "--json", + "--ssh-key" + ] + } + ] + }, { "name": "secrets", "path": "nself secrets", diff --git a/.github/wiki/Commands.md b/.github/wiki/Commands.md index df4e6d48..56aa4869 100644 --- a/.github/wiki/Commands.md +++ b/.github/wiki/Commands.md @@ -94,7 +94,7 @@ tree in `cmd/commands/`. Run `make cmd-inventory` to refresh. ## Complete index Generated from the cobra registration tree in `cmd/commands/`. -Run `make cmd-inventory` to refresh. **Total top-level commands: 50** +Run `make cmd-inventory` to refresh. **Total top-level commands: 51** | Command | Short Description | Group | Subcommands | |---|---|---|---| @@ -134,6 +134,7 @@ Run `make cmd-inventory` to refresh. **Total top-level commands: 50** | `nself remove` | Remove an installed plugin or bundle | extend | — | | `nself reset` | Stop containers, remove all data volumes, and clean generated files | core | — | | `nself restart` | Smart restart with config change detection | core | — | +| `nself runner` | Provision and audit self-hosted GitHub Actions CI runner hosts | advanced | provision, verify | | `nself secrets` | Manage encrypted project secrets (age encryption) | config | audit, decrypt-on-deploy, edit, get, init, lint, list, list-schedules, rekey, retire, rotate, rotation-log, schedule, set, verify | | `nself security` | Server security: audit, setup, and status | advanced | audit, setup, status | | `nself self-heal` | Run targeted self-healing routines for nSelf components | observe | — | diff --git a/.github/wiki/_Sidebar.md b/.github/wiki/_Sidebar.md index 9d307491..166c6adb 100644 --- a/.github/wiki/_Sidebar.md +++ b/.github/wiki/_Sidebar.md @@ -279,7 +279,7 @@ -**All commands (50)** +**All commands (51)** - _A:_ [[cmd-access]] · [[cmd-account]] · [[cmd-admin]] - _B:_ [[cmd-backup]] · [[cmd-build]] · [[cmd-bundle]] @@ -294,7 +294,7 @@ - _M:_ [[cmd-man]] · [[cmd-mcp]] · [[cmd-migrate]] - _O:_ [[cmd-oauth]] · [[cmd-ops]] - _P:_ [[cmd-plugin]] · [[cmd-promote]] -- _R:_ [[cmd-remove]] · [[cmd-reset]] · [[cmd-restart]] +- _R:_ [[cmd-remove]] · [[cmd-reset]] · [[cmd-restart]] · [[cmd-runner]] - _S:_ [[cmd-secrets]] · [[cmd-security]] · [[cmd-self-heal]] · [[cmd-service]] · [[cmd-start]] · [[cmd-status]] · [[cmd-stop]] - _T:_ [[cmd-telemetry]] · [[cmd-template]] · [[cmd-trust]] - _U:_ [[cmd-update]] · [[cmd-urls]] diff --git a/.github/wiki/cmd-runner.md b/.github/wiki/cmd-runner.md new file mode 100644 index 00000000..0568ddd2 --- /dev/null +++ b/.github/wiki/cmd-runner.md @@ -0,0 +1,84 @@ +# nself runner + + +> Provision and audit self-hosted GitHub Actions CI runner hosts. + + +## Synopsis + +``` +nself runner [flags] +``` + +## Description + + +Provision and audit self-hosted GitHub Actions CI runner hosts. + +Runner hosts were previously hand-built: required system dependencies (gh, +zip, unzip, Playwright/Chromium's shared libraries, ...) were discovered +only when a job failed mid-run, and two hosts advertising the identical +GitHub Actions labels (self-hosted,Linux,X64) could silently drift apart — +the same commit would pass or fail depending on which host claimed the job. + +The dependency set is declarative (internal/runner/manifest.yaml, compiled +into this binary) so provision and verify always check the same list. + +Subcommands: + provision Install dependencies, create the runner user, register N + runner instances as systemd services + verify Check one or more hosts against the manifest and print a + parity matrix — the important half: this is how "same + labels, different tools" gets caught before it causes a + mystery failure. + + +## Flags + + +| Flag | Default | Description | +|------|---------|-------------| +| `--help`, `-h` | — | Show help | + + +## Subcommands + + +| Name | Description | +|------|-------------| +| `provision` | Install runner dependencies, user, sudoers, and N runner instances | +| `verify` | Check host(s) against the dependency manifest and report drift | + + +## Examples + + +```bash +# Check this machine against the manifest +nself runner verify + +# Check two remote runner hosts in one pass and print a parity matrix — +# this is how "same GitHub Actions labels, different tools installed" +# gets caught before it causes a mystery job failure +nself runner verify --host ci@runner-a.example.com --host ci@runner-b.example.com + +# Machine-readable output for a CI gate +nself runner verify --host ci@runner-a.example.com --json + +# Provision a fresh host with 2 runner instances +GITHUB_RUNNER_TOKEN=... nself runner provision \ + --host ci@runner-a.example.com \ + --github-url https://github.com/nself-org/cli \ + --instances 2 \ + --labels nself-ci +``` + + +## See Also + + +- [[Commands]] — full command index +- [[Core-Services]] — what a stack is made of + + +← [[Commands]] | [[Home]] → diff --git a/.github/wiki/llms.txt b/.github/wiki/llms.txt index da988aa3..dd433318 100644 --- a/.github/wiki/llms.txt +++ b/.github/wiki/llms.txt @@ -14,7 +14,7 @@ nself build # generate docker-compose + nginx nself start # boot the stack ``` -## Commands (50) +## Commands (51) ### nself access @@ -793,6 +793,21 @@ Flags: Full page: [[cmd-restart]] +### nself runner + +Provision and audit self-hosted GitHub Actions CI runner hosts + +``` +nself runner [flags] +``` + +Subcommands: + +- `provision` — Install runner dependencies, user, sudoers, and N runner instances +- `verify` — Check host(s) against the dependency manifest and report drift + +Full page: [[cmd-runner]] + ### nself secrets Manage encrypted project secrets (age encryption) diff --git a/cmd/commands/error_harness_test.go b/cmd/commands/error_harness_test.go index 5f9a7c3f..d48dd502 100644 --- a/cmd/commands/error_harness_test.go +++ b/cmd/commands/error_harness_test.go @@ -354,6 +354,12 @@ var errorHarnessCases = []errorHarnessCase{ {"verify-sbom", []string{"verify-sbom"}, "(a) no project dir"}, {"verify-sbom", []string{"verify-sbom", "--no-such-flag-xyz"}, "(b) invalid flag"}, {"verify-sbom", []string{"verify-sbom", "unknownsub_xyz"}, "(c) unknown sub"}, + + // ── runner (G-012) ───────────────────────────────────────────────────── + // runner root returns cmd.Help() (nil) — soft case (no project required). + {"runner", []string{"runner"}, "(a) shows help (no project required)"}, + {"runner", []string{"runner", "--no-such-flag-xyz"}, "(b) invalid flag"}, + {"runner", []string{"runner", "unknownsub_xyz"}, "(c) unknown sub"}, } // runErrorHarnessCmd executes the given args against a fresh RootCmd clone diff --git a/cmd/commands/groups.go b/cmd/commands/groups.go index 3fdc9075..7e1dc863 100644 --- a/cmd/commands/groups.go +++ b/cmd/commands/groups.go @@ -109,6 +109,7 @@ var commandGroupAssignments = map[string]string{ "access": groupAdvanced, "security": groupAdvanced, "verify-sbom": groupAdvanced, + "runner": groupAdvanced, } // ApplyCommandGroups registers the groups and assigns each command to one. diff --git a/cmd/commands/runner.go b/cmd/commands/runner.go new file mode 100644 index 00000000..ad3da2a9 --- /dev/null +++ b/cmd/commands/runner.go @@ -0,0 +1,70 @@ +package commands + +// Purpose: `nself runner` — build and audit self-hosted GitHub Actions CI +// runner hosts (G-012). Closes the gap where runner hosts were hand-built +// so required system dependencies were discovered only when a job failed, +// and two hosts advertising identical GitHub Actions labels could +// silently drift apart. +// Inputs: none directly — see runner_provision.go and runner_verify.go for +// each subcommand's flags. +// Outputs: registers `runner` (with `provision` and `verify` subcommands) +// on RootCmd. +// Constraints: all host-affecting logic lives in internal/runner; this file +// is wiring only, matching every other command group in cmd/commands. +// SPORT: CLI-CMD-RUNNER-001 + +import ( + "github.com/nself-org/cli/internal/runner" + "github.com/spf13/cobra" +) + +var runnerCmd = &cobra.Command{ + Use: "runner", + Short: "Provision and audit self-hosted GitHub Actions CI runner hosts", + Long: `Provision and audit self-hosted GitHub Actions CI runner hosts. + +Runner hosts were previously hand-built: required system dependencies (gh, +zip, unzip, Playwright/Chromium's shared libraries, ...) were discovered +only when a job failed mid-run, and two hosts advertising the identical +GitHub Actions labels (self-hosted,Linux,X64) could silently drift apart — +the same commit would pass or fail depending on which host claimed the job. + +The dependency set is declarative (internal/runner/manifest.yaml, compiled +into this binary) so provision and verify always check the same list. + +Subcommands: + provision Install dependencies, create the runner user, register N + runner instances as systemd services + verify Check one or more hosts against the manifest and print a + parity matrix — the important half: this is how "same + labels, different tools" gets caught before it causes a + mystery failure.`, + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() + }, +} + +func init() { + runnerCmd.AddCommand(runnerProvisionCmd) + runnerCmd.AddCommand(runnerVerifyCmd) + RootCmd.AddCommand(runnerCmd) +} + +// runnerExecutorsFromFlags builds one runner.Executor per --host flag +// value, or a single runner.LocalExecutor when no --host is given. Shared +// by both subcommands so "no --host means check/act on this machine" and +// "user@host means SSH" behave identically for provision and verify. +func runnerExecutorsFromFlags(hosts []string, sshKey string) []runner.Executor { + if len(hosts) == 0 { + return []runner.Executor{runner.LocalExecutor{}} + } + executors := make([]runner.Executor, len(hosts)) + for i, h := range hosts { + if h == "local" { + executors[i] = runner.LocalExecutor{} + continue + } + executors[i] = runner.NewSSHExecutor(h, sshKey) + } + return executors +} diff --git a/cmd/commands/runner_provision.go b/cmd/commands/runner_provision.go new file mode 100644 index 00000000..58303eee --- /dev/null +++ b/cmd/commands/runner_provision.go @@ -0,0 +1,102 @@ +package commands + +// Purpose: `nself runner provision [--host user@host]` — install the +// manifest's dependency set on a runner host, create the runner user +// with passwordless sudo, and register N runner instances as systemd +// services. See internal/runner/provision.go for the actual steps. +// Inputs: --host (repeatable; local when omitted), --instances, +// --install-root, --github-url, --labels, --token (or GITHUB_RUNNER_TOKEN +// env), --ssh-key. +// Outputs: each provisioning step's name + captured output printed to +// stdout; non-zero exit on the first failing step. +// Constraints: the registration token is read from an env var by default +// and never logged or echoed back — see runProvision below. + +import ( + "context" + "fmt" + "os" + + "github.com/nself-org/cli/internal/runner" + "github.com/spf13/cobra" +) + +var runnerProvisionCmd = &cobra.Command{ + Use: "provision", + Short: "Install runner dependencies, user, sudoers, and N runner instances", + Long: `Install the declarative dependency set (internal/runner/manifest.yaml), +create the runner service user with passwordless sudo, ensure the shared +work directory is a real directory (never a symlink — a symlinked _work +silently breaks actions/checkout's credential injection), and register +--instances runner instances as systemd services. + +Every step is idempotent: safe to re-run against a host that's already +partially provisioned. + +Examples: + nself runner provision --github-url https://github.com/nself-org/cli + nself runner provision --host ci@167.235.x.x --instances 2 --labels nself-ci + GITHUB_RUNNER_TOKEN=... nself runner provision --github-url https://github.com/nself-org/web`, + RunE: runRunnerProvision, +} + +func init() { + runnerProvisionCmd.Flags().StringSlice("host", nil, + "SSH target(s) user@host to provision (repeatable). Default: this machine.") + runnerProvisionCmd.Flags().String("ssh-key", "", "SSH private key path (default: NSELF_DEPLOY_KEY_PATH or ~/.ssh/id_ed25519)") + runnerProvisionCmd.Flags().Int("instances", 1, "Number of runner instances to install on this host") + runnerProvisionCmd.Flags().String("install-root", "", "Base install directory (default: /opt/actions-runner)") + runnerProvisionCmd.Flags().String("github-url", "", "Repo or org URL runners register against, e.g. https://github.com/nself-org/cli") + runnerProvisionCmd.Flags().StringSlice("labels", nil, "Extra labels appended after self-hosted,Linux,X64") + runnerProvisionCmd.Flags().String("token", "", "GitHub Actions runner registration token (overrides GITHUB_RUNNER_TOKEN env). Never logged.") +} + +func runRunnerProvision(cmd *cobra.Command, args []string) error { + hosts, _ := cmd.Flags().GetStringSlice("host") + sshKey, _ := cmd.Flags().GetString("ssh-key") + instances, _ := cmd.Flags().GetInt("instances") + installRoot, _ := cmd.Flags().GetString("install-root") + githubURL, _ := cmd.Flags().GetString("github-url") + labels, _ := cmd.Flags().GetStringSlice("labels") + token, _ := cmd.Flags().GetString("token") + + if token == "" { + token = os.Getenv("GITHUB_RUNNER_TOKEN") + } + if githubURL == "" { + return fmt.Errorf("--github-url is required (the repo or org runners register against)") + } + if token == "" { + return fmt.Errorf("a runner registration token is required: pass --token or set GITHUB_RUNNER_TOKEN") + } + if len(hosts) > 1 { + return fmt.Errorf("provision takes at most one --host per invocation; run it once per host") + } + + m, err := runner.LoadEmbeddedManifest() + if err != nil { + return err + } + + executors := runnerExecutorsFromFlags(hosts, sshKey) + ex := executors[0] + + opts := runner.ProvisionOptions{ + Instances: instances, + InstallRoot: installRoot, + GithubURL: githubURL, + RegToken: token, + Labels: labels, + } + + fmt.Printf("Provisioning %s (%d instance(s))...\n", ex.Label(), opts.Instances) + result, err := runner.Provision(context.Background(), ex, m, opts) + for _, step := range result.Steps { + fmt.Printf(" [%s] %s\n", step.Name, step.Output) + } + if err != nil { + return err + } + fmt.Println("Provision complete.") + return nil +} diff --git a/cmd/commands/runner_test.go b/cmd/commands/runner_test.go new file mode 100644 index 00000000..5b6ba13b --- /dev/null +++ b/cmd/commands/runner_test.go @@ -0,0 +1,123 @@ +package commands + +// Purpose: guard the `nself runner` command wiring itself (flag +// registration, host/executor resolution, the verify exit-code policy) — +// internal/runner's own package tests cover provision/verify logic. +// Inputs: none (pure function tests + cobra tree inspection). +// Outputs: none. +// Constraints: never invokes RunE — that would shell out — only inspects +// registered flags and exercises the small helper functions in this +// package directly. + +import ( + "testing" + + "github.com/nself-org/cli/internal/runner" +) + +func TestRunnerCmd_RegisteredOnRoot(t *testing.T) { + found := false + for _, c := range RootCmd.Commands() { + if c.Name() == "runner" { + found = true + } + } + if !found { + t.Fatal("runner command not registered on RootCmd") + } +} + +func TestRunnerCmd_HasProvisionAndVerifySubcommands(t *testing.T) { + names := map[string]bool{} + for _, c := range runnerCmd.Commands() { + names[c.Name()] = true + } + if !names["provision"] || !names["verify"] { + t.Fatalf("expected provision and verify subcommands, got %v", names) + } +} + +func TestRunnerExecutorsFromFlags_DefaultsToLocal(t *testing.T) { + executors := runnerExecutorsFromFlags(nil, "") + if len(executors) != 1 { + t.Fatalf("len(executors) = %d, want 1", len(executors)) + } + if executors[0].Label() != "local" { + t.Fatalf("Label() = %q, want local", executors[0].Label()) + } +} + +func TestRunnerExecutorsFromFlags_BuildsSSHPerHost(t *testing.T) { + executors := runnerExecutorsFromFlags([]string{"ci@a.example", "ci@b.example"}, "/key") + if len(executors) != 2 { + t.Fatalf("len(executors) = %d, want 2", len(executors)) + } + if executors[0].Label() != "ci@a.example" || executors[1].Label() != "ci@b.example" { + t.Fatalf("labels = %q, %q", executors[0].Label(), executors[1].Label()) + } +} + +func TestRunnerExecutorsFromFlags_LocalKeyword(t *testing.T) { + executors := runnerExecutorsFromFlags([]string{"local"}, "") + if executors[0].Label() != "local" { + t.Fatalf("Label() = %q, want local for the literal \"local\" host", executors[0].Label()) + } +} + +func TestRunnerVerifyFoundProblems_CleanReportsAreFalse(t *testing.T) { + reports := []runner.HostReport{ + {Host: "a", Checks: []runner.CheckResult{{Name: "dep:git", Status: runner.StatusPass}}}, + } + if runnerVerifyFoundProblems(reports) { + t.Error("expected no problems for an all-pass report") + } +} + +func TestRunnerVerifyFoundProblems_FailingCheckIsTrue(t *testing.T) { + reports := []runner.HostReport{ + {Host: "a", Checks: []runner.CheckResult{{Name: "dep:gh", Status: runner.StatusFail}}}, + } + if !runnerVerifyFoundProblems(reports) { + t.Error("expected a failing check to be reported as a problem") + } +} + +func TestRunnerVerifyFoundProblems_UnreachableHostIsTrue(t *testing.T) { + reports := []runner.HostReport{{Host: "a", Err: "connection refused"}} + if !runnerVerifyFoundProblems(reports) { + t.Error("expected an unreachable host to be reported as a problem") + } +} + +func TestRunnerVerifyFoundProblems_DriftIsTrue(t *testing.T) { + reports := []runner.HostReport{ + {Host: "a", Checks: []runner.CheckResult{{Name: "dep:gh", Status: runner.StatusPass}}}, + {Host: "b", Checks: []runner.CheckResult{{Name: "dep:gh", Status: runner.StatusFail}}}, + } + if !runnerVerifyFoundProblems(reports) { + t.Error("expected cross-host drift to be reported as a problem") + } +} + +func TestRunnerProvisionCmd_RequiresGithubURLAndToken(t *testing.T) { + // Flags are registered with empty defaults; runRunnerProvision itself + // validates presence (see runner_provision.go) rather than cobra + // required-flag machinery, so both --github-url and a token source are + // enforced. This test only asserts the flags exist with the documented + // names — the validation path is exercised implicitly by code review of + // runRunnerProvision's early-return guards, since invoking RunE would + // shell out. + for _, name := range []string{"host", "ssh-key", "instances", "install-root", "github-url", "labels", "token"} { + if runnerProvisionCmd.Flags().Lookup(name) == nil { + t.Errorf("runner provision missing --%s flag", name) + } + } +} + +func TestRunnerVerifyCmd_HasExpectedFlags(t *testing.T) { + for _, name := range []string{"host", "ssh-key", "json"} { + if runnerVerifyCmd.Flags().Lookup(name) == nil { + t.Errorf("runner verify missing --%s flag", name) + } + } +} diff --git a/cmd/commands/runner_verify.go b/cmd/commands/runner_verify.go new file mode 100644 index 00000000..6556eed1 --- /dev/null +++ b/cmd/commands/runner_verify.go @@ -0,0 +1,100 @@ +package commands + +// Purpose: `nself runner verify [--host ...]` — the important half of +// G-012. Checks one or more hosts against the manifest and prints a +// parity matrix, so "same GitHub Actions labels, different tools +// installed" (gh/zip/unzip present on one host, absent on another, +// 2026-09-11) is caught by a single command instead of a mystery job +// failure days later. +// Inputs: --host (repeatable; local when omitted), --ssh-key, --json. +// Outputs: a text parity matrix (default) or a JSON []runner.HostReport +// (--json); exit 1 if any reachable host has a failing check or any +// check drifts across hosts. +// Constraints: never contacts a host that isn't named by the caller via +// --host or implied by "local" — no hardcoded host list. + +import ( + "context" + "encoding/json" + "fmt" + "os" + + "github.com/nself-org/cli/internal/runner" + "github.com/spf13/cobra" +) + +var runnerVerifyCmd = &cobra.Command{ + Use: "verify", + Short: "Check host(s) against the dependency manifest and report drift", + Long: `Check one or more hosts against internal/runner/manifest.yaml's declarative +dependency set: every required package/binary, whether the shared work +directory is a real directory (not a symlink), and whether any cached +Chromium resolves its shared libraries (the exact failure that hid for +hours as a misleading "Target page, context or browser has been closed" +on 2026-09-11). + +Pass multiple --host flags to check several hosts in one run and print a +parity matrix — this is how two hosts advertising identical GitHub Actions +labels but different installed tools gets caught before it causes a job +to pass or fail depending on luck. + +Examples: + nself runner verify + nself runner verify --host ci@167.235.x.x --host ci@167.233.x.x + nself runner verify --host ci@167.235.x.x --json`, + RunE: runRunnerVerify, +} + +func init() { + runnerVerifyCmd.Flags().StringSlice("host", nil, + "SSH target(s) user@host to verify (repeatable). Default: this machine.") + runnerVerifyCmd.Flags().String("ssh-key", "", "SSH private key path (default: NSELF_DEPLOY_KEY_PATH or ~/.ssh/id_ed25519)") + runnerVerifyCmd.Flags().Bool("json", false, "Print raw JSON ([]runner.HostReport) instead of the text parity matrix") +} + +func runRunnerVerify(cmd *cobra.Command, args []string) error { + hosts, _ := cmd.Flags().GetStringSlice("host") + sshKey, _ := cmd.Flags().GetString("ssh-key") + asJSON, _ := cmd.Flags().GetBool("json") + + m, err := runner.LoadEmbeddedManifest() + if err != nil { + return err + } + + executors := runnerExecutorsFromFlags(hosts, sshKey) + reports := runner.VerifyHosts(context.Background(), executors, m) + + if asJSON { + enc := json.NewEncoder(os.Stdout) + enc.SetIndent("", " ") + if err := enc.Encode(reports); err != nil { + return err + } + } else { + fmt.Print(runner.RenderMatrix(reports)) + } + + if runnerVerifyFoundProblems(reports) { + os.Exit(1) + } + return nil +} + +// runnerVerifyFoundProblems reports true when any reachable host has a +// failing check, any host is entirely unreachable, or any check drifts +// across hosts — the three conditions that should make `nself runner +// verify` a usable CI gate, not just an FYI. +func runnerVerifyFoundProblems(reports []runner.HostReport) bool { + for _, r := range reports { + if r.Err != "" { + return true + } + for _, c := range r.Checks { + if c.Status == runner.StatusFail { + return true + } + } + } + return len(runner.DetectDrift(reports)) > 0 +} diff --git a/internal/runner/chromium.go b/internal/runner/chromium.go new file mode 100644 index 00000000..bd7fb7fc --- /dev/null +++ b/internal/runner/chromium.go @@ -0,0 +1,93 @@ +package runner + +// Purpose: the check that catches the specific failure mode that hid for +// hours on 2026-09-11 — `npx playwright install chromium` (the fallback +// path when `--with-deps` fails because sudo needs a terminal) downloads +// a Chromium binary with none of its shared libraries, so the browser +// crashes with `chrome: error while loading shared libraries: +// libnspr4.so`, which Playwright then reports as the generic, misleading +// "Target page, context or browser has been closed". Running `ldd` on +// any cached Chromium and grepping for "not found" catches this directly +// instead of waiting for a job to hit it. +// Inputs: Manifest.ChromiumCacheGlobs (home-relative glob patterns) and an +// Executor. +// Outputs: one CheckResult per cached Chromium/headless-shell binary found +// across every home directory on the host, plus a single StatusWarn +// result when none is cached yet (not itself a failure — Playwright may +// simply never have run there). +// Constraints: expansion happens in the remote/local shell (bash globbing +// with nullglob), not in Go, because Executor has no directory-listing +// primitive of its own and adding one would duplicate what the shell +// already does correctly. + +import ( + "context" + "strings" +) + +const chromiumWarnName = "chromium:cache" + +// checkChromiumLdd finds every cached Chromium/headless-shell binary under +// each home directory and runs `ldd` against it, reporting any shared +// library `ldd` could not resolve. +func checkChromiumLdd(ctx context.Context, ex Executor, m *Manifest) []CheckResult { + if len(m.ChromiumCacheGlobs) == 0 { + return nil + } + out, err := ex.Run(ctx, chromiumLddScript(m.ChromiumCacheGlobs)) + if err != nil && out == "" { + return []CheckResult{{Name: chromiumWarnName, Status: StatusWarn, + Detail: "could not scan for cached Chromium: " + err.Error()}} + } + return parseChromiumLddOutput(out) +} + +// chromiumLddScript builds the shell script that walks every home +// directory, expands each glob (with %h substituted for the loop +// variable), and ldd's anything executable it finds. +func chromiumLddScript(globs []string) string { + var patterns []string + for _, g := range globs { + patterns = append(patterns, strings.ReplaceAll(g, "%h", "$home")) + } + return "shopt -s nullglob; for home in /root /home/*; do for f in " + + strings.Join(patterns, " ") + + `; do [ -x "$f" ] || continue; echo "CHROME $f"; ldd "$f" 2>&1 | grep "not found" | sed "s/^/MISSING /"; done; done` +} + +// parseChromiumLddOutput turns chromiumLddScript's "CHROME " / +// "MISSING " output into one CheckResult per binary found, or a +// single StatusWarn result when nothing was cached. +func parseChromiumLddOutput(out string) []CheckResult { + var results []CheckResult + var current *CheckResult + var missingLibs []string + flush := func() { + if current == nil { + return + } + if len(missingLibs) > 0 { + current.Status = StatusFail + current.Detail = "missing: " + strings.Join(missingLibs, "; ") + } + results = append(results, *current) + } + for _, line := range strings.Split(out, "\n") { + switch { + case strings.HasPrefix(line, "CHROME "): + flush() + path := strings.TrimPrefix(line, "CHROME ") + missingLibs = nil + current = &CheckResult{Name: "chromium:ldd:" + path, Status: StatusPass, + Detail: path + " resolves all shared libraries"} + case strings.HasPrefix(line, "MISSING ") && current != nil: + missingLibs = append(missingLibs, strings.TrimPrefix(line, "MISSING ")) + } + } + flush() + if len(results) == 0 { + return []CheckResult{{Name: chromiumWarnName, Status: StatusWarn, + Detail: "no cached Chromium found under any home directory (Playwright has not run here yet)"}} + } + return results +} diff --git a/internal/runner/chromium_test.go b/internal/runner/chromium_test.go new file mode 100644 index 00000000..5d8143c9 --- /dev/null +++ b/internal/runner/chromium_test.go @@ -0,0 +1,71 @@ +package runner + +import ( + "context" + "strings" + "testing" +) + +func TestChromiumLddScript_SubstitutesHome(t *testing.T) { + script := chromiumLddScript([]string{"%h/.cache/ms-playwright/chromium-*/chrome-linux/chrome"}) + if !strings.Contains(script, "$home/.cache/ms-playwright/chromium-*/chrome-linux/chrome") { + t.Fatalf("script did not substitute %%h: %s", script) + } + if !strings.Contains(script, "nullglob") { + t.Fatalf("script missing nullglob guard: %s", script) + } +} + +func TestParseChromiumLddOutput_AllLibsResolve(t *testing.T) { + out := "CHROME /home/gha-runner/.cache/ms-playwright/chromium-123/chrome-linux/chrome" + results := parseChromiumLddOutput(out) + if len(results) != 1 || results[0].Status != StatusPass { + t.Fatalf("results = %+v, want single pass", results) + } +} + +func TestParseChromiumLddOutput_MissingLib(t *testing.T) { + out := "CHROME /home/gha-runner/.cache/ms-playwright/chromium-123/chrome-linux/chrome\n" + + "MISSING \tlibnspr4.so => not found" + results := parseChromiumLddOutput(out) + if len(results) != 1 { + t.Fatalf("results = %+v, want 1", results) + } + if results[0].Status != StatusFail { + t.Fatalf("status = %v, want fail — this is the exact 2026-09-11 failure mode", results[0].Status) + } + if !strings.Contains(results[0].Detail, "libnspr4.so") { + t.Fatalf("detail = %q, want it to name the missing lib", results[0].Detail) + } +} + +func TestParseChromiumLddOutput_MultipleBinaries(t *testing.T) { + out := "CHROME /a/chrome\n" + + "MISSING libnspr4.so => not found\n" + + "CHROME /b/headless_shell\n" + results := parseChromiumLddOutput(out) + if len(results) != 2 { + t.Fatalf("results = %+v, want 2", results) + } + if results[0].Status != StatusFail { + t.Errorf("first binary should be fail, got %v", results[0].Status) + } + if results[1].Status != StatusPass { + t.Errorf("second binary should be pass, got %v", results[1].Status) + } +} + +func TestParseChromiumLddOutput_NoneCached(t *testing.T) { + results := parseChromiumLddOutput("") + if len(results) != 1 || results[0].Status != StatusWarn || results[0].Name != chromiumWarnName { + t.Fatalf("results = %+v, want single warn", results) + } +} + +func TestCheckChromiumLdd_NoGlobsConfigured(t *testing.T) { + m := &Manifest{} + results := checkChromiumLdd(context.Background(), &fakeExecutor{}, m) + if results != nil { + t.Fatalf("expected nil when no globs configured, got %+v", results) + } +} diff --git a/internal/runner/exec.go b/internal/runner/exec.go new file mode 100644 index 00000000..b2cd1349 --- /dev/null +++ b/internal/runner/exec.go @@ -0,0 +1,92 @@ +package runner + +// Purpose: a single command-execution seam (Executor) that provision.go and +// verify.go run every host action through, so tests substitute a fake +// recorder instead of shelling out, and so `verify` can be pointed at +// several hosts (SSH) or the local machine with identical calling code. +// Inputs: a shell command string. +// Outputs: combined stdout+stderr and an error (non-zero exit is returned +// as an error, mirroring os/exec.CombinedOutput's contract). +// Constraints: SSHExecutor reuses internal/deploy's exported SSH primitives +// (RunRemoteCommand/RemoteTarget) rather than opening a second, divergent +// SSH code path — the same reuse rule the P7-E5 runner-fleet ticket +// applies to job dispatch applies here to host verification. + +import ( + "context" + "os" + "os/exec" + "path/filepath" + "strings" + + "github.com/nself-org/cli/internal/deploy" +) + +// Executor runs one shell command against a bound host and reports its +// label (used in verify's parity matrix and provision's log lines). +type Executor interface { + Run(ctx context.Context, command string) (output string, err error) + Label() string +} + +// LocalExecutor runs commands on the current machine via `bash -c`. Bash +// (not POSIX sh) is required because the chromium-cache check uses +// `shopt -s nullglob`; every provisioning target (Ubuntu runner hosts) and +// every dev machine this ships on has bash available. +type LocalExecutor struct{} + +// Run implements Executor. +func (LocalExecutor) Run(ctx context.Context, command string) (string, error) { + cmd := exec.CommandContext(ctx, "bash", "-c", command) + out, err := cmd.CombinedOutput() + return strings.TrimSpace(string(out)), err +} + +// Label implements Executor. +func (LocalExecutor) Label() string { return "local" } + +// SSHExecutor runs commands on a remote host over SSH, reusing +// deploy.RunRemoteCommand so runner provisioning shares exactly one SSH +// convention (key resolution, StrictHostKeyChecking, ForwardAgent=no) with +// `nself deploy` and every other remote-targeting command in the CLI. +type SSHExecutor struct { + Target deploy.RemoteTarget +} + +// NewSSHExecutor builds an SSHExecutor from a "user@host" (or +// "user@host:/path", though runner provisioning ignores the path +// component) string and an SSH key path. An empty keyPath resolves the +// same NSELF_DEPLOY_KEY_PATH / NSELF_DEPLOY_SSH_KEY env-var convention +// `nself deploy` uses, defaulting to ~/.ssh/id_ed25519, so every +// remote-targeting command in the CLI shares one key-resolution rule. +func NewSSHExecutor(sshTarget, keyPath string) SSHExecutor { + if keyPath == "" { + keyPath = defaultSSHKeyPath() + } + return SSHExecutor{Target: deploy.RemoteTarget{ + SSHTarget: sshTarget, + KeyPath: keyPath, + }} +} + +// defaultSSHKeyPath mirrors deploy's unexported sshKeyPathEnv() default +// resolution (that function isn't exported; this is the same three-step +// fallback, not a divergent convention). +func defaultSSHKeyPath() string { + if k := os.Getenv("NSELF_DEPLOY_KEY_PATH"); k != "" { + return k + } + if k := os.Getenv("NSELF_DEPLOY_SSH_KEY"); k != "" { + return k + } + home, _ := os.UserHomeDir() + return filepath.Join(home, ".ssh", "id_ed25519") +} + +// Run implements Executor. +func (e SSHExecutor) Run(ctx context.Context, command string) (string, error) { + return deploy.RunRemoteCommand(ctx, e.Target, command) +} + +// Label implements Executor. +func (e SSHExecutor) Label() string { return e.Target.SSHTarget } diff --git a/internal/runner/exec_test.go b/internal/runner/exec_test.go new file mode 100644 index 00000000..405648e1 --- /dev/null +++ b/internal/runner/exec_test.go @@ -0,0 +1,79 @@ +package runner + +// Note: LocalExecutor tests below run `echo`/`true` on whatever machine runs +// `go test` (dev laptop or CI runner already executing this test binary) — +// never against nself's protected staging/prod runner hosts. SSHExecutor +// tests only check argument construction, never open a real connection. + +import ( + "context" + "os" + "strings" + "testing" +) + +func TestLocalExecutor_RunReturnsOutput(t *testing.T) { + var ex LocalExecutor + out, err := ex.Run(context.Background(), "echo hello-runner") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out != "hello-runner" { + t.Fatalf("out = %q, want %q", out, "hello-runner") + } + if ex.Label() != "local" { + t.Fatalf("Label() = %q", ex.Label()) + } +} + +func TestLocalExecutor_RunReturnsErrorOnNonZeroExit(t *testing.T) { + var ex LocalExecutor + _, err := ex.Run(context.Background(), "exit 7") + if err == nil { + t.Fatal("expected an error for a non-zero exit") + } +} + +func TestLocalExecutor_SupportsNullglob(t *testing.T) { + // Regression guard: chromium.go relies on `shopt -s nullglob` working, + // which requires bash, not POSIX sh. + var ex LocalExecutor + out, err := ex.Run(context.Background(), "shopt -s nullglob; for f in /no/such/path/*; do echo FOUND; done; echo done") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if out != "done" { + t.Fatalf("out = %q, nullglob should leave the loop empty", out) + } +} + +func TestNewSSHExecutor_DefaultsKeyPath(t *testing.T) { + os.Unsetenv("NSELF_DEPLOY_KEY_PATH") + os.Unsetenv("NSELF_DEPLOY_SSH_KEY") + ex := NewSSHExecutor("ci-user@runner-host", "") + if ex.Target.KeyPath == "" { + t.Fatal("expected a default key path, got empty") + } + if !strings.HasSuffix(ex.Target.KeyPath, "id_ed25519") { + t.Fatalf("KeyPath = %q, want default id_ed25519", ex.Target.KeyPath) + } + if ex.Label() != "ci-user@runner-host" { + t.Fatalf("Label() = %q", ex.Label()) + } +} + +func TestNewSSHExecutor_HonorsExplicitKeyPath(t *testing.T) { + ex := NewSSHExecutor("ci-user@runner-host", "/custom/key") + if ex.Target.KeyPath != "/custom/key" { + t.Fatalf("KeyPath = %q, want /custom/key", ex.Target.KeyPath) + } +} + +func TestNewSSHExecutor_UsesEnvKeyPath(t *testing.T) { + os.Setenv("NSELF_DEPLOY_KEY_PATH", "/env/key") + defer os.Unsetenv("NSELF_DEPLOY_KEY_PATH") + ex := NewSSHExecutor("ci-user@runner-host", "") + if ex.Target.KeyPath != "/env/key" { + t.Fatalf("KeyPath = %q, want /env/key", ex.Target.KeyPath) + } +} diff --git a/internal/runner/fake_test.go b/internal/runner/fake_test.go new file mode 100644 index 00000000..77e290c5 --- /dev/null +++ b/internal/runner/fake_test.go @@ -0,0 +1,49 @@ +package runner + +// Purpose: a fake Executor shared by every *_test.go in this package, so +// provision/verify logic is exercised without ever shelling out — the +// G-012 brief requires tests never touch a real host. +// Inputs: substring-matched rules registered by each test. +// Outputs: recorded command history (fakeExecutor.commands) for assertions +// on exactly what would have been sent to a real host. + +import ( + "context" + "strings" +) + +type fakeResponse struct { + out string + err error +} + +type fakeRule struct { + contains string + resp fakeResponse +} + +// fakeExecutor implements Executor. Rules are checked in order; the first +// whose `contains` substring matches the command wins. commands records +// every command passed to Run, in order, for assertions. +type fakeExecutor struct { + label string + rules []fakeRule + fallback fakeResponse + commands []string +} + +func (f *fakeExecutor) Run(_ context.Context, command string) (string, error) { + f.commands = append(f.commands, command) + for _, r := range f.rules { + if strings.Contains(command, r.contains) { + return r.resp.out, r.resp.err + } + } + return f.fallback.out, f.fallback.err +} + +func (f *fakeExecutor) Label() string { return f.label } + +func (f *fakeExecutor) when(contains string, out string, err error) { + f.rules = append(f.rules, fakeRule{contains: contains, resp: fakeResponse{out: out, err: err}}) +} diff --git a/internal/runner/manifest.go b/internal/runner/manifest.go new file mode 100644 index 00000000..cd3378bc --- /dev/null +++ b/internal/runner/manifest.go @@ -0,0 +1,99 @@ +// Package runner implements `nself runner provision` and `nself runner +// verify` (G-012): building and auditing self-hosted GitHub Actions CI +// runner hosts. +// +// Purpose: CI runner hosts were hand-built, so required system dependencies +// +// were discovered only when a job failed mid-run, and two hosts +// advertising the identical GitHub Actions labels (self-hosted,Linux,X64) +// could silently drift apart — one had `gh`/`zip`/`unzip`, the other +// didn't, and the same commit passed or failed depending on which host +// claimed the job. +// +// Inputs: a declarative Manifest (manifest.yaml, embedded below) plus an +// +// Executor bound to one host (local or SSH). +// +// Outputs: for provision, packages/user/sudoers/systemd units installed on +// +// that host; for verify, a per-host CheckResult set that composeable +// callers turn into a cross-host parity matrix (report.go). +// +// Constraints: this package never talks to a host directly — every side +// +// effect goes through the Executor interface (exec.go), so unit tests +// exercise the exact command strings without touching real hardware. +package runner + +import ( + _ "embed" + "fmt" + + "gopkg.in/yaml.v3" +) + +//go:embed manifest.yaml +var manifestYAML []byte + +// Dependency is one required system package, checked either by looking for +// a binary on PATH (Binary != "") or by asking the package manager whether +// AptPackage is installed (Binary == "", e.g. a runtime library with no +// CLI entry point). +type Dependency struct { + Name string `yaml:"name"` + AptPackage string `yaml:"apt_package"` + Binary string `yaml:"binary"` + Reason string `yaml:"reason"` +} + +// HasBinary reports whether this dependency is checked via `command -v` +// rather than the package manager. +func (d Dependency) HasBinary() bool { return d.Binary != "" } + +// Manifest is the parsed form of manifest.yaml: the full declarative +// dependency set plus the fixed layout conventions (runner user, work dir, +// where to look for a cached Chromium) that provision and verify both need +// to agree on. +type Manifest struct { + SchemaVersion int `yaml:"schema_version"` + RunnerUser string `yaml:"runner_user"` + WorkDir string `yaml:"work_dir"` + Dependencies []Dependency `yaml:"dependencies"` + ChromiumCacheGlobs []string `yaml:"chromium_cache_globs"` +} + +// LoadEmbeddedManifest parses the manifest compiled into the binary. +// Returns an error if the embedded YAML is malformed, which would indicate +// a build-time bug (a hand edit that broke the schema), not a runtime +// condition — callers should treat a non-nil error as fatal. +func LoadEmbeddedManifest() (*Manifest, error) { + return ParseManifest(manifestYAML) +} + +// ParseManifest decodes manifest YAML bytes. Exported so tests (and any +// future NSELF_RUNNER_MANIFEST override, mirroring the deprecation +// registry's pattern) can parse an alternate manifest without touching the +// embedded default. +func ParseManifest(data []byte) (*Manifest, error) { + var m Manifest + if err := yaml.Unmarshal(data, &m); err != nil { + return nil, fmt.Errorf("runner: parse manifest: %w", err) + } + if m.SchemaVersion != 1 { + return nil, fmt.Errorf("runner: unsupported manifest schema_version %d (want 1)", m.SchemaVersion) + } + if len(m.Dependencies) == 0 { + return nil, fmt.Errorf("runner: manifest declares zero dependencies") + } + return &m, nil +} + +// AptPackages returns the apt package names for every dependency, in +// manifest order, for a single `apt-get install -y ` invocation. +func (m *Manifest) AptPackages() []string { + pkgs := make([]string, len(m.Dependencies)) + for i, d := range m.Dependencies { + pkgs[i] = d.AptPackage + } + return pkgs +} diff --git a/internal/runner/manifest.yaml b/internal/runner/manifest.yaml new file mode 100644 index 00000000..77197c74 --- /dev/null +++ b/internal/runner/manifest.yaml @@ -0,0 +1,175 @@ +# CI Runner Host Dependency Manifest (G-012) +# +# Declarative source of truth for what a self-hosted GitHub Actions CI runner +# host must have installed. Compiled into the binary via go:embed +# (manifest.go) and read by both `nself runner provision` (install what's +# missing) and `nself runner verify` (report what's missing/drifted). +# +# Every entry here exists because a real job failed without it — see +# `reason`. Do not remove an entry without checking that reason still holds. +# +# apt_package is the Debian/Ubuntu package name `provision` installs. +# binary, when set, is the command `verify` runs `command -v ` on. +# When binary is empty, the dependency is a library with no CLI entry point +# and verify falls back to `dpkg -s `. +schema_version: 1 + +runner_user: gha-runner +work_dir: /opt/actions-runner/_work + +dependencies: + - name: git + apt_package: git + binary: git + reason: actions/checkout requires git on PATH. + + - name: jq + apt_package: jq + binary: jq + reason: workflow steps and nself-ci parse JSON with jq. + + - name: yq + apt_package: yq + binary: yq + reason: workflow steps parse YAML (compose files, SPORT) with yq. + + - name: gh + apt_package: gh + binary: gh + reason: >- + nself ci posts commit statuses via gh OAuth; release workflows use gh + release/gh api. Present on one prior runner host and absent on another + while both advertised self-hosted,Linux,X64 — the same commit passed or + failed depending on which host claimed the job (2026-09-11). + + - name: make + apt_package: make + binary: make + reason: >- + release.yml and multiple Makefile-driven build steps require make. + Missing on a runner host broke cli release.yml (see + project_sentry_runner_ci_tools.md). + + - name: zip + apt_package: zip + binary: zip + reason: release packaging steps shell out to zip; same incident as make/gh above. + + - name: unzip + apt_package: unzip + binary: unzip + reason: release packaging and several actions extract .zip archives. + + - name: curl + apt_package: curl + binary: curl + reason: health checks, artifact downloads, and install scripts use curl. + + - name: docker + apt_package: docker.io + binary: docker + reason: nself-ci runs each job's gate suite in an ephemeral Docker container. + + - name: build-essential + apt_package: build-essential + binary: gcc + reason: >- + CGO-enabled Go builds and native Node addons need a C toolchain. + build-essential ships no binary of its own; gcc is the proxy check. + + - name: pkg-config + apt_package: pkg-config + binary: pkg-config + reason: native library builds (e.g. libpq, sqlite3) resolve cflags via pkg-config. + + - name: libssl-dev + apt_package: libssl-dev + binary: "" + reason: TLS-linked native builds need OpenSSL headers. + + # --- Playwright / Chromium system libraries ----------------------------- + # None of these ship a binary; verify checks them via dpkg -s. This is the + # exact list `playwright install --with-deps` would have installed had sudo + # not required a terminal for the password on the runner service account + # (2026-09-11). The `npx playwright install chromium` fallback then + # "succeeded" but produced a browser with no shared libs, failing minutes + # later as `chrome: error while loading shared libraries: libnspr4.so` — + # which then surfaced even later, misleadingly, as a generic Playwright + # "Target page, context or browser has been closed". verify's chromium-ldd + # check (see verify.go) exists specifically to catch this before a job runs. + - name: libnspr4 + apt_package: libnspr4 + binary: "" + reason: Playwright/Chromium runtime dependency (see note above). + - name: libnss3 + apt_package: libnss3 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libatk1.0-0t64 + apt_package: libatk1.0-0t64 + binary: "" + reason: Playwright/Chromium runtime dependency (Ubuntu 24.04 t64 package name). + - name: libatk-bridge2.0-0t64 + apt_package: libatk-bridge2.0-0t64 + binary: "" + reason: Playwright/Chromium runtime dependency (Ubuntu 24.04 t64 package name). + - name: libcups2t64 + apt_package: libcups2t64 + binary: "" + reason: Playwright/Chromium runtime dependency (Ubuntu 24.04 t64 package name). + - name: libdrm2 + apt_package: libdrm2 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libxkbcommon0 + apt_package: libxkbcommon0 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libxcomposite1 + apt_package: libxcomposite1 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libxdamage1 + apt_package: libxdamage1 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libxfixes3 + apt_package: libxfixes3 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libxrandr2 + apt_package: libxrandr2 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libgbm1 + apt_package: libgbm1 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libpango-1.0-0 + apt_package: libpango-1.0-0 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libcairo2 + apt_package: libcairo2 + binary: "" + reason: Playwright/Chromium runtime dependency. + - name: libasound2t64 + apt_package: libasound2t64 + binary: "" + reason: Playwright/Chromium runtime dependency (Ubuntu 24.04 t64 package name). + - name: libatspi2.0-0t64 + apt_package: libatspi2.0-0t64 + binary: "" + reason: Playwright/Chromium runtime dependency (Ubuntu 24.04 t64 package name). + - name: fonts-liberation + apt_package: fonts-liberation + binary: "" + reason: Playwright/Chromium needs a default font set to render pages. + +# Paths (glob patterns, expanded per-user at runtime) verify searches for a +# cached Chromium binary to run `ldd` against. Playwright's default cache +# location; the primary runner_user's home is always checked, plus any other +# home directories under /home when running as root. +chromium_cache_globs: + - "%h/.cache/ms-playwright/chromium-*/chrome-linux/chrome" + - "%h/.cache/ms-playwright/chromium_headless_shell-*/chrome-linux/headless_shell" diff --git a/internal/runner/manifest_test.go b/internal/runner/manifest_test.go new file mode 100644 index 00000000..105a7bf5 --- /dev/null +++ b/internal/runner/manifest_test.go @@ -0,0 +1,66 @@ +package runner + +import "testing" + +func TestLoadEmbeddedManifest(t *testing.T) { + m, err := LoadEmbeddedManifest() + if err != nil { + t.Fatalf("LoadEmbeddedManifest: %v", err) + } + if m.SchemaVersion != 1 { + t.Fatalf("SchemaVersion = %d, want 1", m.SchemaVersion) + } + if m.RunnerUser == "" || m.WorkDir == "" { + t.Fatalf("expected runner_user and work_dir to be set, got %+v", m) + } + + want := []string{"git", "jq", "yq", "gh", "make", "zip", "unzip", "curl", + "docker", "build-essential", "pkg-config", "libssl-dev", + "libnspr4", "libnss3", "fonts-liberation"} + byName := map[string]bool{} + for _, d := range m.Dependencies { + byName[d.Name] = true + } + for _, name := range want { + if !byName[name] { + t.Errorf("manifest missing required dependency %q", name) + } + } + if len(m.ChromiumCacheGlobs) == 0 { + t.Error("expected at least one chromium_cache_globs entry") + } +} + +func TestParseManifest_RejectsBadSchemaVersion(t *testing.T) { + _, err := ParseManifest([]byte("schema_version: 2\ndependencies:\n - name: x\n apt_package: x\n")) + if err == nil { + t.Fatal("expected error for unsupported schema_version") + } +} + +func TestParseManifest_RejectsEmptyDependencies(t *testing.T) { + _, err := ParseManifest([]byte("schema_version: 1\ndependencies: []\n")) + if err == nil { + t.Fatal("expected error for zero dependencies") + } +} + +func TestManifest_AptPackages(t *testing.T) { + m := &Manifest{Dependencies: []Dependency{ + {Name: "git", AptPackage: "git"}, + {Name: "libssl-dev", AptPackage: "libssl-dev"}, + }} + got := m.AptPackages() + if len(got) != 2 || got[0] != "git" || got[1] != "libssl-dev" { + t.Fatalf("AptPackages() = %v", got) + } +} + +func TestDependency_HasBinary(t *testing.T) { + if (Dependency{Binary: ""}).HasBinary() { + t.Error("empty binary should report HasBinary() == false") + } + if !(Dependency{Binary: "git"}).HasBinary() { + t.Error("non-empty binary should report HasBinary() == true") + } +} diff --git a/internal/runner/provision.go b/internal/runner/provision.go new file mode 100644 index 00000000..4b2c6a44 --- /dev/null +++ b/internal/runner/provision.go @@ -0,0 +1,196 @@ +package runner + +// Purpose: build a self-hosted GitHub Actions CI runner host from the same +// declarative Manifest verify.go audits against, so a freshly provisioned +// host and a "should be identical" existing host are checked with +// literally the same dependency list — no separate shell script that can +// drift from what verify expects. +// Inputs: a Manifest, an Executor bound to the target host, and +// ProvisionOptions (instance count, GitHub registration details). +// Outputs: a ProvisionResult recording each step's output, for the caller +// to print or log; a non-nil error stops at the first failing step. +// Constraints: every step is idempotent — safe to re-run provision against +// a host that's already partially set up — and every side-effecting +// command runs through Executor so tests assert exact command strings +// against a fake, never a real host. + +import ( + "context" + "fmt" + "strconv" + "strings" +) + +// ProvisionOptions parameterizes one provisioning run. +type ProvisionOptions struct { + // Instances is how many runner instances to register on this host. + // Defaults to 1. + Instances int + // InstallRoot is the base directory each instance installs into, as + // InstallRoot/runner-. Defaults to /opt/actions-runner. + InstallRoot string + // GithubURL is the repo or org URL runners register against, e.g. + // https://github.com/nself-org/cli. + GithubURL string + // RegToken is a GitHub Actions runner registration token. Sourced from + // an env var by the caller (cmd/commands/runner_provision.go) — never + // hardcoded, never included in ProvisionResult. + RegToken string + // Labels are extra labels appended after the standard + // self-hosted,Linux,X64 set. + Labels []string +} + +// ProvisionStep records one step's name and captured output. +type ProvisionStep struct { + Name string + Output string +} + +// ProvisionResult is the full record of a Provision run. +type ProvisionResult struct { + Steps []ProvisionStep +} + +// Provision installs the manifest's dependency set, creates the runner +// user with passwordless sudo, ensures the work directory is a real +// directory (never a symlink — see verify.go's checkWorkDirNotSymlink for +// why that matters), and installs opts.Instances runner instances as +// systemd services. +func Provision(ctx context.Context, ex Executor, m *Manifest, opts ProvisionOptions) (*ProvisionResult, error) { + if opts.Instances < 1 { + opts.Instances = 1 + } + if opts.InstallRoot == "" { + opts.InstallRoot = "/opt/actions-runner" + } + + result := &ProvisionResult{} + baseSteps := []struct { + name string + run func() (string, error) + }{ + {"ensure-gh-apt-repo", func() (string, error) { return ensureGHAptRepo(ctx, ex) }}, + {"install-packages", func() (string, error) { return ensurePackages(ctx, ex, m) }}, + {"create-runner-user", func() (string, error) { return ensureRunnerUser(ctx, ex, m.RunnerUser) }}, + {"grant-passwordless-sudo", func() (string, error) { return ensurePasswordlessSudo(ctx, ex, m.RunnerUser) }}, + {"ensure-work-dir", func() (string, error) { return ensureRealWorkDir(ctx, ex, m.WorkDir) }}, + } + for _, s := range baseSteps { + out, err := s.run() + result.Steps = append(result.Steps, ProvisionStep{Name: s.name, Output: out}) + if err != nil { + return result, fmt.Errorf("runner provision: step %q: %w", s.name, err) + } + } + + for i := 1; i <= opts.Instances; i++ { + out, err := installRunnerInstance(ctx, ex, m, opts, i) + stepName := "install-runner-" + strconv.Itoa(i) + result.Steps = append(result.Steps, ProvisionStep{Name: stepName, Output: out}) + if err != nil { + return result, fmt.Errorf("runner provision: instance %d: %w", i, err) + } + } + return result, nil +} + +// ensureGHAptRepo adds GitHub CLI's official apt repository only when `gh` +// isn't already resolvable — Ubuntu's own archive doesn't reliably carry a +// current `gh` package, which is exactly how one runner host had it and +// another didn't while advertising identical labels (2026-09-11 incident). +func ensureGHAptRepo(ctx context.Context, ex Executor) (string, error) { + script := `if command -v gh >/dev/null 2>&1; then echo "gh already present"; else ` + + `sudo mkdir -p -m 755 /etc/apt/keyrings && ` + + `curl -fsSL https://cli.github.com/packages/githubcli-archive-keyring.gpg | sudo tee /etc/apt/keyrings/githubcli-archive-keyring.gpg >/dev/null && ` + + `sudo chmod go+r /etc/apt/keyrings/githubcli-archive-keyring.gpg && ` + + `echo "deb [arch=$(dpkg --print-architecture) signed-by=/etc/apt/keyrings/githubcli-archive-keyring.gpg] https://cli.github.com/packages stable main" | sudo tee /etc/apt/sources.list.d/github-cli.list >/dev/null && ` + + `echo "gh apt repo installed"; fi` + return ex.Run(ctx, script) +} + +// ensurePackages installs every manifest dependency in one apt-get call. +// apt-get install is itself idempotent for already-installed packages, so +// no separate pre-check is needed here (unlike the sudoers/user/work-dir +// steps, which write host state directly). +func ensurePackages(ctx context.Context, ex Executor, m *Manifest) (string, error) { + quoted := make([]string, len(m.Dependencies)) + for i, d := range m.Dependencies { + quoted[i] = shellQuote(d.AptPackage) + } + script := "sudo DEBIAN_FRONTEND=noninteractive apt-get update -qq && " + + "sudo DEBIAN_FRONTEND=noninteractive apt-get install -y --no-install-recommends " + + strings.Join(quoted, " ") + return ex.Run(ctx, script) +} + +// ensureRunnerUser creates the runner service account and adds it to the +// docker group, but only if it doesn't already exist. +func ensureRunnerUser(ctx context.Context, ex Executor, user string) (string, error) { + q := shellQuote(user) + script := "if id -u " + q + " >/dev/null 2>&1; then echo 'user already exists'; " + + "else sudo useradd -m -s /bin/bash " + q + " && sudo usermod -aG docker " + q + + " && echo 'runner user created'; fi" + return ex.Run(ctx, script) +} + +// ensurePasswordlessSudo grants the runner user NOPASSWD sudo via a +// dedicated /etc/sudoers.d drop-in, validated with visudo -cf before it +// takes effect. This is broad (ALL commands) because the evidence for this +// gap was `playwright install --with-deps` needing an interactive sudo +// prompt with no terminal available — CI steps in general assume +// passwordless sudo on a self-hosted runner. Idempotent: only writes when +// the file is missing or its content differs from the expected line. +func ensurePasswordlessSudo(ctx context.Context, ex Executor, user string) (string, error) { + sudoersFile := "/etc/sudoers.d/" + user + "-nself-runner" + line := user + " ALL=(ALL) NOPASSWD:ALL" + script := fmt.Sprintf( + "if [ -f %[1]s ] && sudo grep -qxF %[2]s %[1]s; then echo 'passwordless sudo already configured'; "+ + "else echo %[2]s | sudo tee %[1]s >/dev/null && sudo chmod 0440 %[1]s && sudo visudo -cf %[1]s "+ + "&& echo 'passwordless sudo granted'; fi", + shellQuote(sudoersFile), shellQuote(line)) + return ex.Run(ctx, script) +} + +// ensureRealWorkDir creates the shared work directory, refusing outright +// if the path already exists as a symlink rather than silently replacing +// it — a symlink there may be pointing at real data, and this is the exact +// misconfiguration G-012 exists to prevent (see verify.go's +// checkWorkDirNotSymlink for the full explanation). +func ensureRealWorkDir(ctx context.Context, ex Executor, workDir string) (string, error) { + if workDir == "" { + return "no work_dir configured; skipped", nil + } + q := shellQuote(workDir) + script := "if [ -L " + q + " ]; then echo 'REFUSING: " + workDir + + " is a symlink - replace it with a real directory or bind mount first (G-012)' 1>&2; exit 1; " + + "else sudo mkdir -p " + q + " && echo '" + workDir + " ready (real directory)'; fi" + return ex.Run(ctx, script) +} + +// installRunnerInstance downloads (if not already present), configures, +// and starts one GitHub Actions runner instance as a systemd service via +// the runner's own svc.sh, which is the supported way to register a +// self-hosted runner as a systemd unit — reimplementing that unit file +// would just have to track GitHub's own runner releases anyway. +func installRunnerInstance(ctx context.Context, ex Executor, m *Manifest, opts ProvisionOptions, idx int) (string, error) { + instanceDir := opts.InstallRoot + "/runner-" + strconv.Itoa(idx) + name := ex.Label() + "-" + strconv.Itoa(idx) + labels := strings.Join(append([]string{"self-hosted", "Linux", "X64"}, opts.Labels...), ",") + + script := fmt.Sprintf(`set -e +sudo mkdir -p %[1]s && sudo chown %[2]s:%[2]s %[1]s +cd %[1]s +if [ ! -f config.sh ]; then + ARCH=$(uname -m); case "$ARCH" in x86_64) RARCH=x64 ;; aarch64) RARCH=arm64 ;; *) RARCH=x64 ;; esac + VER=$(curl -fsSL https://api.github.com/repos/actions/runner/releases/latest | grep tag_name | cut -d '"' -f4 | tr -d v) + curl -fsSL -o actions-runner.tar.gz "https://github.com/actions/runner/releases/download/v${VER}/actions-runner-linux-${RARCH}-${VER}.tar.gz" + tar xzf actions-runner.tar.gz +fi +sudo -u %[2]s ./config.sh --url %[3]s --token %[4]s --name %[5]s --labels %[6]s --work _work --unattended --replace +sudo ./svc.sh install %[2]s +sudo ./svc.sh start +`, shellQuote(instanceDir), shellQuote(m.RunnerUser), shellQuote(opts.GithubURL), + shellQuote(opts.RegToken), shellQuote(name), shellQuote(labels)) + return ex.Run(ctx, script) +} diff --git a/internal/runner/provision_test.go b/internal/runner/provision_test.go new file mode 100644 index 00000000..c928515f --- /dev/null +++ b/internal/runner/provision_test.go @@ -0,0 +1,123 @@ +package runner + +import ( + "context" + "errors" + "strings" + "testing" +) + +func TestProvision_RunsAllStepsInOrder(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "ok", err: nil}} + m := testManifest() + + result, err := Provision(context.Background(), ex, m, ProvisionOptions{ + Instances: 1, + GithubURL: "https://github.com/nself-org/cli", + RegToken: "AREGTOKEN", + }) + if err != nil { + t.Fatalf("Provision returned error: %v", err) + } + wantSteps := []string{ + "ensure-gh-apt-repo", "install-packages", "create-runner-user", + "grant-passwordless-sudo", "ensure-work-dir", "install-runner-1", + } + if len(result.Steps) != len(wantSteps) { + t.Fatalf("got %d steps, want %d: %+v", len(result.Steps), len(wantSteps), result.Steps) + } + for i, name := range wantSteps { + if result.Steps[i].Name != name { + t.Errorf("step %d = %q, want %q", i, result.Steps[i].Name, name) + } + } +} + +func TestProvision_MultipleInstances(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "ok", err: nil}} + result, err := Provision(context.Background(), ex, testManifest(), ProvisionOptions{Instances: 3}) + if err != nil { + t.Fatalf("Provision: %v", err) + } + instanceSteps := 0 + for _, s := range result.Steps { + if strings.HasPrefix(s.Name, "install-runner-") { + instanceSteps++ + } + } + if instanceSteps != 3 { + t.Fatalf("instance steps = %d, want 3", instanceSteps) + } +} + +func TestProvision_StopsOnFirstFailure(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "ok", err: nil}} + ex.when("apt-get install", "", errors.New("apt-get exit 100")) + + result, err := Provision(context.Background(), ex, testManifest(), ProvisionOptions{Instances: 2}) + if err == nil { + t.Fatal("expected an error when install-packages fails") + } + if !strings.Contains(err.Error(), "install-packages") { + t.Fatalf("error should name the failing step, got: %v", err) + } + for _, s := range result.Steps { + if strings.HasPrefix(s.Name, "install-runner-") { + t.Fatalf("no runner instance should be installed after an earlier step failed, got %q", s.Name) + } + } +} + +func TestEnsureRealWorkDir_RefusesSymlink(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "", err: errors.New("exit status 1: REFUSING: symlink")}} + _, err := ensureRealWorkDir(context.Background(), ex, "/opt/actions-runner/_work") + if err == nil { + t.Fatal("expected ensureRealWorkDir to surface the refusal error") + } +} + +func TestEnsureRealWorkDir_Unconfigured(t *testing.T) { + out, err := ensureRealWorkDir(context.Background(), &fakeExecutor{}, "") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if !strings.Contains(out, "skipped") { + t.Fatalf("out = %q", out) + } +} + +func TestEnsurePasswordlessSudo_CommandShape(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "passwordless sudo granted", err: nil}} + _, err := ensurePasswordlessSudo(context.Background(), ex, "gha-runner") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if len(ex.commands) != 1 { + t.Fatalf("expected exactly one command, got %d", len(ex.commands)) + } + cmd := ex.commands[0] + for _, want := range []string{"/etc/sudoers.d/gha-runner-nself-runner", "NOPASSWD:ALL", "visudo -cf"} { + if !strings.Contains(cmd, want) { + t.Errorf("sudoers command missing %q:\n%s", want, cmd) + } + } +} + +func TestInstallRunnerInstance_LabelsIncludeSelfHostedDefaults(t *testing.T) { + ex := &fakeExecutor{label: "runner-a", fallback: fakeResponse{out: "ok", err: nil}} + _, err := installRunnerInstance(context.Background(), ex, testManifest(), ProvisionOptions{ + InstallRoot: "/opt/actions-runner", + GithubURL: "https://github.com/nself-org/cli", + RegToken: "TOKEN", + Labels: []string{"nself-ci"}, + }, 1) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + cmd := ex.commands[0] + for _, want := range []string{"self-hosted,Linux,X64,nself-ci", "runner-a-1", "runner-1", "--unattended"} { + if !strings.Contains(cmd, want) { + t.Errorf("instance command missing %q:\n%s", want, cmd) + } + } +} diff --git a/internal/runner/report.go b/internal/runner/report.go new file mode 100644 index 00000000..db95fac8 --- /dev/null +++ b/internal/runner/report.go @@ -0,0 +1,193 @@ +package runner + +// Purpose: run verify across N hosts and turn the per-host CheckResult sets +// into a single parity matrix, so "same GitHub Actions runner labels, +// different tools installed" (the exact 2026-09-11 gh/zip/unzip +// incident) shows up as one glance at a table instead of N separate +// verify runs a human has to compare by eye. +// Inputs: a Manifest and one Executor per host to check. +// Outputs: []HostReport (one per host) plus DetectDrift's []DriftFinding +// and RenderMatrix's plain-text table. +// Constraints: an unreachable host is reported once (HostReport.Err) and +// excluded from drift comparison — comparing "unreachable" against every +// real status would report every check as drifted, burying the actual +// finding. + +import ( + "context" + "fmt" + "sort" + "strings" +) + +// HostReport is one host's full verify result. +type HostReport struct { + Host string `json:"host"` + Checks []CheckResult `json:"checks,omitempty"` + // Err is set when the host itself could not be reached at all (SSH + // connection failure), as opposed to an individual check failing. + Err string `json:"error,omitempty"` +} + +// VerifyHosts runs a cheap reachability probe against each executor, then +// the full VerifyHost check set against every host that answered. Hosts +// that don't answer get a single HostReport with Err set and no Checks, +// rather than N near-identical "connection refused" failures. +func VerifyHosts(ctx context.Context, executors []Executor, m *Manifest) []HostReport { + reports := make([]HostReport, len(executors)) + for i, ex := range executors { + if _, err := ex.Run(ctx, "echo reachable"); err != nil { + reports[i] = HostReport{Host: ex.Label(), Err: err.Error()} + continue + } + reports[i] = HostReport{Host: ex.Label(), Checks: VerifyHost(ctx, ex, m)} + } + return reports +} + +// DriftFinding names one check that disagrees across two or more reachable +// hosts — the thing `nself runner verify` exists to surface. +type DriftFinding struct { + CheckName string `json:"check"` + ByHost map[string]CheckStatus `json:"by_host"` +} + +// DetectDrift compares every check name present on any reachable host and +// reports the ones whose status differs across hosts. Manifest checks are +// identical across hosts by construction, so any disagreement here is real +// drift, not a different question being asked. +func DetectDrift(reports []HostReport) []DriftFinding { + byCheck := map[string]map[string]CheckStatus{} + var order []string + for _, r := range reports { + if r.Err != "" { + continue + } + for _, c := range r.Checks { + if _, ok := byCheck[c.Name]; !ok { + byCheck[c.Name] = map[string]CheckStatus{} + order = append(order, c.Name) + } + byCheck[c.Name][r.Host] = c.Status + } + } + var findings []DriftFinding + for _, name := range order { + statuses := byCheck[name] + if !allEqual(statuses) { + findings = append(findings, DriftFinding{CheckName: name, ByHost: statuses}) + } + } + return findings +} + +func allEqual(statuses map[string]CheckStatus) bool { + first := "" + for _, st := range statuses { + if first == "" { + first = string(st) + continue + } + if string(st) != first { + return false + } + } + return true +} + +// RenderMatrix formats reports as a plain-text parity matrix: one row per +// check, one column per host, plus a trailing drift summary. This is the +// primary human-facing output of `nself runner verify` when checking more +// than one host. +func RenderMatrix(reports []HostReport) string { + var b strings.Builder + hosts := make([]string, len(reports)) + for i, r := range reports { + hosts[i] = r.Host + } + + checkNames := collectCheckNames(reports) + statusOf := indexStatuses(reports) + + fmt.Fprintf(&b, "%-40s", "CHECK") + for _, h := range hosts { + fmt.Fprintf(&b, " %-12s", truncate(h, 12)) + } + b.WriteString("\n") + + for _, name := range checkNames { + fmt.Fprintf(&b, "%-40s", truncate(name, 40)) + for _, h := range hosts { + b.WriteString(" ") + b.WriteString(cellSymbol(statusOf, name, h)) + b.WriteString(strings.Repeat(" ", 10)) + } + b.WriteString("\n") + } + + drift := DetectDrift(reports) + if len(drift) == 0 { + b.WriteString("\nNo drift detected across reachable hosts.\n") + } else { + fmt.Fprintf(&b, "\nDRIFT DETECTED (%d check(s) disagree across hosts):\n", len(drift)) + for _, f := range drift { + fmt.Fprintf(&b, " - %s:\n", f.CheckName) + for _, h := range hosts { + if st, ok := f.ByHost[h]; ok { + fmt.Fprintf(&b, " %-20s %s\n", h, st) + } + } + } + } + return b.String() +} + +func collectCheckNames(reports []HostReport) []string { + set := map[string]bool{} + for _, r := range reports { + for _, c := range r.Checks { + set[c.Name] = true + } + } + names := make([]string, 0, len(set)) + for n := range set { + names = append(names, n) + } + sort.Strings(names) + return names +} + +func indexStatuses(reports []HostReport) map[string]map[string]CheckStatus { + idx := map[string]map[string]CheckStatus{} + for _, r := range reports { + for _, c := range r.Checks { + if _, ok := idx[c.Name]; !ok { + idx[c.Name] = map[string]CheckStatus{} + } + idx[c.Name][r.Host] = c.Status + } + } + return idx +} + +func cellSymbol(idx map[string]map[string]CheckStatus, checkName, host string) string { + st, ok := idx[checkName][host] + if !ok { + return "?" + } + switch st { + case StatusPass: + return "OK" + case StatusWarn: + return "warn" + default: + return "FAIL" + } +} + +func truncate(s string, n int) string { + if len(s) <= n { + return s + } + return s[:n-1] + "…" +} diff --git a/internal/runner/report_test.go b/internal/runner/report_test.go new file mode 100644 index 00000000..67d4498a --- /dev/null +++ b/internal/runner/report_test.go @@ -0,0 +1,116 @@ +package runner + +import ( + "context" + "errors" + "strings" + "testing" +) + +// TestVerifyHosts_TwoHostsIdentical_NoDrift models the healthy fleet case: +// both hosts pass every check. +func TestVerifyHosts_TwoHostsIdentical_NoDrift(t *testing.T) { + m := testManifest() + host1 := &fakeExecutor{label: "runner-1"} + host1.when("echo reachable", "reachable", nil) + host1.when("command -v 'git'", "/usr/bin/git", nil) + host1.when("dpkg -s 'libssl-dev'", "installed", nil) + host1.when("if [ -L", "DIR", nil) + + host2 := &fakeExecutor{label: "runner-2"} + host2.when("echo reachable", "reachable", nil) + host2.when("command -v 'git'", "/usr/bin/git", nil) + host2.when("dpkg -s 'libssl-dev'", "installed", nil) + host2.when("if [ -L", "DIR", nil) + + reports := VerifyHosts(context.Background(), []Executor{host1, host2}, m) + if len(reports) != 2 { + t.Fatalf("len(reports) = %d, want 2", len(reports)) + } + drift := DetectDrift(reports) + if len(drift) != 0 { + t.Fatalf("expected no drift, got %+v", drift) + } +} + +// TestVerifyHosts_DetectsLabelDrift is the exact scenario from the G-012 +// evidence: two hosts advertise the same GitHub Actions labels but one is +// missing `gh` — verify must surface that as drift, not silently pass. +func TestVerifyHosts_DetectsLabelDrift(t *testing.T) { + m := &Manifest{Dependencies: []Dependency{{Name: "gh", AptPackage: "gh", Binary: "gh"}}} + + hostWithGH := &fakeExecutor{label: "runner-a"} + hostWithGH.when("echo reachable", "reachable", nil) + hostWithGH.when("command -v 'gh'", "/usr/bin/gh", nil) + + hostWithoutGH := &fakeExecutor{label: "runner-b"} + hostWithoutGH.when("echo reachable", "reachable", nil) + hostWithoutGH.fallback = fakeResponse{out: "", err: errors.New("exit status 1")} + + reports := VerifyHosts(context.Background(), []Executor{hostWithGH, hostWithoutGH}, m) + drift := DetectDrift(reports) + if len(drift) != 1 { + t.Fatalf("expected exactly one drift finding, got %+v", drift) + } + if drift[0].CheckName != "dep:gh" { + t.Fatalf("drift on wrong check: %+v", drift[0]) + } + if drift[0].ByHost["runner-a"] != StatusPass || drift[0].ByHost["runner-b"] != StatusFail { + t.Fatalf("drift byHost = %+v", drift[0].ByHost) + } + + matrix := RenderMatrix(reports) + if !strings.Contains(matrix, "DRIFT DETECTED") { + t.Fatalf("matrix should report drift:\n%s", matrix) + } + if !strings.Contains(matrix, "dep:gh") { + t.Fatalf("matrix should name the drifted check:\n%s", matrix) + } +} + +func TestVerifyHosts_UnreachableHostExcludedFromDrift(t *testing.T) { + m := &Manifest{Dependencies: []Dependency{{Name: "gh", AptPackage: "gh", Binary: "gh"}}} + + reachable := &fakeExecutor{label: "runner-a"} + reachable.when("echo reachable", "reachable", nil) + reachable.when("command -v 'gh'", "/usr/bin/gh", nil) + + unreachable := &fakeExecutor{label: "runner-down", fallback: fakeResponse{err: errors.New("dial tcp: connection refused")}} + + reports := VerifyHosts(context.Background(), []Executor{reachable, unreachable}, m) + var downReport *HostReport + for i := range reports { + if reports[i].Host == "runner-down" { + downReport = &reports[i] + } + } + if downReport == nil || downReport.Err == "" { + t.Fatalf("expected runner-down to carry a non-empty Err, got %+v", downReport) + } + if len(downReport.Checks) != 0 { + t.Fatalf("unreachable host should have zero Checks, got %d", len(downReport.Checks)) + } + if drift := DetectDrift(reports); len(drift) != 0 { + t.Fatalf("unreachable host must not be reported as drift, got %+v", drift) + } +} + +func TestRenderMatrix_NoDrift(t *testing.T) { + reports := []HostReport{ + {Host: "a", Checks: []CheckResult{{Name: "dep:git", Status: StatusPass}}}, + {Host: "b", Checks: []CheckResult{{Name: "dep:git", Status: StatusPass}}}, + } + out := RenderMatrix(reports) + if !strings.Contains(out, "No drift detected") { + t.Fatalf("expected no-drift message:\n%s", out) + } +} + +func TestAllEqual(t *testing.T) { + if !allEqual(map[string]CheckStatus{"a": StatusPass, "b": StatusPass}) { + t.Error("expected equal statuses to report true") + } + if allEqual(map[string]CheckStatus{"a": StatusPass, "b": StatusFail}) { + t.Error("expected differing statuses to report false") + } +} diff --git a/internal/runner/verify.go b/internal/runner/verify.go new file mode 100644 index 00000000..1fc9231f --- /dev/null +++ b/internal/runner/verify.go @@ -0,0 +1,121 @@ +package runner + +// Purpose: audit one runner host against the declarative Manifest and +// report exactly what's missing or drifted, so "same GitHub Actions +// labels, different tools installed" is caught by `nself runner verify` +// before it causes a mystery job failure, not after. +// Inputs: a Manifest and an Executor bound to the host under test. +// Outputs: []CheckResult, one per manifest dependency plus the work-dir +// symlink check and the chromium ldd check (chromium.go). +// Constraints: every check is one Executor.Run call with a deterministic, +// easily-mocked command string — see verify_test.go for the fake +// Executor these are asserted against. + +import ( + "context" + "fmt" + "strings" +) + +// CheckStatus is the outcome of a single verify check. +type CheckStatus string + +const ( + // StatusPass means the check found what it expected. + StatusPass CheckStatus = "pass" + // StatusFail means the check found the dependency/condition missing or + // broken — this is what should block a job from being trusted on this + // host. + StatusFail CheckStatus = "fail" + // StatusWarn is informational: the check couldn't run to completion in + // a meaningful way (e.g. no cached Chromium yet) but that's not itself + // evidence of drift. + StatusWarn CheckStatus = "warn" +) + +// CheckResult is one named check's outcome on one host. +type CheckResult struct { + Name string `json:"name"` + Status CheckStatus `json:"status"` + Detail string `json:"detail,omitempty"` +} + +// VerifyHost runs every manifest-declared check against one host and +// returns the full result set. It never returns an error itself — a host +// that is entirely unreachable should be detected by the caller with a +// cheap reachability probe first (see VerifyHosts in report.go); if it +// isn't, every check below simply reports StatusFail with the underlying +// exec error as Detail, which is still an honest (if noisier) answer. +func VerifyHost(ctx context.Context, ex Executor, m *Manifest) []CheckResult { + results := make([]CheckResult, 0, len(m.Dependencies)+2) + for _, d := range m.Dependencies { + results = append(results, checkDependency(ctx, ex, d)) + } + results = append(results, checkWorkDirNotSymlink(ctx, ex, m.WorkDir)) + results = append(results, checkChromiumLdd(ctx, ex, m)...) + return results +} + +// checkDependency runs the binary-on-PATH check when the dependency +// declares one, otherwise falls back to a package-manager query — see +// Dependency.HasBinary's doc comment in manifest.go for why. +func checkDependency(ctx context.Context, ex Executor, d Dependency) CheckResult { + name := "dep:" + d.Name + if d.HasBinary() { + out, err := ex.Run(ctx, "command -v "+shellQuote(d.Binary)) + if err != nil || out == "" { + return CheckResult{Name: name, Status: StatusFail, + Detail: fmt.Sprintf("binary %q not on PATH — %s", d.Binary, d.Reason)} + } + return CheckResult{Name: name, Status: StatusPass, Detail: out} + } + out, err := ex.Run(ctx, "dpkg -s "+shellQuote(d.AptPackage)+ + " >/dev/null 2>&1 && echo installed || echo missing") + if err != nil || !strings.Contains(out, "installed") { + return CheckResult{Name: name, Status: StatusFail, + Detail: fmt.Sprintf("apt package %q not installed — %s", d.AptPackage, d.Reason)} + } + return CheckResult{Name: name, Status: StatusPass} +} + +// checkWorkDirNotSymlink is the highest-severity check in this package. +// +// Git resolves symlinks before matching `includeIf.gitdir:`, so a runner +// whose `_work` directory is a symlink (e.g. onto a bind-mounted data +// volume set up by hand) silently defeats actions/checkout's credential +// injection: the job fails with "fatal: could not read Username for +// 'https://github.com'" and nothing in that error points at the real +// cause. `_work` must always be a real directory or an actual bind mount +// (which `stat`/`readlink` both report as a plain directory, never a +// symlink) — never `ln -s`. +func checkWorkDirNotSymlink(ctx context.Context, ex Executor, workDir string) CheckResult { + const name = "workdir:not-symlink" + if workDir == "" { + return CheckResult{Name: name, Status: StatusWarn, Detail: "no work_dir configured in manifest"} + } + q := shellQuote(workDir) + cmd := "if [ -L " + q + " ]; then echo SYMLINK; elif [ -d " + q + " ]; then echo DIR; else echo ABSENT; fi" + out, err := ex.Run(ctx, cmd) + if err != nil { + return CheckResult{Name: name, Status: StatusFail, Detail: "could not stat " + workDir + ": " + err.Error()} + } + switch strings.TrimSpace(out) { + case "SYMLINK": + return CheckResult{Name: name, Status: StatusFail, Detail: workDir + + " is a symlink — git resolves symlinks before matching includeIf.gitdir, " + + "which silently breaks actions/checkout credential injection. Replace it " + + "with a real directory or a bind mount."} + case "DIR": + return CheckResult{Name: name, Status: StatusPass, Detail: workDir + " is a real directory"} + default: + return CheckResult{Name: name, Status: StatusWarn, Detail: workDir + " does not exist yet"} + } +} + +// shellQuote wraps s in single quotes for safe inclusion in a shell +// command, escaping embedded single quotes POSIX-style. Every value quoted +// with this in the runner package comes from the compiled-in manifest or a +// CLI flag, never from webhook/repo-controlled input. +func shellQuote(s string) string { + return "'" + strings.ReplaceAll(s, "'", `'\''`) + "'" +} diff --git a/internal/runner/verify_test.go b/internal/runner/verify_test.go new file mode 100644 index 00000000..7cd64fac --- /dev/null +++ b/internal/runner/verify_test.go @@ -0,0 +1,119 @@ +package runner + +import ( + "context" + "errors" + "testing" +) + +func testManifest() *Manifest { + return &Manifest{ + SchemaVersion: 1, + RunnerUser: "gha-runner", + WorkDir: "/opt/actions-runner/_work", + Dependencies: []Dependency{ + {Name: "git", AptPackage: "git", Binary: "git", Reason: "checkout needs git"}, + {Name: "libssl-dev", AptPackage: "libssl-dev", Binary: "", Reason: "tls builds need headers"}, + }, + ChromiumCacheGlobs: []string{"%h/.cache/ms-playwright/chromium-*/chrome-linux/chrome"}, + } +} + +func TestCheckDependency_BinaryPresent(t *testing.T) { + ex := &fakeExecutor{} + ex.when("command -v 'git'", "/usr/bin/git", nil) + r := checkDependency(context.Background(), ex, Dependency{Name: "git", Binary: "git"}) + if r.Status != StatusPass { + t.Fatalf("status = %v, want pass; detail=%s", r.Status, r.Detail) + } +} + +func TestCheckDependency_BinaryMissing(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "", err: errors.New("exit status 1")}} + r := checkDependency(context.Background(), ex, Dependency{Name: "gh", Binary: "gh", Reason: "commit status posting"}) + if r.Status != StatusFail { + t.Fatalf("status = %v, want fail", r.Status) + } + if r.Name != "dep:gh" { + t.Fatalf("Name = %q", r.Name) + } +} + +func TestCheckDependency_AptPackageInstalled(t *testing.T) { + ex := &fakeExecutor{} + ex.when("dpkg -s 'libssl-dev'", "installed", nil) + r := checkDependency(context.Background(), ex, Dependency{Name: "libssl-dev", AptPackage: "libssl-dev"}) + if r.Status != StatusPass { + t.Fatalf("status = %v, want pass", r.Status) + } +} + +func TestCheckDependency_AptPackageMissing(t *testing.T) { + ex := &fakeExecutor{} + ex.when("dpkg -s 'libnspr4'", "missing", nil) + r := checkDependency(context.Background(), ex, Dependency{Name: "libnspr4", AptPackage: "libnspr4", Reason: "chromium runtime"}) + if r.Status != StatusFail { + t.Fatalf("status = %v, want fail; detail=%s", r.Status, r.Detail) + } +} + +func TestCheckWorkDirNotSymlink_Symlink(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "SYMLINK", err: nil}} + r := checkWorkDirNotSymlink(context.Background(), ex, "/opt/actions-runner/_work") + if r.Status != StatusFail { + t.Fatalf("status = %v, want fail for symlinked work dir", r.Status) + } + if r.Name != "workdir:not-symlink" { + t.Fatalf("Name = %q", r.Name) + } +} + +func TestCheckWorkDirNotSymlink_RealDir(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "DIR", err: nil}} + r := checkWorkDirNotSymlink(context.Background(), ex, "/opt/actions-runner/_work") + if r.Status != StatusPass { + t.Fatalf("status = %v, want pass for real directory", r.Status) + } +} + +func TestCheckWorkDirNotSymlink_Absent(t *testing.T) { + ex := &fakeExecutor{fallback: fakeResponse{out: "ABSENT", err: nil}} + r := checkWorkDirNotSymlink(context.Background(), ex, "/opt/actions-runner/_work") + if r.Status != StatusWarn { + t.Fatalf("status = %v, want warn for not-yet-created dir", r.Status) + } +} + +func TestCheckWorkDirNotSymlink_Empty(t *testing.T) { + r := checkWorkDirNotSymlink(context.Background(), &fakeExecutor{}, "") + if r.Status != StatusWarn { + t.Fatalf("status = %v, want warn when unconfigured", r.Status) + } +} + +func TestVerifyHost_RunsAllChecks(t *testing.T) { + ex := &fakeExecutor{} + ex.when("command -v 'git'", "/usr/bin/git", nil) + ex.when("dpkg -s 'libssl-dev'", "installed", nil) + ex.when("if [ -L", "DIR", nil) + ex.when("shopt -s nullglob", "", nil) + + results := VerifyHost(context.Background(), ex, testManifest()) + // 2 dependencies + workdir + chromium-cache-warn = 4 + if len(results) != 4 { + t.Fatalf("len(results) = %d, want 4: %+v", len(results), results) + } + for _, r := range results { + if r.Status == StatusFail { + t.Errorf("unexpected fail: %+v", r) + } + } +} + +func TestShellQuote(t *testing.T) { + got := shellQuote(`it's`) + want := `'it'\''s'` + if got != want { + t.Fatalf("shellQuote = %q, want %q", got, want) + } +} From b5164f389792c48d44e1ddf77fc9fa4a770a49d1 Mon Sep 17 00:00:00 2001 From: Aric Camarata Date: Fri, 11 Sep 2026 17:58:56 -0400 Subject: [PATCH 2/2] fix(runner): use errs.Exit in verify, bump command budget, regenerate parity --- .github/command-surface-budget.txt | 10 +++++++++- .github/surface-parity.json | 9 +++++++++ .github/surface-parity.md | 3 ++- cmd/commands/runner_verify.go | 7 ++++++- 4 files changed, 26 insertions(+), 3 deletions(-) diff --git a/.github/command-surface-budget.txt b/.github/command-surface-budget.txt index 182ed055..5f5fe624 100644 --- a/.github/command-surface-budget.txt +++ b/.github/command-surface-budget.txt @@ -10,4 +10,12 @@ # teammate's SSH key on an already-deployed host had no CLI path at all. # `nself access grant/revoke/list` is the command surface named directly # in #238's proposed shape. -50 +# +# 2026-09-11: +1 for `runner` (gap G-012). Deliberate exception, not +# creep: onboarding a self-hosted CI runner host had no CLI path — the +# dependency manifest (gh/zip/unzip/etc.) was installed and verified by +# hand, which is exactly how one host silently drifted from another and +# hid a job failure behind a misleading browser error for hours on +# 2026-09-11. `nself runner provision/verify` is the surface G-012 names +# directly. +51 diff --git a/.github/surface-parity.json b/.github/surface-parity.json index 61c24a57..8c8e4ff7 100644 --- a/.github/surface-parity.json +++ b/.github/surface-parity.json @@ -325,6 +325,15 @@ "env_vars": "n/a", "openapi": "n/a (see below)" }, + { + "name": "runner", + "path": "nself runner", + "group_id": "advanced", + "wiki_page": true, + "mcp_tool": false, + "env_vars": "undocumented: GITHUB_RUNNER_TOKEN", + "openapi": "n/a (see below)" + }, { "name": "secrets", "path": "nself secrets", diff --git a/.github/surface-parity.md b/.github/surface-parity.md index f0b2acc8..9736ca0e 100644 --- a/.github/surface-parity.md +++ b/.github/surface-parity.md @@ -48,6 +48,7 @@ One row per top-level command (CLI-R17), scored against the four surfaces a comm | `nself remove` | extend | yes | no | n/a | n/a (see below) | | `nself reset` | core | yes | no | n/a | n/a (see below) | | `nself restart` | core | yes | yes | n/a | n/a (see below) | +| `nself runner` | advanced | yes | no | undocumented: GITHUB_RUNNER_TOKEN | n/a (see below) | | `nself secrets` | config | yes | no | undocumented: EDITOR | n/a (see below) | | `nself security` | advanced | yes | no | n/a | n/a (see below) | | `nself self-heal` | observe | yes | no | n/a | n/a (see below) | @@ -63,4 +64,4 @@ One row per top-level command (CLI-R17), scored against the four surfaces a comm | `nself verify-sbom` | advanced | yes | no | n/a | n/a (see below) | | `nself version` | account | yes | no | undocumented: BENCH_RESULTS_FILE | n/a (see below) | -Total: 50 commands. Missing wiki page: 0. No MCP tool: 33. Env vars found but undocumented: 16. +Total: 51 commands. Missing wiki page: 0. No MCP tool: 34. Env vars found but undocumented: 17. diff --git a/cmd/commands/runner_verify.go b/cmd/commands/runner_verify.go index 6556eed1..812a6c83 100644 --- a/cmd/commands/runner_verify.go +++ b/cmd/commands/runner_verify.go @@ -19,6 +19,7 @@ import ( "fmt" "os" + "github.com/nself-org/cli/internal/errs" "github.com/nself-org/cli/internal/runner" "github.com/spf13/cobra" ) @@ -76,7 +77,11 @@ func runRunnerVerify(cmd *cobra.Command, args []string) error { } if runnerVerifyFoundProblems(reports) { - os.Exit(1) + // Output already written above — errs.Exit is silent by design + // (internal/errs.ExitError.Silent), so main() prints nothing further. + // Never call os.Exit directly here: main() is the only os.Exit + // caller (internal/repoqa/os_exit_test.go enforces this). + return errs.Exit(1) } return nil }