diff --git a/docs/development.md b/docs/development.md index bfc036b931..89c2e1bd50 100644 --- a/docs/development.md +++ b/docs/development.md @@ -699,9 +699,11 @@ pages or credentials. ### Self-hosted runner -Linux CI/CD jobs run on GitHub-hosted `ubuntu-24.04` unless the repository variable -`CI_SELF_HOSTED_LABELS` holds a JSON array of runner labels that contains `"self-hosted"`, for -example `["self-hosted","linux","x64","herdr-ci"]`. Set, it routes: +`ci.yml`'s Linux jobs of a push or a same-repository pull request run on the maintainer PC's runners +(labels `["self-hosted","self-hosted-ovis"]`) by default; other events stay on `ubuntu-24.04`. The repository variable `CI_SELF_HOSTED_LABELS` +overrides that with another JSON array of runner labels that contains `"self-hosted"`, or with the +JSON string `"ubuntu-24.04"` to keep every job GitHub-hosted while that PC is offline. `pages.yml` +and `release.yml` stay on `ubuntu-24.04` unless the variable holds labels. The variable routes: - in `ci.yml`: Fast checks, Integration tests and both Browser regression jobs; - in `pages.yml`: the site build and the deploy after successful same-repository main push CI, @@ -712,7 +714,10 @@ Detect changes, the Integration and browser summary, the Windows and macOS jobs, `remote-bundles.yml` and `windows-install.yml` always stay GitHub-hosted. In `ci.yml`, Detect changes picks the runner with `scripts/ci-runner-route.sh`. A fork pull request -always gets `ubuntu-24.04`, whatever the variable says. Otherwise the `self_hosted_labels` +always gets `ubuntu-24.04`, whatever the variable says. Since that script runs from the pull +request's own revision, the runner also refuses fork code itself: its job-started hook kills the +job's worker before any step, `if: always()` ones included, can run +(`scripts/self-hosted-runner/job-started.sh`). Otherwise the `self_hosted_labels` workflow_dispatch input, when not empty, overrides the variable for that one manual run. A value that is not a non-empty array of strings containing `"self-hosted"` fails the job instead of falling back. `pages.yml` and `release.yml` read the variable directly. diff --git a/scripts/ci-runner-route.sh b/scripts/ci-runner-route.sh index 33d5bcfadb..6cd66896b2 100755 --- a/scripts/ci-runner-route.sh +++ b/scripts/ci-runner-route.sh @@ -2,10 +2,16 @@ # Picks the runner for ci.yml's Linux jobs and prints `linux=` for $GITHUB_OUTPUT. # A fork pull request always gets the GitHub-hosted image: its code must never reach the # self-hosted runner. Otherwise the workflow_dispatch input wins over the repository variable -# CI_SELF_HOSTED_LABELS, and with neither the job stays on ubuntu-24.04. +# CI_SELF_HOSTED_LABELS. With neither, a push or a same-repository pull request goes to the +# maintainer PC's runners (scripts/self-hosted-runner/, label self-hosted-ovis) and any other event +# stays on ubuntu-24.04. The variable value "ubuntu-24.04" (a JSON string, which pages.yml and +# release.yml also accept) keeps every job hosted while that PC is offline. This script runs from the +# checked-out revision, which a fork pull request controls; the runner itself refuses fork code +# (scripts/self-hosted-runner/job-started.sh). set -euo pipefail hosted='"ubuntu-24.04"' +default_labels='["self-hosted","self-hosted-ovis"]' event="${EVENT_NAME:-}" requested="${REQUESTED_LABELS:-}" configured="${CONFIGURED_LABELS:-}" @@ -25,9 +31,14 @@ if [[ ("$event" == "pull_request" || "$event" == "pull_request_target") && "${HE echo "linux=$hosted" elif [[ -n "${requested//[[:space:]]/}" ]]; then choose_labels "workflow_dispatch input self_hosted_labels" "$requested" +elif [[ "${configured//[[:space:]]/}" == "$hosted" ]]; then + echo "Runner: ubuntu-24.04 (from repository variable CI_SELF_HOSTED_LABELS)" >&2 + echo "linux=$hosted" elif [[ -n "${configured//[[:space:]]/}" ]]; then choose_labels "repository variable CI_SELF_HOSTED_LABELS" "$configured" +elif [[ "$event" == push || "$event" == pull_request ]]; then + choose_labels "default for $event (CI_SELF_HOSTED_LABELS unset)" "$default_labels" else - echo "Runner: ubuntu-24.04 (CI_SELF_HOSTED_LABELS unset)" >&2 + echo "Runner: ubuntu-24.04 ($event, CI_SELF_HOSTED_LABELS unset)" >&2 echo "linux=$hosted" fi diff --git a/scripts/ci-runner-route.test.ts b/scripts/ci-runner-route.test.ts index 144f7e4d1b..538c8671fa 100644 --- a/scripts/ci-runner-route.test.ts +++ b/scripts/ci-runner-route.test.ts @@ -26,9 +26,28 @@ function routed(env: Record): unknown { } describe.skipIf(process.platform === "win32")("CI runner route", () => { - it("stays on the GitHub-hosted image without the variable", () => { - expect(routed({ EVENT_NAME: "pull_request", HEAD_REPO: repo })).toBe("ubuntu-24.04"); - expect(routed({ EVENT_NAME: "push" })).toBe("ubuntu-24.04"); + it("routes to the maintainer PC's runners without the variable", () => { + expect(routed({ EVENT_NAME: "pull_request", HEAD_REPO: repo })).toEqual(["self-hosted", "self-hosted-ovis"]); + expect(routed({ EVENT_NAME: "push" })).toEqual(["self-hosted", "self-hosted-ovis"]); + }); + + it("keeps any other event hosted without the variable", () => { + for (const event of ["workflow_dispatch", "workflow_run", "merge_group", "schedule", ""]) { + expect(routed({ EVENT_NAME: event, HEAD_REPO: repo })).toBe("ubuntu-24.04"); + } + }); + + it("keeps everything on the GitHub-hosted image when the variable names it", () => { + const hosted = '"ubuntu-24.04"'; + expect(routed({ EVENT_NAME: "pull_request", HEAD_REPO: repo, CONFIGURED_LABELS: hosted })).toBe("ubuntu-24.04"); + expect(routed({ EVENT_NAME: "push", CONFIGURED_LABELS: hosted })).toBe("ubuntu-24.04"); + expect(routed({ EVENT_NAME: "workflow_dispatch", REQUESTED_LABELS: labels, CONFIGURED_LABELS: hosted })).toEqual(JSON.parse(labels)); + }); + + it("keeps a fork pull request hosted without the variable", () => { + expect(routed({ EVENT_NAME: "pull_request", HEAD_REPO: "someone/fork" })).toBe("ubuntu-24.04"); + expect(routed({ EVENT_NAME: "pull_request_target", HEAD_REPO: "someone/fork" })).toBe("ubuntu-24.04"); + expect(routed({ EVENT_NAME: "pull_request" })).toBe("ubuntu-24.04"); }); it("routes a same-repository pull request to the configured labels", () => { diff --git a/scripts/self-hosted-runner.test.ts b/scripts/self-hosted-runner.test.ts index d2b5b4ba12..3e9e9bc3c3 100644 --- a/scripts/self-hosted-runner.test.ts +++ b/scripts/self-hosted-runner.test.ts @@ -1,11 +1,46 @@ import { describe, expect, it } from "bun:test"; -import { chmodSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; +import { chmodSync, copyFileSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { delimiter, join } from "node:path"; import { spawnSync } from "node:child_process"; import { fileURLToPath } from "node:url"; const script = fileURLToPath(new URL("./self-hosted-runner/run.sh", import.meta.url)); +const jobStarted = fileURLToPath(new URL("./self-hosted-runner/job-started.sh", import.meta.url)); + +/** Runs the hook as the runner does, under a process named Runner.Worker (a copy of bash) that goes on after it. */ +function runJobStarted(eventName: string, payload: unknown, event: { raw?: string; path?: "missing" | "directory" | "unset"; repository?: "unset"; closedOutput?: boolean } = {}) { + const tempDir = mkdtempSync(join(tmpdir(), "herdr-runner-hook-test-")); + try { + let eventPath: string | undefined = join(tempDir, "event.json"); + writeFileSync(eventPath, event.raw ?? JSON.stringify(payload)); + if (event.path === "missing") eventPath = join(tempDir, "no-such-event.json"); + if (event.path === "directory") eventPath = tempDir; + if (event.path === "unset") eventPath = undefined; + const worker = join(tempDir, "Runner.Worker"); + copyFileSync(spawnSync("bash", ["-c", "command -v bash"], { encoding: "utf8" }).stdout.trim(), worker); + chmodSync(worker, 0o755); + // closedOutput: the hook's stdout and stderr are a pipe nobody reads any more (a FIFO whose only + // reader is closed before the hook starts), so its first write, even an error message, raises SIGPIPE + const hook = event.closedOutput + ? 'mkfifo "$1" && exec 4<>"$1" 5>"$1" 4<&- && bash "$0" >&5 2>&5; echo "next step ran"' + : 'bash "$0"; echo "next step ran"'; + const result = spawnSync(worker, ["-c", hook, jobStarted, join(tempDir, "output.fifo")], { + encoding: "utf8", + timeout: 5_000, + env: { + PATH: process.env.PATH ?? "", + GITHUB_EVENT_NAME: eventName, + ...(event.repository === "unset" ? {} : { GITHUB_REPOSITORY: "devswha/ovis" }), + ...(eventPath === undefined ? {} : { GITHUB_EVENT_PATH: eventPath }), + }, + }); + expect(result.error).toBeUndefined(); + return result; + } finally { + rmSync(tempDir, { recursive: true, force: true }); + } +} function runRunner(options: { count?: number; @@ -139,3 +174,56 @@ describe.skipIf(process.platform === "win32")("self-hosted runner launcher", () } }); }); + +// the runner image is Linux, and the hook finds the worker through /proc +describe.skipIf(process.platform !== "linux")("self-hosted runner job-started hook", () => { + it("runs a job from this repository", () => { + for (const [event, payload] of [ + ["push", { ref: "refs/heads/main" }], + ["pull_request", { pull_request: { head: { repo: { full_name: "devswha/ovis" } } } }], + ["workflow_run", { workflow_run: { head_repository: { full_name: "devswha/ovis" } } }], + ] as const) { + const result = runJobStarted(event, payload); + expect(result.status).toBe(0); + expect(result.stdout).toContain("next step ran"); + } + }); + + it("refuses a job carrying another repository's code before it runs", () => { + for (const [event, payload] of [ + ["pull_request", { pull_request: { head: { repo: { full_name: "someone/ovis" } } } }], + ["pull_request_target", { pull_request: { head: { repo: { full_name: "someone/ovis" } } } }], + ["pull_request", { pull_request: { head: { repo: null } } }], + ["workflow_run", { workflow_run: { head_repository: { full_name: "someone/ovis" } } }], + ] as const) { + const result = runJobStarted(event, payload); + // a failing hook alone would let `if: always()` steps run: the worker itself is killed + expect(result.signal).toBe("SIGKILL"); + expect(result.stdout).toContain("::error::"); + expect(result.stdout).not.toContain("next step ran"); + } + }); + + it("refuses a job it cannot check: fails closed on an unreadable or broken event", () => { + const forkless = { ref: "refs/heads/main" }; + for (const event of [ + { path: "missing" }, + { path: "directory" }, + { path: "unset" }, + { raw: '{"pull_request": {"head":' }, + { raw: "not json" }, + { raw: '{"pull_request": "someone/ovis"}' }, + { repository: "unset" }, + { path: "missing", closedOutput: true }, + // the shell's own `${VAR:?}` message on a closed stderr + { path: "unset", closedOutput: true }, + { repository: "unset", closedOutput: true }, + ] as const) { + const result = runJobStarted("push", forkless, event); + expect(result.signal).toBe("SIGKILL"); + // a closed output takes the refusal with it: the kill alone is the evidence there + if (!("closedOutput" in event)) expect(result.stdout).toContain("::error::"); + expect(result.stdout).not.toContain("next step ran"); + } + }); +}); diff --git a/scripts/self-hosted-runner/Dockerfile b/scripts/self-hosted-runner/Dockerfile index fde237eb12..c44a141794 100644 --- a/scripts/self-hosted-runner/Dockerfile +++ b/scripts/self-hosted-runner/Dockerfile @@ -78,6 +78,11 @@ RUN mkdir /w && chown runner:runner /w COPY entrypoint.sh /entrypoint.sh RUN chmod 0755 /entrypoint.sh +# Runs before every job, outside the workflow's control: fork code never runs here (job-started.sh). +COPY job-started.sh /opt/runner-hooks/job-started.sh +RUN chmod 0755 /opt/runner-hooks/job-started.sh +ENV ACTIONS_RUNNER_HOOK_JOB_STARTED=/opt/runner-hooks/job-started.sh + USER runner WORKDIR /home/runner/actions-runner ENTRYPOINT ["tini", "--", "/entrypoint.sh"] diff --git a/scripts/self-hosted-runner/README.md b/scripts/self-hosted-runner/README.md index 7d8f03da9c..a6c6bab408 100644 --- a/scripts/self-hosted-runner/README.md +++ b/scripts/self-hosted-runner/README.md @@ -12,7 +12,7 @@ An ephemeral Docker GitHub Actions runner for this repository's Linux CI/CD jobs ```sh scripts/self-hosted-runner/run.sh # 1 slot, repo from the current checkout scripts/self-hosted-runner/run.sh --count 3 -scripts/self-hosted-runner/run.sh --repo OWNER/NAME --labels herdr-ci --build +scripts/self-hosted-runner/run.sh --repo OWNER/NAME --labels self-hosted-ovis --build scripts/self-hosted-runner/run.sh --once # one container per slot, then exit ``` @@ -60,13 +60,23 @@ the disposable containers. ## Routing jobs to it +`ci.yml`'s Linux jobs go to `["self-hosted","self-hosted-ovis"]` by default, the launcher's default +label. The repository variable overrides that: + ```sh -gh variable set CI_SELF_HOSTED_LABELS --body '["self-hosted","linux","x64","herdr-ci"]' -gh variable delete CI_SELF_HOSTED_LABELS # back to GitHub-hosted ubuntu-24.04 +gh variable set CI_SELF_HOSTED_LABELS --body '"ubuntu-24.04"' # PC offline: everything GitHub-hosted +gh variable set CI_SELF_HOSTED_LABELS --body '["self-hosted","self-hosted-ovis"]' # pages.yml and release.yml too +gh variable delete CI_SELF_HOSTED_LABELS # back to the default ``` +On the maintainer PC the launcher runs as the systemd user unit `ovis-ci-runner.service` +(`run.sh --repo devswha/ovis --count 4 --labels self-hosted-ovis` from a copy of this directory in +`~/.local/share/ovis-ci-runner`): four slots let one CI run's four Linux jobs start together, and +`check.ts`'s shared slots still cap its herdr-backed lanes at three. Copy the directory again and +restart the unit after changing these files. + A manual run can override the variable with the `self_hosted_labels` workflow_dispatch input. Fork PRs always use `ubuntu-24.04`; Windows/macOS jobs, `remote-bundles.yml` and `windows-install.yml` stay GitHub-hosted. ## Security model -This is a public repository. Fork PR workflow runs need maintainer approval, and a fork PR can edit `runs-on`, so reading the workflow changes before approving is the real boundary. Containers are ephemeral (`--ephemeral`, `--rm`), unprivileged (`--cap-drop ALL`, `no-new-privileges`, no `--privileged`), have no host mounts and no Docker socket, and the image has no sudo. The registration token reaches the container on stdin, never through its environment, so a job cannot read it from `/proc/1/environ`. +This is a public repository. A fork PR's run uses the fork's workflow files and `scripts/ci-runner-route.sh`, so it can choose `runs-on` itself. The runner therefore enforces the boundary: `job-started.sh` runs before every job (`ACTIONS_RUNNER_HOOK_JOB_STARTED`, set in the image, outside the workflow's control) and refuses any job whose event names another head repository (a fork pull request, a deleted head repository, a `workflow_run` of a fork). It fails closed: any exit before it explicitly allows the job, an error of its own included (a missing or unreadable event file, broken JSON, an unset variable), refuses it, and the refusal kills the worker before it logs anything, with SIGPIPE and termination signals ignored, so a closed output cannot stop it halfway. A failing hook alone is not enough: runner v2.338 marks only its "Set up runner" step failed and still runs the job's `if: always()` and `if: failure()` steps. So the hook SIGKILLs the job's `Runner.Worker` before it returns; the job then ends as failed within seconds with no step run and an empty log, and the refusal is written to the container's output (`journalctl --user -u ovis-ci-runner`). Both were checked on a live runner with a refused job: the old exit-only hook ran the `always()` and `failure()` steps, the killing hook ran none. Fork PR runs also need maintainer approval (repository setting: all outside collaborators); never approve one that changes `runs-on`. Containers are ephemeral (`--ephemeral`, `--rm`), unprivileged (`--cap-drop ALL`, `no-new-privileges`, no `--privileged`), have no host mounts and no Docker socket, and the image has no sudo. The registration token reaches the container on stdin, never through its environment, so a job cannot read it from `/proc/1/environ`. diff --git a/scripts/self-hosted-runner/entrypoint.sh b/scripts/self-hosted-runner/entrypoint.sh index db2d9c5f45..cca12297ae 100755 --- a/scripts/self-hosted-runner/entrypoint.sh +++ b/scripts/self-hosted-runner/entrypoint.sh @@ -8,7 +8,7 @@ set -euo pipefail IFS= read -r RUNNER_TOKEN || true : "${RUNNER_TOKEN:?a registration token is required on stdin}" : "${RUNNER_NAME:?RUNNER_NAME is required}" -RUNNER_LABELS="${RUNNER_LABELS:-herdr-ci}" +RUNNER_LABELS="${RUNNER_LABELS:-self-hosted-ovis}" ./config.sh --unattended --ephemeral \ --url "$RUNNER_URL" \ diff --git a/scripts/self-hosted-runner/job-started.sh b/scripts/self-hosted-runner/job-started.sh new file mode 100755 index 0000000000..2a7c55cefb --- /dev/null +++ b/scripts/self-hosted-runner/job-started.sh @@ -0,0 +1,60 @@ +#!/usr/bin/env bash +# Runner-side guard (ACTIONS_RUNNER_HOOK_JOB_STARTED): runs before every job, outside anything a +# workflow controls. A pull request's run executes its own workflow files and route script, so a +# fork could send its job here; this refuses every job whose code comes from another repository +# (a fork pull request, a deleted head repository, a workflow_run of a fork) before a step runs. +# +# Fail closed: every exit before the job is explicitly allowed at the end refuses it, an error of +# this script included (a missing or unreadable event file, broken JSON, an unset variable). +# First of all: a write to a closed output (an error message on a stderr nobody reads) must not +# end this shell with SIGPIPE before the refusal below can run. +trap '' PIPE +set -euo pipefail + +allowed=0 +reason="the guard failed before it could check the job" + +refuse() { + set +e + [[ "$allowed" == 1 ]] && return + # nothing may stop this before the kill: a closed output (SIGPIPE) or a signal ends the shell + # inside its EXIT trap, and the worker would go on + trap '' PIPE HUP INT TERM + local refusal pid parent + # A failing hook only fails its own step: runner v2.338 still runs the job's `if: always()` and + # `if: failure()` steps and its actions' post steps. So end the job first: SIGKILL the + # Runner.Worker that runs it (an ancestor of this hook), which leaves no step to run. + pid=$PPID + while [[ "$pid" -gt 1 ]]; do + parent="$(sed -n 's/^PPid:[[:space:]]*//p' "/proc/$pid/status" 2>/dev/null)" || break + if [[ "$(cat "/proc/$pid/comm" 2>/dev/null)" == Runner.Worker ]]; then + kill -KILL "$pid" + break + fi + pid="${parent:-0}" + done + refusal="This self-hosted runner only runs code from ${GITHUB_REPOSITORY:-its own repository}; refusing ${GITHUB_EVENT_NAME:-this} job${GITHUB_RUN_ID:+ $GITHUB_RUN_ID}: $reason." + echo "::error::$refusal" 2>/dev/null + # the job's log never reaches GitHub once its worker is killed: say it in the container's own + # output too, which the launcher's journal keeps + echo "[job-started] $refusal" 2>/dev/null >/proc/1/fd/2 + exit 1 +} +trap refuse EXIT + +repository="${GITHUB_REPOSITORY:?GITHUB_REPOSITORY is required}" +event_path="${GITHUB_EVENT_PATH:?GITHUB_EVENT_PATH is required}" + +reason="its event file $event_path could not be read as JSON" +head="$(jq -r --arg repo "$repository" ' + if has("pull_request") then (.pull_request.head.repo.full_name // "") + elif has("workflow_run") then (.workflow_run.head_repository.full_name // "") + else $repo end' "$event_path")" + +if [[ "$head" != "$repository" ]]; then + reason="its code comes from ${head:-an unknown repository}" + exit 1 +fi + +allowed=1 +echo "Runner guard: ${GITHUB_EVENT_NAME:-job} from $repository" diff --git a/scripts/self-hosted-runner/run.sh b/scripts/self-hosted-runner/run.sh index 3be5953126..41156e78cd 100755 --- a/scripts/self-hosted-runner/run.sh +++ b/scripts/self-hosted-runner/run.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # Host launcher: keeps N ephemeral runner containers alive, one job per container. # -# Usage: scripts/self-hosted-runner/run.sh [--repo OWNER/NAME] [--count N] [--labels herdr-ci] +# Usage: scripts/self-hosted-runner/run.sh [--repo OWNER/NAME] [--count N] [--labels self-hosted-ovis] # [--build] [--once] set -euo pipefail @@ -10,7 +10,7 @@ IMAGE="herdr-ci-runner:local" REPO="" COUNT=1 -LABELS="herdr-ci" +LABELS="self-hosted-ovis" BUILD=0 ONCE=0 diff --git a/server/shell-agent.test.ts b/server/shell-agent.test.ts index fe08d2b44c..15813331aa 100644 --- a/server/shell-agent.test.ts +++ b/server/shell-agent.test.ts @@ -311,11 +311,18 @@ it("says why the login shell gave no PATH once each time the reason changes", as it("hung or oversized login probe is killed and reaped, and each miss says why", async () => { // a profile that hangs, with a child of its own const hung = fixture(`sleep 30 & echo $! > "${"$"}HOME/child.tmp"; mv "${"$"}HOME/child.tmp" "${"$"}HOME/child.pid"; echo $$ > "${"$"}HOME/shell.tmp"; mv "${"$"}HOME/shell.tmp" "${"$"}HOME/shell.pid"; wait`); + // stopped only once the profile has started both, so a slow start on a busy machine cannot end it first + const stop = new AbortController(); + const stopping = loginPathProbe({ shell: hung.shell, env: hung.env, timeoutMs: 60_000 })(stop.signal); + const pids = [await written(join(hung.home, "shell.pid")), await written(join(hung.home, "child.pid"))]; + stop.abort(); + expect((await stopping).path).toBeNull(); + for (const pid of pids) expect(await running(Number(pid))).toBe(false); + // the same hang outruns its time const started = Date.now(); const timedOut = await once(loginPathProbe({ shell: hung.shell, env: hung.env, timeoutMs: 300 })); expect(timedOut.path).toBeNull(); expect(Date.now() - started).toBeLessThan(5000); - for (const pid of [await written(join(hung.home, "shell.pid")), await written(join(hung.home, "child.pid"))]) expect(await running(Number(pid))).toBe(false); // a profile that prints more than 64 KiB, and then a PATH const noisy = fixture(`echo $$ > "${"$"}HOME/shell.pid"; head -c 200000 /dev/zero | tr '\\\\0' x`);