Skip to content

Commit 6181edd

Browse files
committed
Add ifc label for issue_read tool
Emits an IFC SecurityLabel on the issue_read tool result when the InsidersMode flag is enabled, mirroring the pattern landed for get_me in #2432, list_issues in #2453, get_file_contents in #2454, and search_issues in #2456. issue_read operates on a single issue in a single repository so the label has the same per-repo semantics as list_issues; the helper ifc.LabelListIssues is reused directly. Integrity is always untrusted (issue contents, comments, and label descriptions are user-authored). Public repos are labelled PublicUntrusted; private repos are labelled PrivateUntrusted with the repository's collaborator logins, falling back to [owner] when the collaborators lookup fails. The IssueRead handler dispatches to four sub-functions (GetIssue, GetIssueComments, GetSubIssues, GetIssueLabels). The IFC label is attached at the dispatch site via a single attachIFC closure, so all four method branches emit the label without changes to the underlying helpers. Visibility-lookup failures cause the label to be omitted entirely (consistent with get_file_contents and search_issues). A future cleanup PR can extract attachIFC into a shared helper now that get_file_contents, search_issues, and issue_read use near-identical closures; intentionally not bundled here to keep the diff minimal. Refs github/copilot-mcp-core#1623, github/copilot-mcp-core#1389. Note: chained on #2456 (gokhanarkan/fides-search-issues), which is in turn chained on #2454. GitHub will retarget the base to main once those merge.
1 parent 279a59b commit 6181edd

2 files changed

Lines changed: 167 additions & 4 deletions

File tree

pkg/github/issues.go

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,19 +296,61 @@ Options are:
296296
return utils.NewToolResultErrorFromErr("failed to get GitHub graphql client", err), nil, nil
297297
}
298298

299+
// attachIFC adds the IFC label to a successful tool result when
300+
// InsidersMode is enabled. The visibility and (for private
301+
// repositories) collaborators lookups are performed lazily on
302+
// first use. If the visibility lookup fails the label is omitted
303+
// rather than misclassifying the result; the failure is not
304+
// cached so a subsequent dispatch branch could retry. If only
305+
// the collaborators lookup fails for a private repo we fall back
306+
// to the owner so the reader set is never empty. The label
307+
// matches list_issues semantics: per-repo visibility, integrity
308+
// always untrusted.
309+
var (
310+
ifcLabelKnown bool
311+
ifcIsPrivate bool
312+
ifcReaders []string
313+
)
314+
attachIFC := func(r *mcp.CallToolResult) *mcp.CallToolResult {
315+
if r == nil || r.IsError || !deps.GetFlags(ctx).InsidersMode {
316+
return r
317+
}
318+
if !ifcLabelKnown {
319+
isPrivate, err := FetchRepoIsPrivate(ctx, client, owner, repo)
320+
if err != nil {
321+
return r
322+
}
323+
ifcIsPrivate = isPrivate
324+
if ifcIsPrivate {
325+
if collaborators, err := FetchRepoCollaborators(ctx, client, owner, repo); err == nil {
326+
ifcReaders = collaborators
327+
}
328+
if len(ifcReaders) == 0 {
329+
ifcReaders = []string{owner}
330+
}
331+
}
332+
ifcLabelKnown = true
333+
}
334+
if r.Meta == nil {
335+
r.Meta = mcp.Meta{}
336+
}
337+
r.Meta["ifc"] = ifc.LabelListIssues(ifcIsPrivate, ifcReaders)
338+
return r
339+
}
340+
299341
switch method {
300342
case "get":
301343
result, err := GetIssue(ctx, client, deps, owner, repo, issueNumber)
302-
return result, nil, err
344+
return attachIFC(result), nil, err
303345
case "get_comments":
304346
result, err := GetIssueComments(ctx, client, deps, owner, repo, issueNumber, pagination)
305-
return result, nil, err
347+
return attachIFC(result), nil, err
306348
case "get_sub_issues":
307349
result, err := GetSubIssues(ctx, client, deps, owner, repo, issueNumber, pagination)
308-
return result, nil, err
350+
return attachIFC(result), nil, err
309351
case "get_labels":
310352
result, err := GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber)
311-
return result, nil, err
353+
return attachIFC(result), nil, err
312354
default:
313355
return utils.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil, nil
314356
}

pkg/github/issues_test.go

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,127 @@ func Test_GetIssue(t *testing.T) {
275275
}
276276
}
277277

