From b3a07c38f67d5251e491c055c73bae08258559f0 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 8 Jul 2026 21:12:25 +0000 Subject: [PATCH 1/4] ci: restore fork-PR E2E/tarball checkout after actions/checkout@v7 bump MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1710 bumped actions/checkout v6 -> v7. v7 refuses to check out fork-PR code in a `pull_request_target` workflow by default ("pwn request" protection), failing at the checkout step: Refusing to check out fork pull request code from a 'pull_request_target' workflow. ... set 'allow-unsafe-pr-checkout: true' This broke the `e2e` and `pr-tarball` jobs for every fork PR (they run on pull_request_target and check out `github.event.pull_request.head.sha`). Both jobs already gate on the `authorize` job (actor must be in AUTHORIZED_USERS) before they ever run, so the fork checkout only happens for pre-authorized users — that gate is the real pwn-request mitigation, and it's exactly the protection v7's default is conservatively enforcing. Opt back in with `allow-unsafe-pr-checkout: true` so authorized fork PRs get E2E/tarball coverage again (restores the pre-#1710 behavior while keeping checkout on v7). --- .github/workflows/e2e-tests.yml | 4 ++++ .github/workflows/pr-tarball.yml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 2c005c0c5..f300f053f 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -85,6 +85,10 @@ jobs: with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 + # This job runs only after the `authorize` gate confirms the actor is in AUTHORIZED_USERS, + # so checking out the PR head (incl. fork PRs) is intentional. checkout@v7 blocks fork + # checkout in pull_request_target by default; the authorize gate is our pwn-request guard. + allow-unsafe-pr-checkout: true - uses: actions/setup-node@v6 with: node-version: '20.x' diff --git a/.github/workflows/pr-tarball.yml b/.github/workflows/pr-tarball.yml index 5221b7884..30d458620 100644 --- a/.github/workflows/pr-tarball.yml +++ b/.github/workflows/pr-tarball.yml @@ -35,6 +35,10 @@ jobs: - uses: actions/checkout@v7 with: ref: ${{ github.event.pull_request.head.sha }} + # This job runs only after the `authorize` gate confirms the actor is in AUTHORIZED_USERS, + # so checking out the PR head (incl. fork PRs) is intentional. checkout@v7 blocks fork + # checkout in pull_request_target by default; the authorize gate is our pwn-request guard. + allow-unsafe-pr-checkout: true - uses: actions/setup-node@v6 with: node-version: '20.x' From e4b29e3a375f847897ba4a532058ff80db7ff2d9 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 8 Jul 2026 21:25:56 +0000 Subject: [PATCH 2/4] ci: gate fork-PR E2E/tarball on PR author, not github.actor (fixes pwn-request) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Supersedes the earlier `allow-unsafe-pr-checkout: true` approach, which was unsafe: the `authorize` gate checked `github.actor` (who triggered the run), not the PR author (whose code runs). A trusted actor pushing to a fork PR (e.g. a maintainer editing an external contributor's PR) would pass the gate and then execute the fork's code with the AWS role / contents:write token / App token — a classic pull_request_target "pwn request". Fix: - Gate on `github.event.pull_request.user.login` (the PR author) against AUTHORIZED_USERS. External-author fork PRs now SKIP cleanly (no failure, no code execution with secrets) instead of running or hard-failing at checkout. - Drop `allow-unsafe-pr-checkout: true`; checkout@v7's fork-checkout refusal stays in force for any fork PR that reaches the step. Authorized authors' on-repo PRs run normally; fork PRs get E2E via manual workflow_dispatch after review (as the workflow already documents). Curated AUTHORIZED_USERS is intentionally kept (rather than any-write-collaborator) for these secret-bearing jobs. --- .github/workflows/e2e-tests.yml | 17 ++++++++--------- .github/workflows/pr-tarball.yml | 15 +++++++-------- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index f300f053f..84bbfdb1c 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -50,19 +50,22 @@ jobs: env: EVENT_NAME: ${{ github.event_name }} AUTHORIZED_USERS: ${{ secrets.AUTHORIZED_USERS }} - GITHUB_ACTOR: ${{ github.actor }} + # Gate on the PR AUTHOR (whose code this is), NOT github.actor (who triggered the run). + # On pull_request_target a trusted actor (e.g. a maintainer pushing to a fork PR) triggering + # a run must NOT cause an untrusted author's fork code to execute with our AWS role/secrets. + PR_AUTHOR: ${{ github.event.pull_request.user.login }} run: | if [[ "$EVENT_NAME" == "workflow_dispatch" ]]; then echo "✅ Manual workflow dispatch — authorized" echo "is_authorized=true" >> "$GITHUB_OUTPUT" exit 0 fi - if [[ ",$AUTHORIZED_USERS," == *",${GITHUB_ACTOR},"* ]]; then - echo "✅ User ${GITHUB_ACTOR} is authorized" + if [[ ",$AUTHORIZED_USERS," == *",${PR_AUTHOR},"* ]]; then + echo "✅ PR author ${PR_AUTHOR} is authorized" echo "is_authorized=true" >> "$GITHUB_OUTPUT" else - echo "⏭️ User ${GITHUB_ACTOR} is not in AUTHORIZED_USERS — skipping E2E tests." - echo "ℹ️ External contributors: ask a maintainer to run the E2E tests manually via workflow_dispatch." + echo "⏭️ PR author ${PR_AUTHOR} is not in AUTHORIZED_USERS — skipping E2E tests." + echo "ℹ️ External contributors: a maintainer can run the E2E tests via workflow_dispatch after reviewing the code." echo "is_authorized=false" >> "$GITHUB_OUTPUT" fi @@ -85,10 +88,6 @@ jobs: with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 - # This job runs only after the `authorize` gate confirms the actor is in AUTHORIZED_USERS, - # so checking out the PR head (incl. fork PRs) is intentional. checkout@v7 blocks fork - # checkout in pull_request_target by default; the authorize gate is our pwn-request guard. - allow-unsafe-pr-checkout: true - uses: actions/setup-node@v6 with: node-version: '20.x' diff --git a/.github/workflows/pr-tarball.yml b/.github/workflows/pr-tarball.yml index 30d458620..e26f758d2 100644 --- a/.github/workflows/pr-tarball.yml +++ b/.github/workflows/pr-tarball.yml @@ -17,13 +17,16 @@ jobs: id: check env: AUTHORIZED_USERS: ${{ secrets.AUTHORIZED_USERS }} - GITHUB_ACTOR: ${{ github.actor }} + # Gate on the PR AUTHOR (whose code this npm build runs), NOT github.actor (who triggered the + # run). Otherwise a trusted actor pushing to a fork PR would run untrusted fork npm scripts + # with the contents:write GITHUB_TOKEN + App token in this job. + PR_AUTHOR: ${{ github.event.pull_request.user.login }} run: | - if [[ ",$AUTHORIZED_USERS," == *",${GITHUB_ACTOR},"* ]]; then - echo "✅ User ${GITHUB_ACTOR} is authorized" + if [[ ",$AUTHORIZED_USERS," == *",${PR_AUTHOR},"* ]]; then + echo "✅ PR author ${PR_AUTHOR} is authorized" echo "is_authorized=true" >> "$GITHUB_OUTPUT" else - echo "⏭️ User ${GITHUB_ACTOR} is not in AUTHORIZED_USERS — skipping." + echo "⏭️ PR author ${PR_AUTHOR} is not in AUTHORIZED_USERS — skipping." echo "is_authorized=false" >> "$GITHUB_OUTPUT" fi @@ -35,10 +38,6 @@ jobs: - uses: actions/checkout@v7 with: ref: ${{ github.event.pull_request.head.sha }} - # This job runs only after the `authorize` gate confirms the actor is in AUTHORIZED_USERS, - # so checking out the PR head (incl. fork PRs) is intentional. checkout@v7 blocks fork - # checkout in pull_request_target by default; the authorize gate is our pwn-request guard. - allow-unsafe-pr-checkout: true - uses: actions/setup-node@v6 with: node-version: '20.x' From 7aa4833cb75c8efce44332bf1588f4ad43d193e9 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 8 Jul 2026 21:35:54 +0000 Subject: [PATCH 3/4] ci: also unblock pr-security-review fork checkout (author-gated, read-only) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Completes the checkout@v7 fork-PR fix: pr-security-review.yml has the same pull_request_target + checkout@v7 + fork-head pattern (its ref is computed in a step, so it was easy to miss). Unlike e2e/pr-tarball, this job: - is ALREADY gated on the PR author's write/admin permission (getCollaboratorPermissionLevel), so external forks already skip it, and - only READS the code (compute diff, build prompt, run the review) — it never executes fork code (no npm ci / npm scripts). So `allow-unsafe-pr-checkout: true` is the correct, low-risk fix here (reviewing the PR head is the job's whole purpose), whereas for the secret-executing e2e/pr-tarball jobs we instead moved to author-based gating and dropped the flag. --- .github/workflows/pr-security-review.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/pr-security-review.yml b/.github/workflows/pr-security-review.yml index bac101a83..4b0e8a3ea 100644 --- a/.github/workflows/pr-security-review.yml +++ b/.github/workflows/pr-security-review.yml @@ -160,6 +160,12 @@ jobs: with: ref: ${{ steps.pr.outputs.head_sha }} fetch-depth: 0 + # Safe to opt in here: the `authorize` job already gates on the PR AUTHOR having write/admin + # (getCollaboratorPermissionLevel), and this job only READS the code (compute diff, build + # prompt, run the review) — it never executes fork code (no npm ci / npm scripts). checkout@v7 + # otherwise refuses fork-PR checkout in pull_request_target, which would break the review on + # an authorized author's fork PR. Reviewing the PR head is the whole point of this job. + allow-unsafe-pr-checkout: true - name: Compute diff id: diff From cddffca34d73db456f4a5dc5825e171b2535bacd Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Wed, 8 Jul 2026 21:57:12 +0000 Subject: [PATCH 4/4] ci: restore allow-unsafe-pr-checkout for e2e/pr-tarball (now author-gated) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses review feedback: authorized maintainers routinely open PRs from their own forks, so gating on the author WITHOUT allow-unsafe made those normal fork PRs hard-fail at checkout@v7. Now that the authorize gate keys on the PR author (github.event.pull_request.user.login in AUTHORIZED_USERS), re-adding allow-unsafe-pr-checkout is safe: external authors skip at the gate, so only trusted authors' code is ever checked out — and their fork PRs run normally again. (The earlier scanner flag was about allow-unsafe combined with the ACTOR-based gate, which this no longer uses.) All three pull_request_target workflows now use the same author-gate + allow-unsafe combination. --- .github/workflows/e2e-tests.yml | 5 +++++ .github/workflows/pr-tarball.yml | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/.github/workflows/e2e-tests.yml b/.github/workflows/e2e-tests.yml index 84bbfdb1c..1b505cbb2 100644 --- a/.github/workflows/e2e-tests.yml +++ b/.github/workflows/e2e-tests.yml @@ -88,6 +88,11 @@ jobs: with: ref: ${{ github.event.pull_request.head.sha }} fetch-depth: 0 + # Safe because the `authorize` job above gates on the PR AUTHOR being in AUTHORIZED_USERS: + # external authors skip at the gate, so only trusted authors' code is ever checked out here. + # Needed because authorized maintainers routinely open PRs from their own forks, and + # checkout@v7 otherwise refuses fork-PR checkout in pull_request_target. + allow-unsafe-pr-checkout: true - uses: actions/setup-node@v6 with: node-version: '20.x' diff --git a/.github/workflows/pr-tarball.yml b/.github/workflows/pr-tarball.yml index e26f758d2..b62de6638 100644 --- a/.github/workflows/pr-tarball.yml +++ b/.github/workflows/pr-tarball.yml @@ -38,6 +38,11 @@ jobs: - uses: actions/checkout@v7 with: ref: ${{ github.event.pull_request.head.sha }} + # Safe because the `authorize` job above gates on the PR AUTHOR being in AUTHORIZED_USERS: + # external authors skip at the gate, so only trusted authors' code is ever checked out here. + # Needed because authorized maintainers routinely open PRs from their own forks, and + # checkout@v7 otherwise refuses fork-PR checkout in pull_request_target. + allow-unsafe-pr-checkout: true - uses: actions/setup-node@v6 with: node-version: '20.x'