Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 38 additions & 5 deletions pkg/github/issues.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"time"

ghErrors "github.com/github/github-mcp-server/pkg/errors"
"github.com/github/github-mcp-server/pkg/ifc"
"github.com/github/github-mcp-server/pkg/inventory"
"github.com/github/github-mcp-server/pkg/sanitize"
"github.com/github/github-mcp-server/pkg/scopes"
Expand Down Expand Up @@ -130,6 +131,7 @@ type IssueFragment struct {
// Common interface for all issue query types
type IssueQueryResult interface {
GetIssueFragment() IssueQueryFragment
GetIsPrivate() bool
}

type IssueQueryFragment struct {
Expand All @@ -146,28 +148,32 @@ type IssueQueryFragment 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})"`
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction})"`
IsPrivate githubv4.Boolean
} `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})"`
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction})"`
IsPrivate githubv4.Boolean
} `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})"`
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"`
IsPrivate githubv4.Boolean
} `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})"`
Issues IssueQueryFragment `graphql:"issues(first: $first, after: $after, labels: $labels, states: $states, orderBy: {field: $orderBy, direction: $direction}, filterBy: {since: $since})"`
IsPrivate githubv4.Boolean
} `graphql:"repository(owner: $owner, name: $repo)"`
}

Expand All @@ -176,18 +182,28 @@ 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 getIssueQueryType(hasLabels bool, hasSince bool) any {
switch {
case hasLabels && hasSince:
Expand Down Expand Up @@ -1568,11 +1584,28 @@ func ListIssues(t translations.TranslationHelperFunc) inventory.ServerTool {
}

var resp MinimalIssuesResponse
var isPrivate bool
if queryResult, ok := issueQuery.(IssueQueryResult); ok {
resp = convertToMinimalIssuesResponse(queryResult.GetIssueFragment())
isPrivate = queryResult.GetIsPrivate()
}

return MarshalledTextResult(resp), nil, nil
result := MarshalledTextResult(resp)
if deps.GetFlags(ctx).InsidersMode {
if result.Meta == nil {
result.Meta = mcp.Meta{}
}
// TODO(fides): for private repos, populate readers with the
// repository's collaborator logins. Using [owner] as a
// placeholder until a shared visibility/collaborators helper
// lands (tracked under copilot-mcp-core#1623).
var readers []string
Comment thread
gokhanarkan marked this conversation as resolved.
Resolved
if isPrivate {
readers = []string{owner}
}
result.Meta["ifc"] = ifc.LabelListIssues(isPrivate, readers)
}
return result, nil, nil
})
}

Expand Down
139 changes: 137 additions & 2 deletions pkg/github/issues_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,7 @@ func Test_ListIssues(t *testing.T) {
},
"totalCount": 2,
},
"isPrivate": false,
},
})

Expand All @@ -1132,6 +1133,7 @@ func Test_ListIssues(t *testing.T) {
},
"totalCount": 2,
},
"isPrivate": false,
},
})

Expand All @@ -1147,6 +1149,7 @@ func Test_ListIssues(t *testing.T) {
},
"totalCount": 1,
},
"isPrivate": false,
},
})

Expand Down Expand Up @@ -1272,8 +1275,8 @@ func Test_ListIssues(t *testing.T) {
}

// Define the actual query strings that match the implementation
qBasicNoLabels := "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}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
qWithLabels := "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}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount}}}"
qBasicNoLabels := "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}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"
qWithLabels := "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}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},pageInfo{hasNextPage,hasPreviousPage,startCursor,endCursor},totalCount},isPrivate}}"

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
Expand Down Expand Up @@ -1349,6 +1352,138 @@ func Test_ListIssues(t *testing.T) {
}
}

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!$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}){nodes{number,title,body,state,databaseId,author{login},createdAt,updatedAt,labels(first: 100){nodes{name,id,description}},comments{totalCount}},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),
}

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,
Flags: FeatureFlags{InsidersMode: 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) {
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(false))
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
deps := BaseDeps{
GQLClient: gqlClient,
Flags: FeatureFlags{InsidersMode: true},
}
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"])
confList, ok := ifcMap["confidentiality"].([]any)
require.True(t, ok, "confidentiality should be a list")
require.Len(t, confList, 1)
assert.Equal(t, "public", confList[0])
})

t.Run("insiders mode enabled on private repo emits private untrusted label", func(t *testing.T) {
matcher := githubv4mock.NewQueryMatcher(query, vars, makeResponse(true))
gqlClient := githubv4.NewClient(githubv4mock.NewMockedHTTPClient(matcher))
deps := BaseDeps{
GQLClient: gqlClient,
Flags: FeatureFlags{InsidersMode: true},
}
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"])
confList, ok := ifcMap["confidentiality"].([]any)
require.True(t, ok, "confidentiality should be a list")
require.Len(t, confList, 1)
assert.Equal(t, "octocat", confList[0])
})
}

func Test_UpdateIssue(t *testing.T) {
// Verify tool definition
serverTool := IssueWrite(translations.NullTranslationHelper)
Expand Down
50 changes: 49 additions & 1 deletion pkg/ifc/ifc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,57 @@ type SecurityLabel struct {
Confidentiality []Confidentiality `json:"confidentiality"`
}

func LabelGetMe() SecurityLabel {
// PublicTrusted returns a label for trusted, publicly readable data.
func PublicTrusted() SecurityLabel {
return SecurityLabel{
Integrity: IntegrityTrusted,
Confidentiality: []Confidentiality{ConfidentialityPublic},
}
}

// PublicUntrusted returns a label for untrusted, publicly readable data.
func PublicUntrusted() SecurityLabel {
return SecurityLabel{
Integrity: IntegrityUntrusted,
Confidentiality: []Confidentiality{ConfidentialityPublic},
}
}

// PrivateTrusted returns a label for trusted data restricted to the given readers.
func PrivateTrusted(readers []string) SecurityLabel {
return SecurityLabel{
Integrity: IntegrityTrusted,
Confidentiality: toConfidentiality(readers),
}
}

// PrivateUntrusted returns a label for untrusted data restricted to the given readers.
func PrivateUntrusted(readers []string) SecurityLabel {
return SecurityLabel{
Integrity: IntegrityUntrusted,
Confidentiality: toConfidentiality(readers),
}
}

func toConfidentiality(readers []string) []Confidentiality {
out := make([]Confidentiality, len(readers))
for i, r := range readers {
out[i] = Confidentiality(r)
}
return out
}

func LabelGetMe() SecurityLabel {
return PublicTrusted()
}

// LabelListIssues returns the IFC label for a list_issues result.
// Public repositories are universally readable; private repositories are
// restricted to the provided reader set (typically repository collaborators).
// Issue contents are attacker-controllable, so integrity is always untrusted.
func LabelListIssues(isPrivate bool, readers []string) SecurityLabel {
if isPrivate {
return PrivateUntrusted(readers)
}
return PublicUntrusted()
}
Loading