Skip to content

Commit fb7cbc8

Browse files
authored
Annotate read tools with ifc labels (#2671)
* Annotate read tools with ifc labels * Dont automatically enable IFCLabels in insiders mode * ifc: don't label unpublished repo advisories as public Repository security advisory listings can include draft/triage/closed advisories (via the state filter), which are not world-readable even on a public repository. Deriving confidentiality from repo visibility alone under-classified those results as public. LabelRepositorySecurityAdvisory now takes an allPublished flag and only returns a public label when the repo is public AND every returned advisory is published; otherwise it is private. list_repository_security_advisories computes allPublished from the response state; the org-wide listing stays private-untrusted. Adds unit + handler regression tests covering the draft-advisory-on-public-repo case. Addresses PR review feedback. * ifc: fix confidentiality under-classification in releases, collaborators, get_me Audit for the same bug class as the repo-advisory fix (confidentiality derived from a coarse signal that misses access-restricted items) found three more under-classifications: - Releases (list_releases, get_latest_release, get_release_by_tag): draft releases are visible only to push-access users and are not world-readable even on a public repo. New LabelRelease(isPrivate, hasDraft) returns public only for a non-draft release on a public repo; handlers compute hasDraft from the response (Draft flag / per-item scan). - list_repository_collaborators: a collaborator roster requires push access to list, so it is never world-readable, not even on a public repo. New LabelCollaboratorRoster() is always PrivateTrusted (mirrors LabelTeam), replacing the repo-visibility-derived label. - get_me: the result includes private_gists / total_private_repos / owned_private_repos, which are not part of the public profile. LabelGetMe is now PrivateTrusted instead of PublicTrusted. Verified the remaining public-capable labels are sound: Actions logs are world-readable on public repos; branches/tags are public metadata; gist, project, search, and starred-repo labels read per-item visibility and join. Adds ifc unit tests for the new/changed labels and a get_release_by_tag handler regression test (draft on public repo -> private); updates the get_me handler test to assert private. * ifc: document why list results use one joined label, not per-item Explain on LabelSearchIssues (and cross-ref from LabelGistList) that a tool result is delivered as one opaque payload and the IFC engine makes one allow/deny decision per flow at egress, so the only sound bound for a list is the meet of every item's label. Per-item labels would only be load-bearing if the engine could partition a result and route items to different sinks; until then they would invite unsafe declassification of a public item that arrived alongside private data. Doc-only change.
1 parent 918a42f commit fb7cbc8

26 files changed

Lines changed: 1399 additions & 148 deletions

pkg/github/actions.go

Lines changed: 49 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/github/github-mcp-server/internal/profiler"
1313
buffer "github.com/github/github-mcp-server/pkg/buffer"
1414
ghErrors "github.com/github/github-mcp-server/pkg/errors"
15+
"github.com/github/github-mcp-server/pkg/ifc"
1516
"github.com/github/github-mcp-server/pkg/inventory"
1617
"github.com/github/github-mcp-server/pkg/scopes"
1718
"github.com/github/github-mcp-server/pkg/translations"
@@ -354,6 +355,14 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
354355
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
355356
}
356357

