Skip to content

Commit 667bd3e

Browse files
authored
Fix IFC private repository labels (#2695)
1 parent 4e8eb81 commit 667bd3e

14 files changed

Lines changed: 658 additions & 187 deletions

pkg/github/discussions.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -276,7 +276,7 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool
276276
result := utils.NewToolResultText(string(out))
277277
// Discussion content is user-authored (untrusted); confidentiality
278278
// follows repo visibility.
279-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelListIssues)
279+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoUserContent)
280280
return result, nil, nil
281281
},
282282
)
@@ -384,7 +384,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
384384
result := utils.NewToolResultText(string(out))
385385
// Discussion content is user-authored (untrusted); confidentiality
386386
// follows repo visibility.
387-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelListIssues)
387+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelRepoUserContent)
388388
return result, nil, nil
389389
},
390390
)
@@ -592,7 +592,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
592592
result := utils.NewToolResultText(string(out))
593593
// Discussion comments are user-authored (untrusted); confidentiality
594594
// follows repo visibility.
595-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelListIssues)
595+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelRepoUserContent)
596596
return result, nil, nil
597597
},
598598
)

pkg/github/gists.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,13 +101,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool {
101101
}
102102

103103
result := utils.NewToolResultText(string(r))
104-
// Gist contents are user-authored (untrusted); confidentiality is
105-
// the IFC join of each gist's own public/secret flag.
106-
visibilities := make([]bool, 0, len(gists))
107-
for _, g := range gists {
108-
visibilities = append(visibilities, g.GetPublic())
109-
}
110-
result = attachJoinedIFCLabel(ctx, deps, result, visibilities, ifc.LabelGistList)
104+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGistList())
111105
return result, nil, nil
112106
},
113107
)
@@ -167,9 +161,7 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool {
167161
}
168162

169163
result := utils.NewToolResultText(string(r))
170-
// Gist contents are user-authored (untrusted); confidentiality
171-
// derives from the gist's own public/secret flag.
172-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGist(gist.GetPublic()))
164+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGist())
173165
return result, nil, nil
174166
},
175167
)

