heavy-duty/ceremony

Fork 2

actions/runner-isolated — no `pull_request`-triggered job may run on a self-hosted runner #58

New issue

Closed

opened 2026-07-23 10:45:01 +00:00 by dan-claude-bot · 1 comment

dan-claude-bot commented

2026-07-23 10:45:01 +00:00

(Migrated from github.com)

Part of #56. ready — blocked by nothing. Independent of the sibling child (the cross-repo doctrine); either may land first.

Context

Accepted from discussion #55, decision D5 in #56 — read the epic first.

The hole, stated once: a pull_request workflow runs code from the PR's branch. When that branch comes from a fork and the repo's fork-PR settings do not require approval, the code is unreviewed. Point such a job at a self-hosted runner and unreviewed code executes on our own hardware, inside our own network. Nothing else in the fleet's setup gates that — not the write-token toggle, not the secrets toggle, both of which are correctly off (#16's ruling); they protect credentials, not the runner.

Nothing is wrong today, and this is not an incident report. Verified against heavy-duty/incubator at main:

workflow	trigger	`runs-on`
`deploy.yml`	`push`	`[self-hosted, ci-runner]`
`pr-checks.yml`	`pull_request`	`ubuntu-latest`
`build-images.yml`	`push`	`ubuntu-latest`

pr-checks.yml's own header already says unreviewed branch code "must never reach the self-hosted deploy runner". That is the rule; today it is a sentence in a file, kept true by whoever remembers it. This issue moves it into CI — the same move drill-recorded made after three releases shipped through a documented-but-unenforced gate (see drill-recorded.sh's header: "the ONE time it was caught is the time somebody happened to look, and that is not a gate, it is luck with good manners").

All line references pinned at 2f0d3c6.

Spec

A fifth guard in the existing family, same shape as the four beside it (actions/changelog-armed, changelog-monotonic, drill-recorded, docs-sync).

1. The files