358+
// attachIFC adds the IFC label to a successful Actions result when
359+
// IFC labels are enabled. Workflow definitions, runs, jobs,
360+
// artifacts and logs echo attacker-influenceable run output, so
361+
// integrity is untrusted; confidentiality follows repo visibility.
362+
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
363+
return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult)
364+
}
365+
357366
var resourceIDInt int64
358367
var parseErr error
359368
switch method {
@@ -376,13 +385,17 @@ Use this tool to list workflows in a repository, or list workflow runs, jobs, an
376385

377386
switch method {
378387
case actionsMethodListWorkflows:
379-
return listWorkflows(ctx, client, owner, repo, pagination)
388+
result, payload, err := listWorkflows(ctx, client, owner, repo, pagination)
389+
return attachIFC(result), payload, err
380390
case actionsMethodListWorkflowRuns:
381-
return listWorkflowRuns(ctx, client, args, owner, repo, resourceID, pagination)
391+
result, payload, err := listWorkflowRuns(ctx, client, args, owner, repo, resourceID, pagination)
392+
return attachIFC(result), payload, err
382393
case actionsMethodListWorkflowJobs:
383-
return listWorkflowJobs(ctx, client, args, owner, repo, resourceIDInt, pagination)
394+
result, payload, err := listWorkflowJobs(ctx, client, args, owner, repo, resourceIDInt, pagination)
395+
return attachIFC(result), payload, err
384396
case actionsMethodListWorkflowArtifacts:
385-
return listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination)
397+
result, payload, err := listWorkflowArtifacts(ctx, client, owner, repo, resourceIDInt, pagination)
398+
return attachIFC(result), payload, err
386399
default:
387400
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
388401
}
@@ -465,6 +478,14 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
465478
return nil, nil, fmt.Errorf("failed to get GitHub client: %w", err)
466479
}
467480

481+
// attachIFC adds the IFC label to a successful Actions result when
482+
// IFC labels are enabled. Workflow runs, jobs, artifacts, usage,
483+
// and log URLs reflect attacker-influenceable run output, so
484+
// integrity is untrusted; confidentiality follows repo visibility.
485+
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
486+
return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult)
487+
}
488+
468489
var resourceIDInt int64
469490
var parseErr error
470491
switch method {
@@ -480,17 +501,23 @@ Use this tool to get details about individual workflows, workflow runs, jobs, an
480501

481502
switch method {
482503
case actionsMethodGetWorkflow:
483-
return getWorkflow(ctx, client, owner, repo, resourceID)
504+
result, payload, err := getWorkflow(ctx, client, owner, repo, resourceID)
505+
return attachIFC(result), payload, err
484506
case actionsMethodGetWorkflowRun:
485-
return getWorkflowRun(ctx, client, owner, repo, resourceIDInt)
507+
result, payload, err := getWorkflowRun(ctx, client, owner, repo, resourceIDInt)
508+
return attachIFC(result), payload, err
486509
case actionsMethodGetWorkflowJob:
487-
return getWorkflowJob(ctx, client, owner, repo, resourceIDInt)
510+
result, payload, err := getWorkflowJob(ctx, client, owner, repo, resourceIDInt)
511+
return attachIFC(result), payload, err
488512
case actionsMethodDownloadWorkflowArtifact:
489-
return downloadWorkflowArtifact(ctx, client, owner, repo, resourceIDInt)
513+
result, payload, err := downloadWorkflowArtifact(ctx, client, owner, repo, resourceIDInt)
514+
return attachIFC(result), payload, err
490515
case actionsMethodGetWorkflowRunUsage:
491-
return getWorkflowRunUsage(ctx, client, owner, repo, resourceIDInt)
516+
result, payload, err := getWorkflowRunUsage(ctx, client, owner, repo, resourceIDInt)
517+
return attachIFC(result), payload, err
492518
case actionsMethodGetWorkflowRunLogsURL:
493-
return getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt)
519+
result, payload, err := getWorkflowRunLogsURL(ctx, client, owner, repo, resourceIDInt)
520+
return attachIFC(result), payload, err
494521
default:
495522
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
496523
}
@@ -719,12 +746,22 @@ For single job logs, provide job_id. For all failed jobs in a run, provide run_i
719746
return utils.NewToolResultError("job_id is required when failed_only is false"), nil, nil
720747
}
721748

