Skip to content

Commit ba21d0b

Browse files
committed
Replace ingress IFC reader list with private marker
Switches the ingress IFC labels from emitting a per-repo collaborator list to a single 'private' marker. The CLI engine now fetches readers from the GitHub endpoint on demand at egress decision time (P-F check), with pagination + caching, which removes a wire-bloat ceiling for repos with thousands of collaborators. Drops the per-call FetchRepoCollaborators from list_issues, issue_read, get_file_contents, search_issues, and search_repositories. The shared LabelSearchIssues helper collapses to a single []bool argument; the intersection logic and length-mismatch failure mode go away. This is a breaking wire-format change for _meta.ifc consumers — coordinate with the CLI cut-over. Refs github/copilot-mcp-core#1389.
1 parent fbf68b2 commit ba21d0b

8 files changed

Lines changed: 83 additions & 448 deletions

File tree

pkg/github/issues.go

Lines changed: 4 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -298,11 +298,7 @@ Options are:
298298

299299
// attachIFC adds the IFC label to a successful tool result when
300300
// InsidersMode is enabled. If the visibility lookup fails the
301-
// label is omitted rather than misclassifying the result. If
302-
// only the collaborators lookup fails for a private repo we
303-
// fall back to the owner so the reader set is never empty. The
304-
// label matches list_issues semantics: per-repo visibility,
305-
// integrity always untrusted.
301+
// label is omitted rather than misclassifying the result.
306302
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
307303
if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode {
308304
return r
@@ -311,19 +307,10 @@ Options are:
311307
if err != nil {
312308
return r
313309
}
314-
var readers []string
315-
if isPrivate {
316-
if collaborators, err := FetchRepoCollaborators(ctx, client, owner, repo); err == nil {
317-
readers = collaborators
318-
}
319-
if len(readers) == 0 {
320-
readers = []string{owner}
321-
}
322-
}
323310
if r.Meta == nil {
324311
r.Meta = mcp.Meta{}
325312
}
326-
r.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
313+
r.Meta["ifc"] = ifc.LabelListIssues(isPrivate)
327314
return r
328315
}
329316

@@ -1034,36 +1021,18 @@ func searchIssuesIFCPostProcess(deps ToolDependencies) searchPostProcessFn {
10341021

10351022
uniqueRepos := uniqueSearchIssuesRepos(result)
10361023
visibilities := make([]bool, 0, len(uniqueRepos))
1037-
readerSets := make([][]string, 0, len(uniqueRepos))
10381024
for _, r := range uniqueRepos {
10391025
isPrivate, err := FetchRepoIsPrivate(ctx, client, r.owner, r.repo)
10401026
if err != nil {
10411027
return
10421028
}
10431029
visibilities = append(visibilities, isPrivate)
1044-
if !isPrivate {
1045-
readerSets = append(readerSets, nil)
1046-
continue
1047-
}
1048-
collaborators, err := FetchRepoCollaborators(ctx, client, r.owner, r.repo)
1049-
if err != nil {
1050-
return
1051-
}
1052-
// Preserve an empty collaborator set as-is. Substituting the
1053-
// owner here would corrupt the cross-repo intersection (the
1054-
// owner could appear in another repo's collaborator list and
1055-
// widen the joined reader set incorrectly).
1056-
readerSets = append(readerSets, collaborators)
10571030
}
10581031

1059-
label, ok := ifc.LabelSearchIssues(visibilities, readerSets)
1060-
if !ok {
1061-
return
1062-
}
10631032
if callResult.Meta == nil {
10641033
callResult.Meta = mcp.Meta{}
10651034
}
1066-
callResult.Meta["ifc"] = label
1035+
callResult.Meta["ifc"] = ifc.LabelSearchIssues(visibilities)
10671036
}
10681037
}
10691038

