From 1e7df6f722968ec978616cb641deaeb3e833a624 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 14:01:25 +0000 Subject: [PATCH 1/5] Initial plan From e9716ecb627e66651db81612024922dc05a6ba74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 15 Apr 2026 14:07:36 +0000 Subject: [PATCH 2/5] Add 21 missing granular MCP tools to guard classifications Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/10007b00-6bd5-4623-86ca-e8505618f53c Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> --- .../rust-guard/src/labels/tool_rules.rs | 47 +++++++++ guards/github-guard/rust-guard/src/tools.rs | 99 +++++++++++++++++++ 2 files changed, 146 insertions(+) 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 a0c756e7..69cebc30 100644 --- a/guards/github-guard/rust-guard/src/labels/tool_rules.rs +++ b/guards/github-guard/rust-guard/src/labels/tool_rules.rs @@ -573,6 +573,53 @@ pub fn apply_tool_labels( integrity = writer_integrity(repo_id, ctx); } + // === Granular issue update tools (repo-scoped writes) === + "update_issue_assignees" + | "update_issue_body" + | "update_issue_labels" + | "update_issue_milestone" + | "update_issue_state" + | "update_issue_title" + | "update_issue_type" => { + // Granular PATCH tools that modify individual issue fields. + // S = S(repo); I = writer + secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); + integrity = writer_integrity(repo_id, ctx); + } + + // === Sub-issue management tools (repo-scoped writes) === + "add_sub_issue" | "remove_sub_issue" | "reprioritize_sub_issue" => { + // Sub-issue link creation, removal, and reordering. + // S = S(repo); I = writer + secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); + integrity = writer_integrity(repo_id, ctx); + } + + // === Granular PR update tools (repo-scoped read-write) === + "update_pull_request_body" + | "update_pull_request_draft_state" + | "update_pull_request_state" + | "update_pull_request_title" => { + // Granular PATCH tools that modify individual PR fields. + // S = S(repo); I = writer + secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); + integrity = writer_integrity(repo_id, ctx); + } + + // === PR review tools (repo-scoped writes) === + "add_pull_request_review_comment" + | "create_pull_request_review" + | "delete_pending_pull_request_review" + | "request_pull_request_reviewers" + | "resolve_review_thread" + | "submit_pending_pull_request_review" + | "unresolve_review_thread" => { + // PR review creation, commenting, submission, and thread resolution. + // S = S(repo); I = writer + secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); + integrity = writer_integrity(repo_id, ctx); + } + // === Repo content and structure write operations === "create_or_update_file" | "push_files" | "delete_file" | "create_branch" | "update_pull_request_branch" | "create_repository" | "fork_repository" => { diff --git a/guards/github-guard/rust-guard/src/tools.rs b/guards/github-guard/rust-guard/src/tools.rs index 8dcd7859..d2b20a4d 100644 --- a/guards/github-guard/rust-guard/src/tools.rs +++ b/guards/github-guard/rust-guard/src/tools.rs @@ -73,6 +73,29 @@ pub const WRITE_OPERATIONS: &[&str] = &[ "delete_release", // DELETE /repos/.../releases/{id} // Pre-emptive: gist deletion (gh gist delete) "delete_gist", // DELETE /gists/{gist_id} + + // Granular issue update tools (alongside issue_write composite) + "update_issue_assignees", // PATCH /repos/.../issues/{number} — assignees + "update_issue_body", // PATCH /repos/.../issues/{number} — body text + "update_issue_labels", // PATCH /repos/.../issues/{number} — labels + "update_issue_milestone", // PATCH /repos/.../issues/{number} — milestone + "update_issue_state", // PATCH /repos/.../issues/{number} — open/closed + "update_issue_title", // PATCH /repos/.../issues/{number} — title + "update_issue_type", // PATCH /repos/.../issues/{number} — type + + // Sub-issue management tools (alongside sub_issue_write composite) + "add_sub_issue", // POST /repos/.../issues/{number}/sub_issues + "remove_sub_issue", // DELETE/POST — remove sub-issue link + "reprioritize_sub_issue", // PATCH — reorder sub-issues + + // PR review tools (alongside pull_request_review_write composite) + "add_pull_request_review_comment", // POST /repos/.../pulls/{number}/comments + "create_pull_request_review", // POST /repos/.../pulls/{number}/reviews + "delete_pending_pull_request_review", // DELETE /repos/.../pulls/{number}/reviews/{id} + "request_pull_request_reviewers", // POST /repos/.../pulls/{number}/requested_reviewers + "resolve_review_thread", // PUT /graphql — resolveReviewThread + "submit_pending_pull_request_review", // POST /repos/.../pulls/{number}/reviews/{id}/events + "unresolve_review_thread", // PUT /graphql — unresolveReviewThread ]; /// Read-write operations that both read and modify data @@ -89,6 +112,12 @@ pub const READ_WRITE_OPERATIONS: &[&str] = &[ "create_agent_task", // Deprecated alias coverage "update_project_item", // deprecated alias for projects_write (updateProjectV2ItemFieldValue) + + // Granular PR update tools (alongside update_pull_request composite) + "update_pull_request_body", // PATCH — modifies PR body + "update_pull_request_draft_state", // PATCH — converts to/from draft + "update_pull_request_state", // PATCH — opens or closes a PR + "update_pull_request_title", // PATCH — modifies PR title ]; /// Check if a tool is a write operation @@ -307,4 +336,74 @@ mod tests { ); } } + + #[test] + fn test_granular_issue_update_tools_are_write_operations() { + for op in &[ + "update_issue_assignees", + "update_issue_body", + "update_issue_labels", + "update_issue_milestone", + "update_issue_state", + "update_issue_title", + "update_issue_type", + ] { + assert!( + is_write_operation(op), + "{} must be classified as a write operation", + op + ); + } + } + + #[test] + fn test_sub_issue_management_tools_are_write_operations() { + for op in &["add_sub_issue", "remove_sub_issue", "reprioritize_sub_issue"] { + assert!( + is_write_operation(op), + "{} must be classified as a write operation", + op + ); + } + } + + #[test] + fn test_pr_review_tools_are_write_operations() { + for op in &[ + "add_pull_request_review_comment", + "create_pull_request_review", + "delete_pending_pull_request_review", + "request_pull_request_reviewers", + "resolve_review_thread", + "submit_pending_pull_request_review", + "unresolve_review_thread", + ] { + assert!( + is_write_operation(op), + "{} must be classified as a write operation", + op + ); + } + } + + #[test] + fn test_granular_pr_update_tools_are_read_write_operations() { + for op in &[ + "update_pull_request_body", + "update_pull_request_draft_state", + "update_pull_request_state", + "update_pull_request_title", + ] { + assert!( + is_read_write_operation(op), + "{} must be classified as a read-write operation", + op + ); + assert!( + !is_write_operation(op), + "{} should not be in WRITE_OPERATIONS (it is in READ_WRITE_OPERATIONS)", + op + ); + } + } } From 08f8a8fd21236b7e39f20dc59f640288bb703b18 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 15 Apr 2026 07:48:49 -0700 Subject: [PATCH 3/5] Update guards/github-guard/rust-guard/src/tools.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- guards/github-guard/rust-guard/src/tools.rs | 9 --------- 1 file changed, 9 deletions(-) diff --git a/guards/github-guard/rust-guard/src/tools.rs b/guards/github-guard/rust-guard/src/tools.rs index d2b20a4d..7f788f2c 100644 --- a/guards/github-guard/rust-guard/src/tools.rs +++ b/guards/github-guard/rust-guard/src/tools.rs @@ -74,15 +74,6 @@ pub const WRITE_OPERATIONS: &[&str] = &[ // Pre-emptive: gist deletion (gh gist delete) "delete_gist", // DELETE /gists/{gist_id} - // Granular issue update tools (alongside issue_write composite) - "update_issue_assignees", // PATCH /repos/.../issues/{number} — assignees - "update_issue_body", // PATCH /repos/.../issues/{number} — body text - "update_issue_labels", // PATCH /repos/.../issues/{number} — labels - "update_issue_milestone", // PATCH /repos/.../issues/{number} — milestone - "update_issue_state", // PATCH /repos/.../issues/{number} — open/closed - "update_issue_title", // PATCH /repos/.../issues/{number} — title - "update_issue_type", // PATCH /repos/.../issues/{number} — type - // Sub-issue management tools (alongside sub_issue_write composite) "add_sub_issue", // POST /repos/.../issues/{number}/sub_issues "remove_sub_issue", // DELETE/POST — remove sub-issue link From 7e8fc46442bb6f97e7528f8ea8a9866175d0ddc9 Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 15 Apr 2026 07:57:22 -0700 Subject: [PATCH 4/5] fix: add granular issue update tools to READ_WRITE_OPERATIONS The test expected update_issue_assignees etc. to be write operations, but the tools were never added to any classification list. Following the same pattern as granular PR update tools (which are in READ_WRITE_OPERATIONS alongside update_pull_request), add the granular issue update tools to READ_WRITE_OPERATIONS alongside issue_write. Fix the test to check is_read_write_operation() consistently with the PR update tools test pattern. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- guards/github-guard/rust-guard/src/tools.rs | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/guards/github-guard/rust-guard/src/tools.rs b/guards/github-guard/rust-guard/src/tools.rs index 7f788f2c..9adcaaaf 100644 --- a/guards/github-guard/rust-guard/src/tools.rs +++ b/guards/github-guard/rust-guard/src/tools.rs @@ -104,6 +104,15 @@ pub const READ_WRITE_OPERATIONS: &[&str] = &[ // Deprecated alias coverage "update_project_item", // deprecated alias for projects_write (updateProjectV2ItemFieldValue) + // Granular issue update tools (alongside issue_write composite) + "update_issue_assignees", // PATCH — modifies issue assignees + "update_issue_body", // PATCH — modifies issue body + "update_issue_labels", // PATCH — modifies issue labels + "update_issue_milestone", // PATCH — modifies issue milestone + "update_issue_state", // PATCH — opens or closes an issue + "update_issue_title", // PATCH — modifies issue title + "update_issue_type", // PATCH — modifies issue type + // Granular PR update tools (alongside update_pull_request composite) "update_pull_request_body", // PATCH — modifies PR body "update_pull_request_draft_state", // PATCH — converts to/from draft @@ -329,7 +338,7 @@ mod tests { } #[test] - fn test_granular_issue_update_tools_are_write_operations() { + fn test_granular_issue_update_tools_are_read_write_operations() { for op in &[ "update_issue_assignees", "update_issue_body", @@ -340,8 +349,13 @@ mod tests { "update_issue_type", ] { assert!( - is_write_operation(op), - "{} must be classified as a write operation", + is_read_write_operation(op), + "{} must be classified as a read-write operation", + op + ); + assert!( + !is_write_operation(op), + "{} should not be in WRITE_OPERATIONS (it is in READ_WRITE_OPERATIONS)", op ); } From dbc0123674812a553fc4556551679bda7b3e1e7a Mon Sep 17 00:00:00 2001 From: Landon Cox Date: Wed, 15 Apr 2026 08:11:42 -0700 Subject: [PATCH 5/5] fix: move sub-issue and PR review tools to READ_WRITE_OPERATIONS Address review feedback: granular sub-issue management tools and PR review tools are decomposed versions of composites already classified as read-write (sub_issue_write, pull_request_review_write). Keeping them as pure write would skip read-side secrecy checks and LabelResponse(), allowing private repo content to bypass enforcement. Changes: - Move add_sub_issue, remove_sub_issue, reprioritize_sub_issue from WRITE_OPERATIONS to READ_WRITE_OPERATIONS - Move 7 PR review tools (add_pull_request_review_comment, etc.) from WRITE_OPERATIONS to READ_WRITE_OPERATIONS - Update tests to check is_read_write_operation() consistently - Add apply_tool_labels tests for all 3 new tool groups (issue update, sub-issue management, PR review) asserting writer integrity Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../github-guard/rust-guard/src/labels/mod.rs | 96 +++++++++++++++++++ guards/github-guard/rust-guard/src/tools.rs | 49 ++++++---- 2 files changed, 126 insertions(+), 19 deletions(-) diff --git a/guards/github-guard/rust-guard/src/labels/mod.rs b/guards/github-guard/rust-guard/src/labels/mod.rs index 04724b49..d7bb8dcc 100644 --- a/guards/github-guard/rust-guard/src/labels/mod.rs +++ b/guards/github-guard/rust-guard/src/labels/mod.rs @@ -4735,6 +4735,102 @@ mod tests { assert_eq!(integrity, writer_integrity(repo_id, &ctx), "sub_issue_write should have writer integrity"); } + #[test] + fn test_apply_tool_labels_granular_issue_update_writer_integrity() { + let ctx = default_ctx(); + let repo_id = "github/copilot"; + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "issue_number": 1 + }); + + for tool in &[ + "update_issue_assignees", + "update_issue_body", + "update_issue_labels", + "update_issue_milestone", + "update_issue_state", + "update_issue_title", + "update_issue_type", + ] { + let (secrecy, integrity, _desc) = apply_tool_labels( + tool, + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + + // Repo-scoped secrecy (public repo in default_ctx → empty) + assert_eq!(secrecy, vec![] as Vec, "{tool} secrecy mismatch"); + assert_eq!(integrity, writer_integrity(repo_id, &ctx), "{tool} should have writer integrity"); + } + } + + #[test] + fn test_apply_tool_labels_granular_sub_issue_tools_writer_integrity() { + let ctx = default_ctx(); + let repo_id = "github/copilot"; + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "issue_number": 1, + "sub_issue_id": 2 + }); + + for tool in &["add_sub_issue", "remove_sub_issue", "reprioritize_sub_issue"] { + let (secrecy, integrity, _desc) = apply_tool_labels( + tool, + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + + assert_eq!(secrecy, vec![] as Vec, "{tool} secrecy mismatch"); + assert_eq!(integrity, writer_integrity(repo_id, &ctx), "{tool} should have writer integrity"); + } + } + + #[test] + fn test_apply_tool_labels_granular_pr_review_tools_writer_integrity() { + let ctx = default_ctx(); + let repo_id = "github/copilot"; + let tool_args = json!({ + "owner": "github", + "repo": "copilot", + "pullNumber": 42 + }); + + for tool in &[ + "add_pull_request_review_comment", + "create_pull_request_review", + "delete_pending_pull_request_review", + "request_pull_request_reviewers", + "resolve_review_thread", + "submit_pending_pull_request_review", + "unresolve_review_thread", + ] { + let (secrecy, integrity, _desc) = apply_tool_labels( + tool, + &tool_args, + repo_id, + vec![], + vec![], + String::new(), + &ctx, + ); + + assert_eq!(secrecy, vec![] as Vec, "{tool} secrecy mismatch"); + assert_eq!(integrity, writer_integrity(repo_id, &ctx), "{tool} should have writer integrity"); + } + } + #[test] fn test_apply_tool_labels_fork_repository_writer_integrity() { let ctx = default_ctx(); diff --git a/guards/github-guard/rust-guard/src/tools.rs b/guards/github-guard/rust-guard/src/tools.rs index 9adcaaaf..375a6673 100644 --- a/guards/github-guard/rust-guard/src/tools.rs +++ b/guards/github-guard/rust-guard/src/tools.rs @@ -74,19 +74,6 @@ pub const WRITE_OPERATIONS: &[&str] = &[ // Pre-emptive: gist deletion (gh gist delete) "delete_gist", // DELETE /gists/{gist_id} - // Sub-issue management tools (alongside sub_issue_write composite) - "add_sub_issue", // POST /repos/.../issues/{number}/sub_issues - "remove_sub_issue", // DELETE/POST — remove sub-issue link - "reprioritize_sub_issue", // PATCH — reorder sub-issues - - // PR review tools (alongside pull_request_review_write composite) - "add_pull_request_review_comment", // POST /repos/.../pulls/{number}/comments - "create_pull_request_review", // POST /repos/.../pulls/{number}/reviews - "delete_pending_pull_request_review", // DELETE /repos/.../pulls/{number}/reviews/{id} - "request_pull_request_reviewers", // POST /repos/.../pulls/{number}/requested_reviewers - "resolve_review_thread", // PUT /graphql — resolveReviewThread - "submit_pending_pull_request_review", // POST /repos/.../pulls/{number}/reviews/{id}/events - "unresolve_review_thread", // PUT /graphql — unresolveReviewThread ]; /// Read-write operations that both read and modify data @@ -113,6 +100,20 @@ pub const READ_WRITE_OPERATIONS: &[&str] = &[ "update_issue_title", // PATCH — modifies issue title "update_issue_type", // PATCH — modifies issue type + // Sub-issue management tools (alongside sub_issue_write composite) + "add_sub_issue", // POST /repos/.../issues/{number}/sub_issues + "remove_sub_issue", // DELETE/POST — remove sub-issue link + "reprioritize_sub_issue", // PATCH — reorder sub-issues + + // PR review tools (alongside pull_request_review_write composite) + "add_pull_request_review_comment", // POST /repos/.../pulls/{number}/comments + "create_pull_request_review", // POST /repos/.../pulls/{number}/reviews + "delete_pending_pull_request_review", // DELETE /repos/.../pulls/{number}/reviews/{id} + "request_pull_request_reviewers", // POST /repos/.../pulls/{number}/requested_reviewers + "resolve_review_thread", // PUT /graphql — resolveReviewThread + "submit_pending_pull_request_review", // POST /repos/.../pulls/{number}/reviews/{id}/events + "unresolve_review_thread", // PUT /graphql — unresolveReviewThread + // Granular PR update tools (alongside update_pull_request composite) "update_pull_request_body", // PATCH — modifies PR body "update_pull_request_draft_state", // PATCH — converts to/from draft @@ -362,18 +363,23 @@ mod tests { } #[test] - fn test_sub_issue_management_tools_are_write_operations() { + fn test_sub_issue_management_tools_are_read_write_operations() { for op in &["add_sub_issue", "remove_sub_issue", "reprioritize_sub_issue"] { assert!( - is_write_operation(op), - "{} must be classified as a write operation", + is_read_write_operation(op), + "{} must be classified as a read-write operation", + op + ); + assert!( + !is_write_operation(op), + "{} should not be in WRITE_OPERATIONS (it is in READ_WRITE_OPERATIONS)", op ); } } #[test] - fn test_pr_review_tools_are_write_operations() { + fn test_pr_review_tools_are_read_write_operations() { for op in &[ "add_pull_request_review_comment", "create_pull_request_review", @@ -384,8 +390,13 @@ mod tests { "unresolve_review_thread", ] { assert!( - is_write_operation(op), - "{} must be classified as a write operation", + is_read_write_operation(op), + "{} must be classified as a read-write operation", + op + ); + assert!( + !is_write_operation(op), + "{} should not be in WRITE_OPERATIONS (it is in READ_WRITE_OPERATIONS)", op ); }