Skip to content

Replace ingress IFC reader list with private marker #2478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JoannaaKL merged 4 commits into from
May 14, 2026
+95 −501
Merged

Replace ingress IFC reader list with private marker #2478

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ba21d0b
Replace ingress IFC reader list with private marker
gokhanarkan May 14, 2026
d0b2222
format
gokhanarkan May 14, 2026
e17cf37
Update FetchRepoCollaborators doc comment for marker-only ingress
gokhanarkan May 14, 2026
1876d20
Address review: drop FetchRepoCollaborators and make confidentiality …
gokhanarkan May 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions pkg/github/context_tools_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,10 +192,7 @@ func Test_GetMe_IFC_InsidersMode(t *testing.T) {
require.NoError(t, err)

assert.Equal(t, "trusted", ifcMap["integrity"])
confList, ok := ifcMap["confidentiality"].([]any)
require.True(t, ok, "confidentiality should be a list")
require.Len(t, confList, 1)
assert.Equal(t, "public", confList[0])
assert.Equal(t, "public", ifcMap["confidentiality"])
})
}

Expand Down
1 change: 0 additions & 1 deletion pkg/github/helper_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const (
GetReposByOwnerByRepo = "GET /repos/{owner}/{repo}"
GetReposBranchesByOwnerByRepo = "GET /repos/{owner}/{repo}/branches"
GetReposTagsByOwnerByRepo = "GET /repos/{owner}/{repo}/tags"
GetReposCollaboratorsByOwnerByRepo = "GET /repos/{owner}/{repo}/collaborators"
GetReposCommitsByOwnerByRepo = "GET /repos/{owner}/{repo}/commits"
GetReposCommitsByOwnerByRepoByRef = "GET /repos/{owner}/{repo}/commits/{ref}"
GetReposContentsByOwnerByRepoByPath = "GET /repos/{owner}/{repo}/contents/{path}"
Expand Down
54 changes: 4 additions & 50 deletions pkg/github/issues.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,11 +298,7 @@ Options are:

// attachIFC adds the IFC label to a successful tool result when
// InsidersMode is enabled. If the visibility lookup fails the
// label is omitted rather than misclassifying the result. If
// only the collaborators lookup fails for a private repo we
// fall back to the owner so the reader set is never empty. The
// label matches list_issues semantics: per-repo visibility,
// integrity always untrusted.
// label is omitted rather than misclassifying the result.
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode {
return r
Expand All @@ -311,19 +307,10 @@ Options are:
if err != nil {
return r
}
var readers []string
if isPrivate {
if collaborators, err := FetchRepoCollaborators(ctx, client, owner, repo); err == nil {
readers = collaborators
}
if len(readers) == 0 {
readers = []string{owner}
}
}
if r.Meta == nil {
r.Meta = mcp.Meta{}
}
r.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
r.Meta["ifc"] = ifc.LabelListIssues(isPrivate)
return r
}

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

uniqueRepos := uniqueSearchIssuesRepos(result)
visibilities := make([]bool, 0, len(uniqueRepos))
readerSets := make([][]string, 0, len(uniqueRepos))
for _, r := range uniqueRepos {
isPrivate, err := FetchRepoIsPrivate(ctx, client, r.owner, r.repo)
if err != nil {
return
}
visibilities = append(visibilities, isPrivate)
if !isPrivate {
readerSets = append(readerSets, nil)
continue
}
collaborators, err := FetchRepoCollaborators(ctx, client, r.owner, r.repo)
if err != nil {
return
}
// Preserve an empty collaborator set as-is. Substituting the
// owner here would corrupt the cross-repo intersection (the
// owner could appear in another repo's collaborator list and
// widen the joined reader set incorrectly).
readerSets = append(readerSets, collaborators)
}

label, ok := ifc.LabelSearchIssues(visibilities, readerSets)
if !ok {
return
}
if callResult.Meta == nil {
callResult.Meta = mcp.Meta{}
}
callResult.Meta["ifc"] = label
callResult.Meta["ifc"] = ifc.LabelSearchIssues(visibilities)
}
}