@@ -1728,22 +1697,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
17281697
if result.Meta == nil {
17291698
result.Meta = mcp.Meta{}
17301699
}
1731-
var readers []string
1732-
if isPrivate {
1733-
restClient, err := deps.GetClient(ctx)
1734-
if err == nil {
1735-
if collaborators, err := FetchRepoCollaborators(ctx, restClient, owner, repo); err == nil {
1736-
readers = collaborators
1737-
}
1738-
}
1739-
// Fall back to the repository owner so the reader set is
1740-
// never empty for a private repository even if the
1741-
// collaborators lookup fails.
1742-
if len(readers) == 0 {
1743-
readers = []string{owner}
1744-
}
1745-
}
1746-
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
1700+
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate)
17471701
}
17481702
return result, nil, nil
17491703
})

pkg/github/issues_test.go

Lines changed: 11 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -297,10 +297,6 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
297297
handlers := map[string]http.HandlerFunc{
298298
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue),
299299
GetReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockComments),
300-
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
301-
{Login: github.Ptr("octocat")},
302-
{Login: github.Ptr("alice")},
303-
}),
304300
}
305301
if repoStatus != 0 && repoStatus != http.StatusOK {
306302
handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom")
@@ -374,7 +370,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
374370
require.NotNil(t, result.Meta)
375371
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
376372
assert.Equal(t, "untrusted", ifcMap["integrity"])
377-
assert.Equal(t, []any{"octocat", "alice"}, ifcMap["confidentiality"])
373+
assert.Equal(t, []any{"private"}, ifcMap["confidentiality"])
378374
})
379375

380376
t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
@@ -829,23 +825,17 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
829825
}
830826

831827
type repoFixture struct {
832-
owner string
833-
repo string
834-
isPrivate bool
835-
collaborators []string
836-
repoStatus int
837-
collaboratorsStatus int
828+
owner string
829+
repo string
830+
isPrivate bool
831+
repoStatus int
838832
}
839833

840834
repoHandlers := func(repos []repoFixture) map[string]http.HandlerFunc {
841835
repoByPath := map[string]repoFixture{}
842836
for _, r := range repos {
843837
repoByPath["/repos/"+r.owner+"/"+r.repo] = r
844838
}
845-
collaboratorsByPath := map[string]repoFixture{}
846-
for _, r := range repos {
847-
collaboratorsByPath["/repos/"+r.owner+"/"+r.repo+"/collaborators"] = r
848-
}
849839
return map[string]http.HandlerFunc{
850840
GetReposByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) {
851841
r, ok := repoByPath[req.URL.Path]
@@ -864,25 +854,6 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
864854
w.WriteHeader(http.StatusOK)
865855
_, _ = w.Write(body)
866856
},
867-
GetReposCollaboratorsByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) {
868-
r, ok := collaboratorsByPath[req.URL.Path]
869-
if !ok {
870-
w.WriteHeader(http.StatusOK)
871-
_, _ = w.Write([]byte("[]"))
872-
return
873-
}
874-
if r.collaboratorsStatus != 0 && r.collaboratorsStatus != http.StatusOK {
875-
w.WriteHeader(r.collaboratorsStatus)
876-
return
877-
}
878-
users := make([]*github.User, len(r.collaborators))
879-
for i, login := range r.collaborators {
880-
users[i] = &github.User{Login: github.Ptr(login)}
881-
}
882-
body, _ := json.Marshal(users)
883-
w.WriteHeader(http.StatusOK)
884-
_, _ = w.Write(body)
885-
},
886857
}
887858
}
888859

@@ -909,7 +880,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
909880
assert.Nil(t, result.Meta)
910881
})
911882

912-
t.Run("insiders mode enabled with single public repo emits public untrusted", func(t *testing.T) {
883+
t.Run("insiders mode all public emits public untrusted", func(t *testing.T) {
913884
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}}
914885
deps := BaseDeps{
915886
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})),
@@ -928,14 +899,14 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
928899
assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
929900
})
930901

