Uh oh!
There was an error while loading. Please reload this page.
Annotate read tools with ifc labels - #2671
Conversation
There was a problem hiding this comment.
Pull request overview
Adds/expands IFC (Information Flow Control) labeling across additional read-only GitHub MCP tools, gated behind the ifc_labels insiders feature flag, to help downstream clients reason about confidentiality/integrity of tool outputs.
Changes:
- Adds new IFC label constructors for additional GitHub data domains (repo metadata, commit content, Actions results, advisories, gists, projects, teams, notifications).
- Introduces shared helpers (
attach*IFCLabel,newRepoVisibilityIFCLabeler) to consistently gate/attach labels and avoid repeated visibility lookups. - Wires IFC label attachment into more read tools (repos, search, PRs, Actions, discussions, labels, security tools) and adds/extends unit tests.
Show a summary per file
| File | Description |
|---|---|
| pkg/ifc/ifc.go | Adds new IFC label functions for additional resource types. |
| pkg/ifc/ifc_test.go | Adds unit tests covering the new IFC label functions. |
| pkg/github/ifc_labels.go | Centralizes IFC label attachment helpers and feature-flag gating. |
| pkg/github/security_advisories.go | Attaches IFC labels to advisory tools (global/repo/org). |
| pkg/github/secret_scanning.go | Attaches static private-untrusted IFC labels to secret scanning results. |
| pkg/github/dependabot.go | Attaches static private-untrusted IFC labels to Dependabot alert results. |
| pkg/github/code_scanning.go | Attaches static private-untrusted IFC labels to code scanning alert results. |
| pkg/github/search.go | Centralizes search-related IFC labeling and adds IFC labeling for more search tools. |
| pkg/github/repositories.go | Attaches repo-visibility-derived IFC labels to commit/branch/tag/release/collaborator read tools and refactors get_file_contents labeling to shared helper. |
| pkg/github/repositories_test.go | Adds a feature-flag/visibility-lookup behavior test for IFC labeling on get_commit. |
| pkg/github/issues.go | Refactors IFC label attachment to shared helper and labels additional issue metadata tools. |
| pkg/github/issue_fields.go | Labels issue field definition results (repo-scoped vs org-scoped). |
| pkg/github/pullrequests.go | Attaches repo-visibility-derived IFC labels for pull request read methods and search. |
| pkg/github/actions.go | Attaches repo-visibility-derived IFC labels across GitHub Actions list/get/log tools. |
| pkg/github/discussions.go | Attaches repo-visibility-derived IFC labels to discussion read tools and category listing. |
| pkg/github/labels.go | Attaches repo-visibility-derived IFC labels to label read tools (GraphQL-backed). |
| pkg/github/git.go | Attaches commit-contents IFC label to repository tree tool results. |
| pkg/github/gists.go | Attaches gist/gist-list IFC labels to gist read tools. |
| pkg/github/projects.go | Attaches conservative project IFC labels to project read tools. |
| pkg/github/context_tools.go | Refactors to shared static IFC label attachment for user/team tools. |
| pkg/github/notifications.go | Attaches static IFC label to notification details tool results. |
Copilot's findings
- Files reviewed: 21/21 changed files
- Comments generated: 1
Uh oh!
There was an error while loading. Please reload this page.
Repository security advisory listings can include draft/triage/closed advisories (via the state filter), which are not world-readable even on a public repository. Deriving confidentiality from repo visibility alone under-classified those results as public. LabelRepositorySecurityAdvisory now takes an allPublished flag and only returns a public label when the repo is public AND every returned advisory is published; otherwise it is private. list_repository_security_advisories computes allPublished from the response state; the org-wide listing stays private-untrusted. Adds unit + handler regression tests covering the draft-advisory-on-public-repo case. Addresses PR review feedback.
…ors, get_me Audit for the same bug class as the repo-advisory fix (confidentiality derived from a coarse signal that misses access-restricted items) found three more under-classifications: - Releases (list_releases, get_latest_release, get_release_by_tag): draft releases are visible only to push-access users and are not world-readable even on a public repo. New LabelRelease(isPrivate, hasDraft) returns public only for a non-draft release on a public repo; handlers compute hasDraft from the response (Draft flag / per-item scan). - list_repository_collaborators: a collaborator roster requires push access to list, so it is never world-readable, not even on a public repo. New LabelCollaboratorRoster() is always PrivateTrusted (mirrors LabelTeam), replacing the repo-visibility-derived label. - get_me: the result includes private_gists / total_private_repos / owned_private_repos, which are not part of the public profile. LabelGetMe is now PrivateTrusted instead of PublicTrusted. Verified the remaining public-capable labels are sound: Actions logs are world-readable on public repos; branches/tags are public metadata; gist, project, search, and starred-repo labels read per-item visibility and join. Adds ifc unit tests for the new/changed labels and a get_release_by_tag handler regression test (draft on public repo -> private); updates the get_me handler test to assert private.
Explain on LabelSearchIssues (and cross-ref from LabelGistList) that a tool result is delivered as one opaque payload and the IFC engine makes one allow/deny decision per flow at egress, so the only sound bound for a list is the meet of every item's label. Per-item labels would only be load-bearing if the engine could partition a result and route items to different sinks; until then they would invite unsafe declassification of a public item that arrived alongside private data. Doc-only change.
SamMorrowDrums
left a comment
There was a problem hiding this comment.
Reviewed and approving. The IFC label model is well-reasoned — the join-as-meet documentation on LabelSearchIssues and the "omit on visibility-lookup failure rather than risk misclassification" invariant in the attach* helpers are exactly right.
Two things I checked specifically:
- Context cost when disabled: nil. The
attach*helpers early-return on!IsFeatureEnabled, labels live in_meta.ifc(not schemas/descriptions), so output is byte-identical when the flag is off. - The
InsidersFeatureFlagsremoval: this demotesifc_labelsto opt-in-only (still inAllowedFeatureFlags), so insiders builds no longer auto-pay the extra repo-visibility lookups /_metabytes. Good call as the labeled-tool surface grows — though note the PR description still says "behind an insiders flag," which is now slightly stale (it's opt-in via--features ifc_labels).
CI green across all 19 checks, good test coverage in ifc_test.go and the per-tool tests. 👍
What changed
Added ifc annotations to additional read tools. Behind a
ifc_labelsinsiders flag.MCP impact
Security / limits
Tool renaming
deprecated_tool_aliases.goNote: if you're renaming tools, you must add the tool aliases. For more information on how to do so, please refer to the official docs.
Lint & tests
./script/lint./script/testDocs