Skip to content

Commit fb48631

Browse files
authored
rust-guard: remove hot-path scope/integrity allocations via Cow<str> and zero-alloc rank matching (#5754)
## ✨ Enhancement `policy_scope_token` and `normalize_scope` were allocating on common single-scope/fallthrough paths, and `integrity_rank_normalized` was allocating per integrity level just to compare labels. This change removes those avoidable allocations in the main integrity-labeling path while preserving existing label semantics. **What does this improve?** - **Scope token + normalization fast path** - `policy_scope_token` now returns `Cow<'_, str>`: - empty scopes → borrowed `""` - single scope → borrowed scope label - multi-scope → owned joined token - `normalize_scope` now returns `Cow<'a, str>`, borrowing `scope` when no scoped replacement is needed instead of cloning. - **Integrity rank matching fast path** - `integrity_rank_normalized` now uses `label_matches_normalized(...)` instead of constructing formatted labels for each rank. - Common non-multi-scope comparisons are allocation-free; multi-scope keeps canonical formatting fallback. - **Call-site/lifetime adjustment** - In `label_agent`, scope token derivation was moved to read from `ctx.scopes` after context construction to satisfy borrowing with the new `Cow`-based flow. **Why is this valuable?** - Cuts repeated small heap allocations in per-item integrity computation (`reader/writer/merged/none/max/cap` paths), especially in the dominant single-scope case and rank comparisons. **Implementation approach:** - Introduced `Cow` return types for scope helpers and propagated usage through existing integrity builders without behavior changes. - Added a non-allocating label predicate with rare-path fallback for multi-scope canonical formatting. - Added focused helper tests for: - borrowed vs owned `policy_scope_token` behavior, - borrowed `normalize_scope` fallthrough behavior, - `label_matches_normalized` parity across empty/single/multi-scope forms. ```rust 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()); 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) } } } ```
2 parents 04941b0 + 1c36185 commit fb48631

2 files changed

Lines changed: 128 additions & 16 deletions

File tree

guards/github-guard/rust-guard/src/labels/helpers.rs

Lines changed: 127 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
//! This module contains utility functions used across the labeling system,
44
//! including JSON extraction, integrity determination, and common operations.
55
6+
use std::borrow::Cow;
67
use std::sync::atomic::{AtomicBool, Ordering};
78

89
use serde_json::Value;
@@ -156,10 +157,10 @@ pub struct PolicyContext {
156157
pub demotion_label: String,
157158
}
158159

159-
fn normalize_scope(scope: &str, ctx: &PolicyContext) -> String {
160+
fn normalize_scope<'a>(scope: &'a str, ctx: &'a PolicyContext) -> Cow<'a, str> {
160161
let token = policy_scope_token(&ctx.scopes);
161162
if token.is_empty() {
162-
scope.to_string()
163+
Cow::Borrowed(scope)
163164
} else if ctx
164165
.scopes
165166
.iter()
@@ -176,10 +177,10 @@ fn normalize_scope(scope: &str, ctx: &PolicyContext) -> String {
176177
if matches_any_scope {
177178
token
178179
} else {
179-
scope.to_string()
180+
Cow::Borrowed(scope)
180181
}
181182
} else {
182-
scope.to_string()
183+
Cow::Borrowed(scope)
183184
}
184185
}
185186

@@ -191,16 +192,24 @@ fn split_repo_id(repo_id: &str) -> Option<(&str, &str)> {
191192
Some((owner, repo))
192193
}
193194

194-
pub(crate) fn policy_scope_token(scopes: &[PolicyScopeEntry]) -> String {
195-
let labels: Vec<&str> = scopes
195+
pub(crate) fn policy_scope_token(scopes: &[PolicyScopeEntry]) -> Cow<'_, str> {
196+
let mut labels = scopes
196197
.iter()
197198
.map(|s| s.scope_label.as_str())
198-
.filter(|s| !s.is_empty())
199-
.collect();
200-
if labels.is_empty() {
201-
String::new()
202-
} else {
203-
labels.join(" | ")
199+
.filter(|s| !s.is_empty());
200+
match (labels.next(), labels.next()) {
201+
(None, _) => Cow::Borrowed(""),
202+
(Some(first), None) => Cow::Borrowed(first),
203+
(Some(first), Some(second)) => {
204+
let mut value = String::from(first);
205+
value.push_str(" | ");
206+
value.push_str(second);
207+
for rest in labels {
208+
value.push_str(" | ");
209+
value.push_str(rest);
210+
}
211+
Cow::Owned(value)
212+
}
204213
}
205214
}
206215

@@ -1284,16 +1293,43 @@ fn integrity_rank(scope: &str, labels: &[String], ctx: &PolicyContext) -> u8 {
12841293
}
12851294

12861295
fn integrity_rank_normalized(normalized_scope: &str, labels: &[String]) -> u8 {
1287-
// Check from highest to lowest, allocating one label at a time.
1296+
if normalized_scope.contains('|') {
1297+
// Multi-scope uses canonical "integrity=<base>;scopes=..." encoding.
1298+
for (rank, (prefix, base)) in INTEGRITY_LEVELS.iter().enumerate().rev() {
1299+
let expected = format_integrity_label(prefix, normalized_scope, base);
1300+
if labels.iter().any(|label| label == &expected) {
1301+
return (rank + 1) as u8;
1302+
}
1303+
}
1304+
return 0;
1305+
}
1306+
1307+
// Check from highest to lowest.
12881308
for (rank, (prefix, base)) in INTEGRITY_LEVELS.iter().enumerate().rev() {
1289-
let tag = format_integrity_label(prefix, normalized_scope, base);
1290-
if labels.iter().any(|l| l == &tag) {
1309+
if labels
1310+
.iter()
1311+
.any(|label| label_matches_normalized(label, prefix, normalized_scope, base))
1312+
{
12911313
return (rank + 1) as u8;
12921314
}
12931315
}
12941316
0
12951317
}
12961318

1319+
#[inline]
1320+
fn label_matches_normalized(
1321+
label: &str,
1322+
prefix: &str,
1323+
scope: &str,
1324+
base: &str,
1325+
) -> bool {
1326+
if scope.is_empty() {
1327+
label == base
1328+
} else {
1329+
label.strip_prefix(prefix) == Some(scope)
1330+
}
1331+
}
1332+
12971333
/// Elevate integrity to the max of current and candidate levels for a scope.
12981334
pub fn max_integrity(
12991335
scope: &str,
@@ -1917,11 +1953,54 @@ pub(crate) fn is_any_trusted_actor(username: &str, ctx: &PolicyContext) -> bool
19171953
#[cfg(test)]
19181954
mod tests {
19191955
use super::*;
1956+
use std::borrow::Cow;
19201957

19211958
fn test_ctx() -> PolicyContext {
19221959
PolicyContext::default()
19231960
}
19241961

1962+
#[test]
1963+
fn test_policy_scope_token_borrows_for_single_scope() {
1964+
let scopes = vec![PolicyScopeEntry {
1965+
scope_kind: ScopeKind::Owner,
1966+
scope_owner: Some("octocat".to_string()),
1967+
scope_repo: None,
1968+
scope_label: "octocat".to_string(),
1969+
}];
1970+
let token = policy_scope_token(&scopes);
1971+
assert_eq!(token, "octocat");
1972+
assert!(matches!(token, Cow::Borrowed(_)));
1973+
}
1974+
1975+
#[test]
1976+
fn test_policy_scope_token_owns_for_multi_scope() {
1977+
let scopes = vec![
1978+
PolicyScopeEntry {
1979+
scope_kind: ScopeKind::Owner,
1980+
scope_owner: Some("octocat".to_string()),
1981+
scope_repo: None,
1982+
scope_label: "octocat".to_string(),
1983+
},
1984+
PolicyScopeEntry {
1985+
scope_kind: ScopeKind::RepoPrefix,
1986+
scope_owner: Some("octocat".to_string()),
1987+
scope_repo: Some("hello".to_string()),
1988+
scope_label: "octocat/hello*".to_string(),
1989+
},
1990+
];
1991+
let token = policy_scope_token(&scopes);
1992+
assert_eq!(token, "octocat | octocat/hello*");
1993+
assert!(matches!(token, Cow::Owned(_)));
1994+
}
1995+
1996+
#[test]
1997+
fn test_normalize_scope_borrows_input_when_no_scopes() {
1998+
let ctx = test_ctx();
1999+
let normalized = normalize_scope("owner/repo", &ctx);
2000+
assert_eq!(normalized, "owner/repo");
2001+
assert!(matches!(normalized, Cow::Borrowed(_)));
2002+
}
2003+
19252004
#[test]
19262005
fn test_is_any_trusted_actor_tiers_and_negative() {
19272006
let ctx = PolicyContext {
@@ -2317,6 +2396,39 @@ mod tests {
23172396
assert_eq!(result, candidate, "max_integrity should keep the higher integrity rank");
23182397
}
23192398

2399+
#[test]
2400+
fn test_label_matches_normalized_common_paths() {
2401+
assert!(label_matches_normalized(
2402+
"approved:owner/repo",
2403+
label_constants::WRITER_PREFIX,
2404+
"owner/repo",
2405+
label_constants::WRITER_BASE
2406+
));
2407+
assert!(label_matches_normalized(
2408+
label_constants::READER_BASE,
2409+
label_constants::READER_PREFIX,
2410+
"",
2411+
label_constants::READER_BASE
2412+
));
2413+
assert!(!label_matches_normalized(
2414+
"approved:owner/repo",
2415+
label_constants::MERGED_PREFIX,
2416+
"owner/repo",
2417+
label_constants::MERGED_BASE
2418+
));
2419+
}
2420+
2421+
#[test]
2422+
fn test_integrity_rank_normalized_multiscope_path() {
2423+
let scope = "owner/repo | owner/tool*";
2424+
let labels = vec![format_integrity_label(
2425+
label_constants::WRITER_PREFIX,
2426+
scope,
2427+
label_constants::WRITER_BASE,
2428+
)];
2429+
assert_eq!(integrity_rank_normalized(scope, &labels), 3);
2430+
}
2431+
23202432
#[test]
23212433
fn test_integrity_for_level_mapping() {
23222434
let ctx = test_ctx();

guards/github-guard/rust-guard/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -529,7 +529,6 @@ pub extern "C" fn label_agent(
529529
})
530530
.collect();
531531

532-
let token = labels::helpers::policy_scope_token(&scopes);
533532
let scope_kind_str = normalized_scope_kind(&scopes);
534533

535534
let ctx = PolicyContext {
@@ -547,6 +546,7 @@ pub extern "C" fn label_agent(
547546
};
548547

549548
// Compute integrity before moving ctx into the global — borrows ctx, no clone needed.
549+
let token = labels::helpers::policy_scope_token(&ctx.scopes);
550550
let integrity = match integrity_floor {
551551
MinIntegrity::None => labels::none_integrity(&token, &ctx),
552552
MinIntegrity::Unapproved => labels::reader_integrity(&token, &ctx),

0 commit comments

Comments
 (0)