749+
// attachIFC adds the IFC label to a successful result when IFC
750+
// labels are enabled. Job logs echo attacker-influenceable run
751+
// output, so integrity is untrusted; confidentiality follows repo
752+
// visibility.
753+
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
754+
return attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, r, ifc.LabelActionsResult)
755+
}
756+
722757
if failedOnly && runID > 0 {
723758
// Handle failed-only mode: get logs for all failed jobs in the workflow run
724-
return handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize())
759+
result, payload, err := handleFailedJobLogs(ctx, client, owner, repo, int64(runID), returnContent, tailLines, deps.GetContentWindowSize())
760+
return attachIFC(result), payload, err
725761
} else if jobID > 0 {
726762
// Handle single job mode
727-
return handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize())
763+
result, payload, err := handleSingleJobLogs(ctx, client, owner, repo, int64(jobID), returnContent, tailLines, deps.GetContentWindowSize())
764+
return attachIFC(result), payload, err
728765
}
729766

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

pkg/github/code_scanning.go

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

99
ghErrors "github.com/github/github-mcp-server/pkg/errors"
10+
"github.com/github/github-mcp-server/pkg/ifc"
1011
"github.com/github/github-mcp-server/pkg/inventory"
1112
"github.com/github/github-mcp-server/pkg/scopes"
1213
"github.com/github/github-mcp-server/pkg/translations"
@@ -88,7 +89,12 @@ func GetCodeScanningAlert(t translations.TranslationHelperFunc) inventory.Server
8889
return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, nil
8990
}
9091

91-
return utils.NewToolResultText(string(r)), nil, nil
92+
result := utils.NewToolResultText(string(r))
93+
// Code scanning alerts are access-restricted regardless of repo
94+
// visibility and embed attacker-influenceable code snippets, so the
95+
// label is always private-untrusted.
96+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
97+
return result, nil, nil
9298
},
9399
)
94100
}
@@ -208,7 +214,12 @@ func ListCodeScanningAlerts(t translations.TranslationHelperFunc) inventory.Serv
208214
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, nil
209215
}
210216

211-
return utils.NewToolResultText(string(r)), nil, nil
217+
result := utils.NewToolResultText(string(r))
218+
// Code scanning alerts are access-restricted regardless of repo
219+
// visibility and embed attacker-influenceable code snippets, so the
220+
// label is always private-untrusted.
221+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
222+
return result, nil, nil
212223
},
213224
)
214225
}

pkg/github/context_tools.go

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -106,12 +106,7 @@ func GetMe(t translations.TranslationHelperFunc) inventory.ServerTool {
106106
}
107107

108108
result := MarshalledTextResult(minimalUser)
109-
if deps.IsFeatureEnabled(ctx, FeatureFlagIFCLabels) {
110-
if result.Meta == nil {
111-
result.Meta = mcp.Meta{}
112-
}
113-
result.Meta["ifc"] = ifc.LabelGetMe()
114-
}
109+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelGetMe())
115110
return result, nil, nil
116111
},
117112
)
@@ -221,7 +216,12 @@ func GetTeams(t translations.TranslationHelperFunc) inventory.ServerTool {
221216
organizations = append(organizations, orgTeams)
222217
}
223218

224-
return MarshalledTextResult(organizations), nil, nil
219+
result := MarshalledTextResult(organizations)
220+
// Team membership is maintained by GitHub and cannot be forged by
221+
// outside contributors (trusted). Org team rosters are visible only
222+
// to org members, so confidentiality is private.
223+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelTeam())
224+
return result, nil, nil
225225
},
226226
)
227227
}
@@ -292,7 +292,12 @@ func GetTeamMembers(t translations.TranslationHelperFunc) inventory.ServerTool {
292292
members = append(members, string(member.Login))
293293
}
294294

295-
return MarshalledTextResult(members), nil, nil
295+
result := MarshalledTextResult(members)
296+
// Team membership is maintained by GitHub and cannot be forged by
297+
// outside contributors (trusted). A team's member roster is visible
298+
// only to org members, so confidentiality is private.
299+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelTeam())
300+
return result, nil, nil
296301
},
297302
)
298303
}

pkg/github/context_tools_test.go

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

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

pkg/github/dependabot.go

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

