From 56865cf86bf731b8f716bfa7a80a91c616ae5499 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 13:37:40 +0000 Subject: [PATCH 1/4] Initial plan From a2d85a53e6ab663d68579837e66322d70f8df870 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 13:48:42 +0000 Subject: [PATCH 2/4] Optimize rust-guard scope and integrity hot paths --- .../rust-guard/src/labels/helpers.rs | 124 +++++++++++++++--- guards/github-guard/rust-guard/src/lib.rs | 2 +- 2 files changed, 110 insertions(+), 16 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index e18f7b8af..b7ba71be9 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -3,6 +3,7 @@ //! This module contains utility functions used across the labeling system, //! including JSON extraction, integrity determination, and common operations. +use std::borrow::Cow; use std::sync::atomic::{AtomicBool, Ordering}; use serde_json::Value; @@ -156,10 +157,10 @@ pub struct PolicyContext { pub demotion_label: String, } -fn normalize_scope(scope: &str, ctx: &PolicyContext) -> String { +fn normalize_scope<'a>(scope: &'a str, ctx: &'a PolicyContext) -> Cow<'a, str> { let token = policy_scope_token(&ctx.scopes); if token.is_empty() { - scope.to_string() + Cow::Borrowed(scope) } else if ctx .scopes .iter() @@ -176,10 +177,10 @@ fn normalize_scope(scope: &str, ctx: &PolicyContext) -> String { if matches_any_scope { token } else { - scope.to_string() + Cow::Borrowed(scope) } } else { - scope.to_string() + Cow::Borrowed(scope) } } @@ -191,16 +192,24 @@ fn split_repo_id(repo_id: &str) -> Option<(&str, &str)> { Some((owner, repo)) } -pub(crate) fn policy_scope_token(scopes: &[PolicyScopeEntry]) -> String { - let labels: Vec<&str> = scopes +pub(crate) fn policy_scope_token(scopes: &[PolicyScopeEntry]) -> Cow<'_, str> { + let mut labels = scopes .iter() .map(|s| s.scope_label.as_str()) - .filter(|s| !s.is_empty()) - .collect(); - if labels.is_empty() { - String::new() - } else { - labels.join(" | ") + .filter(|s| !s.is_empty()); + match (labels.next(), labels.next()) { + (None, _) => Cow::Borrowed(""), + (Some(first), None) => Cow::Borrowed(first), + (Some(first), Some(second)) => { + let mut value = String::from(first); + value.push_str(" | "); + value.push_str(second); + for rest in labels { + value.push_str(" | "); + value.push_str(rest); + } + Cow::Owned(value) + } } } @@ -1284,16 +1293,30 @@ fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 { } fn integrity_rank_normalized(normalized_scope: &str, labels: &[String]) -> u8 { - // Check from highest to lowest, allocating one label at a time. + // Check from highest to lowest. for (rank, (prefix, base)) in INTEGRITY_LEVELS.iter().enumerate().rev() { - let tag = format_integrity_label(prefix, normalized_scope, base); - if labels.iter().any(|l| l == &tag) { + if labels + .iter() + .any(|label| label_matches_normalized(label, prefix, normalized_scope, base)) + { return (rank + 1) as u8; } } 0 } +#[inline] +fn label_matches_normalized(label: &str, prefix: &str, scope: &str, base: &str) -> bool { + if scope.is_empty() { + label == base + } else if scope.contains('|') { + // Multi-scope uses canonical "integrity=;scopes=..." encoding. + label == format_integrity_label(prefix, scope, base) + } else { + label.strip_prefix(prefix) == Some(scope) + } +} + /// Elevate integrity to the max of current and candidate levels for a scope. pub fn max_integrity( scope: &str, @@ -1917,11 +1940,54 @@ pub(crate) fn is_any_trusted_actor(username: &str, ctx: &PolicyContext) -> bool #[cfg(test)] mod tests { use super::*; + use std::borrow::Cow; fn test_ctx() -> PolicyContext { PolicyContext::default() } + #[test] + fn test_policy_scope_token_borrows_for_single_scope() { + let scopes = vec![PolicyScopeEntry { + scope_kind: ScopeKind::Owner, + scope_owner: Some("octocat".to_string()), + scope_repo: None, + scope_label: "octocat".to_string(), + }]; + let token = policy_scope_token(&scopes); + assert_eq!(token, "octocat"); + assert!(matches!(token, Cow::Borrowed(_))); + } + + #[test] + fn test_policy_scope_token_owns_for_multi_scope() { + let scopes = vec![ + PolicyScopeEntry { + scope_kind: ScopeKind::Owner, + scope_owner: Some("octocat".to_string()), + scope_repo: None, + scope_label: "octocat".to_string(), + }, + PolicyScopeEntry { + scope_kind: ScopeKind::RepoPrefix, + scope_owner: Some("octocat".to_string()), + scope_repo: Some("hello".to_string()), + scope_label: "octocat/hello*".to_string(), + }, + ]; + let token = policy_scope_token(&scopes); + assert_eq!(token, "octocat | octocat/hello*"); + assert!(matches!(token, Cow::Owned(_))); + } + + #[test] + fn test_normalize_scope_borrows_input_when_no_scopes() { + let ctx = test_ctx(); + let normalized = normalize_scope("owner/repo", &ctx); + assert_eq!(normalized, "owner/repo"); + assert!(matches!(normalized, Cow::Borrowed(_))); + } + #[test] fn test_is_any_trusted_actor_tiers_and_negative() { let ctx = PolicyContext { @@ -2317,6 +2383,34 @@ mod tests { assert_eq!(result, candidate, "max_integrity should keep the higher integrity rank"); } + #[test] + fn test_label_matches_normalized_common_and_multiscope_paths() { + assert!(label_matches_normalized( + "approved:owner/repo", + label_constants::WRITER_PREFIX, + "owner/repo", + label_constants::WRITER_BASE + )); + assert!(label_matches_normalized( + "integrity=approved;scopes=owner/repo,owner/tool*", + label_constants::WRITER_PREFIX, + "owner/repo | owner/tool*", + label_constants::WRITER_BASE + )); + assert!(label_matches_normalized( + label_constants::READER_BASE, + label_constants::READER_PREFIX, + "", + label_constants::READER_BASE + )); + assert!(!label_matches_normalized( + "approved:owner/repo", + label_constants::MERGED_PREFIX, + "owner/repo", + label_constants::MERGED_BASE + )); + } + #[test] fn test_integrity_for_level_mapping() { let ctx = test_ctx(); diff --git a/guards/github-guard/rust-guard/src/lib.rs b/guards/github-guard/rust-guard/src/lib.rs index 88a56a57e..372571a72 100644 --- a/guards/github-guard/rust-guard/src/lib.rs +++ b/guards/github-guard/rust-guard/src/lib.rs @@ -529,7 +529,6 @@ pub extern "C" fn label_agent( }) .collect(); - let token = labels::helpers::policy_scope_token(&scopes); let scope_kind_str = normalized_scope_kind(&scopes); let ctx = PolicyContext { @@ -547,6 +546,7 @@ pub extern "C" fn label_agent( }; // Compute integrity before moving ctx into the global — borrows ctx, no clone needed. + let token = labels::helpers::policy_scope_token(&ctx.scopes); let integrity = match integrity_floor { MinIntegrity::None => labels::none_integrity(&token, &ctx), MinIntegrity::Unapproved => labels::reader_integrity(&token, &ctx), From 570c9693f97c5b37b580a32ceae50a1f42651f2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:33:48 +0000 Subject: [PATCH 3/4] Hoist multi-scope integrity label formatting out of label loop --- .../rust-guard/src/labels/helpers.rs | 31 ++++++++++++++----- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index b7ba71be9..37f18855f 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -1293,11 +1293,16 @@ fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 { } fn integrity_rank_normalized(normalized_scope: &str, labels: &[String]) -> u8 { + let is_multi_scope = normalized_scope.contains('|'); + // Check from highest to lowest. for (rank, (prefix, base)) in INTEGRITY_LEVELS.iter().enumerate().rev() { + let expected = is_multi_scope.then(|| format_integrity_label(prefix, normalized_scope, base)); if labels .iter() - .any(|label| label_matches_normalized(label, prefix, normalized_scope, base)) + .any(|label| { + label_matches_normalized(label, prefix, normalized_scope, base, expected.as_deref()) + }) { return (rank + 1) as u8; } @@ -1306,12 +1311,18 @@ fn integrity_rank_normalized(normalized_scope: &str, labels: &[String]) -> u8 { } #[inline] -fn label_matches_normalized(label: &str, prefix: &str, scope: &str, base: &str) -> bool { +fn label_matches_normalized( + label: &str, + prefix: &str, + scope: &str, + base: &str, + expected_multi_scope: Option<&str>, +) -> bool { if scope.is_empty() { label == base - } else if scope.contains('|') { + } else if let Some(expected) = expected_multi_scope { // Multi-scope uses canonical "integrity=;scopes=..." encoding. - label == format_integrity_label(prefix, scope, base) + label == expected } else { label.strip_prefix(prefix) == Some(scope) } @@ -2389,25 +2400,29 @@ mod tests { "approved:owner/repo", label_constants::WRITER_PREFIX, "owner/repo", - label_constants::WRITER_BASE + label_constants::WRITER_BASE, + None )); assert!(label_matches_normalized( "integrity=approved;scopes=owner/repo,owner/tool*", label_constants::WRITER_PREFIX, "owner/repo | owner/tool*", - label_constants::WRITER_BASE + label_constants::WRITER_BASE, + Some("integrity=approved;scopes=owner/repo,owner/tool*") )); assert!(label_matches_normalized( label_constants::READER_BASE, label_constants::READER_PREFIX, "", - label_constants::READER_BASE + label_constants::READER_BASE, + None )); assert!(!label_matches_normalized( "approved:owner/repo", label_constants::MERGED_PREFIX, "owner/repo", - label_constants::MERGED_BASE + label_constants::MERGED_BASE, + None )); } From 1c3618548c4dad534fc9b14b58b99a4f7c81eab1 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 15 May 2026 14:37:21 +0000 Subject: [PATCH 4/4] Refine multi-scope rank path to avoid per-iteration closure overhead --- .../rust-guard/src/labels/helpers.rs | 49 ++++++++++--------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index 37f18855f..f1b1e66de 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -1293,16 +1293,22 @@ fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 { } fn integrity_rank_normalized(normalized_scope: &str, labels: &[String]) -> u8 { - let is_multi_scope = normalized_scope.contains('|'); + if normalized_scope.contains('|') { + // Multi-scope uses canonical "integrity=;scopes=..." encoding. + for (rank, (prefix, base)) in INTEGRITY_LEVELS.iter().enumerate().rev() { + let expected = format_integrity_label(prefix, normalized_scope, base); + if labels.iter().any(|label| label == &expected) { + return (rank + 1) as u8; + } + } + return 0; + } // Check from highest to lowest. for (rank, (prefix, base)) in INTEGRITY_LEVELS.iter().enumerate().rev() { - let expected = is_multi_scope.then(|| format_integrity_label(prefix, normalized_scope, base)); if labels .iter() - .any(|label| { - label_matches_normalized(label, prefix, normalized_scope, base, expected.as_deref()) - }) + .any(|label| label_matches_normalized(label, prefix, normalized_scope, base)) { return (rank + 1) as u8; } @@ -1316,13 +1322,9 @@ fn label_matches_normalized( prefix: &str, scope: &str, base: &str, - expected_multi_scope: Option<&str>, ) -> bool { if scope.is_empty() { label == base - } else if let Some(expected) = expected_multi_scope { - // Multi-scope uses canonical "integrity=;scopes=..." encoding. - label == expected } else { label.strip_prefix(prefix) == Some(scope) } @@ -2395,37 +2397,38 @@ mod tests { } #[test] - fn test_label_matches_normalized_common_and_multiscope_paths() { + fn test_label_matches_normalized_common_paths() { assert!(label_matches_normalized( "approved:owner/repo", label_constants::WRITER_PREFIX, "owner/repo", - label_constants::WRITER_BASE, - None - )); - assert!(label_matches_normalized( - "integrity=approved;scopes=owner/repo,owner/tool*", - label_constants::WRITER_PREFIX, - "owner/repo | owner/tool*", - label_constants::WRITER_BASE, - Some("integrity=approved;scopes=owner/repo,owner/tool*") + label_constants::WRITER_BASE )); assert!(label_matches_normalized( label_constants::READER_BASE, label_constants::READER_PREFIX, "", - label_constants::READER_BASE, - None + label_constants::READER_BASE )); assert!(!label_matches_normalized( "approved:owner/repo", label_constants::MERGED_PREFIX, "owner/repo", - label_constants::MERGED_BASE, - None + label_constants::MERGED_BASE )); } + #[test] + fn test_integrity_rank_normalized_multiscope_path() { + let scope = "owner/repo | owner/tool*"; + let labels = vec![format_integrity_label( + label_constants::WRITER_PREFIX, + scope, + label_constants::WRITER_BASE, + )]; + assert_eq!(integrity_rank_normalized(scope, &labels), 3); + } + #[test] fn test_integrity_for_level_mapping() { let ctx = test_ctx();