Skip to content

Commit 2dac924

Browse files
committed
refactor!: remove IFC security labels
Remove the IFC (information flow control) labels that tool results carried in _meta.ifc behind the ifc_labels feature flag, along with the code that only existed to compute them: the pkg/ifc package, the label helpers, the flag, the isPrivate/public GraphQL fields, the repository and project visibility lookups, and IFC-only tests. Removes exported Go API: package ifc, FetchRepoIsPrivate, FetchProjectIsPrivate, FeatureFlagIFCLabels and IssueQueryResult.GetIsPrivate.
1 parent f2a07cd commit 2dac924

49 files changed

Lines changed: 268 additions & 3406 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎internal/ghmcp/server_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ func TestStdioGraphQLUserAgent(t *testing.T) {
135135
api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
136136
headers <- r.Header.Clone()
137137
w.Header().Set("Content-Type", "application/json")
138-
_, _ = w.Write([]byte(`{"data":{"repository":{"isPrivate":false,"issues":{"nodes":[],"totalCount":0,"pageInfo":{"hasNextPage":false,"endCursor":""}}}}}`))
138+
_, _ = w.Write([]byte(`{"data":{"repository":{"issues":{"nodes":[],"totalCount":0,"pageInfo":{"hasNextPage":false,"endCursor":""}}}}}`))
139139
}))
140140
defer api.Close()
141141

‎pkg/github/actions.go‎

Lines changed: 12 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ import (
1313
"github.com/github/github-mcp-server/v2/internal/profiler"
1414
buffer "github.com/github/github-mcp-server/v2/pkg/buffer"
1515
ghErrors "github.com/github/github-mcp-server/v2/pkg/errors"
16-
"github.com/github/github-mcp-server/v2/pkg/ifc"
1716
"github.com/github/github-mcp-server/v2/pkg/inventory"
1817
"github.com/github/github-mcp-server/v2/pkg/scopes"
1918
"github.com/github/github-mcp-server/v2/pkg/translations"
@@ -324,14 +323,6 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
324323
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
325324
}
326325

327-
// attachIFC adds the IFC label to a successful Actions result when
328-
// IFC labels are enabled. Workflow definitions, runs, jobs,
329-
// artifacts and logs echo attacker-influenceable run output, so
330-
// integrity is untrusted; confidentiality follows repo visibility.
331-
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
332-
return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult)
333-
}
334-
335326
var resourceIDInt int64
336327
var parseErr error
337328
switch method {
@@ -354,17 +345,13 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
354345

355346
switch method {
356347
case actionsMethodListWorkflows:
357-
result, payload, err := listWorkflows(ctx, client, owner, repo, pagination)
358-
return attachIFC(result), payload, err
348+
return listWorkflows(ctx, client, owner, repo, pagination)
359349
case actionsMethodListWorkflowRuns:
360-
result, payload, err := listWorkflowRuns(ctx, client, args.WorkflowRunsFilter, owner, repo, resourceID, pagination)
361-
return attachIFC(result), payload, err
350+
return listWorkflowRuns(ctx, client, args.WorkflowRunsFilter, owner, repo, resourceID, pagination)
362351
case actionsMethodListWorkflowJobs:
363-
result, payload, err := listWorkflowJobs(ctx, client, args.WorkflowJobsFilter, owner, repo, resourceIDInt, pagination)
364-
return attachIFC(result), payload, err
352+
return listWorkflowJobs(ctx, client, args.WorkflowJobsFilter, owner, repo, resourceIDInt, pagination)
365353
case actionsMethodListWorkflowArtifacts:
366-
result, payload, err := listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination)
367-
return attachIFC(result), payload, err
354+
return listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination)
368355
default:
369356
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
370357
}
@@ -426,14 +413,6 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
426413
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
427414
}
428415

