From 87149d74c8ab5fb73d59f3dd1d87184056f0f59e Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Fri, 27 Mar 2026 09:38:15 +0100 Subject: [PATCH 1/5] ci: Add workflow to close unvetted non-maintainer PRs Automatically closes PRs from non-maintainers that don't meet contribution requirements: must reference a getsentry issue with prior discussion between the PR author and a maintainer, and the issue must not be assigned to someone else. Adds the 'violating-contribution-guidelines' label and posts a reason-specific comment explaining next steps. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/close-unvetted-pr.yml | 243 ++++++++++++++++++++++++ 1 file changed, 243 insertions(+) create mode 100644 .github/workflows/close-unvetted-pr.yml diff --git a/.github/workflows/close-unvetted-pr.yml b/.github/workflows/close-unvetted-pr.yml new file mode 100644 index 0000000000..b3bb73260f --- /dev/null +++ b/.github/workflows/close-unvetted-pr.yml @@ -0,0 +1,243 @@ +name: Close Unvetted Non-Maintainer PRs + +on: + pull_request_target: + types: [opened] + +jobs: + validate-non-maintainer-pr: + name: Validate Non-Maintainer PR + runs-on: ubuntu-24.04 + permissions: + pull-requests: write + contents: write + steps: + - name: Generate GitHub App token + id: app-token + uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v2 + with: + app-id: ${{ vars.SDK_MAINTAINER_BOT_APP_ID }} + private-key: ${{ secrets.SDK_MAINTAINER_BOT_PRIVATE_KEY }} + + - name: Validate PR + uses: actions/github-script@60a0d83039c74a4aee543508d2ffcb1c3799cdea # v7.0.1 + with: + github-token: ${{ steps.app-token.outputs.token }} + script: | + const pullRequest = context.payload.pull_request; + const repo = context.repo; + const prAuthor = pullRequest.user.login; + const contributingUrl = `https://github.com/${repo.owner}/${repo.repo}/blob/master/CONTRIBUTING.md`; + + // --- Helper: check if a user has write+ permission on a repo --- + async function hasWriteAccess(owner, repoName, username) { + try { + const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ + owner, + repo: repoName, + username, + }); + return ['admin', 'maintain', 'write'].includes(data.permission); + } catch { + return false; + } + } + + // --- Step 1: Check if PR author is a maintainer --- + const isMaintainer = await hasWriteAccess(repo.owner, repo.repo, prAuthor); + if (isMaintainer) { + core.info(`PR author ${prAuthor} has write+ access. Skipping.`); + return; + } + core.info(`PR author ${prAuthor} is not a maintainer.`); + + // --- Step 2: Parse issue references from PR body --- + const body = pullRequest.body || ''; + + // Match all issue reference formats: + // #123, Fixes #123, getsentry/repo#123, Fixes getsentry/repo#123 + // https://github.com/getsentry/repo/issues/123 + const issueRefs = []; + const seen = new Set(); + + // Pattern 1: Full GitHub URLs + const urlPattern = /https?:\/\/github\.com\/(getsentry)\/([\w.-]+)\/issues\/(\d+)/gi; + for (const match of body.matchAll(urlPattern)) { + const key = `${match[1]}/${match[2]}#${match[3]}`; + if (!seen.has(key)) { + seen.add(key); + issueRefs.push({ owner: match[1], repo: match[2], number: parseInt(match[3]) }); + } + } + + // Pattern 2: Cross-repo references (getsentry/repo#123) + const crossRepoPattern = /(?:(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)\s+)?(getsentry)\/([\w.-]+)#(\d+)/gi; + for (const match of body.matchAll(crossRepoPattern)) { + const key = `${match[1]}/${match[2]}#${match[3]}`; + if (!seen.has(key)) { + seen.add(key); + issueRefs.push({ owner: match[1], repo: match[2], number: parseInt(match[3]) }); + } + } + + // Pattern 3: Same-repo references (#123) + // Negative lookbehind to avoid matching cross-repo refs or URLs already captured + const sameRepoPattern = /(?:(?:fix|fixes|fixed|close|closes|closed|resolve|resolves|resolved)\s+)?(? 0) { + const assignedToAuthor = issue.assignees.some(a => a.login === prAuthor); + if (!assignedToAuthor) { + core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} is assigned to someone else.`); + hasAssigneeConflict = true; + continue; + } + } + + // Check discussion: both PR author and a maintainer must have commented + const comments = await github.paginate(github.rest.issues.listComments, { + owner: ref.owner, + repo: ref.repo, + issue_number: ref.number, + per_page: 100, + }); + + // Also consider the issue author as a participant (opening the issue is a form of discussion) + const prAuthorParticipated = + issue.user.login === prAuthor || + comments.some(c => c.user.login === prAuthor); + + let maintainerParticipated = false; + if (prAuthorParticipated) { + // Check each commenter (and issue author) for write+ access on the issue's repo + const usersToCheck = new Set(); + usersToCheck.add(issue.user.login); + for (const comment of comments) { + if (comment.user.login !== prAuthor) { + usersToCheck.add(comment.user.login); + } + } + + for (const user of usersToCheck) { + if (user === prAuthor) continue; + if (await hasWriteAccess(ref.owner, ref.repo, user)) { + maintainerParticipated = true; + core.info(`Maintainer ${user} participated in ${ref.owner}/${ref.repo}#${ref.number}.`); + break; + } + } + } + + if (prAuthorParticipated && maintainerParticipated) { + core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} has valid discussion. PR is allowed.`); + return; // PR is valid — at least one issue passes all checks + } + + core.info(`Issue ${ref.owner}/${ref.repo}#${ref.number} lacks discussion between author and maintainer.`); + hasNoDiscussion = true; + } + + // --- Step 5: No valid issue found — close with the most relevant reason --- + if (hasAssigneeConflict) { + core.info('Closing PR: referenced issue is assigned to someone else.'); + await closePR([ + 'This PR has been automatically closed. The referenced issue is already assigned to someone else.', + '', + 'If you believe this assignment is outdated, please comment on the issue to discuss before opening a new PR.', + '', + `Please review our [contributing guidelines](${contributingUrl}) for more details.`, + ].join('\n')); + return; + } + + if (hasNoDiscussion) { + core.info('Closing PR: no discussion between PR author and a maintainer in the referenced issue.'); + await closePR([ + 'This PR has been automatically closed. The referenced issue does not show a discussion between you and a maintainer.', + '', + 'To avoid wasted effort on both sides, please discuss your proposed approach in the issue first and wait for a maintainer to respond before opening a PR.', + '', + `Please review our [contributing guidelines](${contributingUrl}) for more details.`, + ].join('\n')); + return; + } + + // If we get here, all issue refs were unfetchable + core.info('Could not validate any referenced issues. Closing PR.'); + await closePR([ + 'This PR has been automatically closed. The referenced issue(s) could not be found.', + '', + '**Next steps:**', + '1. Ensure the issue exists and is in a `getsentry` repository', + '2. Discuss the approach with a maintainer in the issue', + '3. Once a maintainer has acknowledged your proposed approach, open a new PR referencing the issue', + '', + `Please review our [contributing guidelines](${contributingUrl}) for more details.`, + ].join('\n')); From 0ab97fea5722326b6494fd639cc66cac978c619c Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Fri, 27 Mar 2026 09:42:47 +0100 Subject: [PATCH 2/5] ci: Restrict maintainer check to admin/maintain roles Everyone at Sentry has write access to this repo, so write-level permission is too broad for the maintainer bypass. Only users with admin or maintain roles should skip the contribution validation. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/close-unvetted-pr.yml | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/.github/workflows/close-unvetted-pr.yml b/.github/workflows/close-unvetted-pr.yml index b3bb73260f..5b5f67e9f6 100644 --- a/.github/workflows/close-unvetted-pr.yml +++ b/.github/workflows/close-unvetted-pr.yml @@ -29,24 +29,24 @@ jobs: const prAuthor = pullRequest.user.login; const contributingUrl = `https://github.com/${repo.owner}/${repo.repo}/blob/master/CONTRIBUTING.md`; - // --- Helper: check if a user has write+ permission on a repo --- - async function hasWriteAccess(owner, repoName, username) { + // --- Helper: check if a user has admin or maintain permission on a repo --- + async function isMaintainer(owner, repoName, username) { try { const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo: repoName, username, }); - return ['admin', 'maintain', 'write'].includes(data.permission); + return ['admin', 'maintain'].includes(data.permission); } catch { return false; } } - // --- Step 1: Check if PR author is a maintainer --- - const isMaintainer = await hasWriteAccess(repo.owner, repo.repo, prAuthor); - if (isMaintainer) { - core.info(`PR author ${prAuthor} has write+ access. Skipping.`); + // --- Step 1: Check if PR author is a maintainer (admin or maintain role) --- + const authorIsMaintainer = await isMaintainer(repo.owner, repo.repo, prAuthor); + if (authorIsMaintainer) { + core.info(`PR author ${prAuthor} has admin/maintain access. Skipping.`); return; } core.info(`PR author ${prAuthor} is not a maintainer.`); @@ -187,7 +187,7 @@ jobs: for (const user of usersToCheck) { if (user === prAuthor) continue; - if (await hasWriteAccess(ref.owner, ref.repo, user)) { + if (await isMaintainer(ref.owner, ref.repo, user)) { maintainerParticipated = true; core.info(`Maintainer ${user} participated in ${ref.owner}/${ref.repo}#${ref.number}.`); break; From 6c80dc569e18cfb2bd4d64d16b85224ea3e3ce9e Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Fri, 27 Mar 2026 09:54:06 +0100 Subject: [PATCH 3/5] ci: Cache maintainer lookups and guard against null users Cache isMaintainer API results to avoid redundant permission checks when the same users appear across multiple referenced issues. Add null guards for issue.user and comment.user, which GitHub's API returns as null for deleted or suspended accounts. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/close-unvetted-pr.yml | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/.github/workflows/close-unvetted-pr.yml b/.github/workflows/close-unvetted-pr.yml index 5b5f67e9f6..f4f6830dcb 100644 --- a/.github/workflows/close-unvetted-pr.yml +++ b/.github/workflows/close-unvetted-pr.yml @@ -29,18 +29,24 @@ jobs: const prAuthor = pullRequest.user.login; const contributingUrl = `https://github.com/${repo.owner}/${repo.repo}/blob/master/CONTRIBUTING.md`; - // --- Helper: check if a user has admin or maintain permission on a repo --- + // --- Helper: check if a user has admin or maintain permission on a repo (cached) --- + const maintainerCache = new Map(); async function isMaintainer(owner, repoName, username) { + const key = `${owner}/${repoName}:${username}`; + if (maintainerCache.has(key)) return maintainerCache.get(key); + let result = false; try { const { data } = await github.rest.repos.getCollaboratorPermissionLevel({ owner, repo: repoName, username, }); - return ['admin', 'maintain'].includes(data.permission); + result = ['admin', 'maintain'].includes(data.permission); } catch { - return false; + // noop — result stays false } + maintainerCache.set(key, result); + return result; } // --- Step 1: Check if PR author is a maintainer (admin or maintain role) --- @@ -170,17 +176,18 @@ jobs: }); // Also consider the issue author as a participant (opening the issue is a form of discussion) + // Guard against null user (deleted/suspended GitHub accounts) const prAuthorParticipated = - issue.user.login === prAuthor || - comments.some(c => c.user.login === prAuthor); + issue.user?.login === prAuthor || + comments.some(c => c.user?.login === prAuthor); let maintainerParticipated = false; if (prAuthorParticipated) { - // Check each commenter (and issue author) for write+ access on the issue's repo + // Check each commenter (and issue author) for admin/maintain access on the issue's repo const usersToCheck = new Set(); - usersToCheck.add(issue.user.login); + if (issue.user?.login) usersToCheck.add(issue.user.login); for (const comment of comments) { - if (comment.user.login !== prAuthor) { + if (comment.user?.login && comment.user.login !== prAuthor) { usersToCheck.add(comment.user.login); } } From d760912a0bd00eb28d8db9cb29bd3c06da041c2c Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Fri, 27 Mar 2026 10:08:48 +0100 Subject: [PATCH 4/5] fix(ci): Use role_name instead of permission for maintainer check The GitHub API's permission field uses legacy values where the maintain role is mapped to write, making it impossible to distinguish maintainers from regular write-access users. The role_name field provides the actual assigned role. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/close-unvetted-pr.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/close-unvetted-pr.yml b/.github/workflows/close-unvetted-pr.yml index f4f6830dcb..c0e7e3ff1f 100644 --- a/.github/workflows/close-unvetted-pr.yml +++ b/.github/workflows/close-unvetted-pr.yml @@ -41,7 +41,9 @@ jobs: repo: repoName, username, }); - result = ['admin', 'maintain'].includes(data.permission); + // permission field uses legacy values (admin/write/read/none) where + // maintain maps to write. Use role_name for the actual role. + result = ['admin', 'maintain'].includes(data.role_name); } catch { // noop — result stays false } From 4f91b11feb104c2b7b53bbedcddc4c2c10cf95d7 Mon Sep 17 00:00:00 2001 From: Stephanie Anderson Date: Fri, 27 Mar 2026 10:16:03 +0100 Subject: [PATCH 5/5] fix(ci): Check maintainer status against PR repo, not issue repo When a PR references a cross-repo issue, the discussion participants should be checked for maintainer status on the PR's target repo, not the issue's repo. A sentry-python maintainer commenting on a getsentry/sentry issue should still count as vetting the work. Co-Authored-By: Claude Opus 4.6 (1M context) --- .github/workflows/close-unvetted-pr.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/close-unvetted-pr.yml b/.github/workflows/close-unvetted-pr.yml index c0e7e3ff1f..c4e4c97a00 100644 --- a/.github/workflows/close-unvetted-pr.yml +++ b/.github/workflows/close-unvetted-pr.yml @@ -196,7 +196,7 @@ jobs: for (const user of usersToCheck) { if (user === prAuthor) continue; - if (await isMaintainer(ref.owner, ref.repo, user)) { + if (await isMaintainer(repo.owner, repo.repo, user)) { maintainerParticipated = true; core.info(`Maintainer ${user} participated in ${ref.owner}/${ref.repo}#${ref.number}.`); break;