278+
func Test_IssueRead_IFC_InsidersMode(t *testing.T) {
279+
t.Parallel()
280+
281+
serverTool := IssueRead(translations.NullTranslationHelper)
282+
283+
mockIssue := &github.Issue{
284+
Number: github.Ptr(1),
285+
Title: github.Ptr("Test"),
286+
Body: github.Ptr("body"),
287+
State: github.Ptr("open"),
288+
HTMLURL: github.Ptr("https://github.com/octocat/repo/issues/1"),
289+
User: &github.User{Login: github.Ptr("u")},
290+
}
291+
292+
mockComments := []*github.IssueComment{
293+
{Body: github.Ptr("hello"), User: &github.User{Login: github.Ptr("u")}},
294+
}
295+
296+
makeMockClient := func(isPrivate bool, repoStatus int) *http.Client {
297+
handlers := map[string]http.HandlerFunc{
298+
GetReposIssuesByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockIssue),
299+
GetReposIssuesCommentsByOwnerByRepoByIssueNumber: mockResponse(t, http.StatusOK, mockComments),
300+
GetReposCollaboratorsByOwnerByRepo: mockResponse(t, http.StatusOK, []*github.User{
301+
{Login: github.Ptr("octocat")},
302+
{Login: github.Ptr("alice")},
303+
}),
304+
}
305+
if repoStatus != 0 && repoStatus != http.StatusOK {
306+
handlers[GetReposByOwnerByRepo] = mockResponse(t, repoStatus, "boom")
307+
} else {
308+
handlers[GetReposByOwnerByRepo] = mockResponse(t, http.StatusOK, map[string]any{
309+
"name": "repo",
310+
"private": isPrivate,
311+
})
312+
}
313+
return MockHTTPClientWithHandlers(handlers)
314+
}
315+
316+
getReq := map[string]any{
317+
"method": "get",
318+
"owner": "octocat",
319+
"repo": "repo",
320+
"issue_number": float64(1),
321+
}
322+
commentsReq := map[string]any{
323+
"method": "get_comments",
324+
"owner": "octocat",
325+
"repo": "repo",
326+
"issue_number": float64(1),
327+
}
328+
329+
t.Run("insiders mode disabled omits ifc label", func(t *testing.T) {
330+
deps := BaseDeps{
331+
Client: github.NewClient(makeMockClient(false, 0)),
332+
Flags: FeatureFlags{InsidersMode: false},
333+
}
334+
handler := serverTool.Handler(deps)
335+
336+
request := createMCPRequest(getReq)
337+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
338+
require.NoError(t, err)
339+
require.False(t, result.IsError)
340+
341+
assert.Nil(t, result.Meta)
342+
})
343+
344+
t.Run("insiders mode enabled on public repo emits public untrusted", func(t *testing.T) {
345+
deps := BaseDeps{
346+
Client: github.NewClient(makeMockClient(false, 0)),
347+
Flags: FeatureFlags{InsidersMode: true},
348+
}
349+
handler := serverTool.Handler(deps)
350+
351+
request := createMCPRequest(getReq)
352+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
353+
require.NoError(t, err)
354+
require.False(t, result.IsError)
355+
356+
require.NotNil(t, result.Meta)
357+
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
358+
assert.Equal(t, "untrusted", ifcMap["integrity"])
359+
assert.Equal(t, []any{"public"}, ifcMap["confidentiality"])
360+
})
361+
362+
t.Run("insiders mode enabled on private repo with get_comments emits private untrusted", func(t *testing.T) {
363+
deps := BaseDeps{
364+
Client: github.NewClient(makeMockClient(true, 0)),
365+
Flags: FeatureFlags{InsidersMode: true},
366+
}
367+
handler := serverTool.Handler(deps)
368+
369+
request := createMCPRequest(commentsReq)
370+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
371+
require.NoError(t, err)
372+
require.False(t, result.IsError)
373+
374+
require.NotNil(t, result.Meta)
375+
ifcMap := unmarshalIFC(t, result.Meta["ifc"])
376+
assert.Equal(t, "untrusted", ifcMap["integrity"])
377+
assert.Equal(t, []any{"octocat", "alice"}, ifcMap["confidentiality"])
378+
})
379+
380+
t.Run("insiders mode skips ifc label when visibility lookup fails", func(t *testing.T) {
381+
deps := BaseDeps{
382+
Client: github.NewClient(makeMockClient(false, http.StatusInternalServerError)),
383+
Flags: FeatureFlags{InsidersMode: true},
384+
}
385+
handler := serverTool.Handler(deps)
386+
387+
request := createMCPRequest(getReq)
388+
result, err := handler(ContextWithDeps(context.Background(), deps), &request)
389+
require.NoError(t, err)
390+
require.False(t, result.IsError, "tool call should still succeed when visibility lookup fails")
391+
392+
if result.Meta != nil {
393+
_, hasIFC := result.Meta["ifc"]
394+
assert.False(t, hasIFC, "ifc label should be omitted when visibility lookup fails")
395+
}
396+
})
397+
}
398+
278399
func Test_AddIssueComment(t *testing.T) {
279400
// Verify tool definition once
280401
serverTool := AddIssueComment(translations.NullTranslationHelper)

0 commit comments

Comments
 (0)