feat: add centralized contributor governance workflow#3
Conversation
|
Warning Review limit reached
More reviews will be available in 41 minutes and 11 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate. For paid Pro and Pro+ PR reviews, CodeRabbit uses rolling per-developer review limits. Reviews become available again as older review attempts age out of the rolling limit window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughA new reusable GitHub Actions workflow file, ChangesContributor Governance Reusable Workflow
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
R-Lawton
force-pushed
the
feat/contributor-governance
branch
from
70b691a to
ef0cdfc
Compare
eguzki
left a comment
eguzki
left a comment
There was a problem hiding this comment.
I wonder about adding some documentation in our contributors guides related to this governance workflow. Maybe some template in this repo so others can render their own?
Adds a reusable GitHub Actions workflow that enforces contributor governance across Kuadrant repos: - Auto-labels new issues with triage/needs-triage - Protects triage labels from non-org-members - Closes PRs without a linked, triaged issue Org members are exempt from all enforcement. Requires ORG_MEMBER_TOKEN secret with read:org and repo scope. Signed-off-by: R-Lawton <rlawton@redhat.com>
R-Lawton
force-pushed
the
feat/contributor-governance
branch
from
ef0cdfc to
4ed8c61
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/contributor-governance.yml:
- Around line 319-341: The duplicate PR check only examines
closingIssuesReferences for existing open PRs, but the current PR uses a regex
fallback to detect plain references like `#123` without closing keywords. To
ensure consistency, apply the same regex fallback pattern used for the current
PR to the otherPrIssues check. This involves extracting issue references from
the other PR's body text using the same regex logic, not just relying on
closingIssuesReferences, so that PRs with informal issue references are also
properly detected as duplicates.
- Around line 205-207: The issueRegex pattern in the contributor-governance.yml
workflow does not recognize full GitHub issue URLs in the format
https://github.com/Kuadrant/<repo>/issues/<number>. Update the issueRegex
pattern to also match and extract the issue number from these complete GitHub
URLs, not just shorthand references like `#123` or Kuadrant/repo#123. This will
ensure that PRs containing full GitHub URLs in their body are correctly
identified as having linked issues.
- Around line 308-310: The current loop skips PRs authored by the same actor
using the condition `if (otherPr.user.login === actor) continue;`, which
prevents the one-active-PR-per-external-contributor rule from being enforced.
Remove this condition so that the loop checks all other active PRs by the same
external contributor, allowing the validation logic to detect and block multiple
active PRs from a single external contributor. Keep only the check that skips
the current PR by its number while ensuring the logic properly identifies when
an external contributor has other active PRs already in the repository.
- Around line 88-115: The workflow currently removes an unauthorised triage
label but only restores it with a default 'triage/needs-triage' label instead of
restoring the exact label that was removed. When the action is 'unlabeled',
capture the removed label name from context.payload.label.name (which is a
triage label based on the conditional check), and instead of only adding
'triage/needs-triage' when no triage label remains, directly re-add that
specific removed label to preserve the original triage status. This ensures that
removing a triage label like 'triage/accepted' restores 'triage/accepted' rather
than downgrading it to 'triage/needs-triage'.
- Around line 14-16: The workflow_call trigger in the on section does not
declare the ORG_MEMBER_TOKEN secret that is required by the
protect-triage-labels and check-pr jobs. Add a secrets section under
workflow_call to explicitly declare ORG_MEMBER_TOKEN as a required secret so
that callers of this reusable workflow are aware of the dependency at invocation
time rather than failing at runtime when the jobs attempt to access
secrets.ORG_MEMBER_TOKEN.
- Around line 313-318: The organisation membership check for other PR authors
lacks error handling for non-204 responses, unlike the primary actor check and
protect-triage-labels check elsewhere in the file. After the fetch call in the
otherRes section (around line 313-318), add error handling that throws an error
if the response status is neither 204 (member confirmed) nor 404 (non-member).
This ensures that 401, 403, and 5xx error responses are properly caught and
reported instead of falling through as if the user is a non-member, which could
cause an organisation member's PR to incorrectly block an external contributor's
PR.
- Around line 134-158: The job condition currently only triggers on pull_request
events, which prevents it from running when callers use the pull_request_target
trigger pattern needed for fork pull requests to access secrets. Since this job
performs only API-level operations without checking out untrusted code, modify
the if condition to allow both pull_request and pull_request_target event types
while keeping the action checks for opened and reopened. Update the condition to
include both event names using OR logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 350255b3-7261-41d4-ace0-8096ce615a61
📒 Files selected for processing (1)
.github/workflows/contributor-governance.yml
| on: | ||
| workflow_call: | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/contributor-governance.ymlRepository: Kuadrant/.github
Length of output: 17240
Declare ORG_MEMBER_TOKEN as a required reusable-workflow secret.
The workflow dereferences secrets.ORG_MEMBER_TOKEN in both protect-triage-labels (line 53) and check-pr (line 143) jobs, but the workflow_call contract does not declare this secret. This defers caller misconfiguration to runtime, rather than failing fast at workflow invocation.
Proposed fix
on:
workflow_call:
+ secrets:
+ ORG_MEMBER_TOKEN:
+ required: true📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| on: | |
| workflow_call: | |
| on: | |
| workflow_call: | |
| secrets: | |
| ORG_MEMBER_TOKEN: | |
| required: true |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 14 - 16, The
workflow_call trigger in the on section does not declare the ORG_MEMBER_TOKEN
secret that is required by the protect-triage-labels and check-pr jobs. Add a
secrets section under workflow_call to explicitly declare ORG_MEMBER_TOKEN as a
required secret so that callers of this reusable workflow are aware of the
dependency at invocation time rather than failing at runtime when the jobs
attempt to access secrets.ORG_MEMBER_TOKEN.
| // If they added a label, remove it first | ||
| if (context.payload.action === 'labeled') { | ||
| await github.rest.issues.removeLabel({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| name: label | ||
| }); | ||
| } | ||
|
|
||
| // Re-fetch to see what triage labels remain | ||
| const issue = await github.rest.issues.get({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number | ||
| }); | ||
|
|
||
| const hasTriageLabel = issue.data.labels.some(l => l.name.startsWith('triage/')); | ||
|
|
||
| // An issue should never have zero triage labels | ||
| if (!hasTriageLabel) { | ||
| await github.rest.issues.addLabels({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: context.issue.number, | ||
| labels: ['triage/needs-triage'] | ||
| }); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Restore the exact triage label removed by a non-member.
On unlabeled, the workflow only adds triage/needs-triage when no triage label remains, so removing triage/accepted permanently downgrades the issue instead of reverting the unauthorised change.
Proposed fix
if (context.payload.action === 'labeled') {
await github.rest.issues.removeLabel({
owner: context.repo.owner,
repo: context.repo.repo,
issue_number: context.issue.number,
name: label
});
+ } else if (context.payload.action === 'unlabeled') {
+ await github.rest.issues.addLabels({
+ owner: context.repo.owner,
+ repo: context.repo.repo,
+ issue_number: context.issue.number,
+ labels: [label]
+ });
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // If they added a label, remove it first | |
| if (context.payload.action === 'labeled') { | |
| await github.rest.issues.removeLabel({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| name: label | |
| }); | |
| } | |
| // Re-fetch to see what triage labels remain | |
| const issue = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number | |
| }); | |
| const hasTriageLabel = issue.data.labels.some(l => l.name.startsWith('triage/')); | |
| // An issue should never have zero triage labels | |
| if (!hasTriageLabel) { | |
| await github.rest.issues.addLabels({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| labels: ['triage/needs-triage'] | |
| }); | |
| } | |
| // If they added a label, remove it first | |
| if (context.payload.action === 'labeled') { | |
| await github.rest.issues.removeLabel({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| name: label | |
| }); | |
| } else if (context.payload.action === 'unlabeled') { | |
| await github.rest.issues.addLabels({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| labels: [label] | |
| }); | |
| } | |
| // Re-fetch to see what triage labels remain | |
| const issue = await github.rest.issues.get({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number | |
| }); | |
| const hasTriageLabel = issue.data.labels.some(l => l.name.startsWith('triage/')); | |
| // An issue should never have zero triage labels | |
| if (!hasTriageLabel) { | |
| await github.rest.issues.addLabels({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| issue_number: context.issue.number, | |
| labels: ['triage/needs-triage'] | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 88 - 115, The
workflow currently removes an unauthorised triage label but only restores it
with a default 'triage/needs-triage' label instead of restoring the exact label
that was removed. When the action is 'unlabeled', capture the removed label name
from context.payload.label.name (which is a triage label based on the
conditional check), and instead of only adding 'triage/needs-triage' when no
triage label remains, directly re-add that specific removed label to preserve
the original triage status. This ensures that removing a triage label like
'triage/accepted' restores 'triage/accepted' rather than downgrading it to
'triage/needs-triage'.
| if: github.event_name == 'pull_request' && (github.event.action == 'opened' || github.event.action == 'reopened') | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| pull-requests: write | ||
| issues: read | ||
| steps: | ||
| - name: Check PR requirements | ||
| uses: actions/github-script@v7 | ||
| env: | ||
| ORG_TOKEN: ${{ secrets.ORG_MEMBER_TOKEN }} | ||
| with: | ||
| script: | | ||
| const actor = context.payload.pull_request.user.login; | ||
| const org = context.repo.owner; | ||
|
|
||
| // --- Org membership check: org members skip all PR rules --- | ||
| const res = await fetch( | ||
| `https://api.github.com/orgs/${org}/members/${actor}`, | ||
| { headers: { authorization: `token ${process.env.ORG_TOKEN}` } } | ||
| ); | ||
| if (res.status === 204) { | ||
| console.log(`${actor} is an org member, skipping PR checks`); | ||
| return; | ||
| } | ||
| if (res.status !== 404) throw new Error(`Org membership check failed: ${res.status}`); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
# First, locate and examine the workflow file
find . -name "contributor-governance.yml" -type fRepository: Kuadrant/.github
Length of output: 201
🏁 Script executed:
# Examine the workflow file, specifically around lines 134-158
cat -n ./.github/workflows/contributor-governance.yml | sed -n '130,165p'Repository: Kuadrant/.github
Length of output: 1980
🌐 Web query:
github actions pull_request vs pull_request_target fork secrets permissions
💡 Result:
The difference between pull_request and pull_request_target in GitHub Actions centers on trust, security context, and access to secrets when dealing with pull requests from forks [1][2][3]. pull_request This event is designed for running CI/CD tasks on the code submitted in a pull request [1][2]. When a pull request originates from a fork, GitHub treats the event as untrusted [1][4]. - Security Context: The workflow runs using the workflow file and code from the pull request's merge commit [1][2]. - Secrets & Permissions: To prevent abuse, it receives a read-only GITHUB_TOKEN and has no access to repository or organization secrets [1][5][2]. - Purpose: Safe for building, testing, and linting submitted code [1]. pull_request_target This event was created to allow workflows to interact with pull requests from forks in a trusted context (e.g., labeling PRs, posting status checks, or managing repository metadata) [1][5][2]. - Security Context: The workflow runs using the workflow file and code from the repository's base (default) branch, not the PR branch [1][2]. Because this code is trusted, the workflow is granted higher privileges [1]. - Secrets & Permissions: It receives access to repository and organization secrets and a read/write GITHUB_TOKEN [1][2][3]. - Critical Risk: If a workflow triggered by pull_request_target explicitly checks out and executes code from the PR head, it essentially "promotes" untrusted code into a privileged environment [5][6][3]. This is known as a pwn request [6][3]. Security Best Practices - Prefer pull_request over pull_request_target whenever possible [7][4]. - If using pull_request_target, never check out and run untrusted code from the fork unless absolutely necessary and properly secured [1][7][6]. - As of June 2026, actions/checkout (v7+) now defaults to refusing to fetch fork pull request code in pull_request_target workflows to prevent accidental pwn requests [6]. If you must check out fork code, you must explicitly use the allow-unsafe-pr-checkout flag [6]. - For complex workflows, use the pull_request event to perform untrusted work (e.g., building), store results as artifacts, and then use the workflow_run event to perform privileged operations (e.g., commenting or updating the repo) based on the successful completion of the first workflow [4]. - Use conditions to restrict workflows to specific users or organizations: github.event.pull_request.head.repo.owner.login == "your-org" [3].
Citations:
- 1: https://docs.github.com/en/actions/reference/security/securely-using-pull_request_target
- 2: https://stackoverflow.com/questions/74957218/what-is-the-difference-between-pull-request-and-pull-request-target-event-in-git
- 3: https://securitylab.github.com/resources/github-actions-new-patterns-and-mitigations/
- 4: https://codeql.github.com/codeql-query-help/actions/actions-untrusted-checkout-critical/
- 5: https://saschb2b.com/blog/pull-request-target-trap
- 6: https://github.blog/changelog/2026-06-18-safer-pull_request_target-defaults-for-github-actions-checkout/
- 7: https://github.blog/changelog/2025-11-07-actions-pull_request_target-and-environment-branch-protections-changes/
🏁 Script executed:
# Check the entire workflow step to see if it checks out PR code
cat -n ./.github/workflows/contributor-governance.yml | sed -n '140,200p'Repository: Kuadrant/.github
Length of output: 2938
🏁 Script executed:
# Look for any checkout or code execution steps in the entire workflow
cat -n ./.github/workflows/contributor-governance.yml | head -150Repository: Kuadrant/.github
Length of output: 7123
🏁 Script executed:
# Check if there are any caller workflows that invoke this reusable workflow
grep -r "contributor-governance" . --include="*.yml" --include="*.yaml" | grep -v "\.github/workflows/contributor-governance.yml"Repository: Kuadrant/.github
Length of output: 154
🏁 Script executed:
# Search more broadly in the repository structure
find . -type f -name "*.yml" -o -name "*.yaml" | head -20Repository: Kuadrant/.github
Length of output: 216
🏁 Script executed:
# Let's check the entire check-pr job to understand its complete scope
cat -n ./.github/workflows/contributor-governance.yml | sed -n '133,300p' | head -100Repository: Kuadrant/.github
Length of output: 4757
🏁 Script executed:
# Check the rest of the check-pr job to confirm no code checkout
cat -n ./.github/workflows/contributor-governance.yml | sed -n '232,350p'Repository: Kuadrant/.github
Length of output: 6041
🏁 Script executed:
# Check the final part of the job
cat -n ./.github/workflows/contributor-governance.yml | tail -20Repository: Kuadrant/.github
Length of output: 1153
Allow governance checks to run under pull_request_target trigger for fork pull requests.
This reusable workflow is limited to pull_request events, which cannot access the ORG_MEMBER_TOKEN secret when triggered from forked repositories. Callers using the safer pull_request_target pattern (recommended for fork PRs by GitHub) cannot invoke these governance checks. Since the job performs only API-level operations without checking out untrusted code, it is safe to enable both triggers.
Proposed fix
- if: github.event_name == 'pull_request' && (github.event.action == 'opened' || github.event.action == 'reopened')
+ if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && (github.event.action == 'opened' || github.event.action == 'reopened')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if: github.event_name == 'pull_request' && (github.event.action == 'opened' || github.event.action == 'reopened') | |
| runs-on: ubuntu-latest | |
| permissions: | |
| pull-requests: write | |
| issues: read | |
| steps: | |
| - name: Check PR requirements | |
| uses: actions/github-script@v7 | |
| env: | |
| ORG_TOKEN: ${{ secrets.ORG_MEMBER_TOKEN }} | |
| with: | |
| script: | | |
| const actor = context.payload.pull_request.user.login; | |
| const org = context.repo.owner; | |
| // --- Org membership check: org members skip all PR rules --- | |
| const res = await fetch( | |
| `https://api.github.com/orgs/${org}/members/${actor}`, | |
| { headers: { authorization: `token ${process.env.ORG_TOKEN}` } } | |
| ); | |
| if (res.status === 204) { | |
| console.log(`${actor} is an org member, skipping PR checks`); | |
| return; | |
| } | |
| if (res.status !== 404) throw new Error(`Org membership check failed: ${res.status}`); | |
| if: (github.event_name == 'pull_request' || github.event_name == 'pull_request_target') && (github.event.action == 'opened' || github.event.action == 'reopened') | |
| runs-on: ubuntu-latest | |
| permissions: | |
| pull-requests: write | |
| issues: read | |
| steps: | |
| - name: Check PR requirements | |
| uses: actions/github-script@v7 | |
| env: | |
| ORG_TOKEN: ${{ secrets.ORG_MEMBER_TOKEN }} | |
| with: | |
| script: | | |
| const actor = context.payload.pull_request.user.login; | |
| const org = context.repo.owner; | |
| // --- Org membership check: org members skip all PR rules --- | |
| const res = await fetch( | |
| `https://api.github.com/orgs/${org}/members/${actor}`, | |
| { headers: { authorization: `token ${process.env.ORG_TOKEN}` } } | |
| ); | |
| if (res.status === 204) { | |
| console.log(`${actor} is an org member, skipping PR checks`); | |
| return; | |
| } | |
| if (res.status !== 404) throw new Error(`Org membership check failed: ${res.status}`); |
🧰 Tools
🪛 zizmor (1.26.1)
[error] 141-141: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 134 - 158, The job
condition currently only triggers on pull_request events, which prevents it from
running when callers use the pull_request_target trigger pattern needed for fork
pull requests to access secrets. Since this job performs only API-level
operations without checking out untrusted code, modify the if condition to allow
both pull_request and pull_request_target event types while keeping the action
checks for opened and reopened. Update the condition to include both event names
using OR logic.
| // Method 2: Regex — catches #N references in title/body not formally linked | ||
| const issueRegex = /(?:(?:closes?|fixe?s?|resolves?)\s+)?(?:Kuadrant\/[\w-]+)?#(\d+)/gi; | ||
| const text = `${prTitle}\n${prBody}`; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Recognise full GitHub issue URLs as linked issues.
The fallback only detects #123/Kuadrant/repo#123; a PR body containing https://github.com/Kuadrant/<repo>/issues/123 is treated as unlinked and closed despite satisfying the “linked issue” requirement.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 205 - 207, The
issueRegex pattern in the contributor-governance.yml workflow does not recognize
full GitHub issue URLs in the format
https://github.com/Kuadrant/<repo>/issues/<number>. Update the issueRegex
pattern to also match and extract the issue number from these complete GitHub
URLs, not just shorthand references like `#123` or Kuadrant/repo#123. This will
ensure that PRs containing full GitHub URLs in their body are correctly
identified as having linked issues.
| for (const otherPr of prsInRepo.data) { | ||
| if (otherPr.number === prNumber) continue; | ||
| if (otherPr.user.login === actor) continue; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Implement the one-active-PR-per-external-contributor rule.
The PR objective says additional external submissions should be drafted, but this loop explicitly skips PRs by the same actor and only blocks different contributors targeting the same issue. One external contributor can still keep multiple active PRs in the repository.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 308 - 310, The
current loop skips PRs authored by the same actor using the condition `if
(otherPr.user.login === actor) continue;`, which prevents the
one-active-PR-per-external-contributor rule from being enforced. Remove this
condition so that the loop checks all other active PRs by the same external
contributor, allowing the validation logic to detect and block multiple active
PRs from a single external contributor. Keep only the check that skips the
current PR by its number while ensuring the logic properly identifies when an
external contributor has other active PRs already in the repository.
| const otherRes = await fetch( | ||
| `https://api.github.com/orgs/${org}/members/${otherPr.user.login}`, | ||
| { headers: { authorization: `token ${process.env.ORG_TOKEN}` } } | ||
| ); | ||
| if (otherRes.status === 204) continue; | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
cat -n .github/workflows/contributor-governance.yml | head -350 | tail -50Repository: Kuadrant/.github
Length of output: 2779
🏁 Script executed:
cat -n .github/workflows/contributor-governance.yml | head -310 | tail -80Repository: Kuadrant/.github
Length of output: 3909
🏁 Script executed:
cat -n .github/workflows/contributor-governance.yml | head -200Repository: Kuadrant/.github
Length of output: 9431
Add error handling to the organisation membership check for other PR authors.
The primary actor check (line 158) and the protect-triage-labels check (lines 82–84) both throw an error if the membership API returns a non-404 status. This other PR author check lacks the same safeguard, allowing 401, 403, and 5xx responses to fall through as if the user is confirmed as a non-member. This can cause an organisation member's PR to incorrectly block an external contributor's PR.
Proposed fix
);
if (otherRes.status === 204) continue;
+ if (otherRes.status !== 404) {
+ throw new Error(`Org membership check failed for ${otherPr.user.login}: ${otherRes.status}`);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const otherRes = await fetch( | |
| `https://api.github.com/orgs/${org}/members/${otherPr.user.login}`, | |
| { headers: { authorization: `token ${process.env.ORG_TOKEN}` } } | |
| ); | |
| if (otherRes.status === 204) continue; | |
| const otherRes = await fetch( | |
| `https://api.github.com/orgs/${org}/members/${otherPr.user.login}`, | |
| { headers: { authorization: `token ${process.env.ORG_TOKEN}` } } | |
| ); | |
| if (otherRes.status === 204) continue; | |
| if (otherRes.status !== 404) { | |
| throw new Error(`Org membership check failed for ${otherPr.user.login}: ${otherRes.status}`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 313 - 318, The
organisation membership check for other PR authors lacks error handling for
non-204 responses, unlike the primary actor check and protect-triage-labels
check elsewhere in the file. After the fetch call in the otherRes section
(around line 313-318), add error handling that throws an error if the response
status is neither 204 (member confirmed) nor 404 (non-member). This ensures that
401, 403, and 5xx error responses are properly caught and reported instead of
falling through as if the user is a non-member, which could cause an
organisation member's PR to incorrectly block an external contributor's PR.
| // Check if the other PR references the same issue | ||
| const otherPrQuery = ` | ||
| query($owner: String!, $repo: String!, $number: Int!) { | ||
| repository(owner: $owner, name: $repo) { | ||
| pullRequest(number: $number) { | ||
| closingIssuesReferences(first: 100) { | ||
| nodes { | ||
| number | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
| `; | ||
|
|
||
| const otherPrResult = await github.graphql(otherPrQuery, { | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| number: otherPr.number | ||
| }); | ||
|
|
||
| const otherPrIssues = otherPrResult.repository.pullRequest.closingIssuesReferences.nodes || []; | ||
| const otherPrIssueNumbers = otherPrIssues.map(i => i.number); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Reuse the regex fallback when checking existing open PRs.
The current PR can pass with a plain #123 reference, but the duplicate check only inspects closingIssuesReferences for other PRs. An existing open PR that references the same issue without a closing keyword is invisible, so duplicate work is allowed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/contributor-governance.yml around lines 319 - 341, The
duplicate PR check only examines closingIssuesReferences for existing open PRs,
but the current PR uses a regex fallback to detect plain references like `#123`
without closing keywords. To ensure consistency, apply the same regex fallback
pattern used for the current PR to the otherPrIssues check. This involves
extracting issue references from the other PR's body text using the same regex
logic, not just relying on closingIssuesReferences, so that PRs with informal
issue references are also properly detected as duplicates.
- CONTRIBUTING.md explains the triage process and PR requirements - workflow-templates/ provides a GitHub-native template that repos can adopt from the Actions > New Workflow page Signed-off-by: R-Lawton <rlawton@redhat.com>
2 participants
adds a reusable GitHub Actions workflow that enforces contributor governance across Kuadrant repos:
triage/needs-triagetarget repos adopt this via a thin caller workflow (~15 lines) with
secrets: inherit.TEST REPO https://github.com/Kuadrant/governance-test
requires
ORG_MEMBER_TOKENwithread:orgscope (done)triage/*labels created in target repos (separate rollout step)Summary by CodeRabbit