pkg/github/helper_test.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222
const (
2323
// User endpoints
2424
GetUser = "GET /user"
25+
GetUsersByUsername = "GET /users/{username}"
2526
GetUserStarred = "GET /user/starred"
2627
GetUsersGistsByUsername = "GET /users/{username}/gists"
2728
GetUsersStarredByUsername = "GET /users/{username}/starred"

pkg/github/ifc_labels.go

Lines changed: 27 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,10 @@ func setIFCLabel(r *mcp.CallToolResult, label ifc.SecurityLabel) {
1717
r.Meta["ifc"] = label
1818
}
1919

20+
func shouldAttachIFCLabel(ctx context.Context, deps ToolDependencies, r *mcp.CallToolResult) bool {
21+
return r != nil && !r.IsError && deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels)
22+
}
23+
2024
// attachStaticIFCLabel attaches a fixed IFC label to a successful tool result
2125
// when IFC labels are enabled. It is used by tools whose label does not depend
2226
// on any repository visibility lookup (e.g. security alerts, global
@@ -25,7 +29,7 @@ func setIFCLabel(r *mcp.CallToolResult, label ifc.SecurityLabel) {
2529
// Error results are left untouched, and the label is omitted entirely when the
2630
// IFC feature flag is disabled.
2731
func attachStaticIFCLabel(ctx context.Context, deps ToolDependencies, r *mcp.CallToolResult, label ifc.SecurityLabel) *mcp.CallToolResult {
28-
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
32+
if !shouldAttachIFCLabel(ctx, deps, r) {
2933
return r
3034
}
3135
setIFCLabel(r, label)
@@ -49,7 +53,7 @@ func attachRepoVisibilityIFCLabel(
4953
r *mcp.CallToolResult,
5054
labelFn func(isPrivate bool) ifc.SecurityLabel,
5155
) *mcp.CallToolResult {
52-
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
56+
if !shouldAttachIFCLabel(ctx, deps, r) {
5357
return r
5458
}
5559
isPrivate, err := FetchRepoIsPrivate(ctx, client, owner, repo)
@@ -86,7 +90,7 @@ func attachRepoVisibilityIFCLabelLazy(
8690
r *mcp.CallToolResult,
8791
labelFn func(isPrivate bool) ifc.SecurityLabel,
8892
) *mcp.CallToolResult {
89-
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
93+
if !shouldAttachIFCLabel(ctx, deps, r) {
9094
return r
9195
}
9296
client, err := deps.GetClient(ctx)
@@ -97,26 +101,39 @@ func attachRepoVisibilityIFCLabelLazy(
97101
}
98102

99103
// attachJoinedIFCLabel attaches an IFC label computed by joining a set of
100-
// per-item visibilities (true == private for repositories, true == public for
101-
// gists) when IFC labels are enabled. joinFn is the lattice join for the
102-
// relevant item kind (e.g. ifc.LabelSearchIssues or ifc.LabelGistList). The
103-
// visibility slice is cheap to build from an already-fetched response, so
104-
// callers may construct it unconditionally and let this helper own the
105-
// feature-flag gate.
104+
// per-item visibilities (true == private) when IFC labels are enabled. joinFn
105+
// is the lattice join for the relevant item kind (e.g. ifc.LabelSearchIssues or
106+
// ifc.LabelProjectList). The visibility slice is cheap to build from an
107+
// already-fetched response, so callers may construct it unconditionally and let
108+
// this helper own the feature-flag gate.
106109
func attachJoinedIFCLabel(
107110
ctx context.Context,
108111
deps ToolDependencies,
109112
r *mcp.CallToolResult,
110113
visibilities []bool,
111114
joinFn func([]bool) ifc.SecurityLabel,
112115
) *mcp.CallToolResult {
113-
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
116+
if !shouldAttachIFCLabel(ctx, deps, r) {
114117
return r
115118
}
116119
setIFCLabel(r, joinFn(visibilities))
117120
return r
118121
}
119122

123+
func attachProjectVisibilityIFCLabel(
124+
ctx context.Context,
125+
deps ToolDependencies,
126+
r *mcp.CallToolResult,
127+
isPrivate bool,
128+
labelFn func(isPrivate bool) ifc.SecurityLabel,
129+
) *mcp.CallToolResult {
130+
if !shouldAttachIFCLabel(ctx, deps, r) {
131+
return r
132+
}
133+
setIFCLabel(r, labelFn(isPrivate))
134+
return r
135+
}
136+
120137
// newRepoVisibilityIFCLabeler returns a closure that attaches a repo-visibility
121138
// IFC label to a tool result, for handlers that have several return paths and
122139
// want to label each one. The returned function owns the feature-flag gate (so

pkg/github/issues.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -804,7 +804,7 @@ Options are:
804804
// attachIFC adds the IFC label to a successful tool result when
805805
// IFC labels are enabled. If the visibility lookup fails the
806806
// label is omitted rather than misclassifying the result.
807-
attachIFC := newRepoVisibilityIFCLabeler(ctx, deps, client, owner, repo, ifc.LabelListIssues)
807+
attachIFC := newRepoVisibilityIFCLabeler(ctx, deps, client, owner, repo, ifc.LabelRepoUserContent)
808808

809809
switch method {
810810
case "get":

pkg/github/issues_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -356,7 +356,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
356356
assert.Equal(t, "public", ifcMap["confidentiality"])
357357
})
358358

359-
t.Run("insiders mode enabled on private repo with get_comments emits private untrusted", func(t *testing.T) {
359+
t.Run("insiders mode enabled on private repo with get_comments emits private trusted", func(t *testing.T) {
360360
deps := BaseDeps{
361361
Client: mustNewGHClient(t, makeMockClient(true, 0)),
362362
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
@@ -370,7 +370,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
370370

371371
require.NotNil(t, result.Meta)
372372
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
373-
assert.Equal(t, "untrusted", ifcMap["integrity"])
373+
assert.Equal(t, "trusted", ifcMap["integrity"])
374374
assert.Equal(t, "private", ifcMap["confidentiality"])
375375
})
376376

@@ -2729,7 +2729,7 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
27292729
assert.Equal(t, "public", ifcMap["confidentiality"])
27302730
})
27312731

2732-
t.Run("insiders mode enabled on private repo emits private untrusted label", func(t *testing.T) {
2732+
t.Run("insiders mode enabled on private repo emits private trusted label", func(t *testing.T) {
27332733
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
27342734
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
27352735
deps := BaseDeps{
@@ -2752,7 +2752,7 @@ func Test_ListIssues_IFC_InsidersMode(t *testing.T) {
27522752
var ifcMap map[string]any
27532753
require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap))
27542754

2755-
assert.Equal(t, "untrusted", ifcMap["integrity"])
2755+
assert.Equal(t, "trusted", ifcMap["integrity"])
27562756
assert.Equal(t, "private", ifcMap["confidentiality"])
27572757
})
27582758
}

0 commit comments

Comments
 (0)