ci: run the checks on pull requests, and never publish from one - #2
Merged
Conversation
Until now deploy.yml triggered only on push to main, so the test, copy-drift and
build jobs first executed AFTER a change had already landed. That is how a stale
`npm audit` figure sat in README.md and SECURITY.md claiming 0 advisories while
the real count was 4: nothing ran the assertion until a merge did, and by then
the false claim was already on a public page.
Two things had to be true before this was safe to switch on:
- `deploy` must not run on a pull request. It pushes whatever was built straight
to the public site, so without a guard this change would let any PR publish
itself. Gated to push-on-main.
- The concurrency group must not be shared. It was a single `pages` group with
cancel-in-progress, so a PR opening would have cancelled an in-flight deploy of
main and left the published site mid-update. Keyed by ref now; deploys still
serialise against each other because they all run on refs/heads/main.
Note this makes `github.head_ref` reachable for the first time in the copy-drift
job — on a fork PR that value is attacker-influencable. It was already handled
correctly: passed through `env:` rather than interpolated into the shell,
quoted, validated against ^[A-Za-z0-9._/-]{1,200}$, and refs shaped like an
option are rejected in favour of the default branch. That defence was written
for exactly this case and is now load-bearing rather than theoretical.
Co-Authored-By: Claude Opus 5 <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
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.
deploy.ymltriggered only on push tomain, sotest,copy-driftandbuildfirst ran after a change had landed. That is how README.md andSECURITY.md sat on a public page claiming
npm auditreports 0 advisories whileit actually reported 4 — nothing ran the assertion until a merge did.
Two guards had to go in before this was safe:
deployis gated to push-on-main. It publishes straight to the live site,so without this a PR could publish itself.
pagesgroup withcancel-in-progress, so opening a PR would have cancelled an in-flight deployof
mainand left the site mid-update.This PR is its own test:
pull_requestruns are evaluated against the mergecommit's workflow file, so if the checks appear below, the trigger works.
🤖 Generated with Claude Code