1010
ghErrors "github.com/github/github-mcp-server/pkg/errors"
11+
"github.com/github/github-mcp-server/pkg/ifc"
1112
"github.com/github/github-mcp-server/pkg/inventory"
1213
"github.com/github/github-mcp-server/pkg/scopes"
1314
"github.com/github/github-mcp-server/pkg/translations"
@@ -89,7 +90,12 @@ func GetDependabotAlert(t translations.TranslationHelperFunc) inventory.ServerTo
8990
return utils.NewToolResultErrorFromErr("failed to marshal alert", err), nil, err
9091
}
9192

92-
return utils.NewToolResultText(string(r)), nil, nil
93+
result := utils.NewToolResultText(string(r))
94+
// Dependabot alerts are access-restricted regardless of repo
95+
// visibility and embed attacker-influenceable advisory text, so the
96+
// label is always private-untrusted.
97+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
98+
return result, nil, nil
9399
},
94100
)
95101
}
@@ -197,7 +203,12 @@ func ListDependabotAlerts(t translations.TranslationHelperFunc) inventory.Server
197203
return utils.NewToolResultErrorFromErr("failed to marshal alerts", err), nil, err
198204
}
199205

200-
return utils.NewToolResultText(string(r)), nil, nil
206+
result := utils.NewToolResultText(string(r))
207+
// Dependabot alerts are access-restricted regardless of repo
208+
// visibility and embed attacker-influenceable advisory text, so the
209+
// label is always private-untrusted.
210+
result = attachStaticIFCLabel(ctx, deps, result, ifc.LabelSecurityAlert())
211+
return result, nil, nil
201212
},
202213
)
203214
}

pkg/github/discussions.go

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

9+
"github.com/github/github-mcp-server/pkg/ifc"
910
"github.com/github/github-mcp-server/pkg/inventory"
1011
"github.com/github/github-mcp-server/pkg/scopes"
1112
"github.com/github/github-mcp-server/pkg/translations"
@@ -272,7 +273,11 @@ func ListDiscussions(t translations.TranslationHelperFunc) inventory.ServerTool
272273
if err != nil {
273274
return nil, nil, fmt.Errorf("failed to marshal discussions: %w", err)
274275
}
275-
return utils.NewToolResultText(string(out)), nil, nil
276+
result := utils.NewToolResultText(string(out))
277+
// Discussion content is user-authored (untrusted); confidentiality
278+
// follows repo visibility.
279+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelListIssues)
280+
return result, nil, nil
276281
},
277282
)
278283
}
@@ -376,7 +381,11 @@ func GetDiscussion(t translations.TranslationHelperFunc) inventory.ServerTool {
376381
return nil, nil, fmt.Errorf("failed to marshal discussion: %w", err)
377382
}
378383

379-
return utils.NewToolResultText(string(out)), nil, nil
384+
result := utils.NewToolResultText(string(out))
385+
// Discussion content is user-authored (untrusted); confidentiality
386+
// follows repo visibility.
387+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelListIssues)
388+
return result, nil, nil
380389
},
381390
)
382391
}
@@ -580,7 +589,11 @@ func GetDiscussionComments(t translations.TranslationHelperFunc) inventory.Serve
580589
return nil, nil, fmt.Errorf("failed to marshal comments: %w", err)
581590
}
582591

583-
return utils.NewToolResultText(string(out)), nil, nil
592+
result := utils.NewToolResultText(string(out))
593+
// Discussion comments are user-authored (untrusted); confidentiality
594+
// follows repo visibility.
595+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, params.Owner, params.Repo, result, ifc.LabelListIssues)
596+
return result, nil, nil
584597
},
585598
)
586599
}
@@ -1084,7 +1097,11 @@ func ListDiscussionCategories(t translations.TranslationHelperFunc) inventory.Se
10841097
if err != nil {
10851098
return nil, nil, fmt.Errorf("failed to marshal discussion categories: %w", err)
10861099
}
1087-
return utils.NewToolResultText(string(out)), nil, nil
1100+
result := utils.NewToolResultText(string(out))
1101+
// Discussion categories are repo-defined structural metadata
1102+
// (trusted); confidentiality follows repo visibility.
1103+
result = attachRepoVisibilityIFCLabelLazy(ctx, deps, owner, repo, result, ifc.LabelRepoMetadata)
1104+
return result, nil, nil
10881105
},
10891106
)
10901107
}

