Repository navigation
Review of Restricted Paths Trust Signal Explorations #2104
Description
Scope
I fetched the full gh pr view history for .github/workflows/restricted-paths-guard.yml and the relevant linked discussions I could access:
- [no-ci] CI: Add restricted-paths-guard.yml #1878
- [no-ci] Use collaborator permission API for restricted-paths-guard trust check #1930
- [no-ci] Trust any collaborator in restricted-paths guard #2010
- [no-ci] pr-metadata-check: restrict cuda_bindings changes to NVIDIA organization members #1871
- [no-ci] CI: add dedicated merge gate for restricted-paths review #1896
- [no-ci] CI: drive restricted paths guard from user-in-org-check-bot #1901
- Fix bug 5811173: update cufile tests and configuration #1821
- ONLY FOR TESTING PR 2010 #2011
- CI: Autogenerate a PR comment for contributors making changes to the
cuda_bindings/directory #1139 - Ensure all commits to cuda_python/ and cuda_bindings/ are signed with an @nvidia.com email #1886
- private
#294
I could not access the internal github-apps PRs #78 or #79 with the available token.
What I Learned
The path was twisty because each candidate signal solved one failure mode and exposed another.
#1139 started with the practical policy: external contributions are generally OK, but changes under cuda_bindings/ need early/manual review.
#1871 first tried folding this into pr-metadata-check, then #1878 split it into a dedicated restricted-paths-guard.yml workflow.
author_association looked attractive because it is directly available in the pull request payload. But testing showed webhook payloads can say CONTRIBUTOR for fork PRs even when the author is an NVIDIA org member or explicit repo collaborator. In later live API reads, those same users could appear as MEMBER. That made the event payload too lossy for enforcement.
Direct organization membership APIs would be conclusive in principle. In practice, the normal workflow GITHUB_TOKEN could not reliably query private NVIDIA org membership. That meant the workflow could not distinguish "not a member" from "not allowed to know" well enough.
Public org membership/profile checks were also explored. They behave cleanly as true-positive signals for public org members, but many legitimate internal users do not expose public membership. So that would still leave too many internal contributors in the manual-review bucket.
A checked-in allowlist was considered as a rollout fallback, then removed. It was simple and explicit, but too manual and not really a scalable trust source.
#1930 switched to GET /repos/{owner}/{repo}/collaborators/{username}/permission, trusting admin|maintain|write. This sidestepped the stale/inconsistent webhook author_association value and worked for known collaborators with elevated repo access.
#2010 broadened the trusted permission set to admin|maintain|write|triage|read because #1821 showed an internal/collaborator author returning read, which caused a false positive review label.
The new discovery from PR #2103 is that on a public repo, read is not a collaborator signal. It is just effective public repository access. For example, both LeSingh1 and octocat return permission: read from:
gh api repos/NVIDIA/cuda-python/collaborators/<user>/permissionBut LeSingh1 does not appear in direct/outside/all collaborator listings, the direct collaborator check returns 404, and PR #2103 has author_association: FIRST_TIME_CONTRIBUTOR.
So the current bug is: read is treated as a trusted collaborator signal even though, for a public repository, it can simply mean "any GitHub user can read this repo."
Most Promising Near-Term Fix
Stop trusting read, and combine two live checks:
- Query the live PR API for
author_association, rather than trusting the webhook payload. - Trust live
MEMBER|OWNER. - Decide explicitly whether live
COLLABORATORshould be trusted as a policy matter. - Query collaborator permission, but trust only
triage|write|maintain|admin, notread. - Treat
readlikenonefor public repositories.
That likely handles both sides of the problem:
LeSingh1remains untrusted: liveFIRST_TIME_CONTRIBUTORplus publicread.- Known internal/repo collaborators with
triageor higher still get an automatic trusted signal. - The fork-webhook inconsistency from [no-ci] Use collaborator permission API for restricted-paths-guard trust check #1930 is avoided by querying the live PR object instead of using
github.event.pull_request.author_association.
Clean Long-Term Fix
The clean conclusive path is still a GitHub App or org-scoped credential that emits a machine-readable result, for example:
PR-Author-In-NVIDIA-OrgPR-Author-Outside-NVIDIA-OrgPR-Author-Org-Check-Failure
That was essentially the direction of #1901, and private #294 also points toward reusing or extending copy-pr-bot rather than trying to recreate privileged org checks inside repo workflows.
The advantage is semantic clarity: the privileged component answers the org/trust question once, and restricted-paths-guard.yml only consumes the result.
The caveat is that copy-pr-bot trust is not exactly the same thing as "PR author is in the NVIDIA GitHub org." It considers the PR author, committers, signed commits, commit counts, and other runner-safety conditions. That may be useful, but the exact meaning of the signal should be named carefully.
Robust Policy Alternative
A less clever but robust alternative is to make restricted-path changes require explicit maintainer review unconditionally, using CODEOWNERS or the existing Needs-Restricted-Paths-Review gate.
This would avoid identity ambiguity entirely. The tradeoff is more manual review load for trusted internal contributors.
Recommendation
For the immediate bug, remove read from the trusted collaborator permissions and add a live PR API author_association check.
For a conclusive signal, revive the bot/app-label direction rather than relying on public repo collaborator permission semantics.
Activity
rwgk commentedon Jun 16, 2026
What we have now seems to work: Closing this issue.