From a8237e1537e987a92c9f33e5cd650acbe175fd5e Mon Sep 17 00:00:00 2001 From: Bryan Thompson Date: Thu, 25 Jun 2026 15:55:04 -0500 Subject: [PATCH] Allow non-member PRs scoped to an already-listed plugin repo (#3353) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(ci): allow live external contributors to open scoped PRs Add an opt-in allowlist so a vetted external developer who already has a plugin live in this marketplace — but cannot use the submission form (e.g. an enterprise partner without a Claude account) — can open a reviewable PR instead of having it auto-closed. - .github/external-contributors.json: username -> allowed_sources map (doubles as the allowlist and the per-author source scope). - close-external-prs.yml: skip the auto-close for allowlisted authors (reads the list from the trusted base checkout). Grants ONLY the right to open a PR; CI + maintainer approval are unchanged. - external-pr-scope-guard.yml: required check for allowlisted external authors. Fails unless the PR touches ONLY marketplace.json and the delta is additions-only, with every added entry's source.url under that author's allowed_sources. Anthropic members are unrestricted. Reads head marketplace.json as data via the API (no untrusted checkout). Co-Authored-By: Claude Opus 4.8 (1M context) * docs(ci): neutral wording in external-contributors allowlist note Co-Authored-By: Claude Opus 4.8 (1M context) * refactor(ci): key external-PR allowlist on source org, not individuals Replace the per-username allowlist with a source-org allowlist so no individual is named in the repo. A non-member PR stays open only if it adds marketplace.json entries whose source.url is under an allowlisted prefix and changes nothing else; merge still requires CI + maintainer approval. - external-pr-allowed-sources.json: flat allowed_sources prefixes (no usernames) - scripts/external-pr-scope.js: shared additions-only / allowed-source logic - close-external-prs.yml + external-pr-scope-guard.yml: both use the shared module Co-Authored-By: Claude Opus 4.8 (1M context) * refactor(ci): derive external-PR scope from live repos, no maintained list Replace the curated source-org allowlist with auto-derivation from the live marketplace. A non-member PR stays open only if it ADDS entries whose source.url repo ALREADY backs a live plugin here, additions-only, nothing else touched. No list to maintain, no individuals named. Trust is anchored in the source repo + pinned SHA (org-controlled), not the submitter's identity; merge still requires CI + maintainer approval. - remove external-pr-allowed-sources.json - scripts/external-pr-scope.js: derive allowed repos from base marketplace.json - both workflows drop the allowlist-file arg Co-Authored-By: Claude Opus 4.8 (1M context) * fix(ci): diff external-PR scope against the merge-base, not base tip Comparing base-branch-tip -> head made a fork that is behind main report all of main's later additions as phantom removals/modifications, which would wrongly fail the scope guard for a legitimate additions-only PR. Diff merge-base -> head (the PR's actual changes) instead; keep the "already live" check against the current base branch. Found by an end-to-end run of evaluate() against real PR data (#3298 stayed clean; #3044's phantom drift collapsed to its real one-line change). Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- .github/scripts/external-pr-scope.js | 124 ++++++++++++++++++ .github/workflows/close-external-prs.yml | 21 ++- .github/workflows/external-pr-scope-guard.yml | 52 ++++++++ 3 files changed, 195 insertions(+), 2 deletions(-) create mode 100644 .github/scripts/external-pr-scope.js create mode 100644 .github/workflows/external-pr-scope-guard.yml diff --git a/.github/scripts/external-pr-scope.js b/.github/scripts/external-pr-scope.js new file mode 100644 index 0000000..5438a1d --- /dev/null +++ b/.github/scripts/external-pr-scope.js @@ -0,0 +1,124 @@ +'use strict'; +// Shared logic for letting a NON-MEMBER pull request stay open and be reviewed, scoped to +// the contributor's own already-listed plugin repo. No maintained allowlist, no individuals. +// +// Trust model: we do NOT verify the submitter's identity. We trust the SOURCE REPO. A PR is +// in scope only if it ADDS marketplace.json entries whose source.url is a repo that ALREADY +// backs a live entry in this marketplace (derived from the base marketplace.json), pinned to +// a commit in that repo. Because the repo is org-controlled and the SHA pins to a real commit +// there, the shipped code is the org's code regardless of who opened the PR. Merge still +// requires CI + a maintainer approval. +// +// Used by: +// - close-external-prs.yml (skip the auto-close when in scope) +// - external-pr-scope-guard.yml (required status check: fail a non-member PR that is out of scope) +// +// Security: evaluate() reads base + head marketplace.json as DATA via the API and parses them; +// it never checks out or executes head code. + +const MARKETPLACE = '.claude-plugin/marketplace.json'; + +function normalizeRepo(u) { + return String(u || '').trim().toLowerCase() + .replace(/^git\+/, '') + .replace(/^https?:\/\//, '') + .replace(/\.git$/, '') + .replace(/\/+$/, ''); +} + +function pluginsByName(json) { + const map = {}; + for (const p of (json && json.plugins) || []) { if (p && p.name) map[p.name] = p; } + return map; +} + +// Repos that already back a live entry, derived from the base marketplace.json. +function liveReposOf(base) { + const s = new Set(); + for (const name of Object.keys(base)) { + const u = base[name] && base[name].source && base[name].source.url; + if (!u) continue; + const r = normalizeRepo(u); + if (r.split('/').length >= 3) s.add(r); // host/org/repo + } + return s; +} + +// Pure decision over an already-computed diff. Returns { ok, problems, added, removed, modified }. +// before = plugins at the MERGE-BASE (what head forked from), after = plugins at HEAD, +// liveRepos = repos already live on the current base branch. Diffing before->after (not +// base-tip->head) isolates THIS PR's changes; a stale fork no longer shows main's later +// additions as phantom removals. +function analyze({ changedFiles, before, after, liveRepos }) { + const problems = []; + + const off = changedFiles.filter(n => n !== MARKETPLACE); + if (off.length) problems.push(`changes files other than ${MARKETPLACE}: ${off.join(', ')}`); + + const baseNames = new Set(Object.keys(before)); + const headNames = new Set(Object.keys(after)); + const removed = [...baseNames].filter(n => !headNames.has(n)); + const added = [...headNames].filter(n => !baseNames.has(n)); + const modified = [...headNames].filter( + n => baseNames.has(n) && JSON.stringify(before[n]) !== JSON.stringify(after[n]) + ); + + if (removed.length) problems.push(`removes existing entr${removed.length > 1 ? 'ies' : 'y'}: ${removed.join(', ')}`); + if (modified.length) problems.push(`modifies existing entr${modified.length > 1 ? 'ies' : 'y'}: ${modified.join(', ')}`); + if (!off.length && !added.length && !removed.length && !modified.length) { + problems.push('makes no in-scope change (expected additions to marketplace.json)'); + } + + for (const name of added) { + const u = after[name] && after[name].source && after[name].source.url; + if (!u) { problems.push(`added "${name}" has no source.url to validate`); continue; } + const r = normalizeRepo(u); + if (r.split('/').length < 3) { problems.push(`added "${name}" source.url ${u} is not a valid repo URL`); continue; } + if (!liveRepos.has(r)) { + problems.push(`added "${name}" points at ${u}, a repo with no existing live plugin in this marketplace`); + } + } + + return { ok: problems.length === 0, problems, added, removed, modified, liveRepoCount: liveRepos.size }; +} + +async function readPlugins(github, owner, repo, ref) { + try { + const { data } = await github.rest.repos.getContent({ owner, repo, ref, path: MARKETPLACE }); + return pluginsByName(JSON.parse(Buffer.from(data.content, 'base64').toString('utf8'))); + } catch (e) { + return null; + } +} + +// API wrapper used by both workflows. Fetches the diff + base/head marketplace.json, delegates to analyze(). +async function evaluate({ github, context }) { + const pr = context.payload.pull_request; + const owner = context.repo.owner, repo = context.repo.repo; + + const files = await github.paginate(github.rest.pulls.listFiles, { + owner, repo, pull_number: pr.number, per_page: 100, + }); + const changedFiles = files.map(f => f.filename); + + // Diff THIS PR's changes (merge-base -> head), not base-tip -> head, so a fork that is + // behind main doesn't show main's later additions as phantom removals. + let mergeBaseSha = pr.base.sha; + try { + const cmp = await github.rest.repos.compareCommits({ owner, repo, base: pr.base.sha, head: pr.head.sha }); + if (cmp && cmp.data && cmp.data.merge_base_commit && cmp.data.merge_base_commit.sha) { + mergeBaseSha = cmp.data.merge_base_commit.sha; + } + } catch (e) { /* fall back to base.sha */ } + + const liveBase = await readPlugins(github, owner, repo, pr.base.sha); // current base branch (for "already live") + const before = await readPlugins(github, owner, repo, mergeBaseSha); // what head forked from + const after = await readPlugins(github, pr.head.repo.owner.login, pr.head.repo.name, pr.head.sha); + if (liveBase === null || before === null || after === null) { + return { ok: false, problems: ['could not read marketplace.json at base, merge-base, and/or head'], added: [], removed: [], modified: [] }; + } + + return analyze({ changedFiles, before, after, liveRepos: liveReposOf(liveBase) }); +} + +module.exports = { normalizeRepo, liveReposOf, analyze, readPlugins, evaluate, MARKETPLACE }; diff --git a/.github/workflows/close-external-prs.yml b/.github/workflows/close-external-prs.yml index 1f4fc7e..b1fc280 100644 --- a/.github/workflows/close-external-prs.yml +++ b/.github/workflows/close-external-prs.yml @@ -7,13 +7,17 @@ on: permissions: pull-requests: write issues: write + contents: read jobs: check-membership: if: vars.DISABLE_EXTERNAL_PR_CHECK != 'true' runs-on: ubuntu-latest steps: - - name: Check if author has write access + # pull_request_target: checks out the BASE repo (trusted), so the allowlist + shared + # script below are this repo's versions, never the fork's. + - uses: actions/checkout@v4 + - name: Close PR unless author is a member or the PR is an in-scope external contribution uses: actions/github-script@v7 with: script: | @@ -30,7 +34,20 @@ jobs: return; } - console.log(`${author} has ${data.permission} access, closing PR`); + // Non-member: allow the PR to stay open ONLY if it is an in-scope external + // 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`); + 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.`); + return; + } + + console.log(`Closing PR from ${author}: ${result.problems.join('; ') || 'out of scope'}`); await github.rest.issues.createComment({ owner: context.repo.owner, diff --git a/.github/workflows/external-pr-scope-guard.yml b/.github/workflows/external-pr-scope-guard.yml new file mode 100644 index 0000000..5827085 --- /dev/null +++ b/.github/workflows/external-pr-scope-guard.yml @@ -0,0 +1,52 @@ +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). +# +# Add the scope-guard job as a REQUIRED status check in branch protection for it to block merge. +# +# 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. + +on: + pull_request_target: + types: [opened, synchronize, reopened] + +permissions: + contents: read + pull-requests: read + +jobs: + scope-guard: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 # base repo (trusted) + - uses: actions/github-script@v7 + with: + script: | + const author = context.payload.pull_request.user.login; + + 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.`); + return; + } + + const { evaluate } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`); + const result = await evaluate({ github, context }); + + if (!result.ok) { + core.setFailed( + `Scope guard: a non-member PR may only ADD marketplace.json entries whose source repo already backs a live plugin here.\n - ` + + result.problems.join('\n - ') + ); + return; + } + console.log(`Scope guard passed: adds ${result.added.join(', ') || 'none'}, all from repos already live here.`);