Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions guards/github-guard/rust-guard/src/labels/backend.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,27 @@ fn repo_owner_type_cache() -> &'static Mutex<HashMap<String, bool>> {
CACHE.get_or_init(|| Mutex::new(HashMap::new()))
}

/// Cache for collaborator permission lookups keyed by "owner/repo:username".
/// Caches the raw permission string so it can be reused across multiple items
/// that share the same reactor within a single gateway request.
fn collaborator_permission_cache() -> &'static Mutex<HashMap<String, Option<String>>> {
static CACHE: OnceLock<Mutex<HashMap<String, Option<String>>>> = OnceLock::new();
CACHE.get_or_init(|| Mutex::new(HashMap::new()))
}
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The collaborator-permission cache is implemented as a process-wide static (OnceLock + Mutex) and is never cleared/evicted, but the comment states it is per-request. This can lead to unbounded growth (unique user/repo combos) and stale permission decisions affecting integrity promotion/demotion; consider scoping this cache to a single gateway request (or adding TTL/LRU + explicit reset at request boundaries) and updating the comment accordingly.

Copilot uses AI. Check for mistakes.

fn get_cached_collaborator_permission(key: &str) -> Option<Option<String>> {
collaborator_permission_cache()
.lock()
.ok()
.and_then(|cache| cache.get(key).cloned())
}

fn set_cached_collaborator_permission(key: &str, permission: Option<String>) {
if let Ok(mut cache) = collaborator_permission_cache().lock() {
cache.insert(key.to_string(), permission);
}
}

fn get_cached_repo_visibility(repo_id: &str) -> Option<bool> {
repo_visibility_cache()
.lock()
Expand Down Expand Up @@ -449,6 +470,9 @@ pub fn get_issue_author_info(
/// 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.
///
/// Results are cached per `(owner, repo, username)` to avoid duplicate enrichment
/// calls when the same reactor appears on multiple items in a response collection.
pub fn get_collaborator_permission_with_callback(
callback: GithubMcpCallback,
owner: &str,
Expand All @@ -463,6 +487,23 @@ pub fn get_collaborator_permission_with_callback(
return None;
}

// Cache key uses lowercase username because GitHub usernames are case-insensitive.
// The original case-sensitive username is preserved in the returned CollaboratorPermission
// struct (via `username.to_string()`) so callers see the canonical display form.
let cache_key = format!("{}/{}:{}", owner, repo, username.to_ascii_lowercase());

// Return cached permission if available.
if let Some(cached) = get_cached_collaborator_permission(&cache_key) {
crate::log_debug(&format!(
"get_collaborator_permission: cache hit for {}/{} user {} → permission={:?}",
owner, repo, username, cached
));
return cached.map(|permission| CollaboratorPermission {
permission: Some(permission),
login: Some(username.to_string()),
});
}
Copy link

Copilot AI Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cache key only normalizes username casing; owner/repo are also case-insensitive on GitHub, so differing case in inputs will reduce cache hits and increase cache size. Consider normalizing owner/repo to a canonical form (e.g., lowercasing) when building the cache key.

Copilot uses AI. Check for mistakes.

crate::log_debug(&format!(
"get_collaborator_permission: fetching permission for {}/{} user {}",
owner, repo, username
Expand All @@ -484,6 +525,7 @@ pub fn get_collaborator_permission_with_callback(
"get_collaborator_permission: empty response for {}/{} user {}",
owner, repo, username
));
set_cached_collaborator_permission(&cache_key, None);
return None;
}
Err(code) => {
Expand Down Expand Up @@ -535,6 +577,8 @@ pub fn get_collaborator_permission_with_callback(
owner, repo, username, permission, login
));

set_cached_collaborator_permission(&cache_key, permission.clone());

Some(CollaboratorPermission { permission, login })
}

Expand Down
Loading
Loading