feat: use collaborator permission level for integrity filtering#2863
Merged
feat: use collaborator permission level for integrity filtering#2863
Conversation
Supplement author_association with GitHub's collaborator permission API
(GET /repos/{owner}/{repo}/collaborators/{username}/permission) to get
the user's effective permission level, which correctly includes inherited
org permissions.
This fixes a bug where org admins with CONTRIBUTOR or NONE
author_association had their content silently filtered by the integrity
filter, even though they have admin/write access to the repository.
Changes:
- Rust guard: add get_collaborator_permission backend callback that
queries the synthetic get_collaborator_permission tool
- Rust guard: add collaborator_permission_floor mapping function
(admin/maintain/write -> approved, triage/read -> unapproved)
- Rust guard: tool_rules.rs calls collaborator permission when
author_association gives less than writer integrity
- Gateway unified mode: guardBackendCaller intercepts synthetic tool
and makes direct REST call to GitHub API
- Gateway proxy mode: restBackendCaller handles new tool case
Graceful degradation: if the permission API call fails (no token,
403, 404), falls back to the existing author_association logic.
Fixes #2862
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
Author
|
@copilot make sure that interactions with get_collaborator_permission are extensively logged in the rust and go code. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a collaborator-permission-based integrity signal to the GitHub guard by introducing a synthetic get_collaborator_permission tool that the gateway translates into GitHub’s REST collaborator permission endpoint, improving integrity filtering for users with inherited org permissions (fixes #2862).
Changes:
- Gateway: intercepts
get_collaborator_permissionin unified mode and routes it to GitHub’s REST API; proxy mode supports the same tool via existing REST enrichment plumbing. - Rust guard: maps collaborator permission → integrity floor and uses it as a supplement when
author_associationis below writer/approved. - Tests: adds unit/integration tests covering permission levels, error paths, token selection, and MCP wrapper formatting.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/server/unified.go | Intercepts get_collaborator_permission and performs a direct GitHub REST call, returning an MCP-wrapped response. |
| internal/server/collaborator_permission_test.go | Tests unified-mode collaborator permission interception, token/env lookup, error handling, and MCP response format. |
| internal/proxy/proxy.go | Adds get_collaborator_permission support to the proxy’s REST enrichment tool switch. |
| internal/proxy/collaborator_permission_test.go | Tests proxy-mode behavior for collaborator permission calls and token precedence. |
| guards/github-guard/rust-guard/src/labels/tool_rules.rs | Uses collaborator permission as a fallback integrity signal when author_association is insufficient. |
| guards/github-guard/rust-guard/src/labels/helpers.rs | Introduces collaborator_permission_floor() mapping permission → integrity (with tests). |
| guards/github-guard/rust-guard/src/labels/backend.rs | Adds backend callback plumbing to fetch collaborator permission via the synthetic tool (with tests). |
Comments suppressed due to low confidence (1)
guards/github-guard/rust-guard/src/labels/tool_rules.rs:258
- Using
integrity.len() < 3to decide whether to fetch collaborator permissions couples this logic to the current label vector length. This can become incorrect if integrity tags gain additional entries; consider basing the decision on integrity rank/label presence instead of vector length.
// Supplement with collaborator permission when author_association
// gives less than writer integrity (e.g., CONTRIBUTOR for org admins)
if integrity.len() < 3 {
if let Some(ref login) = facts.author_login {
if let Some(collab) = super::backend::get_collaborator_permission(&owner, &repo, login) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+291
to
+295
| req.Header.Set("Authorization", "token "+token) | ||
| req.Header.Set("Accept", "application/vnd.github+json") | ||
|
|
||
| resp, err := http.DefaultClient.Do(req) | ||
| if err != nil { |
There was a problem hiding this comment.
This REST call uses http.DefaultClient, which has no request timeout configured. A hung connection to GitHub can block guard labeling indefinitely; use a dedicated http.Client with a reasonable Timeout (and ideally a hardened Transport) instead of the default client.
Comment on lines
+472
to
+499
| /// Fetch collaborator permission level for a user in a repository. | ||
| /// Uses the synthetic `get_collaborator_permission` tool which the gateway translates | ||
| /// to GET /repos/{owner}/{repo}/collaborators/{username}/permission. | ||
| /// Returns the user's effective permission (including inherited org permissions), | ||
| /// which is more accurate than author_association for org admins. | ||
| pub fn get_collaborator_permission_with_callback( | ||
| callback: GithubMcpCallback, | ||
| owner: &str, | ||
| repo: &str, | ||
| username: &str, | ||
| ) -> Option<CollaboratorPermission> { | ||
| if owner.is_empty() || repo.is_empty() || username.is_empty() { | ||
| return None; | ||
| } | ||
|
|
||
| let args = serde_json::json!({ | ||
| "owner": owner, | ||
| "repo": repo, | ||
| "username": username, | ||
| }); | ||
|
|
||
| let args_str = args.to_string(); | ||
| let mut result_buffer = vec![0u8; SMALL_BUFFER_SIZE]; | ||
|
|
||
| let len = match callback("get_collaborator_permission", &args_str, &mut result_buffer) { | ||
| Ok(len) if len > 0 => len, | ||
| _ => return None, | ||
| }; |
There was a problem hiding this comment.
This collaborator permission lookup is invoked in labeling paths but has no caching, unlike the repo visibility lookup earlier in this module. Consider adding a small in-memory cache keyed by (owner/repo, username) to reduce rate-limit risk and latency when multiple items trigger permission checks in a single session/run.
| } | ||
| // Supplement with collaborator permission when author_association | ||
| // gives less than writer integrity (e.g., CONTRIBUTOR for org admins) | ||
| if floor.len() < 3 { |
There was a problem hiding this comment.
Using floor.len() < 3 as a proxy for “less than writer integrity” is brittle because it relies on the current label vector shape rather than the integrity rank semantics. Prefer an explicit rank check (e.g. via a helper that mirrors integrity_rank) or checking for the writer label for the normalized scope.
This issue also appears on line 254 of the same file.
Suggested change
| if floor.len() < 3 { | |
| let writer_floor = writer_integrity(repo_id, ctx); | |
| let elevated_floor = max_integrity( | |
| repo_id, | |
| floor.clone(), | |
| writer_floor, | |
| ctx, | |
| ); | |
| if elevated_floor != floor { |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
- Rust backend.rs: log start/empty-args/callback-failure/JSON-parse/success with owner/repo/username and resolved permission at log_info level - Rust tool_rules.rs: log when collaborator permission check is initiated (with current integrity floor/len), result (permission string), and merged integrity len for both issue_read and pull_request_read paths - Go unified.go: log unexpected args type, missing args with values, per-call GET path with owner/repo/username, REST call failures, and success with extracted permission level from JSON response - Go proxy.go: log missing args with field values for get_collaborator_permission; after success log the resolved permission level extracted from the JSON response body Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/988ecd15-f8a9-4a34-a740-c55a2eef9eb3 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Collaborator
Author
|
@copilot apply changes based on the comments in this thread |
Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/988ecd15-f8a9-4a34-a740-c55a2eef9eb3 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Added extensive logging across all four files in commit c87afe9 (Rust) / f53ce55 (Go refactor): Rust Rust Go Go Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Add owner type detection that piggybacks on existing search_repositories calls to determine if a repo belongs to an Organization or User account. The collaborator permission API call is now gated on org ownership since personal repos don't have inherited org permissions that cause author_association to be inaccurate. Changes: - backend.rs: Add repo_owner_type_cache, is_repo_org_owned(), extract_owner_is_org(), owner_is_org_from_items(), and owner_type_from_repo_object() functions - backend.rs: Piggyback owner type extraction in is_repo_private_with_callback success path - tool_rules.rs: Gate get_collaborator_permission calls on is_repo_org_owned() returning Some(true) or unknown (defaults to calling for safety) - backend.rs: Add 10 new tests for owner type detection Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was referenced Mar 30, 2026
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supplements
author_associationwith GitHub's collaborator permission API (GET /repos/{owner}/{repo}/collaborators/{username}/permission) to get the user's effective permission level, which correctly includes inherited org permissions.Problem
GitHub's
author_associationfield reflects a user's direct relationship with a repository, not their effective permissions. Org admins who haven't been explicitly added as collaborators getCONTRIBUTORorNONE, causing the integrity filter to silently block their issues and PRs.Solution
When
author_associationgives less than writer/approved integrity, the guard now also checks the user's collaborator permission level via a new syntheticget_collaborator_permissiontool. The gateway intercepts this tool name and makes a direct REST API call to GitHub.Permission-to-integrity mapping
Graceful degradation
If the permission API call fails (no token, 403, 404, network error), the system falls back to the existing
author_associationlogic. No behavior change when the API is unavailable.Changes
Rust Guard
backend.rs: NewCollaboratorPermissionstruct +get_collaborator_permission_with_callback()function with extensive logging at every step (entry, empty-field validation, callback failure, JSON parse errors, and success with resolved permission/login). 12 unit tests.helpers.rs: Newcollaborator_permission_floor()mapping function (12 unit tests)tool_rules.rs:issue_readandpull_request_readpaths now check collaborator permission whenauthor_associationgives less than writer integrity; each path logs when the check is initiated (with current integrity floor and login), the returned permission, the merged integrity level, and when the lookup returnsNone.Go Gateway
unified.go:guardBackendCaller.CallTool()interceptsget_collaborator_permissionand makes direct REST call to GitHub API using env token. Extensive logging covers unexpected args type, missing required fields, GET path with owner/repo/username, REST call failures, and success with the extractedpermissionfield.proxy.go:restBackendCaller.CallTool()handles new tool via existing enrichment path. Logs missing args with field values and the resolvedpermissionextracted from the successful JSON response.Tests
Total: 50 new tests