Skip to content

feat(guard): add DIFC labels for 30 write tools and clean up stale entries#2873

Merged
lpcox merged 1 commit intomainfrom
fix/write-tool-difc-labels
Mar 30, 2026
Merged

feat(guard): add DIFC labels for 30 write tools and clean up stale entries#2873
lpcox merged 1 commit intomainfrom
fix/write-tool-difc-labels

Conversation

@lpcox
Copy link
Copy Markdown
Collaborator

@lpcox lpcox commented Mar 30, 2026

Summary

Closes #2867

Adds 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.

Problem

Write tool responses were missing DIFC labels, causing asymmetries:

  • list_gists/get_gist had private:user secrecy, but create_gist/update_gist did not
  • Issue/PR write responses contained repo-scoped content without repo-visibility secrecy
  • Notification write operations lacked private:user secrecy

Changes

New labeling rules (tool_rules.rs)

Category Tools Secrecy Integrity
Gist writes create_gist, update_gist private:user reader (user content)
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 S(repo) writer
Repo content writes create_or_update_file, push_files, delete_file, create_branch, update_pull_request_branch, create_repository, fork_repository S(repo) writer
Projects write projects_write writer (owner-scoped)
Label write label_write S(repo) writer
Actions trigger actions_run_trigger S(repo) writer
Copilot ops assign_copilot_to_issue, request_copilot_review S(repo) writer
Notification writes dismiss_notification, mark_all_notifications_read, manage_notification_subscription, manage_repository_notification_subscription private:user none
Star/unstar star_repository, unstar_repository public project:github

Stale entry cleanup (tools.rs)

Removed 10 deprecated tool aliases from WRITE_OPERATIONS and READ_WRITE_OPERATIONS:

  • run_workflow, rerun_workflow_run, rerun_failed_jobs, cancel_workflow_run, delete_workflow_run_logs → consolidated into actions_run_trigger
  • add_project_item, delete_project_item, update_project_item → consolidated into projects_write
  • update_issue → replaced by issue_write
  • create_pull_request_with_copilot → not in current toolsnaps

Testing

  • 19 new Rust tests covering all new label rules
  • All 248 Rust tests pass
  • make agent-finished ✅ (format + build + lint + all tests)

…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>
Copilot AI review requested due to automatic review settings March 30, 2026 20:49
@lpcox lpcox merged commit 46d43a4 into main Mar 30, 2026
12 checks passed
@lpcox lpcox deleted the fix/write-tool-difc-labels branch March 30, 2026 20:52
@lpcox lpcox restored the fix/write-tool-difc-labels branch March 30, 2026 20:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

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_OPERATIONS and READ_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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[guard-coverage] Guard coverage gap: 30 write operations in tool_rules.rs lack explicit DIFC labeling rules

2 participants