feat: add governed local runner routing - #74
Conversation
|
Claude finished @kyle-sexton's task in 5m 50s —— View job Code Review — feat: add governed local runner routingThis is a security-critical PR introducing the central routing contract for an ephemeral local GitHub Actions fleet. I've reviewed all 49 changed files, focusing on the security model, correctness, and CLAUDE.md compliance. Overall AssessmentThe design is well-considered and the threat model is clearly articulated. The fail-closed pattern is consistently applied: every error path (auth failure, API timeout, invalid response, missing config, public repo, fork PR, Dependabot, rerun) routes to a hosted runner. The dual-guard pattern (YAML The findings below are ordered by severity. Findings1.
|
|
Claude finished @kyle-sexton's task in 2m 45s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml |
Open — worth resolving before production routing goes live |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry needs a comment |
Open |
| 3 | runAttempt !== 1 in config-check block after early rerun guard |
Open |
| 4 | GitHub API version string "2026-03-10" untracked for drift |
Open |
| 5 | Canary select-runner SHA pin comment says governed selector review instead of # vX.Y.Z |
Open |
| 6 | safe_path allows // (cosmetic, no exploit path) |
Open |
Summary
The fix: make selector gates reproducible in CI commit is focused and correct. The fetch-depth: 0 addition is the right fix for the root cause (a git show <old-sha> call that fails on GitHub Actions' default shallow clone), the contract test enforces it as an invariant, the cache: parameter clean-up is accurate, and the ShellCheck suppress extension is precise. No new issues were introduced.
|
Claude finished @kyle-sexton's task in 2m 13s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml:64 |
Open — resolve before production routing goes live; changes the trust envelope of the selector job |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry needs a comment |
Open — the allowlist now closes the downstream gap, but the code-comment explaining the intentional divergence is still missing |
| 3 | runAttempt !== 1 in config-check block after early rerun guard — redundancy note |
Open |
| 4 | GitHub API version string "2026-03-10" untracked for drift |
Open |
| 5 | Canary select-runner SHA pin comment says governed selector review instead of # vX.Y.Z |
Open |
| 6 | safe_path allows // (cosmetic, no exploit path) |
Open |
Summary
The fix: constrain hosted fallback labels commit closes a real governance gap — without it, any string could be passed as the hosted fallback runner, bypassing the implicit assumption that only reviewed labels are ever used. The implementation is correct: the allowlist is the first guard, it doesn't short-circuit the downstream safety checks (reserved-label collision, candidate-label collision), case sensitivity follows runner label conventions, and the empty/missing input path was already safe. Two targeted test cases cover the newly enforced constraint. The README and input description updates are accurate and improve clarity for consumers.
No new issues are introduced by this change.
|
Claude finished @kyle-sexton's task in 2m 16s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml:64 |
Open — resolve before production routing goes live; the selector job trust envelope depends on it |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry needs a comment |
Open — exploit surface is closed by the allowlist, but the intentional divergence is still undocumented |
| 3 | runAttempt !== 1 in config-check block after early rerun guard |
Open — redundancy note for future maintainers |
| 4 | GitHub API version string "2026-03-10" untracked for drift |
Open |
| 5 | Canary select-runner SHA pin comment says governed selector review instead of # vX.Y.Z |
Open |
| 6 | safe_path allows // (cosmetic, no exploit path) |
Open |
Summary
The two commits since the last review are a standards sync and its merge. The substantive change is the XXX case-sensitivity split in comment-hygiene-patterns.sh — the shopt sandwiching is correct on both branches, the rationale is sound, and no new issues are introduced. Everything else is comment accuracy improvements across config files.
The six prior findings remain open and unchanged. Finding #1 (ubuntu-slim) is the only one with a security relevance before production routing goes live.
|
Claude finished @kyle-sexton's task in 4m 47s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml:64 |
Open — this is the only remaining item with a trust-envelope implication before production routing goes live |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry |
Resolved — comment added in this batch |
| 3 | runAttempt !== 1 redundancy note after early rerun guard |
Resolved — comment added in this batch |
| 4 | GitHub API version string "2026-03-10" untracked for drift |
Open — cosmetic / no exploit path |
| 5 | Canary select-runner SHA pin comment says governed selector review instead of # vX.Y.Z |
Open — cosmetic, intent is clear |
| 6 | safe_path allows // |
Open — cosmetic, no exploit path |
What Works Well
Contamination model correctness: The label contamination pass runs over the complete paginated inventory, not just the idle subset. An offline unmanaged runner that happens to carry a configured label is caught. A managed runner with ephemeral: false is caught. A managed runner with os: windows is caught. And critically, contamination is per-configured-label: a clean lower-priority label remains selectable when its own bearers all pass the contract, because GitHub cannot route a runs-on: <clean-label> job to a bearer of only the contaminated label.
Ephemeral field relaxation is correct: The 2026-03-10 OpenAPI declares ephemeral optional on runners. The new code path — !Object.hasOwn(runner, "ephemeral") || runner.ephemeral === true — correctly allows omission while keeping explicit false as authoritative exclusion and keeping present non-boolean as a validation failure.
Dual-layer OS verification: The selector rejects inventory-reported OS violations (API layer), and the canary rejects runner.os != Linux or runner.arch != X64 at runtime (context layer). These are independent evidence sources — the REST inventory and the runner context are populated by different GitHub subsystems.
os: unknown acceptance is documented and bounded: unknown is accepted explicitly under the JIT trust assumption, not as a generic wildcard. The README's wording ("accepted solely under the same governed prefix-and-label/JIT trust assumption") makes the distinction clear. Any bearer of a configured label reporting a different OS string (e.g., windows, macOS) still contaminates.
Test coverage is comprehensive: The new suite covers every contamination trigger independently and in combination, including cross-label independence (clean sibling survives contaminated first candidate) and runner label case variants.
Summary
This batch closes the main behavioral gap: a non-Linux or non-ephemeral runner bearing a managed label could previously be selected (if online and idle). The new contamination model prevents that by failing the entire label rather than just excluding that runner. Prior findings #2 and #3 are resolved with targeted comments. The remaining open items (#1, #4, #5, #6) are unchanged from the prior review — finding #1 (ubuntu-slim) is the only one that warrants resolution before production routing goes live, as it determines whether the selector job's observer token mint runs on fully hosted or org-level infrastructure.
|
Revalidated the complete review history against final signed head
The live API issue discovered after the earlier reviews is fixed: official OpenAPI makes |
|
Claude finished @kyle-sexton's task in 6m 34s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Rebuttal verdict |
|---|---|---|
| 1 | ubuntu-slim runner label changes selector trust envelope |
Rebuttal accepted — finding was incorrect. GitHub documents ubuntu-slim as a standard GitHub-hosted runner with level-2 hypervisor isolation, available for both public and private repos. The prior review incorrectly assumed it was an org-level custom runner. The 15-minute cap is satisfied by the 5-minute timeout-minutes on the selector job. |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry |
Resolved in c34f0aa batch. |
| 3 | runAttempt !== 1 redundancy note |
Resolved in c34f0aa batch. |
| 4 | API version string "2026-03-10" untracked for drift |
Deferred correctly. 2026-03-10 is the current supported version; retirement still fails hosted; the drift lifecycle tracks future review. Valid deferred maintenance. |
| 5 | Canary SHA pin comment format | Rebuttal accepted — finding was incorrect. The canary intentionally pins a specific reviewed commit that must not be auto-bumped. The comment # governed selector review is explanatory metadata, not a version tag for Dependabot. The CLAUDE.md SHA-pin requirement is fully met. |
| 6 | safe_path allows // |
Deferred correctly. POSIX collapses interior repeated slashes; content/hash validation remains intact. Cosmetic with no exploit path. |
All six prior findings are now resolved, closed, or correctly deferred.
Incremental Review — feat: add production runner HA proof → fix: repin production HA proof caller
Four commits since the prior review (c34f0aa). Eight files changed. Here is a detailed assessment of each commit cluster.
1. feat: add production runner HA proof — production-ha-proof.cjs + production-ha-proof.yml
JS module (production-ha-proof.cjs)
The module mirrors the pagination, validation, and evidence patterns from select-runner.cjs with several additions appropriate to a production acceptance proof:
assertPositiveInteger/assertSafeString/validateGroup/validateRepository/validateRunnerform a strict chain.validateRunnerrequires: integer ID,SAFE_NAME-matching name, name withinhost.runnerPrefix, OS inACCEPTED_RUNNER_OSES({"linux","unknown"}),statusin{"online","offline"}, booleanbusy, optional-but-type-guardedephemeral, non-empty labels, and the production routing label present. This mirrors the V1 contract from the selector and is correct.- Cross-group runner deduplication:
seenRunnerIdsspans both host groups, ensuring no runner appears in bothci-local-melo-desk-001andci-local-melo-lap-001. This is the correct guard against the inventory misreporting a runner as belonging to two independent groups. assertUniqueIdson runner groups: Ensures the two group IDs are distinct beforevalidateGroupis called on each.- Repository set parity:
repositorySetKey(evidenceGroups[0].selectedRepositories) === repositorySetKey(evidenceGroups[1].selectedRepositories)— both production groups must have exactly the same private repository access. Sorted by ID before comparison so order differences don't produce false mismatches. PROOF_REPOSITORYpresence check: The proof repo itself (melodic-software/ci-runner-canary) must be in the shared repository set, ensuring the caller has access to the inventory it is authorized to read.waitForDesktopDrainvalidatesmaxWaitMinutesis5–25before entering the polling loop, with a stable-observation counter (REQUIRED_STABLE_DRAIN_OBSERVATIONS = 2) that resets on any non-zero desktop observation. The deadline is checked after each observation, preventing an infinite loop. Correct.commonEvidencerequiresRUN_ATTEMPTto match/^1$/u— a JS-layer rerun guard independent of the YAMLif:guard.- Evidence files written with
mode: 0o600— owner-read/write only. Correct. - Failure path writes structured evidence: The
catchblock callssafeFailure(error)to produce a clean{code, message}and thenwriteEvidence(evidencePath, {...base, status: "failed", failure, ...}). SinceevidencePathis bound before thetry, a failure in thetryblock always has a valid path to write to. The only scenario where this fails is ifrequiredEnvironment(env, "EVIDENCE_PATH")itself throws — butEVIDENCE_PATHis set unconditionally to${{ runner.temp }}/...in the YAML env block, so that can't happen.
Workflow (production-ha-proof.yml)
workflow_callonly,permissions: {}at workflow level,permissions: {}on every job — CLAUDE.md compliant. Nopull_request,pull_request_target, orworkflow_runtriggers.preflightjob usesCALLER_WORKFLOW_REF/EXPECTED_CALLER_WORKFLOW_REFto hard-fail on any caller other thanmelodic-software/ci-runner-canary/.github/workflows/production-ha-proof.yml@refs/heads/main. This is a branch ref (not a SHA), which is correct: the reviewed SHA-pinned template caller runs from that branch. The reusable workflow can only be invoked via its@<sha>pin in the template, so the branch ref check validates the caller's identity without needing to predict which commit will be onmain.- All substantive jobs run
ubuntu-24.04exceptproduction-execution, which runs on${{ needs.select-production-runner.outputs.runner }}— the selector's direct output. The selector already guarantees this is a governed label orubuntu-24.04fallback; thevalidate-selectionjob hard-fails if it's not the local production label. select-production-runnerpins selector at66e3e974e9c0132150cc982cdd76aca284df19de— the reviewed selector SHA from the prior batch. This is the same SHA used bylocal-runner-canary.yml. Consistent.EXPECTED_HOSTternary${{ inputs.mode == 'desktop-only' && 'melo-desk-001' || 'melo-lap-001' }}is correct: onlydesktop-onlyacquires the desktop;laptop-only,failover, andlaptop-powerall acquire the laptop. Infailovermode,failover-holdensures the desktop drains beforeproduction-executionruns, so the laptop is the only available host at acquisition time.container_idfrom/etc/hostnameis guarded by[[ "$container_id" =~ ^[A-Za-z0-9._-]{1,128}$ ]]before printf-interpolation into JSON. No JSON-special characters can appear. Correct.- Laptop-power heartbeat loop caps
sleep_fortoremainingbefore the final sleep to avoid overshooting the minimum. The post-looptest "$final_elapsed" -ge "$minimum"is a hard assertion that the loop ran long enough regardless of timing variation. Correct. - Evidence sanitization: the artifacts include REST group IDs, runner names, observation timestamps, and run correlation. They explicitly exclude App tokens, private keys, JIT configuration, and raw API responses. The README documents this limitation set precisely.
- No
secrets: inherit, no untrusted checkout, no rawgh runorgh apicalls in the workflow. ✓
2. fix: harden production HA proof reruns — four targeted hardening additions
This commit adds four independent rerun guards and two token-scope tightenings that were missing from the initial implementation.
REF_PROTECTED check (production-ha-proof.yml:69):
[[ "$REF_PROTECTED" = true ]] || fail "production proof ref is not protected"github.ref_protected is true only when the triggering ref is protected in the repository settings. Without this check, a branch named main that is not actually protected (e.g., protection rules were temporarily disabled) would pass the SOURCE_REF check. Defense-in-depth is correct here.
if: github.run_attempt == 1 on inventory, select-production-runner, validate-selection, failover-hold: These were missing from the initial implementation. Without them, on a rerun, preflight would fail (routing to hosted), but the YAML scheduler might still attempt to evaluate conditions on downstream jobs. Adding the guard to each job ensures no production API call, token mint, selector invocation, or observer hold can execute on attempt ≥ 2, regardless of how preflight fails.
always() → !cancelled() + github.run_attempt == 1 on production-execution: always() would have run the job even when all upstreams were cancelled (e.g., if the workflow itself was cancelled while failover-hold was sleeping). !cancelled() allows the job to run when upstreams complete, fail, or skip — but not when the workflow is cancelled. Combined with github.run_attempt == 1, no production-execution can occur on a rerun or a cancelled-then-restarted run.
The condition also correctly handles the non-failover case:
(inputs.mode != 'failover' || needs.failover-hold.result == 'success')When mode != 'failover', failover-hold is skipped (result == 'skipped'); inputs.mode != 'failover' short-circuits the check to true. When mode == 'failover', only a successful hold allows execution. ✓
repositories: ci-runner-canary on both observer token mints: The create-github-app-token action's repositories parameter scopes repo-level permissions to the named repository. For org-level permissions (organization-self-hosted-runners: read), the scope is already org-wide by the permission grant; the repositories parameter narrows what the token can do for repo-level operations. This is a correct tightening that limits the token's blast radius. ✓
3. feat: pin production HA proof caller + fix: repin production HA proof caller — template and contract tests
Template caller (templates/ci-runner-canary/.github/workflows/production-ha-proof.yml):
- Pins at
fdf0e6e5905e50be8e332f9bc522e24947a61b13(the hardened commit) — not the initial4e2fe88. Correct: the published template must pin to the hardened implementation, not the pre-hardening version. - No runner-label or prefix inputs in the template — the reusable workflow owns fleet details. ✓
fromJSON(inputs.drain-wait-minutes)correctly casts thechoicestring input to number before passing it to thenumber-typed reusable input. ✓- No
secrets: inherit— explicitobserver-private-keyonly. ✓
Contract tests (production-ha-proof.test.cjs):
implementationShais correctly set tofdf0e6e(the hardened commit) — the test verifies the template pins this exact SHA and that the SHA's workflow content includes the rerun guards,REF_PROTECTEDcheck, andrepositories: ci-runner-canaryscoping."every routed or evidence-producing job independently rejects reruns"— assertsgithub.run_attempt == 1appears in theif:header of all 5 substantive jobs."production execution is cancellation-aware while allowing a skipped hold"— asserts!cancelled()and notalways(), and verifies the correct failover-hold exception clause."observer secret is explicit and every API contract remains read-only"— assertsrepositories: ci-runner-canaryappears exactly twice, no write permissions, noPOST/PUT/PATCH/DELETEpatterns, nocancelorrerunpaths, nogh api/gh runcalls."pinned reusable commit contains the exact caller and selector contracts"— runsgit show fdf0e6e:.github/workflows/production-ha-proof.ymland asserts the caller workflow ref, selector SHA,REF_PROTECTEDenv binding,!cancelled()condition, andrepositories: ci-runner-canarycount are all present in the pinned content.
What Works Well
Failure-closed evidence path: Every failure branch writes a structured {status: "failed", failure: {code, message}} evidence artifact. An operator reviewing evidence always sees why the proof failed, not just a missing artifact.
Cross-group runner ID deduplication (seenRunnerIds): A runner that registers itself in both production groups (e.g., after a controller misconfiguration) is caught before the proof can report passing topology. This is the correct guard for the invariant that ci-local-melo-desk-001 and ci-local-melo-lap-001 must be genuinely independent capacity.
Three-layer rerun rejection: (1) YAML if: github.run_attempt == 1 on every substantive job; (2) bash [[ "$RUN_ATTEMPT" = 1 ]] in preflight (before any token is minted); (3) JS requiredEnvironment(env, "RUN_ATTEMPT", /^1$/u) in commonEvidence. Each layer independently prevents a rerun from producing evidence that could be mistaken for a first-attempt production proof.
Limitations explicitly recorded in evidence: The limitations field documents restGroupIdIsScaleSetId: false, scaleSetIdObserved: false, controllerCapacityObserved: false, and batteryOrLogonStateObserved: false. An operator reading the artifact cannot mistake REST group IDs for controller scale-set IDs, and cannot treat the workflow artifact as a substitute for physical host evidence.
Immutable canary chain consistency: local-runner-canary.yml pins the selector at 66e3e97; production-ha-proof.yml pins the same selector SHA; the test for production-ha-proof.yml verifies the pinned content contains that exact SHA. Both acceptance paths use the same reviewed selector revision.
Summary
All six prior findings are closed: findings #2 and #3 were resolved by code changes in the previous batch; findings #1 and #5 were correctly disputed by kyle-sexton (the earlier review was wrong about ubuntu-slim being a non-standard label); findings #4 and #6 are validly deferred. No new findings from this batch.
The production HA proof is the correct capstone for the local runner rollout: it adds a separate two-host acceptance proof that is failure-closed, observer-read-only, rerun-hardened, and evidence-emitting. The fdf0e6e rerun hardening commit is the correct implementation SHA to publish — it closes gaps in the initial implementation before any external caller pins the workflow. The template correctly pins to the hardened commit, not the initial one. Test coverage of the new invariants (rerun guards, cancellation awareness, observer scope, immutable content at the published SHA) is comprehensive.
Pass the governed cache epoch explicitly through the private canary seed and fail closed when it is missing.
|
Claude finished @kyle-sexton's task in 1m 46s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml |
Closed — rebuttal accepted; ubuntu-slim is a standard GitHub-hosted runner |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry |
Resolved |
| 3 | runAttempt !== 1 redundancy note |
Resolved |
| 4 | GitHub API version "2026-03-10" untracked for drift |
Deferred correctly |
| 5 | Canary SHA pin comment format | Closed — rebuttal accepted; first-party reviewed pin intentionally not tagged |
| 6 | safe_path allows // |
Deferred correctly |
Summary
fix: wire governed canary cache epoch is a small, correct fix that closes a real governance gap: without it, the template caller always used the hardcoded default v1 epoch, making IaC's CI_CACHE_EPOCH variable a no-op and preventing governed cache invalidation. The fix is three lines, all three files are consistent with each other (template wires the variable; test enforces the wiring; README documents the requirement), and the fail-closed behavior on an unset variable is already guaranteed by the existing preflight regex. No new issues are introduced.
Install and checksum-pin the same ShellCheck release for shared Actionlint execution on hosted and local workers, with runtime parity proof.
|
Claude finished @kyle-sexton's task in 3m 3s —— View job Code Review — feat: add governed local runner routing (updated at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml |
Closed — rebuttal accepted; standard GitHub-hosted runner |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry |
Resolved |
| 3 | runAttempt !== 1 redundancy note |
Resolved |
| 4 | GitHub API version "2026-03-10" untracked for drift |
Deferred correctly |
| 5 | Canary SHA pin comment format | Closed — rebuttal accepted; first-party reviewed pin |
| 6 | safe_path allows // |
Deferred correctly |
No new findings from this commit.
What Works Well
Three-layer enforcement chain: (1) actionlint-shellcheck-pin.test.cjs asserts version and checksum parity between the two action files at the source level; (2) the CI proof step asserts the running binary matches the pinned version; (3) the proof step asserts actionlint actually invokes ShellCheck by running it against a known-bad fixture and verifying the expected SC2086 finding appears. Each layer independently catches a different failure mode: source drift, install drift, and integration drift.
Drift check is not double-registered: The manifest comment is explicit about the choice to rely on the parity test rather than adding a second drift entry. An operator reading the drift manifest knows to look at the test, not search for a missing second entry.
Fixture is placement-safe: The intentional-bad fixture in .github/actions/actionlint/fixtures/ is not auto-discovered by actionlint's normal CI run, so it cannot pollute normal lint results. Only the proof step explicitly passes it as an argument.
Summary
fix: pin ShellCheck for Actionlint parity closes a real gap: the previous setup silently depended on the runner image's ambient ShellCheck, which is absent on a self-hosted worker. The fix installs a checksum-pinned ShellCheck at the same version as the standalone ShellCheck action, making embedded run: script linting deterministic across both runner types. The three-layer enforcement chain (parity test, version check, fixture proof) is comprehensive and correctly structured. No new issues are introduced.
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_413ef6b8-6abb-48b9-ad20-6a941e3b3cbb) |
|
Claude finished @kyle-sexton's task in 4m 45s —— View job Code Review — feat: add governed local runner routing (final review at
|
| # | Finding | Status |
|---|---|---|
| 1 | ubuntu-slim runner label in select-runner.yml:64 |
Closed — rebuttal accepted; prior review confirmed this is a standard GitHub-hosted runner with hypervisor isolation |
| 2 | configuredCandidateLabels / parseCandidateLabels asymmetry |
Resolved — explicit invariant comment added |
| 3 | runAttempt !== 1 redundancy note |
Resolved — comment added, parameterized tests cover edge values |
| 4 | GitHub API version "2026-03-10" untracked for drift |
Deferred correctly — current supported version; retirement still fails hosted |
| 5 | Canary SHA pin comment format | Closed — rebuttal accepted; intentional first-party reviewed pin, not Dependabot-managed |
| 6 | safe_path allows // |
Deferred correctly — POSIX collapse plus hash validation means no exploit path |
CLAUDE.md Compliance Check
pull_requestonly; neverpull_request_targetorworkflow_runwith secrets. ✅ All five new workflows (select-runner.yml,local-runner-canary.yml,production-ha-proof.yml,selector-conformance.yml,pulumi-version-drift-check.yml) use onlyworkflow_callorpull_request. Theclaude-review.ymltripwire step for privileged triggers is unchanged.- SHA-pin both layers. ✅ All
uses:references carry a 40-character SHA with a version comment. First-party reusable workflow pins carry a descriptive comment explaining which reviewed revision is pinned. - Least privilege, owned by the caller. ✅
permissions: {}at the workflow level with job-level minimums. Observer token scoped toci-runner-canaryonly. Nosecrets: inherit. - No untrusted checkout before the action. ✅ No
actions/checkoutsteps precede the App token mint step in any new workflow. - Public-repo log hygiene. ✅
display_reportandshow_full_outputnot modified. Evidence artifacts explicitly exclude tokens and raw API responses.
Overall Assessment
The PR is well-structured, security-conscious, and consistent throughout. The fail-closed model is uniformly applied: every error path (auth failure, API timeout, invalid response, missing config, public repo, fork, Dependabot, rerun) produces a hosted-runner result. The label contamination model, ephemeral field handling, and OS namespace enforcement close the gaps that existed in earlier iterations. The production HA proof provides a concrete, evidence-emitting acceptance gate before any production traffic is routed locally. Test coverage across the selector, canary, and HA proof is comprehensive.
The two deferred items (API version drift tracking, safe_path double-slash) carry no security relevance and are correctly flagged for future maintenance rather than blocking merge.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Risk: high. Cursor Bugbot did not complete successfully (skipped: usage limit reached), so automated review is unavailable for this security-critical CI change. Human review is required before merge; no reviewers were assigned because no non-author maintainers were identifiable in this repository.
Sent by Cursor Approval Agent: Pull Request Router and Approver
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fa4a44c18
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| [[ "$EVENT_NAME" = workflow_dispatch ]] || fail "caller must use workflow_dispatch" | ||
| [[ "$REPOSITORY_PRIVATE" = true ]] || fail "canary caller must be private" | ||
| [[ "$REPOSITORY" = "$EXPECTED_REPOSITORY" ]] || fail "unexpected canary repository" | ||
| [[ "$SOURCE_REF" = refs/heads/main ]] || fail "canary must run from protected main" |
There was a problem hiding this comment.
When melodic-software/ci-runner-canary dispatches from an unprotected or accidentally unprotected main, this guard still passes because it only checks the ref string and never verifies github.ref_protected (unlike the production proof preflight). That lets the workflow mint the observer token and generate acceptance evidence from a branch that no longer has the protected-main control the workflow and docs require; add a REF_PROTECTED: ${{ github.ref_protected }} check here before secrets are used.
Useful? React with 👍 / 👎.
## Summary - add a centralized GitHub Actions runner policy that enforces approved selector routing, public/hosted boundaries, explicit read-only permissions, cancellation-safe literal hosted fallback, and machine-readable hosted exceptions - derive the required literal fallback from the governed policy default, so changing an approved hosted image is a policy/configuration change rather than parser-code surgery - route Standards' 28 eligible private Linux workloads through the governed selector while retaining exact hosted exceptions for policy and control-plane boundaries - recursively validate repository-local reusable workflows and caller/callee permission narrowing without allowing arbitrary secrets, tokens, inputs, labels, or runner expressions - distribute the locked policy runtime and Node version to six enrolled private consumers from one deterministic manifest - harden staged .NET formatting and PSScriptAnalyzer adapters with deterministic cross-platform path semantics and per-target no-profile PowerShell isolation - keep the complete .NET-format named job managed by Standards while each consumer owns only strict data in `.lefthook/dotnet-format.json` - preserve executable source and consumer index modes through distribution - pin every production selector/reusable and Actionlint parity reference to merged `ci-workflows/main` commit `99ac2f8c5b09dbb785d4eaf18465cbd96c30290c` ## Dependencies The final routing contract is the immutable squash merge from melodic-software/ci-workflows#74 (including stacked #76/#77): - `99ac2f8c5b09dbb785d4eaf18465cbd96c30290c` ## Reviewed head `0795d22c89cb8fae11642ede9757e7b43fd5d546` ## Validation Independent author, reviewer, recheck, and integration-review gates all PASS with no findings. - runner-policy adversarial suite: 83/83, including alternate configured hosted-default proof - Standards private self-audit: PASS - .NET/Lefthook adapter: 12/12 - pinned Lefthook 2.1.9 validate, dump, and actual job execution: PASS - independent argv probe: spaces, semicolons, and `$()` remain inert data with `shell:false` - production distribution suite: 114/114 under checksum-pinned yq 4.53.3 in author native Linux and hosted Linux - independent reviewer inspected the exact-head hosted log and confirmed assertions 1 through 114 - exact executable-bit gate: PASS; both source CLIs are index mode `100755` - routing graph: 28 selectors, 28 workloads, 31 actual `ci-status` gates, zero selector gates - final pin proof: exactly 46 merged-main references, zero stale full/short feature-stack references, and 25 preserved transitional compatibility references - all eight changed files reconstruct byte-for-byte from only the two intended SHA/comment substitutions - Actionlint 1.7.12 plus hosted checksum-verified ShellCheck 0.11.0: PASS - all 10 uniquely referenced workflow/action paths exist at the immutable ci-workflows commit - six workflow schemas, Zizmor medium/high, Biome, Markdown, ShellCheck, Gitleaks, full Lefthook, and diff checks: PASS - signed final pin commit: `0795d22c89cb8fae11642ede9757e7b43fd5d546` All 63 hosted checks pass on this exact head. ## Authoritative basis - GitHub Actions workflow syntax and runner routing: https://docs.github.com/en/actions/reference/workflows-and-actions/workflow-syntax - Node cross-platform path semantics: https://nodejs.org/api/path.html#pathwin32 - Lefthook v2.1.9 named-job merge contract: https://github.com/evilmartians/lefthook/blob/v2.1.9/docs/configuration/jobs.md - Lefthook v2.1.9 job templates: https://github.com/evilmartians/lefthook/blob/v2.1.9/docs/configuration/templates.md ## Rollout safety This PR does not change GitHub variables, secrets, runners, repository settings, or live infrastructure. Production routing remains hosted until the IaC and physical canary gates are applied later. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **High Risk** > Large CI workflow refactor with secrets/vars for runner selection and a new security gate; misconfiguration could break merges or route jobs incorrectly until fleet is live. > > **Overview** > Introduces a **YAML-aware runner policy** (`components/runner-policy`, `.github/runner-policy.json`, `policy.json`) and a hosted **Runner policy** CI lane that tests and enforces it against workflow inventory and repository visibility. > > **CI routing** shifts eligible lint/contract jobs from fixed `ubuntu-latest` to paired `select-runner` + workload jobs using `needs.select-*.outputs.runner || 'ubuntu-24.04'`, `if: ${{ !cancelled() }}`, and `merge_group` support. Control-plane jobs (runner-policy gate, ci-status, zizmor, osv-scanner) stay on explicit hosted runners with documented exceptions. `ci-status` now requires `runner-policy`, treats only `success` as pass (not `skipped`), and pins several workflows to `ci-workflows@99ac2f8`. > > **Local hooks:** Lefthook .NET formatting moves to a consumer-owned `.lefthook/dotnet-format.json` and `dotnet-format-staged.mjs` (shell-less `dotnet format whitespace`). PSScriptAnalyzer staged checks run **one target per fresh `pwsh` worker**; `PSUseCorrectCasing` is removed from settings. Dependabot gains an npm root for `components/runner-policy`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 0795d22. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 participant
Summary
ubuntu-24.04hosted fallback and enforce the governed Linux/JIT inventory namespaceci-runner-canary, and require protectedmainWhy
This is the shared routing contract for the ephemeral local fleet. Eligible private Linux jobs can use idle local capacity while reruns, unsafe events, missing capacity, missing credentials, namespace conflicts, malformed inventory, and every selector/API failure route hosted.
Immutable proof chain
3fa4a44c1883f72be8050995109b0969dd4a627cfdf0e6e5905e50be8e332f9bc522e24947a61b1368b9f40aa65978c180e6a51642fa7f56ad8a6f453fa4a44c1883f72be8050995109b0969dd4a627cmainin the privatemelodic-software/ci-runner-canaryrepositoryValidation
git diff --check: PASSAll 33 hosted checks pass on this exact head. A post-push online Zizmor audit of the two HA-proof workflows reports no findings.
Rollout
Production routing remains
hosted-onlythrough controller publication, App/IaC bootstrap, physical canary, and two-host proof. Downstream consumers will pin the immutable squash-merge commit frommain; they will not pin this feature-branch head.Note
High Risk
Changes authentication/OIDC policy matching, production runner routing, and multiple security-critical CI gates across a very large surface; misconfiguration could block deploys or route untrusted work locally.
Overview
Adds the shared CI routing and proof stack for ephemeral self-hosted runners: a reusable select-runner workflow (
prefer-self-hosted/hosted-only) that fail-closes to reviewedubuntu-24.04, validates managed runner namespaces, and refuses local routing for public repos, forks, Dependabot, reruns, and API/auth failures.Immutable private canary (
local-runner-canary) delegates selection to the pinned selector, asserts managed runner identity on every job, and runs a shared parity script (LFS, toolchains, sudo/cert trust, Native AOT) plus bidirectional artifact/cache proofs and optional long-run/cancellation modes. Production HA proof is a separate reusable workflow with read-only GitHub inventory, hosted failover hold after desktop drain, and laptop execution assertions—backed byproduction-ha-proof.cjsembedded via render scripts.Pulumi deployment guard composite action compares live OIDC personal allow policies to bundled v2 contracts (immutable subjects, exact claims, no wildcards) and inventories operational stack URNs before apply. Pulumi CLI drift reusable workflow opens/ages maintenance issues and hard-fails after 14 days.
OSV scanning moves to a digest-pinned container contract with
osv-scan-guard.shgoverning empty scans and SARIF upload safety. Actionlint now installs checksum-pinned ShellCheck (lockstep with the standalone action). PSScriptAnalyzer runs once per file with exact version import and no #1708 retry subprocesses.Workflow policy tests enforce fixed GA runner labels (no
*-latest), bansecrets: inheriton reusables, and add broad contract tests for pins, templates, and generated workflow drift.Reviewed by Cursor Bugbot for commit 3fa4a44. Bugbot is set up for automated code reviews on this repo. Configure here.