diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index b8002d45..47da21e6 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -61,6 +61,14 @@ var ( } } + // Parse disallowed tools (similar to tools) + var disallowedTools []string + if viper.IsSet("disallowed_tools") { + if err := viper.UnmarshalKey("disallowed_tools", &disallowedTools); err != nil { + return fmt.Errorf("failed to unmarshal disallowed-tools: %w", err) + } + } + // Parse enabled features (similar to toolsets) var enabledFeatures []string if viper.IsSet("features") { @@ -85,6 +93,7 @@ var ( ContentWindowSize: viper.GetInt("content-window-size"), LockdownMode: viper.GetBool("lockdown-mode"), InsidersMode: viper.GetBool("insiders"), + DisallowedTools: disallowedTools, RepoAccessCacheTTL: &ttl, } return ghmcp.RunStdioServer(stdioServerConfig) @@ -126,6 +135,7 @@ func init() { // Add global flags that will be shared by all commands rootCmd.PersistentFlags().StringSlice("toolsets", nil, github.GenerateToolsetsHelp()) rootCmd.PersistentFlags().StringSlice("tools", nil, "Comma-separated list of specific tools to enable") + rootCmd.PersistentFlags().StringSlice("disallowed-tools", nil, "Comma-separated list of tool names to disable regardless of other settings") rootCmd.PersistentFlags().StringSlice("features", nil, "Comma-separated list of feature flags to enable") rootCmd.PersistentFlags().Bool("dynamic-toolsets", false, "Enable dynamic toolsets") rootCmd.PersistentFlags().Bool("read-only", false, "Restrict the server to read-only operations") @@ -147,6 +157,7 @@ func init() { // Bind flag to viper _ = viper.BindPFlag("toolsets", rootCmd.PersistentFlags().Lookup("toolsets")) _ = viper.BindPFlag("tools", rootCmd.PersistentFlags().Lookup("tools")) + _ = viper.BindPFlag("disallowed_tools", rootCmd.PersistentFlags().Lookup("disallowed-tools")) _ = viper.BindPFlag("features", rootCmd.PersistentFlags().Lookup("features")) _ = viper.BindPFlag("dynamic_toolsets", rootCmd.PersistentFlags().Lookup("dynamic-toolsets")) _ = viper.BindPFlag("read-only", rootCmd.PersistentFlags().Lookup("read-only")) diff --git a/docs/server-configuration.md b/docs/server-configuration.md index 46ec3bc6..2c5f875b 100644 --- a/docs/server-configuration.md +++ b/docs/server-configuration.md @@ -9,6 +9,7 @@ We currently support the following ways in which the GitHub MCP Server can be co |---------------|---------------|--------------| | Toolsets | `X-MCP-Toolsets` header or `/x/{toolset}` URL | `--toolsets` flag or `GITHUB_TOOLSETS` env var | | Individual Tools | `X-MCP-Tools` header | `--tools` flag or `GITHUB_TOOLS` env var | +| Disallowed Tools | `X-MCP-Disallowed-Tools` header | `--disallowed-tools` flag or `GITHUB_DISALLOWED_TOOLS` env var | | Read-Only Mode | `X-MCP-Readonly` header or `/readonly` URL | `--read-only` flag or `GITHUB_READ_ONLY` env var | | Dynamic Mode | Not available | `--dynamic-toolsets` flag or `GITHUB_DYNAMIC_TOOLSETS` env var | | Lockdown Mode | `X-MCP-Lockdown` header | `--lockdown-mode` flag or `GITHUB_LOCKDOWN_MODE` env var | @@ -20,10 +21,12 @@ We currently support the following ways in which the GitHub MCP Server can be co ## How Configuration Works -All configuration options are **composable**: you can combine toolsets, individual tools, dynamic discovery, read-only mode and lockdown mode in any way that suits your workflow. +All configuration options are **composable**: you can combine toolsets, individual tools, disallowed tools, dynamic discovery, read-only mode and lockdown mode in any way that suits your workflow. Note: **read-only** mode acts as a strict security filter that takes precedence over any other configuration, by disabling write tools even when explicitly requested. +Note: **disallowed tools** takes precedence over toolsets and individual tools — listed tools are always excluded, even if their toolset is enabled or they are explicitly added via `--tools` / `X-MCP-Tools`. + --- ## Configuration Examples @@ -170,6 +173,56 @@ Enable entire toolsets, then add individual tools from toolsets you don't want f --- +### Disallowing Specific Tools + +**Best for:** Users who want to enable a broad toolset but need to exclude specific tools for security, compliance, or to prevent undesired behavior. + +Listed tools are removed regardless of any other configuration — even if their toolset is enabled or they are individually added. + + + + + + + +
Remote ServerLocal Server
+ +```json +{ + "type": "http", + "url": "https://api.githubcopilot.com/mcp/", + "headers": { + "X-MCP-Toolsets": "pull_requests", + "X-MCP-Disallowed-Tools": "create_pull_request,merge_pull_request" + } +} +``` + + + +```json +{ + "type": "stdio", + "command": "go", + "args": [ + "run", + "./cmd/github-mcp-server", + "stdio", + "--toolsets=pull_requests", + "--disallowed-tools=create_pull_request,merge_pull_request" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}" + } +} +``` + +
+ +**Result:** All pull request tools except `create_pull_request` and `merge_pull_request` — the user gets read and review tools only. + +--- + ### Read-Only Mode **Best for:** Security conscious users who want to ensure the server won't allow operations that modify issues, pull requests, repositories etc. diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 6f5ba4e4..beaa898e 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -135,6 +135,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se WithReadOnly(cfg.ReadOnly). WithToolsets(github.ResolvedEnabledToolsets(cfg.DynamicToolsets, cfg.EnabledToolsets, cfg.EnabledTools)). WithTools(github.CleanTools(cfg.EnabledTools)). + WithDisallowedTools(cfg.DisallowedTools). WithServerInstructions(). WithFeatureChecker(featureChecker). WithInsidersMode(cfg.InsidersMode) @@ -214,6 +215,11 @@ type StdioServerConfig struct { // InsidersMode indicates if we should enable experimental features InsidersMode bool + // DisallowedTools is a list of tool names to disable regardless of other settings. + // These tools will be excluded even if their toolset is enabled or they are + // explicitly listed in EnabledTools. + DisallowedTools []string + // RepoAccessCacheTTL overrides the default TTL for repository access cache entries. RepoAccessCacheTTL *time.Duration } @@ -271,6 +277,7 @@ func RunStdioServer(cfg StdioServerConfig) error { ContentWindowSize: cfg.ContentWindowSize, LockdownMode: cfg.LockdownMode, InsidersMode: cfg.InsidersMode, + DisallowedTools: cfg.DisallowedTools, Logger: logger, RepoAccessTTL: cfg.RepoAccessCacheTTL, TokenScopes: tokenScopes, diff --git a/pkg/github/server.go b/pkg/github/server.go index 14741939..5ff71fc0 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -62,6 +62,11 @@ type MCPServerConfig struct { // RepoAccessTTL overrides the default TTL for repository access cache entries. RepoAccessTTL *time.Duration + // DisallowedTools is a list of tool names that should be disabled regardless of + // other configuration. These tools will be excluded even if their toolset is enabled + // or they are explicitly listed in EnabledTools. + DisallowedTools []string + // TokenScopes contains the OAuth scopes available to the token. // When non-nil, tools requiring scopes not in this list will be hidden. // This is used for PAT scope filtering where we can't issue scope challenges. diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index 6d2f080a..90449888 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -141,6 +141,19 @@ func (b *Builder) WithFilter(filter ToolFilter) *Builder { return b } +// WithDisallowedTools specifies tools that should be disabled regardless of other settings. +// These tools will be excluded even if their toolset is enabled or they are in the +// additional tools list. This takes precedence over all other tool enablement settings. +// Input is cleaned (trimmed, deduplicated) before applying. +// Returns self for chaining. +func (b *Builder) WithDisallowedTools(toolNames []string) *Builder { + cleaned := cleanTools(toolNames) + if len(cleaned) > 0 { + b.filters = append(b.filters, CreateDisallowedToolsFilter(cleaned)) + } + return b +} + // WithInsidersMode enables or disables insiders mode features. // When insiders mode is disabled (default), UI metadata is removed from tools // so clients won't attempt to load UI resources. @@ -150,6 +163,20 @@ func (b *Builder) WithInsidersMode(enabled bool) *Builder { return b } +// CreateDisallowedToolsFilter creates a ToolFilter that excludes tools by name. +// Any tool whose name appears in the disallowed list will be filtered out. +// The input slice should already be cleaned (trimmed, deduplicated). +func CreateDisallowedToolsFilter(disallowed []string) ToolFilter { + set := make(map[string]struct{}, len(disallowed)) + for _, name := range disallowed { + set[name] = struct{}{} + } + return func(_ context.Context, tool *ServerTool) (bool, error) { + _, blocked := set[tool.Tool.Name] + return !blocked, nil + } +} + // cleanTools trims whitespace and removes duplicates from tool names. // Empty strings after trimming are excluded. func cleanTools(tools []string) []string { diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index fc380ab3..cb0f038d 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -2129,3 +2129,151 @@ func TestWithInsidersMode_DoesNotMutateOriginalTools(t *testing.T) { require.Equal(t, "data", tools[0].Tool.Meta["ui"], "original tool should not be mutated") require.Equal(t, "kept", tools[0].Tool.Meta["description"], "original tool should not be mutated") } + +func TestWithDisallowedTools(t *testing.T) { + tools := []ServerTool{ + mockTool("tool1", "toolset1", true), + mockTool("tool2", "toolset1", true), + mockTool("tool3", "toolset2", true), + } + + tests := []struct { + name string + disallowed []string + toolsets []string + expectedNames []string + unexpectedNames []string + }{ + { + name: "single tool disallowed", + disallowed: []string{"tool2"}, + toolsets: []string{"all"}, + expectedNames: []string{"tool1", "tool3"}, + unexpectedNames: []string{"tool2"}, + }, + { + name: "multiple tools disallowed", + disallowed: []string{"tool1", "tool3"}, + toolsets: []string{"all"}, + expectedNames: []string{"tool2"}, + unexpectedNames: []string{"tool1", "tool3"}, + }, + { + name: "empty disallowed list is a no-op", + disallowed: []string{}, + toolsets: []string{"all"}, + expectedNames: []string{"tool1", "tool2", "tool3"}, + unexpectedNames: nil, + }, + { + name: "nil disallowed list is a no-op", + disallowed: nil, + toolsets: []string{"all"}, + expectedNames: []string{"tool1", "tool2", "tool3"}, + unexpectedNames: nil, + }, + { + name: "disallowing non-existent tool is a no-op", + disallowed: []string{"nonexistent"}, + toolsets: []string{"all"}, + expectedNames: []string{"tool1", "tool2", "tool3"}, + unexpectedNames: nil, + }, + { + name: "disallow all tools", + disallowed: []string{"tool1", "tool2", "tool3"}, + toolsets: []string{"all"}, + expectedNames: nil, + unexpectedNames: []string{"tool1", "tool2", "tool3"}, + }, + { + name: "whitespace is trimmed", + disallowed: []string{" tool2 ", " tool3 "}, + toolsets: []string{"all"}, + expectedNames: []string{"tool1"}, + unexpectedNames: []string{"tool2", "tool3"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reg := mustBuild(t, NewBuilder(). + SetTools(tools). + WithToolsets(tt.toolsets). + WithDisallowedTools(tt.disallowed)) + + available := reg.AvailableTools(context.Background()) + names := make(map[string]bool) + for _, tool := range available { + names[tool.Tool.Name] = true + } + + for _, expected := range tt.expectedNames { + require.True(t, names[expected], "tool %q should be available", expected) + } + for _, unexpected := range tt.unexpectedNames { + require.False(t, names[unexpected], "tool %q should be disallowed", unexpected) + } + }) + } +} + +func TestWithDisallowedTools_OverridesAdditionalTools(t *testing.T) { + tools := []ServerTool{ + mockTool("tool1", "toolset1", true), + mockTool("tool2", "toolset1", true), + mockTool("tool3", "toolset2", true), + } + + // tool3 is explicitly enabled via WithTools, but also disallowed + // disallowed should win because builder filters run before additional tools check + reg := mustBuild(t, NewBuilder(). + SetTools(tools). + WithToolsets([]string{"toolset1"}). + WithTools([]string{"tool3"}). + WithDisallowedTools([]string{"tool3"})) + + available := reg.AvailableTools(context.Background()) + names := make(map[string]bool) + for _, tool := range available { + names[tool.Tool.Name] = true + } + + require.True(t, names["tool1"], "tool1 should be available") + require.True(t, names["tool2"], "tool2 should be available") + require.False(t, names["tool3"], "tool3 should be disallowed even though explicitly added via WithTools") +} + +func TestWithDisallowedTools_CombinesWithReadOnly(t *testing.T) { + tools := []ServerTool{ + mockTool("read_tool", "toolset1", true), + mockTool("write_tool", "toolset1", false), + mockTool("another_read", "toolset1", true), + } + + // read-only excludes write_tool, disallowed excludes read_tool + reg := mustBuild(t, NewBuilder(). + SetTools(tools). + WithToolsets([]string{"all"}). + WithReadOnly(true). + WithDisallowedTools([]string{"read_tool"})) + + available := reg.AvailableTools(context.Background()) + require.Len(t, available, 1) + require.Equal(t, "another_read", available[0].Tool.Name) +} + +func TestCreateDisallowedToolsFilter(t *testing.T) { + filter := CreateDisallowedToolsFilter([]string{"blocked_tool"}) + + blockedTool := mockTool("blocked_tool", "toolset1", true) + allowedTool := mockTool("allowed_tool", "toolset1", true) + + allowed, err := filter(context.Background(), &blockedTool) + require.NoError(t, err) + require.False(t, allowed, "blocked_tool should be excluded") + + allowed, err = filter(context.Background(), &allowedTool) + require.NoError(t, err) + require.True(t, allowed, "allowed_tool should be included") +}