ci: keep the publish credential inside the release environment - #6170
Conversation
publish-js and publish-to-cargo held `id-token: write` on every event while naming the `release` environment only on releases, so an ungated run could mint a token that a registry with no environment pinned would accept — reachable by dispatching this workflow from a branch. Each is now release-only with a static `environment: release`, and the off-release smoke tests are separate jobs with no OIDC permission. The js build moves into a composite action rather than being duplicated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
prql-bot
left a comment
There was a problem hiding this comment.
The split reads correctly to me — the pairs match the shape publish-python already uses (if: github.event_name == 'release' + static environment: release), and nothing else in the repo references the old job names, so the rename is contained. env.version and hashFiles('./Cargo.lock') inside a composite action are both already proven by build-prqlc and time-compilation, and every run step in the new action carries shell: bash.
One consequence of the extraction worth naming: the nightly path filter in tests.yaml lists .github/workflows/release.yaml but nothing under .github/actions/, so a PR that only edits .github/actions/build-js/action.yaml won't set steps.changes.outputs.nightly, won't run the nightly job, and therefore won't run test-publish-js. Those same steps live in release.yaml today, which the filter does watch — so the smoke-test coverage on them is lost by the move. The concrete case is a Dependabot bump of actions/setup-node or Swatinem/rust-cache inside the new action landing without the npm publish path having been exercised. Adding .github/actions/build-*/** to the nightly filter closes it — all of .github/actions/** would too, but that also drags in tend-setup changes, which don't need a nightly run. build-prqlc and build-python have the same gap already, so the one filter entry covers them as well. Happy to push it here if you'd rather not carry a separate PR.
The `nightly` path filter watched `release.yaml` but nothing under `.github/actions/`, so moving the js build steps into a composite action would have taken them out of the smoke test. `build-prqlc` and `build-python` had the same gap. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
publish-jsandpublish-to-cargoeach ran on every event, holdingid-token: writethroughout, and named thereleaseenvironment only whengithub.event_name == 'release'. On every other event they minted an OIDC token from a run nothing gates.That token is accepted by a registry whose trusted-publisher config doesn't pin an environment — the claims it checks are the repository and the workflow filename, both of which an ungated run satisfies.
release.yamlacceptsworkflow_dispatch, so an account with write access can dispatch a branch carrying its own copy of this file and publish with it. The environment is the only part of the run it can't fake, which is why the credential has to live inside one.So each pair is now two jobs:
publish-js/publish-to-cargo—if: github.event_name == 'release',environment: release, and the OIDC permission.test-publish-js/test-publish-to-cargo— the smoke testsnightly.yamlruns on PRs, with noid-token: writeat all.The js build steps move into
.github/actions/build-jsrather than being duplicated across the two jobs, followingbuild-prqlcandbuild-python. Thenightlypath filter intests.yamlgains.github/actions/build-*/**to go with that: it watchedrelease.yamlbut nothing under.github/actions/, so a change to the extracted steps would no longer have triggered the smoke test that runs them.build-prqlcandbuild-pythonhad the same gap.Two consequences worth naming:
--provenance, which needs an OIDC token. It still checks that the package builds and thatnpm publishaccepts it.environment:is also readable by tend, whosecredential-environmentscheck declines to judge a job whose environment is an expression. This takes prql from SKIP to PASS on it.This only closes the hole if the registries pin the environment too, which is configured on their side: npm, crates.io and PyPI each have an optional environment field in the trusted publisher config, and a blank one accepts any run of this workflow.