diff --git a/.github/scripts/tests/notify-pr-merge-review.test.mjs b/.github/scripts/tests/notify-pr-merge-review.test.mjs index 56dca3294ce..dbb8dca94da 100644 --- a/.github/scripts/tests/notify-pr-merge-review.test.mjs +++ b/.github/scripts/tests/notify-pr-merge-review.test.mjs @@ -243,9 +243,21 @@ test("workflow uses read permissions, a protected environment, trusted checkout, assert.match(workflow, /ref: \$\{\{ github.sha \}\}/); assert.match(workflow, /persist-credentials: false/); assert.match(workflow, /continue-on-error: true/); + for (const permission of ["actions", "checks", "contents", "pull-requests", "statuses"]) { + assert.ok(workflow.includes(` ${permission}: read`), `Missing ${permission}: read`); + } assert.doesNotMatch(workflow, /: write|download-artifact|restore@|pull_request_target:/); for (const name of ["PAPERCLIP_PR_REVIEW_WEBHOOK_URL", "PAPERCLIP_PR_REVIEW_WEBHOOK_TOKEN"]) { assert.ok(workflow.includes(`secrets.${name}`)); assert.ok(!workflow.includes(`vars.${name}`)); } }); + +test("API probe uses the same read permissions without private webhook access", () => { + const probe = readFileSync(".github/workflows/pr-review-api-access.yml", "utf8"); + for (const permission of ["actions", "checks", "contents", "pull-requests", "statuses"]) { + assert.ok(probe.includes(` ${permission}: read`)); + } + assert.match(probe, /snapshot\(Number\(process.env.PR_NUMBER\)\)/); + assert.doesNotMatch(probe, /\$\{\{ secrets\.|environment:|: write|deliver\(/); +}); diff --git a/.github/workflows/pr-merge-review.yml b/.github/workflows/pr-merge-review.yml index fcb555f9a67..87c08abbc44 100644 --- a/.github/workflows/pr-merge-review.yml +++ b/.github/workflows/pr-merge-review.yml @@ -13,6 +13,7 @@ on: - CI work benchmark - Incremental CI qualification - Backup checkpoint qualification + - PR review API access types: [completed] # External check providers and legacy commit statuses can finish after CI. check_run: @@ -24,6 +25,7 @@ permissions: checks: read contents: read pull-requests: read + statuses: read jobs: notify: diff --git a/.github/workflows/pr-review-api-access.yml b/.github/workflows/pr-review-api-access.yml new file mode 100644 index 00000000000..cdf916fa29e --- /dev/null +++ b/.github/workflows/pr-review-api-access.yml @@ -0,0 +1,43 @@ +name: PR review API access + +on: + pull_request: + paths: + - .github/scripts/notify-pr-merge-review.mjs + - .github/scripts/tests/notify-pr-merge-review.test.mjs + - .github/workflows/pr-merge-review.yml + - .github/workflows/pr-review-api-access.yml + +permissions: + actions: read + checks: read + contents: read + pull-requests: read + statuses: read + +jobs: + probe: + if: github.repository == 'paperclipai/paperclip' + runs-on: ubuntu-latest + timeout-minutes: 5 + # This job has no environment or private webhook secrets. It proves that + # the notifier's status query works with the actual read-only Actions token. + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7 + with: + persist-credentials: false + sparse-checkout: .github/scripts + - name: Read the PR status rollup with the Actions token + env: + GH_TOKEN: ${{ github.token }} + PR_NUMBER: ${{ github.event.pull_request.number }} + PR_HEAD_SHA: ${{ github.event.pull_request.head.sha }} + run: | + node --input-type=module <<'JS' + import assert from 'node:assert/strict'; + import { snapshot } from './.github/scripts/notify-pr-merge-review.mjs'; + const pr = snapshot(Number(process.env.PR_NUMBER)); + assert.equal(pr.headRefOid, process.env.PR_HEAD_SHA); + assert.ok(pr.statusCheckRollup.length > 0); + console.log('Actions token can read the complete PR status rollup.'); + JS diff --git a/doc/PR-MERGE-REVIEW-WEBHOOK.md b/doc/PR-MERGE-REVIEW-WEBHOOK.md index b9e63a50672..6526aca0999 100644 --- a/doc/PR-MERGE-REVIEW-WEBHOOK.md +++ b/doc/PR-MERGE-REVIEW-WEBHOOK.md @@ -34,6 +34,12 @@ PR code or downloads PR artifacts or caches. It reacts to the repository's PR workflow completions, external check completions, and legacy commit statuses. When adding another PR verification workflow, add its name to `workflow_run`. +The separate `PR review API access` workflow checks notifier changes using +the real read-only Actions token. It has no private environment or webhook +credentials and sends no webhook. Both workflows explicitly grant +`actions`, `checks`, `contents`, `pull-requests`, and `statuses` read access. +The GraphQL status rollup needs both check and commit-status permissions. + The notifier reads the current PR's status rollup and branch rules. Both `ci / verify` and `ci / e2e`, plus any additional required checks, must succeed. Other checks must succeed or finish as skipped or neutral. Optional skipped or