feat: add guard tool coverage for 22 missing GitHub MCP server tools#2280
feat: add guard tool coverage for 22 missing GitHub MCP server tools#2280
Conversation
Cross-referenced all tool names from github/github-mcp-server source against the guard's tool_rules.rs match arms. Added explicit labeling rules for every read tool that was previously falling through to the default case (inheriting empty labels). New tools covered: - Actions: get_job_logs (secret secrecy — logs may leak env vars) - Context: get_me, get_teams, get_team_members (private:user) - Discussions: list_discussions, get_discussion, get_discussion_comments, list_discussion_categories - Gists: list_gists, get_gist (private:user, reader integrity) - Git: get_repository_tree (repo visibility, approved) - Labels: list_label (repo visibility, approved) - Notifications: list_notifications, get_notification_details (private:user) - Projects: projects_list, projects_get (new canonical names) - Repos: list_starred_repositories (private:user) - Search: search_orgs (public, github metadata) - Security advisories: list_global_security_advisories, get_global_security_advisory, list_repository_security_advisories, list_org_repository_security_advisories Also updated: - tools.rs: added actions_run_trigger, projects_write, add_reply_to_pull_request_comment to write ops - response_paths.rs: added projects_list alias - INTEGRITY_TAG_SPEC.md and SECRECY_TAG_SPEC.md: documented all new tools Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot make sure that every github tool call has a corresponding set of unit tests that ensure the guard returns correct integrity and secrecy labels for label_resource and label_response. use mock data for these tests. |
There was a problem hiding this comment.
Pull request overview
This PR expands the GitHub guard’s DIFC labeling coverage to include previously-unhandled GitHub MCP server tools, with the goal of preventing un-labeled tools from leaking private data (missing secrecy tags) or being filtered unexpectedly (missing integrity tags).
Changes:
- Added tool-specific secrecy/integrity rules for ~22 previously-uncovered read tools in
tool_rules.rs. - Classified additional operations as write operations in
tools.rs. - Added
projects_listas an alias for existing project-item response path labeling, and updated secrecy/integrity spec docs.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| guards/github-guard/rust-guard/src/tools.rs | Adds additional tool names to write-operation classification. |
| guards/github-guard/rust-guard/src/labels/tool_rules.rs | Adds resource-label rules for newly covered tools (actions logs, discussions, gists, notifications, user/org context, org search, security advisories, repo tree, label list). |
| guards/github-guard/rust-guard/src/labels/response_paths.rs | Treats projects_list as an alias for list_project_items path labeling. |
| guards/github-guard/docs/SECRECY_TAG_SPEC.md | Documents secrecy expectations for the newly-covered tools. |
| guards/github-guard/docs/INTEGRITY_TAG_SPEC.md | Documents integrity expectations for the newly-covered tools. |
Comments suppressed due to low confidence (6)
guards/github-guard/rust-guard/src/labels/tool_rules.rs:526
list_global_security_advisories/get_global_security_advisorysetintegrity = project_github_label(ctx), but with an emptyrepo_idthe currentbaseline_scopewill causeensure_integrity_baselineto collapse the scoped integrity labels down to plainnone. Setbaseline_scope = "github"for these tools (or adjust baseline inference) to actually emitapproved:githubas documented.
// S = public (empty) — these are published advisories
// I = project:github — curated by GitHub security team
secrecy = vec![];
integrity = project_github_label(ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:481
- Same scope issue as
get_me:project_github_label(ctx)returns labels scoped to "github", but without settingbaseline_scopeaccordingly,ensure_integrity_baselinewill collapse this tonone. Setbaseline_scope = "github"for these org/user-metadata tools so they actually emitapproved:githubat the resource-label stage.
// S = private:user (org membership is sensitive)
// I = project:github (GitHub-controlled metadata)
secrecy = private_user_label();
integrity = project_github_label(ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:508
list_starred_repositoriesusesproject_github_label(ctx)but (as with other user-scoped tools)baseline_scopewill typically be empty, causingensure_integrity_baselineto downgrade the scopedapproved:githubintegrity to plainnone. Setbaseline_scopeto "github" (or change baseline inference) so this tool actually carries GitHub-approved integrity.
// S = private:user (personal data)
// I = project:github (GitHub-controlled metadata)
secrecy = private_user_label();
integrity = project_github_label(ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:517
search_orgsassignsintegrity = project_github_label(ctx)(scoped to "github"), but unlessbaseline_scopeis also set to "github",ensure_integrity_baselinewill downgrade it to plainnonewhenrepo_idis empty. Setbaseline_scopehere (or extendinfer_scope_for_baseline) soapproved:githubsurvives.
// S = public (empty)
// I = project:github (GitHub-controlled metadata)
secrecy = vec![];
integrity = project_github_label(ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:436
- Same issue as the other discussion tools:
get_discussion_commentsrelies on response labeling for public repos, but there is no response labeling implementation for this tool name, so integrity will stay at baselinenone. Add response labeling for discussion comment items (or set an explicit integrity baseline for public repos) so this tool doesn’t effectively become unusable under commonmin-integritysettings.
// S = inherits from repo visibility
// I = private repos get approved; public repos deferred to response labeling
secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx);
integrity = private_writer_integrity(repo_id, repo_private, ctx);
}
guards/github-guard/rust-guard/src/labels/tool_rules.rs:405
- The comment here references per-item secrecy refinement only for
list_project_items, but this match arm now also handles the canonicalprojects_list. To avoid confusion, update the comment to mentionprojects_listas the path-labeling entrypoint as well.
| "projects_list" | "projects_get" => {
// Projects are org-scoped; creating/managing projects requires org membership.
// I = approved:<owner> — equivalent to MEMBER author_association
// S = empty by default (public project); per-item secrecy for items is refined in
// label_response_paths for list_project_items
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // S = private:user | ||
| // I = project:github (GitHub-controlled metadata) | ||
| secrecy = private_user_label(); | ||
| integrity = project_github_label(ctx); | ||
| } |
There was a problem hiding this comment.
get_me sets integrity = project_github_label(ctx) (scoped to "github"), but baseline_scope is still repo_id (likely empty), so ensure_integrity_baseline will downgrade the scoped integrity labels to plain none. If the intent is approved:github, update baseline_scope to "github" in this arm (or otherwise ensure the baseline scope matches the scope used by project_github_label).
This issue also appears in the following locations of the same file:
- line 522
- line 477
- line 504
- line 513
| // I = private repos get approved; public repos deferred to response labeling | ||
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | ||
| integrity = private_writer_integrity(repo_id, repo_private, ctx); | ||
| } | ||
|
|
||
| "get_discussion_comments" => { | ||
| // Discussion comments are user-submitted, lowest-trust user content. | ||
| // S = inherits from repo visibility | ||
| // I = private repos get approved; public repos deferred to response labeling | ||
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | ||
| integrity = private_writer_integrity(repo_id, repo_private, ctx); |
There was a problem hiding this comment.
These discussion tools set integrity = private_writer_integrity(...), which becomes empty for public repos and therefore falls back to none via ensure_integrity_baseline. The comment says public repos are “deferred to response labeling”, but there’s currently no label_response_paths/label_response_items handling for list_discussions/get_discussion, so public discussion content will remain none and be filtered at any min-integrity > none. Either add fine-grained response labeling for these tools (author_association floor like issues) or choose a non-empty resource-level integrity baseline for public repos.
This issue also appears on line 432 of the same file.
| // I = private repos get approved; public repos deferred to response labeling | |
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | |
| integrity = private_writer_integrity(repo_id, repo_private, ctx); | |
| } | |
| "get_discussion_comments" => { | |
| // Discussion comments are user-submitted, lowest-trust user content. | |
| // S = inherits from repo visibility | |
| // I = private repos get approved; public repos deferred to response labeling | |
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | |
| integrity = private_writer_integrity(repo_id, repo_private, ctx); | |
| // I = approved — treat discussion content as approved at the resource level | |
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | |
| integrity = writer_integrity(repo_id, ctx); | |
| } | |
| "get_discussion_comments" => { | |
| // Discussion comments are user-submitted, lowest-trust user content. | |
| // S = inherits from repo visibility | |
| // I = approved — treat discussion comments as approved at the resource level | |
| secrecy = apply_repo_visibility_secrecy(&owner, &repo, repo_id, secrecy, ctx); | |
| integrity = writer_integrity(repo_id, ctx); |
| // Resource-level: conservative labeling; response labeling refines per-item. | ||
| // S = private:user (conservative — some gists may be secret) | ||
| // I = reader (user content, no repo-level trust signal) | ||
| secrecy = private_user_label(); |
There was a problem hiding this comment.
For user-scoped tools like gists, integrity = reader_integrity("user", ctx) will get downgraded to plain none because baseline_scope remains repo_id (typically empty here) and ensure_integrity_baseline/max_integrity only ranks labels for the provided scope. This means list_gists/get_gist won't actually carry unapproved:user at the resource-label stage. Set baseline_scope to "user" (or another consistent scope) in this match arm when assigning reader_integrity("user", ...) so the intended integrity survives baseline enforcement.
| secrecy = private_user_label(); | |
| secrecy = private_user_label(); | |
| baseline_scope = "user".to_string(); |
…, and check runs
Add 22 new REST routes to the proxy router covering:
- Actions: individual workflow/run/job gets, workflow-specific runs,
attempt jobs/logs, artifacts, caches, secrets, variables, environment config
- Discussions: list, get single, comments
- User API: /user, /user/keys|ssh_signing_keys|gpg_keys
- Notifications: /notifications
- Check runs/suites: /commits/{sha}/check-{runs,suites}
- Org-scoped Actions: /orgs/{org}/actions/secrets|variables
Add 5 new GraphQL patterns:
- Discussions (singular and plural), discussion categories
- viewer {} mapped to get_me
- organization() mapped to search_orgs
Previously, most repo-scoped endpoints fell through to the generic
get_file_contents fallback, receiving incorrect security labels.
Non-repo-scoped endpoints (/user, /notifications) were blocked entirely.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
…ce + label_response) Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/64f2a73e-be55-425e-b903-a8447aa8edac
Added 37 unit tests covering all 22 new tools in commit 9ea3413. Each tool has a
Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Co-authored-by: lpcox <15877973+lpcox@users.noreply.github.com> Agent-Logs-Url: https://github.com/github/gh-aw-mcpg/sessions/5d3a1a5e-292f-45e0-8856-10ea2317ed26
…n tool integrity (#2281) `ensure_integrity_baseline` ranks integrity labels by matching the `baseline_scope` against label scopes. When `baseline_scope` stays as `repo_id` (often empty) but `integrity` was assigned with a different scope, the rank comparison returns 0 and the labels get replaced by `none`. ## Fixes ### `baseline_scope` not set for `project:github`-scoped tools `project_github_label(ctx)` calls `writer_integrity("github", ctx)`, producing labels like `approved:github`. Without `baseline_scope = "github"`, `ensure_integrity_baseline` downgrades these to `none:`. Affected tools: `get_me`, `get_teams`, `get_team_members`, `list_starred_repositories`, `search_orgs`, `list_global_security_advisories`, `get_global_security_advisory`. ### `baseline_scope` not set for `user`-scoped tools `reader_integrity("user", ctx)` produces `unapproved:user` — same downgrade issue without `baseline_scope = "user"`. Affected tools: `list_gists`, `get_gist`. ### Discussion tools silently producing `none` integrity for public repos `private_writer_integrity` returns `[]` for public repos, which `ensure_integrity_baseline` converts to `none`. No `label_response_paths`/`label_response_items` fallback exists for these tools, so public discussion content was permanently stuck at `none` and filtered at any `min-integrity > none`. Replaced with `writer_integrity(repo_id, ctx)` to assign approved at the resource level unconditionally. Affected tools: `list_discussions`, `get_discussion`, `get_discussion_comments`. ## Test updates - Removed `repo_id = "github"` / `repo_id = "user"` workarounds in 9 tests that were compensating for the missing `baseline_scope` assignments — tests now use realistic empty `repo_id` - Updated discussion assertions from `none_integrity` → `writer_integrity` <!-- START COPILOT CODING AGENT TIPS --> --- 📍 Connect Copilot coding agent with [Jira](https://gh.io/cca-jira-docs), [Azure Boards](https://gh.io/cca-azure-boards-docs) or [Linear](https://gh.io/cca-linear-docs) to delegate work to Copilot in one click without leaving your project management tool.
|
@copilot some of the integrity tags in the summary don't make sense. all tags should fit into the same hierarchy of "merged" > "approved" > "unapproved" > "none" > "blocked. Make sure all the integrity tags for all calls fit within this hierarchy. |
|
@copilot I still see integrity levels "reader" and "deferred" which are not valid |
Replace 'reader' with 'unapproved' and 'deferred to response labeling' with 'refined per-item by response labeling' in tool_rules.rs comments. The code itself already uses correct label values via reader_integrity() which emits unapproved:... tags. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Problem
Cross-referencing all tool names from
github/github-mcp-serversource against the guard'stool_rules.rsrevealed 22 read tools falling through to the default_ =>case, which inherits empty labels. This means these tools had no integrity or secrecy labeling, causing them to either be filtered at anymin-integritythreshold or leak private data without secrecy tags.Changes
New tool coverage in
tool_rules.rs(22 read tools)get_job_logssecretget_meprivate:userget_teams,get_team_membersprivate:userlist_discussions,get_discussionget_discussion_commentslist_discussion_categorieslist_gists,get_gistprivate:userget_repository_treelist_labellist_notifications,get_notification_detailsprivate:userprojects_list,projects_get[]list_starred_repositoriesprivate:usersearch_orgs[]list_global_security_advisories,get_global_security_advisory[]list_repository_security_advisories,list_org_repository_security_advisoriesprivate:repoAlso updated
tools.rs: addedactions_run_trigger,projects_write,add_reply_to_pull_request_commentto write operations.Unit Tests (37 new tests in
labels/mod.rs)Every new tool now has corresponding unit tests verifying correct secrecy and integrity labels for both
label_resourceandlabel_response, using mock data:apply_tool_labels(label_resource): one test per tool confirming the exact secrecy and integrity labels returnedlabel_response_items:list_gists(public/private/mixed),get_gist(public/private),list_notifications,get_notification_details(MCP-wrapped)label_response_paths:list_gists(public/private/mixed),list_notifications,projects_list(ISSUE/DRAFT_ISSUE/PULL_REQUEST items)Proxy router expansion (22 new REST routes + 5 GraphQL patterns)
Added explicit routes for Actions, discussions, user, notifications, and check runs to the proxy router. Previously most fell through to the generic
get_file_contentsfallback (wrong security labels) or were blocked entirely.Valid integrity levels:
merged>approved>unapproved>none>blocked