Expand Down Expand Up @@ -1728,22 +1697,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
if result.Meta == nil {
result.Meta = mcp.Meta{}
}
var readers []string
if isPrivate {
restClient, err := deps.GetClient(ctx)
if err == nil {
if collaborators, err := FetchRepoCollaborators(ctx, restClient, owner, repo); err == nil {
readers = collaborators
}
}
// Fall back to the repository owner so the reader set is
// never empty for a private repository even if the
// collaborators lookup fails.
if len(readers) == 0 {
readers = []string{owner}
}
}
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate)
}
return result, nil, nil
})
Expand Down
145 changes: 15 additions & 130 deletions pkg/github/issues_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,10 +297,6 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
handlers := map[string]http.HandlerFunc{
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue),
GetReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockComments),
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
{Login: github.Ptr("octocat")},
{Login: github.Ptr("alice")},
}),
}
if repoStatus != 0 && repoStatus != http.StatusOK {
handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom")
Expand Down Expand Up @@ -356,7 +352,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
assert.Equal(t, "public", ifcMap["confidentiality"])
})

t.Run("insiders mode enabled on private repo with get_comments emits private untrusted", func(t *testing.T) {
Expand All @@ -374,7 +370,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, []any{"octocat", "alice"}, ifcMap["confidentiality"])
assert.Equal(t, "private", ifcMap["confidentiality"])
})

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

type repoFixture struct {
owner string
repo string
isPrivate bool
collaborators []string
repoStatus int
collaboratorsStatus int
owner string
repo string
isPrivate bool
repoStatus int
}

repoHandlers := func(repos []repoFixture) map[string]http.HandlerFunc {
repoByPath := map[string]repoFixture{}
for _, r := range repos {
repoByPath["/repos/"+r.owner+"/"+r.repo] = r
}
collaboratorsByPath := map[string]repoFixture{}
for _, r := range repos {
collaboratorsByPath["/repos/"+r.owner+"/"+r.repo+"/collaborators"] = r
}
return map[string]http.HandlerFunc{
GetReposByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) {
r, ok := repoByPath[req.URL.Path]
Expand All @@ -864,25 +854,6 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
w.WriteHeader(http.StatusOK)
_, _ = w.Write(body)
},
GetReposCollaboratorsByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) {
r, ok := collaboratorsByPath[req.URL.Path]
if !ok {
w.WriteHeader(http.StatusOK)
_, _ = w.Write([]byte("[]"))
return
}
if r.collaboratorsStatus != 0 && r.collaboratorsStatus != http.StatusOK {
w.WriteHeader(r.collaboratorsStatus)
return
}
users := make([]*github.User, len(r.collaborators))
for i, login := range r.collaborators {
users[i] = &github.User{Login: github.Ptr(login)}
}
body, _ := json.Marshal(users)
w.WriteHeader(http.StatusOK)
_, _ = w.Write(body)
},
}
}

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

t.Run("insiders mode enabled with single public repo emits public untrusted", func(t *testing.T) {
t.Run("insiders mode all public emits public untrusted", func(t *testing.T) {
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}}
deps := BaseDeps{
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})),
Expand All @@ -925,17 +896,17 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
assert.Equal(t, "public", ifcMap["confidentiality"])
})

t.Run("insiders mode mixed public and private keeps the private readers", func(t *testing.T) {
t.Run("insiders mode mixed public and private emits private untrusted", func(t *testing.T) {
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{
makeIssue("octocat", "private-repo", 1),
makeIssue("octocat", "public-repo", 2),
}}
deps := BaseDeps{
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
{owner: "octocat", repo: "private-repo", isPrivate: true, collaborators: []string{"alice"}},
{owner: "octocat", repo: "private-repo", isPrivate: true},
{owner: "octocat", repo: "public-repo"},
})),
Flags: FeatureFlags{InsidersMode: true},
Expand All @@ -950,32 +921,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, []any{"alice"}, ifcMap["confidentiality"])
})