429-
// attachIFC adds the IFC label to a successful Actions result when
430-
// IFC labels are enabled. Workflow runs, jobs, artifacts, usage,
431-
// and log URLs reflect attacker-influenceable run output, so
432-
// integrity is untrusted; confidentiality follows repo visibility.
433-
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
434-
return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult)
435-
}
436-
437416
var resourceIDInt int64
438417
var parseErr error
439418
switch method {
@@ -449,23 +428,17 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
449428

450429
switch method {
451430
case actionsMethodGetWorkflow:
452-
result, payload, err := getWorkflow(ctx, client, owner, repo, resourceID)
453-
return attachIFC(result), payload, err
431+
return getWorkflow(ctx, client, owner, repo, resourceID)
454432
case actionsMethodGetWorkflowRun:
455-
result, payload, err := getWorkflowRun(ctx, client, owner, repo, resourceIDInt)
456-
return attachIFC(result), payload, err
433+
return getWorkflowRun(ctx, client, owner, repo, resourceIDInt)
457434
case actionsMethodGetWorkflowJob:
458-
result, payload, err := getWorkflowJob(ctx, client, owner, repo, resourceIDInt)
459-
return attachIFC(result), payload, err
435+
return getWorkflowJob(ctx, client, owner, repo, resourceIDInt)
460436
case actionsMethodDownloadWorkflowArtifact:
461-
result, payload, err := downloadWorkflowArtifact(ctx, client, owner, repo, resourceIDInt)
462-
return attachIFC(result), payload, err
437+
return downloadWorkflowArtifact(ctx, client, owner, repo, resourceIDInt)
463438
case actionsMethodGetWorkflowRunUsage:
464-
result, payload, err := getWorkflowRunUsage(ctx, client, owner, repo, resourceIDInt)
465-
return attachIFC(result), payload, err
439+
return getWorkflowRunUsage(ctx, client, owner, repo, resourceIDInt)
466440
case actionsMethodGetWorkflowRunLogsURL:
467-
result, payload, err := getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt)
468-
return attachIFC(result), payload, err
441+
return getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt)
469442
default:
470443
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
471444
}
@@ -636,22 +609,12 @@ For single job logs, provide job_id. For all failed jobs in a run, provide run_i
636609
return utils.NewToolResultError("job_id is required when failed_only is false"), nil, nil
637610
}
638611

639-
// attachIFC adds the IFC label to a successful result when IFC
640-
// labels are enabled. Job logs echo attacker-influenceable run
641-
// output, so integrity is untrusted; confidentiality follows repo
642-
// visibility.
643-
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
644-
return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult)
645-
}
646-
647612
if failedOnly && runID > 0 {
648613
// Handle failed-only mode: get logs for all failed jobs in the workflow run
649-
result, payload, err := handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize())
650-
return attachIFC(result), payload, err
614+
return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize())
651615
} else if jobID > 0 {
652616
// Handle single job mode
653-
result, payload, err := handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize())
654-
return attachIFC(result), payload, err
617+
return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize())
655618
}
656619

657620
return utils.NewToolResultError("Either job_id must be provided for single job logs, or run_id with failed_only=true for failed job logs"), nil, nil

‎pkg/github/code_scanning.go‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"net/http"
88

99
ghErrors "github.com/github/github-mcp-server/v2/pkg/errors"
10-
"github.com/github/github-mcp-server/v2/pkg/ifc"
1110
"github.com/github/github-mcp-server/v2/pkg/inventory"
1211
"github.com/github/github-mcp-server/v2/pkg/scopes"
1312
"github.com/github/github-mcp-server/v2/pkg/translations"
@@ -86,12 +85,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server
8685
return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, nil
8786
}
8887

89-
result := utils.NewToolResultText(string(r))
90-
// Code scanning alerts are access-restricted regardless of repo
91-
// visibility and embed attacker-influenceable code snippets, so the
92-
// label is always private-untrusted.
93-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
94-
return result, codeScanningAlertOutput(alert), nil
88+
return utils.NewToolResultText(string(r)), codeScanningAlertOutput(alert), nil
9589
},
9690
normalizeSecurityIntegerArguments("alertNumber"),
9791
)
@@ -193,12 +187,7 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv
193187
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, nil
194188
}
195189

196-
result := utils.NewToolResultText(string(r))
197-
// Code scanning alerts are access-restricted regardless of repo
198-
// visibility and embed attacker-influenceable code snippets, so the
199-
// label is always private-untrusted.
200-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
201-
return result, mapSecurityOutputs(alerts, codeScanningAlertOutput), nil
190+
return utils.NewToolResultText(string(r)), mapSecurityOutputs(alerts, codeScanningAlertOutput), nil
202191
},
203192
normalizeSecurityIntegerArguments("page", "perPage"),
204193
normalizeTypedReadArguments(nil, false),

‎pkg/github/context_tools.go‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"time"
1010

1111
ghErrors "github.com/github/github-mcp-server/v2/pkg/errors"
12-
"github.com/github/github-mcp-server/v2/pkg/ifc"
1312
"github.com/github/github-mcp-server/v2/pkg/inventory"
1413
"github.com/github/github-mcp-server/v2/pkg/scopes"
1514
"github.com/github/github-mcp-server/v2/pkg/translations"
@@ -178,9 +177,7 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool {
178177
},
179178
}
180179

181-
result := MarshalledTextResult(minimalUser)
182-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGetMe())
183-
return result, minimalUser, nil
180+
return MarshalledTextResult(minimalUser), minimalUser, nil
184181
},
185182
)
186183
}
@@ -283,10 +280,6 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool {
283280
}
284281

