Skip to content

ci: run mirror verification on the self-hosted runners - #11

Closed
thedancingdeveloper wants to merge 1 commit into
mainfrom
ci/self-hosted-mirror-verify
Closed

ci: run mirror verification on the self-hosted runners#11
thedancingdeveloper wants to merge 1 commit into
mainfrom
ci/self-hosted-mirror-verify

Conversation

@thedancingdeveloper

Copy link
Copy Markdown
Contributor

cadastre spent 326 GitHub-hosted minutes in August, second only to FarmEggs, in breach of docs/CI-RUNNER-GATES.md. This moves the part that can move and formally excepts the part that should not.

What changes

publish.yml / verify[self-hosted, node-b, linux, x64].

That workflow triggers only on push: branches: [main] and workflow_dispatch. It never builds unreviewed fork code, so the security argument written at the top of ci.yaml does not reach it. It is pure uv + Python — astral-sh/setup-uv brings its own interpreter, so it needs nothing the runner image lacks.

What deliberately does not change

ci.yaml stays on ubuntu-latest. Its header already states the reason, and it is correct:

This is a public repository, so pull_request builds untrusted fork code: putting that on the self-hosted pool would hand an arbitrary contributor code execution inside the estate.

The self-hosted workers are persistent, run as root inside their container, and sit on the tailnet with reachable Forgejo, Infisical, Komodo and registry endpoints. Org fork-PR approval is first_time_contributors, so a returning contributor's PR would execute without approval (gap 12 in ops/docs/github-actions-runner-gaps.md).

Instead of a silent breach, it is now an owner-approved, time-bounded entry in github-policy/runner-exceptions.json that the daily expiry check will surface.

Safety

verify is not a required status check on main — those are install, test (3.11)test (3.14) and full-stack e2e, all in ci.yaml and all untouched. The workflow name Mirror verification (main) is preserved, per the warning in its own header.

🤖 Generated with Claude Code

publish.yml triggers only on push to main and workflow_dispatch, so it never
builds unreviewed fork code. The fork-PR argument recorded at the top of
ci.yaml is specific to pull_request and does not reach this file, which means
there was never a reason for this job to be hosted.

ci.yaml is deliberately left alone and is covered by a time-bounded entry in
github-policy/runner-exceptions.json instead: it does build fork PRs, and the
self-hosted pool is persistent and runs as root inside the tailnet.

`verify` is not one of main's required status checks (those are install,
test 3.11-3.14 and full-stack e2e), so this cannot make main unmergeable.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@thedancingdeveloper

Copy link
Copy Markdown
Contributor Author

Closing: this conflicts with a tested invariant in this repository, and that invariant is the better-reasoned rule.

tests/test_release_workflow.py draws the line at tag-gated, not at not-fork-reachable:

# Only a tag-gated workflow may take the trusted self-hosted pool. Everything
# else is reachable from a fork's pull request.
SELF_HOSTED_WORKFLOWS = frozenset({"release-images.yml", "release-pypi.yml"})

and test_self_hosted_workflows_are_all_tag_gated additionally asserts every entry on that allowlist is reachable only from refs/tags/v*. Moving publish.yml would have required weakening both tests.

My reasoning in the PR body was narrowly correct — a push to main needs write access, so publish.yml has no fork exposure — but it argued past the actual rule, which deliberately keeps the self-hosted surface as small as possible rather than merely fork-free. test (3.11)test (3.14) caught it, which is the invariant doing its job.

The org-side resolution is to widen the exception instead: runner-exceptions.json now covers cadastre/publish.yml as well as ci.yaml, recording that this repository's hosted usage is a deliberate, tested security posture rather than drift. See TheDancingDeveloper-org/github-policy#5.

@thedancingdeveloper
thedancingdeveloper deleted the ci/self-hosted-mirror-verify branch August 17, 2026 03:55
thedancingdeveloper pushed a commit to TheDancingDeveloper-org/github-policy that referenced this pull request Aug 17, 2026
cadastre enforces a stricter rule than this one, in its own test suite, and
that rule is the better-reasoned of the two.

tests/test_release_workflow.py pins SELF_HOSTED_WORKFLOWS to the two tag-gated
release workflows and asserts every other job is not self-hosted;
test_self_hosted_workflows_are_all_tag_gated then asserts everything on that
allowlist is reachable only from refs/tags/v*. So the line there is drawn at
tag-gated, not at not-fork-reachable.

TheDancingDeveloper-org/cadastre#11 tried to move publish.yml on the grounds
that push-to-main needs write access and therefore has no fork exposure. That
is true and beside the point: the invariant deliberately keeps the self-hosted
surface as small as possible. Its own test matrix caught the change, and the
PR is closed.

Recording it as an exception is the honest outcome -- cadastre's hosted usage
is a deliberate, tested security posture, not drift, and the expiry keeps it
under review.

