From cbc7d77931207da6d4380dd39533fce0d38caa6a Mon Sep 17 00:00:00 2001 From: Bryan Thompson Date: Fri, 26 Jun 2026 10:45:10 -0500 Subject: [PATCH] Exempt the bump bot from the external-PR scope guard (#3402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Exempt the bump bot from the external-PR scope guard The External PR Scope Guard (#3353) and the auto-closer both look up the PR author's collaborator permission and, for anyone who is not write/admin, require the PR to ADD marketplace.json entries (additions-only). Internal bump PRs are authored by github-actions[bot], which is not reported as a member, so a SHA-bump — a legitimate MODIFY of an existing entry — fails the guard (e.g. #3391 "modifies existing entry: astronomer-data-agents"). Add a shared isExemptAuthor() helper that exempts both org members and the repo's own automation bot, and route both workflows through it. Safe under pull_request_target: a fork PR cannot author as github-actions[bot] (only the org's own GITHUB_TOKEN workflow can), and the member path is still a real permission lookup. The helper also wraps getCollaboratorPermissionLevel in try/catch — previously a non-collaborator/unknown-user lookup threw and errored the job instead of falling through to scope evaluation. Co-Authored-By: Claude Opus 4.8 (1M context) * Correct stale "required status check" guidance in scope-guard comments The scope guard is advisory, not a required status check — the merge gate is validate + scan + a maintainer approval. The old header told operators to add it to branch protection, which is now contra-indicated (it would block the no-approval bump-merge path). Update both workflow comments to match. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .github/scripts/external-pr-scope.js | 31 +++++++++++++++++- .github/workflows/close-external-prs.yml | 19 ++++++----- .github/workflows/external-pr-scope-guard.yml | 32 ++++++++++--------- 3 files changed, 56 insertions(+), 26 deletions(-) diff --git a/.github/scripts/external-pr-scope.js b/.github/scripts/external-pr-scope.js index 5438a1d..705a18a 100644 --- a/.github/scripts/external-pr-scope.js +++ b/.github/scripts/external-pr-scope.js @@ -121,4 +121,33 @@ async function evaluate({ github, context }) { return analyze({ changedFiles, before, after, liveRepos: liveReposOf(liveBase) }); } -module.exports = { normalizeRepo, liveReposOf, analyze, readPlugins, evaluate, MARKETPLACE }; +// Authors that are NOT subject to the external-contributor scope rules: +// - the repo's own automation bot — its bump PRs legitimately MODIFY existing entries +// (SHA bumps), which the additions-only external-contributor rule forbids; AND +// - org members (write/admin). +// Safe under pull_request_target: a fork PR cannot set its author to github-actions[bot] +// (that login is only ever the org's own GITHUB_TOKEN workflow), and the member path is a +// real permission lookup. Wrapped in try/catch because getCollaboratorPermissionLevel throws +// for a non-collaborator/unknown user — without this, both callers would error the job rather +// than fall through to scope evaluation. +const EXEMPT_BOTS = new Set(['github-actions[bot]']); + +async function isExemptAuthor({ github, context }) { + const author = context.payload.pull_request.user.login; + if (EXEMPT_BOTS.has(author)) { + return { exempt: true, reason: `${author} is the trusted automation bot` }; + } + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner: context.repo.owner, repo: context.repo.repo, username: author, + }); + if (['admin', 'write'].includes(data.permission)) { + return { exempt: true, reason: `${author} is ${data.permission} (member)` }; + } + } catch (e) { + // not a collaborator / lookup failed → not exempt; fall through to scope evaluation + } + return { exempt: false }; +} + +module.exports = { normalizeRepo, liveReposOf, analyze, readPlugins, evaluate, isExemptAuthor, MARKETPLACE }; diff --git a/.github/workflows/close-external-prs.yml b/.github/workflows/close-external-prs.yml index b1fc280..21b2e2c 100644 --- a/.github/workflows/close-external-prs.yml +++ b/.github/workflows/close-external-prs.yml @@ -23,14 +23,13 @@ jobs: script: | const author = context.payload.pull_request.user.login; - const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ - owner: context.repo.owner, - repo: context.repo.repo, - username: author - }); + const { evaluate, isExemptAuthor } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`); - if (['admin', 'write'].includes(data.permission)) { - console.log(`${author} has ${data.permission} access, allowing PR`); + // Members (write/admin) and the repo's own automation bot (bump SHA PRs) are never + // auto-closed. + const ex = await isExemptAuthor({ github, context }); + if (ex.exempt) { + console.log(`${ex.reason} — allowing PR`); return; } @@ -38,9 +37,9 @@ jobs: // contribution — it adds marketplace.json entries whose source repo ALREADY backs // a live plugin here, and changes nothing else. (No maintained allowlist: the set // of allowed repos is derived from the live marketplace.) This grants only the - // right to open a reviewable PR; the External PR Scope Guard required check and a - // maintainer approval still gate the merge. - const { evaluate } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`); + // right to open a reviewable PR; the validate + scan checks and a maintainer + // approval still gate the merge (the External PR Scope Guard is advisory signal, + // not a required check). const result = await evaluate({ github, context }); if (result.ok && result.added.length > 0) { console.log(`In-scope external contribution (adds: ${result.added.join(', ')}) — allowing PR.`); diff --git a/.github/workflows/external-pr-scope-guard.yml b/.github/workflows/external-pr-scope-guard.yml index 5827085..3d67296 100644 --- a/.github/workflows/external-pr-scope-guard.yml +++ b/.github/workflows/external-pr-scope-guard.yml @@ -1,14 +1,17 @@ name: External PR Scope Guard -# Required status check that constrains what a NON-MEMBER pull request may change. -# Members (write/admin) are unrestricted and skip this check. For a non-member PR this -# fails unless the PR is an in-scope external contribution per .github/scripts/external-pr-scope.js: -# it changes ONLY .claude-plugin/marketplace.json, the delta is additions-only (no existing -# entry modified or removed), and every ADDED entry's source.url is a repo that ALREADY backs -# a live plugin in this marketplace (the allowed set is derived from the live marketplace — -# there is no maintained allowlist). +# Advisory check that surfaces what a NON-MEMBER pull request may change. +# Members (write/admin) and the repo's own automation bot (bump SHA PRs) are unrestricted and +# skip this check. For a non-member PR this fails unless the PR is an in-scope external +# contribution per .github/scripts/external-pr-scope.js: it changes ONLY +# .claude-plugin/marketplace.json, the delta is additions-only (no existing entry modified or +# removed), and every ADDED entry's source.url is a repo that ALREADY backs a live plugin in +# this marketplace (the allowed set is derived from the live marketplace — there is no +# maintained allowlist). # -# Add the scope-guard job as a REQUIRED status check in branch protection for it to block merge. +# Do NOT add this job to branch protection as a required status check. The merge gate is the +# `validate` + `scan` checks plus a maintainer approval; this guard is advisory signal for the +# reviewer, not a hard gate. (Making it required would block the no-approval bump-merge path.) # # Security: runs on pull_request_target but checks out only the BASE repo (trusted) for the # shared script; the head marketplace.json is fetched as DATA via the API and parsed, never executed. @@ -29,17 +32,16 @@ jobs: - uses: actions/github-script@v7 with: script: | - const author = context.payload.pull_request.user.login; + const { evaluate, isExemptAuthor } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`); - const { data: perm } = await github.rest.repos.getCollaboratorPermissionLevel({ - owner: context.repo.owner, repo: context.repo.repo, username: author, - }); - if (['admin', 'write'].includes(perm.permission)) { - console.log(`${author} is ${perm.permission} (member) — scope guard not applicable.`); + // Members (write/admin) and the repo's own automation bot (bump SHA PRs) are + // unrestricted; only genuinely external contributions are scope-checked. + const ex = await isExemptAuthor({ github, context }); + if (ex.exempt) { + console.log(`${ex.reason} — scope guard not applicable.`); return; } - const { evaluate } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`); const result = await evaluate({ github, context }); if (!result.ok) {