285282
result := MarshalledTextResult(organizations)
286-
// Team membership is maintained by GitHub and cannot be forged by
287-
// outside contributors (trusted). Org team rosters are visible only
288-
// to org members, so confidentiality is private.
289-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelTeam())
290283
if organizations == nil {
291284
organizations = []OrganizationTeams{}
292285
}
@@ -355,10 +348,6 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool {
355348
}
356349

357350
result := MarshalledTextResult(members)
358-
// Team membership is maintained by GitHub and cannot be forged by
359-
// outside contributors (trusted). A team's member roster is visible
360-
// only to org members, so confidentiality is private.
361-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelTeam())
362351
if members == nil {
363352
members = []string{}
364353
}

‎pkg/github/context_tools_test.go‎

Lines changed: 0 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -460,72 +460,6 @@ func TestContextToolsTypedRegistration(t *testing.T) {
460460
assert.Nil(t, result.StructuredContent, "handler errors must not expose a successful typed output")
461461
}
462462

463-
func Test_GetMe_IFC_FeatureFlag(t *testing.T) {
464-
t.Parallel()
465-
466-
serverTool := GetMe(translations.NullTranslationHelper)
467-
468-
mockUser := &github.User{
469-
Login: new("testuser"),
470-
HTMLURL: new("https://github.com/testuser"),
471-
CreatedAt: &github.Timestamp{Time: time.Now()},
472-
}
473-
mockedHTTPClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{
474-
GetUser: mockResponse(t, http.StatusOK, mockUser),
475-
})
476-
477-
depsWithIFCFeature := func(enabled bool) *BaseDeps {
478-
return NewBaseDeps(
479-
mustNewGHClient(t, mockedHTTPClient), nil, nil, nil,
480-
translations.NullTranslationHelper,
481-
FeatureFlags{},
482-
0,
483-
func(_ context.Context, flagName string) (bool, error) {
484-
return flagName == FeatureFlagIFCLabels && enabled, nil
485-
},
486-
stubExporters(),
487-
)
488-
}
489-
490-
t.Run("feature disabled omits ifc label from result meta", func(t *testing.T) {
491-
deps := depsWithIFCFeature(false)
492-
handler := serverTool.Handler(deps)
493-
494-
request := createMCPRequest(map[string]any{})
495-
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
496-
require.NoError(t, err)
497-
require.False(t, result.IsError)
498-
499-
assert.Nil(t, result.Meta, "result meta should be nil when IFC labels are disabled")
500-
})
501-
502-
t.Run("feature enabled includes ifc label in result meta", func(t *testing.T) {
503-
deps := depsWithIFCFeature(true)
504-
handler := serverTool.Handler(deps)
505-
506-
request := createMCPRequest(map[string]any{})
507-
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
508-
require.NoError(t, err)
509-
require.False(t, result.IsError)
510-
511-
require.NotNil(t, result.Meta, "result meta should be set when IFC labels are enabled")
512-
ifcLabel, ok := result.Meta["ifc"]
513-
require.True(t, ok, "result meta should contain ifc key")
514-
515-
ifcJSON, err := json.Marshal(ifcLabel)
516-
require.NoError(t, err)
517-
518-
var ifcMap map[string]any
519-
err = json.Unmarshal(ifcJSON, &ifcMap)
520-
require.NoError(t, err)
521-
522-
assert.Equal(t, "trusted", ifcMap["integrity"])
523-
// get_me returns the caller's private repo/gist counts, which are not
524-
// part of the public profile, so confidentiality is private.
525-
assert.Equal(t, "private", ifcMap["confidentiality"])
526-
})
527-
}
528-
529463
func Test_GetTeams(t *testing.T) {
530464
t.Parallel()
531465

‎pkg/github/dependabot.go‎

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"net/http"
99

1010
ghErrors "github.com/github/github-mcp-server/v2/pkg/errors"
11-
"github.com/github/github-mcp-server/v2/pkg/ifc"
1211
"github.com/github/github-mcp-server/v2/pkg/inventory"
1312
"github.com/github/github-mcp-server/v2/pkg/scopes"
1413
"github.com/github/github-mcp-server/v2/pkg/translations"
@@ -87,12 +86,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo
8786
return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, err
8887
}
8988

