Skip to content

Commit fcffda3

Browse files
committed
ifc: fix confidentiality under-classification in releases, collaborators, get_me
Audit for the same bug class as the repo-advisory fix (confidentiality derived from a coarse signal that misses access-restricted items) found three more under-classifications: - Releases (list_releases, get_latest_release, get_release_by_tag): draft releases are visible only to push-access users and are not world-readable even on a public repo. New LabelRelease(isPrivate, hasDraft) returns public only for a non-draft release on a public repo; handlers compute hasDraft from the response (Draft flag / per-item scan). - list_repository_collaborators: a collaborator roster requires push access to list, so it is never world-readable, not even on a public repo. New LabelCollaboratorRoster() is always PrivateTrusted (mirrors LabelTeam), replacing the repo-visibility-derived label. - get_me: the result includes private_gists / total_private_repos / owned_private_repos, which are not part of the public profile. LabelGetMe is now PrivateTrusted instead of PublicTrusted. Verified the remaining public-capable labels are sound: Actions logs are world-readable on public repos; branches/tags are public metadata; gist, project, search, and starred-repo labels read per-item visibility and join. Adds ifc unit tests for the new/changed labels and a get_release_by_tag handler regression test (draft on public repo -> private); updates the get_me handler test to assert private.
1 parent 2813f07 commit fcffda3

5 files changed

Lines changed: 230 additions & 11 deletions

File tree

pkg/github/context_tools_test.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ func Test_GetMe_IFC_FeatureFlag(t *testing.T) {
199199
require.NoError(t, err)
200200

201201
assert.Equal(t, "trusted", ifcMap["integrity"])
202-
assert.Equal(t, "public", ifcMap["confidentiality"])
202+
// get_me returns the caller's private repo/gist counts, which are not
203+
// part of the public profile, so confidentiality is private.
204+
assert.Equal(t, "private", ifcMap["confidentiality"])
203205
})
204206
}
205207

pkg/github/repositories.go

Lines changed: 33 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1806,8 +1806,21 @@ func ListReleases(t translations.TranslationHelperFunc) inventory.ServerTool {
18061806

18071807
result := utils.NewToolResultText(string(r))
18081808
// Releases are published by collaborators with push access, so
1809-
// integrity is trusted. Confidentiality follows repo visibility.
1810-
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata)
1809+
// integrity is trusted. Confidentiality follows repo visibility,
1810+
// but draft releases are visible only to push-access users and are
1811+
// not world-readable even on a public repo, so the result is only
1812+
// public when no returned release is a draft.
1813+
hasDraft := false
1814+
for _, mr := range minimalReleases {
1815+
if mr.Draft {
1816+
hasDraft = true
1817+
break
1818+
}
1819+
}
1820+
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result,
1821+
func(isPrivate bool) ifc.SecurityLabel {
1822+
return ifc.LabelRelease(isPrivate, hasDraft)
1823+
})
18111824
return result, nil, nil
18121825
},
18131826
)
@@ -1876,8 +1889,13 @@ func GetLatestRelease(t translations.TranslationHelperFunc) inventory.ServerTool
18761889

18771890
result := utils.NewToolResultText(string(r))
18781891
// Releases are published by collaborators with push access, so
1879-
// integrity is trusted. Confidentiality follows repo visibility.
1880-
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata)
1892+
// integrity is trusted. The "latest release" endpoint never returns
1893+
// a draft, but the draft flag is honored defensively: a draft is
1894+
// not world-readable even on a public repo.
1895+
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result,
1896+
func(isPrivate bool) ifc.SecurityLabel {
1897+
return ifc.LabelRelease(isPrivate, release.GetDraft())
1898+
})
18811899
return result, nil, nil
18821900
},
18831901
)
@@ -1957,8 +1975,13 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool
19571975

19581976
result := utils.NewToolResultText(string(r))
19591977
// Releases are published by collaborators with push access, so
1960-
// integrity is trusted. Confidentiality follows repo visibility.
1961-
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata)
1978+
// integrity is trusted. A release fetched by tag may be a draft,
1979+
// which is visible only to push-access users and not world-readable
1980+
// even on a public repo, so a draft forces private confidentiality.
1981+
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result,
1982+
func(isPrivate bool) ifc.SecurityLabel {
1983+
return ifc.LabelRelease(isPrivate, release.GetDraft())
1984+
})
19621985
return result, nil, nil
19631986
},
19641987
)
@@ -2343,9 +2366,10 @@ func ListRepositoryCollaborators(t translations.TranslationHelperFunc) inventory
23432366

