Conversation
…e URL fallback loop - Add is_any_trusted_actor(username, ctx) helper combining the three constituent trust predicates (first-party bot, configured bot, trusted user); replaces copy-pasted triple-OR at three call sites in helpers.rs and tool_rules.rs - Replace three structurally identical if-let blocks in extract_repo_from_item with a for-field loop over ["repository_url", "html_url", "url"], matching the idiom already used in extract_number_from_url All 317 Rust guard tests pass. Closes #4252 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the Rust GitHub guard labeling logic to reduce duplication by centralizing the “trusted actor” predicate and simplifying repeated URL-field fallback extraction.
Changes:
- Adds
is_any_trusted_actor(username, ctx)helper and replaces repeated three-way OR checks with a single call. - Collapses repeated URL-field parsing in
extract_repo_from_iteminto a loop over candidate fields. - Updates
tool_rules.rsimports/call site to use the new helper.
Show a summary per file
| File | Description |
|---|---|
guards/github-guard/rust-guard/src/labels/helpers.rs |
Introduces is_any_trusted_actor and deduplicates URL fallback logic in extract_repo_from_item. |
guards/github-guard/rust-guard/src/labels/tool_rules.rs |
Switches trusted-actor elevation logic to the new helper and updates imports accordingly. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
Comment on lines
+976
to
983
| // URL field fallback (repository_url for search results, html_url / url as generic fallbacks) | ||
| for field in &["repository_url", "html_url", "url"] { | ||
| if let Some(url) = item.get(*field).and_then(|v| v.as_str()) { | ||
| if let Some(repo_id) = extract_repo_from_github_url(url) { | ||
| return repo_id; | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
extract_repo_from_item now clearly falls back across repository_url, html_url, and url, but the function-level doc comment above still describes only html_url parsing. Please update the doc comment to match the actual fallback fields/order (and consider matching the existing extract_number_from_url loop style by avoiding the unnecessary *field deref for consistency).
Comment on lines
+1757
to
+1763
| /// Returns `true` if `username` belongs to any trusted actor tier: | ||
| /// first-party bots, gateway-configured bots, or trusted users. | ||
| pub(crate) fn is_any_trusted_actor(username: &str, ctx: &PolicyContext) -> bool { | ||
| is_trusted_first_party_bot(username) | ||
| || is_configured_trusted_bot(username, ctx) | ||
| || is_trusted_user(username, ctx) | ||
| } |
There was a problem hiding this comment.
is_any_trusted_actor is a new helper that centralizes a security-relevant trust decision and is now used at multiple call sites. Please add a small unit test covering the three tiers (first-party bot, configured trusted bot, trusted user) and a negative case to lock in the intended semantics and catch future omissions when new trust tiers are added.
Collaborator
|
@copilot address the review feedback #4260 (review) |
…lback docs Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/ecd07254-bbdd-4935-a0c0-b4d4b0114e76 Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Contributor
Addressed in 761545c. I updated the Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
lpcox
deleted the
repo-assist/fix-issue-4252-rust-guard-trusted-actor-url-loop-be9d08bbcb5c1877
branch
This was referenced Apr 21, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 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/: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.extract_repo_from_itemhad three structurally identicalif letblocks differing only in the JSON field name (repository_url,html_url,url). The same for-loop idiom is already used inextract_number_from_urlin the same file.Fix
helpers.rsis_any_trusted_actor(username, ctx)— a single helper that combines the three predicates with a docstring explaining its semantics.helpers.rswith one-lineris_any_trusted_actorcalls.if letURL-field blocks inextract_repo_from_iteminto afor field in &[...]loop (same pattern asextract_number_from_url).tool_rules.rsis_any_trusted_actor.is_any_trusted_actorinstead of the three individual functions.Trade-offs
is_any_trusted_actorispub(crate)— callable within the crate but not exported.Test Status
cargo build— compiles without warningscargo test— 317 / 317 tests passproxy.golang.orgblocked by network firewall (pre-existing infrastructure limitation, unrelated to this change)Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.