Skip to content

Harden reusable workflows and introduce v1 versioning#88

Open
mtfishman wants to merge 1 commit intomainfrom
mf/integration-test-fork-skip
Open

Harden reusable workflows and introduce v1 versioning#88
mtfishman wants to merge 1 commit intomainfrom
mf/integration-test-fork-skip

Conversation

@mtfishman
Copy link
Copy Markdown
Member

Summary

Restructures the IntegrationTest, IntegrationTestRequest, and FormatCheck reusable 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 @v1 instead of @main.

IntegrationTest

  • Matrix expansion and the aggregation gate move into the reusable. Input changes from pkg: (scalar) to pkgs: (JSON array). Per-repo callers shrink considerably.
  • A pre-leg step probes each URL-based matrix entry anonymously with 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, and workflow_dispatch events; on fork pull_request: events they skip with a /integrationtest <url> notice.
  • A new gate job (named IntegrationTest to 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. Empty pkgs: '[]' passes through (no-downstream case).
  • The previously-recommended pull_request_target: trigger at the caller level is no longer needed; per-repo callers should use pull_request: once they migrate.

IntegrationTestRequest

  • Comment-body parsing moves from \${{ github.event.comment.body }} shell interpolation to an env: mapping (closes a script-injection-shape pattern).
  • The single package extracted from the comment is now JSON-encoded into a pkgs: '[\"...\"]' array for the new IntegrationTest interface.
  • On a successful run, posts a passing IntegrationTest check run via the GitHub Checks API. This satisfies branch protection on fork PRs whose private-only matrix would otherwise leave the IntegrationTest gate failing.

FormatCheck (split into parse + postback)

  • FormatCheck.yml (parse phase): runs on pull_request:, no secrets in scope, runs the formatter on PR head, uploads the diff and PR metadata as a format-check artifact, propagates the formatter exit code as the check status.
  • FormatCheckPostback.yml (new file, postback phase): runs on workflow_run: after a Format Check run completes, in the trusted base-repo context with secrets, downloads the artifact, and posts or updates the format-suggestion comment.
  • Branch protection should require the parse phase's check (Format Check / Check Formatting); the postback exists only to update the comment.

Hygiene across the rest of the reusables

  • \${{ inputs.* }} interpolations into shell run: blocks moved to env: mappings in Tests.yml, Documentation.yml, CompatHelper.yml, LiterateCheck.yml, Registrator.yml, and FormatPullRequest.yml.
  • Explicit per-job permissions: blocks added to every reusable that lacked one.
  • julia-actions/julia-buildpkg@latest in IntegrationTest.yml pinned to @v1.

Versioning

  • New top-level Versioning section in the README documents the convention (vMAJOR.MINOR.PATCH annotated tag + vMAJOR mutable tag).
  • New .scripts/release.sh cuts a release: rewrites internal @main references 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 IntegrationTest reusable's input shape changes from pkg: (scalar) to pkgs: (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

  • Open a smoke-test PR against a public-matrix repo (e.g. SparseArraysBase.jl) pointing its IntegrationTest.yml at this branch's SHA. Verify the matrix runs and the aggregate check passes on an internal PR.
  • Open a smoke-test PR against a Tennis-using repo (e.g. 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 /integrationtest notice and that the aggregate IntegrationTest check fails accordingly.
  • On the same fork PR, type /integrationtest https://github.com/ITensor/Tennis.jl as a maintainer; verify the manual run completes and the IntegrationTest check flips to passing via the Checks API post.
  • Verify FormatCheck parse phase emits its check on pull_request: events from forks with no secrets in scope, and that FormatCheckPostback fires on workflow_run: and posts/updates the comment.
  • Run .scripts/release.sh v1.0.0 locally on a clean main checkout (after this PR merges) and confirm the script produces the expected immutable + mutable tags without pushing.

🤖 Generated with Claude Code

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>
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