Fix approval gate using GitHub App token for org membership check#11479
Fix approval gate using GitHub App token for org membership check#11479
Conversation
Replace the invalid 'permissions: members: read' (which is not a valid GITHUB_TOKEN scope) with a GitHub App token from the existing radius-functional-tests app, which already has org-level members:read. This correctly identifies org members with private membership visibility, fixing the false-positive approval gate triggers for private org members.
sylvainsf
requested a deployment
to
external-contributor-approval
GitHub Actions
Waiting
There was a problem hiding this comment.
Pull request overview
Re-implements the functional-test-cloud approval gate so org members with private membership visibility are treated as trusted contributors, without using the invalid permissions: members: read GITHUB_TOKEN scope.
Changes:
- Add a
check-trustjob that uses a GitHub App token to check org membership via the GitHub API. - Update
approval-gateto depend oncheck-trustoutput (is-external) rather thanauthor_association. - Adjust
setupjob dependencies/conditions to ensure tests only proceed when approval is granted (or not required).
| if: github.event_name == 'pull_request_target' | ||
| outputs: | ||
| is-external: ${{ steps.check.outputs.is-external }} | ||
| permissions: {} | ||
| steps: | ||
| - name: Generate App Token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0 | ||
| with: | ||
| app-id: ${{ vars.FUNCTIONAL_TEST_APP_ID }} | ||
| private-key: ${{ secrets.FUNCTIONAL_TEST_APP_PRIVATE_KEY }} | ||
| owner: ${{ github.repository_owner }} | ||
| permission-members: read |
There was a problem hiding this comment.
check-trust runs on pull_request_target for any fork of this repository. On forks, secrets.FUNCTIONAL_TEST_APP_PRIVATE_KEY/vars.FUNCTIONAL_TEST_APP_ID won't exist, so this job will fail and block the workflow. Add a github.repository == vars.RADIUS_REPOSITORY guard (job-level or on the app-token step) so forks skip this logic cleanly, consistent with other steps in this workflow that gate on vars.RADIUS_REPOSITORY.
| - name: Generate App Token | ||
| id: app-token | ||
| uses: actions/create-github-app-token@f8d387b68d61c58ab83c6c016672934102569859 # v3.0.0 | ||
| with: | ||
| app-id: ${{ vars.FUNCTIONAL_TEST_APP_ID }} | ||
| private-key: ${{ secrets.FUNCTIONAL_TEST_APP_PRIVATE_KEY }} | ||
| owner: ${{ github.repository_owner }} | ||
| permission-members: read | ||
|
|
There was a problem hiding this comment.
The App token is generated unconditionally, even for same-repo PRs where it isn't needed (the job exits before making any API calls). Gate the token generation step so it only runs for fork PRs, to minimize secret usage and reduce blast radius if anything in this job changes in the future.
| HEAD_REPO: ${{ github.event.pull_request.head.repo.full_name }} | ||
| BASE_REPO: ${{ github.event.pull_request.base.repo.full_name }} | ||
| PR_AUTHOR: ${{ github.event.pull_request.user.login }} | ||
| ORG: ${{ github.repository_owner }} | ||
| run: | | ||
| # Same-repo PRs are always trusted (requires write access to push branches) | ||
| if [ "${HEAD_REPO}" = "${BASE_REPO}" ]; then | ||
| echo "Same-repo PR from ${PR_AUTHOR} — trusted" | ||
| echo "is-external=false" >> "${GITHUB_OUTPUT}" | ||
| exit 0 | ||
| fi | ||
|
|
||
| # Fork PR: check if the author is an org member via GitHub API. | ||
| # Uses app token which can read org membership regardless of visibility settings. | ||
| # gh api returns exit code 0 for 204 (member) and non-zero for 404/302 (not a member). | ||
| if gh api "orgs/${ORG}/members/${PR_AUTHOR}" --silent 2>/dev/null; then | ||
| echo "Fork PR from org member ${PR_AUTHOR} — trusted" |
There was a problem hiding this comment.
For fork PRs, the trust decision is based on github.event.pull_request.user.login (PR author). A PR can be opened from a repo/branch the author has access to but does not own; in that case the head repo owner may be an external user who can still push new commits to the PR branch. Consider checking org membership for github.event.pull_request.head.repo.owner.login (or requiring both author and head repo owner to be members) so the gate is based on who controls the code being executed.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11479 +/- ##
==========================================
- Coverage 51.05% 51.05% -0.01%
==========================================
Files 699 699
Lines 44316 44316
==========================================
- Hits 22626 22624 -2
- Misses 19522 19523 +1
- Partials 2168 2169 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Description
The approval gate in
functional-test-cloud.yamlwas reverted (#11474) because PR #11435 usedpermissions: members: read— an invalid GITHUB_TOKEN scope that caused a workflow parse error at line 114 col 7 after merging to main.This PR re-implements the fix using a GitHub App token from the existing
radius-functional-testsapp (which already has org-levelmembers:read) instead of the GITHUB_TOKEN.Root Cause (from #11435)
github.event.pull_request.author_associationreturnsCONTRIBUTORinstead ofMEMBERfor org members with private membership visibility, causing the approval gate to trigger incorrectly.Fix
check-trustjob that generates an app token viaactions/create-github-app-tokenwithpermission-members: readgh api orgs/{org}/members/{username}using the app token (works regardless of membership visibility)Fixes: #11435
Contributor checklist
Please verify that the PR meets the following requirements, where applicable: