From 51bc74d6bd4314039e39127a067c6b83dedae7de Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Fri, 7 Nov 2025 11:03:39 +0100 Subject: [PATCH 1/6] Add lockdown mode to filter issue --- cmd/github-mcp-server/generate_docs.go | 4 +- cmd/github-mcp-server/main.go | 3 + internal/ghmcp/server.go | 19 ++- pkg/github/feature_flags.go | 6 + pkg/github/issues.go | 14 +- pkg/github/issues_test.go | 173 +++++++++++++++++++++---- pkg/github/server_test.go | 6 + pkg/github/tools.go | 4 +- pkg/lockdown/lockdown.go | 96 ++++++++++++++ 9 files changed, 291 insertions(+), 34 deletions(-) create mode 100644 pkg/github/feature_flags.go create mode 100644 pkg/lockdown/lockdown.go diff --git a/cmd/github-mcp-server/generate_docs.go b/cmd/github-mcp-server/generate_docs.go index 6e3d5353b..4886d7815 100644 --- a/cmd/github-mcp-server/generate_docs.go +++ b/cmd/github-mcp-server/generate_docs.go @@ -64,7 +64,7 @@ func generateReadmeDocs(readmePath string) error { t, _ := translations.TranslationHelper() // Create toolset group with mock clients - tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000) + tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}) // Generate toolsets documentation toolsetsDoc := generateToolsetsDoc(tsg) @@ -302,7 +302,7 @@ func generateRemoteToolsetsDoc() string { t, _ := translations.TranslationHelper() // Create toolset group with mock clients - tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000) + tsg := github.DefaultToolsetGroup(false, mockGetClient, mockGetGQLClient, mockGetRawClient, t, 5000, github.FeatureFlags{}) // Generate table header buf.WriteString("| Name | Description | API URL | 1-Click Install (VS Code) | Read-only Link | 1-Click Read-only Install (VS Code) |\n") diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 647ec1d19..99943905a 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -61,6 +61,7 @@ var ( EnableCommandLogging: viper.GetBool("enable-command-logging"), LogFilePath: viper.GetString("log-file"), ContentWindowSize: viper.GetInt("content-window-size"), + LockdownEnabled: viper.GetBool("lockdown-enabled"), } return ghmcp.RunStdioServer(stdioServerConfig) }, @@ -82,6 +83,7 @@ func init() { rootCmd.PersistentFlags().Bool("export-translations", false, "Save translations to a JSON file") rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size") + rootCmd.PersistentFlags().Bool("lockdown-enabled", false, "Enable lockdown mode") // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) @@ -92,6 +94,7 @@ func init() { _ = viper.BindPFlag("export-translations", rootCmd.PersistentFlags().Lookup("export-translations")) _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) _ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size")) + _ = viper.BindPFlag("lockdown-enabled", rootCmd.PersistentFlags().Lookup("lockdown-enabled")) // Add subcommands rootCmd.AddCommand(stdioCmd) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 5e1ee58c8..3343bd92f 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -51,6 +51,9 @@ type MCPServerConfig struct { // Content window size ContentWindowSize int + + // LockdownEnabled indicates if we should enable lockdown mode + LockdownEnabled bool } const stdioServerLogPrefix = "stdioserver" @@ -154,7 +157,15 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { } // Create default toolsets - tsg := github.DefaultToolsetGroup(cfg.ReadOnly, getClient, getGQLClient, getRawClient, cfg.Translator, cfg.ContentWindowSize) + tsg := github.DefaultToolsetGroup( + cfg.ReadOnly, + getClient, + getGQLClient, + getRawClient, + cfg.Translator, + cfg.ContentWindowSize, + github.FeatureFlags{LockdownEnabled: cfg.LockdownEnabled}, + ) err = tsg.EnableToolsets(enabledToolsets, nil) if err != nil { @@ -205,6 +216,9 @@ type StdioServerConfig struct { // Content window size ContentWindowSize int + + // LockdownEnabled indicates if we should enable lockdown mode + LockdownEnabled bool } // RunStdioServer is not concurrent safe. @@ -224,6 +238,7 @@ func RunStdioServer(cfg StdioServerConfig) error { ReadOnly: cfg.ReadOnly, Translator: t, ContentWindowSize: cfg.ContentWindowSize, + LockdownEnabled: cfg.LockdownEnabled, }) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) @@ -245,7 +260,7 @@ func RunStdioServer(cfg StdioServerConfig) error { slogHandler = slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: slog.LevelInfo}) } logger := slog.New(slogHandler) - logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly) + logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownEnabled) stdLogger := log.New(logOutput, stdioServerLogPrefix, 0) stdioServer.SetErrorLogger(stdLogger) diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go new file mode 100644 index 000000000..720dcd018 --- /dev/null +++ b/pkg/github/feature_flags.go @@ -0,0 +1,6 @@ +package github + +// FeatureFlags defines runtime feature toggles that adjust tool behavior. +type FeatureFlags struct { + LockdownEnabled bool +} diff --git a/pkg/github/issues.go b/pkg/github/issues.go index b61b3e152..4a52586a8 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -10,6 +10,7 @@ import ( "time" ghErrors "github.com/github/github-mcp-server/pkg/errors" + "github.com/github/github-mcp-server/pkg/lockdown" "github.com/github/github-mcp-server/pkg/sanitize" "github.com/github/github-mcp-server/pkg/translations" "github.com/go-viper/mapstructure/v2" @@ -227,7 +228,7 @@ func fragmentToIssue(fragment IssueFragment) *github.Issue { } // GetIssue creates a tool to get details of a specific issue in a GitHub repository. -func IssueRead(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc) (tool mcp.Tool, handler server.ToolHandlerFunc) { +func IssueRead(getClient GetClientFn, getGQLClient GetGQLClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (tool mcp.Tool, handler server.ToolHandlerFunc) { return mcp.NewTool("issue_read", mcp.WithDescription(t("TOOL_ISSUE_READ_DESCRIPTION", "Get information about a specific issue in a GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ @@ -296,7 +297,7 @@ Options are: switch method { case "get": - return GetIssue(ctx, client, owner, repo, issueNumber) + return GetIssue(ctx, client, gqlClient, owner, repo, issueNumber, flags) case "get_comments": return GetIssueComments(ctx, client, owner, repo, issueNumber, pagination) case "get_sub_issues": @@ -309,7 +310,7 @@ Options are: } } -func GetIssue(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) { +func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Client, owner string, repo string, issueNumber int, flags FeatureFlags) (*mcp.CallToolResult, error) { issue, resp, err := client.Issues.Get(ctx, owner, repo, issueNumber) if err != nil { return nil, fmt.Errorf("failed to get issue: %w", err) @@ -324,6 +325,13 @@ func GetIssue(ctx context.Context, client *github.Client, owner string, repo str return mcp.NewToolResultError(fmt.Sprintf("failed to get issue: %s", string(body))), nil } + if flags.LockdownEnabled { + shouldFilter := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, *issue.Repository.Owner.Login, *issue.Repository.Name) + if shouldFilter { + return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil + } + } + // Sanitize title/body on response if issue != nil { if issue.Title != nil { diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 03c57ce75..68761ec59 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -22,8 +22,8 @@ import ( func Test_GetIssue(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - mockGQLClient := githubv4.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQLClient), translations.NullTranslationHelper) + defaultGQLClient := githubv4.NewClient(nil) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(defaultGQLClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -44,15 +44,24 @@ func Test_GetIssue(t *testing.T) { User: &github.User{ Login: github.Ptr("testuser"), }, + Repository: &github.Repository{ + Name: github.Ptr("repo"), + Owner: &github.User{ + Login: github.Ptr("owner"), + }, + }, } tests := []struct { - name string - mockedClient *http.Client - requestArgs map[string]interface{} - expectError bool - expectedIssue *github.Issue - expectedErrMsg string + name string + mockedClient *http.Client + gqlHTTPClient *http.Client + requestArgs map[string]interface{} + expectHandlerError bool + expectResultError bool + expectedIssue *github.Issue + expectedErrMsg string + lockdownEnabled bool }{ { name: "successful issue retrieval", @@ -68,7 +77,6 @@ func Test_GetIssue(t *testing.T) { "repo": "repo", "issue_number": float64(42), }, - expectError: false, expectedIssue: mockIssue, }, { @@ -85,34 +93,149 @@ func Test_GetIssue(t *testing.T) { "repo": "repo", "issue_number": float64(999), }, - expectError: true, - expectedErrMsg: "failed to get issue", + expectHandlerError: true, + expectedErrMsg: "failed to get issue", + }, + { + name: "lockdown enabled - private repository", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesByOwnerByRepoByIssueNumber, + mockIssue, + ), + ), + gqlHTTPClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $name)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "isPrivate": true, + }, + }), + ), + ), + requestArgs: map[string]interface{}{ + "method": "get", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectedIssue: mockIssue, + lockdownEnabled: true, + }, + { + name: "lockdown enabled - user lacks push access", + mockedClient: mock.NewMockedHTTPClient( + mock.WithRequestMatch( + mock.GetReposIssuesByOwnerByRepoByIssueNumber, + mockIssue, + ), + ), + gqlHTTPClient: githubv4mock.NewMockedHTTPClient( + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $name)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "isPrivate": false, + }, + }), + ), + githubv4mock.NewQueryMatcher( + struct { + Repository struct { + Collaborators struct { + Edges []struct { + Permission githubv4.String + Node struct { + Login githubv4.String + } + } + } `graphql:"collaborators(query: $username, first: 1)"` + } `graphql:"repository(owner: $owner, name: $name)"` + }{}, + map[string]any{ + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + "username": githubv4.String("testuser"), + }, + githubv4mock.DataResponse(map[string]any{ + "repository": map[string]any{ + "collaborators": map[string]any{ + "edges": []any{ + map[string]any{ + "permission": "READ", + "node": map[string]any{ + "login": "testuser", + }, + }, + }, + }, + }, + }), + ), + ), + requestArgs: map[string]interface{}{ + "method": "get", + "owner": "owner", + "repo": "repo", + "issue_number": float64(42), + }, + expectResultError: true, + expectedErrMsg: "access to issue details is restricted by lockdown mode", + lockdownEnabled: true, }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(mockGQLClient), translations.NullTranslationHelper) - // Create call request - request := createMCPRequest(tc.requestArgs) + var gqlClient *githubv4.Client + if tc.gqlHTTPClient != nil { + gqlClient = githubv4.NewClient(tc.gqlHTTPClient) + } else { + gqlClient = defaultGQLClient + } - // Call handler + flags := stubFeatureFlags(map[string]bool{"lockdown": tc.lockdownEnabled}) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, flags) + + request := createMCPRequest(tc.requestArgs) result, err := handler(context.Background(), request) - // Verify results - if tc.expectError { + if tc.expectHandlerError { require.Error(t, err) assert.Contains(t, err.Error(), tc.expectedErrMsg) return } require.NoError(t, err) + require.NotNil(t, result) + + if tc.expectResultError { + errorContent := getErrorResult(t, result) + assert.Contains(t, errorContent.Text, tc.expectedErrMsg) + return + } + textContent := getTextResult(t, result) - // Unmarshal and verify the result var returnedIssue github.Issue err = json.Unmarshal([]byte(textContent.Text), &returnedIssue) require.NoError(t, err) @@ -1589,7 +1712,7 @@ func Test_GetIssueComments(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) gqlClient := githubv4.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -1695,7 +1818,7 @@ func Test_GetIssueComments(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) gqlClient := githubv4.NewClient(nil) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1732,7 +1855,7 @@ func Test_GetIssueLabels(t *testing.T) { // Verify tool definition mockGQClient := githubv4.NewClient(nil) mockClient := github.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQClient), translations.NullTranslationHelper) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -1807,7 +1930,7 @@ func Test_GetIssueLabels(t *testing.T) { t.Run(tc.name, func(t *testing.T) { gqlClient := githubv4.NewClient(tc.mockedClient) client := github.NewClient(nil) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) request := createMCPRequest(tc.requestArgs) result, err := handler(context.Background(), request) @@ -2498,7 +2621,7 @@ func Test_GetSubIssues(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) gqlClient := githubv4.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -2695,7 +2818,7 @@ func Test_GetSubIssues(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) gqlClient := githubv4.NewClient(nil) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index b92664d75..934a7a8ce 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -38,6 +38,12 @@ func stubGetGQLClientFn(client *githubv4.Client) GetGQLClientFn { } } +func stubFeatureFlags(enabledFlags map[string]bool) FeatureFlags { + return FeatureFlags{ + LockdownEnabled: enabledFlags["lockdown"], + } +} + func stubGetRawClientFn(client *raw.Client) raw.GetRawClientFn { return func(_ context.Context) (*raw.Client, error) { return client, nil diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 4296aaa72..85b726a98 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -155,7 +155,7 @@ func GetDefaultToolsetIDs() []string { } } -func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc, contentWindowSize int) *toolsets.ToolsetGroup { +func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetGQLClientFn, getRawClient raw.GetRawClientFn, t translations.TranslationHelperFunc, contentWindowSize int, flags FeatureFlags) *toolsets.ToolsetGroup { tsg := toolsets.NewToolsetGroup(readOnly) // Define all available features with their default state (disabled) @@ -191,7 +191,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG ) issues := toolsets.NewToolset(ToolsetMetadataIssues.ID, ToolsetMetadataIssues.Description). AddReadTools( - toolsets.NewServerTool(IssueRead(getClient, getGQLClient, t)), + toolsets.NewServerTool(IssueRead(getClient, getGQLClient, t, flags)), toolsets.NewServerTool(SearchIssues(getClient, t)), toolsets.NewServerTool(ListIssues(getGQLClient, t)), toolsets.NewServerTool(ListIssueTypes(getClient, t)), diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go new file mode 100644 index 000000000..843246717 --- /dev/null +++ b/pkg/lockdown/lockdown.go @@ -0,0 +1,96 @@ +package lockdown + +import ( + "context" + "fmt" + "strings" + + "github.com/shurcooL/githubv4" +) + +func ShouldRemoveContent(ctx context.Context, client *githubv4.Client, username, owner, repo string) bool { + isPrivate, err := IsPrivateRepo(ctx, client, owner, repo) + if err != nil { + return false + } + + // Do not filter content for private repositories + if isPrivate { + return false + } + hasPushAccess, err := HasPushAccess(ctx, client, username, owner, repo) + if err != nil { + return false + } + + return !hasPushAccess +} + +func HasPushAccess(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, error) { + if client == nil { + return false, fmt.Errorf("nil GraphQL client") + } + + var query struct { + Repository struct { + Collaborators struct { + Edges []struct { + Permission githubv4.String + Node struct { + Login githubv4.String + } + } + } `graphql:"collaborators(query: $username, first: 1)"` + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + "username": githubv4.String(username), + } + + err := client.Query(ctx, &query, variables) + if err != nil { + return false, fmt.Errorf("failed to query user permissions: %w", err) + } + + // Check if the user has push access + hasPush := false + for _, edge := range query.Repository.Collaborators.Edges { + login := string(edge.Node.Login) + if strings.EqualFold(login, username) { + permission := string(edge.Permission) + // WRITE, ADMIN, and MAINTAIN permissions have push access + hasPush = permission == "WRITE" || permission == "ADMIN" || permission == "MAINTAIN" + break + } + } + + return hasPush, nil +} + +// IsPrivateRepo checks if a repository is private using GraphQL +func IsPrivateRepo(ctx context.Context, client *githubv4.Client, owner, repo string) (bool, error) { + if client == nil { + return false, fmt.Errorf("nil GraphQL client") + } + + var query struct { + Repository struct { + IsPrivate githubv4.Boolean + } `graphql:"repository(owner: $owner, name: $name)"` + } + + variables := map[string]interface{}{ + "owner": githubv4.String(owner), + "name": githubv4.String(repo), + } + + err := client.Query(ctx, &query, variables) + if err != nil { + return false, fmt.Errorf("failed to query repository visibility: %w", err) + } + + return bool(query.Repository.IsPrivate), nil +} From 89f62f0202f9858c7d63607b0a013f0f9b54b536 Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Fri, 7 Nov 2025 12:10:17 +0100 Subject: [PATCH 2/6] Update flag name --- cmd/github-mcp-server/main.go | 6 +++--- internal/ghmcp/server.go | 14 +++++++------- pkg/github/feature_flags.go | 2 +- pkg/github/issues.go | 22 ++++++++++++---------- pkg/github/issues_test.go | 16 ++++++++-------- pkg/github/pullrequests.go | 4 ++-- pkg/github/pullrequests_test.go | 24 ++++++++++++------------ pkg/github/server_test.go | 2 +- pkg/github/tools.go | 2 +- 9 files changed, 47 insertions(+), 45 deletions(-) diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 99943905a..125cd5a8d 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -61,7 +61,7 @@ var ( EnableCommandLogging: viper.GetBool("enable-command-logging"), LogFilePath: viper.GetString("log-file"), ContentWindowSize: viper.GetInt("content-window-size"), - LockdownEnabled: viper.GetBool("lockdown-enabled"), + LockdownMode: viper.GetBool("lockdown-mode"), } return ghmcp.RunStdioServer(stdioServerConfig) }, @@ -83,7 +83,7 @@ func init() { rootCmd.PersistentFlags().Bool("export-translations", false, "Save translations to a JSON file") rootCmd.PersistentFlags().String("gh-host", "", "Specify the GitHub hostname (for GitHub Enterprise etc.)") rootCmd.PersistentFlags().Int("content-window-size", 5000, "Specify the content window size") - rootCmd.PersistentFlags().Bool("lockdown-enabled", false, "Enable lockdown mode") + rootCmd.PersistentFlags().Bool("lockdown-mode", false, "Enable lockdown mode") // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) @@ -94,7 +94,7 @@ func init() { _ = viper.BindPFlag("export-translations", rootCmd.PersistentFlags().Lookup("export-translations")) _ = viper.BindPFlag("host", rootCmd.PersistentFlags().Lookup("gh-host")) _ = viper.BindPFlag("content-window-size", rootCmd.PersistentFlags().Lookup("content-window-size")) - _ = viper.BindPFlag("lockdown-enabled", rootCmd.PersistentFlags().Lookup("lockdown-enabled")) + _ = viper.BindPFlag("lockdown-mode", rootCmd.PersistentFlags().Lookup("lockdown-mode")) // Add subcommands rootCmd.AddCommand(stdioCmd) diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 3343bd92f..0721354cd 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -52,8 +52,8 @@ type MCPServerConfig struct { // Content window size ContentWindowSize int - // LockdownEnabled indicates if we should enable lockdown mode - LockdownEnabled bool + // LockdownMode indicates if we should enable lockdown mode + LockdownMode bool } const stdioServerLogPrefix = "stdioserver" @@ -164,7 +164,7 @@ func NewMCPServer(cfg MCPServerConfig) (*server.MCPServer, error) { getRawClient, cfg.Translator, cfg.ContentWindowSize, - github.FeatureFlags{LockdownEnabled: cfg.LockdownEnabled}, + github.FeatureFlags{LockdownMode: cfg.LockdownMode}, ) err = tsg.EnableToolsets(enabledToolsets, nil) @@ -217,8 +217,8 @@ type StdioServerConfig struct { // Content window size ContentWindowSize int - // LockdownEnabled indicates if we should enable lockdown mode - LockdownEnabled bool + // LockdownMode indicates if we should enable lockdown mode + LockdownMode bool } // RunStdioServer is not concurrent safe. @@ -238,7 +238,7 @@ func RunStdioServer(cfg StdioServerConfig) error { ReadOnly: cfg.ReadOnly, Translator: t, ContentWindowSize: cfg.ContentWindowSize, - LockdownEnabled: cfg.LockdownEnabled, + LockdownMode: cfg.LockdownMode, }) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) @@ -260,7 +260,7 @@ func RunStdioServer(cfg StdioServerConfig) error { slogHandler = slog.NewTextHandler(logOutput, &slog.HandlerOptions{Level: slog.LevelInfo}) } logger := slog.New(slogHandler) - logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownEnabled) + logger.Info("starting server", "version", cfg.Version, "host", cfg.Host, "dynamicToolsets", cfg.DynamicToolsets, "readOnly", cfg.ReadOnly, "lockdownEnabled", cfg.LockdownMode) stdLogger := log.New(logOutput, stdioServerLogPrefix, 0) stdioServer.SetErrorLogger(stdLogger) diff --git a/pkg/github/feature_flags.go b/pkg/github/feature_flags.go index 720dcd018..047042e44 100644 --- a/pkg/github/feature_flags.go +++ b/pkg/github/feature_flags.go @@ -2,5 +2,5 @@ package github // FeatureFlags defines runtime feature toggles that adjust tool behavior. type FeatureFlags struct { - LockdownEnabled bool + LockdownMode bool } diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 4a52586a8..920db283b 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -299,11 +299,11 @@ Options are: case "get": return GetIssue(ctx, client, gqlClient, owner, repo, issueNumber, flags) case "get_comments": - return GetIssueComments(ctx, client, owner, repo, issueNumber, pagination) + return GetIssueComments(ctx, client, owner, repo, issueNumber, pagination, flags) case "get_sub_issues": - return GetSubIssues(ctx, client, owner, repo, issueNumber, pagination) + return GetSubIssues(ctx, client, owner, repo, issueNumber, pagination, flags) case "get_labels": - return GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber) + return GetIssueLabels(ctx, gqlClient, owner, repo, issueNumber, flags) default: return mcp.NewToolResultError(fmt.Sprintf("unknown method: %s", method)), nil } @@ -325,10 +325,12 @@ func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Cl return mcp.NewToolResultError(fmt.Sprintf("failed to get issue: %s", string(body))), nil } - if flags.LockdownEnabled { - shouldFilter := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, *issue.Repository.Owner.Login, *issue.Repository.Name) - if shouldFilter { - return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil + if flags.LockdownMode { + if issue.User != nil && issue.Repository != nil && issue.Repository.Owner != nil && issue.Repository.Name != nil { + shouldRemoveContent := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, *issue.Repository.Owner.Login, *issue.Repository.Name) + if shouldRemoveContent { + return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil + } } } @@ -350,7 +352,7 @@ func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Cl return mcp.NewToolResultText(string(r)), nil } -func GetIssueComments(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetIssueComments(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams, _ FeatureFlags) (*mcp.CallToolResult, error) { opts := &github.IssueListCommentsOptions{ ListOptions: github.ListOptions{ Page: pagination.Page, @@ -380,7 +382,7 @@ func GetIssueComments(ctx context.Context, client *github.Client, owner string, return mcp.NewToolResultText(string(r)), nil } -func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams) (*mcp.CallToolResult, error) { +func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo string, issueNumber int, pagination PaginationParams, _ FeatureFlags) (*mcp.CallToolResult, error) { opts := &github.IssueListOptions{ ListOptions: github.ListOptions{ Page: pagination.Page, @@ -415,7 +417,7 @@ func GetSubIssues(ctx context.Context, client *github.Client, owner string, repo return mcp.NewToolResultText(string(r)), nil } -func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int) (*mcp.CallToolResult, error) { +func GetIssueLabels(ctx context.Context, client *githubv4.Client, owner string, repo string, issueNumber int, _ FeatureFlags) (*mcp.CallToolResult, error) { // Get current labels on the issue using GraphQL var query struct { Repository struct { diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 68761ec59..736fdc550 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -23,7 +23,7 @@ func Test_GetIssue(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) defaultGQLClient := githubv4.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(defaultGQLClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(defaultGQLClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -213,7 +213,7 @@ func Test_GetIssue(t *testing.T) { gqlClient = defaultGQLClient } - flags := stubFeatureFlags(map[string]bool{"lockdown": tc.lockdownEnabled}) + flags := stubFeatureFlags(map[string]bool{"lockdown-mode": tc.lockdownEnabled}) _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, flags) request := createMCPRequest(tc.requestArgs) @@ -1712,7 +1712,7 @@ func Test_GetIssueComments(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) gqlClient := githubv4.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -1818,7 +1818,7 @@ func Test_GetIssueComments(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) gqlClient := githubv4.NewClient(nil) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1855,7 +1855,7 @@ func Test_GetIssueLabels(t *testing.T) { // Verify tool definition mockGQClient := githubv4.NewClient(nil) mockClient := github.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(mockGQClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -1930,7 +1930,7 @@ func Test_GetIssueLabels(t *testing.T) { t.Run(tc.name, func(t *testing.T) { gqlClient := githubv4.NewClient(tc.mockedClient) client := github.NewClient(nil) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) request := createMCPRequest(tc.requestArgs) result, err := handler(context.Background(), request) @@ -2621,7 +2621,7 @@ func Test_GetSubIssues(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) gqlClient := githubv4.NewClient(nil) - tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + tool, _ := IssueRead(stubGetClientFn(mockClient), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "issue_read", tool.Name) @@ -2818,7 +2818,7 @@ func Test_GetSubIssues(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) gqlClient := githubv4.NewClient(nil) - _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown": false})) + _, handler := IssueRead(stubGetClientFn(client), stubGetGQLClientFn(gqlClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/pullrequests.go b/pkg/github/pullrequests.go index a9505161a..c4f0e219f 100644 --- a/pkg/github/pullrequests.go +++ b/pkg/github/pullrequests.go @@ -19,7 +19,7 @@ import ( ) // GetPullRequest creates a tool to get details of a specific pull request. -func PullRequestRead(getClient GetClientFn, t translations.TranslationHelperFunc) (mcp.Tool, server.ToolHandlerFunc) { +func PullRequestRead(getClient GetClientFn, t translations.TranslationHelperFunc, flags FeatureFlags) (mcp.Tool, server.ToolHandlerFunc) { return mcp.NewTool("pull_request_read", mcp.WithDescription(t("TOOL_PULL_REQUEST_READ_DESCRIPTION", "Get information on a specific pull request in GitHub repository.")), mcp.WithToolAnnotation(mcp.ToolAnnotation{ @@ -98,7 +98,7 @@ Possible options: case "get_reviews": return GetPullRequestReviews(ctx, client, owner, repo, pullNumber) case "get_comments": - return GetIssueComments(ctx, client, owner, repo, pullNumber, pagination) + return GetIssueComments(ctx, client, owner, repo, pullNumber, pagination, flags) default: return nil, fmt.Errorf("unknown method: %s", method) } diff --git a/pkg/github/pullrequests_test.go b/pkg/github/pullrequests_test.go index a66e2852a..c86f1b427 100644 --- a/pkg/github/pullrequests_test.go +++ b/pkg/github/pullrequests_test.go @@ -21,7 +21,7 @@ import ( func Test_GetPullRequest(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "pull_request_read", tool.Name) @@ -102,7 +102,7 @@ func Test_GetPullRequest(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1133,7 +1133,7 @@ func Test_SearchPullRequests(t *testing.T) { func Test_GetPullRequestFiles(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "pull_request_read", tool.Name) @@ -1236,7 +1236,7 @@ func Test_GetPullRequestFiles(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1277,7 +1277,7 @@ func Test_GetPullRequestFiles(t *testing.T) { func Test_GetPullRequestStatus(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "pull_request_read", tool.Name) @@ -1404,7 +1404,7 @@ func Test_GetPullRequestStatus(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1566,7 +1566,7 @@ func Test_UpdatePullRequestBranch(t *testing.T) { func Test_GetPullRequestComments(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "pull_request_read", tool.Name) @@ -1658,7 +1658,7 @@ func Test_GetPullRequestComments(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -1700,7 +1700,7 @@ func Test_GetPullRequestComments(t *testing.T) { func Test_GetPullRequestReviews(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "pull_request_read", tool.Name) @@ -1788,7 +1788,7 @@ func Test_GetPullRequestReviews(t *testing.T) { t.Run(tc.name, func(t *testing.T) { // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) @@ -2789,7 +2789,7 @@ func TestGetPullRequestDiff(t *testing.T) { // Verify tool definition once mockClient := github.NewClient(nil) - tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper) + tool, _ := PullRequestRead(stubGetClientFn(mockClient), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) require.NoError(t, toolsnaps.Test(tool.Name, tool)) assert.Equal(t, "pull_request_read", tool.Name) @@ -2847,7 +2847,7 @@ index 5d6e7b2..8a4f5c3 100644 // Setup client with mock client := github.NewClient(tc.mockedClient) - _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper) + _, handler := PullRequestRead(stubGetClientFn(client), translations.NullTranslationHelper, stubFeatureFlags(map[string]bool{"lockdown-mode": false})) // Create call request request := createMCPRequest(tc.requestArgs) diff --git a/pkg/github/server_test.go b/pkg/github/server_test.go index 934a7a8ce..307426832 100644 --- a/pkg/github/server_test.go +++ b/pkg/github/server_test.go @@ -40,7 +40,7 @@ func stubGetGQLClientFn(client *githubv4.Client) GetGQLClientFn { func stubFeatureFlags(enabledFlags map[string]bool) FeatureFlags { return FeatureFlags{ - LockdownEnabled: enabledFlags["lockdown"], + LockdownMode: enabledFlags["lockdown-mode"], } } diff --git a/pkg/github/tools.go b/pkg/github/tools.go index 85b726a98..e9995e0dc 100644 --- a/pkg/github/tools.go +++ b/pkg/github/tools.go @@ -216,7 +216,7 @@ func DefaultToolsetGroup(readOnly bool, getClient GetClientFn, getGQLClient GetG ) pullRequests := toolsets.NewToolset(ToolsetMetadataPullRequests.ID, ToolsetMetadataPullRequests.Description). AddReadTools( - toolsets.NewServerTool(PullRequestRead(getClient, t)), + toolsets.NewServerTool(PullRequestRead(getClient, t, flags)), toolsets.NewServerTool(ListPullRequests(getClient, t)), toolsets.NewServerTool(SearchPullRequests(getClient, t)), ). From 5a05b1a2035d9f24658b4da05098bfc73ecf920d Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Fri, 7 Nov 2025 12:47:52 +0100 Subject: [PATCH 3/6] Update pkg/lockdown/lockdown.go Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- pkg/lockdown/lockdown.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go index 843246717..99930da43 100644 --- a/pkg/lockdown/lockdown.go +++ b/pkg/lockdown/lockdown.go @@ -8,22 +8,22 @@ import ( "github.com/shurcooL/githubv4" ) -func ShouldRemoveContent(ctx context.Context, client *githubv4.Client, username, owner, repo string) bool { +func ShouldRemoveContent(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, error) { isPrivate, err := IsPrivateRepo(ctx, client, owner, repo) if err != nil { - return false + return false, err } // Do not filter content for private repositories if isPrivate { - return false + return false, nil } hasPushAccess, err := HasPushAccess(ctx, client, username, owner, repo) if err != nil { - return false + return false, err } - return !hasPushAccess + return !hasPushAccess, nil } func HasPushAccess(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, error) { From 77ef6d0a4b54960b643146db2eb616203ae43914 Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Fri, 7 Nov 2025 13:27:57 +0100 Subject: [PATCH 4/6] Merge two graphql queries into one --- pkg/github/issues.go | 5 ++++- pkg/github/issues_test.go | 36 +++++++++++++++++------------------ pkg/lockdown/lockdown.go | 40 ++++++--------------------------------- 3 files changed, 27 insertions(+), 54 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 920db283b..22e665053 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -327,7 +327,10 @@ func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Cl if flags.LockdownMode { if issue.User != nil && issue.Repository != nil && issue.Repository.Owner != nil && issue.Repository.Name != nil { - shouldRemoveContent := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, *issue.Repository.Owner.Login, *issue.Repository.Name) + shouldRemoveContent, err := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, *issue.Repository.Owner.Login, *issue.Repository.Name) + if err != nil { + return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil + } if shouldRemoveContent { return mcp.NewToolResultError("access to issue details is restricted by lockdown mode"), nil } diff --git a/pkg/github/issues_test.go b/pkg/github/issues_test.go index 736fdc550..0fe94e42c 100644 --- a/pkg/github/issues_test.go +++ b/pkg/github/issues_test.go @@ -108,16 +108,28 @@ func Test_GetIssue(t *testing.T) { githubv4mock.NewQueryMatcher( struct { Repository struct { - IsPrivate githubv4.Boolean + IsPrivate githubv4.Boolean + Collaborators struct { + Edges []struct { + Permission githubv4.String + Node struct { + Login githubv4.String + } + } + } `graphql:"collaborators(query: $username, first: 1)"` } `graphql:"repository(owner: $owner, name: $name)"` }{}, map[string]any{ - "owner": githubv4.String("owner"), - "name": githubv4.String("repo"), + "owner": githubv4.String("owner"), + "name": githubv4.String("repo"), + "username": githubv4.String("testuser"), }, githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ "isPrivate": true, + "collaborators": map[string]any{ + "edges": []any{}, + }, }, }), ), @@ -143,22 +155,7 @@ func Test_GetIssue(t *testing.T) { githubv4mock.NewQueryMatcher( struct { Repository struct { - IsPrivate githubv4.Boolean - } `graphql:"repository(owner: $owner, name: $name)"` - }{}, - map[string]any{ - "owner": githubv4.String("owner"), - "name": githubv4.String("repo"), - }, - githubv4mock.DataResponse(map[string]any{ - "repository": map[string]any{ - "isPrivate": false, - }, - }), - ), - githubv4mock.NewQueryMatcher( - struct { - Repository struct { + IsPrivate githubv4.Boolean Collaborators struct { Edges []struct { Permission githubv4.String @@ -176,6 +173,7 @@ func Test_GetIssue(t *testing.T) { }, githubv4mock.DataResponse(map[string]any{ "repository": map[string]any{ + "isPrivate": false, "collaborators": map[string]any{ "edges": []any{ map[string]any{ diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go index 99930da43..b5435aca5 100644 --- a/pkg/lockdown/lockdown.go +++ b/pkg/lockdown/lockdown.go @@ -9,7 +9,7 @@ import ( ) func ShouldRemoveContent(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, error) { - isPrivate, err := IsPrivateRepo(ctx, client, owner, repo) + isPrivate, hasPushAccess, err := repoAccessInfo(ctx, client, username, owner, repo) if err != nil { return false, err } @@ -18,21 +18,18 @@ func ShouldRemoveContent(ctx context.Context, client *githubv4.Client, username, if isPrivate { return false, nil } - hasPushAccess, err := HasPushAccess(ctx, client, username, owner, repo) - if err != nil { - return false, err - } return !hasPushAccess, nil } -func HasPushAccess(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, error) { +func repoAccessInfo(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, bool, error) { if client == nil { - return false, fmt.Errorf("nil GraphQL client") + return false, false, fmt.Errorf("nil GraphQL client") } var query struct { Repository struct { + IsPrivate githubv4.Boolean Collaborators struct { Edges []struct { Permission githubv4.String @@ -52,7 +49,7 @@ func HasPushAccess(ctx context.Context, client *githubv4.Client, username, owner err := client.Query(ctx, &query, variables) if err != nil { - return false, fmt.Errorf("failed to query user permissions: %w", err) + return false, false, fmt.Errorf("failed to query repository access info: %w", err) } // Check if the user has push access @@ -67,30 +64,5 @@ func HasPushAccess(ctx context.Context, client *githubv4.Client, username, owner } } - return hasPush, nil -} - -// IsPrivateRepo checks if a repository is private using GraphQL -func IsPrivateRepo(ctx context.Context, client *githubv4.Client, owner, repo string) (bool, error) { - if client == nil { - return false, fmt.Errorf("nil GraphQL client") - } - - var query struct { - Repository struct { - IsPrivate githubv4.Boolean - } `graphql:"repository(owner: $owner, name: $name)"` - } - - variables := map[string]interface{}{ - "owner": githubv4.String(owner), - "name": githubv4.String(repo), - } - - err := client.Query(ctx, &query, variables) - if err != nil { - return false, fmt.Errorf("failed to query repository visibility: %w", err) - } - - return bool(query.Repository.IsPrivate), nil + return bool(query.Repository.IsPrivate), hasPush, nil } From 34e7f143b09c3b2b82cf9b0fa7b7f5418994275d Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Fri, 7 Nov 2025 13:41:18 +0100 Subject: [PATCH 5/6] Don't use Issue.Repository --- pkg/github/issues.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/github/issues.go b/pkg/github/issues.go index 22e665053..83d59be16 100644 --- a/pkg/github/issues.go +++ b/pkg/github/issues.go @@ -326,8 +326,8 @@ func GetIssue(ctx context.Context, client *github.Client, gqlClient *githubv4.Cl } if flags.LockdownMode { - if issue.User != nil && issue.Repository != nil && issue.Repository.Owner != nil && issue.Repository.Name != nil { - shouldRemoveContent, err := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, *issue.Repository.Owner.Login, *issue.Repository.Name) + if issue.User != nil { + shouldRemoveContent, err := lockdown.ShouldRemoveContent(ctx, gqlClient, *issue.User.Login, owner, repo) if err != nil { return mcp.NewToolResultError(fmt.Sprintf("failed to check lockdown mode: %v", err)), nil } From dc97dc39a6c191febe0be4f54aa5fde2cc7f5220 Mon Sep 17 00:00:00 2001 From: JoannaaKL Date: Fri, 7 Nov 2025 13:42:37 +0100 Subject: [PATCH 6/6] Add function signature --- pkg/lockdown/lockdown.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pkg/lockdown/lockdown.go b/pkg/lockdown/lockdown.go index b5435aca5..5a474f73c 100644 --- a/pkg/lockdown/lockdown.go +++ b/pkg/lockdown/lockdown.go @@ -8,6 +8,9 @@ import ( "github.com/shurcooL/githubv4" ) +// ShouldRemoveContent determines if content should be removed based on +// lockdown mode rules. It checks if the repository is private and if the user +// has push access to the repository. func ShouldRemoveContent(ctx context.Context, client *githubv4.Client, username, owner, repo string) (bool, error) { isPrivate, hasPushAccess, err := repoAccessInfo(ctx, client, username, owner, repo) if err != nil {