Add ifc label for get_file_contents tool#2454
Open
gokhanarkan wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds IFC SecurityLabel annotations to the get_file_contents tool result behind InsidersMode, aligning this ingress tool with the existing _meta.ifc pattern used by other tools (e.g., get_me, list_issues).
Changes:
- Add
ifc.LabelGetFileContents(isPrivate, readers)helper for computing labels forget_file_contents. - Add
FetchRepoIsPrivate(ctx, client, owner, repo)helper and use it (lazily) to determine repo visibility when attaching IFC metadata. - Add unit tests covering insiders off / insiders on (public) / insiders on (private) label emission for
get_file_contents.
Show a summary per file
| File | Description |
|---|---|
| pkg/ifc/ifc.go | Adds LabelGetFileContents helper to compute IFC labels for file-content results. |
| pkg/github/repositories.go | Adds FetchRepoIsPrivate and attaches _meta.ifc to successful get_file_contents responses in insiders mode. |
| pkg/github/repositories_test.go | Adds Test_GetFileContents_IFC_InsidersMode to validate label emission behavior. |
Copilot's findings
- Files reviewed: 3/3 changed files
- Comments generated: 4
41cf9f1 to
0803f48
Compare
Emits an IFC SecurityLabel on the get_file_contents tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in Public repositories are labelled PublicUntrusted (anyone can author file content via pull requests). Private repositories are labelled PrivateTrusted with the repository owner as a placeholder reader, since only collaborators can land changes there. Full collaborator enumeration is intentionally deferred to a follow-up shared helper. A new exported FetchRepoIsPrivate helper wraps Repositories.Get for visibility lookups; it is invoked lazily and only when InsidersMode is on, so non-insiders pay no extra round trip. Visibility lookup failures skip the label rather than fail the user-facing call. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389.
- FetchRepoIsPrivate: tighten doc to 'returns whether a repository is private' and close the underlying *github.Response body. - attachIFC: skip emitting the ifc label when the repository visibility lookup fails, instead of falling through to PublicUntrusted (which would mislabel a private or unknown-visibility repo as public). The failure is no longer cached so a subsequent return path can retry. - Add a test asserting the tool still succeeds and omits result.Meta ["ifc"] when the visibility lookup returns 500.
0803f48 to
4fc24be
Compare
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Emits an IFC
SecurityLabelon theget_file_contentstool result when theInsidersModeflag is enabled, mirroring the pattern landed forget_mein #2432 andlist_issuesin #2453.Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. One of the ingress tools listed in #1623's tool table.
What this PR does
_meta.ifconto theget_file_contentsCallToolResultwhendeps.GetFlags(ctx).InsidersModeis true. No behaviour change when the flag is off.PublicUntrusted()(file contents can be authored by anyone via PRs, so integrity isuntrusted; readers are["public"]).PrivateTrusted([owner])— only collaborators can land changes in private repos, so integrity istrusted.[owner]is a placeholder reader set; full collaborator enumeration is deferred (see limitation).matchFilesGit-tree fallback) via a smallattachIFCclosure to avoid duplicating the insiders block five times.New shared helper
FetchRepoIsPrivate(ctx, client, owner, repo) (bool, error)in pkg/github/repositories.go. Thin wrapper aroundclient.Repositories.Get. Exported so upcoming ingress tools (search_issues,issue_read) can reuse the same visibility lookup. Flagging this for reviewers in case the export surface is a concern.The lookup is invoked lazily and only when
InsidersModeis on, so non-insiders pay no extra round trip. If the lookup fails, we skip the label rather than fail the user-facing call (label is best-effort metadata, not security gating yet).LabelGetFileContents(isPrivate, readers)is a new helper inpkg/ifc/ifc.gothat composes the existingPublicUntrusted/PrivateTrustedconstructors.Known limitation (called out for reviewers)
Same as #2453: private-repo confidentiality currently uses
[owner]rather than the full collaborator list. Fetching collaborators requires a paginated REST call; rather than bake that into this PR, a shared visibility/collaborators helper is intended as a follow-up thatget_file_contents,list_issues,search_issues, andissue_readcan all share.TODO(fides)marker is at the call site.Tests
Test_GetFileContents_IFC_InsidersModemirrorsTest_ListIssues_IFC_InsidersModewith three subtests:result.Meta == nil.integrity=untrusted,confidentiality=["public"].integrity=trusted,confidentiality=["<owner>"].Existing
Test_GetFileContentscases are unchanged (theGetReposByOwnerByRepomock is already wired in those cases — its response is just inspected only when insiders is on).Validation
go test -race ./...— green.gofmt -sclean;go vet ./...clean../script/lintitself fails locally with a pre-existing golangci-lint Go-version mismatch unrelated to this change.)