From 6c63bbd240be01f10fbefdfebd2222142150f795 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 23 Mar 2026 19:35:53 -0700 Subject: [PATCH 1/4] fix(rust-guard): handle GraphQL-format search results and bot-author filtering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP server v0.32.0+ returns search results in two formats: - REST: {"total_count": N, "items": [...]} - GraphQL: {"totalCount": N, "issues": [...], "pageInfo": {}} The guard only handled the REST format in several places, causing: 1. is_search_result_wrapper() only checked 'total_count', missing GraphQL 'totalCount' — empty GraphQL search results bypassed metadata detection and received unscoped 'none:' integrity tags that fail DIFC matching. 2. response_items.rs only checked 'items' key for PR/issue extraction, missing GraphQL 'issues' and 'pull_requests' keys. 3. response_paths.rs returned Some(PathLabelResult) with zero labeled paths for empty search results, bypassing the server metadata handler in lib.rs that properly scopes integrity tags via writer_integrity(). 4. default_labels in response_paths used none_integrity("") producing 'none:' tags with empty scope that never match agent tags. Fixes: - is_search_result_wrapper now detects both total_count and totalCount - Added search_result_total_count helper for format-agnostic count access - response_items.rs checks 'issues' and 'pull_requests' keys - response_paths.rs returns None for empty search results so lib.rs metadata fallback handles them with properly-scoped writer_integrity - default_labels use actual repo scope instead of empty string - lib.rs uses search_result_total_count instead of hardcoded total_count Tests: - Updated is_search_result_wrapper tests for GraphQL format - Added search_result_total_count test - Added bot-authored issue/PR integrity tests (both REST and GraphQL) Relates to github/gh-aw#22533 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../rust-guard/src/labels/helpers.rs | 19 +++- .../github-guard/rust-guard/src/labels/mod.rs | 98 ++++++++++++++++++- .../rust-guard/src/labels/response_items.rs | 8 +- .../rust-guard/src/labels/response_paths.rs | 22 ++++- guards/github-guard/rust-guard/src/lib.rs | 5 +- 5 files changed, 137 insertions(+), 15 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index 257b78ed6..c6095bdd9 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -668,11 +668,22 @@ pub fn is_graphql_wrapper(response: &Value) -> bool { response.get("data").is_some() } -/// Returns true if the response is a search result wrapper (has "total_count"). -/// Used to prevent treating `{"total_count":0,"incomplete_results":false}` as a -/// single data item when the search returned zero results. +/// Returns true if the response is a search result wrapper. +/// Handles both REST format (`total_count`) and GraphQL format (`totalCount`) +/// returned by different MCP server versions. Used to prevent treating +/// `{"total_count":0,"incomplete_results":false}` or +/// `{"totalCount":0,"issues":[],"pageInfo":{}}` as single data items. pub fn is_search_result_wrapper(response: &Value) -> bool { - response.get("total_count").is_some() + response.get("total_count").is_some() || response.get("totalCount").is_some() +} + +/// Returns the total count from a search result wrapper, handling both +/// REST format (`total_count`) and GraphQL format (`totalCount`). +pub fn search_result_total_count(response: &Value) -> Option { + response + .get("total_count") + .and_then(|v| v.as_u64()) + .or_else(|| response.get("totalCount").and_then(|v| v.as_u64())) } /// Returns true if the response is an MCP content wrapper where the text was not diff --git a/guards/github-guard/rust-guard/src/labels/mod.rs b/guards/github-guard/rust-guard/src/labels/mod.rs index cfd94c1df..1bd0f797e 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -49,8 +49,8 @@ pub use helpers::{ has_author_association, is_blocked_user, is_bot, is_graphql_wrapper, is_mcp_text_wrapper, is_search_result_wrapper, issue_integrity, limit_items_with_log, make_item_path, merged_integrity, none_integrity, pr_integrity, private_scope_label, private_user_label, - project_github_label, reader_integrity, secret_label, writer_integrity, MinIntegrity, - PolicyContext, PolicyScopeEntry, ScopeKind, + project_github_label, reader_integrity, search_result_total_count, secret_label, + writer_integrity, MinIntegrity, PolicyContext, PolicyScopeEntry, ScopeKind, }; #[cfg(test)] pub use helpers::has_approval_label; @@ -3234,12 +3234,106 @@ mod tests { fn test_helpers_is_search_result_wrapper() { use helpers::is_search_result_wrapper; + // REST format assert!(is_search_result_wrapper(&serde_json::json!({"total_count": 0, "incomplete_results": false}))); assert!(is_search_result_wrapper(&serde_json::json!({"total_count": 5, "items": []}))); + // GraphQL format (MCP server v0.32.0+) + assert!(is_search_result_wrapper(&serde_json::json!({"totalCount": 0, "issues": [], "pageInfo": {}}))); + assert!(is_search_result_wrapper(&serde_json::json!({"totalCount": 3, "issues": [{}]}))); + // Non-search assert!(!is_search_result_wrapper(&serde_json::json!({"number": 42, "title": "issue"}))); assert!(!is_search_result_wrapper(&serde_json::json!({}))); } + #[test] + fn test_helpers_search_result_total_count() { + use helpers::search_result_total_count; + + // REST format + assert_eq!(search_result_total_count(&json!({"total_count": 0})), Some(0)); + assert_eq!(search_result_total_count(&json!({"total_count": 42})), Some(42)); + // GraphQL format + assert_eq!(search_result_total_count(&json!({"totalCount": 0})), Some(0)); + assert_eq!(search_result_total_count(&json!({"totalCount": 7})), Some(7)); + // REST takes precedence when both present + assert_eq!(search_result_total_count(&json!({"total_count": 1, "totalCount": 2})), Some(1)); + // Neither present + assert_eq!(search_result_total_count(&json!({"number": 42})), None); + } + + #[test] + fn test_bot_authored_issue_gets_writer_integrity() { + // Verify that github-actions[bot] authored issues get writer integrity + // regardless of author_association value. This is the core test for + // issue github/gh-aw#22533. + let ctx = default_ctx(); + let repo = "github/gh-aw-mcpg"; + + // REST format: user.login present + let rest_item = json!({ + "number": 2320, + "title": "[Repo Assist] Monthly Activity", + "user": {"login": "github-actions[bot]", "id": 41898282}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/github/gh-aw-mcpg" + }); + let integrity = issue_integrity(&rest_item, repo, false, &ctx); + assert!( + integrity.iter().any(|t| t.contains("approved")), + "Bot-authored issue (REST) should have at least approved integrity, got: {:?}", + integrity + ); + + // GraphQL format: author.login present, no user field + let graphql_item = json!({ + "number": 2320, + "title": "[Repo Assist] Monthly Activity", + "author": {"login": "github-actions[bot]"}, + "authorAssociation": "CONTRIBUTOR" + }); + let integrity = issue_integrity(&graphql_item, repo, false, &ctx); + assert!( + integrity.iter().any(|t| t.contains("approved")), + "Bot-authored issue (GraphQL) should have at least approved integrity, got: {:?}", + integrity + ); + } + + #[test] + fn test_bot_authored_pr_gets_writer_integrity() { + let ctx = default_ctx(); + let repo = "github/gh-aw-mcpg"; + + // REST format: user.login present + let rest_item = json!({ + "number": 2320, + "title": "Auto-merge PR", + "user": {"login": "dependabot[bot]", "id": 49699333}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/github/gh-aw-mcpg" + }); + let integrity = pr_integrity(&rest_item, repo, false, None, &ctx); + assert!( + integrity.iter().any(|t| t.contains("approved")), + "Bot-authored PR (REST) should have at least approved integrity, got: {:?}", + integrity + ); + + // GraphQL format: author.login present + let graphql_item = json!({ + "number": 2320, + "title": "Auto-merge PR", + "author": {"login": "dependabot[bot]"}, + "authorAssociation": "CONTRIBUTOR" + }); + let integrity = pr_integrity(&graphql_item, repo, false, None, &ctx); + assert!( + integrity.iter().any(|t| t.contains("approved")), + "Bot-authored PR (GraphQL) should have at least approved integrity, got: {:?}", + integrity + ); + } + #[test] fn test_helpers_is_mcp_text_wrapper() { use helpers::is_mcp_text_wrapper; diff --git a/guards/github-guard/rust-guard/src/labels/response_items.rs b/guards/github-guard/rust-guard/src/labels/response_items.rs index 7eaab4b8d..5ff56fa51 100644 --- a/guards/github-guard/rust-guard/src/labels/response_items.rs +++ b/guards/github-guard/rust-guard/src/labels/response_items.rs @@ -111,7 +111,7 @@ pub fn label_response_items( if tool_name == "pull_request_read" && !method.is_empty() && method != "get" { // Fall through — use resource-level labels from tool_rules } else { - // Handle array, {items: [...]}, GraphQL nested, GraphQL single, or REST single object. + // Handle array, {items: [...]}, {pull_requests: [...]}, GraphQL nested, GraphQL single, or REST single object. // Work directly with &[Value] slices to avoid allocating a Vec<&Value>. let single_item_buf; let graphql_single_buf; @@ -119,6 +119,8 @@ pub fn label_response_items( arr.as_slice() } else if let Some(items_arr) = actual_response.get("items").and_then(|v| v.as_array()) { items_arr.as_slice() + } else if let Some(items_arr) = actual_response.get("pull_requests").and_then(|v| v.as_array()) { + items_arr.as_slice() } else if let Some(nodes) = extract_graphql_nodes(&actual_response) { nodes.as_slice() } else if let Some(obj) = extract_graphql_single_object(&actual_response) { @@ -232,7 +234,7 @@ pub fn label_response_items( if tool_name == "issue_read" && !method.is_empty() && method != "get" { // Fall through — use resource-level labels from tool_rules } else { - // Handle single issue, array of issues, GraphQL nested, or GraphQL single object + // Handle single issue, array of issues, {issues: [...]}, GraphQL nested, or GraphQL single object let all_items: Vec<&Value> = if actual_response.is_array() { actual_response .as_array() @@ -240,6 +242,8 @@ pub fn label_response_items( .unwrap_or_default() } else if let Some(items_arr) = actual_response.get("items").and_then(|v| v.as_array()) { items_arr.iter().collect() + } else if let Some(items_arr) = actual_response.get("issues").and_then(|v| v.as_array()) { + items_arr.iter().collect() } else if let Some(nodes) = extract_graphql_nodes(&actual_response) { nodes.iter().collect() } else if let Some(obj) = extract_graphql_single_object(&actual_response) { diff --git a/guards/github-guard/rust-guard/src/labels/response_paths.rs b/guards/github-guard/rust-guard/src/labels/response_paths.rs index 54a28f927..7b5cc523b 100644 --- a/guards/github-guard/rust-guard/src/labels/response_paths.rs +++ b/guards/github-guard/rust-guard/src/labels/response_paths.rs @@ -48,7 +48,13 @@ pub fn label_response_paths( match tool_name { // === Repository Search - label by private/public === "search_repositories" => { - if let Some(items) = actual_response.get("items").and_then(|v| v.as_array()) { + let items_opt = actual_response.get("items").and_then(|v| v.as_array()) + .or_else(|| actual_response.get("repositories").and_then(|v| v.as_array())); + if let Some(items) = items_opt { + // Empty search results are server metadata — let lib.rs fallback handle + if items.is_empty() && is_search_result_wrapper(&actual_response) { + return None; + } crate::log_info(&format!( "label_response_paths: search_repositories found {} items", items.len() @@ -109,6 +115,11 @@ pub fn label_response_paths( let (items, items_path) = extract_items_array(&actual_response); if let Some(items) = items { + // Empty search results are server metadata — let lib.rs handle + // them with properly-scoped writer_integrity via the metadata fallback. + if items.is_empty() && is_search_result_wrapper(&actual_response) { + return None; + } // Try tool_args first, fall back to extracting from first item let (mut arg_owner, mut arg_repo, arg_repo_full) = extract_repo_info(tool_args); // For search operations, extract repo from query when tool_args lacks owner/repo @@ -198,7 +209,7 @@ pub fn label_response_paths( integrity: if default_repo_private { writer_integrity(&default_repo, ctx) } else { - none_integrity("", ctx) + none_integrity(&default_repo, ctx) }, }), items_path: if items_path.is_empty() { @@ -225,6 +236,11 @@ pub fn label_response_paths( let (items, items_path) = extract_items_array(&actual_response); if let Some(items) = items { + // Empty search results are server metadata — let lib.rs handle + // them with properly-scoped writer_integrity via the metadata fallback. + if items.is_empty() && is_search_result_wrapper(&actual_response) { + return None; + } // Try tool_args first, fall back to extracting from first item let (mut arg_owner, mut arg_repo, arg_repo_full) = extract_repo_info(tool_args); // For search operations, extract repo from query when tool_args lacks owner/repo @@ -300,7 +316,7 @@ pub fn label_response_paths( integrity: if default_repo_private { writer_integrity(&default_repo, ctx) } else { - none_integrity("", ctx) + none_integrity(&default_repo, ctx) }, }), items_path: if items_path.is_empty() { diff --git a/guards/github-guard/rust-guard/src/lib.rs b/guards/github-guard/rust-guard/src/lib.rs index 628fd7685..a97efd118 100644 --- a/guards/github-guard/rust-guard/src/lib.rs +++ b/guards/github-guard/rust-guard/src/lib.rs @@ -855,10 +855,7 @@ pub extern "C" fn label_response( let actual_response = labels::extract_mcp_response(&input.tool_result); let is_server_metadata = labels::is_mcp_text_wrapper(&actual_response) || (labels::is_search_result_wrapper(&actual_response) - && actual_response - .get("total_count") - .and_then(|v| v.as_u64()) - == Some(0)); + && labels::search_result_total_count(&actual_response) == Some(0)); if is_server_metadata { let scope = if baseline_scope.is_empty() { From 44ee1c99ec538960b6ec7bd9c6898c7129abc641 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Mon, 23 Mar 2026 19:47:45 -0700 Subject: [PATCH 2/4] test(rust-guard): add regression tests for github/gh-aw#22533 Add 10 targeted tests that reproduce the exact scenarios from issue #22533 where search_issues returns github-actions[bot]-authored issues that get DIFC-filtered, causing Repo Assist to create duplicate monthly issues. Tests cover: - REST format search with bot-authored items (licensee/licensee scenario) - GraphQL format search with bot-authored items (v0.32.0 format) - Empty GraphQL search returns None (defers to metadata handler) - Empty REST search returns None - Empty search_pull_requests for both formats - MCP-wrapped GraphQL search results (full pipeline) - Legacy response_items fallback with GraphQL 'issues' key - Legacy response_items fallback with 'pull_requests' key - Non-empty search does not incorrectly return None - Non-bot CONTRIBUTOR items get properly-scoped labels All tests use owner_scoped_ctx("licensee") to match the exact policy configuration from the broken licensee/licensee runs, verifying that integrity tags are correctly normalized to owner scope (approved:licensee). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../github-guard/rust-guard/src/labels/mod.rs | 367 ++++++++++++++++++ 1 file changed, 367 insertions(+) diff --git a/guards/github-guard/rust-guard/src/labels/mod.rs b/guards/github-guard/rust-guard/src/labels/mod.rs index 1bd0f797e..9c782c657 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -3343,4 +3343,371 @@ mod tests { assert!(!is_mcp_text_wrapper(&serde_json::json!({"number": 42}))); assert!(!is_mcp_text_wrapper(&serde_json::json!({}))); } + + // ========================================================================= + // Issue github/gh-aw#22533: Bot-authored search results DIFC-filtered + // + // Regression between MCP server v0.31.0 → v0.32.0. + // search_issues finds bot-authored issues (github-actions[bot]) but they + // are DIFC-filtered because the guard assigns insufficient integrity. + // The agent then sees zero results and creates duplicate issues. + // + // Root causes: + // 1. MCP server v0.32.0 returns GraphQL format {"issues":[], "totalCount":N} + // in addition to REST format {"items":[], "total_count":N} + // 2. is_search_result_wrapper only detected REST format (total_count) + // 3. response_items.rs only checked "items" key, missing "issues" + // 4. Empty GraphQL results bypassed metadata handler and got unscoped tags + // 5. default_labels used none_integrity("") producing unmatchable "none:" + // ========================================================================= + + /// Reproduces the exact scenario from github/gh-aw#22533: + /// search_issues returns github-actions[bot]-authored issues in REST format + /// with an owner-scoped policy. Items must receive owner-scoped approved + /// integrity so they pass DIFC and the agent can see them. + #[test] + fn test_issue_22533_rest_search_issues_bot_author_owner_scoped() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue \"[Repo Assist] Monthly Activity 2026-03\" in:title" + }); + let response = json!({ + "total_count": 3, + "incomplete_results": false, + "items": [ + { + "number": 941, + "title": "[Repo Assist] Monthly Activity 2026-03", + "state": "open", + "user": {"login": "github-actions[bot]", "id": 41898282}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + }, + { + "number": 954, + "title": "[Repo Assist] Monthly Activity 2026-03", + "state": "open", + "user": {"login": "github-actions[bot]", "id": 41898282}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + }, + { + "number": 955, + "title": "[Repo Assist] Monthly Activity 2026-03", + "state": "open", + "user": {"login": "github-actions[bot]", "id": 41898282}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ] + }); + + let result = label_response_paths("search_issues", &tool_args, &response, &ctx) + .expect("search_issues should produce path labels"); + + assert_eq!(result.labeled_paths.len(), 3, "all 3 bot-authored issues must be labeled"); + + // Every item must have approved:licensee integrity (owner-scoped) + // so DIFC passes when agent has ["none:licensee", "unapproved:licensee", "approved:licensee"] + for (i, entry) in result.labeled_paths.iter().enumerate() { + assert!( + entry.labels.integrity.contains(&"approved:licensee".to_string()), + "item {} must have 'approved:licensee' for DIFC to pass, got: {:?}", + i, entry.labels.integrity + ); + } + } + + /// Same scenario as above but with GraphQL format that MCP server v0.32.0 + /// can return. The "issues" key (not "items") and "totalCount" (not + /// "total_count") must be recognized. + #[test] + fn test_issue_22533_graphql_search_issues_bot_author_owner_scoped() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue \"[Repo Assist] Monthly Activity\" in:title" + }); + // GraphQL format: "issues" key with "totalCount" + let response = json!({ + "totalCount": 2, + "issues": [ + { + "number": 941, + "title": "[Repo Assist] Monthly Activity 2026-03", + "state": "OPEN", + "author": {"login": "github-actions[bot]"}, + "authorAssociation": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + }, + { + "number": 954, + "title": "[Repo Assist] Monthly Activity 2026-03", + "state": "OPEN", + "author": {"login": "github-actions[bot]"}, + "authorAssociation": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ], + "pageInfo": {"hasNextPage": false, "hasPreviousPage": false} + }); + + let result = label_response_paths("search_issues", &tool_args, &response, &ctx) + .expect("search_issues with GraphQL format should produce path labels"); + + assert_eq!(result.labeled_paths.len(), 2, "both GraphQL items must be labeled"); + + for (i, entry) in result.labeled_paths.iter().enumerate() { + assert!( + entry.labels.integrity.contains(&"approved:licensee".to_string()), + "GraphQL item {} must have 'approved:licensee', got: {:?}", + i, entry.labels.integrity + ); + assert_eq!( + entry.path, + format!("/issues/{}", i), + "path should use /issues/ prefix for GraphQL format" + ); + } + } + + /// Empty GraphQL search result must NOT produce path labels. + /// It should return None so lib.rs metadata handler assigns + /// properly-scoped writer_integrity (not unscoped "none:"). + #[test] + fn test_issue_22533_empty_graphql_search_returns_none() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue \"nonexistent\" in:title" + }); + let response = json!({ + "totalCount": 0, + "issues": [], + "pageInfo": {"hasNextPage": false, "hasPreviousPage": false} + }); + + let result = label_response_paths("search_issues", &tool_args, &response, &ctx); + assert!( + result.is_none(), + "empty GraphQL search must return None (defer to metadata handler), got: {:?}", + result.map(|r| r.labeled_paths.len()) + ); + } + + /// Empty REST search result must also return None to defer to metadata handler. + #[test] + fn test_issue_22533_empty_rest_search_returns_none() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue \"nonexistent\" in:title" + }); + let response = json!({ + "total_count": 0, + "incomplete_results": false, + "items": [] + }); + + let result = label_response_paths("search_issues", &tool_args, &response, &ctx); + assert!( + result.is_none(), + "empty REST search must return None (defer to metadata handler)" + ); + } + + /// Empty search_pull_requests must also return None for both formats. + #[test] + fn test_issue_22533_empty_search_pull_requests_returns_none() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ "query": "repo:licensee/licensee is:open is:pr" }); + + // GraphQL format + let graphql = json!({ + "totalCount": 0, + "issues": [], + "pageInfo": {} + }); + assert!( + label_response_paths("search_pull_requests", &tool_args, &graphql, &ctx).is_none(), + "empty GraphQL search_pull_requests must return None" + ); + + // REST format + let rest = json!({ + "total_count": 0, + "incomplete_results": false, + "items": [] + }); + assert!( + label_response_paths("search_pull_requests", &tool_args, &rest, &ctx).is_none(), + "empty REST search_pull_requests must return None" + ); + } + + /// MCP-wrapped GraphQL search results must be correctly extracted and + /// labeled. This tests the full pipeline from MCP wrapper through + /// extract_mcp_response → extract_items_array → per-item labeling. + #[test] + fn test_issue_22533_mcp_wrapped_graphql_search_issues() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue \"[Repo Assist]\" in:title" + }); + + let inner = json!({ + "totalCount": 1, + "issues": [ + { + "number": 941, + "title": "[Repo Assist] Monthly Activity 2026-03", + "state": "OPEN", + "author": {"login": "github-actions[bot]"}, + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ], + "pageInfo": {"hasNextPage": false} + }); + let mcp_wrapped = json!({ + "content": [{"type": "text", "text": inner.to_string()}] + }); + + let result = label_response_paths("search_issues", &tool_args, &mcp_wrapped, &ctx) + .expect("MCP-wrapped GraphQL search should produce path labels"); + + assert_eq!(result.labeled_paths.len(), 1); + assert!( + result.labeled_paths[0].labels.integrity.contains(&"approved:licensee".to_string()), + "MCP-wrapped bot item must have approved:licensee, got: {:?}", + result.labeled_paths[0].labels.integrity + ); + } + + /// Verify the legacy response_items path also handles GraphQL format. + /// This is the fallback used when response_paths returns None. + #[test] + fn test_issue_22533_response_items_graphql_issues_key() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue \"[Repo Assist]\" in:title" + }); + // GraphQL format passed directly (no MCP wrapper) + let response = json!({ + "totalCount": 1, + "issues": [ + { + "number": 941, + "title": "[Repo Assist] Monthly Activity 2026-03", + "author": {"login": "github-actions[bot]"}, + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ] + }); + + let items = label_response_items("search_issues", &tool_args, &response, &ctx); + + assert_eq!(items.len(), 1, "response_items must find item in 'issues' array"); + assert!( + items[0].labels.integrity.contains(&"approved:licensee".to_string()), + "Bot-authored item from 'issues' key must have approved:licensee, got: {:?}", + items[0].labels.integrity + ); + } + + /// Verify that search_pull_requests also handles GraphQL format items + /// in the legacy response_items path. + #[test] + fn test_issue_22533_response_items_graphql_pull_requests_key() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:pr" + }); + // The PR section of response_items looks for "items" and now + // also "pull_requests" key. + let response = json!({ + "totalCount": 1, + "pull_requests": [ + { + "number": 100, + "title": "Fix typo", + "user": {"login": "dependabot[bot]"}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ] + }); + + let items = label_response_items("search_pull_requests", &tool_args, &response, &ctx); + + assert_eq!(items.len(), 1, "response_items must find item in 'pull_requests' array"); + assert!( + items[0].labels.integrity.contains(&"approved:licensee".to_string()), + "Bot-authored PR from 'pull_requests' key must have approved:licensee, got: {:?}", + items[0].labels.integrity + ); + } + + /// Non-empty search results with real items must NOT return None. + /// Only empty results should defer to the metadata handler. + #[test] + fn test_issue_22533_nonempty_search_does_not_return_none() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue" + }); + let response = json!({ + "total_count": 1, + "items": [ + { + "number": 941, + "title": "Test issue", + "user": {"login": "octocat"}, + "author_association": "NONE", + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ] + }); + + let result = label_response_paths("search_issues", &tool_args, &response, &ctx); + assert!( + result.is_some(), + "non-empty search must return Some with path labels" + ); + assert_eq!(result.unwrap().labeled_paths.len(), 1); + } + + /// search_issues with non-bot CONTRIBUTOR items in owner-scoped context. + /// These should still get properly-scoped labels even without bot elevation. + #[test] + fn test_issue_22533_non_bot_contributor_scoped_labels() { + let ctx = owner_scoped_ctx("licensee"); + let tool_args = json!({ + "query": "repo:licensee/licensee is:open is:issue" + }); + let response = json!({ + "total_count": 1, + "items": [ + { + "number": 100, + "title": "Community fix", + "user": {"login": "contributor-human"}, + "author_association": "CONTRIBUTOR", + "repository_url": "https://api.github.com/repos/licensee/licensee" + } + ] + }); + + let result = label_response_paths("search_issues", &tool_args, &response, &ctx) + .expect("should produce path labels"); + + // CONTRIBUTOR → reader_integrity → ["none:licensee", "unapproved:licensee"] + let integrity = &result.labeled_paths[0].labels.integrity; + assert!( + integrity.contains(&"unapproved:licensee".to_string()), + "CONTRIBUTOR should have unapproved:licensee, got: {:?}", + integrity + ); + assert!( + integrity.contains(&"none:licensee".to_string()), + "CONTRIBUTOR should have none:licensee, got: {:?}", + integrity + ); + } } From 51cfbacd18e4f9102ccd21cb737173e8bb05ef05 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 03:07:51 +0000 Subject: [PATCH 3/4] Initial plan From bfb3ca7db0f44838af201927d135fb5e025d4819 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Mar 2026 03:20:37 +0000 Subject: [PATCH 4/4] fix(rust-guard): use correct JSON pointer key for search_repositories response Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/5dec785d-967e-45f7-ab2c-878e153c5247 --- .../github-guard/rust-guard/src/labels/mod.rs | 69 +++++++++++++++++++ .../rust-guard/src/labels/response_paths.rs | 14 ++-- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/mod.rs b/guards/github-guard/rust-guard/src/labels/mod.rs index 9c782c657..8b9adc260 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -3710,4 +3710,73 @@ mod tests { integrity ); } + + /// search_repositories with `repositories` key (GraphQL-style) must use + /// `/repositories/{i}` paths, not `/items/{i}`. + #[test] + fn test_search_repositories_repositories_key_uses_correct_paths() { + let ctx = default_ctx(); + let tool_args = json!({}); + let response = json!({ + "totalCount": 2, + "repositories": [ + { + "full_name": "owner/public-repo", + "private": false + }, + { + "full_name": "owner/private-repo", + "private": true + } + ] + }); + + let result = label_response_paths("search_repositories", &tool_args, &response, &ctx) + .expect("search_repositories with repositories key should produce path labels"); + + assert_eq!(result.labeled_paths.len(), 2, "both items must be labeled"); + assert_eq!( + result.items_path, + Some("/repositories".to_string()), + "items_path must reflect the actual key used" + ); + assert_eq!( + result.labeled_paths[0].path, "/repositories/0", + "first item path must use /repositories/ prefix" + ); + assert_eq!( + result.labeled_paths[1].path, "/repositories/1", + "second item path must use /repositories/ prefix" + ); + } + + /// search_repositories with `items` key (REST format) must use `/items/{i}` paths. + #[test] + fn test_search_repositories_items_key_uses_correct_paths() { + let ctx = default_ctx(); + let tool_args = json!({}); + let response = json!({ + "total_count": 1, + "items": [ + { + "full_name": "owner/public-repo", + "private": false + } + ] + }); + + let result = label_response_paths("search_repositories", &tool_args, &response, &ctx) + .expect("search_repositories with items key should produce path labels"); + + assert_eq!(result.labeled_paths.len(), 1); + assert_eq!( + result.items_path, + Some("/items".to_string()), + "items_path must be /items for REST format" + ); + assert_eq!( + result.labeled_paths[0].path, "/items/0", + "item path must use /items/ prefix for REST format" + ); + } } diff --git a/guards/github-guard/rust-guard/src/labels/response_paths.rs b/guards/github-guard/rust-guard/src/labels/response_paths.rs index 7b5cc523b..0771cd44b 100644 --- a/guards/github-guard/rust-guard/src/labels/response_paths.rs +++ b/guards/github-guard/rust-guard/src/labels/response_paths.rs @@ -48,8 +48,14 @@ pub fn label_response_paths( match tool_name { // === Repository Search - label by private/public === "search_repositories" => { - let items_opt = actual_response.get("items").and_then(|v| v.as_array()) - .or_else(|| actual_response.get("repositories").and_then(|v| v.as_array())); + let (items_opt, items_key) = + if let Some(arr) = actual_response.get("items").and_then(|v| v.as_array()) { + (Some(arr), "items") + } else if let Some(arr) = actual_response.get("repositories").and_then(|v| v.as_array()) { + (Some(arr), "repositories") + } else { + (None, "items") + }; if let Some(items) = items_opt { // Empty search results are server metadata — let lib.rs fallback handle if items.is_empty() && is_search_result_wrapper(&actual_response) { @@ -79,7 +85,7 @@ pub fn label_response_paths( }; labeled_paths.push(PathLabelEntry { - path: format!("/items/{}", i), + path: format!("/{}/{}", items_key, i), labels: crate::ResourceLabels { description: format!("repo:{}", full_name), secrecy, @@ -95,7 +101,7 @@ pub fn label_response_paths( secrecy: vec![], integrity: none_integrity("", ctx), }), - items_path: Some("/items".to_string()), + items_path: Some(format!("/{}", items_key)), }); } }