diff --git a/internal/ghmcp/server_test.go b/internal/ghmcp/server_test.go index 1d6f844c41..dcc980190e 100644 --- a/internal/ghmcp/server_test.go +++ b/internal/ghmcp/server_test.go @@ -135,7 +135,7 @@ func TestStdioGraphQLUserAgent(t *testing.T) { api := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { headers <- r.Header.Clone() w.Header().Set("Content-Type", "application/json") - _, _ = w.Write([]byte(`{"data":{"repository":{"isPrivate":false,"issues":{"nodes":[],"totalCount":0,"pageInfo":{"hasNextPage":false,"endCursor":""}}}}}`)) + _, _ = w.Write([]byte(`{"data":{"repository":{"issues":{"nodes":[],"totalCount":0,"pageInfo":{"hasNextPage":false,"endCursor":""}}}}}`)) })) defer api.Close() diff --git a/pkg/github/actions.go b/pkg/github/actions.go index dc71ab5261..f8822beed1 100644 --- a/pkg/github/actions.go +++ b/pkg/github/actions.go @@ -13,7 +13,6 @@ import ( "github.com/github/github-mcp-server/v2/internal/profiler" buffer "github.com/github/github-mcp-server/v2/pkg/buffer" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "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 return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // attachIFC adds the IFC label to a successful Actions result when - // IFC labels are enabled. Workflow definitions, runs, jobs, - // artifacts and logs echo attacker-influenceable run output, so - // integrity is untrusted; confidentiality follows repo visibility. - attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult { - return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult) - } - var resourceIDInt int64 var parseErr error switch method { @@ -354,17 +345,13 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an switch method { case actionsMethodListWorkflows: - result, payload, err := listWorkflows(ctx, client, owner, repo, pagination) - return attachIFC(result), payload, err + return listWorkflows(ctx, client, owner, repo, pagination) case actionsMethodListWorkflowRuns: - result, payload, err := listWorkflowRuns(ctx, client, args.WorkflowRunsFilter, owner, repo, resourceID, pagination) - return attachIFC(result), payload, err + return listWorkflowRuns(ctx, client, args.WorkflowRunsFilter, owner, repo, resourceID, pagination) case actionsMethodListWorkflowJobs: - result, payload, err := listWorkflowJobs(ctx, client, args.WorkflowJobsFilter, owner, repo, resourceIDInt, pagination) - return attachIFC(result), payload, err + return listWorkflowJobs(ctx, client, args.WorkflowJobsFilter, owner, repo, resourceIDInt, pagination) case actionsMethodListWorkflowArtifacts: - result, payload, err := listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination) - return attachIFC(result), payload, err + return listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -426,14 +413,6 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err) } - // attachIFC adds the IFC label to a successful Actions result when - // IFC labels are enabled. Workflow runs, jobs, artifacts, usage, - // and log URLs reflect attacker-influenceable run output, so - // integrity is untrusted; confidentiality follows repo visibility. - attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult { - return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult) - } - var resourceIDInt int64 var parseErr error switch method { @@ -449,23 +428,17 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an switch method { case actionsMethodGetWorkflow: - result, payload, err := getWorkflow(ctx, client, owner, repo, resourceID) - return attachIFC(result), payload, err + return getWorkflow(ctx, client, owner, repo, resourceID) case actionsMethodGetWorkflowRun: - result, payload, err := getWorkflowRun(ctx, client, owner, repo, resourceIDInt) - return attachIFC(result), payload, err + return getWorkflowRun(ctx, client, owner, repo, resourceIDInt) case actionsMethodGetWorkflowJob: - result, payload, err := getWorkflowJob(ctx, client, owner, repo, resourceIDInt) - return attachIFC(result), payload, err + return getWorkflowJob(ctx, client, owner, repo, resourceIDInt) case actionsMethodDownloadWorkflowArtifact: - result, payload, err := downloadWorkflowArtifact(ctx, client, owner, repo, resourceIDInt) - return attachIFC(result), payload, err + return downloadWorkflowArtifact(ctx, client, owner, repo, resourceIDInt) case actionsMethodGetWorkflowRunUsage: - result, payload, err := getWorkflowRunUsage(ctx, client, owner, repo, resourceIDInt) - return attachIFC(result), payload, err + return getWorkflowRunUsage(ctx, client, owner, repo, resourceIDInt) case actionsMethodGetWorkflowRunLogsURL: - result, payload, err := getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt) - return attachIFC(result), payload, err + return getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -636,22 +609,12 @@ For single job logs, provide job_id. For all failed jobs in a run, provide run_i return utils.NewToolResultError("job_id is required when failed_only is false"), nil, nil } - // attachIFC adds the IFC label to a successful result when IFC - // labels are enabled. Job logs echo attacker-influenceable run - // output, so integrity is untrusted; confidentiality follows repo - // visibility. - attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult { - return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult) - } - if failedOnly && runID > 0 { // Handle failed-only mode: get logs for all failed jobs in the workflow run - result, payload, err := handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize()) - return attachIFC(result), payload, err + return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize()) } else if jobID > 0 { // Handle single job mode - result, payload, err := handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize()) - return attachIFC(result), payload, err + return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize()) } 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 diff --git a/pkg/github/code_scanning.go b/pkg/github/code_scanning.go index 6d0ef073b5..b4326e6d7e 100644 --- a/pkg/github/code_scanning.go +++ b/pkg/github/code_scanning.go @@ -7,7 +7,6 @@ import ( "net/http" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -86,12 +85,7 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, nil } - result := utils.NewToolResultText(string(r)) - // Code scanning alerts are access-restricted regardless of repo - // visibility and embed attacker-influenceable code snippets, so the - // label is always private-untrusted. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert()) - return result, codeScanningAlertOutput(alert), nil + return utils.NewToolResultText(string(r)), codeScanningAlertOutput(alert), nil }, normalizeSecurityIntegerArguments("alertNumber"), ) @@ -193,12 +187,7 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, nil } - result := utils.NewToolResultText(string(r)) - // Code scanning alerts are access-restricted regardless of repo - // visibility and embed attacker-influenceable code snippets, so the - // label is always private-untrusted. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert()) - return result, mapSecurityOutputs(alerts, codeScanningAlertOutput), nil + return utils.NewToolResultText(string(r)), mapSecurityOutputs(alerts, codeScanningAlertOutput), nil }, normalizeSecurityIntegerArguments("page", "perPage"), normalizeTypedReadArguments(nil, false), diff --git a/pkg/github/context_tools.go b/pkg/github/context_tools.go index 93f9eed04c..ffd66b8858 100644 --- a/pkg/github/context_tools.go +++ b/pkg/github/context_tools.go @@ -9,7 +9,6 @@ import ( "time" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -178,9 +177,7 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool { }, } - result := MarshalledTextResult(minimalUser) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGetMe()) - return result, minimalUser, nil + return MarshalledTextResult(minimalUser), minimalUser, nil }, ) } @@ -283,10 +280,6 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool { } result := MarshalledTextResult(organizations) - // Team membership is maintained by GitHub and cannot be forged by - // outside contributors (trusted). Org team rosters are visible only - // to org members, so confidentiality is private. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelTeam()) if organizations == nil { organizations = []OrganizationTeams{} } @@ -355,10 +348,6 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool { } result := MarshalledTextResult(members) - // Team membership is maintained by GitHub and cannot be forged by - // outside contributors (trusted). A team's member roster is visible - // only to org members, so confidentiality is private. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelTeam()) if members == nil { members = []string{} } diff --git a/pkg/github/context_tools_test.go b/pkg/github/context_tools_test.go index 041f2d168c..181c58ff5e 100644 --- a/pkg/github/context_tools_test.go +++ b/pkg/github/context_tools_test.go @@ -460,72 +460,6 @@ func TestContextToolsTypedRegistration(t *testing.T) { assert.Nil(t, result.StructuredContent, "handler errors must not expose a successful typed output") } -func Test_GetMe_IFC_FeatureFlag(t *testing.T) { - t.Parallel() - - serverTool := GetMe(translations.NullTranslationHelper) - - mockUser := &github.User{ - Login: new("testuser"), - HTMLURL: new("https://github.com/testuser"), - CreatedAt: &github.Timestamp{Time: time.Now()}, - } - mockedHTTPClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetUser: mockResponse(t, http.StatusOK, mockUser), - }) - - depsWithIFCFeature := func(enabled bool) *BaseDeps { - return NewBaseDeps( - mustNewGHClient(t, mockedHTTPClient), nil, nil, nil, - translations.NullTranslationHelper, - FeatureFlags{}, - 0, - func(_ context.Context, flagName string) (bool, error) { - return flagName == FeatureFlagIFCLabels && enabled, nil - }, - stubExporters(), - ) - } - - t.Run("feature disabled omits ifc label from result meta", func(t *testing.T) { - deps := depsWithIFCFeature(false) - handler := serverTool.Handler(deps) - - request := createMCPRequest(map[string]any{}) - 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("feature enabled includes ifc label in result meta", func(t *testing.T) { - deps := depsWithIFCFeature(true) - handler := serverTool.Handler(deps) - - request := createMCPRequest(map[string]any{}) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta, "result meta should be set when IFC labels are enabled") - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - - var ifcMap map[string]any - err = json.Unmarshal(ifcJSON, &ifcMap) - require.NoError(t, err) - - assert.Equal(t, "trusted", ifcMap["integrity"]) - // get_me returns the caller's private repo/gist counts, which are not - // part of the public profile, so confidentiality is private. - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) -} - func Test_GetTeams(t *testing.T) { t.Parallel() diff --git a/pkg/github/dependabot.go b/pkg/github/dependabot.go index 32669788c0..c95d75bf8c 100644 --- a/pkg/github/dependabot.go +++ b/pkg/github/dependabot.go @@ -8,7 +8,6 @@ import ( "net/http" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -87,12 +86,7 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, err } - result := utils.NewToolResultText(string(r)) - // Dependabot alerts are access-restricted regardless of repo - // visibility and embed attacker-influenceable advisory text, so the - // label is always private-untrusted. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert()) - return result, dependabotAlertOutput(alert), nil + return utils.NewToolResultText(string(r)), dependabotAlertOutput(alert), nil }, normalizeSecurityIntegerArguments("alertNumber"), ) @@ -198,12 +192,7 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), DependabotAlertsOutput{}, err } - result := utils.NewToolResultText(string(r)) - // Dependabot alerts are access-restricted regardless of repo - // visibility and embed attacker-influenceable advisory text, so the - // label is always private-untrusted. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert()) - return result, output, nil + return utils.NewToolResultText(string(r)), output, nil }, normalizeTypedReadArguments(nil, false), ) diff --git a/pkg/github/discussions.go b/pkg/github/discussions.go index 2de3b3fd95..c05dfc0fd8 100644 --- a/pkg/github/discussions.go +++ b/pkg/github/discussions.go @@ -6,7 +6,6 @@ import ( "fmt" "strings" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/sanitize" "github.com/github/github-mcp-server/v2/pkg/scopes" @@ -279,9 +278,6 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool return nil, nil, fmt.Errorf("failed to marshal discussions: %w", err) } result := utils.NewToolResultText(string(out)) - // Discussion content is user-authored (untrusted); confidentiality - // follows repo visibility. - result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoUserContent) items := make([]*DiscussionListItemOutput, 0, len(discussions)) for _, discussion := range discussions { items = append(items, &DiscussionListItemOutput{ @@ -395,9 +391,6 @@ 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.LabelRepoUserContent) output := &DiscussionOutput{ Number: int(d.Number), Title: sanitize.PlainText(string(d.Title)), Body: sanitize.Content(string(d.Body)), HTMLURL: string(d.URL), @@ -596,11 +589,7 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve return nil, nil, fmt.Errorf("failed to marshal comments: %w", err) } - 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.LabelRepoUserContent) - return result, &DiscussionCommentsOutput{ + return utils.NewToolResultText(string(out)), &DiscussionCommentsOutput{ Comments: discussionCommentOutputs(comments), PageInfo: DiscussionPageInfoOutput{ HasNextPage: bool(pageInfo.HasNextPage), HasPreviousPage: bool(pageInfo.HasPreviousPage), @@ -1126,9 +1115,6 @@ func ListDiscussionCategories(t translations.TranslationHelperFunc) inventory.Se return nil, nil, fmt.Errorf("failed to marshal discussion categories: %w", err) } result := utils.NewToolResultText(string(out)) - // Discussion categories are repo-defined structural metadata - // (trusted); confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) items := make([]DiscussionCategoryItemOutput, 0, len(categories)) for _, category := range categories { items = append(items, DiscussionCategoryItemOutput{ID: category["id"], Name: category["name"]}) diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 1bf679f5c9..1c1fc8f9c8 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -13,9 +13,6 @@ const MCPAppsDisableFormDeferralFeatureFlag = "mcp_apps_disable_form_deferral" // FeatureFlagCSVOutput is the feature flag name for CSV output on list tools. const FeatureFlagCSVOutput = "csv_output" -// FeatureFlagIFCLabels is the feature flag name for IFC security labels in tool results. -const FeatureFlagIFCLabels = "ifc_labels" - // FeatureFlagFileBlame is the feature flag name for the get_file_blame tool, // which exposes git blame information for a file. It is gated so the extra tool // is not advertised by default, keeping the tool surface small unless opted in. @@ -46,7 +43,6 @@ const FeatureFlagThreadResolutionReason = "thread_resolution_reason" var AllowedFeatureFlags = []string{ MCPAppsDisableFormDeferralFeatureFlag, FeatureFlagCSVOutput, - FeatureFlagIFCLabels, FeatureFlagIssuesGranular, FeatureFlagPullRequestsGranular, FeatureFlagFileBlame, diff --git a/pkg/github/feature_flags_test.go b/pkg/github/feature_flags_test.go index 173a8cfd12..2f0f55c15b 100644 --- a/pkg/github/feature_flags_test.go +++ b/pkg/github/feature_flags_test.go @@ -167,23 +167,12 @@ func TestResolveFeatureFlags(t *testing.T) { insidersMode: true, expectedFlags: InsidersFeatureFlags, }, - { - name: "insiders mode does not auto-enable ifc labels", - enabledFeatures: nil, - insidersMode: true, - unexpectedFlags: []string{FeatureFlagIFCLabels}, - }, { name: "insiders mode does not disable MCP Apps form deferral", enabledFeatures: nil, insidersMode: true, unexpectedFlags: []string{MCPAppsDisableFormDeferralFeatureFlag}, }, - { - name: "ifc_labels can be directly enabled", - enabledFeatures: []string{FeatureFlagIFCLabels}, - expectedFlags: []string{FeatureFlagIFCLabels}, - }, { name: "unknown flags are filtered out", enabledFeatures: []string{"unknown_flag", "another_unknown"}, diff --git a/pkg/github/fields_filtering_test.go b/pkg/github/fields_filtering_test.go index b54480252f..5f269cf576 100644 --- a/pkg/github/fields_filtering_test.go +++ b/pkg/github/fields_filtering_test.go @@ -274,7 +274,7 @@ func Test_SearchIssues_FieldsTelemetry(t *testing.T) { // getIssueQueryType; see Test_ListIssues for the canonical copies. const listIssuesFieldsFieldValuesSelection = "issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}" -const listIssuesFieldsQuery = "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + listIssuesFieldsFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" +const listIssuesFieldsQuery = "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + listIssuesFieldsFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" func listIssuesFieldsMockClient() *http.Client { vars := map[string]any{ @@ -314,7 +314,6 @@ func listIssuesFieldsMockClient() *http.Client { }, "totalCount": 1, }, - "isPrivate": false, }, }) matcher := githubv4mock.NewQueryMatcher(listIssuesFieldsQuery, vars, response) diff --git a/pkg/github/find_duplicate.go b/pkg/github/find_duplicate.go index b276d608da..ffaa3790ba 100644 --- a/pkg/github/find_duplicate.go +++ b/pkg/github/find_duplicate.go @@ -9,7 +9,6 @@ import ( "strconv" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -161,11 +160,7 @@ func FindDuplicate(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to marshal duplicate candidates", err), nil, nil } - // Candidate issue titles are user-authored content scoped to the source - // repository, so classify the result like issue_read. - result := utils.NewToolResultText(string(r)) - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoUserContent) - return result, candidates, nil + return utils.NewToolResultText(string(r)), candidates, nil }, normalizeIssueStrings([]string{"owner", "repo"}, nil), normalizeIssueIntegers([]string{"issue_number"}, []string{"page", "perPage"}), normalizeDuplicateThreshold) st.FeatureRule = featureEnabledRule(FeatureFlagDuplicateDetection) diff --git a/pkg/github/find_duplicate_test.go b/pkg/github/find_duplicate_test.go index e6c2dc233e..733d3ae1ec 100644 --- a/pkg/github/find_duplicate_test.go +++ b/pkg/github/find_duplicate_test.go @@ -373,102 +373,3 @@ func Test_FindDuplicate_Errors(t *testing.T) { getErrorResult(t, result) }) } - -func Test_FindDuplicate_IFCLabels(t *testing.T) { - serverTool := FindDuplicate(translations.NullTranslationHelper) - - rankedResults := []map[string]any{ - { - "issue": map[string]any{ - "number": 585, - "title": "Improve the onboarding flow for new users", - "state": "open", - "html_url": "https://github.com/owner/repo/issues/585", - }, - "score": 1.93, - "confidence": "high", - "likely_duplicate": true, - }, - } - - // makeClient serves the semantic-similarity endpoint plus the repo lookup - // that the IFC labeler uses to resolve visibility. - makeClient := func(isPrivate bool, repoStatus int) *http.Client { - handlers := map[string]http.HandlerFunc{ - string(endpointSemanticallySimilar): mockResponse(t, http.StatusOK, rankedResults), - } - if repoStatus != 0 && repoStatus != http.StatusOK { - handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom") - } else { - handlers[GetReposByOwnerByRepo] = mockResponse(t, http.StatusOK, map[string]any{ - "name": "repo", - "private": isPrivate, - }) - } - return MockHTTPClientWithHandlers(handlers) - } - - req := map[string]any{ - "owner": "owner", - "repo": "repo", - "issue_number": float64(769), - } - - t.Run("flag disabled omits ifc label", func(t *testing.T) { - deps := BaseDeps{Client: mustNewGHClient(t, makeClient(false, 0))} - handler := serverTool.Handler(deps) - request := createMCPRequest(req) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - assert.Nil(t, result.Meta) - }) - - t.Run("flag enabled on public repo emits public untrusted", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeClient(false, 0)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - request := createMCPRequest(req) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - t.Run("flag enabled on private repo emits private trusted", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeClient(true, 0)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - request := createMCPRequest(req) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("visibility lookup failure omits label but still succeeds", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeClient(false, http.StatusInternalServerError)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - request := createMCPRequest(req) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails") - if result.Meta != nil { - _, hasIFC := result.Meta["ifc"] - assert.False(t, hasIFC, "label must be omitted on visibility lookup failure") - } - }) -} diff --git a/pkg/github/gists.go b/pkg/github/gists.go index cca3529644..e1e24d3ac8 100644 --- a/pkg/github/gists.go +++ b/pkg/github/gists.go @@ -8,7 +8,6 @@ import ( "net/http" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -101,9 +100,7 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } - result := utils.NewToolResultText(string(r)) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGistList()) - return result, projectGists(gists), nil + return utils.NewToolResultText(string(r)), projectGists(gists), nil }, gitGistNormalizer(validateListGistsArguments), ) @@ -163,9 +160,7 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } - result := utils.NewToolResultText(string(r)) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGist()) - return result, &GistReadOutput{Gist: projectGist(gist)}, nil + return utils.NewToolResultText(string(r)), &GistReadOutput{Gist: projectGist(gist)}, nil }, gitGistNormalizer(validateGetGistArguments), ) diff --git a/pkg/github/git.go b/pkg/github/git.go index 872e342527..e263065746 100644 --- a/pkg/github/git.go +++ b/pkg/github/git.go @@ -7,7 +7,6 @@ import ( "strings" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -173,13 +172,7 @@ func GetRepositoryTree(t translations.TranslationHelperFunc) inventory.ServerToo return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // The repository tree exposes committed file structure; in public - // repos anyone can land content via a PR (untrusted), in private - // repos only collaborators can (trusted). Confidentiality follows - // repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelCommitContents) - return result, projectRepositoryTree(response), nil + return utils.NewToolResultText(string(r)), projectRepositoryTree(response), nil }, gitGistNormalizer(validateTreeArguments), ) diff --git a/pkg/github/ifc_labels.go b/pkg/github/ifc_labels.go deleted file mode 100644 index a1b7ca60ad..0000000000 --- a/pkg/github/ifc_labels.go +++ /dev/null @@ -1,171 +0,0 @@ -package github - -import ( - "context" - - "github.com/github/github-mcp-server/v2/pkg/ifc" - "github.com/google/go-github/v92/github" - "github.com/modelcontextprotocol/go-sdk/mcp" -) - -// setIFCLabel writes the given IFC security label into a tool result's _meta -// under the "ifc" key, allocating the Meta map if necessary. -func setIFCLabel(r *mcp.CallToolResult, label ifc.SecurityLabel) { - if r.Meta == nil { - r.Meta = mcp.Meta{} - } - 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 -// advisories, team membership, notification subjects). -// -// 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 !shouldAttachIFCLabel(ctx, deps, r) { - return r - } - setIFCLabel(r, label) - return r -} - -// attachRepoVisibilityIFCLabel attaches an IFC label derived from a single -// repository's visibility to a successful tool result when IFC labels are -// enabled. The concrete label is produced by labelFn, which receives whether -// the repository is private. -// -// The repository visibility is resolved via FetchRepoIsPrivate. Consistent -// with the other IFC-labeled tools, if the visibility lookup fails the label -// is omitted rather than risking a misclassification. Error results and the -// disabled-feature case are left untouched. -func attachRepoVisibilityIFCLabel( - ctx context.Context, - deps ToolDependencies, - client *github.Client, - owner, repo string, - r *mcp.CallToolResult, - labelFn func(isPrivate bool) ifc.SecurityLabel, -) *mcp.CallToolResult { - if !shouldAttachIFCLabel(ctx, deps, r) { - return r - } - isPrivate, err := FetchRepoIsPrivate(ctx, client, owner, repo) - if err != nil { - return r - } - setIFCLabel(r, labelFn(isPrivate)) - return r -} - -// ifcSearchPostProcessOption returns a searchOption that attaches IFC labels to -// a multi-repository search result. The feature-flag check is centralized here -// (mirroring the attach* helpers above) rather than in each search tool -// handler: when IFC labels are disabled it returns a no-op option, so callers -// can pass it unconditionally to searchHandler. -func ifcSearchPostProcessOption(ctx context.Context, deps ToolDependencies) searchOption { - if !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { - return func(*searchConfig) {} - } - return withSearchPostProcess(searchIssuesIFCPostProcess(deps)) -} - -// attachRepoVisibilityIFCLabelLazy is like attachRepoVisibilityIFCLabel but -// resolves the REST client itself, only when IFC labels are enabled. It is used -// by tools whose handler holds a GraphQL client (or no client yet) and would -// otherwise have to acquire a REST client solely to compute the label. The -// feature-flag check is centralized here so callers can invoke it -// unconditionally; if the client cannot be obtained or the visibility lookup -// fails, the label is omitted rather than risking a misclassification. -func attachRepoVisibilityIFCLabelLazy( - ctx context.Context, - deps ToolDependencies, - owner, repo string, - r *mcp.CallToolResult, - labelFn func(isPrivate bool) ifc.SecurityLabel, -) *mcp.CallToolResult { - if !shouldAttachIFCLabel(ctx, deps, r) { - return r - } - client, err := deps.GetClient(ctx) - if err != nil { - return r - } - return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, labelFn) -} - -// attachJoinedIFCLabel attaches an IFC label computed by joining a set of -// 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 !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 -// callers invoke it unconditionally) and caches the repository visibility -// lookup across calls, so a handler that returns from many branches only pays -// for one FetchRepoIsPrivate call. A failed visibility lookup is not cached, so -// a later return path can retry; on persistent failure the label is omitted -// rather than risking a misclassification. -func newRepoVisibilityIFCLabeler( - ctx context.Context, - deps ToolDependencies, - client *github.Client, - owner, repo string, - labelFn func(isPrivate bool) ifc.SecurityLabel, -) func(*mcp.CallToolResult) *mcp.CallToolResult { - var ( - known bool - isPrivate bool - ) - return func(r *mcp.CallToolResult) *mcp.CallToolResult { - if r == nil || r.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { - return r - } - if !known { - p, err := FetchRepoIsPrivate(ctx, client, owner, repo) - if err != nil { - return r - } - isPrivate = p - known = true - } - setIFCLabel(r, labelFn(isPrivate)) - return r - } -} diff --git a/pkg/github/issue_fields.go b/pkg/github/issue_fields.go index 9937f74bfd..590a72fee5 100644 --- a/pkg/github/issue_fields.go +++ b/pkg/github/issue_fields.go @@ -8,7 +8,6 @@ import ( ghcontext "github.com/github/github-mcp-server/v2/pkg/context" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/translations" "github.com/github/github-mcp-server/v2/pkg/utils" @@ -152,17 +151,7 @@ func ListIssueFields(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultErrorFromErr("failed to marshal issue fields", err), nil, nil } - result := utils.NewToolResultText(string(r)) - // Issue field definitions are repo/org structural metadata - // (trusted). When scoped to a specific repo, confidentiality - // follows that repo's visibility; for an org-level lookup (no - // repo) it is conservatively treated as private. - if repo == "" { - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoMetadata(true)) - } else { - result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) - } - return result, issueFieldOutputs(fields), nil + return utils.NewToolResultText(string(r)), issueFieldOutputs(fields), nil }, normalizeIssueStrings([]string{"owner"}, []string{"repo"})) return st } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 30005854aa..3a29a344bc 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -14,7 +14,6 @@ import ( ghcontext "github.com/github/github-mcp-server/v2/pkg/context" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/lockdown" "github.com/github/github-mcp-server/v2/pkg/sanitize" @@ -550,12 +549,10 @@ type issueFragmentWithoutFieldValues struct { // Common interface for all issue query types type IssueQueryResult interface { GetIssueFragment() IssueQueryFragment - GetIsPrivate() bool } type issueQueryResultWithoutFieldValues interface { getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues - GetIsPrivate() bool } type IssueQueryFragment struct { @@ -583,60 +580,52 @@ type issueQueryFragmentWithoutFieldValues struct { // ListIssuesQuery is the root query structure for fetching issues with optional label filtering. type ListIssuesQuery struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"` - IsPrivate githubv4.Boolean + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"` } `graphql:"repository(owner: $owner, name: $repo)"` } // ListIssuesQueryTypeWithLabels is the query structure for fetching issues with optional label filtering. type ListIssuesQueryTypeWithLabels struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"` - IsPrivate githubv4.Boolean + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues})"` } `graphql:"repository(owner: $owner, name: $repo)"` } // ListIssuesQueryWithSince is the query structure for fetching issues without label filtering but with since filtering. type ListIssuesQueryWithSince struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})"` - IsPrivate githubv4.Boolean + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})"` } `graphql:"repository(owner: $owner, name: $repo)"` } // ListIssuesQueryTypeWithLabelsWithSince is the query structure for fetching issues with both label and since filtering. type ListIssuesQueryTypeWithLabelsWithSince struct { Repository struct { - Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})"` - IsPrivate githubv4.Boolean + Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since, issueFieldValues: $issueFieldValues})"` } `graphql:"repository(owner: $owner, name: $repo)"` } type listIssuesQueryWithoutFieldValues struct { Repository struct { - Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"` - IsPrivate githubv4.Boolean + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"` } `graphql:"repository(owner: $owner, name: $repo)"` } type listIssuesQueryWithLabelsWithoutFieldValues struct { Repository struct { - Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"` - IsPrivate githubv4.Boolean + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"` } `graphql:"repository(owner: $owner, name: $repo)"` } type listIssuesQueryWithSinceWithoutFieldValues struct { Repository struct { - Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` - IsPrivate githubv4.Boolean + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` } `graphql:"repository(owner: $owner, name: $repo)"` } type listIssuesQueryWithLabelsAndSinceWithoutFieldValues struct { Repository struct { - Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` - IsPrivate githubv4.Boolean + Issues issueQueryFragmentWithoutFieldValues `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"` } `graphql:"repository(owner: $owner, name: $repo)"` } @@ -655,60 +644,34 @@ func (q *ListIssuesQueryTypeWithLabels) GetIssueFragment() IssueQueryFragment { return q.Repository.Issues } -func (q *ListIssuesQueryTypeWithLabels) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) } - func (q *ListIssuesQuery) GetIssueFragment() IssueQueryFragment { return q.Repository.Issues } -func (q *ListIssuesQuery) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) } - func (q *ListIssuesQueryWithSince) GetIssueFragment() IssueQueryFragment { return q.Repository.Issues } -func (q *ListIssuesQueryWithSince) GetIsPrivate() bool { return bool(q.Repository.IsPrivate) } - func (q *ListIssuesQueryTypeWithLabelsWithSince) GetIssueFragment() IssueQueryFragment { return q.Repository.Issues } -func (q *ListIssuesQueryTypeWithLabelsWithSince) GetIsPrivate() bool { - return bool(q.Repository.IsPrivate) -} - func (q *listIssuesQueryWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { return q.Repository.Issues } -func (q *listIssuesQueryWithoutFieldValues) GetIsPrivate() bool { - return bool(q.Repository.IsPrivate) -} - func (q *listIssuesQueryWithLabelsWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { return q.Repository.Issues } -func (q *listIssuesQueryWithLabelsWithoutFieldValues) GetIsPrivate() bool { - return bool(q.Repository.IsPrivate) -} - func (q *listIssuesQueryWithSinceWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { return q.Repository.Issues } -func (q *listIssuesQueryWithSinceWithoutFieldValues) GetIsPrivate() bool { - return bool(q.Repository.IsPrivate) -} - func (q *listIssuesQueryWithLabelsAndSinceWithoutFieldValues) getIssueFragmentWithoutFieldValues() issueQueryFragmentWithoutFieldValues { return q.Repository.Issues } -func (q *listIssuesQueryWithLabelsAndSinceWithoutFieldValues) GetIsPrivate() bool { - return bool(q.Repository.IsPrivate) -} - func getIssueQueryType(hasLabels bool, hasSince bool) IssueQueryResult { switch { case hasLabels && hasSince: @@ -873,27 +836,17 @@ func IssueRead(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to get GitHub graphql client", err), nil, nil } - // 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.LabelRepoUserContent) - switch method { case "get": - result, output, err := getIssue(ctx, client, deps, owner, repo, issueNumber) - return attachIFC(result), output, err + return getIssue(ctx, client, deps, owner, repo, issueNumber) case "get_comments": - result, output, err := getIssueComments(ctx, client, deps, owner, repo, issueNumber, pagination) - return attachIFC(result), output, err + return getIssueComments(ctx, client, deps, owner, repo, issueNumber, pagination) case "get_sub_issues": - result, output, err := getSubIssues(ctx, client, deps, owner, repo, issueNumber, pagination) - return attachIFC(result), output, err + return getSubIssues(ctx, client, deps, owner, repo, issueNumber, pagination) case "get_parent": - result, output, err := getIssueParent(ctx, gqlClient, deps, owner, repo, issueNumber) - return attachIFC(result), output, err + return getIssueParent(ctx, gqlClient, deps, owner, repo, issueNumber) case "get_labels": - result, output, err := getIssueLabels(ctx, gqlClient, owner, repo, issueNumber) - return attachIFC(result), output, err + return getIssueLabels(ctx, gqlClient, owner, repo, issueNumber) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1361,9 +1314,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to marshal issue types", err), nil, nil } - result := utils.NewToolResultText(string(r)) - result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) - return result, issueTypeOutputs(issueTypes), nil + return utils.NewToolResultText(string(r)), issueTypeOutputs(issueTypes), nil } issueTypes, resp, err := client.Organizations.ListIssueTypes(ctx, owner) @@ -1385,13 +1336,7 @@ func ListIssueTypes(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to marshal issue types", err), nil, nil } - result := utils.NewToolResultText(string(r)) - // Issue types are org-defined structural metadata (trusted, not - // attacker-authored). They are scoped to an organization rather - // than a single repo, so confidentiality is conservatively treated - // as private (restricted to org members). - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepoMetadata(true)) - return result, issueTypeOutputs(issueTypes), nil + return utils.NewToolResultText(string(r)), issueTypeOutputs(issueTypes), nil }, normalizeIssueStrings([]string{"owner"}, []string{"repo"})) return st } @@ -2035,79 +1980,14 @@ func SearchIssues(t translations.TranslationHelperFunc, opts ...ToolOption) inve }, scopes.PublicRead(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input SearchIssuesInput) (*mcp.CallToolResult, SearchIssuesOutput, error) { - options := []searchOption{ifcSearchPostProcessOption(ctx, deps)} - options = append(options, withFieldsFiltering(deps, "search_issues", input.Fields)) - result, response, err := searchIssuesHandler(ctx, deps, input, mode, options...) + result, response, err := searchIssuesHandler(ctx, deps, input, mode, + withFieldsFiltering(deps, "search_issues", input.Fields)) return result, structuredSearchIssuesOutput(response, input.Fields), err }, normalizeTypedReadArguments(nil, false), ) } -// searchIssuesIFCPostProcess returns a searchPostProcessFn that attaches the -// IFC label for a search_issues result. It looks up the visibility (and, for -// private repos, collaborators) of every repository represented in the search -// payload and joins the labels via ifc.LabelSearchIssues. If any per-repo -// lookup fails the label is omitted to avoid misclassifying the result. -func searchIssuesIFCPostProcess(deps ToolDependencies) searchPostProcessFn { - return func(ctx context.Context, result *github.IssuesSearchResult, callResult *mcp.CallToolResult) { - if callResult == nil || callResult.IsError || result == nil { - return - } - - client, err := deps.GetClient(ctx) - if err != nil { - return - } - - uniqueRepos := uniqueSearchIssuesRepos(result) - visibilities := make([]bool, 0, len(uniqueRepos)) - for _, r := range uniqueRepos { - isPrivate, err := FetchRepoIsPrivate(ctx, client, r.owner, r.repo) - if err != nil { - return - } - visibilities = append(visibilities, isPrivate) - } - - if callResult.Meta == nil { - callResult.Meta = mcp.Meta{} - } - callResult.Meta["ifc"] = ifc.LabelSearchIssues(visibilities) - } -} - -type searchIssuesRepoRef struct { - owner string - repo string -} - -// uniqueSearchIssuesRepos extracts the owner/repo pairs of every issue in the -// search result, preserving order of first appearance and deduplicating. -func uniqueSearchIssuesRepos(result *github.IssuesSearchResult) []searchIssuesRepoRef { - if result == nil { - return nil - } - seen := make(map[string]struct{}) - var out []searchIssuesRepoRef - for _, issue := range result.Issues { - if issue == nil { - continue - } - owner, repo, ok := parseRepositoryURL(issue.GetRepositoryURL()) - if !ok { - continue - } - key := owner + "/" + repo - if _, dup := seen[key]; dup { - continue - } - seen[key] = struct{}{} - out = append(out, searchIssuesRepoRef{owner: owner, repo: repo}) - } - return out -} - // parseRepositoryURL extracts the owner and repo from a GitHub API repository // URL of the form https://api.github.com/repos/{owner}/{repo}. func parseRepositoryURL(repoURL string) (string, string, bool) { @@ -2528,9 +2408,8 @@ func fetchIssueReadEnrichment(ctx context.Context, gqlClient *githubv4.Client, n return enrichment, nil } -// searchIssuesHandler runs the REST issues search, enriches each hit with custom field values -// fetched via a single follow-up GraphQL nodes() query, and applies any post-process options -// (e.g. IFC labelling). +// searchIssuesHandler runs the REST issues search and enriches each hit with custom field values +// fetched via a single follow-up GraphQL nodes() query. func searchIssuesHandler(ctx context.Context, deps ToolDependencies, input SearchIssuesInput, mode searchMode, options ...searchOption) (*mcp.CallToolResult, SearchIssuesResponse, error) { const errorPrefix = "failed to search issues" @@ -2621,11 +2500,7 @@ func searchIssuesHandler(ctx context.Context, deps ToolDependencies, input Searc recordFieldsUsageFor(ctx, cfg.fieldsDeps, cfg.fieldsTool, response, filtered, len(r)) } - callResult := utils.NewToolResultText(string(r)) - if cfg.postProcess != nil { - cfg.postProcess(ctx, result, callResult) - } - return callResult, output, nil + return utils.NewToolResultText(string(r)), output, nil } // IssueWriteUIResourceURI is the URI for the issue_write tool's MCP App UI resource. @@ -3965,10 +3840,8 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { issueFieldsErr := client.Query(ctxWithFeatures, issueQuery, vars) var resp MinimalIssuesResponse - var isPrivate bool if issueFieldsErr == nil { resp = convertToMinimalIssuesResponse(issueQuery.GetIssueFragment()) - isPrivate = issueQuery.GetIsPrivate() } else { if len(fieldFilters) > 0 || !isUnsupportedListIssuesIssueFieldsError(issueFieldsErr) { return ghErrors.NewGitHubGraphQLErrorResponse( @@ -3994,7 +3867,6 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { } resp = convertToMinimalIssuesResponseWithoutFieldValues(issueQueryWithoutFieldValues.getIssueFragmentWithoutFieldValues()) - isPrivate = issueQueryWithoutFieldValues.GetIsPrivate() } filtered := false @@ -4019,9 +3891,7 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool { recordFieldsUsageFor(ctx, deps, "list_issues", resp, filtered, len(r)) - result := utils.NewToolResultText(string(r)) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelListIssues(isPrivate)) - return result, structuredListIssuesOutput(resp, input.Fields), nil + return utils.NewToolResultText(string(r)), structuredListIssuesOutput(resp, input.Fields), nil }, normalizeTypedReadArguments([]string{"state", "orderBy", "direction"}, true), ) diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 79eeb22100..a6f163db74 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -312,122 +312,6 @@ func Test_GetIssue(t *testing.T) { } } -func Test_IssueRead_IFC_InsidersMode(t *testing.T) { - t.Parallel() - - serverTool := IssueRead(translations.NullTranslationHelper) - - mockIssue := &github.Issue{ - Number: new(1), - Title: new("Test"), - Body: new("body"), - State: new("open"), - HTMLURL: new("https://github.com/octocat/repo/issues/1"), - User: &github.User{Login: new("u")}, - } - - mockComments := []*github.IssueComment{ - {Body: new("hello"), User: &github.User{Login: new("u")}}, - } - - makeMockClient := func(isPrivate bool, repoStatus int) *http.Client { - handlers := map[string]http.HandlerFunc{ - GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue), - GetReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockComments), - } - if repoStatus != 0 && repoStatus != http.StatusOK { - handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom") - } else { - handlers[GetReposByOwnerByRepo] = mockResponse(t, http.StatusOK, map[string]any{ - "name": "repo", - "private": isPrivate, - }) - } - return MockHTTPClientWithHandlers(handlers) - } - - getReq := map[string]any{ - "method": "get", - "owner": "octocat", - "repo": "repo", - "issue_number": float64(1), - } - commentsReq := map[string]any{ - "method": "get_comments", - "owner": "octocat", - "repo": "repo", - "issue_number": float64(1), - } - - t.Run("insiders mode disabled omits ifc label", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, 0)), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(getReq) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - assert.Nil(t, result.Meta) - }) - - t.Run("insiders mode enabled on public repo emits public untrusted", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, 0)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(getReq) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - 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), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(commentsReq) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, http.StatusInternalServerError)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(getReq) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails") - - if result.Meta != nil { - _, hasIFC := result.Meta["ifc"] - assert.False(t, hasIFC, "ifc label should be omitted when visibility lookup fails") - } - }) -} - func Test_GetIssue_FieldValues(t *testing.T) { // The raw REST issue_field_values are always cleared. Enriched field_values are // only populated via GraphQL when the issue has a node ID; this issue has none, @@ -1432,171 +1316,6 @@ func Test_SearchIssues(t *testing.T) { } } -func Test_SearchIssues_IFC_InsidersMode(t *testing.T) { - t.Parallel() - - serverTool := SearchIssues(translations.NullTranslationHelper) - - makeIssue := func(owner, repo string, number int) *github.Issue { - return &github.Issue{ - Number: new(number), - Title: new("issue"), - State: new("open"), - RepositoryURL: new("https://api.github.com/repos/" + owner + "/" + repo), - User: &github.User{Login: new("u")}, - } - } - - type repoFixture struct { - owner string - repo string - isPrivate bool - repoStatus int - } - - repoHandlers := func(repos []repoFixture) map[string]http.HandlerFunc { - repoByPath := map[string]repoFixture{} - for _, r := range repos { - repoByPath["/repos/"+r.owner+"/"+r.repo] = r - } - return map[string]http.HandlerFunc{ - GetReposByOwnerByRepo: func(w http.ResponseWriter, req *http.Request) { - r, ok := repoByPath[req.URL.Path] - if !ok { - w.WriteHeader(http.StatusNotFound) - return - } - if r.repoStatus != 0 && r.repoStatus != http.StatusOK { - w.WriteHeader(r.repoStatus) - return - } - body, _ := json.Marshal(map[string]any{ - "name": r.repo, - "private": r.isPrivate, - }) - w.WriteHeader(http.StatusOK) - _, _ = w.Write(body) - }, - } - } - - makeMockClient := func(searchResult *github.IssuesSearchResult, repos []repoFixture) *http.Client { - handlers := repoHandlers(repos) - handlers[GetSearchIssues] = mockResponse(t, http.StatusOK, searchResult) - return MockHTTPClientWithHandlers(handlers) - } - - reqParams := map[string]any{"query": "bug"} - - t.Run("insiders mode disabled omits ifc label", func(t *testing.T) { - searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}} - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})), - } - handler := serverTool.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) - }) - - t.Run("insiders mode all public emits public untrusted", func(t *testing.T) { - searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "public-repo", 1)}} - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{{owner: "octocat", repo: "public-repo"}})), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode mixed public and private emits private untrusted", func(t *testing.T) { - searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{ - makeIssue("octocat", "private-repo", 1), - makeIssue("octocat", "public-repo", 2), - }} - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{ - {owner: "octocat", repo: "private-repo", isPrivate: true}, - {owner: "octocat", repo: "public-repo"}, - })), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) { - searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{makeIssue("octocat", "broken", 1)}} - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, []repoFixture{ - {owner: "octocat", repo: "broken", repoStatus: http.StatusInternalServerError}, - })), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails") - - if result.Meta != nil { - _, hasIFC := result.Meta["ifc"] - assert.False(t, hasIFC, "ifc label should be omitted when visibility lookup fails") - } - }) - - t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) { - searchResult := &github.IssuesSearchResult{Issues: []*github.Issue{}} - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(searchResult, nil)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) -} - -func unmarshalIFC(t *testing.T, ifcLabel any) map[string]any { - t.Helper() - require.NotNil(t, ifcLabel, "ifc label should be present") - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - return ifcMap -} - func Test_SearchIssues_FieldValuesEnrichment(t *testing.T) { serverTool := SearchIssues(translations.NullTranslationHelper) @@ -3341,7 +3060,6 @@ func Test_ListIssues(t *testing.T) { }, "totalCount": 2, }, - "isPrivate": false, }, }) @@ -3357,7 +3075,6 @@ func Test_ListIssues(t *testing.T) { }, "totalCount": 2, }, - "isPrivate": false, }, }) @@ -3373,7 +3090,6 @@ func Test_ListIssues(t *testing.T) { }, "totalCount": 1, }, - "isPrivate": false, }, }) @@ -3507,8 +3223,8 @@ func Test_ListIssues(t *testing.T) { // Define the actual query strings that match the implementation issueFieldValuesSelection := "issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}" - qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" - qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + qBasicNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}," + issueFieldValuesSelection + "},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { @@ -3667,7 +3383,6 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { }, "totalCount": 2, }, - "isPrivate": false, }, }, }) @@ -3679,13 +3394,13 @@ func Test_ListIssues_IssueFieldsSchemaCompatibility(t *testing.T) { const selection = "{nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}" switch { case hasLabels && hasSince: - return "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})" + selection + ",isPrivate}}" + return "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})" + selection + "}}" case hasLabels: - return "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})" + selection + ",isPrivate}}" + return "query($after:String$direction:OrderDirection!$first:Int!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})" + selection + "}}" case hasSince: - return "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})" + selection + ",isPrivate}}" + return "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$since:DateTime!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})" + selection + "}}" default: - return "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})" + selection + ",isPrivate}}" + return "query($after:String$direction:OrderDirection!$first:Int!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})" + selection + "}}" } } @@ -4034,7 +3749,6 @@ func Test_ListIssues_FieldFilters(t *testing.T) { "pageInfo": pageInfo, "totalCount": 1, }, - "isPrivate": false, }, }) @@ -4091,8 +3805,8 @@ func Test_ListIssues_FieldFilters(t *testing.T) { ) } - qNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" - qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" + qNoLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" + qWithLabels := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$labels:[String!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}" baseVars := func() map[string]any { return map[string]any{ @@ -4413,132 +4127,6 @@ func (t *headerCaptureTransport) RoundTrip(req *http.Request) (*http.Response, e return t.inner.RoundTrip(req) } -func Test_ListIssues_IFC_InsidersMode(t *testing.T) { - t.Parallel() - - serverTool := ListIssues(translations.NullTranslationHelper) - - mockIssues := []map[string]any{ - { - "number": 1, - "title": "An issue", - "body": "body", - "state": "OPEN", - "databaseId": 1, - "createdAt": "2023-01-01T00:00:00Z", - "updatedAt": "2023-01-01T00:00:00Z", - "author": map[string]any{"login": "user1"}, - "labels": map[string]any{"nodes": []map[string]any{}}, - "comments": map[string]any{"totalCount": 0}, - }, - } - - pageInfo := map[string]any{ - "hasNextPage": false, - "hasPreviousPage": false, - "startCursor": "", - "endCursor": "", - } - - makeResponse := func(isPrivate bool) githubv4mock.GQLResponse { - return githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ - "issues": map[string]any{ - "nodes": mockIssues, - "pageInfo": pageInfo, - "totalCount": 1, - }, - "isPrivate": isPrivate, - }, - }) - } - - query := "query($after:String$direction:OrderDirection!$first:Int!$issueFieldValues:[IssueFieldValueFilter!]!$orderBy:IssueOrderField!$owner:String!$repo:String!$states:[IssueState!]!){repository(owner: $owner, name: $repo){issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {issueFieldValues: $issueFieldValues}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},assignees(first: 100){nodes{login}},comments{totalCount},issueFieldValues(first: 25){nodes{__typename,... on IssueFieldDateValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldNumberValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},valueNumber: value},... on IssueFieldSingleSelectValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value},... on IssueFieldTextValue{field{... on IssueFieldDate{name,fullDatabaseId},... on IssueFieldNumber{name,fullDatabaseId},... on IssueFieldSingleSelect{name,fullDatabaseId},... on IssueFieldText{name,fullDatabaseId}},value}}}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}" - - vars := map[string]any{ - "owner": "octocat", - "repo": "hello", - "states": []any{"OPEN", "CLOSED"}, - "orderBy": "CREATED_AT", - "direction": "DESC", - "first": float64(30), - "after": (*string)(nil), - "issueFieldValues": []any{}, - } - - reqParams := map[string]any{"owner": "octocat", "repo": "hello"} - - t.Run("insiders mode disabled omits ifc label from result meta", func(t *testing.T) { - matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(false)) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) - deps := BaseDeps{ - GQLClient: gqlClient, - } - handler := serverTool.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 insiders mode is disabled") - }) - - t.Run("insiders mode enabled on public repo emits public untrusted label", func(t *testing.T) { - matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(false)) - gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher)) - deps := BaseDeps{ - GQLClient: gqlClient, - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - 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{ - GQLClient: gqlClient, - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) -} - func TestIssueWriteUpdatesIssueType(t *testing.T) { tests := []struct { name string diff --git a/pkg/github/labels.go b/pkg/github/labels.go index c9479e5062..5bd4539607 100644 --- a/pkg/github/labels.go +++ b/pkg/github/labels.go @@ -7,7 +7,6 @@ import ( "strings" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -115,11 +114,7 @@ func getLabelLegacy(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to marshal label: %w", err) } - result := utils.NewToolResultText(string(out)) - // Labels are structural repo metadata defined by collaborators - // (trusted); confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) - return result, nil, nil + return utils.NewToolResultText(string(out)), nil, nil }, ) } @@ -260,11 +255,7 @@ func listLabelsLegacy(t translations.TranslationHelperFunc) inventory.ServerTool return nil, nil, fmt.Errorf("failed to marshal labels: %w", err) } - result := utils.NewToolResultText(string(out)) - // Labels are structural repo metadata defined by collaborators - // (trusted); confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata) - return result, nil, nil + return utils.NewToolResultText(string(out)), nil, nil }, ) } diff --git a/pkg/github/notifications.go b/pkg/github/notifications.go index 332d2ffd3b..4717849edb 100644 --- a/pkg/github/notifications.go +++ b/pkg/github/notifications.go @@ -9,7 +9,6 @@ import ( "time" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -413,13 +412,7 @@ func GetNotificationDetails(t translations.TranslationHelperFunc) inventory.Serv return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } - result := utils.NewToolResultText(string(r)) - // A notification subject points at an issue, PR, comment, or - // discussion whose content is user-authored (untrusted). It is - // delivered to a specific recipient and may reference private - // repositories, so confidentiality is private. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelNotificationDetails()) - return result, notificationOutput(thread), nil + return utils.NewToolResultText(string(r)), notificationOutput(thread), nil }, ) } diff --git a/pkg/github/projects.go b/pkg/github/projects.go index e55b2a29d6..2bd9fae6b8 100644 --- a/pkg/github/projects.go +++ b/pkg/github/projects.go @@ -13,7 +13,6 @@ import ( ghcontext "github.com/github/github-mcp-server/v2/pkg/context" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/sanitize" "github.com/github/github-mcp-server/v2/pkg/scopes" @@ -81,22 +80,12 @@ type statusUpdateNode struct { } } -type projectVisibility struct { - Public githubv4.Boolean -} - -type statusUpdateNodeWithProject struct { - statusUpdateNode - Project projectVisibility -} - type statusUpdateConnection struct { Nodes []statusUpdateNode PageInfo PageInfoFragment } type statusUpdatesProject struct { - Public githubv4.Boolean StatusUpdates statusUpdateConnection `graphql:"statusUpdates(first: $first, after: $after, orderBy: {field: CREATED_AT, direction: DESC})"` } @@ -117,7 +106,7 @@ type statusUpdatesOrgQuery struct { // statusUpdateNodeQuery is the GraphQL query for fetching a single status update by node ID. type statusUpdateNodeQuery struct { Node struct { - StatusUpdate statusUpdateNodeWithProject `graphql:"... on ProjectV2StatusUpdate"` + StatusUpdate statusUpdateNode `graphql:"... on ProjectV2StatusUpdate"` } `graphql:"node(id: $id)"` } @@ -153,20 +142,14 @@ type projectViewVisibleFieldNode struct { } `graphql:"... on ProjectV2SingleSelectField"` } -type projectViewNodeWithProject struct { - projectViewNode - Project projectVisibility -} - type projectViewConnection struct { Nodes []projectViewNode PageInfo PageInfoFragment } type projectViewsProject struct { - ID githubv4.ID - Public githubv4.Boolean - Views projectViewConnection `graphql:"views(first: $first, after: $after, last: $last, before: $before)"` + ID githubv4.ID + Views projectViewConnection `graphql:"views(first: $first, after: $after, last: $last, before: $before)"` } type projectViewsUserQuery struct { @@ -183,7 +166,7 @@ type projectViewsOrgQuery struct { type projectViewNodeQuery struct { Node struct { - ProjectView projectViewNodeWithProject `graphql:"... on ProjectV2View"` + ProjectView projectViewNode `graphql:"... on ProjectV2View"` } `graphql:"node(id: $id)"` } @@ -383,9 +366,7 @@ Use this tool to list projects for a user or organization, or list project field switch method { case projectsMethodListProjects: - result, visibilities, payload, err := listProjects(ctx, client, args, owner, ownerType) - result = attachJoinedIFCLabel(ctx, deps, result, visibilities, ifc.LabelProjectList) - return result, payload, err + return listProjects(ctx, client, args, owner, ownerType) case projectsMethodListProjectFields, projectsMethodListProjectItems, projectsMethodListProjectStatusUpdates, projectsMethodListProjectViews: // All other methods require project_number and ownerType detection projectNumber, err := RequiredInt(args, "project_number") @@ -401,43 +382,25 @@ Use this tool to list projects for a user or organization, or list project field switch method { case projectsMethodListProjectFields: - result, payload, err := listProjectFields(ctx, client, args, owner, ownerType) - if shouldAttachIFCLabel(ctx, deps, result) { - isPrivate, visibilityErr := FetchProjectIsPrivate(ctx, client, owner, ownerType, projectNumber) - if visibilityErr == nil { - result = attachProjectVisibilityIFCLabel(ctx, deps, result, isPrivate, ifc.LabelProject) - } - } - return result, payload, err + return listProjectFields(ctx, client, args, owner, ownerType) case projectsMethodListProjectItems: gqlClient, gqlErr := deps.GetGQLClient(ctx) if gqlErr != nil { return utils.NewToolResultError(gqlErr.Error()), nil, nil } - result, payload, err := listProjectItems(ctx, client, gqlClient, args, owner, ownerType) - if shouldAttachIFCLabel(ctx, deps, result) { - isPrivate, visibilityErr := FetchProjectIsPrivate(ctx, client, owner, ownerType, projectNumber) - if visibilityErr == nil { - result = attachProjectVisibilityIFCLabel(ctx, deps, result, isPrivate, ifc.LabelProjectContent) - } - } - return result, payload, err + return listProjectItems(ctx, client, gqlClient, args, owner, ownerType) case projectsMethodListProjectStatusUpdates: gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, isPrivate, payload, err := listProjectStatusUpdates(ctx, gqlClient, args, owner, ownerType) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) - return result, payload, err + return listProjectStatusUpdates(ctx, gqlClient, args, owner, ownerType) case projectsMethodListProjectViews: gqlClient, err := deps.GetGQLClient(ctx) if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, isPrivate, payload, err := listProjectViews(ctx, gqlClient, args, owner, ownerType) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) - return result, payload, err + return listProjectViews(ctx, gqlClient, args, owner, ownerType) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -543,9 +506,7 @@ Use this tool to get details about individual projects, project fields, project if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, isPrivate, payload, err := getProjectStatusUpdate(ctx, gqlClient, statusUpdateID) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) - return result, payload, err + return getProjectStatusUpdate(ctx, gqlClient, statusUpdateID) } if method == projectsMethodGetProjectView { viewID, err := RequiredParam[string](args, "view_id") @@ -556,9 +517,7 @@ Use this tool to get details about individual projects, project fields, project if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, isPrivate, payload, err := getProjectView(ctx, gqlClient, viewID) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProjectContent(isPrivate)) - return result, payload, err + return getProjectView(ctx, gqlClient, viewID) } owner, err := RequiredParam[string](args, "owner") @@ -591,22 +550,13 @@ Use this tool to get details about individual projects, project fields, project switch method { case projectsMethodGetProject: - result, isPrivate, payload, err := getProject(ctx, client, owner, ownerType, projectNumber) - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelProject(isPrivate)) - return result, payload, err + return getProject(ctx, client, owner, ownerType, projectNumber) case projectsMethodGetProjectField: fieldID, err := RequiredBigInt(args, "field_id") if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, payload, err := getProjectField(ctx, client, owner, ownerType, projectNumber, fieldID) - if shouldAttachIFCLabel(ctx, deps, result) { - isPrivate, visibilityErr := FetchProjectIsPrivate(ctx, client, owner, ownerType, projectNumber) - if visibilityErr == nil { - result = attachProjectVisibilityIFCLabel(ctx, deps, result, isPrivate, ifc.LabelProject) - } - } - return result, payload, err + return getProjectField(ctx, client, owner, ownerType, projectNumber, fieldID) case projectsMethodGetProjectItem: itemID, err := RequiredBigInt(args, "item_id") if err != nil { @@ -637,14 +587,7 @@ Use this tool to get details about individual projects, project fields, project } fields = append(fields, resolvedIDs...) } - result, payload, err := getProjectItem(ctx, client, owner, ownerType, projectNumber, itemID, fields) - if shouldAttachIFCLabel(ctx, deps, result) { - isPrivate, visibilityErr := FetchProjectIsPrivate(ctx, client, owner, ownerType, projectNumber) - if visibilityErr == nil { - result = attachProjectVisibilityIFCLabel(ctx, deps, result, isPrivate, ifc.LabelProjectContent) - } - } - return result, payload, err + return getProjectItem(ctx, client, owner, ownerType, projectNumber, itemID, fields) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1052,15 +995,15 @@ func ProjectsWrite(t translations.TranslationHelperFunc) inventory.ServerTool { // Helper functions for consolidated projects tools -func listProjects(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, []bool, any, error) { +func listProjects(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) { queryStr, err := OptionalParam[string](args, "query") if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } pagination, err := extractPaginationOptionsFromArgs(args) if err != nil { - return utils.NewToolResultError(err.Error()), nil, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } var resp *github.Response @@ -1083,7 +1026,7 @@ func listProjects(ctx context.Context, client *github.Client, args map[string]an "failed to list projects", resp, err, - ), nil, nil, nil + ), nil, nil } default: projects, resp, err = client.Projects.ListUserProjects(ctx, owner, opts) @@ -1092,7 +1035,7 @@ func listProjects(ctx context.Context, client *github.Client, args map[string]an "failed to list projects", resp, err, - ), nil, nil, nil + ), nil, nil } } @@ -1113,18 +1056,18 @@ func listProjects(ctx context.Context, client *github.Client, args map[string]an r, err := json.Marshal(response) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return utils.NewToolResultText(string(r)), projectVisibilities(minimalProjects), nil, nil + return utils.NewToolResultText(string(r)), nil, nil } - return nil, nil, nil, fmt.Errorf("unexpected state in listProjects") + return nil, nil, fmt.Errorf("unexpected state in listProjects") } // listProjectsFromBothOwnerTypes fetches projects from both user and org endpoints // when owner_type is not specified, combining the results with owner_type labels. -func listProjectsFromBothOwnerTypes(ctx context.Context, client *github.Client, owner string, opts *github.ListProjectsOptions) (*mcp.CallToolResult, []bool, any, error) { +func listProjectsFromBothOwnerTypes(ctx context.Context, client *github.Client, owner string, opts *github.ListProjectsOptions) (*mcp.CallToolResult, any, error) { var minimalProjects []MinimalProject var resp *github.Response @@ -1155,7 +1098,7 @@ func listProjectsFromBothOwnerTypes(ctx context.Context, client *github.Client, // If both failed, return error if (userErr != nil || userResp == nil || userResp.StatusCode != http.StatusOK) && (orgErr != nil || orgResp == nil || orgResp.StatusCode != http.StatusOK) { - return utils.NewToolResultError(fmt.Sprintf("failed to list projects for owner '%s': not found as user or organization", owner)), nil, nil, nil + return utils.NewToolResultError(fmt.Sprintf("failed to list projects for owner '%s': not found as user or organization", owner)), nil, nil } response := map[string]any{ @@ -1169,21 +1112,9 @@ func listProjectsFromBothOwnerTypes(ctx context.Context, client *github.Client, r, err := json.Marshal(response) if err != nil { - return nil, nil, nil, fmt.Errorf("failed to marshal response: %w", err) - } - return utils.NewToolResultText(string(r)), projectVisibilities(minimalProjects), nil, nil -} - -func projectVisibilities(projects []MinimalProject) []bool { - visibilities := make([]bool, 0, len(projects)) - for _, project := range projects { - isPrivate := true - if project.Public != nil { - isPrivate = !*project.Public - } - visibilities = append(visibilities, isPrivate) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return visibilities + return utils.NewToolResultText(string(r)), nil, nil } func listProjectFields(ctx context.Context, client *github.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) { @@ -1313,54 +1244,40 @@ func listProjectItems(ctx context.Context, client *github.Client, gqlClient *git return utils.NewToolResultText(string(r)), nil, nil } -func fetchProjectV2(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int) (*github.ProjectV2, *github.Response, error) { - if ownerType == "org" { - return client.Projects.GetOrganizationProject(ctx, owner, projectNumber) - } - return client.Projects.GetUserProject(ctx, owner, projectNumber) -} +func getProject(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, any, error) { + var resp *github.Response + var project *github.ProjectV2 + var err error -// FetchProjectIsPrivate returns whether a GitHub Project is private. -func FetchProjectIsPrivate(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int) (bool, error) { - project, resp, err := fetchProjectV2(ctx, client, owner, ownerType, projectNumber) - if resp != nil && resp.Body != nil { - defer func() { _ = resp.Body.Close() }() - } - if err != nil { - return false, err - } - if resp == nil || resp.StatusCode != http.StatusOK { - return false, fmt.Errorf("failed to fetch project visibility") + if ownerType == "org" { + project, resp, err = client.Projects.GetOrganizationProject(ctx, owner, projectNumber) + } else { + project, resp, err = client.Projects.GetUserProject(ctx, owner, projectNumber) } - return !project.GetPublic(), nil -} - -func getProject(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int) (*mcp.CallToolResult, bool, any, error) { - project, resp, err := fetchProjectV2(ctx, client, owner, ownerType, projectNumber) if err != nil { return ghErrors.NewGitHubAPIErrorResponse(ctx, "failed to get project", resp, err, - ), false, nil, nil + ), nil, nil } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { body, err := io.ReadAll(resp.Body) if err != nil { - return nil, false, nil, fmt.Errorf("failed to read response body: %w", err) + return nil, nil, fmt.Errorf("failed to read response body: %w", err) } - return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project", resp, body), false, nil, nil + return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get project", resp, body), nil, nil } minimalProject := convertToMinimalProject(project) r, err := json.Marshal(minimalProject) if err != nil { - return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return utils.NewToolResultText(string(r)), !project.GetPublic(), nil, nil + return utils.NewToolResultText(string(r)), nil, nil } func getProjectField(ctx context.Context, client *github.Client, owner, ownerType string, projectNumber int, fieldID int64) (*mcp.CallToolResult, any, error) { @@ -1774,19 +1691,19 @@ func createProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, } // listProjectStatusUpdates lists status updates for a project via GraphQL. -func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, bool, any, error) { +func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) { if ownerType != "user" && ownerType != "org" { - return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil } projectNumber, err := RequiredInt(args, "project_number") if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } perPage, err := optionalProjectsPerPage(args) if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } if perPage > MaxProjectsPerPage { perPage = MaxProjectsPerPage @@ -1797,7 +1714,7 @@ func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, a afterCursor, err := OptionalParam[string](args, "after") if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } vars := map[string]any{ @@ -1813,26 +1730,23 @@ func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, a var nodes []statusUpdateNode var pi PageInfoFragment - var isPrivate bool if ownerType == "org" { var q statusUpdatesOrgQuery if err := gqlClient.Query(ctx, &q, vars); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectStatusUpdateListFailedError, err)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectStatusUpdateListFailedError, err)), nil, nil } project := q.Organization.ProjectV2 nodes = project.StatusUpdates.Nodes pi = project.StatusUpdates.PageInfo - isPrivate = !bool(project.Public) } else { var q statusUpdatesUserQuery if err := gqlClient.Query(ctx, &q, vars); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectStatusUpdateListFailedError, err)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectStatusUpdateListFailedError, err)), nil, nil } project := q.User.ProjectV2 nodes = project.StatusUpdates.Nodes pi = project.StatusUpdates.PageInfo - isPrivate = !bool(project.Public) } updates := make([]MinimalProjectStatusUpdate, 0, len(nodes)) @@ -1852,34 +1766,33 @@ func listProjectStatusUpdates(ctx context.Context, gqlClient *githubv4.Client, a r, err := json.Marshal(response) if err != nil { - return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return utils.NewToolResultText(string(r)), isPrivate, nil, nil + return utils.NewToolResultText(string(r)), nil, nil } // getProjectStatusUpdate fetches a single status update by its node ID via GraphQL. -func getProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, statusUpdateID string) (*mcp.CallToolResult, bool, any, error) { +func getProjectStatusUpdate(ctx context.Context, gqlClient *githubv4.Client, statusUpdateID string) (*mcp.CallToolResult, any, error) { var q statusUpdateNodeQuery vars := map[string]any{ "id": githubv4.ID(statusUpdateID), } if err := gqlClient.Query(ctx, &q, vars); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectStatusUpdateGetFailedError, err)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectStatusUpdateGetFailedError, err)), nil, nil } if q.Node.StatusUpdate.ID == nil || q.Node.StatusUpdate.ID == "" { - return utils.NewToolResultError(fmt.Sprintf("%s: node is not a ProjectV2StatusUpdate or was not found", ProjectStatusUpdateGetFailedError)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: node is not a ProjectV2StatusUpdate or was not found", ProjectStatusUpdateGetFailedError)), nil, nil } - update := convertToMinimalStatusUpdate(q.Node.StatusUpdate.statusUpdateNode) - isPrivate := !bool(q.Node.StatusUpdate.Project.Public) + update := convertToMinimalStatusUpdate(q.Node.StatusUpdate) r, err := json.Marshal(update) if err != nil { - return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return utils.NewToolResultText(string(r)), isPrivate, nil, nil + return utils.NewToolResultText(string(r)), nil, nil } func convertToMinimalProjectView(node projectViewNode) MinimalProjectView { @@ -1932,32 +1845,32 @@ func parseProjectViewLayout(layout string) (githubv4.ProjectV2ViewLayout, error) } } -func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, bool, any, error) { +func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string) (*mcp.CallToolResult, any, error) { if ownerType != "user" && ownerType != "org" { - return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("invalid owner_type %q: must be \"user\" or \"org\"", ownerType)), nil, nil } projectNumber, err := RequiredInt(args, "project_number") if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } perPage, err := optionalProjectsPerPage(args) if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } if perPage < 1 || perPage > MaxProjectsPerPage { perPage = MaxProjectsPerPage } after, err := OptionalParam[string](args, "after") if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } before, err := OptionalParam[string](args, "before") if err != nil { - return utils.NewToolResultError(err.Error()), false, nil, nil + return utils.NewToolResultError(err.Error()), nil, nil } if after != "" && before != "" { - return utils.NewToolResultError("provide either 'after' or 'before', not both"), false, nil, nil + return utils.NewToolResultError("provide either 'after' or 'before', not both"), nil, nil } vars := map[string]any{ @@ -1986,18 +1899,18 @@ func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[ if ownerType == "org" { var query projectViewsOrgQuery if err := gqlClient.Query(ctx, &query, vars); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), nil, nil } project = query.Organization.ProjectV2 } else { var query projectViewsUserQuery if err := gqlClient.Query(ctx, &query, vars); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewListFailedError, err)), nil, nil } project = query.User.ProjectV2 } if project.ID == nil || project.ID == "" { - return utils.NewToolResultError(fmt.Sprintf("%s: project was not found", ProjectViewListFailedError)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: project was not found", ProjectViewListFailedError)), nil, nil } views := make([]MinimalProjectView, 0, len(project.Views.Nodes)) @@ -2015,27 +1928,27 @@ func listProjectViews(ctx context.Context, gqlClient *githubv4.Client, args map[ } result, err := json.Marshal(response) if err != nil { - return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return utils.NewToolResultText(string(result)), !bool(project.Public), nil, nil + return utils.NewToolResultText(string(result)), nil, nil } -func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID string) (*mcp.CallToolResult, bool, any, error) { +func getProjectView(ctx context.Context, gqlClient *githubv4.Client, viewID string) (*mcp.CallToolResult, any, error) { var query projectViewNodeQuery vars := map[string]any{"id": githubv4.ID(viewID)} if err := gqlClient.Query(ctx, &query, vars); err != nil { - return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewGetFailedError, err)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: %v", ProjectViewGetFailedError, err)), nil, nil } if query.Node.ProjectView.ID == nil || query.Node.ProjectView.ID == "" { - return utils.NewToolResultError(fmt.Sprintf("%s: node is not a ProjectV2View or was not found", ProjectViewGetFailedError)), false, nil, nil + return utils.NewToolResultError(fmt.Sprintf("%s: node is not a ProjectV2View or was not found", ProjectViewGetFailedError)), nil, nil } - view := convertToMinimalProjectView(query.Node.ProjectView.projectViewNode) + view := convertToMinimalProjectView(query.Node.ProjectView) result, err := json.Marshal(view) if err != nil { - return nil, false, nil, fmt.Errorf("failed to marshal response: %w", err) + return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - return utils.NewToolResultText(string(result)), !bool(query.Node.ProjectView.Project.Public), nil, nil + return utils.NewToolResultText(string(result)), nil, nil } func projectViewVisibleFieldsInput(ctx context.Context, gqlClient *githubv4.Client, args map[string]any, owner, ownerType string, projectNumber int) (*ProjectV2ViewConfigurationInput, error) { diff --git a/pkg/github/projects_test.go b/pkg/github/projects_test.go index 9cbbdf26bf..7fac275db1 100644 --- a/pkg/github/projects_test.go +++ b/pkg/github/projects_test.go @@ -477,151 +477,6 @@ func Test_detectOwnerType(t *testing.T) { }) } -func Test_ProjectsList_IFC_InsidersMode(t *testing.T) { - toolDef := ProjectsList(translations.NullTranslationHelper) - - t.Run("list_projects joins returned project visibilities", func(t *testing.T) { - projects := []map[string]any{ - {"id": 1, "node_id": "NODE1", "title": "Public Project", "public": true}, - {"id": 2, "node_id": "NODE2", "title": "Private Project", "public": false}, - } - mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetOrgsProjectsV2: mockResponse(t, http.StatusOK, projects), - }) - client := mustNewGHClient(t, mockedClient) - deps := BaseDeps{ - Client: client, - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "list_projects", - "owner": "octo-org", - "owner_type": "org", - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("list_project_fields uses project metadata label", func(t *testing.T) { - fields := []map[string]any{{"id": 101, "name": "Status", "data_type": "single_select"}} - project := map[string]any{"id": 1, "node_id": "NODE1", "title": "Private Project", "public": false} - mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetOrgsProjectsV2FieldsByProject: mockResponse(t, http.StatusOK, fields), - GetOrgsProjectsV2ByProject: mockResponse(t, http.StatusOK, project), - }) - client := mustNewGHClient(t, mockedClient) - deps := BaseDeps{ - Client: client, - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "list_project_fields", - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("list_project_items uses project content label", func(t *testing.T) { - items := []map[string]any{verbosePullRequestProjectItemFixture()} - project := map[string]any{"id": 1, "node_id": "NODE1", "title": "Private Project", "public": false} - mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetOrgsProjectsV2ItemsByProject: mockResponse(t, http.StatusOK, items), - GetOrgsProjectsV2ByProject: mockResponse(t, http.StatusOK, project), - }) - client := mustNewGHClient(t, mockedClient) - deps := BaseDeps{ - Client: client, - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "list_project_items", - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("list_project_status_updates uses GraphQL project visibility", func(t *testing.T) { - gqlMockedClient := githubv4mock.NewMockedHTTPClient( - githubv4mock.NewQueryMatcher( - statusUpdatesOrgQuery{}, - map[string]any{ - "owner": githubv4.String("octo-org"), - "projectNumber": githubv4.Int(1), - "first": githubv4.Int(50), - "after": (*githubv4.String)(nil), - }, - githubv4mock.DataResponse(map[string]any{ - "organization": map[string]any{ - "projectV2": map[string]any{ - "public": true, - "statusUpdates": map[string]any{ - "nodes": []map[string]any{}, - "pageInfo": map[string]any{ - "hasNextPage": false, - "hasPreviousPage": false, - "startCursor": "", - "endCursor": "", - }, - }, - }, - }, - }), - ), - ) - deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), - GQLClient: githubv4.NewClient(gqlMockedClient), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "list_project_status_updates", - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) -} - func Test_ProjectsGet(t *testing.T) { // Verify tool definition once toolDef := ProjectsGet(translations.NullTranslationHelper) @@ -696,79 +551,6 @@ func Test_ProjectsGet_GetProject(t *testing.T) { }) } -func Test_ProjectsGet_IFC_InsidersMode(t *testing.T) { - toolDef := ProjectsGet(translations.NullTranslationHelper) - - t.Run("get_project uses project metadata label", func(t *testing.T) { - project := map[string]any{"id": 123, "node_id": "NODE1", "title": "Private Project", "public": false} - mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetOrgsProjectsV2ByProject: mockResponse(t, http.StatusOK, project), - }) - client := mustNewGHClient(t, mockedClient) - deps := BaseDeps{ - Client: client, - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "get_project", - "owner": "octo-org", - "owner_type": "org", - "project_number": float64(1), - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("get_project_status_update uses GraphQL project visibility", func(t *testing.T) { - gqlMockedClient := githubv4mock.NewMockedHTTPClient( - githubv4mock.NewQueryMatcher( - statusUpdateNodeQuery{}, - map[string]any{ - "id": githubv4.ID("SU_abc123"), - }, - githubv4mock.DataResponse(map[string]any{ - "node": map[string]any{ - "id": "SU_abc123", - "body": "On track", - "status": "ON_TRACK", - "createdAt": "2026-01-15T10:00:00Z", - "startDate": "2026-01-01", - "targetDate": "2026-03-01", - "creator": map[string]any{"login": "octocat"}, - "project": map[string]any{"public": true}, - }, - }), - ), - ) - deps := BaseDeps{ - GQLClient: githubv4.NewClient(gqlMockedClient), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := toolDef.Handler(deps) - request := createMCPRequest(map[string]any{ - "method": "get_project_status_update", - "status_update_id": "SU_abc123", - }) - - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) -} - func Test_ProjectsGet_GetProjectField(t *testing.T) { toolDef := ProjectsGet(translations.NullTranslationHelper) @@ -1682,7 +1464,6 @@ func Test_ProjectsList_ListProjectStatusUpdates(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "user": map[string]any{ "projectV2": map[string]any{ - "public": false, "statusUpdates": map[string]any{ "nodes": []map[string]any{ { @@ -1753,7 +1534,6 @@ func Test_ProjectsGet_GetProjectStatusUpdate(t *testing.T) { "startDate": "2026-01-01", "targetDate": "2026-03-01", "creator": map[string]any{"login": "octocat"}, - "project": map[string]any{"public": false}, }, }), ), diff --git a/pkg/github/projects_v2_test.go b/pkg/github/projects_v2_test.go index f5255e99e5..c1a532afe5 100644 --- a/pkg/github/projects_v2_test.go +++ b/pkg/github/projects_v2_test.go @@ -574,7 +574,7 @@ func Test_ProjectsWrite_CreateIterationField(t *testing.T) { func Test_ProjectsList_ListProjectViews(t *testing.T) { toolDef := ProjectsList(translations.NullTranslationHelper) - t.Run("lists organization views with forward pagination and IFC", func(t *testing.T) { + t.Run("lists organization views with forward pagination", func(t *testing.T) { first := githubv4.Int(2) after := githubv4.String("after-cursor") matcher := githubv4mock.NewQueryMatcher( @@ -590,8 +590,7 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "organization": map[string]any{ "projectV2": map[string]any{ - "id": "PVT_project7", - "public": false, + "id": "PVT_project7", "views": map[string]any{ "nodes": []map[string]any{ { @@ -624,9 +623,8 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { matcher, ) deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), - GQLClient: githubv4.NewClient(gqlClient), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{})), + GQLClient: githubv4.NewClient(gqlClient), } handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ @@ -657,10 +655,6 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { VisibleFields: []int64{101, 202}, }, response.Views[0]) assert.Equal(t, "end-cursor", response.PageInfo["nextCursor"]) - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) }) t.Run("lists user views with backward pagination", func(t *testing.T) { @@ -679,8 +673,7 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { githubv4mock.DataResponse(map[string]any{ "user": map[string]any{ "projectV2": map[string]any{ - "id": "PVT_project8", - "public": true, + "id": "PVT_project8", "views": map[string]any{ "nodes": []map[string]any{}, "pageInfo": map[string]any{ @@ -748,19 +741,18 @@ func Test_ProjectsList_ListProjectViews(t *testing.T) { func Test_ProjectsGet_GetProjectView(t *testing.T) { toolDef := ProjectsGet(translations.NullTranslationHelper) - t.Run("gets a private project view by node ID", func(t *testing.T) { + t.Run("gets a project view by node ID", func(t *testing.T) { gqlClient := githubv4mock.NewMockedHTTPClient( githubv4mock.NewQueryMatcher( projectViewNodeQuery{}, map[string]any{"id": githubv4.ID("PVTV_view1")}, githubv4mock.DataResponse(map[string]any{ "node": map[string]any{ - "id": "PVTV_view1", - "number": 1, - "name": "Ready work", - "layout": "BOARD_LAYOUT", - "filter": "status:Ready", - "project": map[string]any{"public": false}, + "id": "PVTV_view1", + "number": 1, + "name": "Ready work", + "layout": "BOARD_LAYOUT", + "filter": "status:Ready", "configuration": map[string]any{ "visibleFields": map[string]any{ "nodes": []map[string]any{{"databaseId": 101}, {"databaseId": 202}}, @@ -771,8 +763,7 @@ func Test_ProjectsGet_GetProjectView(t *testing.T) { ), ) deps := BaseDeps{ - GQLClient: githubv4.NewClient(gqlClient), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + GQLClient: githubv4.NewClient(gqlClient), } handler := toolDef.Handler(deps) request := createMCPRequest(map[string]any{ @@ -789,9 +780,6 @@ func Test_ProjectsGet_GetProjectView(t *testing.T) { assert.Equal(t, "PVTV_view1", view.ID) assert.Equal(t, "board", view.Layout) assert.Equal(t, []int64{101, 202}, view.VisibleFields) - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) }) t.Run("rejects a missing or wrong node type", func(t *testing.T) { diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index 70fdfa197a..100d8d1237 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -15,7 +15,6 @@ import ( "github.com/shurcooL/githubv4" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/octicons" "github.com/github/github-mcp-server/v2/pkg/scopes" @@ -113,32 +112,17 @@ Possible options: return utils.NewToolResultErrorFromErr("failed to get GitHub client", err), nil, nil } - // attachIFC adds the IFC label to a successful tool result when - // IFC labels are enabled. Pull request content (descriptions, - // diffs, comments, reviews) is user-authored and therefore - // untrusted; confidentiality follows repo visibility. If the - // visibility lookup fails the label is omitted rather than - // misclassifying the result. - attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult { - return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelRepoUserContent) - } - switch method { case "get": - result, output, err := getPullRequest(ctx, client, deps, owner, repo, pullNumber) - return attachIFC(result), output, err + return getPullRequest(ctx, client, deps, owner, repo, pullNumber) case "get_diff": - result, output, err := getPullRequestDiff(ctx, client, deps, owner, repo, pullNumber) - return attachIFC(result), output, err + return getPullRequestDiff(ctx, client, deps, owner, repo, pullNumber) case "get_status": - result, output, err := getPullRequestStatus(ctx, client, owner, repo, pullNumber) - return attachIFC(result), output, err + return getPullRequestStatus(ctx, client, owner, repo, pullNumber) case "get_files": - result, output, err := getPullRequestFiles(ctx, client, deps, owner, repo, pullNumber, pagination) - return attachIFC(result), output, err + return getPullRequestFiles(ctx, client, deps, owner, repo, pullNumber, pagination) case "get_commits": - result, output, err := getPullRequestCommits(ctx, client, deps, owner, repo, pullNumber, pagination) - return attachIFC(result), output, err + return getPullRequestCommits(ctx, client, deps, owner, repo, pullNumber, pagination) case "get_review_comments": gqlClient, err := deps.GetGQLClient(ctx) if err != nil { @@ -148,17 +132,14 @@ Possible options: if err != nil { return utils.NewToolResultError(err.Error()), nil, nil } - result, output, err := getPullRequestReviewComments(ctx, gqlClient, deps, owner, repo, pullNumber, cursorPagination) - return attachIFC(result), output, err + return getPullRequestReviewComments(ctx, gqlClient, deps, owner, repo, pullNumber, cursorPagination) case "get_reviews": - result, output, err := getPullRequestReviews(ctx, client, deps, owner, repo, pullNumber, pagination) - return attachIFC(result), output, err + return getPullRequestReviews(ctx, client, deps, owner, repo, pullNumber, pagination) case "get_comments": result, output, err := getIssueComments(ctx, client, deps, owner, repo, pullNumber, pagination) - return attachIFC(result), pullRequestCommentsOutput(output), err + return result, pullRequestCommentsOutput(output), err case "get_check_runs": - result, output, err := getPullRequestCheckRuns(ctx, client, owner, repo, pullNumber, pagination) - return attachIFC(result), output, err + return getPullRequestCheckRuns(ctx, client, owner, repo, pullNumber, pagination) default: return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil } @@ -1657,11 +1638,7 @@ func ListPullRequests(t translations.TranslationHelperFunc) inventory.ServerTool recordFieldsUsageFor(ctx, deps, "list_pull_requests", minimalPRs, filtered, len(r)) - result := utils.NewToolResultText(string(r)) - // Pull request titles/bodies are user-authored (untrusted); - // confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelRepoUserContent) - return result, structuredListPullRequestsOutput(minimalPRs, input.Fields), nil + return utils.NewToolResultText(string(r)), structuredListPullRequestsOutput(minimalPRs, input.Fields), nil }, normalizeTypedReadArguments(nil, false), ) @@ -1855,9 +1832,8 @@ func SearchPullRequests(t translations.TranslationHelperFunc) inventory.ServerTo }, scopes.PublicRead(scopes.Repo), func(ctx context.Context, deps ToolDependencies, _ *mcp.CallToolRequest, input SearchIssuesInput) (*mcp.CallToolResult, SearchIssuesOutput, error) { - options := []searchOption{ifcSearchPostProcessOption(ctx, deps)} - options = append(options, withFieldsFiltering(deps, "search_pull_requests", input.Fields)) - result, response, err := searchHandler(ctx, deps.GetClient, input, "pr", "failed to search pull requests", options...) + result, response, err := searchHandler(ctx, deps.GetClient, input, "pr", "failed to search pull requests", + withFieldsFiltering(deps, "search_pull_requests", input.Fields)) return result, structuredSearchIssuesOutput(response, input.Fields), err }, normalizeTypedReadArguments(nil, false), diff --git a/pkg/github/repositories.go b/pkg/github/repositories.go index b884f4888d..1049b4cc77 100644 --- a/pkg/github/repositories.go +++ b/pkg/github/repositories.go @@ -13,7 +13,6 @@ import ( "time" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/octicons" "github.com/github/github-mcp-server/v2/pkg/sanitize" @@ -130,13 +129,7 @@ func GetCommit(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, MinimalCommit{}, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // Commit content is reachable from the repo's history; in public - // repos anyone can land it via a PR (untrusted), in private repos - // only collaborators can (trusted). Confidentiality follows repo - // visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelCommitContents) - return result, minimalCommit, nil + return utils.NewToolResultText(string(r)), minimalCommit, nil }, normalizeTypedReadArguments(nil, false), ) @@ -333,12 +326,7 @@ func ListCommits(t translations.TranslationHelperFunc) inventory.ServerTool { recordFieldsUsageFor(ctx, deps, "list_commits", minimalCommits, filtered, len(r)) - result := utils.NewToolResultText(string(r)) - // Commit content is reachable from the repo's history; integrity - // follows the same public-untrusted / private-trusted rule as file - // contents. Confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelCommitContents) - return result, structuredListCommitsOutput(minimalCommits, input.Fields), nil + return utils.NewToolResultText(string(r)), structuredListCommitsOutput(minimalCommits, input.Fields), nil }, normalizeTypedReadArguments(nil, false), normalizeRepositoryFieldsArguments, @@ -433,12 +421,7 @@ func ListBranches(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // Branches are structural repo metadata that only collaborators - // with push access can create, so integrity is trusted. - // Confidentiality follows repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelRepoMetadata) - return result, minimalBranches, nil + return utils.NewToolResultText(string(r)), minimalBranches, nil }, normalizeTypedReadArguments(nil, false), ) @@ -962,20 +945,6 @@ func repositoryIDForDeletion(ctx context.Context, client *github.Client, owner, return repository.GetID(), nil } -// FetchRepoIsPrivate returns whether a repository is private. It is a thin -// wrapper around the GitHub Repositories.Get endpoint provided as a shared -// helper for IFC label computation across tools. -func FetchRepoIsPrivate(ctx context.Context, client *github.Client, owner, repo string) (bool, error) { - r, resp, err := client.Repositories.Get(ctx, owner, repo) - if resp != nil { - defer func() { _ = resp.Body.Close() }() - } - if err != nil { - return false, err - } - return r.GetPrivate(), nil -} - // GetFileContents creates a tool to get the contents of a file or directory from // a GitHub repository. func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool { @@ -1049,15 +1018,6 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultError("failed to get GitHub client"), nil, nil } - // attachIFC adds the IFC label to a successful tool result when - // IFC labels are enabled. The visibility lookup is performed - // lazily on first use and cached because GetFileContents has - // many possible return paths and would otherwise re-fetch on - // each. If the visibility lookup fails we skip the label rather - // than misclassify the result; the failure is not cached so a - // later return path can retry. - attachIFC := newRepoVisibilityIFCLabeler(ctx, deps, client, owner, repo, ifc.LabelGetFileContents) - rawOpts, fallbackUsed, err := resolveGitReference(ctx, client, owner, repo, ref, sha) if err != nil { return utils.NewToolResultError(fmt.Sprintf("failed to resolve git reference: %s", err)), nil, nil @@ -1083,7 +1043,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool if err != nil { return nil, nil, err } - return repositoryContentResult(attachIFC(res)) + return repositoryContentResult(res) } if fileContent != nil && fileContent.SHA != nil { @@ -1111,10 +1071,10 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool return utils.NewToolResultError(fmt.Sprintf("failed to inspect repository file: %s", err)), nil, nil } if read.Metadata != nil && read.Metadata.Type == "submodule" { - return repositoryContentResult(attachIFC(utils.NewToolResultText(marshalRepositoryPathMetadata(read.Metadata, "", successNote)))) + return repositoryContentResult(utils.NewToolResultText(marshalRepositoryPathMetadata(read.Metadata, "", successNote))) } if read.Metadata != nil && fileContent.GetType() == "symlink" && !read.ContentAvailable { - return repositoryContentResult(attachIFC(utils.NewToolResultText(marshalRepositoryPathMetadata(read.Metadata, "not_returned", successNote)))) + return repositoryContentResult(utils.NewToolResultText(marshalRepositoryPathMetadata(read.Metadata, "not_returned", successNote))) } // Empty files (0 bytes) have no content to decode; return @@ -1130,7 +1090,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool } message := fmt.Sprintf("successfully downloaded empty file (SHA: %s)%s", fileSHA, successNote) message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResource(message, result))) + return repositoryContentResult(utils.NewToolResultResource(message, result)) } // For files >= 1MB, return a ResourceLink instead of content @@ -1148,9 +1108,9 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool resourceLink.Title = fmt.Sprintf("Dereferenced target %s via symlink %s", read.Metadata.ResolvedTargetPath, path) } message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResourceLink( + return repositoryContentResult(utils.NewToolResultResourceLink( message, - resourceLink))) + resourceLink)) } if !read.ContentAvailable { @@ -1178,7 +1138,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool } message := fmt.Sprintf("successfully downloaded text file (SHA: %s)%s", fileSHA, successNote) message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResource(message, result))) + return repositoryContentResult(utils.NewToolResultResource(message, result)) } result := &mcp.ResourceContents{ @@ -1188,7 +1148,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool } message := fmt.Sprintf("successfully downloaded binary file (SHA: %s)%s", fileSHA, successNote) message = repositoryReadMessage(read, message, successNote) - return repositoryContentResult(attachIFC(utils.NewToolResultResource(message, result))) + return repositoryContentResult(utils.NewToolResultResource(message, result)) } else if dirContent != nil { // file content or file SHA is nil which means it's a directory filtered := false @@ -1217,7 +1177,7 @@ func GetFileContents(t translations.TranslationHelperFunc) inventory.ServerTool } } } - return attachIFC(utils.NewToolResultText(string(r))), &RepositoryContentsOutput{Directory: output}, nil + return utils.NewToolResultText(string(r)), &RepositoryContentsOutput{Directory: output}, nil } return utils.NewToolResultError("failed to get file contents"), nil, nil @@ -1950,12 +1910,7 @@ func ListTags(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // Tags are structural repo metadata created by collaborators with - // push access, so integrity is trusted. Confidentiality follows - // repo visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, input.Owner, input.Repo, result, ifc.LabelRepoMetadata) - return result, minimalTags, nil + return utils.NewToolResultText(string(r)), minimalTags, nil }, normalizeTypedReadArguments(nil, false), ) @@ -2037,9 +1992,7 @@ func GetTag(t translations.TranslationHelperFunc) inventory.ServerTool { if err != nil { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) - return result, &RepositoryTagOutput{Reference: repositoryReferenceOutput(ref)}, nil + return utils.NewToolResultText(string(r)), &RepositoryTagOutput{Reference: repositoryReferenceOutput(ref)}, nil } tagObj, resp, err := client.Git.GetTag(ctx, owner, repo, *ref.Object.SHA) @@ -2065,12 +2018,7 @@ func GetTag(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // An annotated tag object is structural repo metadata created by a - // collaborator with push access. Confidentiality follows repo - // visibility. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelRepoMetadata) - return result, &RepositoryTagOutput{Tag: repositoryAnnotatedTagOutput(tagObj)}, nil + return utils.NewToolResultText(string(r)), &RepositoryTagOutput{Tag: repositoryAnnotatedTagOutput(tagObj)}, nil }, ) } @@ -2170,28 +2118,11 @@ func ListReleases(t translations.TranslationHelperFunc) inventory.ServerTool { recordFieldsUsageFor(ctx, deps, "list_releases", minimalReleases, filtered, len(r)) - result := utils.NewToolResultText(string(r)) - // Releases are published by collaborators with push access, so - // integrity is trusted. Confidentiality follows repo visibility, - // but draft releases are visible only to push-access users and are - // not world-readable even on a public repo, so the result is only - // public when no returned release is a draft. - hasDraft := false - for _, mr := range minimalReleases { - if mr.Draft { - hasDraft = true - break - } - } - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, - func(isPrivate bool) ifc.SecurityLabel { - return ifc.LabelRelease(isPrivate, hasDraft) - }) var output []ReleaseListOutput if err := json.Unmarshal(r, &output); err != nil { return nil, nil, fmt.Errorf("failed to decode typed releases: %w", err) } - return result, output, nil + return utils.NewToolResultText(string(r)), output, nil }, normalizeRepositoryFieldsArguments, normalizeRepositoryArguments, @@ -2261,17 +2192,8 @@ func GetLatestRelease(t translations.TranslationHelperFunc) inventory.ServerTool return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // Releases are published by collaborators with push access, so - // integrity is trusted. The "latest release" endpoint never returns - // a draft, but the draft flag is honored defensively: a draft is - // not world-readable even on a public repo. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, - func(isPrivate bool) ifc.SecurityLabel { - return ifc.LabelRelease(isPrivate, release.GetDraft()) - }) output := convertToMinimalRelease(release) - return result, &output, nil + return utils.NewToolResultText(string(r)), &output, nil }, ) } @@ -2350,17 +2272,8 @@ func GetReleaseByTag(t translations.TranslationHelperFunc) inventory.ServerTool return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - result := utils.NewToolResultText(string(r)) - // Releases are published by collaborators with push access, so - // integrity is trusted. A release fetched by tag may be a draft, - // which is visible only to push-access users and not world-readable - // even on a public repo, so a draft forces private confidentiality. - result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, - func(isPrivate bool) ifc.SecurityLabel { - return ifc.LabelRelease(isPrivate, release.GetDraft()) - }) output := convertToMinimalRelease(release) - return result, &output, nil + return utils.NewToolResultText(string(r)), &output, nil }, ) } @@ -2490,19 +2403,7 @@ func ListStarredRepositories(t translations.TranslationHelperFunc) inventory.Ser return nil, nil, fmt.Errorf("failed to marshal starred repositories: %w", err) } - result := utils.NewToolResultText(string(r)) - // A starred-repository listing exposes repository data across many - // repos; reuse the multi-repo join shared with search_repositories - // (public-only results stay public-untrusted, mixed-visibility - // results become private-untrusted, all-private results become - // private-trusted). Visibility is read directly from the response, - // so no extra API call is needed. - visibilities := make([]bool, 0, len(minimalRepos)) - for _, mr := range minimalRepos { - visibilities = append(visibilities, mr.Private) - } - result = attachJoinedIFCLabel(ctx, deps, result, visibilities, ifc.LabelSearchIssues) - return result, minimalRepos, nil + return utils.NewToolResultText(string(r)), minimalRepos, nil }, normalizeRepositoryArguments, ) @@ -3044,9 +2945,7 @@ func GetFileBlame(t translations.TranslationHelperFunc) inventory.ServerTool { return nil, nil, fmt.Errorf("failed to marshal response: %w", err) } - callResult := utils.NewToolResultText(string(payload)) - callResult = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, callResult, ifc.LabelCommitContents) - return callResult, &RepositoryBlameOutput{Result: &result}, nil + return utils.NewToolResultText(string(payload)), &RepositoryBlameOutput{Result: &result}, nil }, normalizeRepositoryBlameArguments, ) @@ -3146,13 +3045,7 @@ func ListRepositoryCollaborators(t translations.TranslationHelperFunc) inventory FirstPage: resp.FirstPage, LastPage: resp.LastPage, } - callResult := MarshalledTextResult(response) - // The collaborator roster is GitHub-maintained membership data - // (trusted, not attacker-authored). Listing collaborators requires - // push access, so the roster is never world-readable — not even on - // a public repo — hence always private confidentiality. - callResult = attachStaticIFCLabel(ctx, deps, callResult, ifc.LabelCollaboratorRoster()) - return callResult, response, nil + return MarshalledTextResult(response), response, nil }, normalizeRepositoryArguments, ) diff --git a/pkg/github/repositories_test.go b/pkg/github/repositories_test.go index 9b40c932ff..2163fc26ca 100644 --- a/pkg/github/repositories_test.go +++ b/pkg/github/repositories_test.go @@ -976,295 +976,6 @@ func Test_GetFileContents_DirectoryFieldsTelemetry(t *testing.T) { assert.Greater(t, full.value, sent.value, "filtering should remove bytes") } -func Test_GetFileContents_IFC_InsidersMode(t *testing.T) { - t.Parallel() - - serverTool := GetFileContents(translations.NullTranslationHelper) - - mockRawContent := []byte("hello") - - makeMockClient := func(isPrivate bool) *http.Client { - return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, "{\"ref\": \"refs/heads/main\", \"object\": {\"sha\": \"\"}}"), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, map[string]any{ - "name": "repo", - "default_branch": "main", - "private": isPrivate, - }), - GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - encodedContent := base64.StdEncoding.EncodeToString(mockRawContent) - fileContent := &github.RepositoryContent{ - Name: new("README.md"), - Path: new("README.md"), - SHA: new(gitBlobSHA(mockRawContent)), - Type: new("file"), - Content: new(encodedContent), - Size: new(len(mockRawContent)), - Encoding: new("base64"), - } - contentBytes, _ := json.Marshal(fileContent) - _, _ = w.Write(contentBytes) - }, - }) - } - - reqParams := map[string]any{ - "owner": "octocat", - "repo": "repo", - "path": "README.md", - "ref": "refs/heads/main", - } - - t.Run("insiders mode disabled omits ifc label from result meta", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false)), - } - handler := serverTool.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 insiders mode is disabled") - }) - - t.Run("insiders mode enabled on public repo emits public untrusted label", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode enabled on private repo emits private trusted label", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(true)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) { - mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, "{\"ref\": \"refs/heads/main\", \"object\": {\"sha\": \"\"}}"), - GetReposByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"), - GetReposContentsByOwnerByRepoByPath: func(w http.ResponseWriter, _ *http.Request) { - w.WriteHeader(http.StatusOK) - encodedContent := base64.StdEncoding.EncodeToString(mockRawContent) - fileContent := &github.RepositoryContent{ - Name: new("README.md"), - Path: new("README.md"), - SHA: new(gitBlobSHA(mockRawContent)), - Type: new("file"), - Content: new(encodedContent), - Size: new(len(mockRawContent)), - Encoding: new("base64"), - } - contentBytes, _ := json.Marshal(fileContent) - _, _ = w.Write(contentBytes) - }, - }) - deps := BaseDeps{ - Client: mustNewGHClient(t, mockedClient), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails") - - if result.Meta != nil { - _, hasIFC := result.Meta["ifc"] - assert.False(t, hasIFC, "ifc label should be omitted when visibility lookup fails") - } - }) - - t.Run("detected symlink preserves ifc label", func(t *testing.T) { - target, content := []byte("target.txt"), []byte("resolved") - linkSHA := gitBlobSHA(target) - client := mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposGitRefByOwnerByRepoByRef: mockResponse(t, http.StatusOK, "{\"ref\": \"refs/heads/main\", \"object\": {\"sha\": \"\"}}"), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, map[string]any{"private": false}), - GetReposContentsByOwnerByRepoByPath: mockResponse(t, http.StatusOK, &github.RepositoryContent{ - Type: new("file"), Path: new("link"), SHA: new(linkSHA), - Content: new(string(content)), Size: new(len(content)), - }), - GetReposGitBlobsByOwnerByRepoByFileSHA: func(w http.ResponseWriter, _ *http.Request) { - _, _ = w.Write(target) - }, - })) - deps := BaseDeps{Client: client, featureChecker: featureCheckerFor(FeatureFlagIFCLabels)} - handler := serverTool.Handler(deps) - request := createMCPRequest(map[string]any{ - "owner": "octocat", "repo": "repo", "path": "link", "ref": "refs/heads/main", - }) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - assert.Equal(t, "symlink", repositoryPathMetadataFromResult(t, result).Type) - require.Contains(t, result.Meta, "ifc") - }) -} - -// Test_GetCommit_IFC_FeatureFlag verifies that the IFC security label is only -// attached to get_commit results when the ifc_labels feature flag is enabled, -// and that the label content matches the commit-contents rule (untrusted on -// public repos, trusted on private). It also confirms the label is omitted -// when the repository visibility lookup fails, so the result is never -// misclassified. get_commit is representative of every tool wired through the -// shared attachRepoVisibilityIFCLabel helper. -func Test_GetCommit_IFC_FeatureFlag(t *testing.T) { - t.Parallel() - - serverTool := GetCommit(translations.NullTranslationHelper) - - mockCommit := &github.RepositoryCommit{ - SHA: new("abc123def456"), - Commit: &github.Commit{Message: new("First commit")}, - HTMLURL: new("https://github.com/owner/repo/commit/abc123def456"), - } - - makeMockClient := func(isPrivate bool) *http.Client { - return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposCommitsByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockCommit), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, map[string]any{ - "name": "repo", - "private": isPrivate, - }), - }) - } - - reqParams := map[string]any{ - "owner": "owner", - "repo": "repo", - "sha": "abc123def456", - } - - t.Run("feature flag disabled omits ifc label from result meta", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false)), - } - handler := serverTool.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("feature flag enabled on public repo emits public untrusted label", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - t.Run("feature flag enabled on private repo emits private trusted label", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(true)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcLabel, ok := result.Meta["ifc"] - require.True(t, ok, "result meta should contain ifc key") - - ifcJSON, err := json.Marshal(ifcLabel) - require.NoError(t, err) - var ifcMap map[string]any - require.NoError(t, json.Unmarshal(ifcJSON, &ifcMap)) - - assert.Equal(t, "trusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("feature flag enabled skips ifc label when visibility lookup fails", func(t *testing.T) { - mockedClient := MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposCommitsByOwnerByRepoByRef: mockResponse(t, http.StatusOK, mockCommit), - GetReposByOwnerByRepo: mockResponse(t, http.StatusInternalServerError, "boom"), - }) - deps := BaseDeps{ - Client: mustNewGHClient(t, mockedClient), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails") - - if result.Meta != nil { - _, hasIFC := result.Meta["ifc"] - assert.False(t, hasIFC, "ifc label should be omitted when visibility lookup fails") - } - }) -} - func Test_ForkRepository(t *testing.T) { // Verify tool definition once serverTool := ForkRepository(translations.NullTranslationHelper) @@ -5159,107 +4870,6 @@ func Test_GetReleaseByTag(t *testing.T) { } } -// Test_GetReleaseByTag_IFC_FeatureFlag verifies the IFC label on -// get_release_by_tag. The label is only present when the ifc_labels flag is -// enabled, and confidentiality is public only for a non-draft release on a -// public repo. A draft release is visible only to push-access users, so even -// on a public repo it must be labeled private. Guards against the same -// under-classification fixed for repository security advisories. -func Test_GetReleaseByTag_IFC_FeatureFlag(t *testing.T) { - t.Parallel() - - serverTool := GetReleaseByTag(translations.NullTranslationHelper) - - makeRelease := func(draft bool) *github.RepositoryRelease { - return &github.RepositoryRelease{ - ID: 1, - TagName: "v1.0.0", - Name: new("v1.0.0"), - Draft: draft, - } - } - - makeMockClient := func(isPrivate bool, release *github.RepositoryRelease) *http.Client { - return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposReleasesTagsByOwnerByRepoByTag: mockResponse(t, http.StatusOK, release), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, map[string]any{ - "name": "repo", - "private": isPrivate, - }), - }) - } - - reqParams := map[string]any{"owner": "owner", "repo": "repo", "tag": "v1.0.0"} - - 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, makeRelease(false)))} - handler := serverTool.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) - }) - - t.Run("public repo with published release is public", func(t *testing.T) { - t.Parallel() - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, makeRelease(false))), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.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) - assert.Equal(t, "trusted", label["integrity"]) - assert.Equal(t, "public", label["confidentiality"]) - }) - - t.Run("public repo with draft release is private", func(t *testing.T) { - t.Parallel() - // Reviewer-class scenario: a draft release on a public repo is not - // world-readable, so the label must not be public. - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(false, makeRelease(true))), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.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) - assert.Equal(t, "trusted", label["integrity"]) - assert.Equal(t, "private", label["confidentiality"], "draft release on public repo must be private") - }) -} - func Test_looksLikeSHA(t *testing.T) { tests := []struct { name string diff --git a/pkg/github/search.go b/pkg/github/search.go index 6caaf389d5..2d8f3ca30a 100644 --- a/pkg/github/search.go +++ b/pkg/github/search.go @@ -9,7 +9,6 @@ import ( "slices" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -133,36 +132,12 @@ func SearchRepositories(t translations.TranslationHelperFunc) inventory.ServerTo } } - callResult := utils.NewToolResultText(string(r)) - attachSearchRepositoriesIFCLabel(ctx, deps, result.Repositories, callResult) - return callResult, output, nil + return utils.NewToolResultText(string(r)), output, nil }, normalizeSearchArguments(true), ) } -// attachSearchRepositoriesIFCLabel joins per-repository IFC labels across -// every matched repository and attaches the result to callResult when IFC -// labels are enabled. Visibility is read directly from the search response — -// no extra API call. The join math is shared with search_issues via -// ifc.LabelSearchIssues: public-only results stay public-untrusted, -// mixed-visibility results become private-untrusted, and all-private results -// become private-trusted. The -// feature-flag check is centralized here (mirroring the attach* helpers in -// ifc_labels.go) so the handler can call this unconditionally. -func attachSearchRepositoriesIFCLabel(ctx context.Context, deps ToolDependencies, repos []*github.Repository, callResult *mcp.CallToolResult) { - if callResult == nil || callResult.IsError || !deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) { - return - } - - visibilities := make([]bool, 0, len(repos)) - for _, repo := range repos { - visibilities = append(visibilities, repo.GetPrivate()) - } - - setIFCLabel(callResult, ifc.LabelSearchIssues(visibilities)) -} - // SearchCode creates a tool to search for code across GitHub repositories. type SearchCodeInput struct { Query string `json:"query"` @@ -382,19 +357,8 @@ func SearchCode(t translations.TranslationHelperFunc) inventory.ServerTool { recordSearchCodeFieldsUsage(ctx, deps, minimalResult, filtered, len(r)) - callResult := utils.NewToolResultText(string(r)) - // Code search spans repositories; the IFC label is the conservative - // join across every matched repository's visibility, read directly - // from the search response. - visibilities := make([]bool, 0, len(result.CodeResults)) - for _, code := range result.CodeResults { - if code.Repository != nil { - visibilities = append(visibilities, code.Repository.GetPrivate()) - } - } - callResult = attachJoinedIFCLabel(ctx, deps, callResult, visibilities, ifc.LabelSearchIssues) output = structuredSearchCodeOutput(*minimalResult, input.Fields) - return callResult, output, nil + return utils.NewToolResultText(string(r)), output, nil }, normalizeTypedReadArguments(nil, false), ) @@ -476,11 +440,7 @@ func userOrOrgHandler(ctx context.Context, accountType string, deps ToolDependen if err != nil { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } - callResult := utils.NewToolResultText(string(r)) - // User and organization search returns public profile information that is - // authored by the account holders themselves, so it is public-untrusted. - callResult = attachStaticIFCLabel(ctx, deps, callResult, ifc.PublicUntrusted()) - return callResult, minimalResp, nil + return utils.NewToolResultText(string(r)), minimalResp, nil } // SearchUsers creates a tool to search for GitHub users. @@ -657,18 +617,7 @@ func SearchCommits(t translations.TranslationHelperFunc) inventory.ServerTool { return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil } - callResult := utils.NewToolResultText(string(r)) - // Commit search spans repositories; the IFC label is the conservative - // join across every matched repository's visibility, read directly - // from the search response. - visibilities := make([]bool, 0, len(result.Commits)) - for _, commit := range result.Commits { - if commit.Repository != nil { - visibilities = append(visibilities, commit.Repository.GetPrivate()) - } - } - callResult = attachJoinedIFCLabel(ctx, deps, callResult, visibilities, ifc.LabelSearchIssues) - return callResult, minimalResult, nil + return utils.NewToolResultText(string(r)), minimalResult, nil }, normalizeSearchArguments(false), ) diff --git a/pkg/github/search_test.go b/pkg/github/search_test.go index a94380876a..4af74e2da5 100644 --- a/pkg/github/search_test.go +++ b/pkg/github/search_test.go @@ -167,116 +167,6 @@ func Test_SearchRepositories(t *testing.T) { } } -func Test_SearchRepositories_IFC_InsidersMode(t *testing.T) { - t.Parallel() - - serverTool := SearchRepositories(translations.NullTranslationHelper) - - type repoFixture struct { - owner string - name string - isPrivate bool - } - - makeRepo := func(r repoFixture) *github.Repository { - return &github.Repository{ - ID: new(int64(1)), - Name: new(r.name), - FullName: new(r.owner + "/" + r.name), - Private: new(r.isPrivate), - Owner: &github.User{Login: new(r.owner)}, - } - } - - makeMockClient := func(repos []repoFixture) *http.Client { - searchResult := &github.RepositoriesSearchResult{ - Total: new(len(repos)), - IncompleteResults: new(false), - } - for _, r := range repos { - searchResult.Repositories = append(searchResult.Repositories, makeRepo(r)) - } - return MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetSearchRepositories: mockResponse(t, http.StatusOK, searchResult), - }) - } - - reqParams := map[string]any{"query": "octocat"} - - t.Run("insiders mode disabled omits ifc label", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient([]repoFixture{{owner: "octocat", name: "public-repo"}})), - } - handler := serverTool.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) - }) - - t.Run("insiders mode all public emits public untrusted", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient([]repoFixture{ - {owner: "octocat", name: "public-a"}, - {owner: "octocat", name: "public-b"}, - })), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode mixed public and private emits private untrusted", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient([]repoFixture{ - {owner: "octocat", name: "private-repo", isPrivate: true}, - {owner: "octocat", name: "public-repo"}, - })), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "private", ifcMap["confidentiality"]) - }) - - t.Run("insiders mode empty results emits public untrusted", func(t *testing.T) { - deps := BaseDeps{ - Client: mustNewGHClient(t, makeMockClient(nil)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - } - handler := serverTool.Handler(deps) - - request := createMCPRequest(reqParams) - result, err := handler(ContextWithDeps(context.Background(), deps), &request) - require.NoError(t, err) - require.False(t, result.IsError) - - require.NotNil(t, result.Meta) - ifcMap := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", ifcMap["integrity"]) - assert.Equal(t, "public", ifcMap["confidentiality"]) - }) -} - func Test_SearchRepositories_FullOutput(t *testing.T) { mockSearchResult := &github.RepositoriesSearchResult{ Total: new(1), diff --git a/pkg/github/search_utils.go b/pkg/github/search_utils.go index 0f62e8b26e..005536f32f 100644 --- a/pkg/github/search_utils.go +++ b/pkg/github/search_utils.go @@ -38,13 +38,7 @@ func hasTypeFilter(query string) bool { return hasFilter(query, "type") } -// searchPostProcessFn is invoked after a successful search response, before -// the call result is returned. It may attach additional metadata (such as IFC -// labels) to the call result based on the search payload. -type searchPostProcessFn func(ctx context.Context, result *github.IssuesSearchResult, callResult *mcp.CallToolResult) - type searchConfig struct { - postProcess searchPostProcessFn // fields, when non-empty, restricts each result item to the requested // subset of fields. fieldsTool and fieldsDeps identify the calling tool and // its dependencies so fields telemetry can be recorded. @@ -66,12 +60,6 @@ type SearchIssuesInput struct { type searchOption func(*searchConfig) -// withSearchPostProcess registers a callback invoked after a successful search -// response. The callback may mutate the call result (e.g. to attach _meta.ifc). -func withSearchPostProcess(fn searchPostProcessFn) searchOption { - return func(c *searchConfig) { c.postProcess = fn } -} - // withFieldsFiltering enables the optional `fields` response filtering for a // search tool. When fields is non-empty, each result item is reduced to the // requested subset while the total_count / incomplete_results wrapper is @@ -230,9 +218,6 @@ func searchHandler( } callResult := utils.NewToolResultText(string(r)) - if cfg.postProcess != nil { - cfg.postProcess(ctx, result, callResult) - } output = SearchIssuesResponse{ Total: result.Total, IncompleteResults: result.IncompleteResults, diff --git a/pkg/github/secret_scanning.go b/pkg/github/secret_scanning.go index 43dd1fd005..83ec8c43da 100644 --- a/pkg/github/secret_scanning.go +++ b/pkg/github/secret_scanning.go @@ -8,7 +8,6 @@ import ( "net/http" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -82,17 +81,12 @@ func GetSecretScanningAlert(t translations.TranslationHelperFunc) inventory.Serv return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to get alert", resp, body), nil, nil } - r, err := json.Marshal(alert) //nolint:gosec // G117: This security_events-scoped tool intentionally returns the alert's secret; the result is labeled private-untrusted. + r, err := json.Marshal(alert) //nolint:gosec // G117: This security_events-scoped tool intentionally returns the alert's secret. if err != nil { return nil, nil, fmt.Errorf("failed to marshal alert: %w", err) } - result := utils.NewToolResultText(string(r)) - // Secret scanning alerts are access-restricted regardless of repo - // visibility and surface the matched secret material itself, so the - // label is always private-untrusted. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert()) - return result, secretScanningAlertOutput(alert), nil + return utils.NewToolResultText(string(r)), secretScanningAlertOutput(alert), nil }, normalizeSecurityIntegerArguments("alertNumber"), ) @@ -180,17 +174,12 @@ func ListSecretScanningAlerts(t translations.TranslationHelperFunc) inventory.Se return ghErrors.NewGitHubAPIStatusErrorResponse(ctx, "failed to list alerts", resp, body), nil, nil } - r, err := json.Marshal(alerts) //nolint:gosec // G117: This security_events-scoped tool intentionally returns alert secrets; the result is labeled private-untrusted. + r, err := json.Marshal(alerts) //nolint:gosec // G117: This security_events-scoped tool intentionally returns alert secrets. if err != nil { return nil, nil, fmt.Errorf("failed to marshal alerts: %w", err) } - result := utils.NewToolResultText(string(r)) - // Secret scanning alerts are access-restricted regardless of repo - // visibility and surface the matched secret material itself, so the - // label is always private-untrusted. - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert()) - return result, mapSecurityOutputs(alerts, secretScanningAlertOutput), nil + return utils.NewToolResultText(string(r)), mapSecurityOutputs(alerts, secretScanningAlertOutput), nil }, normalizeSecurityIntegerArguments("page", "perPage"), normalizeTypedReadArguments(nil, false), diff --git a/pkg/github/security_advisories.go b/pkg/github/security_advisories.go index 42eb505f77..ea1143f6a6 100644 --- a/pkg/github/security_advisories.go +++ b/pkg/github/security_advisories.go @@ -8,7 +8,6 @@ import ( "net/http" ghErrors "github.com/github/github-mcp-server/v2/pkg/errors" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -153,12 +152,7 @@ func ListGlobalSecurityAdvisories(t translations.TranslationHelperFunc) inventor return nil, nil, fmt.Errorf("failed to marshal advisories: %w", err) } - result := utils.NewToolResultText(string(r)) - // Global advisories come from the world-readable GitHub Advisory - // Database (public) but contain externally authored prose - // (untrusted). - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGlobalSecurityAdvisory()) - return result, mapSecurityOutputs(advisories, globalSecurityAdvisoryOutput), nil + return utils.NewToolResultText(string(r)), mapSecurityOutputs(advisories, globalSecurityAdvisoryOutput), nil }, normalizeGlobalAdvisoryArguments, ) @@ -248,17 +242,7 @@ func ListRepositorySecurityAdvisories(t translations.TranslationHelperFunc) inve return nil, nil, fmt.Errorf("failed to marshal advisories: %w", err) } - result := utils.NewToolResultText(string(r)) - // 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, args.Owner, args.Repo, result, - func(isPrivate bool) ifc.SecurityLabel { - return ifc.LabelRepositorySecurityAdvisory(isPrivate, allPublished) - }) - return result, mapSecurityOutputs(advisories, securityAdvisoryOutput), nil + return utils.NewToolResultText(string(r)), mapSecurityOutputs(advisories, securityAdvisoryOutput), nil }, ) } @@ -314,11 +298,7 @@ func GetGlobalSecurityAdvisory(t translations.TranslationHelperFunc) inventory.S return nil, nil, fmt.Errorf("failed to marshal advisory: %w", err) } - result := utils.NewToolResultText(string(r)) - // A global advisory is world-readable (public) but externally - // authored (untrusted). - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGlobalSecurityAdvisory()) - return result, globalSecurityAdvisoryOutput(advisory), nil + return utils.NewToolResultText(string(r)), globalSecurityAdvisoryOutput(advisory), nil }, ) } @@ -400,28 +380,7 @@ func ListOrgRepositorySecurityAdvisories(t translations.TranslationHelperFunc) i return nil, nil, fmt.Errorf("failed to marshal advisories: %w", err) } - 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 (isPrivate=true, - // which forces private regardless of publication state). - result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelRepositorySecurityAdvisory(true, false)) - return result, mapSecurityOutputs(advisories, securityAdvisoryOutput), nil + return utils.NewToolResultText(string(r)), mapSecurityOutputs(advisories, securityAdvisoryOutput), 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 -} diff --git a/pkg/github/security_advisories_test.go b/pkg/github/security_advisories_test.go index 70186e0ccf..f78e64d7ad 100644 --- a/pkg/github/security_advisories_test.go +++ b/pkg/github/security_advisories_test.go @@ -9,7 +9,6 @@ import ( "github.com/github/github-mcp-server/v2/pkg/translations" "github.com/google/go-github/v92/github" "github.com/google/jsonschema-go/jsonschema" - "github.com/modelcontextprotocol/go-sdk/mcp" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -366,132 +365,6 @@ 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: new("GHSA-1111-1111-1111"), - Summary: new("Published advisory"), - State: new("published"), - } - draftAdvisory := &github.SecurityAdvisory{ - GHSAID: new("GHSA-2222-2222-2222"), - Summary: new("Draft advisory"), - State: new("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) diff --git a/pkg/github/typed_account_repository_search_test.go b/pkg/github/typed_account_repository_search_test.go index 64d32b2973..3df84c8556 100644 --- a/pkg/github/typed_account_repository_search_test.go +++ b/pkg/github/typed_account_repository_search_test.go @@ -112,8 +112,7 @@ func TestTypedAccountRepositorySearchOutputs(t *testing.T) { _, _ = w.Write([]byte(body)) } deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{fixture.path: handler})), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{fixture.path: handler})), } session := connectTypedSearchClient(t, tool, deps, protocol) list, err := session.ListTools(context.Background(), nil) @@ -147,15 +146,6 @@ func TestTypedAccountRepositorySearchOutputs(t *testing.T) { } else { assert.Equal(t, expected, getTextResult(t, result).Text) } - require.NotNil(t, result.Meta) - label := unmarshalIFC(t, result.Meta["ifc"]) - if tool.Tool.Name == "search_commits" && body == fixture.body { - assert.Equal(t, "trusted", label["integrity"]) - assert.Equal(t, "private", label["confidentiality"]) - } else { - assert.Equal(t, "untrusted", label["integrity"]) - assert.Equal(t, "public", label["confidentiality"]) - } if resolved == nil { assert.Nil(t, result.StructuredContent) } else { diff --git a/pkg/github/typed_actions_outputs_test.go b/pkg/github/typed_actions_outputs_test.go index cc99ae3370..d8f1b9663f 100644 --- a/pkg/github/typed_actions_outputs_test.go +++ b/pkg/github/typed_actions_outputs_test.go @@ -305,71 +305,49 @@ func TestTypedActionsClientErrorPrecedence(t *testing.T) { } } -func TestTypedActionsIFCAndAnnotations(t *testing.T) { +func TestTypedActionsAnnotations(t *testing.T) { for _, protocol := range typedActionsProtocols { - for _, private := range []bool{false, true} { - for _, enabled := range []bool{false, true} { - for _, tc := range []actionsWireCase{ - {name: "actions_list", method: "list_workflows"}, - {name: "actions_get", method: "get_workflow", args: map[string]any{"resource_id": "ci.yml"}}, - {name: "get_job_logs", args: map[string]any{"run_id": "7e0", "failed_only": true}}, - {name: "actions_run_trigger", method: "delete_workflow_run_logs", args: map[string]any{"run_id": 7}}, - } { - t.Run(fmt.Sprintf("%s/%s/private=%t/enabled=%t", tc.name, protocol, private, enabled), func(t *testing.T) { - client := &http.Client{Transport: recorderTransport{handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - if r.URL.Path == "/repos/owner/repo" { - _, _ = fmt.Fprintf(w, `{"private":%t}`, private) - return - } - if r.Method == http.MethodDelete { - w.WriteHeader(http.StatusNoContent) - return - } - _, _ = w.Write([]byte(`{}`)) - })}} - deps := BaseDeps{Client: mustNewGHClient(t, client), - RepoAccessCache: stubRepoAccessCache(nil, time.Minute)} - if enabled { - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) - } - session, schemas := actionsTypedSession(t, deps, protocol) - listed, err := session.ListTools(context.Background(), nil) - require.NoError(t, err) - for _, tool := range listed.Tools { - require.NotNil(t, tool.Annotations) - assert.Equal(t, tool.Name != "actions_run_trigger", tool.Annotations.ReadOnlyHint) - if tool.Name == "actions_run_trigger" { - require.NotNil(t, tool.Annotations.DestructiveHint) - assert.True(t, *tool.Annotations.DestructiveHint) - } - } - result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: actionsCaseArgs(tc)}) - require.NoError(t, err) - text := getTextResult(t, result).Text - projected := text - if tc.method != "" { - legacy := "{}" - if tc.method == "delete_workflow_run_logs" { - legacy = `{"message":"Workflow run logs have been deleted","run_id":7,"status":"204 No Content","status_code":204}` - } - projected = actionsProjectedEnvelope(t, tc.method, legacy) - text = legacy - } - assertActionsWireResult(t, result, schemas[tc.name], text, false, projected) - if !enabled || tc.name == "actions_run_trigger" { - assert.Nil(t, result.Meta["ifc"]) - return - } - label := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", label["integrity"]) - confidentiality := "public" - if private { - confidentiality = "private" - } - assert.Equal(t, confidentiality, label["confidentiality"]) - }) + for _, tc := range []actionsWireCase{ + {name: "actions_list", method: "list_workflows"}, + {name: "actions_get", method: "get_workflow", args: map[string]any{"resource_id": "ci.yml"}}, + {name: "get_job_logs", args: map[string]any{"run_id": "7e0", "failed_only": true}}, + {name: "actions_run_trigger", method: "delete_workflow_run_logs", args: map[string]any{"run_id": 7}}, + } { + t.Run(fmt.Sprintf("%s/%s", tc.name, protocol), func(t *testing.T) { + client := &http.Client{Transport: recorderTransport{handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.Method == http.MethodDelete { + w.WriteHeader(http.StatusNoContent) + return + } + _, _ = w.Write([]byte(`{}`)) + })}} + deps := BaseDeps{Client: mustNewGHClient(t, client), + RepoAccessCache: stubRepoAccessCache(nil, time.Minute)} + session, schemas := actionsTypedSession(t, deps, protocol) + listed, err := session.ListTools(context.Background(), nil) + require.NoError(t, err) + for _, tool := range listed.Tools { + require.NotNil(t, tool.Annotations) + assert.Equal(t, tool.Name != "actions_run_trigger", tool.Annotations.ReadOnlyHint) + if tool.Name == "actions_run_trigger" { + require.NotNil(t, tool.Annotations.DestructiveHint) + assert.True(t, *tool.Annotations.DestructiveHint) + } } - } + result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: actionsCaseArgs(tc)}) + require.NoError(t, err) + text := getTextResult(t, result).Text + projected := text + if tc.method != "" { + legacy := "{}" + if tc.method == "delete_workflow_run_logs" { + legacy = `{"message":"Workflow run logs have been deleted","run_id":7,"status":"204 No Content","status_code":204}` + } + projected = actionsProjectedEnvelope(t, tc.method, legacy) + text = legacy + } + assertActionsWireResult(t, result, schemas[tc.name], text, false, projected) + }) } } } diff --git a/pkg/github/typed_compatibility_test.go b/pkg/github/typed_compatibility_test.go index d1e68a6b1f..397380444f 100644 --- a/pkg/github/typed_compatibility_test.go +++ b/pkg/github/typed_compatibility_test.go @@ -9,7 +9,6 @@ import ( "testing" "github.com/github/github-mcp-server/v2/internal/githubv4mock" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/translations" "github.com/google/go-github/v92/github" @@ -267,22 +266,3 @@ func TestTypedCompatibilityDirectoryProjection(t *testing.T) { assert.JSONEq(t, `[{"url":"api-content-url","git_url":"api-git-url"}]`, getTextResult(t, result).Text) } } - -func TestTypedCompatibilityBlameIFC(t *testing.T) { - for _, private := range []bool{false, true} { - for _, protocol := range []string{"2025-11-25", inventory.ProtocolVersionMultiRoundTrip} { - deps := typedRepositoryDeps(t) - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) - deps.Client = mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: &private}), - })) - session := connectTypedSearchClient(t, GetFileBlame(translations.NullTranslationHelper), deps, protocol) - result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ - Name: "get_file_blame", Arguments: map[string]any{"owner": "owner", "repo": "repo", "path": "file"}, - }) - require.NoError(t, err) - require.False(t, result.IsError) - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelCommitContents(private)), mustMarshalJSON(t, result.Meta["ifc"])) - } - } -} diff --git a/pkg/github/typed_consolidated_issue_enrichment_test.go b/pkg/github/typed_consolidated_issue_enrichment_test.go index 2e02dcf09c..1c3b7086e7 100644 --- a/pkg/github/typed_consolidated_issue_enrichment_test.go +++ b/pkg/github/typed_consolidated_issue_enrichment_test.go @@ -16,11 +16,10 @@ import ( "github.com/stretchr/testify/require" ) -func TestTypedIssueReadIFCLabels(t *testing.T) { +func TestTypedIssueReadStructuredContent(t *testing.T) { for _, protocol := range []string{inventory.ProtocolVersionMultiRoundTrip, "2025-11-25", ""} { t.Run("protocol="+protocol, func(t *testing.T) { deps := consolidatedIssueDeps(t) - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) session, schemas := consolidatedIssueSession(t, deps, protocol, false) for _, method := range []string{"get", "get_comments", "get_sub_issues", "get_parent", "get_labels"} { args := map[string]any{"method": method, "owner": "owner", "repo": "repo", "issue_number": 1} @@ -30,9 +29,6 @@ func TestTypedIssueReadIFCLabels(t *testing.T) { result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: "issue_read", Arguments: args}) require.NoError(t, err) require.False(t, result.IsError) - label := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "untrusted", label["integrity"]) - assert.Equal(t, "public", label["confidentiality"]) if schema := schemas["issue_read"]; schema != nil { require.NotNil(t, result.StructuredContent) require.NoError(t, schema.Validate(result.StructuredContent)) diff --git a/pkg/github/typed_consolidated_pull_request_outputs_test.go b/pkg/github/typed_consolidated_pull_request_outputs_test.go index 60afd2f1f1..6fd4cd97cf 100644 --- a/pkg/github/typed_consolidated_pull_request_outputs_test.go +++ b/pkg/github/typed_consolidated_pull_request_outputs_test.go @@ -490,8 +490,6 @@ func TestTypedPullRequestContentParity(t *testing.T) { deps := consolidatedPullRequestDeps(t) deps.Client = mustNewGHClient(t, &http.Client{Transport: recorderTransport{handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { switch strings.TrimPrefix(r.URL.Path, "/api/v3") { - case "/repos/owner/repo": - _, _ = w.Write([]byte(`{"private":false}`)) case "/repos/owner/repo/pulls/1": _, _ = w.Write([]byte(`{"number":1,"title":"Subject","body":"bo\u200bdy","state":"open","html_url":"https://github.com/owner/repo/pull/1","user":{"login":"octocat"}}`)) default: @@ -501,7 +499,6 @@ func TestTypedPullRequestContentParity(t *testing.T) { })}}) deps.Flags = stubFeatureFlags(map[string]bool{"lockdown-mode": true}) deps.RepoAccessCache = stubRepoAccessCache(mockRESTPermissionServer(t, permission, nil), time.Minute) - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) session, schemas := consolidatedPullRequestSession(t, deps, protocol, false) result, err := session.CallTool(context.Background(), &mcp.CallToolParams{ Name: "pull_request_read", @@ -516,7 +513,6 @@ func TestTypedPullRequestContentParity(t *testing.T) { } require.False(t, result.IsError) text := getTextResult(t, result).Text - require.NotNil(t, result.Meta["ifc"]) if protocol == inventory.ProtocolVersionMultiRoundTrip { assert.JSONEq(t, mustMarshalJSON(t, result.StructuredContent), text) require.NoError(t, schemas["pull_request_read"].Validate(result.StructuredContent)) diff --git a/pkg/github/typed_discussion_notification_outputs_test.go b/pkg/github/typed_discussion_notification_outputs_test.go index de7f27817a..1c8d8ea5c7 100644 --- a/pkg/github/typed_discussion_notification_outputs_test.go +++ b/pkg/github/typed_discussion_notification_outputs_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/github/github-mcp-server/v2/internal/toolsnaps" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/translations" "github.com/google/go-github/v92/github" @@ -114,10 +113,8 @@ func typedDiscussionNotificationDeps(t *testing.T) BaseDeps { require.NoError(t, json.NewEncoder(w).Encode(map[string]any{"data": data})) })}} return BaseDeps{ - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), - GQLClient: githubv4.NewClient(gql), + GQLClient: githubv4.NewClient(gql), Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(true)}), GetNotifications: mockResponse(t, http.StatusOK, []*github.Notification{notification}), GetReposNotificationsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.Notification{notification}), GetNotificationsThreadsByThreadID: mockResponse(t, http.StatusOK, notification), @@ -225,14 +222,6 @@ func TestTypedDiscussionNotificationOutputs(t *testing.T) { require.NoError(t, err) require.False(t, result.IsError, "%s", result) require.Len(t, result.Content, 1, "text must not duplicate structured content") - switch tc.name { - case "get_notification_details": - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelNotificationDetails()), mustMarshalJSON(t, result.Meta["ifc"])) - case "list_discussions", "get_discussion", "get_discussion_comments": - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoUserContent(true)), mustMarshalJSON(t, result.Meta["ifc"])) - case "list_discussion_categories": - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoMetadata(true)), mustMarshalJSON(t, result.Meta["ifc"])) - } if protocol == "2025-11-25" { assert.Equal(t, tc.text, getTextResult(t, result).Text, "legacy text must remain byte-equivalent") assert.Nil(t, result.StructuredContent) diff --git a/pkg/github/typed_git_gist_outputs_test.go b/pkg/github/typed_git_gist_outputs_test.go index ef411295df..9a48dcf972 100644 --- a/pkg/github/typed_git_gist_outputs_test.go +++ b/pkg/github/typed_git_gist_outputs_test.go @@ -9,7 +9,6 @@ import ( "testing" "time" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/scopes" "github.com/github/github-mcp-server/v2/pkg/translations" @@ -84,12 +83,12 @@ type gitGistCase struct { structured string } -func gitGistDeps(t *testing.T, current **gitGistCase, apiError bool, private ...bool) BaseDeps { +func gitGistDeps(t *testing.T, current **gitGistCase, apiError bool) BaseDeps { t.Helper() client := &http.Client{Transport: recorderTransport{handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { tc := *current if r.URL.Path == "/repos/owner/repo" { - _, _ = fmt.Fprintf(w, `{"default_branch":"main","private":%t}`, len(private) > 0 && private[0]) + _, _ = w.Write([]byte(`{"default_branch":"main"}`)) return } assert.Equal(t, tc.method, r.Method) @@ -316,7 +315,7 @@ func TestListGistsLegacyPaginationBounds(t *testing.T) { } } -func TestGitGistIFCScopesAndLockdown(t *testing.T) { +func TestGitGistScopesAndLockdown(t *testing.T) { tr := translations.NullTranslationHelper for _, tc := range []struct { tool inventory.ServerTool @@ -349,43 +348,24 @@ func TestGitGistIFCScopesAndLockdown(t *testing.T) { } for _, protocol := range typedGitGistProtocols { - for _, private := range []bool{false, true} { - for _, enabled := range []bool{false, true} { - for _, lockdown := range []bool{false, true} { - t.Run(fmt.Sprintf("%s/private=%t/ifc=%t/lockdown=%t", protocol, private, enabled, lockdown), func(t *testing.T) { - var current *gitGistCase - deps := gitGistDeps(t, ¤t, false, private) - deps.Flags = stubFeatureFlags(map[string]bool{"lockdown-mode": lockdown}) - if enabled { - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) - } - session, schemas := gitGistTypedSession(t, deps, protocol) - seen := make(map[string]bool) - for _, tc := range gitGistWireCases() { - if seen[tc.name] { - continue - } - seen[tc.name] = true - current = &tc - result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: tc.args}) - require.NoError(t, err) - assertGitGistResult(t, result, schemas[tc.name], tc.text, false, tc.structured) - if !enabled || tc.name == "create_gist" || tc.name == "update_gist" { - assert.Nil(t, result.Meta["ifc"]) - continue - } - expected := ifc.LabelGist() - switch tc.name { - case "get_repository_tree": - expected = ifc.LabelCommitContents(private) - case "list_gists": - expected = ifc.LabelGistList() - } - assert.JSONEq(t, mustMarshalJSON(t, expected), mustMarshalJSON(t, result.Meta["ifc"])) - } - }) + for _, lockdown := range []bool{false, true} { + t.Run(fmt.Sprintf("%s/lockdown=%t", protocol, lockdown), func(t *testing.T) { + var current *gitGistCase + deps := gitGistDeps(t, ¤t, false) + deps.Flags = stubFeatureFlags(map[string]bool{"lockdown-mode": lockdown}) + session, schemas := gitGistTypedSession(t, deps, protocol) + seen := make(map[string]bool) + for _, tc := range gitGistWireCases() { + if seen[tc.name] { + continue + } + seen[tc.name] = true + current = &tc + result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.name, Arguments: tc.args}) + require.NoError(t, err) + assertGitGistResult(t, result, schemas[tc.name], tc.text, false, tc.structured) } - } + }) } } } diff --git a/pkg/github/typed_governance_outputs_test.go b/pkg/github/typed_governance_outputs_test.go index 094de91547..9f92bbef6c 100644 --- a/pkg/github/typed_governance_outputs_test.go +++ b/pkg/github/typed_governance_outputs_test.go @@ -173,10 +173,6 @@ func governanceWireDeps(t *testing.T, tc governanceWireCase) BaseDeps { _, _ = io.WriteString(w, body) return } - if r.URL.Path == "/repos/o/r" { - _, _ = io.WriteString(w, `{"private":false}`) - return - } if r.URL.Path == "/graphql" { graphqlCalls++ var body string @@ -577,27 +573,6 @@ func TestTypedGovernanceLevelRoutingNormalization(t *testing.T) { } } -func TestTypedGovernanceLabelRetainsIFCMetadata(t *testing.T) { - for _, tc := range governanceWireCases(t)[:2] { - t.Run(tc.tool.Tool.Name, func(t *testing.T) { - deps := governanceWireDeps(t, tc) - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) - session, _ := governanceTypedSession(t, tc.tool, deps, inventory.ProtocolVersionMultiRoundTrip) - result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.tool.Tool.Name, Arguments: tc.args}) - require.NoError(t, err) - require.False(t, result.IsError) - legacyDeps := governanceWireDeps(t, tc) - legacyDeps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) - legacy := governanceLegacyTool(tc.tool.Tool.Name) - before, err := legacy.Handler(legacyDeps)(ContextWithDeps(context.Background(), legacyDeps), new(createMCPRequest(tc.args))) - require.NoError(t, err) - require.NotNil(t, result.Meta["ifc"]) - assert.JSONEq(t, mustMarshalJSON(t, before.Meta["ifc"]), mustMarshalJSON(t, result.Meta["ifc"])) - assertGovernanceWireText(t, true, getTextResult(t, before).Text, result) - }) - } -} - func TestTypedGetLabelToolsetVariant(t *testing.T) { issuesTool := GetLabel(translations.NullTranslationHelper) labelsTool := GetLabelForLabelsToolset(translations.NullTranslationHelper) diff --git a/pkg/github/typed_projects_outputs_test.go b/pkg/github/typed_projects_outputs_test.go index 34cbc9acf2..361f13bad4 100644 --- a/pkg/github/typed_projects_outputs_test.go +++ b/pkg/github/typed_projects_outputs_test.go @@ -118,18 +118,18 @@ func projectWireCases() []projectWireCase { {tool: "projects_list", method: projectsMethodListProjectItems, args: map[string]any{"fields": []any{"3", "5"}, "before": "prev"}, steps: []projectWireStep{rest("GET", "/orgs/o/projectsV2/2/items", "before=prev&fields=3%2C5&per_page=50", "["+projectWireItem+"]")}, text: `{"items":[` + projectWireItemText + `],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false}}`}, - {tool: "projects_list", method: projectsMethodListProjectStatusUpdates, steps: []projectWireStep{gql(`{"data":{"organization":{"projectV2":{"public":true,"statusUpdates":{"nodes":[` + projectWireStatus + `],"pageInfo":{"hasNextPage":true,"hasPreviousPage":false,"endCursor":"next","startCursor":""}}}}}}`)}, + {tool: "projects_list", method: projectsMethodListProjectStatusUpdates, steps: []projectWireStep{gql(`{"data":{"organization":{"projectV2":{"statusUpdates":{"nodes":[` + projectWireStatus + `],"pageInfo":{"hasNextPage":true,"hasPreviousPage":false,"endCursor":"next","startCursor":""}}}}}}`)}, text: `{"pageInfo":{"hasNextPage":true,"hasPreviousPage":false,"nextCursor":"next","prevCursor":""},"statusUpdates":[` + projectWireStatusText + `]}`}, - {tool: "projects_list", method: projectsMethodListProjectViews, steps: []projectWireStep{gql(`{"data":{"organization":{"projectV2":{"id":"P","public":true,"views":{"nodes":[` + projectWireView + `],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"endCursor":"","startCursor":""}}}}}}`)}, + {tool: "projects_list", method: projectsMethodListProjectViews, steps: []projectWireStep{gql(`{"data":{"organization":{"projectV2":{"id":"P","views":{"nodes":[` + projectWireView + `],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"endCursor":"","startCursor":""}}}}}}`)}, text: `{"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"nextCursor":"","prevCursor":""},"views":[` + projectWireViewText + `]}`}, {tool: "projects_get", method: projectsMethodGetProject, steps: []projectWireStep{rest("GET", "/orgs/o/projectsV2/2", "", projectWireProject)}, text: projectWireProjectText}, {tool: "projects_get", method: projectsMethodGetProjectField, args: map[string]any{"field_id": "3e0"}, steps: []projectWireStep{rest("GET", "/orgs/o/projectsV2/2/fields/3", "", projectWireField)}, text: projectWireFieldText}, {tool: "projects_get", method: projectsMethodGetProjectItem, args: map[string]any{"item_id": "4", "fields": []any{"3"}}, steps: []projectWireStep{rest("GET", "/orgs/o/projectsV2/2/items/4", "fields=3", projectWireItem)}, text: projectWireItemText}, {tool: "projects_get", method: projectsMethodGetProjectStatusUpdate, args: map[string]any{"status_update_id": "S", "owner": false, "project_number": nil}, - steps: []projectWireStep{gql(`{"data":{"node":` + projectWireStatus[:len(projectWireStatus)-1] + `,"project":{"public":true}}}}`)}, text: projectWireStatusText}, + steps: []projectWireStep{gql(`{"data":{"node":` + projectWireStatus + `}}`)}, text: projectWireStatusText}, {tool: "projects_get", method: projectsMethodGetProjectView, args: map[string]any{"view_id": "V"}, - steps: []projectWireStep{gql(`{"data":{"node":` + projectWireView[:len(projectWireView)-1] + `,"project":{"public":true}}}}`)}, text: projectWireViewText}, + steps: []projectWireStep{gql(`{"data":{"node":` + projectWireView + `}}`)}, text: projectWireViewText}, {tool: "projects_write", method: projectsMethodAddProjectItem, args: map[string]any{"item_type": "issue", "item_owner": "o", "item_repo": "r", "issue_number": "7"}, steps: []projectWireStep{gql(`{"data":{"repository":{"issue":{"id":"ISSUE"}}}}`), gql(projectWireResolve), gql(`{"data":{"addProjectV2ItemById":{"item":{"id":"I","fullDatabaseId":"4"}}}}`)}, text: `{"full_database_id":"4","id":"I","item_id":4,"message":"Successfully added issue o/r#7 to project o/2"}`}, @@ -420,7 +420,7 @@ func TestTypedProjectsRESTListPageShapes(t *testing.T) { name: "status_updates_empty_final_page", tc: projectWireCase{ tool: "projects_list", method: projectsMethodListProjectStatusUpdates, - steps: []projectWireStep{{method: "POST", path: "/graphql", body: `{"data":{"organization":{"projectV2":{"public":true,"statusUpdates":{"nodes":[],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"endCursor":"","startCursor":""}}}}}}`}}, + steps: []projectWireStep{{method: "POST", path: "/graphql", body: `{"data":{"organization":{"projectV2":{"statusUpdates":{"nodes":[],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"endCursor":"","startCursor":""}}}}}}`}}, text: `{"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"nextCursor":"","prevCursor":""},"statusUpdates":[]}`, }, }, @@ -428,7 +428,7 @@ func TestTypedProjectsRESTListPageShapes(t *testing.T) { name: "views_empty_final_page", tc: projectWireCase{ tool: "projects_list", method: projectsMethodListProjectViews, - steps: []projectWireStep{{method: "POST", path: "/graphql", body: `{"data":{"organization":{"projectV2":{"id":"P","public":true,"views":{"nodes":[],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"endCursor":"","startCursor":""}}}}}}`}}, + steps: []projectWireStep{{method: "POST", path: "/graphql", body: `{"data":{"organization":{"projectV2":{"id":"P","views":{"nodes":[],"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"endCursor":"","startCursor":""}}}}}}`}}, text: `{"pageInfo":{"hasNextPage":false,"hasPreviousPage":false,"nextCursor":"","prevCursor":""},"views":[]}`, }, }, @@ -589,32 +589,6 @@ func TestTypedProjectsWriteBackendErrors(t *testing.T) { } } -func TestTypedProjectsWireIFC(t *testing.T) { - for _, protocol := range typedGitGistProtocols { - t.Run("protocol="+protocol, func(t *testing.T) { - tc := projectWireCases()[5] - tc.steps[0].body = `{"id":1,"node_id":"P","title":"Roadmap","public":false,"number":2}` - tc.text = `{"id":1,"node_id":"P","title":"Roadmap","description":"","public":false,"closed_at":"0001-01-01T00:00:00Z","created_at":"0001-01-01T00:00:00Z","updated_at":"0001-01-01T00:00:00Z","deleted_at":"0001-01-01T00:00:00Z","number":2,"short_description":""}` - deps := projectWireDeps(t, tc) - deps.featureChecker = featureCheckerFor(FeatureFlagIFCLabels) - session, schemas := projectsTypedSession(t, deps, protocol) - result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: tc.tool, Arguments: projectWireArgs(tc)}) - require.NoError(t, err) - require.False(t, result.IsError) - label := unmarshalIFC(t, result.Meta["ifc"]) - assert.Equal(t, "trusted", label["integrity"]) - assert.Equal(t, "private", label["confidentiality"]) - if schemas[tc.tool] != nil { - assert.JSONEq(t, projectWireStructured(t, tc), mustMarshalJSON(t, result.StructuredContent)) - assert.Equal(t, mustMarshalJSON(t, result.StructuredContent), getTextResult(t, result).Text) - } else { - assert.Equal(t, tc.text, getTextResult(t, result).Text) - assert.Nil(t, result.StructuredContent) - } - }) - } -} - func TestTypedProjectsWireErrors(t *testing.T) { cases := []struct { tool string diff --git a/pkg/github/typed_repository_commit_outputs_test.go b/pkg/github/typed_repository_commit_outputs_test.go index 59b5f207ff..47de5f737b 100644 --- a/pkg/github/typed_repository_commit_outputs_test.go +++ b/pkg/github/typed_repository_commit_outputs_test.go @@ -82,7 +82,6 @@ func TestTypedRepositoryCommitOutputs(t *testing.T) { Commit: &github.Commit{Message: new("A commit")}, } handlers := map[string]http.HandlerFunc{ - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(false)}), GetReposCommitsByOwnerByRepoByRef: func(w http.ResponseWriter, r *http.Request) { assert.Equal(t, "2", r.URL.Query().Get("page")) assert.Equal(t, "7", r.URL.Query().Get("per_page")) @@ -103,8 +102,7 @@ func TestTypedRepositoryCommitOutputs(t *testing.T) { }, } deps := BaseDeps{ - Client: mustNewGHClient(t, MockHTTPClientWithHandlers(handlers)), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), + Client: mustNewGHClient(t, MockHTTPClientWithHandlers(handlers)), } tools := []inventory.ServerTool{ GetCommit(translations.NullTranslationHelper), @@ -182,7 +180,6 @@ func TestTypedRepositoryCommitOutputs(t *testing.T) { require.NoError(t, err, call.name) require.False(t, result.IsError, "%s: %s", call.name, result) require.Len(t, result.Content, 1, "%s must preserve one legacy text result", call.name) - assert.NotNil(t, result.Meta["ifc"], "typed and legacy outputs retain IFC labeling") text := getTextResult(t, result).Text if protocolVersion == "2025-11-25" { assert.Equal(t, call.text, text, "%s legacy text is byte-exact", call.name) diff --git a/pkg/github/typed_repository_outputs_test.go b/pkg/github/typed_repository_outputs_test.go index 672837c711..2e0248c7b5 100644 --- a/pkg/github/typed_repository_outputs_test.go +++ b/pkg/github/typed_repository_outputs_test.go @@ -209,7 +209,6 @@ func typedRepositoryDeps(t *testing.T) BaseDeps { })}} return BaseDeps{ Client: mustNewGHClient(t, MockHTTPClientWithHandlers(handlers)), GQLClient: githubv4.NewClient(gql), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } } @@ -352,7 +351,6 @@ func TestTypedRepositoryOutputs(t *testing.T) { result, err := session.CallTool(context.Background(), &mcp.CallToolParams{Name: call.name, Arguments: call.args}) require.NoError(t, err) require.False(t, result.IsError, result) - assert.JSONEq(t, mustMarshalJSON(t, legacy.Meta["ifc"]), mustMarshalJSON(t, result.Meta["ifc"]), "IFC labels remain protocol-independent") if protocol != inventory.ProtocolVersionMultiRoundTrip { assert.Equal(t, mustMarshalJSON(t, legacy.Content), mustMarshalJSON(t, result.Content), "legacy content blocks remain byte-exact") assertLegacyRepositoryText(t, call.name, call.args, result) @@ -424,7 +422,6 @@ func TestTypedRepositoryOutputs(t *testing.T) { require.NoError(t, err) require.True(t, result.IsError, result) assert.Nil(t, result.StructuredContent, "errors never return success-shaped output") - assert.NotContains(t, result.Meta, "ifc", "errors never receive success IFC labels") } }) } diff --git a/pkg/github/typed_search_outputs_test.go b/pkg/github/typed_search_outputs_test.go index 93aab4ff99..ec1b7aa2e5 100644 --- a/pkg/github/typed_search_outputs_test.go +++ b/pkg/github/typed_search_outputs_test.go @@ -7,7 +7,6 @@ import ( "testing" "github.com/github/github-mcp-server/v2/internal/toolsnaps" - "github.com/github/github-mcp-server/v2/pkg/ifc" "github.com/github/github-mcp-server/v2/pkg/inventory" "github.com/github/github-mcp-server/v2/pkg/translations" "github.com/github/github-mcp-server/v2/pkg/utils" @@ -32,7 +31,6 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { assert.Equal(t, []string{"repo"}, tool.ScopeAccess.Scopes, "%s must retain repository read scope", tool.Tool.Name) } deps := BaseDeps{ - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetSearchCode: mockResponse(t, http.StatusOK, &github.CodeSearchResult{ Total: new(1), @@ -43,7 +41,6 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { SHA: new("abc123"), Repository: &github.Repository{ FullName: new("owner/repo"), - Private: new(false), }, TextMatches: []*github.TextMatch{{ ObjectType: new("FileContent"), @@ -72,7 +69,6 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { GetReposTagsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.RepositoryTag{ {Name: new("v1.0.0"), Commit: &github.Commit{SHA: new("abc123")}}, }), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(false)}), })), } @@ -118,7 +114,6 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { require.NoError(t, err) require.False(t, branches.IsError, branches) assert.Equal(t, "[]", getTextResult(t, branches).Text) - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoMetadata(false)), mustMarshalJSON(t, branches.Meta["ifc"])) if protocolVersion == inventory.ProtocolVersionMultiRoundTrip { assert.Equal(t, "[]", mustMarshalJSON(t, branches.StructuredContent)) require.NoError(t, outputSchemas["list_branches"].Validate(branches.StructuredContent)) @@ -133,7 +128,6 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { require.NoError(t, err) require.False(t, result.IsError, result) require.Equal(t, `[{"name":"v1.0.0","sha":"abc123"}]`, getTextResult(t, result).Text) - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelRepoMetadata(false)), mustMarshalJSON(t, result.Meta["ifc"])) if protocolVersion == "2025-11-25" { assert.Nil(t, result.StructuredContent) } else { @@ -155,7 +149,6 @@ func TestTypedProjectedReadToolOutputSchemas(t *testing.T) { if protocolVersion == "2025-11-25" { assert.Equal(t, `{"incomplete_results":false,"items":[{"name":"main.go"}],"total_count":1}`, getTextResult(t, result).Text) } - assert.JSONEq(t, mustMarshalJSON(t, ifc.LabelSearchIssues([]bool{false})), mustMarshalJSON(t, result.Meta["ifc"])) if protocolVersion == "2025-11-25" { assert.Nil(t, result.StructuredContent) } else { diff --git a/pkg/github/typed_security_outputs_test.go b/pkg/github/typed_security_outputs_test.go index e793fcbbd2..069fa149f8 100644 --- a/pkg/github/typed_security_outputs_test.go +++ b/pkg/github/typed_security_outputs_test.go @@ -155,19 +155,17 @@ func TestTypedSecurityToolOutputs(t *testing.T) { deps := BaseDeps{ Client: mustNewGHClient(t, MockHTTPClientWithHandlers(map[string]http.HandlerFunc{ GetReposCodeQualityFindingsByOwnerByRepoByFindingNumber: mockResponse(t, http.StatusOK, codeQualityJSON), - GetReposByOwnerByRepo: mockResponse(t, http.StatusOK, &github.Repository{Private: new(true)}), - GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, codeScanningAlert), - GetReposCodeScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.Alert{codeScanningAlert}), - GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, secretScanningAlert), - GetReposSecretScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecretScanningAlert{secretScanningAlert}), - GetReposDependabotAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, dependabotAlert), - GetReposDependabotAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.DependabotAlert{dependabotAlert}), - GetAdvisories: mockResponse(t, http.StatusOK, []*github.GlobalSecurityAdvisory{globalAdvisory}), - GetAdvisoriesByGhsaID: mockResponse(t, http.StatusOK, globalAdvisory), - GetReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), - GetOrgsSecurityAdvisoriesByOrg: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), + GetReposCodeScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, codeScanningAlert), + GetReposCodeScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.Alert{codeScanningAlert}), + GetReposSecretScanningAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, secretScanningAlert), + GetReposSecretScanningAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecretScanningAlert{secretScanningAlert}), + GetReposDependabotAlertsByOwnerByRepoByAlertNumber: mockResponse(t, http.StatusOK, dependabotAlert), + GetReposDependabotAlertsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.DependabotAlert{dependabotAlert}), + GetAdvisories: mockResponse(t, http.StatusOK, []*github.GlobalSecurityAdvisory{globalAdvisory}), + GetAdvisoriesByGhsaID: mockResponse(t, http.StatusOK, globalAdvisory), + GetReposSecurityAdvisoriesByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), + GetOrgsSecurityAdvisoriesByOrg: mockResponse(t, http.StatusOK, []*github.SecurityAdvisory{advisory}), })), - featureChecker: featureCheckerFor(FeatureFlagIFCLabels), } tools := []inventory.ServerTool{ GetCodeQualityFinding(translations.NullTranslationHelper), @@ -195,7 +193,6 @@ func TestTypedSecurityToolOutputs(t *testing.T) { {Name: "list_repository_security_advisories", Arguments: map[string]any{"owner": "owner", "repo": "repo"}}, {Name: "list_org_repository_security_advisories", Arguments: map[string]any{"org": "owner"}}, } - legacyIFC := make(map[string]string) t.Run("unknown direct caller", func(t *testing.T) { for i, tool := range tools { call := calls[i] @@ -244,16 +241,11 @@ func TestTypedSecurityToolOutputs(t *testing.T) { text := getTextResult(t, result).Text if protocolVersion == "2025-11-25" { assert.Equal(t, mustMarshalJSON(t, legacyOutputs[call.Name]), text, "%s must retain byte-exact main serialization for legacy clients", call.Name) - legacyIFC[call.Name] = mustMarshalJSON(t, result.Meta["ifc"]) assert.Nil(t, result.StructuredContent, "%s must remain text-only for legacy clients", call.Name) continue } require.NotNil(t, result.StructuredContent, "%s must return structured content", call.Name) - assert.Equal(t, legacyIFC[call.Name], mustMarshalJSON(t, result.Meta["ifc"]), "IFC labels must protect text and structured content identically") - if call.Name != "get_code_quality_finding" { - assert.NotNil(t, result.Meta["ifc"], "%s must retain its IFC label", call.Name) - } schemaJSON, err := json.Marshal(toolsByName[call.Name].OutputSchema) require.NoError(t, err) var schema jsonschema.Schema @@ -303,9 +295,6 @@ func TestTypedSecurityToolOutputs(t *testing.T) { if call.Name == "get_secret_scanning_alert" { assert.NotContains(t, mustMarshalJSON(t, result.StructuredContent), "legacy response detail") } - if call.Name == "get_code_scanning_alert" { - assert.NotNil(t, result.Meta["ifc"], "typed output must retain the security alert IFC label") - } } }) } diff --git a/pkg/http/server_test.go b/pkg/http/server_test.go index 60b0df0da3..6960bb6c97 100644 --- a/pkg/http/server_test.go +++ b/pkg/http/server_test.go @@ -441,12 +441,6 @@ func TestCreateHTTPFeatureChecker(t *testing.T) { flagName: github.FeatureFlagCSVOutput, wantEnabled: true, }, - { - name: "insiders mode does not auto-enable ifc labels", - flagName: github.FeatureFlagIFCLabels, - insidersMode: true, - wantEnabled: false, - }, { name: "insiders mode does not enable granular flags", flagName: github.FeatureFlagIssuesGranular, diff --git a/pkg/ifc/ifc.go b/pkg/ifc/ifc.go deleted file mode 100644 index f23383ce73..0000000000 --- a/pkg/ifc/ifc.go +++ /dev/null @@ -1,383 +0,0 @@ -// Package ifc provides Information Flow Control labels for annotating MCP tool outputs. -// The actual IFC enforcement engine lives in a separate service; this package only -// defines the label schema used for annotations. -package ifc - -type Integrity string - -const ( - IntegrityTrusted Integrity = "trusted" - IntegrityUntrusted Integrity = "untrusted" -) - -type Confidentiality string - -const ( - ConfidentialityPublic Confidentiality = "public" - ConfidentialityPrivate Confidentiality = "private" -) - -type SecurityLabel struct { - Integrity Integrity `json:"integrity"` - Confidentiality Confidentiality `json:"confidentiality"` -} - -// PublicTrusted returns a label for trusted, publicly readable data. -func PublicTrusted() SecurityLabel { - return SecurityLabel{ - Integrity: IntegrityTrusted, - Confidentiality: ConfidentialityPublic, - } -} - -// PublicUntrusted returns a label for untrusted, publicly readable data. -func PublicUntrusted() SecurityLabel { - return SecurityLabel{ - Integrity: IntegrityUntrusted, - Confidentiality: ConfidentialityPublic, - } -} - -// PrivateTrusted returns a label for trusted data restricted to the readers -// of the originating repository. The reader set is opaque on the wire (a -// single "private" marker); the client engine resolves the concrete readers -// from the GitHub API on demand at egress decision time. -func PrivateTrusted() SecurityLabel { - return SecurityLabel{ - Integrity: IntegrityTrusted, - Confidentiality: ConfidentialityPrivate, - } -} - -// PrivateUntrusted returns a label for untrusted data restricted to the -// readers of the originating repository. See PrivateTrusted for the reader -// resolution model. -func PrivateUntrusted() SecurityLabel { - return SecurityLabel{ - Integrity: IntegrityUntrusted, - Confidentiality: ConfidentialityPrivate, - } -} - -// LabelGetMe returns the IFC label for the authenticated user's own profile -// (get_me). -// -// Integrity is trusted: this is GitHub-maintained data about the caller's own -// account, not attacker-authored content. -// -// Confidentiality is private. The result includes fields that are NOT part of -// the user's public profile — private_gists, total_private_repos, and -// owned_private_repos — which are visible only to the authenticated user. The -// result therefore must not be treated as world-readable. -func LabelGetMe() SecurityLabel { - return PrivateTrusted() -} - -// LabelListIssues returns the IFC label for a list_issues result. -// Public repositories are universally readable; private repositories are -// restricted to their collaborators (resolved client-side from the marker). -// Public repository issue contents are attacker-controllable, while private -// repository issues are treated as trusted collaborator-authored data. -func LabelListIssues(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateTrusted() - } - return PublicUntrusted() -} - -// LabelRepoUserContent returns the IFC label for user-authored content scoped -// to a repository when that tool has not opted into a more specific integrity -// policy. Public repository content is untrusted because it may be authored by -// outside contributors. Private repository content is trusted because users who -// can read it are trusted collaborators. -func LabelRepoUserContent(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateTrusted() - } - return PublicUntrusted() -} - -// LabelGetFileContents returns the IFC label for a get_file_contents result. -// Public repository file contents may be authored by anyone via pull requests -// and are therefore untrusted. In private repositories only collaborators can -// land changes, so contents are treated as trusted. -func LabelGetFileContents(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateTrusted() - } - return PublicUntrusted() -} - -// LabelSearchIssues returns the IFC label for a multi-repository search -// result, joining per-repository labels across all matched repositories. -// Used by both search_issues and search_repositories. -// -// Public-only results are untrusted and public. All-private results are trusted -// and private because private repository content is treated as trusted -// collaborator-authored data. Mixed public/private results are untrusted and -// private: the public items keep the joined payload's integrity untrusted, -// while the private items keep the joined payload's confidentiality private. -// The reader set is opaque (the "private" marker); the client engine resolves -// concrete readers on demand at egress decision time. -// -// An empty result set is treated as public-untrusted (no repository data is -// leaked). -// -// Why a single joined label rather than one label per item: a tool result is -// delivered as one opaque payload (a single content block) and the IFC engine -// makes one allow/deny decision per flow at egress. Once the items share a -// buffer in the agent's context they can be copied anywhere together, so the -// only sound bound for the whole result is the meet of every item's label. -// Per-item labels would only become load-bearing if the enforcement engine -// could partition a result and route individual items to different sinks; -// until then they would invite unsafe declassification of a "public" item that -// actually arrived alongside private data. -func LabelSearchIssues(repoVisibilities []bool) SecurityLabel { - var anyPrivate, anyPublic bool - for _, isPrivate := range repoVisibilities { - if isPrivate { - anyPrivate = true - } else { - anyPublic = true - } - } - switch { - case anyPrivate && anyPublic: - return PrivateUntrusted() - case anyPrivate: - return PrivateTrusted() - default: - return PublicUntrusted() - } -} - -// LabelRepoMetadata returns the IFC label for structural repository metadata -// that only collaborators with write access can define: labels, branches, -// tags, releases, issue types, issue field definitions, discussion -// categories, and the collaborator roster. -// -// Integrity is trusted because, unlike issue/PR/comment bodies, these -// artifacts cannot be authored by arbitrary outsiders — creating a branch, -// tag, release, or label requires push access, so the data reflects decisions -// made by the repository's trusted writers rather than attacker-controllable -// input. -// -// Confidentiality follows repository visibility: public repositories are -// universally readable; private repositories restrict the reader set (the -// opaque "private" marker, resolved client-side at egress time). -func LabelRepoMetadata(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateTrusted() - } - return PublicTrusted() -} - -// LabelRelease returns the IFC label for repository releases (list_releases, -// get_latest_release, get_release_by_tag). -// -// Integrity is trusted: releases are published by collaborators with push -// access, not by arbitrary outsiders. -// -// Confidentiality is public only when the repository is public AND no returned -// release is a draft. Draft releases are visible only to users with push -// access — they are NOT world-readable even on a public repository — so a -// result containing one must be private. hasDraft reflects whether any release -// in the result is a draft; private repositories are always private regardless. -func LabelRelease(isPrivate bool, hasDraft bool) SecurityLabel { - if isPrivate || hasDraft { - return PrivateTrusted() - } - return PublicTrusted() -} - -// LabelCollaboratorRoster returns the IFC label for a repository's collaborator -// list (list_repository_collaborators). -// -// Integrity is trusted: the roster is GitHub-maintained membership data, not -// attacker-authored content. -// -// Confidentiality is always private. Listing collaborators requires push -// access to the repository, so the roster is never world-readable — not even -// for public repositories. This mirrors LabelTeam: membership data is -// restricted regardless of the repository's own visibility. -func LabelCollaboratorRoster() SecurityLabel { - return PrivateTrusted() -} - -// LabelCommitContents returns the IFC label for committed repository content -// reachable from the default branch and its history: commits, commit diffs, -// and the repository file tree. -// -// It shares the reasoning of LabelGetFileContents. In public repositories any -// outsider can land content via a pull request, so the integrity of committed -// content is untrusted. In private repositories only collaborators can push, -// so committed content is trusted. Confidentiality follows repository -// visibility. -func LabelCommitContents(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateTrusted() - } - return PublicUntrusted() -} - -// LabelActionsResult returns the IFC label for GitHub Actions resources: -// workflow definitions, runs, jobs, artifacts, and job logs. -// -// Integrity is untrusted. Workflow logs echo arbitrary text produced during a -// run — including output derived from pull-request branches, dependency -// downloads, and other attacker-influenceable sources — so log and artifact -// content must be treated as low integrity. Workflow definitions are -// themselves editable through pull requests in public repositories. -// -// Confidentiality follows repository visibility. -func LabelActionsResult(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateUntrusted() - } - return PublicUntrusted() -} - -// LabelSecurityAlert returns the IFC label for security findings: code -// scanning alerts, secret scanning alerts, and Dependabot alerts. -// -// Integrity is untrusted because alert payloads embed attacker-influenceable -// material — the offending code snippet, the matched secret string, or a -// vulnerable dependency's advisory text — none of which the agent should treat -// as a trustworthy instruction source. -// -// Confidentiality is always private. Security alerts are access-restricted by -// GitHub regardless of repository visibility (only users with a security role -// can read them), so the reader set is narrow even for public repositories. -// Secret scanning results additionally surface the secret material itself. -func LabelSecurityAlert() SecurityLabel { - return PrivateUntrusted() -} - -// LabelGlobalSecurityAdvisory returns the IFC label for advisories served from -// the public GitHub Advisory Database (global advisories). -// -// The advisory database is world-readable, so confidentiality is public. -// Integrity is untrusted: advisory descriptions are externally authored prose -// and must not be treated as a trusted instruction source. -func LabelGlobalSecurityAdvisory() SecurityLabel { - return PublicUntrusted() -} - -// LabelRepositorySecurityAdvisory returns the IFC label for repository- or -// organization-scoped security advisories. -// -// 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() -} - -// LabelGist returns the IFC label for gist content. -// -// Integrity is untrusted: gist contents are arbitrary user-authored text. -// Confidentiality is public because secret gists are URL-accessible and cannot -// be modeled as private to a GitHub reader set. -func LabelGist() SecurityLabel { - return PublicUntrusted() -} - -// LabelGistList returns the IFC label for a list of gists belonging to a user, -// joining the per-gist confidentiality across the result set. -// -// Integrity is untrusted (user-authored content). Confidentiality is public -// because even secret gists are URL-accessible. -// -// See LabelSearchIssues for why list results carry a single joined label -// rather than one label per item. -func LabelGistList() SecurityLabel { - return PublicUntrusted() -} - -// LabelProject returns the IFC label for GitHub Project metadata (Projects v2), -// such as get_project results and project field definitions. -// -// Public project metadata can contain public user-authored text, so it is -// untrusted. Private project metadata is treated as trusted -// collaborator-controlled data. -// -// Confidentiality derives from the project's own privacy — private projects -// restrict the reader set, while public projects are universally readable. -func LabelProject(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateTrusted() - } - return PublicUntrusted() -} - -// LabelProjectList returns the IFC label for a list_projects result, joining -// the per-project labels across every returned project. -// -// Public-only results are untrusted and public. All-private results are trusted -// and private. Mixed public/private results are untrusted and private: public -// items keep the joined payload's integrity untrusted, while private items keep -// the joined payload's confidentiality private. -func LabelProjectList(projectVisibilities []bool) SecurityLabel { - var anyPrivate, anyPublic bool - for _, isPrivate := range projectVisibilities { - if isPrivate { - anyPrivate = true - } else { - anyPublic = true - } - } - switch { - case anyPrivate && anyPublic: - return PrivateUntrusted() - case anyPrivate: - return PrivateTrusted() - default: - return PublicUntrusted() - } -} - -// LabelProjectContent returns the IFC label for project results that can include -// item content, field values, or status update bodies. These can aggregate -// content from a variety of sources, so integrity remains untrusted even when -// the project is private. -func LabelProjectContent(isPrivate bool) SecurityLabel { - if isPrivate { - return PrivateUntrusted() - } - return PublicUntrusted() -} - -// LabelTeam returns the IFC label for organization team membership data -// (get_teams, get_team_members). -// -// Integrity is trusted: team membership is maintained by GitHub and cannot be -// forged by outside contributors, so it is not an attacker-controllable -// instruction source. -// -// Confidentiality is private. Organization team rosters and the teams a user -// belongs to are visible only to members of the organization, not to the -// public, so the reader set is restricted (the opaque "private" marker). -func LabelTeam() SecurityLabel { - return PrivateTrusted() -} - -// LabelNotificationDetails returns the IFC label for the subject of a single -// notification. -// -// Integrity is untrusted: a notification subject points at an issue, pull -// request, comment, or discussion whose content is user-authored and may carry -// attacker-controlled text. Confidentiality is private because notifications -// are delivered to a specific recipient and may reference private -// repositories; the result cannot be assumed to be publicly readable. -func LabelNotificationDetails() SecurityLabel { - return PrivateUntrusted() -} diff --git a/pkg/ifc/ifc_test.go b/pkg/ifc/ifc_test.go deleted file mode 100644 index f4b25c1876..0000000000 --- a/pkg/ifc/ifc_test.go +++ /dev/null @@ -1,366 +0,0 @@ -package ifc - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestLabelListIssues(t *testing.T) { - t.Parallel() - - t.Run("public repo issues are untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelListIssues(false) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private repo issues are trusted and private", func(t *testing.T) { - t.Parallel() - label := LabelListIssues(true) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelRepoUserContent(t *testing.T) { - t.Parallel() - - t.Run("public repo user content is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelRepoUserContent(false) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private repo user content is trusted and private", func(t *testing.T) { - t.Parallel() - label := LabelRepoUserContent(true) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelSearchIssues(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - visibilities []bool // true == private - wantIntegrity Integrity - wantConfidential Confidentiality - }{ - { - name: "empty result is treated as public", - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPublic, - }, - { - name: "single public repo", - visibilities: []bool{false}, - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPublic, - }, - { - name: "all public repos stay public", - visibilities: []bool{false, false, false}, - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPublic, - }, - { - name: "mixed public and private repos become untrusted private", - visibilities: []bool{false, true, false}, - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPrivate, - }, - { - name: "all private repos stay trusted private", - visibilities: []bool{true, true}, - wantIntegrity: IntegrityTrusted, - wantConfidential: ConfidentialityPrivate, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - label := LabelSearchIssues(tc.visibilities) - assert.Equal(t, tc.wantIntegrity, label.Integrity) - assert.Equal(t, tc.wantConfidential, label.Confidentiality) - }) - } -} - -func TestLabelRepoMetadata(t *testing.T) { - t.Parallel() - - t.Run("public repo metadata is trusted and public", func(t *testing.T) { - t.Parallel() - label := LabelRepoMetadata(false) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private repo metadata is trusted and private", func(t *testing.T) { - t.Parallel() - label := LabelRepoMetadata(true) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelGetMe(t *testing.T) { - t.Parallel() - - // get_me exposes private_gists/total_private_repos/owned_private_repos, - // which are not part of the public profile, so the result is trusted but - // private — never public. - label := LabelGetMe() - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) -} - -func TestLabelRelease(t *testing.T) { - t.Parallel() - - t.Run("public repo with no draft is trusted and public", func(t *testing.T) { - t.Parallel() - label := LabelRelease(false, false) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("public repo with a draft release is private", func(t *testing.T) { - t.Parallel() - // Draft releases are visible only to push-access users, so a draft on - // a public repo must not be labeled public. - label := LabelRelease(false, true) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) - - t.Run("private repo is private regardless of draft", func(t *testing.T) { - t.Parallel() - for _, hasDraft := range []bool{false, true} { - label := LabelRelease(true, hasDraft) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - } - }) -} - -func TestLabelCollaboratorRoster(t *testing.T) { - t.Parallel() - - // A collaborator roster requires push access to list, so it is never - // world-readable — always trusted and private. - label := LabelCollaboratorRoster() - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) -} - -func TestLabelCommitContents(t *testing.T) { - t.Parallel() - - t.Run("public repo commit content is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelCommitContents(false) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private repo commit content is trusted and private", func(t *testing.T) { - t.Parallel() - label := LabelCommitContents(true) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelActionsResult(t *testing.T) { - t.Parallel() - - t.Run("public repo actions result is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelActionsResult(false) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private repo actions result is untrusted and private", func(t *testing.T) { - t.Parallel() - label := LabelActionsResult(true) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelSecurityAlert(t *testing.T) { - t.Parallel() - label := LabelSecurityAlert() - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality, - "security alerts are access-restricted regardless of repo visibility") -} - -func TestLabelGlobalSecurityAdvisory(t *testing.T) { - t.Parallel() - label := LabelGlobalSecurityAdvisory() - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) -} - -func TestLabelRepositorySecurityAdvisory(t *testing.T) { - t.Parallel() - - t.Run("public repo with all published advisories is untrusted and public", func(t *testing.T) { - t.Parallel() - 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, 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) - }) -} - -func TestLabelGist(t *testing.T) { - t.Parallel() - - t.Run("public gist is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelGist() - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("secret gist is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelGist() - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) -} - -func TestLabelGistList(t *testing.T) { - t.Parallel() - - label := LabelGistList() - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) -} - -func TestLabelProject(t *testing.T) { - t.Parallel() - - t.Run("public project is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelProject(false) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private project metadata is trusted and private", func(t *testing.T) { - t.Parallel() - label := LabelProject(true) - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelProjectList(t *testing.T) { - t.Parallel() - - tests := []struct { - name string - visibilities []bool // true == private - wantIntegrity Integrity - wantConfidential Confidentiality - }{ - { - name: "empty result is treated as public", - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPublic, - }, - { - name: "all public projects stay public", - visibilities: []bool{false, false}, - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPublic, - }, - { - name: "mixed public and private projects become untrusted private", - visibilities: []bool{false, true}, - wantIntegrity: IntegrityUntrusted, - wantConfidential: ConfidentialityPrivate, - }, - { - name: "all private projects stay trusted private", - visibilities: []bool{true, true}, - wantIntegrity: IntegrityTrusted, - wantConfidential: ConfidentialityPrivate, - }, - } - - for _, tc := range tests { - t.Run(tc.name, func(t *testing.T) { - t.Parallel() - label := LabelProjectList(tc.visibilities) - assert.Equal(t, tc.wantIntegrity, label.Integrity) - assert.Equal(t, tc.wantConfidential, label.Confidentiality) - }) - } -} - -func TestLabelProjectContent(t *testing.T) { - t.Parallel() - - t.Run("public project content is untrusted and public", func(t *testing.T) { - t.Parallel() - label := LabelProjectContent(false) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPublic, label.Confidentiality) - }) - - t.Run("private project content is untrusted and private", func(t *testing.T) { - t.Parallel() - label := LabelProjectContent(true) - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) - }) -} - -func TestLabelTeam(t *testing.T) { - t.Parallel() - label := LabelTeam() - assert.Equal(t, IntegrityTrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) -} - -func TestLabelNotificationDetails(t *testing.T) { - t.Parallel() - label := LabelNotificationDetails() - assert.Equal(t, IntegrityUntrusted, label.Integrity) - assert.Equal(t, ConfidentialityPrivate, label.Confidentiality) -} diff --git a/script/get-me b/script/get-me index ffd24a357f..77c6874404 100755 --- a/script/get-me +++ b/script/get-me @@ -11,7 +11,7 @@ output=$( ) if command -v jq &> /dev/null; then - echo "$output" | jq '{_meta: .result._meta, content: (.result.content[0].text | fromjson)}' + echo "$output" | jq '.result.content[0].text | fromjson' else echo "$output" fi