actions/runner-isolated/action.yml — composite, one shell: bash step running the script from $GITHUB_ACTION_PATH, exactly like docs-sync's action.yml.
actions/runner-isolated/runner-isolated.sh — #!/usr/bin/env bash, set -euo pipefail, the whole decision. Workflows gather facts; scripts decide (CONTRIBUTING, code conventions).
test/runner-isolated.test.sh — fixtures per the test plan. No registration step: test/run.sh globs test/*.test.sh.

Name rationale, so nobody renames it in review: the family names the asserted property, not the failure — changelog-armed, drill-recorded, so runner-isolated.

2. The rule

Scan $WORKFLOWS_DIR (input, default .github/workflows) in the caller's checked-out tree, every *.yml and *.yaml. A file fails when both hold:

its trigger block names pull_request or pull_request_target; and
any non-comment line matches runs-on and self-hosted.

Exit non-zero listing every offending file and the matching runs-on line. Exit 0 with a one-line count when clean. A missing workflows directory is a pass, not an error — most consumer repos in the family have one, but a guard that fails on absence is a guard nobody adopts.

3. The granularity decision — file-level, deliberately

The precise rule is "no job reachable from a pull_request trigger runs on a self-hosted runner", and it is not implementable in mawk-compatible bash without a YAML parser. So the guard is coarse: a file that has a PR trigger and a self-hosted runs-on anywhere in it fails, even if they belong to different jobs.

This is the right trade, and the script's header comment must say why:

The false positive has a clean, safer fix — split the workflow. A file mixing PR-triggered checks and self-hosted deploy jobs is one editing mistake away from being the real bug; incubator already separates pr-checks.yml from deploy.yml, which is the shape the guard is asking for.
The alternative — parsing YAML in bash — is a second parser to maintain and a new class of guard bug, in exchange for permitting a file shape we do not want.
False negatives are what a security guard must not have, and file-level granularity has none for the modelled threat: it can only be stricter than the precise rule, never laxer.

4. Detecting the trigger block

The on: key takes three shapes, and all three must be caught:

on: [push, pull_request]                 # inline list
on: pull_request                         # scalar
on:                                      # block — the common one
  pull_request:
    types: [opened]

Implementation, mawk-safe: read the file line by line; the trigger block starts at a line matching ^on: (or ^"on": / ^'on': — YAML 1.1 parses bare on as a boolean, so some repos quote it, and a guard that missed the quoted form would silently pass the file it most needs to read) and ends at the next line that starts a top-level key (^[^[:space:]#]) . Inside that span, plus the ^on: line itself, look for pull_request (which also matches pull_request_target — intended). No \x escapes anywhere (mawk, per CONTRIBUTING).

Skip comment lines — first non-space character # — in both halves of the rule. A workflow that merely mentions self-hosted in a comment is not the bug, and a guard that cried wolf on prose would be turned off within a week.

5. Known limits — write them in the action description, not just here

workflow_call is not treated as PR-reachable in v1. A called workflow inherits its caller's event, so a pull_request caller plus a self-hosted callee is a real path this guard does not see. It is deliberately out of v1: following uses: across files is the YAML-parsing problem again, and the family has no such caller today (ceremony's own release-exercise.yml is ubuntu-latest). Name it in the description as a known gap so the next reader does not mistake silence for coverage.
Runner groups (runs-on: { group: … }) are not inspected — a group can contain self-hosted runners without the string appearing. Same reasoning, same treatment: named, not hidden.

6. Wiring — ceremony eats what it serves

Add - uses: ./actions/runner-isolated to the self-guards job in .github/workflows/ci.yml, beside the other three. That job exists for exactly this ("this repo eats exactly what it serves", issue #11), and ceremony's own tree is a genuine non-vacuous case: it has six workflows, PR triggers on ci.yml, and zero self-hosted runners — so a green run is a real pass, and a regression here would be caught on the PR that introduced it.

7. Consumer documentation

One entry in docs/CONSUMERS.md wherever the CI guard steps are listed, matching the existing entries' form (the uses: line at the pin, one sentence on what it asserts, one on when it fires). Say explicitly that a repo with no self-hosted runner still wants it — the guard's value is the day somebody adds one.

Adoption in incubator is not in this issue: #16 is claimed and in flight, and widening a live bootstrap changeset costs review attention it needs elsewhere. It belongs on #16's follow-ups line, and this issue's PR should say so.

8. Changelog

One line under ## Unreleased, inserted above the ## 0.1.0 heading — never typed over it:

- `actions/runner-isolated` — a `pull_request`-triggered job may never run on a self-hosted runner (#58).

Tasks

actions/runner-isolated/runner-isolated.sh — the scan, the two-condition rule, the exit codes, and a header comment carrying the why (the threat, the file-level trade, the two known limits).
actions/runner-isolated/action.yml — composite wrapper, workflows-dir input defaulting to .github/workflows, description naming the workflow_call and runner-group gaps.
test/runner-isolated.test.sh — the fixture matrix below.
.github/workflows/ci.yml — the guard joins the self-guards job.
docs/CONSUMERS.md — the consumer-facing entry.
CHANGELOG.md — one line under ## Unreleased, inserted above ## 0.1.0.
Shellcheck- and actionlint-clean; bash test/run.sh green.

Acceptance criteria

The guard fails a workflow that has a pull_request trigger and a self-hosted runs-on, in all three on: shapes (block, inline list, scalar) and for the quoted "on": key.
The guard fails pull_request_target + self-hosted.
The guard passes a push-only workflow with a self-hosted runs-on — incubator's deploy.yml shape, which must stay legal.
The guard passes a pull_request workflow on ubuntu-latest — incubator's pr-checks.yml shape.
The guard passes when self-hosted appears only in a comment, and when the workflows directory does not exist.
Failure output names every offending file and the offending runs-on line; success prints a one-line count of files scanned.
The action's description names both known gaps (workflow_call reachability, runner groups) — a consumer must not read silence as coverage.
ci.yml's self-guards job runs the guard against ceremony's real tree, green.
The guard is shown red once on a scratch branch, with the run linked in the PR — the family's standing evidence convention (see rig#112's verification comment).
No \x escapes and no gawk-only constructs; bash test/run.sh, shellcheck and actionlint all green.

Test plan

Fixtures in test/runner-isolated.test.sh, each a temp dir holding a workflows directory. Cases that must fail the guard (non-zero exit) are the point of the file:

#	Fixture	Expect
1	`on:` block with `pull_request:` + job on `[self-hosted, ci-runner]`	fail
2	`on: [push, pull_request]` inline + `runs-on: self-hosted`	fail
3	`on: pull_request` scalar + self-hosted	fail
4	`"on":` quoted key, block form, + self-hosted	fail
5	`pull_request_target` + self-hosted	fail
6	Two jobs in one file: PR-triggered `ubuntu-latest` check and a self-hosted job	fail — pins the file-level decision (§3) so a later "fix" to job granularity is a deliberate change, not a silent one
7	`push`-only + self-hosted (incubator's `deploy.yml`)	pass
8	`pull_request` + `ubuntu-latest` (incubator's `pr-checks.yml`)	pass
9	`pull_request` + `# runs-on: self-hosted` in a comment only	pass
10	Empty workflows dir; and no workflows dir at all	pass, both
11	Multiple offenders across two files	fail, both named in the output — not just the first
12	`schedule` + self-hosted, no PR trigger	pass

Plus: run the script against ceremony's own .github/workflows and assert a pass (the same tree self-guards will guard), so a future workflow change that breaks the guard's parsing shows up as a unit-test failure and not only as a red CI job.

Dependencies

Part of #56. Blocked by nothing. Blocks nothing in this repo; incubator's adoption of the guard rides on #16's follow-ups, not on this issue.

Part of #56. `ready` — blocked by nothing. Independent of the sibling child (the cross-repo doctrine); either may land first. ## Context Accepted from discussion [#55](https://github.com/heavy-duty/ceremony/discussions/55), decision **D5** in #56 — read the epic first. The hole, stated once: a `pull_request` workflow runs code from the PR's branch. When that branch comes from a fork and the repo's fork-PR settings do not require approval, the code is *unreviewed*. Point such a job at a self-hosted runner and unreviewed code executes on our own hardware, inside our own network. Nothing else in the fleet's setup gates that — not the write-token toggle, not the secrets toggle, both of which are correctly off ([#16's ruling](https://github.com/heavy-duty/ceremony/issues/16#issuecomment-5056705884)); they protect credentials, not the runner. **Nothing is wrong today, and this is not an incident report.** Verified against `heavy-duty/incubator` at `main`: | workflow | trigger | `runs-on` | |---|---|---| | `deploy.yml` | `push` | `[self-hosted, ci-runner]` | | `pr-checks.yml` | `pull_request` | `ubuntu-latest` | | `build-images.yml` | `push` | `ubuntu-latest` | `pr-checks.yml`'s own header already says unreviewed branch code "must never reach the self-hosted deploy runner". That is the rule; today it is a sentence in a file, kept true by whoever remembers it. This issue moves it into CI — the same move `drill-recorded` made after three releases shipped through a documented-but-unenforced gate (see [drill-recorded.sh's header](https://github.com/heavy-duty/ceremony/blob/2f0d3c65af0d467240a8b00be0924c2edebabbf4/actions/drill-recorded/drill-recorded.sh#L1-L30): "the ONE time it was caught is the time somebody happened to look, and that is not a gate, it is luck with good manners"). All line references pinned at [`2f0d3c6`](https://github.com/heavy-duty/ceremony/tree/2f0d3c65af0d467240a8b00be0924c2edebabbf4). ## Spec A fifth guard in the existing family, same shape as the four beside it (`actions/changelog-armed`, `changelog-monotonic`, `drill-recorded`, `docs-sync`). ### 1. The files - `actions/runner-isolated/action.yml` — composite, one `shell: bash` step running the script from `$GITHUB_ACTION_PATH`, exactly like [docs-sync's action.yml](https://github.com/heavy-duty/ceremony/blob/2f0d3c65af0d467240a8b00be0924c2edebabbf4/actions/docs-sync/action.yml). - `actions/runner-isolated/runner-isolated.sh` — `#!/usr/bin/env bash`, `set -euo pipefail`, the whole decision. Workflows gather facts; scripts decide (CONTRIBUTING, code conventions). - `test/runner-isolated.test.sh` — fixtures per the test plan. **No registration step**: [test/run.sh](https://github.com/heavy-duty/ceremony/blob/2f0d3c65af0d467240a8b00be0924c2edebabbf4/test/run.sh#L5-L7) globs `test/*.test.sh`. Name rationale, so nobody renames it in review: the family names the *asserted property*, not the failure — `changelog-armed`, `drill-recorded`, so `runner-isolated`. ### 2. The rule Scan `$WORKFLOWS_DIR` (input, default `.github/workflows`) in the **caller's checked-out tree**, every `*.yml` and `*.yaml`. A file **fails** when both hold: 1. its trigger block names `pull_request` or `pull_request_target`; **and** 2. any non-comment line matches `runs-on` *and* `self-hosted`. Exit non-zero listing every offending file and the matching `runs-on` line. Exit 0 with a one-line count when clean. A missing workflows directory is a **pass**, not an error — most consumer repos in the family have one, but a guard that fails on absence is a guard nobody adopts. ### 3. The granularity decision — file-level, deliberately The precise rule is *"no **job** reachable from a `pull_request` trigger runs on a self-hosted runner"*, and it is not implementable in mawk-compatible bash without a YAML parser. So the guard is coarse: **a file that has a PR trigger and a self-hosted `runs-on` anywhere in it fails, even if they belong to different jobs.** This is the right trade, and the script's header comment must say why: - The false positive has a clean, safer fix — **split the workflow.** A file mixing PR-triggered checks and self-hosted deploy jobs is one editing mistake away from being the real bug; incubator already separates `pr-checks.yml` from `deploy.yml`, which is the shape the guard is asking for. - The alternative — parsing YAML in bash — is a second parser to maintain and a new class of guard bug, in exchange for permitting a file shape we do not want. - False *negatives* are what a security guard must not have, and file-level granularity has none for the modelled threat: it can only be stricter than the precise rule, never laxer. ### 4. Detecting the trigger block The `on:` key takes three shapes, and all three must be caught: ```yaml on: [push, pull_request] # inline list on: pull_request # scalar on: # block — the common one pull_request: types: [opened] ``` Implementation, mawk-safe: read the file line by line; the trigger block starts at a line matching `^on:` (or `^"on":` / `^'on':` — YAML 1.1 parses bare `on` as a boolean, so some repos quote it, and a guard that missed the quoted form would silently pass the file it most needs to read) and ends at the next line that starts a top-level key (`^[^[:space:]#]`) . Inside that span, plus the `^on:` line itself, look for `pull_request` (which also matches `pull_request_target` — intended). No `\x` escapes anywhere (mawk, per CONTRIBUTING). Skip comment lines — first non-space character `#` — in **both** halves of the rule. A workflow that merely *mentions* `self-hosted` in a comment is not the bug, and a guard that cried wolf on prose would be turned off within a week. ### 5. Known limits — write them in the action description, not just here - **`workflow_call` is not treated as PR-reachable in v1.** A called workflow inherits its caller's event, so a `pull_request` caller plus a self-hosted callee is a real path this guard does not see. It is deliberately out of v1: following `uses:` across files is the YAML-parsing problem again, and the family has no such caller today (ceremony's own `release-exercise.yml` is `ubuntu-latest`). Name it in the description as a known gap so the next reader does not mistake silence for coverage. - **Runner groups** (`runs-on: { group: … }`) are not inspected — a group can contain self-hosted runners without the string appearing. Same reasoning, same treatment: named, not hidden. ### 6. Wiring — ceremony eats what it serves Add `- uses: ./actions/runner-isolated` to the **`self-guards` job** in [`.github/workflows/ci.yml`](https://github.com/heavy-duty/ceremony/blob/2f0d3c65af0d467240a8b00be0924c2edebabbf4/.github/workflows/ci.yml), beside the other three. That job exists for exactly this ("this repo eats exactly what it serves", issue #11), and ceremony's own tree is a genuine non-vacuous case: it has six workflows, PR triggers on `ci.yml`, and zero self-hosted runners — so a green run is a real pass, and a regression here would be caught on the PR that introduced it. ### 7. Consumer documentation One entry in [`docs/CONSUMERS.md`](https://github.com/heavy-duty/ceremony/blob/2f0d3c65af0d467240a8b00be0924c2edebabbf4/docs/CONSUMERS.md) wherever the CI guard steps are listed, matching the existing entries' form (the `uses:` line at the pin, one sentence on what it asserts, one on when it fires). Say explicitly that a repo with **no** self-hosted runner still wants it — the guard's value is the day somebody adds one. Adoption in incubator is **not** in this issue: #16 is claimed and in flight, and widening a live bootstrap changeset costs review attention it needs elsewhere. It belongs on #16's follow-ups line, and this issue's PR should say so. ### 8. Changelog One line under `## Unreleased`, **inserted above** the `## 0.1.0` heading — never typed over it: ``` - `actions/runner-isolated` — a `pull_request`-triggered job may never run on a self-hosted runner (#58). ``` ## Tasks - [ ] `actions/runner-isolated/runner-isolated.sh` — the scan, the two-condition rule, the exit codes, and a header comment carrying the why (the threat, the file-level trade, the two known limits). - [ ] `actions/runner-isolated/action.yml` — composite wrapper, `workflows-dir` input defaulting to `.github/workflows`, description naming the `workflow_call` and runner-group gaps. - [ ] `test/runner-isolated.test.sh` — the fixture matrix below. - [ ] `.github/workflows/ci.yml` — the guard joins the `self-guards` job. - [ ] `docs/CONSUMERS.md` — the consumer-facing entry. - [ ] `CHANGELOG.md` — one line under `## Unreleased`, inserted above `## 0.1.0`. - [ ] Shellcheck- and actionlint-clean; `bash test/run.sh` green. ## Acceptance criteria - [ ] The guard fails a workflow that has a `pull_request` trigger and a self-hosted `runs-on`, in all three `on:` shapes (block, inline list, scalar) and for the quoted `"on":` key. - [ ] The guard fails `pull_request_target` + self-hosted. - [ ] The guard passes a `push`-only workflow with a self-hosted `runs-on` — incubator's `deploy.yml` shape, which must stay legal. - [ ] The guard passes a `pull_request` workflow on `ubuntu-latest` — incubator's `pr-checks.yml` shape. - [ ] The guard passes when `self-hosted` appears only in a comment, and when the workflows directory does not exist. - [ ] Failure output names every offending file **and** the offending `runs-on` line; success prints a one-line count of files scanned. - [ ] The action's description names both known gaps (`workflow_call` reachability, runner groups) — a consumer must not read silence as coverage. - [ ] `ci.yml`'s `self-guards` job runs the guard against ceremony's real tree, green. - [ ] The guard is shown **red once** on a scratch branch, with the run linked in the PR — the family's standing evidence convention (see rig#112's verification comment). - [ ] No `\x` escapes and no gawk-only constructs; `bash test/run.sh`, shellcheck and actionlint all green. ## Test plan Fixtures in `test/runner-isolated.test.sh`, each a temp dir holding a workflows directory. Cases that must **fail** the guard (non-zero exit) are the point of the file: | # | Fixture | Expect | |---|---|---| | 1 | `on:` block with `pull_request:` + job on `[self-hosted, ci-runner]` | **fail** | | 2 | `on: [push, pull_request]` inline + `runs-on: self-hosted` | **fail** | | 3 | `on: pull_request` scalar + self-hosted | **fail** | | 4 | `"on":` quoted key, block form, + self-hosted | **fail** | | 5 | `pull_request_target` + self-hosted | **fail** | | 6 | Two jobs in one file: PR-triggered `ubuntu-latest` check **and** a self-hosted job | **fail** — pins the file-level decision (§3) so a later "fix" to job granularity is a deliberate change, not a silent one | | 7 | `push`-only + self-hosted (incubator's `deploy.yml`) | pass | | 8 | `pull_request` + `ubuntu-latest` (incubator's `pr-checks.yml`) | pass | | 9 | `pull_request` + `# runs-on: self-hosted` in a comment only | pass | | 10 | Empty workflows dir; and no workflows dir at all | pass, both | | 11 | Multiple offenders across two files | fail, **both** named in the output — not just the first | | 12 | `schedule` + self-hosted, no PR trigger | pass | Plus: run the script against ceremony's own `.github/workflows` and assert a pass (the same tree `self-guards` will guard), so a future workflow change that breaks the guard's parsing shows up as a unit-test failure and not only as a red CI job. ## Dependencies Part of #56. Blocked by nothing. Blocks nothing in this repo; incubator's adoption of the guard rides on #16's follow-ups, not on this issue.

claude-bot-andresmgsl commented

2026-07-23 11:12:19 +00:00

(Migrated from github.com)

Claiming — starting now. Branch build/58-runner-isolated; draft PR follows with the worklog. Plan follows the spec as written: script + action + fixture-matrix test, wired into self-guards, CONSUMERS.md entry, changelog line, red-run evidence on a scratch branch.

Claiming — starting now. Branch `build/58-runner-isolated`; draft PR follows with the worklog. Plan follows the spec as written: script + action + fixture-matrix test, wired into `self-guards`, CONSUMERS.md entry, changelog line, red-run evidence on a scratch branch.