Skip to content

Commit 6d9f188

Browse files
committed
list_issues: populate readers with repo collaborators
Addresses Joanna's review feedback: for private repositories, populate the IFC confidentiality reader set with the repository's collaborator logins instead of the [owner] placeholder. Adds an exported FetchRepoCollaborators helper in pkg/github/repositories.go that paginates through Repositories.ListCollaborators. Mirrors the helper in github-mcp-server-remote (without the cache for now; cache can land in a follow-up). The lookup is invoked only for private repos under InsidersMode; if it fails we fall back to [owner] so the reader set is never empty for a private repo.
1 parent 9366658 commit 6d9f188

4 files changed

Lines changed: 78 additions & 8 deletions

File tree

pkg/github/helper_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ const (
3131
GetReposByOwnerByRepo = "GET /repos/{owner}/{repo}"
3232
GetReposBranchesByOwnerByRepo = "GET /repos/{owner}/{repo}/branches"
3333
GetReposTagsByOwnerByRepo = "GET /repos/{owner}/{repo}/tags"
34+
GetReposCollaboratorsByOwnerByRepo = "GET /repos/{owner}/{repo}/collaborators"
3435
GetReposCommitsByOwnerByRepo = "GET /repos/{owner}/{repo}/commits"
3536
GetReposCommitsByOwnerByRepoByRef = "GET /repos/{owner}/{repo}/commits/{ref}"
3637
GetReposContentsByOwnerByRepoByPath = "GET /repos/{owner}/{repo}/contents/{path}"

pkg/github/issues.go

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1595,13 +1595,20 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
15951595
if result.Meta == nil {
15961596
result.Meta = mcp.Meta{}
15971597
}
1598-
// TODO(fides): for private repos, populate readers with the
1599-
// repository's collaborator logins. Using [owner] as a
1600-
// placeholder until a shared visibility/collaborators helper
1601-
// lands (tracked under copilot-mcp-core#1623).
16021598
var readers []string
16031599
if isPrivate {
1604-
readers = []string{owner}
1600+
restClient, err := deps.GetClient(ctx)
1601+
if err == nil {
1602+
if collaborators, err := FetchRepoCollaborators(ctx, restClient, owner, repo); err == nil {
1603+
readers = collaborators
1604+
}
1605+
}
1606+
// Fall back to the repository owner so the reader set is
1607+
// never empty for a private repository even if the
1608+
// collaborators lookup fails.
1609+
if len(readers) == 0 {
1610+
readers = []string{owner}
1611+
}
16051612
}
16061613
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
16071614
}

pkg/github/issues_test.go

Lines changed: 37 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1453,10 +1453,18 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
14531453
assert.Equal(t, "public", confList[0])
14541454
})
14551455

1456-
t.Run("insiders mode enabled on private repo emits private untrusted label", func(t *testing.T) {
1456+
t.Run("insiders mode enabled on private repo emits private untrusted label with collaborators", func(t *testing.T) {
14571457
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
14581458
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1459+
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1460+
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
1461+
{Login: github.Ptr("octocat")},
1462+
{Login: github.Ptr("alice")},
1463+
{Login: github.Ptr("bob")},
1464+
}),
1465+
}))
14591466
deps := BaseDeps{
1467+
Client: restClient,
14601468
GQLClient: gqlClient,
14611469
Flags: FeatureFlags{InsidersMode: true},
14621470
}
@@ -1479,8 +1487,34 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
14791487
assert.Equal(t, "untrusted", ifcMap["integrity"])
14801488
confList, ok := ifcMap["confidentiality"].([]any)
14811489
require.True(t, ok, "confidentiality should be a list")
1482-
require.Len(t, confList, 1)
1483-
assert.Equal(t, "octocat", confList[0])
1490+
assert.Equal(t, []any{"octocat", "alice", "bob"}, confList)
1491+
})
1492+
1493+
t.Run("insiders mode enabled on private repo falls back to owner when collaborators lookup fails", func(t *testing.T) {
1494+
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
1495+
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
1496+
restClient := github.NewClient(MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
1497+
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"),
1498+
}))
1499+
deps := BaseDeps{
1500+
Client: restClient,
1501+
GQLClient: gqlClient,
1502+
Flags: FeatureFlags{InsidersMode: true},
1503+
}
1504+
handler := serverTool.Handler(deps)
1505+
1506+
request := createMCPRequest(reqParams)
1507+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
1508+
require.NoError(t, err)
1509+
require.False(t, result.IsError)
1510+
1511+
require.NotNil(t, result.Meta)
1512+
ifcJSON, err := json.Marshal(result.Meta["ifc"])
1513+
require.NoError(t, err)
1514+
var ifcMap map[string]any
1515+
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
1516+
1517+
assert.Equal(t, []any{"octocat"}, ifcMap["confidentiality"])
14841518
})
14851519
}
14861520

pkg/github/repositories.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,34 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool
653653
)
654654
}
655655

656+
// FetchRepoCollaborators returns the login names of all collaborators on a
657+
// repository. It is provided as a shared helper for IFC label computation so
658+
// tools can populate the reader set for private repositories. The full list
659+
// is fetched eagerly via pagination; callers are expected to invoke this only
660+
// when needed (e.g. private repos under InsidersMode).
661+
func FetchRepoCollaborators(ctx context.Context, client *github.Client, owner, repo string) ([]string, error) {
662+
opts := &github.ListCollaboratorsOptions{
663+
ListOptions: github.ListOptions{PerPage: 100},
664+
}
665+
var logins []string
666+
for {
667+
page, resp, err := client.Repositories.ListCollaborators(ctx, owner, repo, opts)
668+
if err != nil {
669+
return nil, err
670+
}
671+
for _, c := range page {
672+
if login := c.GetLogin(); login != "" {
673+
logins = append(logins, login)
674+
}
675+
}
676+
if resp == nil || resp.NextPage == 0 {
677+
break
678+
}
679+
opts.Page = resp.NextPage
680+
}
681+
return logins, nil
682+
}
683+
656684
// GetFileContents creates a tool to get the contents of a file or directory from a GitHub repository.
657685
func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool {
658686
return NewTool(

0 commit comments

Comments
 (0)