Bare `pull_request` hands fork PRs a read-only GITHUB_TOKEN, so the close/comment API calls 403 and a first-time/external contributor's draft PR survives — bypassing the auto-close for exactly the population the job targets. Switch to `pull_request_target`, which runs in the base-repo context with a write-capable token even for fork PRs. Safe because the job never checks out or executes PR-supplied code; it only reads event metadata and calls the GitHub API. The minimal `permissions: pull-requests: write` block still constrains the token. Add a regression guard in tests/workflow-maintainer-skip.test.cjs asserting the workflow triggers on pull_request_target and not bare pull_request. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
13
.github/workflows/close-draft-prs.yml
vendored
13
.github/workflows/close-draft-prs.yml
vendored
@@ -1,7 +1,18 @@
|
||||
name: Close Draft PRs
|
||||
|
||||
# pull_request_target (not pull_request) so the job runs in the base-repo
|
||||
# context with a write-capable token even for PRs from forks. Without this,
|
||||
# fork PRs from first-time/external contributors get a read-only GITHUB_TOKEN
|
||||
# and the close/comment API calls 403 — letting them bypass the auto-close.
|
||||
# Safe because this job only reads event metadata and never checks out or
|
||||
# executes PR-supplied code.
|
||||
# Residual platform limitation: GitHub deliberately does NOT trigger
|
||||
# pull_request_target for fork branches whose names look like a Git SHA, so a
|
||||
# contributor could still evade this by naming their head branch like a commit
|
||||
# hash. Fully closing that gap needs a scheduled base-context sweep (separate
|
||||
# concern); a draft PR that evades this still cannot merge and still fails the other gates.
|
||||
on:
|
||||
pull_request:
|
||||
pull_request_target:
|
||||
types: [opened, reopened, converted_to_draft]
|
||||
|
||||
concurrency:
|
||||
|
||||
@@ -29,6 +29,18 @@ describe('PR policy workflow maintainer carve-outs', () => {
|
||||
assertMaintainerSkip(workflow);
|
||||
});
|
||||
|
||||
test('draft PR auto-close triggers on pull_request_target so fork PRs cannot bypass it', () => {
|
||||
const workflow = readWorkflow('.github/workflows/close-draft-prs.yml');
|
||||
|
||||
// A bare `pull_request` trigger hands fork PRs (how first-time/external
|
||||
// contributors contribute) a read-only GITHUB_TOKEN, so the close/comment
|
||||
// API calls 403 and the draft PR survives — bypassing the auto-close.
|
||||
// `pull_request_target` runs in the base-repo context with a write-capable
|
||||
// token. Guard against a regression back to the bypassable trigger.
|
||||
assert.match(workflow, /^\s*pull_request_target:/m);
|
||||
assert.doesNotMatch(workflow, /^\s*pull_request:\s*$/m);
|
||||
});
|
||||
|
||||
test('PR target validator does not run for maintainer-authored PRs', () => {
|
||||
const workflow = readWorkflow('.github/workflows/pr-target-validator.yml');
|
||||
|
||||
|
||||
Reference in New Issue
Block a user