931-
t.Run("insiders mode mixed public and private keeps the private readers", func(t *testing.T) {
902+
t.Run("insiders mode mixed public and private emits private untrusted", func(t *testing.T) {
932903
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{
933904
makeIssue("octocat", "private-repo", 1),
934905
makeIssue("octocat", "public-repo", 2),
935906
}}
936907
deps := BaseDeps{
937908
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
938-
{owner: "octocat", repo: "private-repo", isPrivate: true, collaborators: []string{"alice"}},
909+
{owner: "octocat", repo: "private-repo", isPrivate: true},
939910
{owner: "octocat", repo: "public-repo"},
940911
})),
941912
Flags: FeatureFlags{InsidersMode: true},
@@ -950,32 +921,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
950921
require.NotNil(t, result.Meta)
951922
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
952923
assert.Equal(t, "untrusted", ifcMap["integrity"])
953-
assert.Equal(t, []any{"alice"}, ifcMap["confidentiality"])
954-
})
955-
956-
t.Run("insiders mode two private repos intersect collaborators", func(t *testing.T) {
957-
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{
958-
makeIssue("octocat", "repo-a", 1),
959-
makeIssue("octocat", "repo-b", 2),
960-
}}
961-
deps := BaseDeps{
962-
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
963-
{owner: "octocat", repo: "repo-a", isPrivate: true, collaborators: []string{"alice", "bob", "carol"}},
964-
{owner: "octocat", repo: "repo-b", isPrivate: true, collaborators: []string{"bob", "carol", "dan"}},
965-
})),
966-
Flags: FeatureFlags{InsidersMode: true},
967-
}
968-
handler := serverTool.Handler(deps)
969-
970-
request := createMCPRequest(reqParams)
971-
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
972-
require.NoError(t, err)
973-
require.False(t, result.IsError)
974-
975-
require.NotNil(t, result.Meta)
976-
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
977-
assert.Equal(t, "untrusted", ifcMap["integrity"])
978-
assert.Equal(t, []any{"bob", "carol"}, ifcMap["confidentiality"])
924+
assert.Equal(t, []any{"private"}, ifcMap["confidentiality"])
979925
})
980926

981927
t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
@@ -999,27 +945,6 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
999945
}
1000946
})
1001947

1002-
t.Run("insiders mode skips ifc label when collaborators lookup fails", func(t *testing.T) {
1003-
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "private-repo", 1)}}
1004-
deps := BaseDeps{
1005-
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
1006-
{owner: "octocat", repo: "private-repo", isPrivate: true, collaboratorsStatus: http.StatusInternalServerError},
1007-
})),
1008-
Flags: FeatureFlags{InsidersMode: true},
1009-
}
1010-
handler := serverTool.Handler(deps)
1011-
1012-
request := createMCPRequest(reqParams)
1013-
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1014-
require.NoError(t, err)
1015-
require.False(t, result.IsError, "tool call should still succeed when collaborators lookup fails")
1016-
1017-
if result.Meta != nil {
1018-
_, hasIFC := result.Meta["ifc"]
1019-
assert.False(t, hasIFC, "ifc label should be omitted when collaborators lookup fails")
1020-
}
1021-
})
1022-
1023948
t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) {
1024949
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{}}
1025950
deps := BaseDeps{
@@ -1810,18 +1735,10 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
18101735
assert.Equal(t, "public", confList[0])
18111736
})
18121737

