diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index bf1a68ce..83ddf79d 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -1291,6 +1291,72 @@ pub fn collaborator_permission_floor( } } +/// Elevate integrity via collaborator permission fallback for org repos. +/// +/// Rank threshold for writer-level integrity (none=1, reader=2, writer=3, merged=4). +const WRITER_RANK: u8 = 3; + +/// Attempt to elevate integrity for an author in an org-owned repository +/// by checking their effective collaborator permission. +/// +/// When `author_association` gives insufficient integrity (below writer level), +/// this function checks the user's effective permission via the GitHub +/// collaborator permission API. This correctly handles org owners/admins whose +/// `author_association` is reported as "NONE" because their access is inherited +/// through org ownership rather than direct collaboration. +/// +/// Backend calls are cached per-user, so repeated lookups for the same author +/// across list/search items are inexpensive. +/// +/// Parameters: +/// - `author_login`: the issue/PR/commit author's GitHub login +/// - `repo_full_name`: "owner/repo" string +/// - `resource_label`: label for logging (e.g. "issue", "pr", "commit") +/// - `resource_id`: number or identifier for logging +/// - `integrity`: current integrity labels to potentially elevate +/// - `ctx`: policy context +/// +/// Returns the (potentially elevated) integrity labels. +pub fn elevate_via_collaborator_permission( + author_login: &str, + repo_full_name: &str, + resource_label: &str, + resource_id: &str, + integrity: Vec, + ctx: &PolicyContext, +) -> Vec { + if integrity_rank(repo_full_name, &integrity, ctx) >= WRITER_RANK || author_login.is_empty() { + return integrity; + } + let (owner, repo) = match repo_full_name.split_once('/') { + Some((o, r)) if !o.is_empty() && !r.is_empty() => (o, r), + _ => return integrity, + }; + let is_org = super::backend::is_repo_org_owned(owner, repo).unwrap_or(false); + if !is_org { + return integrity; + } + crate::log_debug(&format!( + "[integrity] {}:{}: author_association floor below writer (rank={}), checking collaborator permission for {}", + resource_label, resource_id, integrity_rank(repo_full_name, &integrity, ctx), author_login + )); + if let Some(collab) = super::backend::get_collaborator_permission(owner, repo, author_login) { + let perm_floor = collaborator_permission_floor(repo_full_name, collab.permission.as_deref(), ctx); + let merged = max_integrity(repo_full_name, integrity, perm_floor, ctx); + crate::log_debug(&format!( + "[integrity] {}:{}: collaborator permission={:?} → merged rank={}", + resource_label, resource_id, collab.permission, integrity_rank(repo_full_name, &merged, ctx) + )); + merged + } else { + crate::log_debug(&format!( + "[integrity] {}:{}: collaborator permission lookup returned None for {}, keeping author_association floor", + resource_label, resource_id, author_login + )); + integrity + } +} + /// Check if a branch/ref should be treated as default branch context pub fn is_default_branch_ref(branch_ref: &str) -> bool { branch_ref.is_empty() @@ -1417,6 +1483,16 @@ pub fn pr_integrity( } } + // Collaborator permission fallback for org repos (handles org owners/admins + // whose author_association is "NONE" due to inherited org access). + if !repo_private { + let number = item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0); + integrity = elevate_via_collaborator_permission( + author_login, repo_full_name, "pr", &format!("{}#{}", repo_full_name, number), + integrity, ctx, + ); + } + if repo_private { integrity = max_integrity( repo_full_name, @@ -1527,6 +1603,16 @@ pub fn issue_integrity( } } + // Collaborator permission fallback for org repos (handles org owners/admins + // whose author_association is "NONE" due to inherited org access). + if !repo_private { + let number = item.get(field_names::NUMBER).and_then(|v| v.as_u64()).unwrap_or(0); + integrity = elevate_via_collaborator_permission( + author_login, repo_full_name, "issue", &format!("{}#{}", repo_full_name, number), + integrity, ctx, + ); + } + if repo_private { integrity = max_integrity( repo_full_name, @@ -1579,6 +1665,17 @@ pub fn commit_integrity( let mut integrity = author_association_floor(item, repo_full_name, ctx); + // Collaborator permission fallback for org repos (handles org owners/admins + // whose author_association is "NONE" due to inherited org access). + if !repo_private { + let sha = item.get("sha").and_then(|v| v.as_str()).unwrap_or("unknown"); + let short_sha = if sha.len() > 8 { &sha[..8] } else { sha }; + integrity = elevate_via_collaborator_permission( + author_login, repo_full_name, "commit", &format!("{}@{}", repo_full_name, short_sha), + integrity, ctx, + ); + } + if repo_private { integrity = max_integrity( repo_full_name, diff --git a/guards/github-guard/rust-guard/src/labels/mod.rs b/guards/github-guard/rust-guard/src/labels/mod.rs index 04724b49..47f6144a 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -4957,4 +4957,97 @@ mod tests { helpers::blocked_integrity(repo, &ctx) ); } + + // === elevate_via_collaborator_permission tests === + + #[test] + fn test_elevate_via_collab_permission_skips_when_already_writer() { + let ctx = default_ctx(); + let repo = "github/copilot"; + let writer = writer_integrity(repo, &ctx); + let result = helpers::elevate_via_collaborator_permission( + "someuser", repo, "issue", "github/copilot#1", + writer.clone(), &ctx, + ); + assert_eq!(result, writer, "should return unchanged when already at writer level"); + } + + #[test] + fn test_elevate_via_collab_permission_skips_when_merged() { + let ctx = default_ctx(); + let repo = "github/copilot"; + let merged = merged_integrity(repo, &ctx); + let result = helpers::elevate_via_collaborator_permission( + "someuser", repo, "issue", "github/copilot#1", + merged.clone(), &ctx, + ); + assert_eq!(result, merged, "should return unchanged when already at merged level"); + } + + #[test] + fn test_elevate_via_collab_permission_skips_empty_login() { + let ctx = default_ctx(); + let repo = "github/copilot"; + let none = none_integrity(repo, &ctx); + let result = helpers::elevate_via_collaborator_permission( + "", repo, "issue", "github/copilot#1", + none.clone(), &ctx, + ); + assert_eq!(result, none, "should return unchanged when author_login is empty"); + } + + #[test] + fn test_elevate_via_collab_permission_skips_invalid_repo() { + let ctx = default_ctx(); + let none = none_integrity("invalid", &ctx); + let result = helpers::elevate_via_collaborator_permission( + "someuser", "invalid", "issue", "invalid#1", + none.clone(), &ctx, + ); + assert_eq!(result, none, "should return unchanged for invalid repo format"); + } + + #[test] + fn test_elevate_via_collab_permission_skips_empty_owner_or_repo() { + let ctx = default_ctx(); + let none = none_integrity("/repo", &ctx); + let result = helpers::elevate_via_collaborator_permission( + "someuser", "/repo", "issue", "/repo#1", + none.clone(), &ctx, + ); + assert_eq!(result, none, "should return unchanged for empty owner"); + + let none2 = none_integrity("owner/", &ctx); + let result2 = helpers::elevate_via_collaborator_permission( + "someuser", "owner/", "issue", "owner/#1", + none2.clone(), &ctx, + ); + assert_eq!(result2, none2, "should return unchanged for empty repo"); + } + + #[test] + fn test_elevate_via_collab_permission_no_elevation_non_org_repo() { + // In test mode, is_repo_org_owned returns None (no cache) → unwrap_or(false) + // so the function should return integrity unchanged + let ctx = default_ctx(); + let repo = "github/copilot"; + let none = none_integrity(repo, &ctx); + let result = helpers::elevate_via_collaborator_permission( + "dsyme", repo, "issue", "github/copilot#42", + none.clone(), &ctx, + ); + assert_eq!(result, none, "should return unchanged when repo is not org-owned (cache miss → false)"); + } + + #[test] + fn test_elevate_via_collab_permission_preserves_reader_integrity() { + let ctx = default_ctx(); + let repo = "github/copilot"; + let reader = reader_integrity(repo, &ctx); + let result = helpers::elevate_via_collaborator_permission( + "contributor", repo, "pr", "github/copilot#10", + reader.clone(), &ctx, + ); + assert_eq!(result, reader, "should preserve reader integrity when no org lookup available"); + } } diff --git a/guards/github-guard/rust-guard/src/labels/tool_rules.rs b/guards/github-guard/rust-guard/src/labels/tool_rules.rs index a0c756e7..bd89d994 100644 --- a/guards/github-guard/rust-guard/src/labels/tool_rules.rs +++ b/guards/github-guard/rust-guard/src/labels/tool_rules.rs @@ -7,7 +7,8 @@ use serde_json::Value; use super::constants::{field_names, SENSITIVE_FILE_KEYWORDS, SENSITIVE_FILE_PATTERNS}; use super::helpers::{ - author_association_floor_from_str, collaborator_permission_floor, ensure_integrity_baseline, + author_association_floor_from_str, + elevate_via_collaborator_permission, ensure_integrity_baseline, extract_number_as_string, extract_repo_info, extract_repo_info_from_search_query, format_repo_id, is_configured_trusted_bot, is_default_branch_commit_context, is_default_branch_ref, is_trusted_first_party_bot, is_trusted_user, max_integrity, @@ -100,35 +101,10 @@ fn resolve_author_integrity( { floor = max_integrity(repo_id, floor, writer_integrity(repo_id, ctx), ctx); } - if floor.len() < 3 { - let is_org = super::backend::is_repo_org_owned(owner, repo).unwrap_or(false); - if is_org { - crate::log_info(&format!( - "{} {}/{}#{}: author_association floor insufficient (len={}), checking collaborator permission for {}", - resource_label, owner, repo, resource_num, floor.len(), login - )); - if let Some(collab) = - super::backend::get_collaborator_permission(owner, repo, login) - { - let perm_floor = collaborator_permission_floor( - repo_id, - collab.permission.as_deref(), - ctx, - ); - let merged = max_integrity(repo_id, floor, perm_floor, ctx); - crate::log_info(&format!( - "{} {}/{}#{}: collaborator permission={:?} → merged floor len={}", - resource_label, owner, repo, resource_num, collab.permission, merged.len() - )); - floor = merged; - } else { - crate::log_info(&format!( - "{} {}/{}#{}: collaborator permission lookup returned None for {}, keeping author_association floor", - resource_label, owner, repo, resource_num, login - )); - } - } - } + let resource_id = format!("{}/{}#{}", owner, repo, resource_num); + floor = elevate_via_collaborator_permission( + login, repo_id, resource_label, &resource_id, floor, ctx, + ); } max_integrity(repo_id, base_integrity, floor, ctx)