ci: fork-safe benchmarks via image artifacts (replaces #113)#115
Merged
Conversation
…)" This reverts commit ad4353a. #113's trusted `workflow_run` executor checked out and built the PR's solver code via `actions/checkout` with `ref: <PR head SHA>`. In a `workflow_run` context actions/checkout@v6 refuses that by default (the "pwn request" guard), so the build job fails for every solver-changing PR — main is broken. Opting in (`allow-unsafe-pr-checkout: true`) would run fork-authored build_base.sh / Dockerfile with the base repo's packages:write token, which is the exact vulnerability the guard prevents. Revert to the working pre-#113 flow; a fork-safe redesign that never executes fork code in the trusted context follows separately. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Rework the benchmark image flow so fork PRs work without ever pushing to or privately reading from GHCR, and without executing fork code in a trusted context (the pitfall that broke #113): - build-tesseracts.yml: on the PR path (workflow_call) the built image is `docker save`d and uploaded as a `solver-image-<name>` artifact instead of pushed to GHCR. The push path is retained only for push-to-main / dispatch, which publishes the baseline `:latest` / `:<sha>` images. GHCR login is skipped on the PR path (no auth needed to save an artifact). - benchmark-run.yml: the run job downloads the `solver-image-*` artifacts and `docker load`s them back to `<name>:latest` — the exact local tag `mosaic run --no-build` resolves — then pulls only the *unchanged* solvers from GHCR at the base SHA. GHCR is public-read, so this works from fork PRs; login is best-effort (rate limits only). - pull-solver-images.py: add --skip-images so the registry pull skips the changed solvers already loaded from artifacts (they were never published at the base SHA). Drop the now-unused --tag/image_tag plumbing. Everything stays on the pull_request trigger. The build runs fork code in the untrusted context (read-only token, no secrets) — harmless — and the run only consumes images, so no trusted workflow_run, commit-status gating, or handoff is needed. bench-ok remains a normal job-based gate; docs RTD/comment continue via the existing docs-publish.yml workflow_run. Requires the mosaic/* GHCR packages to be public-read (one-time org setting) so fork PRs can pull base images and warm the build cache-from. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The build and benchmarks jobs call reusable workflows that reference secrets.GITHUB_TOKEN (docker/login-action). Dropping `secrets: inherit` left that reference unsatisfied, so GitHub rejected the whole run at graph-resolution time (startup_failure, no jobs, no logs). Restore `secrets: inherit` on both calls — matching the working baseline. The earlier secret-minimization was defense-in-depth for the abandoned trusted-context design and isn't needed here (no fork code runs with elevated scope in this flow). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Two errors made every Benchmark run on this branch fail at graph resolution (startup_failure, no jobs, no logs): 1. The `build` caller job dropped `packages: write`, but the reusable build-tesseracts.yml declares `packages: write` at the workflow level. A reusable workflow can't be granted less than it declares, so the run was rejected before starting. Restore `packages: write` on the caller (unused on the PR path, which never pushes, but required to satisfy the contract). 2. The publish-vs-artifact split gated on `github.event_name != 'workflow_call'`, but inside a called workflow `github.event_name` is the caller's event (pull_request / push), never 'workflow_call' — so the gate was always true and PRs would have tried to push. Gate on `inputs.matrix` instead: it is set only when benchmark.yml calls this for a PR, and empty on the push-to-main / dispatch publish path. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
Note on the red
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
#113 broke
main: its trustedworkflow_runexecutor checked out the PR's fork code (ref: <head SHA>) to build it, butactions/checkout@v6refuses a fork PR checkout in aworkflow_runcontext by default (the "pwn request" guard), so the build job failed for every solver-changing PR. Opting in (allow-unsafe-pr-checkout: true) would run fork-authoredbuild_base.sh/Dockerfile with the base repo'spackages: writetoken — exactly the vulnerability the guard exists to prevent.This PR reverts #113 and replaces it with a much smaller, fork-safe flow that never executes fork code in a trusted context and never requires a fork to push to (or privately read from) GHCR.
How it works now
Everything stays on the
pull_requesttrigger:docker saves it, and uploads asolver-image-<name>artifact instead of pushing to GHCR. The GHCR push path is retained only for push-to-main / dispatch (which publishes the baseline:latest/:<sha>images).docker loads those artifacts back to<name>:latest— the exact local tagmosaic run --no-buildresolves — then pulls only the unchanged solvers from public GHCR at the PR's base SHA, skipping the loaded ones (pull-solver-images.py --skip-images).bench-okstays a normal job-based merge gate (no branch-protection change). Docs RTD/preview + PR comment continue via the existingdocs-publish.yml(workflow_run).Fork code only ever runs in the untrusted
pull_requestcontext (read-only token, no secrets) — the build produces an artifact, the run only consumes images — so no trustedworkflow_run, commit-status gating, or handoff is needed.Required setup (done)
The
mosaic/*GHCR packages must be public-read so fork PRs can pull base images and warm the buildcache-from. This has been flipped to public. Write stays maintainer-only (unchanged).Verification
actionlintclean (only pre-existing legacy shellcheck/head.refnits, none in new code).docker save <name>:latest→docker loadreproduces exactly<name>:latest.config.py/runner.pyand found no correctness bugs.workflow_run/artifact behavior can only be fully exercised once onmain; after merge, re-run a fork PR (e.g. perf: upgrade INS.jl tesseract to v5.0.0 (native ν gradient, in-place forward, float32) #103) to confirm end-to-end.🤖 Generated with Claude Code