diff --git a/.github/external-pr-allowed-sources.json b/.github/external-pr-allowed-sources.json deleted file mode 100644 index 6f4a1c10..00000000 --- a/.github/external-pr-allowed-sources.json +++ /dev/null @@ -1,20 +0,0 @@ -{ - "$comment": [ - "Source orgs/repos from which a pull request opened by a non-member is allowed to stay", - "open (instead of being auto-closed). Used by .github/workflows/close-external-prs.yml", - "and .github/workflows/external-pr-scope-guard.yml via .github/scripts/external-pr-scope.js.", - "", - "This keys on the SOURCE repo, not on individuals — no usernames are listed. A non-member", - "PR is in scope only if it ADDS marketplace.json entries whose source.url is under one of", - "these prefixes and changes nothing else. Being in scope grants ONLY the right to open a", - "reviewable PR: all CI still runs and a maintainer approval is still required before merge.", - "", - "Matching is case-insensitive and boundary-safe on the normalized https URL (leading", - "https:// and trailing .git removed): github.com/ui5 does NOT match github.com/ui5-evil." - ], - "allowed_sources": [ - "github.com/UI5/", - "github.com/SAP/", - "github.com/SAP-samples/" - ] -} diff --git a/.github/scripts/external-pr-scope.js b/.github/scripts/external-pr-scope.js index 62a51323..64c1ee07 100644 --- a/.github/scripts/external-pr-scope.js +++ b/.github/scripts/external-pr-scope.js @@ -1,33 +1,29 @@ 'use strict'; -// Shared logic for the external-PR allowlist (keyed on source org, not on individuals). +// 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. // -// A pull request opened by a non-member is "in scope" only if it ADDS plugin entries to -// .claude-plugin/marketplace.json whose source.url is under an allowlisted prefix -// (.github/external-pr-allowed-sources.json) and changes nothing else — no other files, -// no removals, no edits to existing entries. +// 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 the head marketplace.json as DATA via the API and parses it; -// it never checks out or executes head code. The allowlist + this script are read from the -// trusted base checkout. +// Security: evaluate() reads base + head marketplace.json as DATA via the API and parses them; +// it never checks out or executes head code. -const fs = require('fs'); const MARKETPLACE = '.claude-plugin/marketplace.json'; -function normalizeUrl(u) { - return String(u).trim().toLowerCase() +function normalizeRepo(u) { + return String(u || '').trim().toLowerCase() .replace(/^git\+/, '') .replace(/^https?:\/\//, '') .replace(/\.git$/, '') - .replace(/\/+$/, ''); // no trailing slash; matching adds the boundary -} - -function loadAllowed(allowlistPath) { - const j = JSON.parse(fs.readFileSync(allowlistPath, 'utf8')); - return (j.allowed_sources || []).map(normalizeUrl); + .replace(/\/+$/, ''); } function pluginsByName(json) { @@ -36,20 +32,26 @@ function pluginsByName(json) { return map; } -function sourceAllowed(url, allowed) { - const n = normalizeUrl(url); - if (n.split('/').length < 3) return false; // require a real host/org/repo path - // Boundary-safe: exact repo, or strictly under the allowed prefix. - return allowed.some(a => n === a || n.startsWith(a + '/')); +// 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 }. -function analyze({ changedFiles, base, head, allowed }) { +function analyze({ changedFiles, base, head }) { const problems = []; const off = changedFiles.filter(n => n !== MARKETPLACE); if (off.length) problems.push(`changes files other than ${MARKETPLACE}: ${off.join(', ')}`); + const liveRepos = liveReposOf(base); const baseNames = new Set(Object.keys(base)); const headNames = new Set(Object.keys(head)); const removed = [...baseNames].filter(n => !headNames.has(n)); @@ -65,14 +67,16 @@ function analyze({ changedFiles, base, head, allowed }) { } for (const name of added) { - const url = head[name] && head[name].source && head[name].source.url; - if (!url) { problems.push(`added "${name}" has no source.url to validate`); continue; } - if (!sourceAllowed(url, allowed)) { - problems.push(`added "${name}" points at ${url}, outside the allowed sources`); + const u = head[name] && head[name].source && head[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 }; + return { ok: problems.length === 0, problems, added, removed, modified, liveRepoCount: liveRepos.size }; } async function readPlugins(github, owner, repo, ref) { @@ -84,14 +88,11 @@ async function readPlugins(github, owner, repo, ref) { } } -// API wrapper used by both workflows. Fetches the diff and delegates to analyze(). -async function evaluate({ github, context, allowlistPath }) { +// 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 allowed = loadAllowed(allowlistPath); - if (!allowed.length) return { ok: false, problems: ['allowed_sources is empty'], added: [], removed: [], modified: [] }; - const files = await github.paginate(github.rest.pulls.listFiles, { owner, repo, pull_number: pr.number, per_page: 100, }); @@ -103,7 +104,7 @@ async function evaluate({ github, context, allowlistPath }) { return { ok: false, problems: ['could not read marketplace.json at base and/or head'], added: [], removed: [], modified: [] }; } - return analyze({ changedFiles, base, head, allowed }); + return analyze({ changedFiles, base, head }); } -module.exports = { normalizeUrl, sourceAllowed, analyze, readPlugins, evaluate, MARKETPLACE }; +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 46537bc9..b1fc2808 100644 --- a/.github/workflows/close-external-prs.yml +++ b/.github/workflows/close-external-prs.yml @@ -35,15 +35,13 @@ jobs: } // Non-member: allow the PR to stay open ONLY if it is an in-scope external - // contribution — it adds marketplace.json entries pointing at an allowlisted - // source org and changes nothing else (see .github/external-pr-allowed-sources.json). - // 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. + // 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, - allowlistPath: `${process.env.GITHUB_WORKSPACE}/.github/external-pr-allowed-sources.json`, - }); + 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; diff --git a/.github/workflows/external-pr-scope-guard.yml b/.github/workflows/external-pr-scope-guard.yml index a586f3ea..58270854 100644 --- a/.github/workflows/external-pr-scope-guard.yml +++ b/.github/workflows/external-pr-scope-guard.yml @@ -4,14 +4,14 @@ name: External PR Scope Guard # 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 under an allowlisted prefix -# in .github/external-pr-allowed-sources.json. +# 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 -# allowlist + script; the head marketplace.json is fetched as DATA via the API and parsed, -# never executed. +# shared script; the head marketplace.json is fetched as DATA via the API and parsed, never executed. on: pull_request_target: @@ -40,16 +40,13 @@ jobs: } const { evaluate } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`); - const result = await evaluate({ - github, context, - allowlistPath: `${process.env.GITHUB_WORKSPACE}/.github/external-pr-allowed-sources.json`, - }); + const result = await evaluate({ github, context }); if (!result.ok) { core.setFailed( - `Scope guard: a non-member PR may only ADD marketplace.json entries for an allowlisted source org.\n - ` + + `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 within allowed sources.`); + console.log(`Scope guard passed: adds ${result.added.join(', ') || 'none'}, all from repos already live here.`);