ifc: don't label unpublished repo advisories as public

Repository security advisory listings can include draft/triage/closed advisories (via the state filter), which are not world-readable even on a public repository. Deriving confidentiality from repo visibility alone under-classified those results as public. LabelRepositorySecurityAdvisory now takes an allPublished flag and only returns a public label when the repo is public AND every returned advisory is published; otherwise it is private. list_repository_security_advisories computes allPublished from the response state; the org-wide listing stays private-untrusted. Adds unit + handler regression tests covering the draft-advisory-on-public-repo case. Addresses PR review feedback.
SamMorrowDrums · Jun 11, 2026 · Jun 11, 2026 · Jun 11, 2026 · Jun 11, 2026 · Jun 11, 2026
commit 2813f074e2202458e60aa35c0c5fe584c5300e92
@@ -314,9 +314,15 @@ func ListRepositorySecurityAdvisories(t translations.TranslationHelperFunc) inve
 			}
 
 			result := utils.NewToolResultText(string(r))
-			// Repository advisories carry externally authored prose (untrusted);
-			// confidentiality follows repo visibility.
-			result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepositorySecurityAdvisory)
+			// Repository advisories carry externally authored prose (untrusted).
+			// Confidentiality follows repo visibility, but draft/triage/closed
+			// advisories are not world-readable even on a public repo, so the
+			// result is only public when every returned advisory is published.
+			allPublished := allAdvisoriesPublished(advisories)
+			result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result,
+				func(isPrivate bool) ifc.SecurityLabel {
+					return ifc.LabelRepositorySecurityAdvisory(isPrivate, allPublished)
+				})
 			return result, nil, nil
 		},
 	)
@@ -476,9 +482,25 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i
 			result := utils.NewToolResultText(string(r))
 			// Org-wide advisory listings span the organization's repositories
 			// (including private ones) and are restricted to org members, so
-			// they are conservatively labeled private-untrusted.
-			result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepositorySecurityAdvisory(true))
+			// they are conservatively labeled private-untrusted (isPrivate=true,
+			// which forces private regardless of publication state).
+			result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepositorySecurityAdvisory(true, false))
 			return result, nil, nil
 		},
 	)
 }
+
+// allAdvisoriesPublished reports whether every advisory in the slice is in the
+// "published" state. Repository security advisories can also be in draft,
+// triage, or closed states, none of which are world-readable even on a public
+// repository. An empty slice is treated as published (true) since there is no
+// non-public content to protect. Used to decide whether a repository advisory
+// listing may carry a public confidentiality label.
+func allAdvisoriesPublished(advisories []*github.SecurityAdvisory) bool {
+	for _, advisory := range advisories {
+		if advisory.GetState() != "published" {
+			return false
+		}
+	}
+	return true
+}
@@ -10,6 +10,7 @@ import (
 	"github.com/github/github-mcp-server/pkg/translations"
 	"github.com/google/go-github/v87/github"
 	"github.com/google/jsonschema-go/jsonschema"
+	"github.com/modelcontextprotocol/go-sdk/mcp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -370,6 +371,132 @@ func Test_ListRepositorySecurityAdvisories(t *testing.T) {
 	}
 }
 