t.Run("insiders mode two private repos intersect collaborators", func(t *testing.T) {
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{
makeIssue("octocat", "repo-a", 1),
makeIssue("octocat", "repo-b", 2),
}}
deps := BaseDeps{
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
{owner: "octocat", repo: "repo-a", isPrivate: true, collaborators: []string{"alice", "bob", "carol"}},
{owner: "octocat", repo: "repo-b", isPrivate: true, collaborators: []string{"bob", "carol", "dan"}},
})),
Flags: FeatureFlags{InsidersMode: true},
}
handler := serverTool.Handler(deps)

request := createMCPRequest(reqParams)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)

require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, []any{"bob", "carol"}, ifcMap["confidentiality"])
assert.Equal(t, "private", ifcMap["confidentiality"])
})

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

t.Run("insiders mode skips ifc label when collaborators lookup fails", func(t *testing.T) {
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "private-repo", 1)}}
deps := BaseDeps{
Client: github.NewClient(makeMockClient(searchResult, []repoFixture{
{owner: "octocat", repo: "private-repo", isPrivate: true, collaboratorsStatus: http.StatusInternalServerError},
})),
Flags: FeatureFlags{InsidersMode: true},
}
handler := serverTool.Handler(deps)

request := createMCPRequest(reqParams)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError, "tool call should still succeed when collaborators lookup fails")

if result.Meta != nil {
_, hasIFC := result.Meta["ifc"]
assert.False(t, hasIFC, "ifc label should be omitted when collaborators lookup fails")
}
})

t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) {
searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{}}
deps := BaseDeps{
Expand All @@ -1036,7 +961,7 @@ func Test_SearchIssues_IFC_InsidersMode(t *testing.T) {
require.NotNil(t, result.Meta)
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
assert.Equal(t, "public", ifcMap["confidentiality"])
})
}

Expand Down Expand Up @@ -1804,24 +1729,13 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))

assert.Equal(t, "untrusted", ifcMap["integrity"])
confList, ok := ifcMap["confidentiality"].([]any)
require.True(t, ok, "confidentiality should be a list")
require.Len(t, confList, 1)
assert.Equal(t, "public", confList[0])
assert.Equal(t, "public", ifcMap["confidentiality"])
})

t.Run("insiders mode enabled on private repo emits private untrusted label with collaborators", func(t *testing.T) {
t.Run("insiders mode enabled on private repo emits private untrusted label", func(t *testing.T) {
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
{Login: github.Ptr("octocat")},
{Login: github.Ptr("alice")},
{Login: github.Ptr("bob")},
}),
}))
deps := BaseDeps{
Client: restClient,
GQLClient: gqlClient,
Flags: FeatureFlags{InsidersMode: true},
}
Expand All @@ -1842,36 +1756,7 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))

assert.Equal(t, "untrusted", ifcMap["integrity"])
confList, ok := ifcMap["confidentiality"].([]any)
require.True(t, ok, "confidentiality should be a list")
assert.Equal(t, []any{"octocat", "alice", "bob"}, confList)
})

t.Run("insiders mode enabled on private repo falls back to owner when collaborators lookup fails", func(t *testing.T) {
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"),
}))
deps := BaseDeps{
Client: restClient,
GQLClient: gqlClient,
Flags: FeatureFlags{InsidersMode: true},
}
handler := serverTool.Handler(deps)

request := createMCPRequest(reqParams)
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
require.NoError(t, err)
require.False(t, result.IsError)

require.NotNil(t, result.Meta)
ifcJSON, err := json.Marshal(result.Meta["ifc"])
require.NoError(t, err)
var ifcMap map[string]any
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))

assert.Equal(t, []any{"octocat"}, ifcMap["confidentiality"])
assert.Equal(t, "private", ifcMap["confidentiality"])
})
}

Expand Down
Loading
Loading