Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
96 changes: 96 additions & 0 deletions guards/github-guard/rust-guard/src/labels/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<String>, "{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<String>, "{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<String>, "{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();
Expand Down
47 changes: 47 additions & 0 deletions guards/github-guard/rust-guard/src/labels/tool_rules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Comment on lines +576 to +621
Copy link

Copilot AI Apr 15, 2026

Choose a reason for hiding this comment

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

New tool-specific labeling logic was added here, but there are no corresponding unit tests asserting the expected secrecy/integrity outputs for these tool names. There are existing apply_tool_labels tests for similar repo-scoped write tools (e.g., sub_issue_write), so adding coverage for these new granular tools would help prevent future classification regressions.

Copilot uses AI. Check for mistakes.

// === Repo content and structure write operations ===
"create_or_update_file" | "push_files" | "delete_file" | "create_branch"
| "update_pull_request_branch" | "create_repository" | "fork_repository" => {
Expand Down
115 changes: 115 additions & 0 deletions guards/github-guard/rust-guard/src/tools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ pub const WRITE_OPERATIONS: &[&str] = &[
"delete_release", // DELETE /repos/.../releases/{id}
// Pre-emptive: gist deletion (gh gist delete)
"delete_gist", // DELETE /gists/{gist_id}

];

/// Read-write operations that both read and modify data
Expand All @@ -89,6 +90,35 @@ pub const READ_WRITE_OPERATIONS: &[&str] = &[
"create_agent_task",
// 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

// 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
"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
Expand Down Expand Up @@ -307,4 +337,89 @@ mod tests {
);
}
}

#[test]
fn test_granular_issue_update_tools_are_read_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_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_sub_issue_management_tools_are_read_write_operations() {
for op in &["add_sub_issue", "remove_sub_issue", "reprioritize_sub_issue"] {
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
);
}
}

#[test]
fn test_pr_review_tools_are_read_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_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_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
);
}
}
}
Loading