From b2791beb9ccf7c121522efac23cecb2d50e98f90 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 15 Jul 2026 13:14:12 +0000 Subject: [PATCH] fix: handle bare-string labels from MCP tool path in guard The issue_read MCP tool returns labels as bare strings (e.g. ["approved"]), while the REST/proxy path returns label objects (e.g. [{"name":"approved"}]). extract_github_label_names only handled the object shape, so approval-labels and refusal-labels were silently ignored for items read via MCP tools. Fix: try as_str() first (bare string), then fall back to get("name") (object shape). Both approval-label promotion and refusal-label demotion are now handled correctly on both code paths. Closes #9382 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../rust-guard/src/labels/helpers.rs | 45 ++++++++++++++++++- 1 file changed, 43 insertions(+), 2 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/helpers.rs b/guards/github-guard/rust-guard/src/labels/helpers.rs index 685b878d7..cb6d7c62a 100644 --- a/guards/github-guard/rust-guard/src/labels/helpers.rs +++ b/guards/github-guard/rust-guard/src/labels/helpers.rs @@ -391,13 +391,19 @@ pub(crate) fn is_blocked_user(username: &str, ctx: &PolicyContext) -> bool { /// Extract GitHub label names from a content item's `labels` array. /// -/// Returns the `name` field from each element of the item's `labels` array. +/// Handles two shapes emitted by different GitHub API paths: +/// - REST/proxy shape: `[{"name": "x", ...}]` — extract the `name` field. +/// - MCP tool shape: `["x"]` — the element itself is the string. fn extract_github_label_names(item: &Value) -> Vec<&str> { item.get("labels") .and_then(|v| v.as_array()) .map(|arr| { arr.iter() - .filter_map(|label| label.get("name").and_then(|v| v.as_str())) + .filter_map(|label| { + label + .as_str() + .or_else(|| label.get("name").and_then(|v| v.as_str())) + }) .collect() }) .unwrap_or_default() @@ -3794,6 +3800,41 @@ mod tests { assert!(!has_approval_label(&item, &ctx)); } + // ----------------------------------------------------------------------- + // Bare-string label shape (MCP tool path, issue #9382) + // ----------------------------------------------------------------------- + + #[test] + fn test_has_approval_label_matches_bare_string_label() { + // MCP issue_read returns labels as bare strings, not objects. + let ctx = PolicyContext { + approval_labels: vec!["approved".to_string()], + ..Default::default() + }; + let item = serde_json::json!({"labels": ["approved"]}); + assert!(has_approval_label(&item, &ctx)); + } + + #[test] + fn test_has_approval_label_bare_string_case_insensitive() { + let ctx = PolicyContext { + approval_labels: vec!["approved".to_string()], + ..Default::default() + }; + let item = serde_json::json!({"labels": ["APPROVED"]}); + assert!(has_approval_label(&item, &ctx)); + } + + #[test] + fn test_has_refusal_label_matches_bare_string_label() { + let ctx = PolicyContext { + refusal_labels: vec!["spam".to_string()], + ..Default::default() + }; + let item = serde_json::json!({"labels": ["SPAM"]}); + assert!(has_refusal_label(&item, &ctx)); + } + #[test] fn test_has_refusal_label_matches_case_insensitively() { let ctx = PolicyContext {