-
Notifications
You must be signed in to change notification settings - Fork 8.3k
fix: Check org membership for community label workflow #11211
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe workflow changes from unconditionally labeling all pull requests as "community" to conditionally labeling only PRs from non-organization members. A membership check is added to fetch the PR author and verify if they belong to the repository owner's organization, with 404 responses treated as non-membership. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks and finishing touchesImportant Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 inconclusive)
✅ Passed checks (5 passed)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI Agents
In @.github/workflows/community-label.yml:
- Around line 28-31: The catch currently sets isOrgMember = false for all
errors; instead inspect the caught error (e.g., check error.status or
error.response?.status from Octokit) and only set isOrgMember = false when
status === 404 (confirmed non-member); for other statuses (network, 403, 429,
5xx) either rethrow or set a separate flag (e.g., isOrgMember = null or throw a
new Error) so the caller can treat "unable to determine" differently and avoid
incorrectly applying the community label—update the catch block that wraps the
membership check and downstream logic that consumes isOrgMember accordingly.
- Around line 17-31: The org membership check using
github.rest.orgs.checkMembershipForUser can misclassify private members because
the default GITHUB_TOKEN may lack read:org/Members permission; replace or
supplement this logic by first checking
context.payload.pull_request.author_association for MEMBER/OWNER/COLLABORATOR
and treat those as org members, and if you still need API confirmation use
github.rest.orgs.getMembership (or request an elevated token with
read:org/Members) instead of checkMembershipForUser; update the code paths
around context.payload.pull_request.user.login,
github.rest.orgs.checkMembershipForUser, and any token/permissions configuration
to prefer author_association fallback and only call getMembership when the token
has the required scope.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/community-label.yml
🔇 Additional comments (1)
.github/workflows/community-label.yml (1)
33-41: LGTM: Conditional labeling logic is correct.The conditional check and labeling logic are implemented correctly. The workflow appropriately adds the "community" label only when the user is not an organization member, and the API call is properly structured with all required parameters.
Note: The effectiveness of this logic depends on resolving the membership check issue flagged above.
| const author = context.payload.pull_request.user.login; | ||
| const org = context.repo.owner; | ||
| // Check if user is a member of the organization | ||
| let isOrgMember = false; | ||
| try { | ||
| await github.rest.orgs.checkMembershipForUser({ | ||
| org: org, | ||
| username: author | ||
| }); | ||
| isOrgMember = true; | ||
| } catch (error) { | ||
| // 404 means user is not a member, any other error we assume not a member | ||
| isOrgMember = false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🌐 Web query:
GitHub API orgs.checkMembershipForUser public membership only documentation
💡 Result:
Short answer: GitHub provides two separate REST endpoints.
-
GET /orgs/{org}/members/{username} — checks whether a user is a member (public or private). Requires appropriate auth/permissions; returns 204 if member, 404 otherwise. [1]
-
GET /orgs/{org}/public_members/{username} — checks whether a user is a public member only. Can be used without auth for public info; returns 204 if public member, 404 if not. [1]
Full docs (members + public_members) are in the GitHub REST API "Organization members" pages. [1]
Sources: GitHub REST API docs. [1]
🌐 Web query:
GitHub REST API check organization membership private vs public
💡 Result:
Short answer: use two REST endpoints.
-
Check if someone is any member (public or private): GET /orgs/:org/members/:username — returns 204 if the caller can see the membership (i.e., if caller is an org member or the membership is public), 404 if not visible. Authenticated org members see both public and private members. [1][2]
-
Check specifically whether the membership is public: GET /orgs/:org/public_members/:username — returns 204 if the user is a public member, 404 if not. Users can publicize/conceal their own membership with PUT/DELETE to the same path. [1][2]
Extra/details (behavior):
- If you need the membership state/role (admin/member, pending) use GET /orgs/:org/memberships/:username (or GET /user/memberships/orgs for the authenticated user) which returns JSON with "state" and "role" (requires appropriate auth/permissions). [1]
Sources: GitHub REST API docs for organization members. [1] [2]
References
[1] GitHub REST API — Organization members (check public membership / members endpoints).
[2] GitHub Enterprise / Enterprise Cloud docs — organization members (same endpoints and status codes).
🌐 Web query:
GitHub API alternative to check organization membership both public and private
💡 Result:
Use the REST Members endpoints:
-
To check public-only membership: GET /orgs/{org}/public_members/{username} — returns 204 if the user is a public member, 404 otherwise. [1]
-
To check membership including concealed (private) membership: GET /orgs/{org}/members/{username} (or the documented "Check organization membership for a user" REST endpoint). This returns 204 if the user is a member (public or private). The request must be authenticated with a token that has the organization "Members" read permission (or be an appropriate GitHub App/installation token). [1]
GraphQL options:
- For the authenticated viewer you can use organization.viewerIsAMember to know whether the current token’s user is a member. To inspect other users you can query membersWithRole/members, but membership privacy rules still apply (private memberships won’t be exposed unless the requester has permission). [2]
References:
[1] GitHub REST API — Organization members (Check organization membership / Check public membership).
[2] GitHub GraphQL — Organization object (viewerIsAMember / membersWithRole).
Critical: Organization membership check does not detect private members.
The code uses github.rest.orgs.checkMembershipForUser, which can check both public and private membership. However, the default GITHUB_TOKEN in GitHub Actions workflows lacks the necessary "Members" read permission to view private organization memberships. Consequently, private members will return a 404 response and be incorrectly labeled as "community" contributors.
To check all organization members (public and private), use one of these approaches:
- Use
github.rest.orgs.getMembership— Returns membership details (state, role) if the token has appropriate permissions. - Request elevated permissions — Configure the workflow to use a Personal Access Token or GitHub App with the
read:orgscope or "Members" read permission. - Use author_association fallback — Combine with the
pull_request.author_associationfield (which indicatesMEMBER,OWNER,COLLABORATOR, etc.) as a supplementary check. - Maintain a member list — Store organization member names in the repository and validate against it.
🤖 Prompt for AI Agents
In @.github/workflows/community-label.yml around lines 17 - 31, The org
membership check using github.rest.orgs.checkMembershipForUser can misclassify
private members because the default GITHUB_TOKEN may lack read:org/Members
permission; replace or supplement this logic by first checking
context.payload.pull_request.author_association for MEMBER/OWNER/COLLABORATOR
and treat those as org members, and if you still need API confirmation use
github.rest.orgs.getMembership (or request an elevated token with
read:org/Members) instead of checkMembershipForUser; update the code paths
around context.payload.pull_request.user.login,
github.rest.orgs.checkMembershipForUser, and any token/permissions configuration
to prefer author_association fallback and only call getMembership when the token
has the required scope.
| } catch (error) { | ||
| // 404 means user is not a member, any other error we assume not a member | ||
| isOrgMember = false; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improve error handling to distinguish error types.
The catch block treats all errors uniformly as "not a member", which means network failures, rate limits, or permission errors would incorrectly add the community label. Consider checking the error status code to differentiate between "confirmed non-member" (404) and "unable to determine" (other errors).
🔎 Suggested improvement
} catch (error) {
- // 404 means user is not a member, any other error we assume not a member
- isOrgMember = false;
+ // 404 means user is not a member
+ if (error.status === 404) {
+ isOrgMember = false;
+ } else {
+ // For other errors (rate limit, network, etc.), log and skip labeling
+ console.log(`Unable to check membership: ${error.message}`);
+ return; // Exit without labeling
+ }
}Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In @.github/workflows/community-label.yml around lines 28 - 31, The catch
currently sets isOrgMember = false for all errors; instead inspect the caught
error (e.g., check error.status or error.response?.status from Octokit) and only
set isOrgMember = false when status === 404 (confirmed non-member); for other
statuses (network, 403, 429, 5xx) either rethrow or set a separate flag (e.g.,
isOrgMember = null or throw a new Error) so the caller can treat "unable to
determine" differently and avoid incorrectly applying the community label—update
the catch block that wraps the membership check and downstream logic that
consumes isOrgMember accordingly.
Summary
The workflow was incorrectly adding the community label to PRs from org members. The author_association field in pull_request_target events doesn't reliably identify org members.
This change queries the GitHub API directly to check if the PR author is a member of the organization before deciding whether to add the community label.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.