+// Test_ListRepositorySecurityAdvisories_IFC_FeatureFlag verifies the IFC label
+// attached to list_repository_security_advisories. The label is only present
+// when the ifc_labels feature flag is enabled, and — critically — confidentiality
+// is public only when the repository is public AND every returned advisory is
+// published. Draft/triage/closed advisories are not world-readable even on a
+// public repo, so a result containing one must be labeled private. This guards
+// against the under-classification raised in PR review.
+func Test_ListRepositorySecurityAdvisories_IFC_FeatureFlag(t *testing.T) {
+	t.Parallel()
+
+	toolDef := ListRepositorySecurityAdvisories(translations.NullTranslationHelper)
+
+	publishedAdvisory := &github.SecurityAdvisory{
+		GHSAID:  github.Ptr("GHSA-1111-1111-1111"),
+		Summary: github.Ptr("Published advisory"),
+		State:   github.Ptr("published"),
+	}
+	draftAdvisory := &github.SecurityAdvisory{
+		GHSAID:  github.Ptr("GHSA-2222-2222-2222"),
+		Summary: github.Ptr("Draft advisory"),
+		State:   github.Ptr("draft"),
+	}
+
+	makeMockClient := func(isPrivate bool, advisories []*github.SecurityAdvisory) *http.Client {
+		return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
+			GetReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusOK, advisories),
+			GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, map[string]any{
+				"name":    "repo",
+				"private": isPrivate,
+			}),
+		})
+	}
+
+	reqParams := map[string]any{
+		"owner": "owner",
+		"repo":  "repo",
+	}
+
+	readIFC := func(t *testing.T, result *mcp.CallToolResult) (map[string]any, bool) {
+		t.Helper()
+		if result.Meta == nil {
+			return nil, false
+		}
+		label, ok := result.Meta["ifc"]
+		if !ok {
+			return nil, false
+		}
+		labelJSON, err := json.Marshal(label)
+		require.NoError(t, err)
+		var labelMap map[string]any
+		require.NoError(t, json.Unmarshal(labelJSON, &labelMap))
+		return labelMap, true
+	}
+
+	t.Run("feature flag disabled omits ifc label", func(t *testing.T) {
+		t.Parallel()
+		deps := BaseDeps{Client: mustNewGHClient(t, makeMockClient(false, []*github.SecurityAdvisory{publishedAdvisory}))}
+		handler := toolDef.Handler(deps)
+
+		request := createMCPRequest(reqParams)
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		require.False(t, result.IsError)
+		assert.Nil(t, result.Meta, "result meta should be nil when IFC labels are disabled")
+	})
+
+	t.Run("public repo with only published advisories is public", func(t *testing.T) {
+		t.Parallel()
+		deps := BaseDeps{
+			Client:         mustNewGHClient(t, makeMockClient(false, []*github.SecurityAdvisory{publishedAdvisory})),
+			featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
+		}
+		handler := toolDef.Handler(deps)
+
+		request := createMCPRequest(reqParams)
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		require.False(t, result.IsError)
+
+		label, ok := readIFC(t, result)
+		require.True(t, ok, "result meta should contain ifc key")
+		assert.Equal(t, "untrusted", label["integrity"])
+		assert.Equal(t, "public", label["confidentiality"])
+	})
+
+	t.Run("public repo with a draft advisory is private", func(t *testing.T) {
+		t.Parallel()
+		// Reviewer scenario: a draft advisory on a public repo is not
+		// world-readable, so the label must not be public.
+		deps := BaseDeps{
+			Client:         mustNewGHClient(t, makeMockClient(false, []*github.SecurityAdvisory{publishedAdvisory, draftAdvisory})),
+			featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
+		}
+		handler := toolDef.Handler(deps)
+
+		request := createMCPRequest(reqParams)
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		require.False(t, result.IsError)
+
+		label, ok := readIFC(t, result)
+		require.True(t, ok, "result meta should contain ifc key")
+		assert.Equal(t, "untrusted", label["integrity"])
+		assert.Equal(t, "private", label["confidentiality"], "draft advisory on public repo must be private")
+	})
+
+	t.Run("private repo is private", func(t *testing.T) {
+		t.Parallel()
+		deps := BaseDeps{
+			Client:         mustNewGHClient(t, makeMockClient(true, []*github.SecurityAdvisory{publishedAdvisory})),
+			featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
+		}
+		handler := toolDef.Handler(deps)
+
+		request := createMCPRequest(reqParams)
+		result, err := handler(ContextWithDeps(context.Background(), deps), &request)
+		require.NoError(t, err)
+		require.False(t, result.IsError)
+
+		label, ok := readIFC(t, result)
+		require.True(t, ok, "result meta should contain ifc key")
+		assert.Equal(t, "untrusted", label["integrity"])
+		assert.Equal(t, "private", label["confidentiality"])
+	})
+}
+
 func Test_ListOrgRepositorySecurityAdvisories(t *testing.T) {
 	// Verify tool definition once
 	toolDef := ListOrgRepositorySecurityAdvisories(translations.NullTranslationHelper)

@@ -190,12 +190,17 @@ func LabelGlobalSecurityAdvisory() SecurityLabel {
 // LabelRepositorySecurityAdvisory returns the IFC label for repository- or
 // organization-scoped security advisories.
 //
-// Integrity is untrusted (externally authored advisory prose). Confidentiality
-// follows repository visibility: advisories on public repositories are
-// readable by anyone, while those on private repositories are restricted to
-// the repository's readers.
-func LabelRepositorySecurityAdvisory(isPrivate bool) SecurityLabel {
-	if isPrivate {
+// Integrity is untrusted (externally authored advisory prose).
+//
+// Confidentiality is public only when the repository is public AND every
+// advisory in the result is in the "published" state. Repository security
+// advisories also exist in draft, triage, and closed states; those are visible
+// only to maintainers and are NOT world-readable even on a public repository.
+// Treating any non-published advisory as private (allPublished == false)
+// prevents misclassifying an unpublished advisory from a public repo as
+// public-readable. Private repositories are always private regardless of state.
+func LabelRepositorySecurityAdvisory(isPrivate bool, allPublished bool) SecurityLabel {
+	if isPrivate || !allPublished {
 		return PrivateUntrusted()
 	}
 	return PublicUntrusted()

@@ -122,16 +122,32 @@ func TestLabelGlobalSecurityAdvisory(t *testing.T) {
 func TestLabelRepositorySecurityAdvisory(t *testing.T) {
 	t.Parallel()
 
-	t.Run("public repo advisory is untrusted and public", func(t *testing.T) {
+	t.Run("public repo with all published advisories is untrusted and public", func(t *testing.T) {
 		t.Parallel()
-		label := LabelRepositorySecurityAdvisory(false)
+		label := LabelRepositorySecurityAdvisory(false, true)
 		assert.Equal(t, IntegrityUntrusted, label.Integrity)
 		assert.Equal(t, ConfidentialityPublic, label.Confidentiality)
 	})
 
+	t.Run("public repo with an unpublished advisory is untrusted and private", func(t *testing.T) {
+		t.Parallel()
+		// draft/triage/closed advisories are not world-readable even on a
+		// public repo, so confidentiality must be private.
+		label := LabelRepositorySecurityAdvisory(false, false)
+		assert.Equal(t, IntegrityUntrusted, label.Integrity)
+		assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
+	})
+
 	t.Run("private repo advisory is untrusted and private", func(t *testing.T) {
 		t.Parallel()
-		label := LabelRepositorySecurityAdvisory(true)
+		label := LabelRepositorySecurityAdvisory(true, true)
+		assert.Equal(t, IntegrityUntrusted, label.Integrity)
+		assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
+	})
+
+	t.Run("private repo with unpublished advisory is untrusted and private", func(t *testing.T) {
+		t.Parallel()
+		label := LabelRepositorySecurityAdvisory(true, false)
 		assert.Equal(t, IntegrityUntrusted, label.Integrity)
 		assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
 	})