1813-
t.Run("insiders mode enabled on private repo emits private untrusted label with collaborators", func(t *testing.T) {
1738+
t.Run("insiders mode enabled on private repo emits private untrusted label", func(t *testing.T) {
18141739
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
18151740
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1816-
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1817-
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
1818-
{Login: github.Ptr("octocat")},
1819-
{Login: github.Ptr("alice")},
1820-
{Login: github.Ptr("bob")},
1821-
}),
1822-
}))
18231741
deps := BaseDeps{
1824-
Client: restClient,
18251742
GQLClient: gqlClient,
18261743
Flags: FeatureFlags{InsidersMode: true},
18271744
}
@@ -1844,34 +1761,7 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
18441761
assert.Equal(t, "untrusted", ifcMap["integrity"])
18451762
confList, ok := ifcMap["confidentiality"].([]any)
18461763
require.True(t, ok, "confidentiality should be a list")
1847-
assert.Equal(t, []any{"octocat", "alice", "bob"}, confList)
1848-
})
1849-
1850-
t.Run("insiders mode enabled on private repo falls back to owner when collaborators lookup fails", func(t *testing.T) {
1851-
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
1852-
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1853-
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1854-
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"),
1855-
}))
1856-
deps := BaseDeps{
1857-
Client: restClient,
1858-
GQLClient: gqlClient,
1859-
Flags: FeatureFlags{InsidersMode: true},
1860-
}
1861-
handler := serverTool.Handler(deps)
1862-
1863-
request := createMCPRequest(reqParams)
1864-
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1865-
require.NoError(t, err)
1866-
require.False(t, result.IsError)
1867-
1868-
require.NotNil(t, result.Meta)
1869-
ifcJSON, err := json.Marshal(result.Meta["ifc"])
1870-
require.NoError(t, err)
1871-
var ifcMap map[string]any
1872-
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
1873-
1874-
assert.Equal(t, []any{"octocat"}, ifcMap["confidentiality"])
1764+
assert.Equal(t, []any{"private"}, confList)
18751765
})
18761766
}
18771767

pkg/github/repositories.go

Lines changed: 7 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -769,17 +769,15 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool
769769
}
770770

771771
// attachIFC adds the IFC label to a successful tool result when
772-
// InsidersMode is enabled. The visibility and (for private
773-
// repositories) collaborators lookups are performed lazily on
774-
// first use. If the visibility lookup fails we skip the label
775-
// rather than misclassify the result; the failure is not cached
776-
// so a later return path can retry. If only the collaborators
777-
// lookup fails for a private repo we fall back to the owner so
778-
// the reader set is never empty.
772+
// InsidersMode is enabled. The visibility lookup is performed
773+
// lazily on first use and cached because GetFileContents has
774+
// many possible return paths and would otherwise re-fetch on
775+
// each. If the visibility lookup fails we skip the label rather
776+
// than misclassify the result; the failure is not cached so a
777+
// later return path can retry.
779778
var (
780779
ifcLabelKnown bool
781780
ifcIsPrivate bool
782-
ifcReaders []string
783781
)
784782
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
785783
if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode {
@@ -791,20 +789,12 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool
791789
return r
792790
}
793791
ifcIsPrivate = isPrivate
794-
if ifcIsPrivate {
795-
if collaborators, err := FetchRepoCollaborators(ctx, client, owner, repo); err == nil {
796-
ifcReaders = collaborators
797-
}
798-
if len(ifcReaders) == 0 {
799-
ifcReaders = []string{owner}
800-
}
801-
}
802792
ifcLabelKnown = true
803793
}
804794
if r.Meta == nil {
805795
r.Meta = mcp.Meta{}
806796
}
807-
r.Meta["ifc"] = ifc.LabelGetFileContents(ifcIsPrivate, ifcReaders)
797+
r.Meta["ifc"] = ifc.LabelGetFileContents(ifcIsPrivate)
808798
return r
809799
}
810800

pkg/github/repositories_test.go

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -492,10 +492,6 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) {
492492
"default_branch": "main",
493493
"private": isPrivate,
494494
}),
495-
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
496-
{Login: github.Ptr("octocat")},
497-
{Login: github.Ptr("alice")},
498-
}),
499495
GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) {
500496
w.WriteHeader(http.StatusOK)
501497
encodedContent := base64.StdEncoding.EncodeToString(mockRawContent)
@@ -588,7 +584,7 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) {
588584
assert.Equal(t, "trusted", ifcMap["integrity"])
589585
confList, ok := ifcMap["confidentiality"].([]any)
590586
require.True(t, ok, "confidentiality should be a list")
591-
assert.Equal(t, []any{"octocat", "alice"}, confList)
587+
assert.Equal(t, []any{"private"}, confList)
592588
})
593589

594590
t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {

0 commit comments

Comments
 (0)