Add ifc label for search_issues tool#2456
Open
gokhanarkan wants to merge 2 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Adds IFC _meta.ifc labeling to the search_issues tool when InsidersMode is enabled, following the established insiders-only metadata pattern for other ingress tools.
Changes:
- Added
ifc.LabelSearchIssues(...)(plusintersectReaders) to join per-repo labels forsearch_issuesresults. - Extended the shared
searchHandlerwith an additive post-processing hook so callers can attach metadata without duplicating the search logic. - Updated
search_issuesto compute/join per-repo visibility + collaborators and attach the resulting IFC label; added unit and integration-style tests.
Show a summary per file
| File | Description |
|---|---|
| pkg/ifc/ifc.go | Adds label-join helper for search_issues and reader-set intersection logic. |
| pkg/ifc/ifc_test.go | Adds table-driven tests for LabelSearchIssues join semantics. |
| pkg/github/search_utils.go | Introduces optional post-processing hook for searchHandler results. |
| pkg/github/issues.go | Wires insiders-only IFC labeling into search_issues via the new post-process hook. |
| pkg/github/issues_test.go | Adds insiders-mode test coverage for search_issues IFC behavior. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 3
gokhanarkan
added a commit
that referenced
this pull request
May 12, 2026
- LabelSearchIssues now returns (SecurityLabel, bool); the bool is
false when len(repoVisibilities) != len(readerSets), so callers can
omit the label rather than emit one computed from inconsistent
inputs.
- searchIssuesIFCPostProcess no longer substitutes [owner] when the
collaborators API returns an empty list. The substitution was
inconsistent with the cross-repo intersection semantics: the owner
could appear in another matched private repo's collaborator list and
thereby widen the joined reader set incorrectly. Empty collaborator
sets are now passed through unchanged.
- Add a subtest exercising the collaborators-failure branch (500 on
/repos/{owner}/{repo}/collaborators), asserting the tool still
succeeds and result.Meta["ifc"] is absent.
- Extend the LabelSearchIssues table tests with the slice-length
mismatch case.
Addresses the three Copilot findings on #2456.
gokhanarkan
added a commit
that referenced
this pull request
May 12, 2026
Emits an IFC SecurityLabel on the issue_read tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, get_file_contents in #2454, and search_issues in #2456. issue_read operates on a single issue in a single repository so the label has the same per-repo semantics as list_issues; the helper ifc.LabelListIssues is reused directly. Integrity is always untrusted (issue contents, comments, and label descriptions are user-authored). Public repos are labelled PublicUntrusted; private repos are labelled PrivateUntrusted with the repository's collaborator logins, falling back to [owner] when the collaborators lookup fails. The IssueRead handler dispatches to four sub-functions (GetIssue, GetIssueComments, GetSubIssues, GetIssueLabels). The IFC label is attached at the dispatch site via a single attachIFC closure, so all four method branches emit the label without changes to the underlying helpers. Visibility-lookup failures cause the label to be omitted entirely (consistent with get_file_contents and search_issues). A future cleanup PR can extract attachIFC into a shared helper now that get_file_contents, search_issues, and issue_read use near-identical closures; intentionally not bundled here to keep the diff minimal. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: chained on #2456 (gokhanarkan/fides-search-issues), which is in turn chained on #2454. GitHub will retarget the base to main once those merge.
Emits an IFC SecurityLabel on the search_issues tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, and get_file_contents in #2454. Search results may span multiple repositories, so the label is the IFC join of the per-repository labels: - Integrity is always untrusted (issues are user-authored). - If any matched repository is public, the joined readers are ["public"] (the public side dominates the lub). - Otherwise the joined readers are the intersection of the collaborator sets across all matched private repositories. - Empty result sets are labelled public-untrusted (no data leaked). The shared searchHandler in search_utils.go gains an additive variadic 'searchOption' hook so SearchIssues can attach _meta.ifc without duplicating the search call. SearchPullRequests is unaffected; it does not pass any options. If any per-repository visibility or collaborators lookup fails the label is omitted entirely, consistent with get_file_contents, to avoid misclassifying the result. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: this PR is chained on #2454 (gokhanarkan/fides-get-file-contents) because it depends on the FetchRepoIsPrivate and FetchRepoCollaborators helpers introduced there. GitHub will retarget the base to main once #2454 merges.
- LabelSearchIssues now returns (SecurityLabel, bool); the bool is
false when len(repoVisibilities) != len(readerSets), so callers can
omit the label rather than emit one computed from inconsistent
inputs.
- searchIssuesIFCPostProcess no longer substitutes [owner] when the
collaborators API returns an empty list. The substitution was
inconsistent with the cross-repo intersection semantics: the owner
could appear in another matched private repo's collaborator list and
thereby widen the joined reader set incorrectly. Empty collaborator
sets are now passed through unchanged.
- Add a subtest exercising the collaborators-failure branch (500 on
/repos/{owner}/{repo}/collaborators), asserting the tool still
succeeds and result.Meta["ifc"] is absent.
- Extend the LabelSearchIssues table tests with the slice-length
mismatch case.
Addresses the three Copilot findings on #2456.
279a59b to
1df735c
Compare
gokhanarkan
added a commit
that referenced
this pull request
May 12, 2026
Emits an IFC SecurityLabel on the issue_read tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, get_file_contents in #2454, and search_issues in #2456. issue_read operates on a single issue in a single repository so the label has the same per-repo semantics as list_issues; the helper ifc.LabelListIssues is reused directly. Integrity is always untrusted (issue contents, comments, and label descriptions are user-authored). Public repos are labelled PublicUntrusted; private repos are labelled PrivateUntrusted with the repository's collaborator logins, falling back to [owner] when the collaborators lookup fails. The IssueRead handler dispatches to four sub-functions (GetIssue, GetIssueComments, GetSubIssues, GetIssueLabels). The IFC label is attached at the dispatch site via a single attachIFC closure, so all four method branches emit the label without changes to the underlying helpers. Visibility-lookup failures cause the label to be omitted entirely (consistent with get_file_contents and search_issues). A future cleanup PR can extract attachIFC into a shared helper now that get_file_contents, search_issues, and issue_read use near-identical closures; intentionally not bundled here to keep the diff minimal. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: chained on #2456 (gokhanarkan/fides-search-issues), which is in turn chained on #2454. GitHub will retarget the base to main once those merge.
gokhanarkan
added a commit
that referenced
this pull request
May 12, 2026
Emits an IFC SecurityLabel on the search_repositories tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me, list_issues, get_file_contents, search_issues, and issue_read. Per the spec in github/copilot-mcp-core#1623, the label is PublicUntrusted() regardless of whether matched repositories are private. Repository search is treated as a discovery surface: matched repositories are by definition already accessible to the caller, so the joined readers are universal. Integrity is untrusted because repository names, descriptions, and topics are user-authored. Implementation note: SearchRepositories has its own handler distinct from the shared searchHandler used by SearchIssues, so the IFC attach is wired inline at the success return rather than via the postProcess hook added in #2456. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: chained on #2457 (gokhanarkan/fides-issue-read), which is in turn chained on #2456. GitHub will retarget the base to main once those merge.
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 thesearch_issuestool result when theInsidersModeflag is enabled, mirroring the pattern landed forget_mein #2432,list_issuesin #2453, andget_file_contentsin #2454.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
Search results may span multiple repositories, so
_meta.ifcforsearch_issuesis the IFC join of the per-repository labels:untrusted(issues are user-authored).["public"](the public side dominates).["public"](no data leaked).If any per-repo visibility or collaborators lookup fails, the label is omitted entirely (consistent with
get_file_contents) to avoid misclassifying the result.New shared helpers
ifc.LabelSearchIssues(repoVisibilities []bool, readerSets [][]string)inpkg/ifc/ifc.go— pure join function, separately unit-tested.searchHandlerin pkg/github/search_utils.go gains an additive variadicsearchOptionhook soSearchIssuescan attach_meta.ifcwithout duplicating the search call.search_pull_requestsis unaffected — it passes no options and continues to behave exactly as before.Empty-intersection semantics (called out for reviewers)
If two private repos have zero overlapping collaborators, the joined reader set is
[]. We emitconfidentiality: []rather than skipping the label — this matches the IFC math (no one can read) and lets the client engine decide what to do. Worth a sanity-check from @JoannaaKL.Cost note
A search returning 30 results across 30 different private repos = 60 API calls (visibility + collaborators each). Bounded but not free; cache lands in a follow-up per the standing agreement.
Tests
Test_LabelSearchIssuesin new pkg/ifc/ifc_test.go — table-driven coverage of the join math (empty, single public, mixed, intersecting private, no overlap, dedupe).Test_SearchIssues_IFC_InsidersModein pkg/github/issues_test.go with 6 subtests:result.Meta == nil.integrity=untrusted,confidentiality=["public"].["public"](public wins).ifcmeta.["public"].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.)