From 75a8f2a1fbd373bba8309dd7103b36cdfabce412 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 02:53:37 +0000 Subject: [PATCH 1/3] Initial plan From d43d08b09dfaace3ccf5292da8bb19b357dfde46 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 03:04:14 +0000 Subject: [PATCH 2/3] Add pin_issue, unpin_issue (write ops) and block transfer_repository in GitHub guard Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/2b4d0d1c-031e-4164-bfd6-507ba2f3429e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../github-guard/rust-guard/src/labels/mod.rs | 94 +++++++++++++++++++ .../rust-guard/src/labels/tool_rules.rs | 25 +++++ guards/github-guard/rust-guard/src/lib.rs | 61 +++++++++++- guards/github-guard/rust-guard/src/tools.rs | 61 ++++++++++++ 4 files changed, 239 insertions(+), 2 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/mod.rs b/guards/github-guard/rust-guard/src/labels/mod.rs index 06220222..8d9ad1a2 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -4141,4 +4141,98 @@ mod tests { writer_integrity(repo, &ctx) ); } + + // ========================================================================= + // pin_issue / unpin_issue tests + // ========================================================================= + + #[test] + fn test_apply_tool_labels_pin_issue_writer_integrity() { + let ctx = default_ctx(); + let repo_id = "github/copilot"; + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "issue_number": 42 + }); + + let (_secrecy, integrity, desc) = apply_tool_labels( + "pin_issue", + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + + assert_eq!( + integrity, + writer_integrity(repo_id, &ctx), + "pin_issue should require writer-level integrity" + ); + assert_eq!(desc, "issue:github/copilot#42"); + } + + #[test] + fn test_apply_tool_labels_unpin_issue_writer_integrity() { + let ctx = default_ctx(); + let repo_id = "github/copilot"; + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "issue_number": 7 + }); + + let (_secrecy, integrity, desc) = apply_tool_labels( + "unpin_issue", + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + + assert_eq!( + integrity, + writer_integrity(repo_id, &ctx), + "unpin_issue should require writer-level integrity" + ); + assert_eq!(desc, "issue:github/copilot#7"); + } + + // ========================================================================= + // transfer_repository tests + // ========================================================================= + + #[test] + fn test_apply_tool_labels_transfer_repository_secrecy_inherits_repo_visibility() { + // apply_tool_labels sets the correct secrecy for transfer_repository. + // The blocked_integrity enforcement happens in label_resource (lib.rs) via + // is_blocked_tool(), not in apply_tool_labels, because ensure_integrity_baseline + // would otherwise raise blocked-level tags to none-level. + let ctx = default_ctx(); + let repo_id = "github/copilot"; + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "new_owner": "other-org" + }); + + let (secrecy, _integrity, _desc) = apply_tool_labels( + "transfer_repository", + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + + // Secrecy should be scoped to the repository (not empty public secrecy) + // The test uses cfg(test) backend which may keep current_secrecy unchanged; + // we just verify the call completes and does not panic. + let _ = secrecy; + } } diff --git a/guards/github-guard/rust-guard/src/labels/tool_rules.rs b/guards/github-guard/rust-guard/src/labels/tool_rules.rs index 2065f662..aed07eb9 100644 --- a/guards/github-guard/rust-guard/src/labels/tool_rules.rs +++ b/guards/github-guard/rust-guard/src/labels/tool_rules.rs @@ -146,6 +146,31 @@ pub fn apply_tool_labels( } } + // === Issue Pin/Unpin (repo-scoped write) === + "pin_issue" | "unpin_issue" => { + // Pinning/unpinning an issue is a repo-level cosmetic write operation. + // S = S(repo) — inherits from repository visibility + // I = writer (requires repo write access to change issue pin state) + if !owner.is_empty() && !repo.is_empty() { + if let Some(issue_num) = + extract_number_as_string(tool_args, field_names::ISSUE_NUMBER) + { + desc = format!("issue:{}/{}#{}", owner, repo, issue_num); + } + } + secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); + integrity = writer_integrity(repo_id, ctx); + } + + // === Repository Transfer (blocked: irreversible ownership change) === + "transfer_repository" => { + // Repository transfers are irreversible and cannot be allowed by agents. + // Blocking is enforced in label_resource via is_blocked_tool(); this arm + // applies repo-visibility secrecy so the resource is at least correctly + // classified before the integrity override happens in label_resource. + secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); + } + // Search issues: extract repo scope from query or tool_args when available "search_issues" => { let (s_owner, s_repo, s_repo_id) = resolve_search_scope(tool_args, &owner, &repo); diff --git a/guards/github-guard/rust-guard/src/lib.rs b/guards/github-guard/rust-guard/src/lib.rs index d6807401..a37f4f51 100644 --- a/guards/github-guard/rust-guard/src/lib.rs +++ b/guards/github-guard/rust-guard/src/lib.rs @@ -15,8 +15,8 @@ mod tools; use labels::constants::policy_integrity; use labels::{ - extract_repo_info, extract_repo_info_from_search_query, MinIntegrity, PolicyContext, - PolicyScopeEntry, ScopeKind, + blocked_integrity, extract_repo_info, extract_repo_info_from_search_query, MinIntegrity, + PolicyContext, PolicyScopeEntry, ScopeKind, }; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -670,6 +670,20 @@ pub extern "C" fn label_resource( let baseline_scope = infer_scope_for_baseline(&input.tool_name, &input.tool_args, &repo_id); let final_integrity = labels::ensure_integrity_baseline(&baseline_scope, final_integrity, &ctx); + // Unconditionally blocked tools: override integrity to blocked_integrity so the + // DIFC evaluator always denies them. This must happen after ensure_integrity_baseline + // because that helper would otherwise raise blocked-level tags to none-level. + let final_integrity = if tools::is_blocked_tool(&input.tool_name) { + log_info(&format!( + " tool '{}' is unconditionally blocked — overriding integrity to blocked", + input.tool_name + )); + let scope = if repo_id.is_empty() { "global" } else { &repo_id }; + blocked_integrity(scope, &ctx) + } else { + final_integrity + }; + // Log computed labels log_info(&format!(" desc={}", final_desc)); if final_secrecy.is_empty() { @@ -1131,4 +1145,47 @@ mod tests { let inferred = infer_scope_for_baseline("search_pull_requests", &tool_args, ""); assert_eq!(inferred, "github/gh-aw-mcpg"); } + + #[test] + fn transfer_repository_integrity_is_blocked_after_ensure_baseline() { + // Verify that the is_blocked_tool + blocked_integrity override logic produces + // a "blocked:" tag, proving that ensure_integrity_baseline cannot raise it + // back to "none:". + let ctx = PolicyContext::default(); + let repo_id = "github/copilot"; + + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "new_owner": "other-org" + }); + + let (_, integrity, _) = labels::apply_tool_labels( + "transfer_repository", + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + let baseline_scope = + infer_scope_for_baseline("transfer_repository", &tool_args, repo_id); + let after_baseline = labels::ensure_integrity_baseline(&baseline_scope, integrity, &ctx); + + // Simulate the is_blocked_tool override performed in label_resource + let final_integrity = if tools::is_blocked_tool("transfer_repository") { + let scope = if repo_id.is_empty() { "global" } else { repo_id }; + blocked_integrity(scope, &ctx) + } else { + after_baseline + }; + + assert!( + final_integrity.iter().any(|t| t.contains("blocked")), + "transfer_repository must have blocked integrity after label_resource override; \ + got: {:?}", + final_integrity + ); + } } diff --git a/guards/github-guard/rust-guard/src/tools.rs b/guards/github-guard/rust-guard/src/tools.rs index 3ca639a7..89f4ae3f 100644 --- a/guards/github-guard/rust-guard/src/tools.rs +++ b/guards/github-guard/rust-guard/src/tools.rs @@ -40,6 +40,9 @@ pub const WRITE_OPERATIONS: &[&str] = &[ // Pre-emptive entries for anticipated future MCP tools (no equivalent tool today) "archive_repository", // gh repo archive "transfer_issue", // gh issue transfer + "transfer_repository", // gh repo transfer — blocked: repo ownership transfer is irreversible + "pin_issue", // gh issue pin + "unpin_issue", // gh issue unpin "enable_workflow", // gh workflow enable "disable_workflow", // gh workflow disable "set_secret", // gh secret set @@ -103,3 +106,61 @@ pub fn is_lock_operation(tool_name: &str) -> bool { pub fn is_unlock_operation(tool_name: &str) -> bool { tool_name.starts_with("unlock_") } + +/// Check if a tool is unconditionally blocked (always denied regardless of agent integrity). +/// +/// Blocked tools are listed here when the operation is considered too dangerous +/// to ever permit via an agent, even if the agent would otherwise satisfy the +/// integrity requirements for a normal write operation. +/// +/// Current entries: +/// - `transfer_repository`: repository ownership transfer is irreversible and +/// must never be performed by an automated agent. +pub fn is_blocked_tool(tool_name: &str) -> bool { + matches!(tool_name, "transfer_repository") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_is_blocked_tool_transfer_repository() { + assert!( + is_blocked_tool("transfer_repository"), + "transfer_repository must be unconditionally blocked" + ); + } + + #[test] + fn test_is_blocked_tool_other_write_ops_not_blocked() { + // Regular write operations should not be blocked + for op in &["create_issue", "add_issue_comment", "pin_issue", "unpin_issue"] { + assert!( + !is_blocked_tool(op), + "{} should not be blocked", + op + ); + } + } + + #[test] + fn test_transfer_repository_is_write_operation() { + assert!( + is_write_operation("transfer_repository"), + "transfer_repository must be classified as a write operation" + ); + } + + #[test] + fn test_pin_unpin_issue_are_write_operations() { + assert!( + is_write_operation("pin_issue"), + "pin_issue must be classified as a write operation" + ); + assert!( + is_write_operation("unpin_issue"), + "unpin_issue must be classified as a write operation" + ); + } +} From 71d9ef0201dc98ccb4f3e7cc88f7e3b933ca844c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 29 Mar 2026 03:06:48 +0000 Subject: [PATCH 3/3] Improve transfer_repository test to assert expected secrecy behavior Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/2b4d0d1c-031e-4164-bfd6-507ba2f3429e Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- guards/github-guard/rust-guard/src/labels/mod.rs | 14 ++++++++++---- 1 file changed, 10 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 8d9ad1a2..111246d2 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -4230,9 +4230,15 @@ mod tests { &ctx, ); - // Secrecy should be scoped to the repository (not empty public secrecy) - // The test uses cfg(test) backend which may keep current_secrecy unchanged; - // we just verify the call completes and does not panic. - let _ = secrecy; + // In the test environment the backend host is unavailable, so + // apply_repo_visibility_secrecy returns current_secrecy unchanged + // (the cfg(test) fallback path). An empty initial secrecy therefore + // remains empty. What matters is that the call completes without + // panicking and returns no unexpected secrecy labels. + assert!( + secrecy.is_empty(), + "transfer_repository secrecy should be empty in test env (no backend); got: {:?}", + secrecy + ); } }