Verified: all 51 organization repositories now pass, with FarmEggs#2 applied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thedancingdeveloper added a commit to TheDancingDeveloper-org/github-policy that referenced this pull request Aug 24, 2026
Two problems, one cause. The gate carried an inlined copy of the checker
because a private policy repository cannot be checked out by a public caller,
and the exceptions file was consumed by nothing but an expiry loop -- so
`runner-exceptions.json` could record an exception but never grant one. The
documented workaround was to drop the required status check on the repository
entirely, which trades a narrow, expiring, reviewable exception for no gate
at all.

Making this repository public removes the constraint. scripts/runner_policy.py
is now the only copy of the rule; the reusable gate checks this repository out
alongside the caller and runs it.

Exceptions are keyed by repo and workflow file and may be narrowed to named
jobs, which matters immediately: FarmEggs' ci.yml holds two compliant jobs and
two that cannot move, and a file-wide exception there would also hide a future
regression in the compliant pair. An expired entry still fails the gate and
suppresses nothing.

Three entries recorded, all with a reason and an expiry:
  cadastre/ci.yaml       -- builds fork PRs on a public repo
  FarmEggs/ci.yml mobile -- needs a JDK and the Android SDK
  FarmEggs/ci.yml web    -- needs google-chrome and npm

Verified against all 50 organization repositories: with these entries applied,
the only failures are the jobs that TheDancingDeveloper-org/FarmEggs#2 and
TheDancingDeveloper-org/cadastre#11 move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thedancingdeveloper added a commit to TheDancingDeveloper-org/github-policy that referenced this pull request Aug 24, 2026
cadastre enforces a stricter rule than this one, in its own test suite, and
that rule is the better-reasoned of the two.

tests/test_release_workflow.py pins SELF_HOSTED_WORKFLOWS to the two tag-gated
release workflows and asserts every other job is not self-hosted;
test_self_hosted_workflows_are_all_tag_gated then asserts everything on that
allowlist is reachable only from refs/tags/v*. So the line there is drawn at
tag-gated, not at not-fork-reachable.

TheDancingDeveloper-org/cadastre#11 tried to move publish.yml on the grounds
that push-to-main needs write access and therefore has no fork exposure. That
is true and beside the point: the invariant deliberately keeps the self-hosted
surface as small as possible. Its own test matrix caught the change, and the
PR is closed.

Recording it as an exception is the honest outcome -- cadastre's hosted usage
is a deliberate, tested security posture, not drift, and the expiry keeps it
under review.

Verified: all 51 organization repositories now pass, with FarmEggs#2 applied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
thedancingdeveloper added a commit to TheDancingDeveloper-org/github-policy that referenced this pull request Aug 24, 2026
* policy: one checker, and exceptions that actually suppress

Two problems, one cause. The gate carried an inlined copy of the checker
because a private policy repository cannot be checked out by a public caller,
and the exceptions file was consumed by nothing but an expiry loop -- so
`runner-exceptions.json` could record an exception but never grant one. The
documented workaround was to drop the required status check on the repository
entirely, which trades a narrow, expiring, reviewable exception for no gate
at all.

Making this repository public removes the constraint. scripts/runner_policy.py
is now the only copy of the rule; the reusable gate checks this repository out
alongside the caller and runs it.

Exceptions are keyed by repo and workflow file and may be narrowed to named
jobs, which matters immediately: FarmEggs' ci.yml holds two compliant jobs and
two that cannot move, and a file-wide exception there would also hide a future
regression in the compliant pair. An expired entry still fails the gate and
suppresses nothing.

Three entries recorded, all with a reason and an expiry:
  cadastre/ci.yaml       -- builds fork PRs on a public repo
  FarmEggs/ci.yml mobile -- needs a JDK and the Android SDK
  FarmEggs/ci.yml web    -- needs google-chrome and npm

Verified against all 50 organization repositories: with these entries applied,
the only failures are the jobs that TheDancingDeveloper-org/FarmEggs#2 and
TheDancingDeveloper-org/cadastre#11 move.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

* policy: except cadastre/publish.yml too

cadastre enforces a stricter rule than this one, in its own test suite, and
that rule is the better-reasoned of the two.

tests/test_release_workflow.py pins SELF_HOSTED_WORKFLOWS to the two tag-gated
release workflows and asserts every other job is not self-hosted;
test_self_hosted_workflows_are_all_tag_gated then asserts everything on that
allowlist is reachable only from refs/tags/v*. So the line there is drawn at
tag-gated, not at not-fork-reachable.

TheDancingDeveloper-org/cadastre#11 tried to move publish.yml on the grounds
that push-to-main needs write access and therefore has no fork exposure. That
is true and beside the point: the invariant deliberately keeps the self-hosted
surface as small as possible. Its own test matrix caught the change, and the
PR is closed.

Recording it as an exception is the honest outcome -- cadastre's hosted usage
is a deliberate, tested security posture, not drift, and the expiry keeps it
under review.

Verified: all 51 organization repositories now pass, with FarmEggs#2 applied.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: thedancingdeveloper <306930456+thedancingdeveloper@users.noreply.github.com>
Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant