Skip to content

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

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.

Sign up for GitHub

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

Merged
bashandbone merged 3 commits into from
Dec 2, 2025
Merged

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

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5cee674
Initial plan
Copilot Dec 2, 2025
27c6d1f
fix: use collaborator check and CLA_ACCESS_TOKEN for membership verif…
Copilot Dec 2, 2025
1ce7de2
fix: add validation and error handling for org membership check
Copilot Dec 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 34 additions & 7 deletions .github/workflows/cla-check.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ jobs:
- name: Check Membership (check all PR committers)
id: check-membership
if: steps.check-repo.outputs.is_knitli_repo == 'true'
env:
CLA_ACCESS_TOKEN: ${{ secrets.CLA_ACCESS_TOKEN }}
run: |
set -euo pipefail
REPO="${{ github.repository }}"
Expand Down Expand Up @@ -146,17 +148,42 @@ jobs:
continue
fi

# 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.
collab_response=$(curl -s -o /dev/null -w "%{http_code}" \
-H "Authorization: Bearer ${{ secrets.GITHUB_TOKEN }}" \
"https://api.github.com/orgs/knitli/members/$user")
"https://api.github.com/repos/$REPO/collaborators/$user")

if [ "$response" == "204" ]; then
echo "User $user is a Knitli org member"
EXEMPT_USERS+=("$user (org member)")
if [ "$collab_response" == "204" ]; then
echo "User $user is a repository collaborator"
EXEMPT_USERS+=("$user (collaborator)")
continue
fi

# Fallback: Check org membership using CLA_ACCESS_TOKEN (has org:read scope)
# Only attempt if CLA_ACCESS_TOKEN is available
if [ -n "$CLA_ACCESS_TOKEN" ]; then
org_response=$(curl -s -o /dev/null -w "%{http_code}" \
-H "Authorization: Bearer $CLA_ACCESS_TOKEN" \
"https://api.github.com/orgs/knitli/members/$user")

if [ "$org_response" == "204" ]; then
echo "User $user is a Knitli org member"
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.
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
Comment on lines +174 to +179
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.
echo "Warning: Org membership check for $user returned $org_response, requiring CLA as precaution"
NEEDS_CLA+=("$user")
fi
else
echo "User $user is NOT a Knitli org member and may require CLA"
# No CLA_ACCESS_TOKEN available, can't check org membership
echo "Warning: CLA_ACCESS_TOKEN not available, cannot check org membership for $user"
echo "User $user is NOT a collaborator and may require CLA"
NEEDS_CLA+=("$user")
fi
done
Expand Down