90-
result := utils.NewToolResultText(string(r))
91-
// Dependabot alerts are access-restricted regardless of repo
92-
// visibility and embed attacker-influenceable advisory text, so the
93-
// label is always private-untrusted.
94-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
95-
return result, dependabotAlertOutput(alert), nil
89+
return utils.NewToolResultText(string(r)), dependabotAlertOutput(alert), nil
9690
},
9791
normalizeSecurityIntegerArguments("alertNumber"),
9892
)
@@ -198,12 +192,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
198192
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), DependabotAlertsOutput{}, err
199193
}
200194

201-
result := utils.NewToolResultText(string(r))
202-
// Dependabot alerts are access-restricted regardless of repo
203-
// visibility and embed attacker-influenceable advisory text, so the
204-
// label is always private-untrusted.
205-
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
206-
return result, output, nil
195+
return utils.NewToolResultText(string(r)), output, nil
207196
},
208197
normalizeTypedReadArguments(nil, false),
209198
)

‎pkg/github/discussions.go‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"fmt"
77
"strings"
88

9-
"github.com/github/github-mcp-server/v2/pkg/ifc"
109
"github.com/github/github-mcp-server/v2/pkg/inventory"
1110
"github.com/github/github-mcp-server/v2/pkg/sanitize"
1211
"github.com/github/github-mcp-server/v2/pkg/scopes"
@@ -279,9 +278,6 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool
279278
return nil, nil, fmt.Errorf("failed to marshal discussions: %w", err)
280279
}
281280
result := utils.NewToolResultText(string(out))
282-
// Discussion content is user-authored (untrusted); confidentiality
283-
// follows repo visibility.
284-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoUserContent)
285281
items := make([]*DiscussionListItemOutput, 0, len(discussions))
286282
for _, discussion := range discussions {
287283
items = append(items, &DiscussionListItemOutput{
@@ -395,9 +391,6 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
395391
}
396392

397393
result := utils.NewToolResultText(string(out))
398-
// Discussion content is user-authored (untrusted); confidentiality
399-
// follows repo visibility.
400-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelRepoUserContent)
401394
output := &DiscussionOutput{
402395
Number: int(d.Number), Title: sanitize.PlainText(string(d.Title)),
403396
Body: sanitize.Content(string(d.Body)), HTMLURL: string(d.URL),
@@ -596,11 +589,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
596589
return nil, nil, fmt.Errorf("failed to marshal comments: %w", err)
597590
}
598591

599-
result := utils.NewToolResultText(string(out))
600-
// Discussion comments are user-authored (untrusted); confidentiality
601-
// follows repo visibility.
602-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelRepoUserContent)
603-
return result, &DiscussionCommentsOutput{
592+
return utils.NewToolResultText(string(out)), &DiscussionCommentsOutput{
604593
Comments: discussionCommentOutputs(comments),
605594
PageInfo: DiscussionPageInfoOutput{
606595
HasNextPage: bool(pageInfo.HasNextPage), HasPreviousPage: bool(pageInfo.HasPreviousPage),
@@ -1126,9 +1115,6 @@ func ListDiscussionCategories(t translations.TranslationHelperFunc) inventory.Se
11261115
return nil, nil, fmt.Errorf("failed to marshal discussion categories: %w", err)
11271116
}
11281117
result := utils.NewToolResultText(string(out))
1129-
// Discussion categories are repo-defined structural metadata
1130-
// (trusted); confidentiality follows repo visibility.
1131-
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata)
11321118
items := make([]DiscussionCategoryItemOutput, 0, len(categories))
11331119
for _, category := range categories {
11341120
items = append(items, DiscussionCategoryItemOutput{ID: category["id"], Name: category["name"]})

‎pkg/github/feature_flags.go‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,6 @@ const MCPAppsDisableFormDeferralFeatureFlag = "mcp_apps_disable_form_deferral"
1313
// FeatureFlagCSVOutput is the feature flag name for CSV output on list tools.
1414
const FeatureFlagCSVOutput = "csv_output"
1515

16-
// FeatureFlagIFCLabels is the feature flag name for IFC security labels in tool results.
17-
const FeatureFlagIFCLabels = "ifc_labels"
18-
1916
// FeatureFlagFileBlame is the feature flag name for the get_file_blame tool,
2017
// which exposes git blame information for a file. It is gated so the extra tool
2118
// is not advertised by default, keeping the tool surface small unless opted in.
@@ -46,7 +43,6 @@ const FeatureFlagThreadResolutionReason = "thread_resolution_reason"
4643
var AllowedFeatureFlags = []string{
4744
MCPAppsDisableFormDeferralFeatureFlag,
4845
FeatureFlagCSVOutput,
49-
FeatureFlagIFCLabels,
5046
FeatureFlagIssuesGranular,
5147
FeatureFlagPullRequestsGranular,
5248
FeatureFlagFileBlame,

0 commit comments

Comments
 (0)