Harden reusable workflows and introduce v1 versioning#88
Open
Harden reusable workflows and introduce v1 versioning#88
Conversation
Restructures the IntegrationTest, IntegrationTestRequest, and FormatCheck reusable workflows to address security risks discussed in https://discourse.julialang.org/t/reviewing-github-actions-workflows-tokens/136400, and introduces a versioning convention so callers can pin to `@v1` instead of `@main`. IntegrationTest.yml: matrix expansion and aggregation gate move inside the reusable; the input changes from a scalar `pkg` to a JSON-array `pkgs`. A pre-leg step probes URL-based matrix entries anonymously and skips them on fork PRs that lack authentication (closing the pattern where PR-controlled Julia ran with `INTEGRATIONTEST_PAT` in scope under `pull_request_target`). The aggregate `IntegrationTest` check fails on fork-PR matrices that all-skip on private deps, with a `/integrationtest <pkg>` hint. IntegrationTestRequest.yml: comment-body parsing moves from shell interpolation to an `env:` mapping; on a successful manual `/integrationtest` run, posts a passing `IntegrationTest` check via the GitHub Checks API so the run can clear the gate above. FormatCheck.yml + new FormatCheckPostback.yml: split into a parse phase (runs on `pull_request:`, no secrets, uploads diff artifact) and a postback phase (runs on `workflow_run:`, in the trusted base context, downloads artifact, posts/updates the format-suggestion comment). Branch protection should require the parse phase's check. Tests, Documentation, CompatHelper, LiterateCheck, Registrator, FormatPullRequest: `${{ inputs.* }}` interpolations into shell `run:` blocks moved to `env:` mappings, and explicit per-job `permissions:` blocks added. `julia-actions/julia-buildpkg@latest` pinned to `@v1` in IntegrationTest.yml. New `.scripts/release.sh` tags `vX.Y.Z` (annotated, immutable) and maintains a `vX` mutable major tag, rewriting internal `@main` references in the released commit so the tagged release is fully snapshot-stable. README: new top-level Versioning section; FormatCheck section rewritten to document the parse + postback split. Breaking change: the IntegrationTest workflow's input shape changes from `pkg:` (scalar) to `pkgs:` (JSON array). Per-repo callers will need to update to the new form and to `@v1` pinning; that work is staged as a follow-up sweep across the ecosystem repos. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Restructures the
IntegrationTest,IntegrationTestRequest, andFormatCheckreusable workflows to address security risks discussed in the Discourse thread on reviewing GitHub Actions workflows / tokens, and introduces a versioning convention so callers can pin to@v1instead of@main.IntegrationTest
pkg:(scalar) topkgs:(JSON array). Per-repo callers shrink considerably.git ls-remote(credentials disabled). Entries reachable without authentication run as before. Entries that need authentication run on push, internal PRs,pull_request_target,issue_comment, andworkflow_dispatchevents; on forkpull_request:events they skip with a/integrationtest <url>notice.gatejob (namedIntegrationTestto match the existing branch-protection check) aggregates the matrix result. It explicitly fails when a fork PR's matrix all-skipped because every entry needed authentication, prompting the maintainer to use/integrationtest. Emptypkgs: '[]'passes through (no-downstream case).pull_request_target:trigger at the caller level is no longer needed; per-repo callers should usepull_request:once they migrate.IntegrationTestRequest
\${{ github.event.comment.body }}shell interpolation to anenv:mapping (closes a script-injection-shape pattern).pkgs: '[\"...\"]'array for the newIntegrationTestinterface.IntegrationTestcheck run via the GitHub Checks API. This satisfies branch protection on fork PRs whose private-only matrix would otherwise leave theIntegrationTestgate failing.FormatCheck (split into parse + postback)
FormatCheck.yml(parse phase): runs onpull_request:, no secrets in scope, runs the formatter on PR head, uploads the diff and PR metadata as aformat-checkartifact, propagates the formatter exit code as the check status.FormatCheckPostback.yml(new file, postback phase): runs onworkflow_run:after aFormat Checkrun completes, in the trusted base-repo context with secrets, downloads the artifact, and posts or updates the format-suggestion comment.Format Check / Check Formatting); the postback exists only to update the comment.Hygiene across the rest of the reusables
\${{ inputs.* }}interpolations into shellrun:blocks moved toenv:mappings inTests.yml,Documentation.yml,CompatHelper.yml,LiterateCheck.yml,Registrator.yml, andFormatPullRequest.yml.permissions:blocks added to every reusable that lacked one.julia-actions/julia-buildpkg@latestinIntegrationTest.ymlpinned to@v1.Versioning
Versioningsection in the README documents the convention (vMAJOR.MINOR.PATCHannotated tag +vMAJORmutable tag)..scripts/release.shcuts a release: rewrites internal@mainreferences in the released commit to a fixed SHA so the tagged release is snapshot-stable, then tags the rewrite commit. Doesn't push automatically.Breaking change
The
IntegrationTestreusable's input shape changes frompkg:(scalar) topkgs:(JSON array). Per-repo callers need to update to the new form. That sweep across ecosystem repos is staged as a follow-up.Test plan
SparseArraysBase.jl) pointing itsIntegrationTest.ymlat this branch's SHA. Verify the matrix runs and the aggregate check passes on an internal PR.GaussianFermions.jl) pointing at this branch's SHA. Verify on an internal PR that the URL probe routes through the auth path and the integration test runs to completion. Verify on a fork PR that the URL entry skips with the/integrationtestnotice and that the aggregateIntegrationTestcheck fails accordingly./integrationtest https://github.com/ITensor/Tennis.jlas a maintainer; verify the manual run completes and theIntegrationTestcheck flips to passing via the Checks API post.FormatCheckparse phase emits its check onpull_request:events from forks with no secrets in scope, and thatFormatCheckPostbackfires onworkflow_run:and posts/updates the comment..scripts/release.sh v1.0.0locally on a cleanmaincheckout (after this PR merges) and confirm the script produces the expected immutable + mutable tags without pushing.🤖 Generated with Claude Code