Skip to content

Commit 6b4e51d

Browse files
authored
[Repo Assist] refactor(rust-guard): extract is_any_trusted_actor helper and collapse URL fallback loop (#4260)
🤖 *This is an automated pull request from Repo Assist.* Closes #4252 ## Root Cause Two minor duplication issues in `guards/github-guard/rust-guard/src/labels/`: 1. The three-predicate trust check `is_trusted_first_party_bot(x) || is_configured_trusted_bot(x, ctx) || is_trusted_user(x, ctx)` was copy-pasted verbatim at **three call sites** (`helpers.rs:1282–1284`, `helpers.rs:1479–1481`, `tool_rules.rs:98–100`). Adding a fourth trust tier would require three matching edits. 2. `extract_repo_from_item` had three structurally identical `if let` blocks differing only in the JSON field name (`repository_url`, `html_url`, `url`). The same for-loop idiom is already used in `extract_number_from_url` in the same file. ## Fix **`helpers.rs`** - Added `is_any_trusted_actor(username, ctx)` — a single helper that combines the three predicates with a docstring explaining its semantics. - Replaced both call sites in `helpers.rs` with one-liner `is_any_trusted_actor` calls. - Collapsed the three repeated `if let` URL-field blocks in `extract_repo_from_item` into a `for field in &[...]` loop (same pattern as `extract_number_from_url`). **`tool_rules.rs`** - Replaced the third call site with `is_any_trusted_actor`. - Updated the import to use `is_any_trusted_actor` instead of the three individual functions. ## Trade-offs - Purely mechanical refactor; zero semantic change. - `is_any_trusted_actor` is `pub(crate)` — callable within the crate but not exported. ## Test Status - ✅ `cargo build` — compiles without warnings - ✅ `cargo test` — **317 / 317 tests pass** - ⚠️ Go build/test skipped — `proxy.golang.org` blocked by network firewall (pre-existing infrastructure limitation, unrelated to this change) > [!WARNING] > <details> > <summary><strong>⚠️ Firewall blocked 1 domain</strong></summary> > > The following domain was blocked by the firewall during workflow execution: > > - `proxy.golang.org` > > To allow these domains, add them to the `network.allowed` list in your workflow frontmatter: > > ```yaml > network: > allowed: > - defaults > - "proxy.golang.org" > ``` > > See [Network Configuration](https://github.github.com/gh-aw/reference/network/) for more information. > > </details> > Generated by [Repo Assist](https://github.com/github/gh-aw-mcpg/actions/runs/24722841729/agentic_workflow) · ● 4.8M · [◷](https://github.com/search?q=repo%3Agithub%2Fgh-aw-mcpg+%22gh-aw-workflow-id%3A+repo-assist%22&type=pullrequests) > > To install this [agentic workflow](https://github.com/githubnext/agentics/blob/851905c06e905bf362a9f6cc54f912e3df747d55/workflows/repo-assist.md), run > ``` > gh aw add githubnext/agentics@851905c > ``` <!-- gh-aw-agentic-workflow: Repo Assist, engine: copilot, model: auto, id: 24722841729, workflow_id: repo-assist, run: https://github.com/github/gh-aw-mcpg/actions/runs/24722841729 --> <!-- gh-aw-workflow-id: repo-assist -->
2 parents 625f48a + 761545c commit 6b4e51d

2 files changed

Lines changed: 35 additions & 33 deletions

File tree

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

Lines changed: 32 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -940,7 +940,8 @@ pub(crate) fn extract_repo_from_github_url(url: &str) -> Option<String> {
940940

941941
/// Extract repository full name from a response item
942942
/// Tries multiple fields in order: full_name, repository.full_name,
943-
/// base.repo.full_name, head.repo.full_name, html_url parsing
943+
/// base.repo.full_name, head.repo.full_name, then URL parsing from
944+
/// repository_url, html_url, and url.
944945
/// Returns empty string if no repo info found
945946
pub fn extract_repo_from_item(item: &Value) -> String {
946947
// Direct full_name (repositories)
@@ -973,23 +974,12 @@ pub fn extract_repo_from_item(item: &Value) -> String {
973974
{
974975
return name.to_string();
975976
}
976-
// repository_url parsing for search endpoints
977-
if let Some(url) = item.get("repository_url").and_then(|v| v.as_str()) {
978-
if let Some(repo_id) = extract_repo_from_github_url(url) {
979-
return repo_id;
980-
}
981-
}
982-
// html_url parsing as last resort - extract owner/repo from URLs like:
983-
// https://github.com/owner/repo/pull/123 or https://github.com/owner/repo/issues/456
984-
if let Some(url) = item.get("html_url").and_then(|v| v.as_str()) {
985-
if let Some(repo_id) = extract_repo_from_github_url(url) {
986-
return repo_id;
987-
}
988-
}
989-
// Generic URL field fallback
990-
if let Some(url) = item.get("url").and_then(|v| v.as_str()) {
991-
if let Some(repo_id) = extract_repo_from_github_url(url) {
992-
return repo_id;
977+
// URL field fallback (repository_url for search results, html_url / url as generic fallbacks)
978+
for field in &["repository_url", "html_url", "url"] {
979+
if let Some(url) = item.get(field).and_then(|v| v.as_str()) {
980+
if let Some(repo_id) = extract_repo_from_github_url(url) {
981+
return repo_id;
982+
}
993983
}
994984
}
995985
String::new()
@@ -1278,11 +1268,7 @@ pub fn has_author_association(item: &Value) -> bool {
12781268
/// Users in the trusted_users list are also elevated to approved integrity.
12791269
pub fn author_association_floor(item: &Value, scope: &str, ctx: &PolicyContext) -> Vec<String> {
12801270
let author_login = extract_author_login(item);
1281-
if !author_login.is_empty()
1282-
&& (is_trusted_first_party_bot(author_login)
1283-
|| is_configured_trusted_bot(author_login, ctx)
1284-
|| is_trusted_user(author_login, ctx))
1285-
{
1271+
if !author_login.is_empty() && is_any_trusted_actor(author_login, ctx) {
12861272
return writer_integrity(scope, ctx);
12871273
}
12881274

@@ -1476,10 +1462,7 @@ pub fn pr_integrity(
14761462
);
14771463
// Elevate trusted bots and trusted users
14781464
let enriched_floor = if let Some(ref login) = facts.author_login {
1479-
if is_trusted_first_party_bot(login)
1480-
|| is_configured_trusted_bot(login, ctx)
1481-
|| is_trusted_user(login, ctx)
1482-
{
1465+
if is_any_trusted_actor(login, ctx) {
14831466
max_integrity(
14841467
repo_full_name,
14851468
enriched_floor,
@@ -1772,6 +1755,14 @@ pub fn is_trusted_user(username: &str, ctx: &PolicyContext) -> bool {
17721755
username_in_list(username, &ctx.trusted_users)
17731756
}
17741757

1758+
/// Returns `true` if `username` belongs to any trusted actor tier:
1759+
/// first-party bots, gateway-configured bots, or trusted users.
1760+
pub(crate) fn is_any_trusted_actor(username: &str, ctx: &PolicyContext) -> bool {
1761+
is_trusted_first_party_bot(username)
1762+
|| is_configured_trusted_bot(username, ctx)
1763+
|| is_trusted_user(username, ctx)
1764+
}
1765+
17751766

17761767
#[cfg(test)]
17771768
mod tests {
@@ -1781,6 +1772,20 @@ mod tests {
17811772
PolicyContext::default()
17821773
}
17831774

1775+
#[test]
1776+
fn test_is_any_trusted_actor_tiers_and_negative() {
1777+
let ctx = PolicyContext {
1778+
trusted_bots: vec!["custom-bot".to_string()],
1779+
trusted_users: vec!["trusted-human".to_string()],
1780+
..Default::default()
1781+
};
1782+
1783+
assert!(is_any_trusted_actor("github-actions[bot]", &ctx));
1784+
assert!(is_any_trusted_actor("custom-bot", &ctx));
1785+
assert!(is_any_trusted_actor("trusted-human", &ctx));
1786+
assert!(!is_any_trusted_actor("random-user", &ctx));
1787+
}
1788+
17841789
#[test]
17851790
fn test_collaborator_permission_floor_admin() {
17861791
let ctx = test_ctx();

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

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@ use super::helpers::{
1010
author_association_floor_from_str,
1111
elevate_via_collaborator_permission, ensure_integrity_baseline,
1212
extract_number_as_string, extract_repo_info, extract_repo_info_from_search_query,
13-
format_repo_id, is_configured_trusted_bot, is_default_branch_commit_context,
14-
is_default_branch_ref, is_trusted_first_party_bot, is_trusted_user, max_integrity,
13+
format_repo_id, is_any_trusted_actor, is_default_branch_commit_context,
14+
is_default_branch_ref, max_integrity,
1515
merged_integrity, policy_private_scope_label, private_user_label, project_github_label,
1616
reader_integrity, writer_integrity, PolicyContext,
1717
};
@@ -95,10 +95,7 @@ fn resolve_author_integrity(
9595
let mut floor = author_association_floor_from_str(repo_id, author_association, ctx);
9696

9797
if let Some(login) = author_login {
98-
if is_trusted_first_party_bot(login)
99-
|| is_configured_trusted_bot(login, ctx)
100-
|| is_trusted_user(login, ctx)
101-
{
98+
if is_any_trusted_actor(login, ctx) {
10299
floor = max_integrity(repo_id, floor, writer_integrity(repo_id, ctx), ctx);
103100
}
104101
let resource_id = format!("{}/{}#{}", owner, repo, resource_num);

0 commit comments

Comments
 (0)