From 5a24e6afeb6abf64ce901900fcb33f7dcc0a0efd Mon Sep 17 00:00:00 2001 From: Carter Monaco Date: Mon, 10 Aug 2026 23:56:24 -0400 Subject: [PATCH] ci: run the checks on pull requests, and never publish from one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .github/workflows/deploy.yml | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/.github/workflows/deploy.yml b/.github/workflows/deploy.yml index 10026f2..e0d2d8d 100644 --- a/.github/workflows/deploy.yml +++ b/.github/workflows/deploy.yml @@ -3,6 +3,12 @@ name: Deploy demo to GitHub Pages on: push: branches: [main] + # Run the checks on PRs too. Without this the only thing that ever exercised + # them was a merge, so a problem was reported AFTER it had landed on main — + # which is how a stale `npm audit` figure sat in README.md and SECURITY.md + # until the first push that happened to run the test job. + pull_request: + branches: [main] workflow_dispatch: permissions: @@ -11,7 +17,11 @@ permissions: id-token: write concurrency: - group: pages + # Keyed by ref, NOT a single shared "pages" group. With one group and + # cancel-in-progress, opening a PR would cancel an in-flight deploy of main + # and leave the published site mid-update. Deploys still serialise with each + # other because they all run on refs/heads/main. + group: pages-${{ github.ref }} cancel-in-progress: true jobs: @@ -162,6 +172,11 @@ jobs: deploy: needs: [test, build] + # Publish ONLY from main. The jobs above are the reason to run on a pull + # request; this one is the reason that would otherwise be dangerous, because + # it pushes whatever was built straight to the public site. A PR must be + # able to prove itself without being able to publish itself. + if: github.event_name == 'push' && github.ref == 'refs/heads/main' runs-on: ubuntu-latest environment: name: github-pages