From 2f0cf49da478e5c79345f4e9319dbf8e302563c3 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:19:58 +0000 Subject: [PATCH 1/2] Initial plan From db301c1d32f2d699f100914b43736b2152d745fb Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 3 Aug 2026 15:28:45 +0000 Subject: [PATCH 2/2] refactor(rust-guard): centralize repo-private fallback helpers --- .../rust-guard/src/labels/helpers.rs | 46 +++++++++++++++++++ .../rust-guard/src/labels/response_items.rs | 17 ++----- .../rust-guard/src/labels/response_paths.rs | 11 +---- .../rust-guard/src/labels/tool_rules.rs | 10 ++-- 4 files changed, 55 insertions(+), 29 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index 6f949cb69..01258556d 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -1051,6 +1051,27 @@ pub(crate) fn repo_visibility_private_for_repo_id(repo_id: &str) -> Option super::backend::is_repo_private(owner, repo) } +/// Best-effort repo-private lookup for fallback/default behavior only. +/// Returns `false` (fail-open) when owner/repo are missing or visibility is unknown. +/// Do not use this for security-sensitive secrecy decisions; prefer +/// [`repo_private_or_secure_default`] or [`repo_visibility_secrecy`]. +pub(crate) fn repo_private_fallback(owner: &str, repo: &str) -> bool { + if owner.is_empty() || repo.is_empty() { + return false; + } + super::backend::is_repo_private(owner, repo).unwrap_or(false) +} + +/// Resolves repo-private visibility for security-sensitive integrity decisions. +/// Fails secure in production (`true` when unknown), while tests resolve unknown +/// visibility to `false` for deterministic fixtures. +pub(crate) fn repo_private_or_secure_default(owner: &str, repo: &str) -> bool { + match super::backend::is_repo_private(owner, repo) { + Some(value) => value, + None => !cfg!(test), + } +} + // ============================================================================ // JSON Field Extraction Helpers // ============================================================================ @@ -2386,6 +2407,31 @@ mod tests { ); } + #[test] + fn test_repo_private_fallback_returns_false_when_owner_or_repo_missing() { + assert!(!repo_private_fallback("", "repo")); + assert!(!repo_private_fallback("owner", "")); + } + + #[test] + fn test_repo_private_fallback_uses_cached_visibility_when_present() { + let _guard = super::super::backend::cache_repo_visibility_for_tests("owner/repo", true); + assert!(repo_private_fallback("owner", "repo")); + } + + #[test] + fn test_repo_private_or_secure_default_uses_cached_visibility() { + let _guard = super::super::backend::cache_repo_visibility_for_tests("owner/repo", false); + assert!(!repo_private_or_secure_default("owner", "repo")); + } + + #[test] + fn test_repo_private_or_secure_default_fails_open_in_tests_when_unknown() { + let _lock = super::super::backend::lock_repo_visibility_cache_for_tests(); + super::super::backend::clear_repo_visibility_cache_for_tests(); + assert!(!repo_private_or_secure_default("owner", "repo")); + } + #[test] fn test_collaborator_permission_matches_author_association_writer() { let ctx = test_ctx(); diff --git a/guards/github-guard/rust-guard/src/labels/response_items.rs b/guards/github-guard/rust-guard/src/labels/response_items.rs index 5b69bbc62..a4ee61a53 100644 --- a/guards/github-guard/rust-guard/src/labels/response_items.rs +++ b/guards/github-guard/rust-guard/src/labels/response_items.rs @@ -147,11 +147,7 @@ pub fn label_response_items( let items_to_process = limit_items_with_log(items, "list_pull_requests"); let (arg_owner, arg_repo, arg_repo_full) = extract_repo_scope_with_query_fallback(tool_args); - let default_repo_private = if !arg_owner.is_empty() && !arg_repo.is_empty() { - super::backend::is_repo_private(&arg_owner, &arg_repo).unwrap_or(false) - } else { - false - }; + let default_repo_private = repo_private_fallback(&arg_owner, &arg_repo); // All tools in this match arm use shared repo secrecy except search_pull_requests, // which uses per-item secrecy derived from each PR's repository. let secrecy = if !matches!( @@ -257,11 +253,7 @@ pub fn label_response_items( // Get owner/repo from tool_args for contributor verification let (arg_owner, arg_repo, default_repo_full_name) = extract_repo_scope_with_query_fallback(tool_args); - let default_repo_private = if !arg_owner.is_empty() && !arg_repo.is_empty() { - super::backend::is_repo_private(&arg_owner, &arg_repo).unwrap_or(false) - } else { - false - }; + let default_repo_private = repo_private_fallback(&arg_owner, &arg_repo); // All tools in this match arm use shared repo secrecy except search_issues, // which uses per-item secrecy derived from each issue's repository. let secrecy = if !matches!(tool_name, "search_issues" | "search_issues_ff_fields_param") @@ -344,10 +336,7 @@ pub fn label_response_items( let arg_branch = tool_args.get("sha").and_then(|v| v.as_str()).unwrap_or(""); let secrecy = repo_visibility_secrecy(&arg_owner, &arg_repo, &repo_full_name, ctx); let repo_private = if !arg_owner.is_empty() && !arg_repo.is_empty() { - match super::backend::is_repo_private(&arg_owner, &arg_repo) { - Some(value) => value, - None => !cfg!(test), - } + repo_private_or_secure_default(&arg_owner, &arg_repo) } else { false }; diff --git a/guards/github-guard/rust-guard/src/labels/response_paths.rs b/guards/github-guard/rust-guard/src/labels/response_paths.rs index c57c38935..3ac5ab2c0 100644 --- a/guards/github-guard/rust-guard/src/labels/response_paths.rs +++ b/guards/github-guard/rust-guard/src/labels/response_paths.rs @@ -60,11 +60,7 @@ fn resolve_repo_item_context<'a>( // Try tool_args first, fall back to extracting from first item let (arg_owner, arg_repo, arg_repo_full) = extract_repo_scope_with_query_fallback(tool_args); - let default_repo_private = if !arg_owner.is_empty() && !arg_repo.is_empty() { - super::backend::is_repo_private(&arg_owner, &arg_repo).unwrap_or(false) - } else { - false - }; + let default_repo_private = repo_private_fallback(&arg_owner, &arg_repo); let default_repo = default_repo_for_items(arg_repo_full, items); let default_secrecy_shared: crate::SharedLabels = if tool_name != search_tool_name { repo_visibility_secrecy(&arg_owner, &arg_repo, &default_repo, ctx) @@ -352,10 +348,7 @@ pub fn label_response_paths( let default_secrecy: crate::SharedLabels = repo_visibility_secrecy(&arg_owner, &arg_repo, &default_repo, ctx).into(); let repo_private = if !arg_owner.is_empty() && !arg_repo.is_empty() { - match super::backend::is_repo_private(&arg_owner, &arg_repo) { - Some(value) => value, - None => !cfg!(test), - } + repo_private_or_secure_default(&arg_owner, &arg_repo) } else { false }; 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 48d4aee1c..a1b5326cf 100644 --- a/guards/github-guard/rust-guard/src/labels/tool_rules.rs +++ b/guards/github-guard/rust-guard/src/labels/tool_rules.rs @@ -13,8 +13,8 @@ use super::helpers::{ ensure_integrity_baseline, extract_number_as_string, extract_repo_info_from_search_query, format_repo_id, get_string_field, is_any_trusted_actor, is_default_branch_commit_context, is_default_branch_ref, max_integrity, merged_integrity, policy_private_scope_label, - private_scope_label, private_user_label, project_github_label, reader_integrity, short_sha, - writer_integrity, + private_scope_label, private_user_label, project_github_label, reader_integrity, + repo_private_or_secure_default, short_sha, writer_integrity, PolicyContext, }; use std::borrow::Cow; @@ -388,10 +388,8 @@ pub fn apply_tool_labels( .and_then(|v| v.as_str()) .unwrap_or(""); let is_default_ref = is_default_branch_commit_context(tool_name, sha_or_ref); - let repo_private_effective = match repo_private { - Some(value) => value, - None => !cfg!(test), - }; + let repo_private_effective = repo_private + .unwrap_or_else(|| repo_private_or_secure_default(&owner, &repo)); integrity = if is_default_ref { merged_integrity(repo_id, ctx)