Skip to content

[no-ci] Use collaborator permission API for restricted-paths-guard trust check #1930

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
rparolin merged 11 commits into from
Apr 20, 2026
+54 −10
Merged

[no-ci] Use collaborator permission API for restricted-paths-guard trust check #1930

Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
5500d84
Use collaborator permission API instead of event payload author_assoc…
rwgk Apr 16, 2026
b814323
TEMPORARY: Switch to pull_request trigger for testing
rwgk Apr 16, 2026
4898bdd
Revert "TEMPORARY: Switch to pull_request trigger for testing"
rwgk Apr 16, 2026
081ff96
Merge branch 'main' into restricted-paths-guard_addl_collaborator_check
rwgk Apr 17, 2026
45b8357
Add explicit handling for non-trusted permission levels
rwgk Apr 17, 2026
8686adb
TEMPORARY: Switch to pull_request trigger for testing
rwgk Apr 16, 2026
f5aa8a1
Revert "TEMPORARY: Switch to pull_request trigger for testing"
rwgk Apr 17, 2026
a98ddb5
Merge branch 'main' into restricted-paths-guard_addl_collaborator_check
rwgk Apr 18, 2026
500cb29
Fail restricted-paths guard on collaborator API errors
rwgk Apr 18, 2026
2a019b6
TEMPORARY: Switch to pull_request trigger for testing
rwgk Apr 16, 2026
4943745
Revert "TEMPORARY: Switch to pull_request trigger for testing"
rwgk Apr 18, 2026
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
33 changes: 23 additions & 10 deletions .github/workflows/restricted-paths-guard.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@ jobs:
if: github.repository_owner == 'NVIDIA'
runs-on: ubuntu-latest
permissions:
contents: write # needed for collaborator permission check
pull-requests: write
steps:
- name: Inspect PR author signals for restricted paths
env:
# PR metadata inputs
AUTHOR_ASSOCIATION: ${{ github.event.pull_request.author_association || 'NONE' }}
# PR metadata inputs (author_association from event payload is
# unreliable for fork PRs, so we query the collaborator API directly)
PR_AUTHOR: ${{ github.event.pull_request.user.login }}
PR_NUMBER: ${{ github.event.pull_request.number }}
PR_URL: ${{ github.event.pull_request.html_url }}
Expand All @@ -38,6 +39,15 @@ jobs:
run: |
set -euo pipefail

# Query the collaborator permission API to get the author's actual
# permission level. This is authoritative regardless of whether the
# PR originates from a fork or a branch in the main repo.
# Returns: admin, maintain, write, triage, read, or none.
COLLABORATOR_PERMISSION=$(
gh api "repos/$REPO/collaborators/$PR_AUTHOR/permission" \
--jq '.permission' 2>/dev/null || echo "none"
)

if ! MATCHING_RESTRICTED_PATHS=$(
gh api \
--paginate \
Expand All @@ -63,7 +73,7 @@ jobs:
echo ""
echo "- **Error**: Failed to inspect the PR file list."
echo "- **Author**: $PR_AUTHOR"
echo "- **Author association**: $AUTHOR_ASSOCIATION"
echo "- **Collaborator permission**: $COLLABORATOR_PERMISSION"
echo ""
echo "Please update the PR at: $PR_URL"
} >> "$GITHUB_STEP_SUMMARY"
Expand All @@ -83,7 +93,7 @@ jobs:
echo ""
echo "- **Error**: Failed to inspect the current PR labels."
echo "- **Author**: $PR_AUTHOR"
echo "- **Author association**: $AUTHOR_ASSOCIATION"
echo "- **Collaborator permission**: $COLLABORATOR_PERMISSION"
echo ""
echo "Please update the PR at: $PR_URL"
} >> "$GITHUB_STEP_SUMMARY"
Expand All @@ -107,11 +117,14 @@ jobs:
TRUSTED_SIGNALS="(none)"

if [ "$TOUCHES_RESTRICTED_PATHS" = "true" ]; then
case "$AUTHOR_ASSOCIATION" in
COLLABORATOR|MEMBER|OWNER)
case "$COLLABORATOR_PERMISSION" in
admin|maintain|write)
Copy link
Copy Markdown
Contributor

@cpcloud cpcloud Apr 17, 2026

Choose a reason for hiding this comment

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

I don't see any handling of the none case you're creating above.

Copy link
Copy Markdown
Contributor Author

@rwgk rwgk Apr 17, 2026

Choose a reason for hiding this comment

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

It's explicit now: commit 45b8357

Copy link
Copy Markdown
Contributor

@cpcloud cpcloud Apr 17, 2026

Choose a reason for hiding this comment

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

I meant: does it need to fail there? Or is none acceptable?

HAS_TRUSTED_SIGNAL=true
LABEL_ACTION="not needed (author association is a trusted signal)"
TRUSTED_SIGNALS="author_association:$AUTHOR_ASSOCIATION"
LABEL_ACTION="not needed (collaborator permission is a trusted signal)"
TRUSTED_SIGNALS="collaborator_permission:$COLLABORATOR_PERMISSION"
;;
*)
# triage, read, none, or API error: not a trusted signal
;;
esac
fi
Expand All @@ -136,7 +149,7 @@ jobs:
echo ""
echo "- **Error**: Failed to add the \`$REVIEW_LABEL\` label."
echo "- **Author**: $PR_AUTHOR"
echo "- **Author association**: $AUTHOR_ASSOCIATION"
echo "- **Collaborator permission**: $COLLABORATOR_PERMISSION"
echo ""
write_matching_restricted_paths
echo ""
Expand All @@ -154,7 +167,7 @@ jobs:
echo "## Restricted Paths Guard Completed"
echo ""
echo "- **Author**: $PR_AUTHOR"
echo "- **Author association**: $AUTHOR_ASSOCIATION"
echo "- **Collaborator permission**: $COLLABORATOR_PERMISSION"
echo "- **Touches restricted paths**: $TOUCHES_RESTRICTED_PATHS"
echo "- **Restricted paths**: \`cuda_bindings/\`, \`cuda_python/\`"
echo "- **Trusted signals**: $TRUSTED_SIGNALS"
Expand Down
Loading