Skip to content

fix(guard): add enable_toolset DIFC unit test + backport coverage changes from #3762#3768

Merged
lpcox merged 4 commits intomainfrom
copilot/fix-comments-from-review-thread
Apr 14, 2026
Merged

fix(guard): add enable_toolset DIFC unit test + backport coverage changes from #3762#3768
lpcox merged 4 commits intomainfrom
copilot/fix-comments-from-review-thread

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 14, 2026

enable_toolset had no apply_tool_labels test coverage despite being a capability-expansion tool that requires writer-level integrity to prevent low-trust agent self-escalation.

Changes

  • Cherry-picked PR fix(guard): cover deprecated tool aliases, enable_toolset DIFC rule, and pre-emptive CLI entries #3762 code changes (tool_rules.rs, tools.rs):

    • Extended projects_write and actions_run_trigger match arms with deprecated aliases (add_project_item, update_project_item, delete_project_item, run_workflow, delete_workflow_run_logs)
    • Added explicit enable_toolset rule: S = public (empty), I = writer:github
    • Added pre-emptive CLI rules for update_issue_comment, delete_issue_comment, create_release, edit_release, delete_release, delete_gist
    • Classified all of the above in WRITE_OPERATIONS / READ_WRITE_OPERATIONS
  • Added unit test (labels/mod.rs) for the enable_toolset rule:

let (secrecy, integrity, _) = apply_tool_labels("enable_toolset", &tool_args, "", vec![], vec![], String::new(), &ctx);

assert!(secrecy.is_empty());  // public — no repo-scoped data
assert_eq!(integrity, writer_integrity("github", &ctx));  // writer:github scope

lpcox and others added 3 commits April 14, 2026 15:53
…and pre-emptive CLI entries

- Add 5 deprecated MCP tool aliases to write classification:
  run_workflow, delete_workflow_run_logs, add_project_item,
  delete_project_item (WRITE_OPERATIONS), update_project_item
  (READ_WRITE_OPERATIONS)
- Add DIFC labeling rules for deprecated aliases by extending
  existing match arms (projects_write, actions_run_trigger)
- Add explicit enable_toolset DIFC rule with writer-level integrity
  to prevent low-trust agents from self-escalating
- Add 6 pre-emptive CLI entries: update_issue_comment,
  delete_issue_comment, create_release, edit_release,
  delete_release, delete_gist
- Add DIFC labeling rules for all pre-emptive CLI entries
- Add tests for new classifications

Fixes #3720

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Resolves review comment r3080468796: adds a test asserting that
enable_toolset returns writer-level integrity on the github scope
and empty (public) secrecy, preventing regressions on this
capability-expansion guard rule.

Also cherry-picks the underlying code changes from PR #3762 (commits
5110c2a and 0b4c9ba) which add the enable_toolset rule plus
deprecated alias coverage to tool_rules.rs and tools.rs.

Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/2b9cfc07-0be9-489a-afaf-cbe535fc52b8

Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix code according to review comments fix(guard): add enable_toolset DIFC unit test + backport coverage changes from #3762 Apr 14, 2026
Copilot AI requested a review from lpcox April 14, 2026 15:57
@lpcox lpcox marked this pull request as ready for review April 14, 2026 15:58
Copilot AI review requested due to automatic review settings April 14, 2026 15:58
@lpcox lpcox merged commit 1cd1394 into main Apr 14, 2026
3 checks passed
@lpcox lpcox deleted the copilot/fix-comments-from-review-thread branch April 14, 2026 15:58
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 tightens DIFC enforcement in the GitHub guard by explicitly labeling capability-expansion and deprecated/CLI-alias tool names as writes, and adds unit coverage for the previously-uncovered enable_toolset labeling rule.

Changes:

  • Extends write classification (WRITE_OPERATIONS / READ_WRITE_OPERATIONS) to include deprecated tool aliases and pre-emptive CLI tool names, with unit tests for classification.
  • Expands apply_tool_labels match arms to cover deprecated aliases and adds explicit DIFC labeling for enable_toolset plus pre-emptive CLI tools (*_issue_comment, *_release, delete_gist).
  • Adds a unit test validating enable_toolset secrecy/integrity labeling.
Show a summary per file
File Description
guards/github-guard/rust-guard/src/tools.rs Classifies deprecated aliases and pre-emptive CLI tool names as write/read-write operations; adds classification tests.
guards/github-guard/rust-guard/src/labels/tool_rules.rs Adds/extends DIFC labeling rules for deprecated aliases, enable_toolset, and several pre-emptive CLI write tools.
guards/github-guard/rust-guard/src/labels/mod.rs Adds unit test coverage for enable_toolset labeling behavior.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 2

// Enabling a toolset expands the agent's runtime capability set.
// Requires writer-level integrity to prevent low-trust agents from
// self-escalating by enabling additional tool groups.
// S = public (empty — no repository-scoped data); I = writer (global)
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

enable_toolset sets integrity to writer_integrity("github"), but label_resource later applies ensure_integrity_baseline(infer_scope_for_baseline(...)). For tools with empty repo_id (like enable_toolset), infer_scope_for_baseline currently returns an empty scope, which can downgrade scoped labels like approved:github to unscoped none under scoped allowlists (no Public/All token). Please ensure the final baseline scope matches "github" for this tool (e.g., teach infer_scope_for_baseline about enable_toolset, or switch this rule to use unscoped integrity if that’s the intended policy).

Suggested change
// S = public (empty — no repository-scoped data); I = writer (global)
// S = public (empty — no repository-scoped data); I = writer (global)
// Keep `repo_id` aligned with the intended global scope so later
// baseline inference does not treat this as unscoped and weaken
// `approved:github` / writer-level labels.
repo_id = "github";

Copilot uses AI. Check for mistakes.
Comment on lines +696 to +721
// === Issue/PR comment editing/deletion (pre-emptive) ===
"update_issue_comment" | "delete_issue_comment" => {
// Editing or deleting an issue/PR comment is a repo-scoped write.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Release management (pre-emptive) ===
"create_release" | "edit_release" | "delete_release" => {
// Release operations are repo-scoped writes.
// S = S(repo); I = writer
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = writer_integrity(repo_id, ctx);
}

// === Gist deletion (pre-emptive) ===
"delete_gist" => {
// Gist deletion is a write on user-scoped content.
// Conservatively treat gists as private/user-scoped, consistent with
// other gist operations that may target secret gists.
// S = private_user; I = writer(user)
secrecy = private_user_label();
baseline_scope = "user".to_string();
integrity = writer_integrity("user", ctx);
}
Copy link

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

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

New apply_tool_labels match arms were added for update_issue_comment/delete_issue_comment, release management, and delete_gist, but there are no corresponding unit tests in labels/mod.rs validating the expected secrecy/integrity outputs. Given these are write operations that impact DIFC enforcement, please add tests asserting S(repo); I=writer for repo-scoped tools and S=private:user; I=writer(user) for delete_gist.

Copilot uses AI. Check for mistakes.
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.

3 participants