feat(guard): add DIFC labels for 30 write tools and clean up stale entries#2873
feat(guard): add DIFC labels for 30 write tools and clean up stale entries#2873
Conversation
…tries Add explicit DIFC labeling rules in tool_rules.rs for all 30 write/mutating tools that previously fell through to the default handler. This ensures write operation responses carry proper secrecy and integrity labels instead of inheriting only the calling agent's ambient labels. Changes by category: - Gist writes (create_gist, update_gist): private:user secrecy + reader integrity, matching list_gists/get_gist for consistency - Issue/PR writes (create_issue, issue_write, sub_issue_write, add_issue_comment, create_pull_request, update_pull_request, merge_pull_request, pull_request_review_write, add_comment_to_pending_review, add_reply_to_pull_request_comment): repo-visibility secrecy + writer integrity - Repo content writes (create_or_update_file, push_files, delete_file, create_branch, update_pull_request_branch, create_repository, fork_repository): repo-visibility secrecy + writer integrity - Projects write: owner-scoped writer integrity - Label write: repo-visibility secrecy + writer integrity - Actions run trigger: repo-visibility secrecy + writer integrity - Copilot operations (assign_copilot_to_issue, request_copilot_review): repo-visibility secrecy + writer integrity - Notification writes (dismiss_notification, mark_all_notifications_read, manage_notification_subscription, manage_repository_notification_subscription): private:user secrecy - Star/unstar: public secrecy + project:github integrity Also removes 10 stale entries from WRITE_OPERATIONS and READ_WRITE_OPERATIONS (deprecated aliases consolidated into canonical tool names). Adds 19 new Rust tests covering all new label rules. Closes #2867 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR closes a DIFC labeling gap in the GitHub guard by adding explicit tool-specific label rules for write/mutating tools that previously fell through to the default handler, and cleans up stale write-tool classifications.
Changes:
- Added explicit DIFC secrecy/integrity labeling for multiple write/mutating tools (gists, notifications, issues/PRs, repo content, projects, labels, actions triggers, Copilot ops, star/unstar).
- Removed deprecated/stale tool aliases from
WRITE_OPERATIONSandREAD_WRITE_OPERATIONS. - Added Rust unit tests to cover the new tool-labeling behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
guards/github-guard/rust-guard/src/tools.rs |
Removes deprecated/stale tool aliases from write/read-write classification lists. |
guards/github-guard/rust-guard/src/labels/tool_rules.rs |
Adds explicit match arms to apply repo-/user-/org-scoped secrecy and appropriate integrity for previously-unlabeled write tools. |
guards/github-guard/rust-guard/src/labels/mod.rs |
Adds unit tests validating secrecy/integrity outcomes for newly covered write tools. |
Comments suppressed due to low confidence (5)
guards/github-guard/rust-guard/src/labels/tool_rules.rs:624
- New match arm covers multiple Issue/PR write tools, but unit tests only cover a subset (e.g., create_issue/issue_write/add_issue_comment/create_pull_request/merge_pull_request/pull_request_review_write/sub_issue_write). Please add tests exercising the remaining tools in this arm (update_pull_request, add_comment_to_pending_review, add_reply_to_pull_request_comment) to ensure they get the intended secrecy/integrity behavior and remain covered against regressions.
// === Issue/PR write operations (repo-scoped) ===
"create_issue" | "issue_write" | "sub_issue_write" | "add_issue_comment"
| "create_pull_request" | "update_pull_request" | "merge_pull_request"
| "pull_request_review_write" | "add_comment_to_pending_review"
| "add_reply_to_pull_request_comment" => {
guards/github-guard/rust-guard/src/labels/tool_rules.rs:640
- This new arm adds explicit labeling for several repo content/structure write tools, but current unit tests only cover create_or_update_file, push_files, create_branch, and fork_repository. Please add tests for the remaining tools here (delete_file, update_pull_request_branch, create_repository) so their labeling semantics are locked in.
// === Repo content and structure write operations ===
"create_or_update_file" | "push_files" | "delete_file" | "create_branch"
| "update_pull_request_branch" | "create_repository" | "fork_repository" => {
// Write operations that modify repo content/structure.
// S = S(repo) — response references repo-scoped content
// I = writer (agent-authored content)
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:578
- Notifications write tools were added to the notifications labeling rule, but tests only cover list_notifications/get_notification_details (existing) and dismiss_notification (new). Please add tests for mark_all_notifications_read, manage_notification_subscription, and manage_repository_notification_subscription to ensure they consistently receive private:user secrecy and none-level integrity.
This issue also appears in the following locations of the same file:
- line 620
- line 632
- line 668
- line 676
// === Notifications (user-scoped, private) ===
"list_notifications" | "get_notification_details"
| "dismiss_notification" | "mark_all_notifications_read"
| "manage_notification_subscription"
| "manage_repository_notification_subscription" => {
// Notifications are private to the authenticated user.
// S = private:user
// I = none (notifications reference external content of unknown trust)
secrecy = private_user_label();
integrity = vec![];
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:683
- Star/unstar is now explicitly labeled as public/project:github, but tests only cover star_repository. Please add a test for unstar_repository as well to keep the two tools’ labeling behavior symmetric and regression-proof.
// === Star/unstar operations (public metadata) ===
"star_repository" | "unstar_repository" => {
// Starring is a public action; response is minimal metadata.
// S = public (empty); I = project:github
secrecy = vec![];
baseline_scope = "github".to_string();
integrity = project_github_label(ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:674
- Copilot repo-scoped write tools are labeled here, but tests only cover assign_copilot_to_issue. Please add a test for request_copilot_review too, to ensure it stays repo-scoped secrecy with writer integrity.
// === Copilot agent operations (repo-scoped) ===
"assign_copilot_to_issue" | "request_copilot_review" => {
// Copilot assignment/review requests return repo-scoped content.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Closes #2867
Adds explicit DIFC labeling rules in
tool_rules.rsfor all 30 write/mutating tools that previously fell through to the default_ => {}handler. This ensures write operation responses carry proper secrecy and integrity labels instead of inheriting only the calling agent's ambient labels.Problem
Write tool responses were missing DIFC labels, causing asymmetries:
list_gists/get_gisthadprivate:usersecrecy, butcreate_gist/update_gistdid notprivate:usersecrecyChanges
New labeling rules (tool_rules.rs)
create_gist,update_gistprivate:usercreate_issue,issue_write,sub_issue_write,add_issue_comment,create_pull_request,update_pull_request,merge_pull_request,pull_request_review_write,add_comment_to_pending_review,add_reply_to_pull_request_commentcreate_or_update_file,push_files,delete_file,create_branch,update_pull_request_branch,create_repository,fork_repositoryprojects_writelabel_writeactions_run_triggerassign_copilot_to_issue,request_copilot_reviewdismiss_notification,mark_all_notifications_read,manage_notification_subscription,manage_repository_notification_subscriptionprivate:userstar_repository,unstar_repositoryStale entry cleanup (tools.rs)
Removed 10 deprecated tool aliases from
WRITE_OPERATIONSandREAD_WRITE_OPERATIONS:run_workflow,rerun_workflow_run,rerun_failed_jobs,cancel_workflow_run,delete_workflow_run_logs→ consolidated intoactions_run_triggeradd_project_item,delete_project_item,update_project_item→ consolidated intoprojects_writeupdate_issue→ replaced byissue_writecreate_pull_request_with_copilot→ not in current toolsnapsTesting
make agent-finished✅ (format + build + lint + all tests)