Address review: drop FetchRepoCollaborators and make confidentiality a scalar

Per Joanna's review on #2478:

- Remove FetchRepoCollaborators entirely (no callers left after the marker
  switch). Drops the GetReposCollaboratorsByOwnerByRepo mock route too.
- Change SecurityLabel.Confidentiality from []Confidentiality to a scalar
  Confidentiality. Wire format is now {integrity, confidentiality} where
  confidentiality is a single 'public' or 'private' string. Updated all
  tests and the LabelSearchIssues helper accordingly.

Author: gokhanarkan

pkg/github/context_tools_test.go

@@ -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"])
 	})
 }
 

pkg/github/helper_test.go

@@ -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}"

pkg/github/issues_test.go

@@ -352,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) {
@@ -370,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{"private"}, ifcMap["confidentiality"])
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
@@ -896,7 +896,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"])
 	})
 
 	t.Run("insiders mode mixed public and private emits private untrusted", func(t *testing.T) {
@@ -921,7 +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{"private"}, ifcMap["confidentiality"])
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
@@ -961,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"])
 	})
 }
 
@@ -1729,10 +1729,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")
-		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", func(t *testing.T) {
@@ -1759,9 +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{"private"}, confList)
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 }
 

pkg/github/repositories.go

@@ -654,38 +654,6 @@ func CreateRepository(t translations.TranslationHelperFunc) inventory.ServerTool
 	)
 }
 
-// FetchRepoCollaborators returns the login names of all collaborators on a
-// repository, paginated.
-//
-// The github-mcp-server itself no longer calls this from its IFC label paths:
-// ingress tools emit a single "private" marker and the client engine resolves
-// concrete readers from the GitHub API on demand at egress decision time
-// (where it can paginate + cache). The helper is retained as an exported
-// utility for external library consumers that may want the same one-shot
-// pagination behaviour.
-func FetchRepoCollaborators(ctx context.Context, client *github.Client, owner, repo string) ([]string, error) {
-	opts := &github.ListCollaboratorsOptions{
-		ListOptions: github.ListOptions{PerPage: 100},
-	}
-	var logins []string
-	for {
-		page, resp, err := client.Repositories.ListCollaborators(ctx, owner, repo, opts)
-		if err != nil {
-			return nil, err
-		}
-		for _, c := range page {
-			if login := c.GetLogin(); login != "" {
-				logins = append(logins, login)
-			}
-		}
-		if resp == nil || resp.NextPage == 0 {
-			break
-		}
-		opts.Page = resp.NextPage
-	}
-	return logins, nil
-}
-
 // FetchRepoIsPrivate returns whether a repository is private. It is a thin
 // wrapper around the GitHub Repositories.Get endpoint provided as a shared
 // helper for IFC label computation across tools.

pkg/github/repositories_test.go

@@ -554,10 +554,7 @@ func Test_GetFileContents_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 trusted label", func(t *testing.T) {
@@ -582,9 +579,7 @@ func Test_GetFileContents_IFC_InsidersMode(t *testing.T) {
 		require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
 
 		assert.Equal(t, "trusted", ifcMap["integrity"])
-		confList, ok := ifcMap["confidentiality"].([]any)
-		require.True(t, ok, "confidentiality should be a list")
-		assert.Equal(t, []any{"private"}, confList)
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {

pkg/github/search_test.go

@@ -235,7 +235,7 @@ func Test_SearchRepositories_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 any private match emits private untrusted", func(t *testing.T) {
@@ -256,7 +256,7 @@ func Test_SearchRepositories_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{"private"}, ifcMap["confidentiality"])
+		assert.Equal(t, "private", ifcMap["confidentiality"])
 	})
 
 	t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) {
@@ -274,7 +274,7 @@ func Test_SearchRepositories_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"])
 	})
 }
 

pkg/ifc/ifc.go

@@ -18,23 +18,23 @@ const (
 )
 
 type SecurityLabel struct {
-	Integrity       Integrity         `json:"integrity"`
-	Confidentiality []Confidentiality `json:"confidentiality"`
+	Integrity       Integrity       `json:"integrity"`
+	Confidentiality Confidentiality `json:"confidentiality"`
 }
 
 // PublicTrusted returns a label for trusted, publicly readable data.
 func PublicTrusted() SecurityLabel {
 	return SecurityLabel{
 		Integrity:       IntegrityTrusted,
-		Confidentiality: []Confidentiality{ConfidentialityPublic},
+		Confidentiality: ConfidentialityPublic,
 	}
 }
 
 // PublicUntrusted returns a label for untrusted, publicly readable data.
 func PublicUntrusted() SecurityLabel {
 	return SecurityLabel{
 		Integrity:       IntegrityUntrusted,
-		Confidentiality: []Confidentiality{ConfidentialityPublic},
+		Confidentiality: ConfidentialityPublic,
 	}
 }
 
@@ -45,7 +45,7 @@ func PublicUntrusted() SecurityLabel {
 func PrivateTrusted() SecurityLabel {
 	return SecurityLabel{
 		Integrity:       IntegrityTrusted,
-		Confidentiality: []Confidentiality{ConfidentialityPrivate},
+		Confidentiality: ConfidentialityPrivate,
 	}
 }
 
@@ -55,7 +55,7 @@ func PrivateTrusted() SecurityLabel {
 func PrivateUntrusted() SecurityLabel {
 	return SecurityLabel{
 		Integrity:       IntegrityUntrusted,
-		Confidentiality: []Confidentiality{ConfidentialityPrivate},
+		Confidentiality: ConfidentialityPrivate,
 	}
 }
 

pkg/ifc/ifc_test.go

@@ -45,7 +45,7 @@ func TestLabelSearchIssues(t *testing.T) {
 			t.Parallel()
 			label := LabelSearchIssues(tc.visibilities)
 			assert.Equal(t, IntegrityUntrusted, label.Integrity)
-			assert.Equal(t, []Confidentiality{tc.wantConfidential}, label.Confidentiality)
+			assert.Equal(t, tc.wantConfidential, label.Confidentiality)
 		})
 	}
 }