Skip to content

fix: use collaborator check and CLA_ACCESS_TOKEN for membership verification#3

Merged
bashandbone merged 3 commits intomainfrom
copilot/fix-cla-check-issue
Dec 2, 2025
Merged

fix: use collaborator check and CLA_ACCESS_TOKEN for membership verification#3
bashandbone merged 3 commits intomainfrom
copilot/fix-cla-check-issue

Conversation

Copy link
Contributor

Copilot AI commented Dec 2, 2025

The CLA workflow incorrectly required org members to sign the CLA because GITHUB_TOKEN lacks read:org scope, causing the /orgs/knitli/members/{user} API to return 404 for valid members.

Changes

  • Add repository collaborator check first: Use /repos/{owner}/{repo}/collaborators/{user} with GITHUB_TOKEN (works for repo-level permissions)
  • Fallback to org membership check: Use CLA_ACCESS_TOKEN (has org:read scope) for the org membership API
  • Add validation and error handling: Check token availability, handle 204/404/302/error responses appropriately

Before

# Always used GITHUB_TOKEN for org check - returns 404 for members
response=$(curl -s -o /dev/null -w "%{http_code}" \
  -H "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \
  "https://api.github.com/orgs/knitli/members/$user")

After

# 1. Check collaborator status (works with GITHUB_TOKEN)
collab_response=$(curl ... "https://api.github.com/repos/$REPO/collaborators/$user")

# 2. Fallback: Check org membership (requires CLA_ACCESS_TOKEN with org:read)
if [ -n "$CLA_ACCESS_TOKEN" ]; then
  org_response=$(curl -H "Authorization: Bearer $CLA_ACCESS_TOKEN" \
    "https://api.github.com/orgs/knitli/members/$user")
fi
Original prompt

We've attempted to fix a persistent issue with CLA Check. We're getting closer but it's still producing undesirable results.

I want you to review, as an example, the following PR:
knitli/codeweaver#185

  • You'll see that the first message it gives correctly indicates that no one needs to sign the CLA.
  • Then a later message says the exact same people need to sign the CLA.

(.github/workflows/cla-check.yml)

Please help get this fixed!


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 2 commits December 2, 2025 20:10
…ication

- Add repository collaborator check using GITHUB_TOKEN (works for repo collaborators)
- Fallback to org membership check using CLA_ACCESS_TOKEN (has org:read scope)
- This fixes the issue where GITHUB_TOKEN couldn't verify org membership

The previous implementation used GITHUB_TOKEN to check org membership,
but the default GITHUB_TOKEN doesn't have read:org scope, so the API
returned 404 for actual org members. This caused the CLA assistant
to incorrectly request CLA signatures from exempt users.

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
- Validate CLA_ACCESS_TOKEN is not empty before using it
- Add proper error handling for different API responses:
  - 204: User is an org member (exempt)
  - 404/302: User is not a member (requires CLA)
  - Other: API error, fail open and require CLA as precaution
- Add warning messages for missing token or API errors

Co-authored-by: bashandbone <89049923+bashandbone@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix persistent issue with CLA Check results fix: use collaborator check and CLA_ACCESS_TOKEN for membership verification Dec 2, 2025
Copilot AI requested a review from bashandbone December 2, 2025 20:16
@bashandbone bashandbone marked this pull request as ready for review December 2, 2025 21:06
Copilot AI review requested due to automatic review settings December 2, 2025 21:06
@bashandbone bashandbone merged commit e680f0a into main Dec 2, 2025
8 checks passed
@bashandbone bashandbone deleted the copilot/fix-cla-check-issue branch December 2, 2025 21:11
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a critical issue in the CLA workflow where organization members were incorrectly required to sign the CLA. The root cause was that the standard GITHUB_TOKEN lacks the read:org scope needed to verify organization membership via the /orgs/knitli/members/{user} API, causing it to return 404 even for valid members.

Key Changes:

  • Implements a two-tier verification approach: first checking repository collaborator status (using GITHUB_TOKEN), then falling back to org membership (using CLA_ACCESS_TOKEN with org:read scope)
  • Adds the CLA_ACCESS_TOKEN environment variable to enable org membership verification with proper permissions
  • Improves error handling for different API response scenarios (204/404/302/errors)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

# Query org membership API
response=$(curl -s -o /dev/null -w "%{http_code}" \
# Check if user is a repository collaborator (works with GITHUB_TOKEN)
# This API returns 204 if user has push access, 404 otherwise
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "This API returns 204 if user has push access, 404 otherwise", but the GitHub collaborators API actually returns 204 if the user has any collaborator permission (read, write, or admin), not just push/write access. Consider updating the comment to: "This API returns 204 if user is a collaborator (read/write/admin), 404 otherwise"

Suggested change
# This API returns 204 if user has push access, 404 otherwise
# This API returns 204 if user is a collaborator (read/write/admin), 404 otherwise

Copilot uses AI. Check for mistakes.
Comment on lines +174 to +179
elif [ "$org_response" == "404" ] || [ "$org_response" == "302" ]; then
# 404 = not a member, 302 = requester is not an org member (can't see membership)
echo "User $user is NOT a collaborator or org member and may require CLA"
NEEDS_CLA+=("$user")
else
# API error (rate limit, network issue, etc.) - fail open and require CLA
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 302 status code typically indicates a redirect and often suggests an authentication issue rather than a definitive "not a member" response. Treating 302 the same as 404 might incorrectly categorize authentication failures as "not a member". Consider treating 302 as an error case (moving it to the else block on line 179) to fail securely when authentication issues occur: elif [ "$org_response" == "404" ]; then

Suggested change
elif [ "$org_response" == "404" ] || [ "$org_response" == "302" ]; then
# 404 = not a member, 302 = requester is not an org member (can't see membership)
echo "User $user is NOT a collaborator or org member and may require CLA"
NEEDS_CLA+=("$user")
else
# API error (rate limit, network issue, etc.) - fail open and require CLA
elif [ "$org_response" == "404" ]; then
# 404 = not a member
echo "User $user is NOT a collaborator or org member and may require CLA"
NEEDS_CLA+=("$user")
else
# API error (rate limit, network issue, 302 redirect, etc.) - fail open and require CLA

Copilot uses AI. Check for mistakes.
EXEMPT_USERS+=("$user (org member)")
continue
elif [ "$org_response" == "404" ] || [ "$org_response" == "302" ]; then
# 404 = not a member, 302 = requester is not an org member (can't see membership)
Copy link

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should be updated to clarify that 302 indicates an authentication/redirect issue rather than membership visibility. If the logic is changed to treat 302 as an error case, update to: # 404 = not a member or membership is concealed

Suggested change
# 404 = not a member, 302 = requester is not an org member (can't see membership)
# 404 = not a member or membership is concealed, 302 = authentication/redirect issue (requester is not an org member, cannot see membership)

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants