Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions pkg/github/discussions.go
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool
result := utils.NewToolResultText(string(out))
// Discussion content is user-authored (untrusted); confidentiality
// follows repo visibility.
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelListIssues)
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoUserContent)
return result, nil, nil
},
)
Expand Down Expand Up @@ -384,7 +384,7 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
result := utils.NewToolResultText(string(out))
// Discussion content is user-authored (untrusted); confidentiality
// follows repo visibility.
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelListIssues)
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelRepoUserContent)
return result, nil, nil
},
)
Expand Down Expand Up @@ -592,7 +592,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
result := utils.NewToolResultText(string(out))
// Discussion comments are user-authored (untrusted); confidentiality
// follows repo visibility.
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelListIssues)
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelRepoUserContent)
return result, nil, nil
},
)
Expand Down
12 changes: 2 additions & 10 deletions pkg/github/gists.go
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool {
}

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

result := utils.NewToolResultText(string(r))
// Gist contents are user-authored (untrusted); confidentiality
// derives from the gist's own public/secret flag.
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGist(gist.GetPublic()))
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGist())
return result, nil, nil
},
)
Expand Down
1 change: 1 addition & 0 deletions pkg/github/helper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
const (
// User endpoints
GetUser = "GET /user"
GetUsersByUsername = "GET /users/{username}"
GetUserStarred = "GET /user/starred"
GetUsersGistsByUsername = "GET /users/{username}/gists"
GetUsersStarredByUsername = "GET /users/{username}/starred"
Expand Down
37 changes: 27 additions & 10 deletions pkg/github/ifc_labels.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@ func setIFCLabel(r *mcp.CallToolResult, label ifc.SecurityLabel) {
r.Meta["ifc"] = label
}

func shouldAttachIFCLabel(ctx context.Context, deps ToolDependencies, r *mcp.CallToolResult) bool {
return r != nil && !r.IsError && deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels)
}

// attachStaticIFCLabel attaches a fixed IFC label to a successful tool result
// when IFC labels are enabled. It is used by tools whose label does not depend
// on any repository visibility lookup (e.g. security alerts, global
Expand All @@ -25,7 +29,7 @@ func setIFCLabel(r *mcp.CallToolResult, label ifc.SecurityLabel) {
// Error results are left untouched, and the label is omitted entirely when the
// IFC feature flag is disabled.
func attachStaticIFCLabel(ctx context.Context, deps ToolDependencies, r *mcp.CallToolResult, label ifc.SecurityLabel) *mcp.CallToolResult {
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
if !shouldAttachIFCLabel(ctx, deps, r) {
return r
}
setIFCLabel(r, label)
Expand All @@ -49,7 +53,7 @@ func attachRepoVisibilityIFCLabel(
r *mcp.CallToolResult,
labelFn func(isPrivate bool) ifc.SecurityLabel,
) *mcp.CallToolResult {
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
if !shouldAttachIFCLabel(ctx, deps, r) {
return r
}
isPrivate, err := FetchRepoIsPrivate(ctx, client, owner, repo)
Expand Down Expand Up @@ -86,7 +90,7 @@ func attachRepoVisibilityIFCLabelLazy(
r *mcp.CallToolResult,
labelFn func(isPrivate bool) ifc.SecurityLabel,
) *mcp.CallToolResult {
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
if !shouldAttachIFCLabel(ctx, deps, r) {
return r
}
client, err := deps.GetClient(ctx)
Expand All @@ -97,26 +101,39 @@ func attachRepoVisibilityIFCLabelLazy(
}

// attachJoinedIFCLabel attaches an IFC label computed by joining a set of
// per-item visibilities (true == private for repositories, true == public for
// gists) when IFC labels are enabled. joinFn is the lattice join for the
// relevant item kind (e.g. ifc.LabelSearchIssues or ifc.LabelGistList). The
// visibility slice is cheap to build from an already-fetched response, so
// callers may construct it unconditionally and let this helper own the
// feature-flag gate.
// per-item visibilities (true == private) when IFC labels are enabled. joinFn
// is the lattice join for the relevant item kind (e.g. ifc.LabelSearchIssues or
// ifc.LabelProjectList). The visibility slice is cheap to build from an
// already-fetched response, so callers may construct it unconditionally and let
// this helper own the feature-flag gate.
func attachJoinedIFCLabel(
ctx context.Context,
deps ToolDependencies,
r *mcp.CallToolResult,
visibilities []bool,
joinFn func([]bool) ifc.SecurityLabel,
) *mcp.CallToolResult {
if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
if !shouldAttachIFCLabel(ctx, deps, r) {
return r
}
setIFCLabel(r, joinFn(visibilities))
return r
}

func attachProjectVisibilityIFCLabel(
ctx context.Context,
deps ToolDependencies,
r *mcp.CallToolResult,
isPrivate bool,
labelFn func(isPrivate bool) ifc.SecurityLabel,
) *mcp.CallToolResult {
if !shouldAttachIFCLabel(ctx, deps, r) {
return r
}
setIFCLabel(r, labelFn(isPrivate))
return r
}

// newRepoVisibilityIFCLabeler returns a closure that attaches a repo-visibility
// IFC label to a tool result, for handlers that have several return paths and
// want to label each one. The returned function owns the feature-flag gate (so
Expand Down
2 changes: 1 addition & 1 deletion pkg/github/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,7 @@ Options are:
// attachIFC adds the IFC label to a successful tool result when
// IFC labels are enabled. If the visibility lookup fails the
// label is omitted rather than misclassifying the result.
attachIFC := newRepoVisibilityIFCLabeler(ctx, deps, client, owner, repo, ifc.LabelListIssues)
attachIFC := newRepoVisibilityIFCLabeler(ctx, deps, client, owner, repo, ifc.LabelRepoUserContent)

switch method {
case "get":
Expand Down
8 changes: 4 additions & 4 deletions pkg/github/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,7 +356,7 @@ func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
assert.Equal(t, "public", ifcMap["confidentiality"])
})

t.Run("insiders mode enabled on private repo with get_comments emits private untrusted", func(t *testing.T) {
t.Run("insiders mode enabled on private repo with get_comments emits private trusted", func(t *testing.T) {
deps := BaseDeps{
Client: mustNewGHClient(t, makeMockClient(true, 0)),
featureChecker: featureCheckerFor(FeatureFlagIFCLabels),
Expand All @@ -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, "trusted", ifcMap["integrity"])
assert.Equal(t, "private", ifcMap["confidentiality"])
})

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

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

assert.Equal(t, "untrusted", ifcMap["integrity"])
assert.Equal(t, "trusted", ifcMap["integrity"])
assert.Equal(t, "private", ifcMap["confidentiality"])
Comment thread
RossTarrant marked this conversation as resolved.
})
}
Expand Down
Loading
Loading