diff --git a/README.md b/README.md index 1b926b132..fb582aa17 100644 --- a/README.md +++ b/README.md @@ -422,6 +422,24 @@ To specify toolsets you want available to the LLM, you can pass an allow-list in The environment variable `GITHUB_TOOLSETS` takes precedence over the command line argument if both are provided. +#### Strict Toolset Validation + +You can enable strict validation to fail fast when configuration includes unknown toolset names. + +1. **Using Command Line Argument**: + + ```bash + github-mcp-server --toolsets repos,issues --strict-toolsets + ``` + +2. **Using Environment Variable**: + + ```bash + GITHUB_TOOLSETS="repos,issues" GITHUB_STRICT_TOOLSETS=true ./github-mcp-server + ``` + +When enabled, startup returns an error if any configured toolset name is invalid. + #### Specifying Individual Tools You can also configure specific tools using the `--tools` flag. Tools can be used independently or combined with toolsets and dynamic toolsets discovery for fine-grained control. @@ -469,6 +487,7 @@ When using Docker, you can pass the toolsets as environment variables: docker run -i --rm \ -e GITHUB_PERSONAL_ACCESS_TOKEN= \ -e GITHUB_TOOLSETS="repos,issues,pull_requests,actions,code_security" \ + -e GITHUB_STRICT_TOOLSETS=true \ ghcr.io/github/github-mcp-server ``` diff --git a/cmd/github-mcp-server/main.go b/cmd/github-mcp-server/main.go index 05c2c6e0b..7a13e6349 100644 --- a/cmd/github-mcp-server/main.go +++ b/cmd/github-mcp-server/main.go @@ -79,22 +79,23 @@ var ( ttl := viper.GetDuration("repo-access-cache-ttl") stdioServerConfig := ghmcp.StdioServerConfig{ - Version: version, - Host: viper.GetString("host"), - Token: token, - EnabledToolsets: enabledToolsets, - EnabledTools: enabledTools, - EnabledFeatures: enabledFeatures, - DynamicToolsets: viper.GetBool("dynamic_toolsets"), - ReadOnly: viper.GetBool("read-only"), - ExportTranslations: viper.GetBool("export-translations"), - EnableCommandLogging: viper.GetBool("enable-command-logging"), - LogFilePath: viper.GetString("log-file"), - ContentWindowSize: viper.GetInt("content-window-size"), - LockdownMode: viper.GetBool("lockdown-mode"), - InsidersMode: viper.GetBool("insiders"), - ExcludeTools: excludeTools, - RepoAccessCacheTTL: &ttl, + Version: version, + Host: viper.GetString("host"), + Token: token, + EnabledToolsets: enabledToolsets, + StrictToolsetValidation: viper.GetBool("strict_toolsets"), + EnabledTools: enabledTools, + EnabledFeatures: enabledFeatures, + DynamicToolsets: viper.GetBool("dynamic_toolsets"), + ReadOnly: viper.GetBool("read-only"), + ExportTranslations: viper.GetBool("export-translations"), + EnableCommandLogging: viper.GetBool("enable-command-logging"), + LogFilePath: viper.GetString("log-file"), + ContentWindowSize: viper.GetInt("content-window-size"), + LockdownMode: viper.GetBool("lockdown-mode"), + InsidersMode: viper.GetBool("insiders"), + ExcludeTools: excludeTools, + RepoAccessCacheTTL: &ttl, } return ghmcp.RunStdioServer(stdioServerConfig) }, @@ -138,6 +139,7 @@ func init() { rootCmd.PersistentFlags().StringSlice("exclude-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("strict-toolsets", false, "Fail startup if configured toolsets include unknown names") rootCmd.PersistentFlags().Bool("read-only", false, "Restrict the server to read-only operations") rootCmd.PersistentFlags().String("log-file", "", "Path to log file") rootCmd.PersistentFlags().Bool("enable-command-logging", false, "When enabled, the server will log all command requests and responses to the log file") @@ -160,6 +162,7 @@ func init() { _ = viper.BindPFlag("exclude_tools", rootCmd.PersistentFlags().Lookup("exclude-tools")) _ = viper.BindPFlag("features", rootCmd.PersistentFlags().Lookup("features")) _ = viper.BindPFlag("dynamic_toolsets", rootCmd.PersistentFlags().Lookup("dynamic-toolsets")) + _ = viper.BindPFlag("strict_toolsets", rootCmd.PersistentFlags().Lookup("strict-toolsets")) _ = viper.BindPFlag("read-only", rootCmd.PersistentFlags().Lookup("read-only")) _ = viper.BindPFlag("log-file", rootCmd.PersistentFlags().Lookup("log-file")) _ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging")) diff --git a/docs/server-configuration.md b/docs/server-configuration.md index a334eb1a2..43f4d630a 100644 --- a/docs/server-configuration.md +++ b/docs/server-configuration.md @@ -12,6 +12,7 @@ We currently support the following ways in which the GitHub MCP Server can be co | Exclude Tools | `X-MCP-Exclude-Tools` header | `--exclude-tools` flag or `GITHUB_EXCLUDE_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 | +| Strict Toolset Validation | Not available | `--strict-toolsets` flag or `GITHUB_STRICT_TOOLSETS` env var | | Lockdown Mode | `X-MCP-Lockdown` header | `--lockdown-mode` flag or `GITHUB_LOCKDOWN_MODE` env var | | Insiders Mode | `X-MCP-Insiders` header or `/insiders` URL | `--insiders` flag or `GITHUB_INSIDERS` env var | | Scope Filtering | Always enabled | Always enabled | @@ -124,6 +125,58 @@ The examples below use VS Code configuration format to illustrate the concepts. --- +### Strict Toolset Validation (Local Only) + +**Best for:** Users who want startup to fail fast on invalid toolset names (for example, catching typos in CI or shared config files). + +When enabled, the local server validates configured toolsets at startup and returns an error if any toolset name is unknown. + + + + + + +
Local Server Only
+ +```json +{ + "type": "stdio", + "command": "go", + "args": [ + "run", + "./cmd/github-mcp-server", + "stdio", + "--toolsets=issues,pull_requests", + "--strict-toolsets" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}" + } +} +``` + +**Using environment variable instead of flag:** +```json +{ + "type": "stdio", + "command": "go", + "args": [ + "run", + "./cmd/github-mcp-server", + "stdio", + "--toolsets=issues,pull_requests" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}", + "GITHUB_STRICT_TOOLSETS": "true" + } +} +``` + +
+ +--- + ### Enabling Toolsets + Tools **Best for:** Users who want broad functionality from some areas, plus specific tools from others. @@ -336,6 +389,40 @@ Starts with only discovery tools (`enable_toolset`, `list_available_toolsets`, ` When both dynamic mode and specific tools are enabled in the server configuration, the server will start with the 3 dynamic tools + the specified tools. +### Strict Toolset Validation + +**Best for:** Locked-down environments where toolset allow-lists must fail closed. + +By default, unknown toolset names are ignored and logged as warnings so existing configurations remain backward compatible. If you want startup to fail when a configured toolset name is unknown, enable strict validation. + + + + + + +
Local Server Only
+ +```json +{ + "type": "stdio", + "command": "go", + "args": [ + "run", + "./cmd/github-mcp-server", + "stdio", + "--toolsets=repos,issues,typo", + "--strict-toolsets" + ], + "env": { + "GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}" + } +} +``` + +
+ +Use this when a typo in a toolset name should be treated as a startup error instead of silently falling back to a narrower or unintended capability set. + --- ### Lockdown Mode diff --git a/internal/ghmcp/server.go b/internal/ghmcp/server.go index 5c4e7f6f1..231e0e190 100644 --- a/internal/ghmcp/server.go +++ b/internal/ghmcp/server.go @@ -134,6 +134,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se WithDeprecatedAliases(github.DeprecatedToolAliases). WithReadOnly(cfg.ReadOnly). WithToolsets(github.ResolvedEnabledToolsets(cfg.DynamicToolsets, cfg.EnabledToolsets, cfg.EnabledTools)). + WithStrictToolsetValidation(cfg.StrictToolsetValidation). WithTools(github.CleanTools(cfg.EnabledTools)). WithExcludeTools(cfg.ExcludeTools). WithServerInstructions(). @@ -181,6 +182,9 @@ type StdioServerConfig struct { // See: https://github.com/github/github-mcp-server?tab=readme-ov-file#tool-configuration EnabledToolsets []string + // StrictToolsetValidation fails startup when enabled toolsets include unknown names. + StrictToolsetValidation bool + // EnabledTools is a list of specific tools to enable (additive to toolsets) // When specified, these tools are registered in addition to any specified toolset tools EnabledTools []string @@ -265,22 +269,23 @@ func RunStdioServer(cfg StdioServerConfig) error { } ghServer, err := NewStdioMCPServer(ctx, github.MCPServerConfig{ - Version: cfg.Version, - Host: cfg.Host, - Token: cfg.Token, - EnabledToolsets: cfg.EnabledToolsets, - EnabledTools: cfg.EnabledTools, - EnabledFeatures: cfg.EnabledFeatures, - DynamicToolsets: cfg.DynamicToolsets, - ReadOnly: cfg.ReadOnly, - Translator: t, - ContentWindowSize: cfg.ContentWindowSize, - LockdownMode: cfg.LockdownMode, - InsidersMode: cfg.InsidersMode, - ExcludeTools: cfg.ExcludeTools, - Logger: logger, - RepoAccessTTL: cfg.RepoAccessCacheTTL, - TokenScopes: tokenScopes, + Version: cfg.Version, + Host: cfg.Host, + Token: cfg.Token, + EnabledToolsets: cfg.EnabledToolsets, + StrictToolsetValidation: cfg.StrictToolsetValidation, + EnabledTools: cfg.EnabledTools, + EnabledFeatures: cfg.EnabledFeatures, + DynamicToolsets: cfg.DynamicToolsets, + ReadOnly: cfg.ReadOnly, + Translator: t, + ContentWindowSize: cfg.ContentWindowSize, + LockdownMode: cfg.LockdownMode, + InsidersMode: cfg.InsidersMode, + ExcludeTools: cfg.ExcludeTools, + Logger: logger, + RepoAccessTTL: cfg.RepoAccessCacheTTL, + TokenScopes: tokenScopes, }) if err != nil { return fmt.Errorf("failed to create MCP server: %w", err) diff --git a/internal/ghmcp/server_test.go b/internal/ghmcp/server_test.go index 6f0e3ac3f..9e10a414f 100644 --- a/internal/ghmcp/server_test.go +++ b/internal/ghmcp/server_test.go @@ -1 +1,42 @@ package ghmcp + +import ( + "context" + "io" + "log/slog" + "testing" + + "github.com/github/github-mcp-server/pkg/github" + "github.com/github/github-mcp-server/pkg/inventory" + "github.com/github/github-mcp-server/pkg/translations" + "github.com/stretchr/testify/require" +) + +func TestNewStdioMCPServer_StrictToolsetValidation(t *testing.T) { + t.Parallel() + + _, err := NewStdioMCPServer(context.Background(), testMCPServerConfig([]string{"repos", "typo"}, true)) + require.Error(t, err) + require.ErrorIs(t, err, inventory.ErrUnknownToolsets) + require.Contains(t, err.Error(), "typo") +} + +func TestNewStdioMCPServer_AllowsUnknownToolsetsWhenNotStrict(t *testing.T) { + t.Parallel() + + server, err := NewStdioMCPServer(context.Background(), testMCPServerConfig([]string{"repos", "typo"}, false)) + require.NoError(t, err) + require.NotNil(t, server) +} + +func testMCPServerConfig(toolsets []string, strict bool) github.MCPServerConfig { + return github.MCPServerConfig{ + Version: "test", + Token: "test-token", + EnabledToolsets: toolsets, + StrictToolsetValidation: strict, + Translator: translations.NullTranslationHelper, + ContentWindowSize: 5000, + Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), + } +} diff --git a/pkg/github/server.go b/pkg/github/server.go index 06c12575d..d60dc2d4b 100644 --- a/pkg/github/server.go +++ b/pkg/github/server.go @@ -30,6 +30,9 @@ type MCPServerConfig struct { // See: https://github.com/github/github-mcp-server?tab=readme-ov-file#tool-configuration EnabledToolsets []string + // StrictToolsetValidation fails startup when EnabledToolsets contains unknown names. + StrictToolsetValidation bool + // EnabledTools is a list of specific tools to enable (additive to toolsets) // When specified, these tools are registered in addition to any specified toolset tools EnabledTools []string diff --git a/pkg/inventory/builder.go b/pkg/inventory/builder.go index d492e69b5..79ec9651a 100644 --- a/pkg/inventory/builder.go +++ b/pkg/inventory/builder.go @@ -12,6 +12,8 @@ import ( var ( // ErrUnknownTools is returned when tools specified via WithTools() are not recognized. ErrUnknownTools = errors.New("unknown tools specified in WithTools") + // ErrUnknownToolsets is returned when toolsets specified via WithToolsets() are not recognized. + ErrUnknownToolsets = errors.New("unknown toolsets specified in WithToolsets") ) // ToolFilter is a function that determines if a tool should be included. @@ -49,6 +51,7 @@ type Builder struct { filters []ToolFilter // filters to apply to all tools generateInstructions bool insidersMode bool + strictToolsets bool } // NewBuilder creates a new Builder. @@ -111,6 +114,13 @@ func (b *Builder) WithToolsets(toolsetIDs []string) *Builder { return b } +// WithStrictToolsetValidation controls whether unknown toolset IDs should fail Build(). +// When disabled, unknown toolsets are recorded on the inventory for warning-only behavior. +func (b *Builder) WithStrictToolsetValidation(strict bool) *Builder { + b.strictToolsets = strict + return b +} + // WithTools specifies additional tools that bypass toolset filtering. // These tools are additive - they will be included even if their toolset is not enabled. // Read-only filtering still applies to these tools. @@ -222,6 +232,9 @@ func (b *Builder) Build() (*Inventory, error) { // Process toolsets and pre-compute metadata in a single pass r.enabledToolsets, r.unrecognizedToolsets, r.toolsetIDs, r.toolsetIDSet, r.defaultToolsetIDs, r.toolsetDescriptions = b.processToolsets() + if b.strictToolsets && len(r.unrecognizedToolsets) > 0 { + return nil, fmt.Errorf("%w: %s", ErrUnknownToolsets, strings.Join(r.unrecognizedToolsets, ", ")) + } // Build set of valid tool names for validation validToolNames := make(map[string]bool, len(tools)) diff --git a/pkg/inventory/registry_test.go b/pkg/inventory/registry_test.go index 207e65dba..dff7eb9fe 100644 --- a/pkg/inventory/registry_test.go +++ b/pkg/inventory/registry_test.go @@ -280,6 +280,34 @@ func TestUnrecognizedToolsets(t *testing.T) { } } +func TestBuildErrorsOnUnrecognizedToolsetsWhenStrict(t *testing.T) { + tools := []ServerTool{ + mockTool("tool1", "toolset1", true), + } + + _, err := NewBuilder(). + SetTools(tools). + WithToolsets([]string{"toolset1", "typo"}). + WithStrictToolsetValidation(true). + Build() + + require.Error(t, err, "expected error for unrecognized toolset in strict mode") + require.ErrorIs(t, err, ErrUnknownToolsets) + require.Contains(t, err.Error(), "typo") +} + +func TestBuildAllowsUnrecognizedToolsetsWhenNotStrict(t *testing.T) { + tools := []ServerTool{ + mockTool("tool1", "toolset1", true), + } + + reg := mustBuild(t, NewBuilder(). + SetTools(tools). + WithToolsets([]string{"toolset1", "typo"})) + + require.Equal(t, []string{"typo"}, reg.UnrecognizedToolsets()) +} + func TestBuildErrorsOnUnrecognizedTools(t *testing.T) { tools := []ServerTool{ mockTool("tool1", "toolset1", true),