23442367
callResult := MarshalledTextResult(response)
23452368
// The collaborator roster is GitHub-maintained membership data
2346-
// (trusted, not attacker-authored). Confidentiality follows repo
2347-
// visibility — collaborators of a private repo are not public.
2348-
callResult = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, callResult, ifc.LabelRepoMetadata)
2369+
// (trusted, not attacker-authored). Listing collaborators requires
2370+
// push access, so the roster is never world-readable — not even on
2371+
// a public repo — hence always private confidentiality.
2372+
callResult = attachStaticIFCLabel(ctx, deps, callResult, ifc.LabelCollaboratorRoster())
23492373
return callResult, nil, nil
23502374
},
23512375
)

pkg/github/repositories_test.go

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3838,6 +3838,107 @@ func Test_GetReleaseByTag(t *testing.T) {
38383838
}
38393839
}
38403840

3841+
// Test_GetReleaseByTag_IFC_FeatureFlag verifies the IFC label on
3842+
// get_release_by_tag. The label is only present when the ifc_labels flag is
3843+
// enabled, and confidentiality is public only for a non-draft release on a
3844+
// public repo. A draft release is visible only to push-access users, so even
3845+
// on a public repo it must be labeled private. Guards against the same
3846+
// under-classification fixed for repository security advisories.
3847+
func Test_GetReleaseByTag_IFC_FeatureFlag(t *testing.T) {
3848+
t.Parallel()
3849+
3850+
serverTool := GetReleaseByTag(translations.NullTranslationHelper)
3851+
3852+
makeRelease := func(draft bool) *github.RepositoryRelease {
3853+
return &github.RepositoryRelease{
3854+
ID: github.Ptr(int64(1)),
3855+
TagName: github.Ptr("v1.0.0"),
3856+
Name: github.Ptr("v1.0.0"),
3857+
Draft: github.Ptr(draft),
3858+
}
3859+
}
3860+
3861+
makeMockClient := func(isPrivate bool, release *github.RepositoryRelease) *http.Client {
3862+
return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
3863+
GetReposReleasesTagsByOwnerByRepoByTag: mockResponse(t, http.StatusOK, release),
3864+
GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, map[string]any{
3865+
"name": "repo",
3866+
"private": isPrivate,
3867+
}),
3868+
})
3869+
}
3870+
3871+
reqParams := map[string]any{"owner": "owner", "repo": "repo", "tag": "v1.0.0"}
3872+
3873+
readIFC := func(t *testing.T, result *mcp.CallToolResult) (map[string]any, bool) {
3874+
t.Helper()
3875+
if result.Meta == nil {
3876+
return nil, false
3877+
}
3878+
label, ok := result.Meta["ifc"]
3879+
if !ok {
3880+
return nil, false
3881+
}
3882+
labelJSON, err := json.Marshal(label)
3883+
require.NoError(t, err)
3884+
var labelMap map[string]any
3885+
require.NoError(t, json.Unmarshal(labelJSON, &labelMap))
3886+
return labelMap, true
3887+
}
3888+
3889+
t.Run("feature flag disabled omits ifc label", func(t *testing.T) {
3890+
t.Parallel()
3891+
deps := BaseDeps{Client: mustNewGHClient(t, makeMockClient(false, makeRelease(false)))}
3892+
handler := serverTool.Handler(deps)
3893+
3894+
request := createMCPRequest(reqParams)
3895+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
3896+
require.NoError(t, err)
3897+
require.False(t, result.IsError)
3898+
assert.Nil(t, result.Meta)
3899+
})
3900+
3901+
t.Run("public repo with published release is public", func(t *testing.T) {
3902+
t.Parallel()
3903+
deps := BaseDeps{
3904+
Client: mustNewGHClient(t, makeMockClient(false, makeRelease(false))),
3905+
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
3906+
}
3907+
handler := serverTool.Handler(deps)
3908+
3909+
request := createMCPRequest(reqParams)
3910+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
3911+
require.NoError(t, err)
3912+
require.False(t, result.IsError)
3913+
3914+
label, ok := readIFC(t, result)
3915+
require.True(t, ok)
3916+
assert.Equal(t, "trusted", label["integrity"])
3917+
assert.Equal(t, "public", label["confidentiality"])
3918+
})
3919+
3920+
t.Run("public repo with draft release is private", func(t *testing.T) {
3921+
t.Parallel()
3922+
// Reviewer-class scenario: a draft release on a public repo is not
3923+
// world-readable, so the label must not be public.
3924+
deps := BaseDeps{
3925+
Client: mustNewGHClient(t, makeMockClient(false, makeRelease(true))),
3926+
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
3927+
}
3928+
handler := serverTool.Handler(deps)
3929+
3930+
request := createMCPRequest(reqParams)
3931+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
3932+
require.NoError(t, err)
3933+
require.False(t, result.IsError)
3934+
3935+
label, ok := readIFC(t, result)
3936+
require.True(t, ok)
3937+
assert.Equal(t, "trusted", label["integrity"])
3938+
assert.Equal(t, "private", label["confidentiality"], "draft release on public repo must be private")
3939+
})
3940+
}
3941+
38413942
func Test_looksLikeSHA(t *testing.T) {
38423943
tests := []struct {
38433944
name string

pkg/ifc/ifc.go

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,18 @@ func PrivateUntrusted() SecurityLabel {
5959
}
6060
}
6161

62+
// LabelGetMe returns the IFC label for the authenticated user's own profile
63+
// (get_me).
64+
//
65+
// Integrity is trusted: this is GitHub-maintained data about the caller's own
66+
// account, not attacker-authored content.
67+
//
68+
// Confidentiality is private. The result includes fields that are NOT part of
69+
// the user's public profile — private_gists, total_private_repos, and
70+
// owned_private_repos — which are visible only to the authenticated user. The
71+
// result therefore must not be treated as world-readable.
6272
func LabelGetMe() SecurityLabel {
63-
return PublicTrusted()
73+
return PrivateTrusted()
6474
}
6575

6676
// LabelListIssues returns the IFC label for a list_issues result.
@@ -128,6 +138,38 @@ func LabelRepoMetadata(isPrivate bool) SecurityLabel {
128138
return PublicTrusted()
129139
}
130140

141+
// LabelRelease returns the IFC label for repository releases (list_releases,
142+
// get_latest_release, get_release_by_tag).
143+
//
144+
// Integrity is trusted: releases are published by collaborators with push
145+
// access, not by arbitrary outsiders.
146+
//
147+
// Confidentiality is public only when the repository is public AND no returned
148+
// release is a draft. Draft releases are visible only to users with push
149+
// access — they are NOT world-readable even on a public repository — so a
150+
// result containing one must be private. hasDraft reflects whether any release
151+
// in the result is a draft; private repositories are always private regardless.
152+
func LabelRelease(isPrivate bool, hasDraft bool) SecurityLabel {
153+
if isPrivate || hasDraft {
154+
return PrivateTrusted()
155+
}
156+
return PublicTrusted()
157+
}
158+
159+
// LabelCollaboratorRoster returns the IFC label for a repository's collaborator
160+
// list (list_repository_collaborators).
161+
//
162+
// Integrity is trusted: the roster is GitHub-maintained membership data, not
163+
// attacker-authored content.
164+
//
165+
// Confidentiality is always private. Listing collaborators requires push
166+
// access to the repository, so the roster is never world-readable — not even
167+
// for public repositories. This mirrors LabelTeam: membership data is
168+
// restricted regardless of the repository's own visibility.
169+
func LabelCollaboratorRoster() SecurityLabel {
170+
return PrivateTrusted()
171+
}
172+
131173
// LabelCommitContents returns the IFC label for committed repository content
132174
// reachable from the default branch and its history: commits, commit diffs,
133175
// and the repository file tree.

pkg/ifc/ifc_test.go

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,56 @@ func TestLabelRepoMetadata(t *testing.T) {
6868
})
6969
}
7070

