Przeglądaj źródła

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) <noreply@anthropic.com>
Bryan Thompson 3 miesięcy temu
rodzic
commit
3dc12ff7d6

+ 0 - 20
.github/external-pr-allowed-sources.json

@@ -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/"
-  ]
-}

+ 36 - 35
.github/scripts/external-pr-scope.js

@@ -1,33 +1,29 @@
 'use strict';
 '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:
 // Used by:
 //   - close-external-prs.yml      (skip the auto-close when in scope)
 //   - 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)
 //   - 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';
 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(/^git\+/, '')
     .replace(/^https?:\/\//, '')
     .replace(/^https?:\/\//, '')
     .replace(/\.git$/, '')
     .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) {
 function pluginsByName(json) {
@@ -36,20 +32,26 @@ function pluginsByName(json) {
   return map;
   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 }.
 // 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 problems = [];
 
 
   const off = changedFiles.filter(n => n !== MARKETPLACE);
   const off = changedFiles.filter(n => n !== MARKETPLACE);
   if (off.length) problems.push(`changes files other than ${MARKETPLACE}: ${off.join(', ')}`);
   if (off.length) problems.push(`changes files other than ${MARKETPLACE}: ${off.join(', ')}`);
 
 
+  const liveRepos = liveReposOf(base);
   const baseNames = new Set(Object.keys(base));
   const baseNames = new Set(Object.keys(base));
   const headNames = new Set(Object.keys(head));
   const headNames = new Set(Object.keys(head));
   const removed = [...baseNames].filter(n => !headNames.has(n));
   const removed = [...baseNames].filter(n => !headNames.has(n));
@@ -65,14 +67,16 @@ function analyze({ changedFiles, base, head, allowed }) {
   }
   }
 
 
   for (const name of added) {
   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) {
 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 pr = context.payload.pull_request;
   const owner = context.repo.owner, repo = context.repo.repo;
   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, {
   const files = await github.paginate(github.rest.pulls.listFiles, {
     owner, repo, pull_number: pr.number, per_page: 100,
     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 { 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 };

+ 6 - 8
.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
             // 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 { 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) {
             if (result.ok && result.added.length > 0) {
               console.log(`In-scope external contribution (adds: ${result.added.join(', ')}) — allowing PR.`);
               console.log(`In-scope external contribution (adds: ${result.added.join(', ')}) — allowing PR.`);
               return;
               return;

+ 7 - 10
.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
 # 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:
 # 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
 # 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.
 # 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
 # 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:
 on:
   pull_request_target:
   pull_request_target:
@@ -40,16 +40,13 @@ jobs:
             }
             }
 
 
             const { evaluate } = require(`${process.env.GITHUB_WORKSPACE}/.github/scripts/external-pr-scope.js`);
             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) {
             if (!result.ok) {
               core.setFailed(
               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 - ')
                 result.problems.join('\n - ')
               );
               );
               return;
               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.`);