pkg/github/feature_flags.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@ var AllowedFeatureFlags = []string{
3636
var InsidersFeatureFlags = []string{
3737
MCPAppsFeatureFlag,
3838
FeatureFlagCSVOutput,
39-
FeatureFlagIFCLabels,
4039
FeatureFlagIssueFields,
4140
}
4241

pkg/github/feature_flags_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,10 +162,10 @@ func TestResolveFeatureFlags(t *testing.T) {
162162
expectedFlags: InsidersFeatureFlags,
163163
},
164164
{
165-
name: "insiders mode enables internal-only flags",
165+
name: "insiders mode does not auto-enable ifc labels",
166166
enabledFeatures: nil,
167167
insidersMode: true,
168-
expectedFlags: []string{FeatureFlagIFCLabels},
168+
unexpectedFlags: []string{FeatureFlagIFCLabels},
169169
},
170170
{
171171
name: "ifc_labels can be directly enabled",

pkg/github/gists.go

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

1010
ghErrors "github.com/github/github-mcp-server/pkg/errors"
11+
"github.com/github/github-mcp-server/pkg/ifc"
1112
"github.com/github/github-mcp-server/pkg/inventory"
1213
"github.com/github/github-mcp-server/pkg/scopes"
1314
"github.com/github/github-mcp-server/pkg/translations"
@@ -99,7 +100,15 @@ func ListGists(t translations.TranslationHelperFunc) inventory.ServerTool {
99100
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
100101
}
101102

102-
return utils.NewToolResultText(string(r)), nil, nil
103+
result := utils.NewToolResultText(string(r))
104+
// Gist contents are user-authored (untrusted); confidentiality is
105+
// the IFC join of each gist's own public/secret flag.
106+
visibilities := make([]bool, 0, len(gists))
107+
for _, g := range gists {
108+
visibilities = append(visibilities, g.GetPublic())
109+
}
110+
result = attachJoinedIFCLabel(ctx, deps, result, visibilities, ifc.LabelGistList)
111+
return result, nil, nil
103112
},
104113
)
105114
}
@@ -157,7 +166,11 @@ func GetGist(t translations.TranslationHelperFunc) inventory.ServerTool {
157166
return utils.NewToolResultErrorFromErr("failed to marshal response", err), nil, nil
158167
}
159168

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

pkg/github/git.go

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"strings"
88

99
ghErrors "github.com/github/github-mcp-server/pkg/errors"
10+
"github.com/github/github-mcp-server/pkg/ifc"
1011
"github.com/github/github-mcp-server/pkg/inventory"
1112
"github.com/github/github-mcp-server/pkg/scopes"
1213
"github.com/github/github-mcp-server/pkg/translations"
@@ -171,7 +172,13 @@ func GetRepositoryTree(t translations.TranslationHelperFunc) inventory.ServerToo
171172
return nil, nil, fmt.Errorf("failed to marshal response: %w", err)
172173
}
173174

174-
return utils.NewToolResultText(string(r)), nil, nil
175+
result := utils.NewToolResultText(string(r))
176+
// The repository tree exposes committed file structure; in public
177+
// repos anyone can land content via a PR (untrusted), in private
178+
// repos only collaborators can (trusted). Confidentiality follows
179+
// repo visibility.
180+
result = attachRepoVisibilityIFCLabel(ctx, deps, client, owner, repo, result, ifc.LabelCommitContents)
181+
return result, nil, nil
175182
},
176183
)
177184
}

0 commit comments

Comments
 (0)