Skip to content

rust-guard: fix configured trusted bot elevation in apply_tool_labels + deduplicate item collection #2574

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lpcox merged 2 commits into from
Mar 26, 2026
Merged

rust-guard: fix configured trusted bot elevation in apply_tool_labels + deduplicate item collection #2574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b288413
Initial plan
Copilot Mar 26, 2026
0692cc9
rust-guard: fix is_configured_trusted_bot in apply_tool_labels and ex…
Copilot Mar 26, 2026
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
17 changes: 17 additions & 0 deletions guards/github-guard/rust-guard/src/labels/helpers.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -660,6 +660,23 @@ pub fn extract_items_array(response: &Value) -> (Option<&Vec<Value>>, String) {
(None, String::new())
}

/// Collect items from a response that is either a JSON array or a single object.
///
/// Returns a `Vec<&Value>` of items to process. Wrappers like MCP text envelopes
/// and search-result metadata objects are excluded from single-object promotion.
pub(crate) fn collect_items_simple(response: &Value) -> Vec<&Value> {
Comment on lines +663 to +667

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

collect_items_simple is now a core piece of response item collection, but there are no unit tests that pin down its behavior (array passthrough vs. single-object promotion vs. returning empty for MCP text wrappers and search-result metadata). Adding small, direct tests for this helper would help prevent future regressions when response shapes change.

Copilot uses AI. Check for mistakes.
if let Some(arr) = response.as_array() {
arr.iter().collect()
} else if response.is_object()
&& !is_search_result_wrapper(response)
&& !is_mcp_text_wrapper(response)
{
vec![response]
} else {
vec![]
}
}

/// GraphQL collection fields under data.repository and their JSON Pointer paths.
const GRAPHQL_COLLECTION_FIELDS: &[(&str, &str)] = &[
("issues", "/data/repository/issues/nodes"),
Expand Down
44 changes: 4 additions & 40 deletions guards/github-guard/rust-guard/src/labels/response_items.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,16 +316,7 @@ pub fn label_response_items(

// === File Contents - repo-scoped secrecy ===
"get_file_contents" => {
let all_items: Vec<&Value> = if actual_response.is_array() {
actual_response
.as_array()
.map(|arr| arr.iter().collect())
.unwrap_or_default()
} else if actual_response.is_object() && !is_search_result_wrapper(&actual_response) && !is_mcp_text_wrapper(&actual_response) {
vec![&actual_response]
} else {
vec![]
};
let all_items = collect_items_simple(&actual_response);

let items_limited = limit_items_with_log(all_items.as_slice(), "get_file_contents");
let (arg_owner, arg_repo, repo_full_name) = extract_repo_info(tool_args);
Expand All @@ -351,16 +342,7 @@ pub fn label_response_items(

// === Commits - label by branch (default branch = merged) ===
"list_commits" | "get_commit" => {
let all_items: Vec<&Value> = if actual_response.is_array() {
actual_response
.as_array()
.map(|arr| arr.iter().collect())
.unwrap_or_default()
} else if actual_response.is_object() && !is_search_result_wrapper(&actual_response) && !is_mcp_text_wrapper(&actual_response) {
vec![&actual_response]
} else {
vec![]
};
let all_items = collect_items_simple(&actual_response);

// Limit items to prevent WASM memory exhaustion
let items_limited = limit_items_with_log(all_items.as_slice(), "list_commits");
Expand Down Expand Up @@ -403,16 +385,7 @@ pub fn label_response_items(

// === Gists - label by visibility ===
"list_gists" | "get_gist" => {
let all_items: Vec<&Value> = if actual_response.is_array() {
actual_response
.as_array()
.map(|arr| arr.iter().collect())
.unwrap_or_default()
} else if actual_response.is_object() && !is_search_result_wrapper(&actual_response) && !is_mcp_text_wrapper(&actual_response) {
vec![&actual_response]
} else {
vec![]
};
let all_items = collect_items_simple(&actual_response);

// Limit items to prevent WASM memory exhaustion
let items_limited = limit_items_with_log(all_items.as_slice(), "list_gists");
Expand Down Expand Up @@ -460,16 +433,7 @@ pub fn label_response_items(

// === Releases - merged-level integrity (endorsed) ===
"list_releases" | "get_latest_release" | "get_release_by_tag" => {
let all_items: Vec<&Value> = if actual_response.is_array() {
actual_response
.as_array()
.map(|arr| arr.iter().collect())
.unwrap_or_default()
} else if actual_response.is_object() && !is_search_result_wrapper(&actual_response) && !is_mcp_text_wrapper(&actual_response) {
vec![&actual_response]
} else {
vec![]
};
let all_items = collect_items_simple(&actual_response);

// Limit items to prevent WASM memory exhaustion
let items_limited = limit_items_with_log(all_items.as_slice(), "list_releases");
Expand Down
15 changes: 10 additions & 5 deletions guards/github-guard/rust-guard/src/labels/tool_rules.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ use super::constants::{field_names, SENSITIVE_FILE_KEYWORDS, SENSITIVE_FILE_PATT
use super::helpers::{
author_association_floor_from_str, ensure_integrity_baseline, extract_number_as_string,
extract_repo_info, extract_repo_info_from_search_query, format_repo_id,
is_default_branch_commit_context, is_default_branch_ref, is_trusted_first_party_bot,
max_integrity, merged_integrity, policy_private_scope_label, private_user_label,
project_github_label, reader_integrity, secret_label, writer_integrity, PolicyContext,
is_configured_trusted_bot, is_default_branch_commit_context, is_default_branch_ref,
is_trusted_first_party_bot, max_integrity, merged_integrity, policy_private_scope_label,
private_user_label, project_github_label, reader_integrity, secret_label, writer_integrity,
PolicyContext,
};

fn apply_repo_visibility_secrecy(
Expand Down Expand Up @@ -127,7 +128,9 @@ pub fn apply_tool_labels(
);
// Elevate trusted first-party bots to approved
if let Some(ref login) = info.author_login {
if is_trusted_first_party_bot(login) {
if is_trusted_first_party_bot(login)
|| is_configured_trusted_bot(login, ctx)
{
Comment on lines 129 to +133

Copilot AI Mar 26, 2026

Copy link

Choose a reason for hiding this comment

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

This adds configured-trusted-bot elevation for resource-level get_issue integrity, but this branch is hard to exercise in unit tests because backend calls are stubbed to fail under cfg(test) (so get_issue_author_info returns None). Consider extracting the elevation decision into a pure helper or injecting a backend callback (similar to the existing *_with_callback helpers) so a unit test can cover the trusted_bots path.

Copilot uses AI. Check for mistakes.
floor = max_integrity(
repo_id,
floor,
Expand Down Expand Up @@ -185,7 +188,9 @@ pub fn apply_tool_labels(

// Elevate trusted first-party bots to approved
if let Some(ref login) = facts.author_login {
if is_trusted_first_party_bot(login) {
if is_trusted_first_party_bot(login)
|| is_configured_trusted_bot(login, ctx)
{
integrity = max_integrity(
repo_id,
integrity,
Expand Down
Loading