71+
func TestLabelGetMe(t *testing.T) {
72+
t.Parallel()
73+
74+
// get_me exposes private_gists/total_private_repos/owned_private_repos,
75+
// which are not part of the public profile, so the result is trusted but
76+
// private — never public.
77+
label := LabelGetMe()
78+
assert.Equal(t, IntegrityTrusted, label.Integrity)
79+
assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
80+
}
81+
82+
func TestLabelRelease(t *testing.T) {
83+
t.Parallel()
84+
85+
t.Run("public repo with no draft is trusted and public", func(t *testing.T) {
86+
t.Parallel()
87+
label := LabelRelease(false, false)
88+
assert.Equal(t, IntegrityTrusted, label.Integrity)
89+
assert.Equal(t, ConfidentialityPublic, label.Confidentiality)
90+
})
91+
92+
t.Run("public repo with a draft release is private", func(t *testing.T) {
93+
t.Parallel()
94+
// Draft releases are visible only to push-access users, so a draft on
95+
// a public repo must not be labeled public.
96+
label := LabelRelease(false, true)
97+
assert.Equal(t, IntegrityTrusted, label.Integrity)
98+
assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
99+
})
100+
101+
t.Run("private repo is private regardless of draft", func(t *testing.T) {
102+
t.Parallel()
103+
for _, hasDraft := range []bool{false, true} {
104+
label := LabelRelease(true, hasDraft)
105+
assert.Equal(t, IntegrityTrusted, label.Integrity)
106+
assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
107+
}
108+
})
109+
}
110+
111+
func TestLabelCollaboratorRoster(t *testing.T) {
112+
t.Parallel()
113+
114+
// A collaborator roster requires push access to list, so it is never
115+
// world-readable — always trusted and private.
116+
label := LabelCollaboratorRoster()
117+
assert.Equal(t, IntegrityTrusted, label.Integrity)
118+
assert.Equal(t, ConfidentialityPrivate, label.Confidentiality)
119+
}
120+
71121
func TestLabelCommitContents(t *testing.T) {
72122
t.Parallel()
73123

0 commit comments

Comments
 (0)