Skip to content

Commit deeabef

Browse files
committed
Fail closed on unknown toolsets in strict mode
1 parent 486e9fe commit deeabef

File tree

7 files changed

+159
-32
lines changed

7 files changed

+159
-32
lines changed

cmd/github-mcp-server/main.go

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -79,22 +79,23 @@ var (
7979

8080
ttl := viper.GetDuration("repo-access-cache-ttl")
8181
stdioServerConfig := ghmcp.StdioServerConfig{
82-
Version: version,
83-
Host: viper.GetString("host"),
84-
Token: token,
85-
EnabledToolsets: enabledToolsets,
86-
EnabledTools: enabledTools,
87-
EnabledFeatures: enabledFeatures,
88-
DynamicToolsets: viper.GetBool("dynamic_toolsets"),
89-
ReadOnly: viper.GetBool("read-only"),
90-
ExportTranslations: viper.GetBool("export-translations"),
91-
EnableCommandLogging: viper.GetBool("enable-command-logging"),
92-
LogFilePath: viper.GetString("log-file"),
93-
ContentWindowSize: viper.GetInt("content-window-size"),
94-
LockdownMode: viper.GetBool("lockdown-mode"),
95-
InsidersMode: viper.GetBool("insiders"),
96-
ExcludeTools: excludeTools,
97-
RepoAccessCacheTTL: &ttl,
82+
Version: version,
83+
Host: viper.GetString("host"),
84+
Token: token,
85+
EnabledToolsets: enabledToolsets,
86+
StrictToolsetValidation: viper.GetBool("strict_toolsets"),
87+
EnabledTools: enabledTools,
88+
EnabledFeatures: enabledFeatures,
89+
DynamicToolsets: viper.GetBool("dynamic_toolsets"),
90+
ReadOnly: viper.GetBool("read-only"),
91+
ExportTranslations: viper.GetBool("export-translations"),
92+
EnableCommandLogging: viper.GetBool("enable-command-logging"),
93+
LogFilePath: viper.GetString("log-file"),
94+
ContentWindowSize: viper.GetInt("content-window-size"),
95+
LockdownMode: viper.GetBool("lockdown-mode"),
96+
InsidersMode: viper.GetBool("insiders"),
97+
ExcludeTools: excludeTools,
98+
RepoAccessCacheTTL: &ttl,
9899
}
99100
return ghmcp.RunStdioServer(stdioServerConfig)
100101
},
@@ -138,6 +139,7 @@ func init() {
138139
rootCmd.PersistentFlags().StringSlice("exclude-tools", nil, "Comma-separated list of tool names to disable regardless of other settings")
139140
rootCmd.PersistentFlags().StringSlice("features", nil, "Comma-separated list of feature flags to enable")
140141
rootCmd.PersistentFlags().Bool("dynamic-toolsets", false, "Enable dynamic toolsets")
142+
rootCmd.PersistentFlags().Bool("strict-toolsets", false, "Fail startup if configured toolsets include unknown names")
141143
rootCmd.PersistentFlags().Bool("read-only", false, "Restrict the server to read-only operations")
142144
rootCmd.PersistentFlags().String("log-file", "", "Path to log file")
143145
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() {
160162
_ = viper.BindPFlag("exclude_tools", rootCmd.PersistentFlags().Lookup("exclude-tools"))
161163
_ = viper.BindPFlag("features", rootCmd.PersistentFlags().Lookup("features"))
162164
_ = viper.BindPFlag("dynamic_toolsets", rootCmd.PersistentFlags().Lookup("dynamic-toolsets"))
165+
_ = viper.BindPFlag("strict_toolsets", rootCmd.PersistentFlags().Lookup("strict-toolsets"))
163166
_ = viper.BindPFlag("read-only", rootCmd.PersistentFlags().Lookup("read-only"))
164167
_ = viper.BindPFlag("log-file", rootCmd.PersistentFlags().Lookup("log-file"))
165168
_ = viper.BindPFlag("enable-command-logging", rootCmd.PersistentFlags().Lookup("enable-command-logging"))

docs/server-configuration.md

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,40 @@ Starts with only discovery tools (`enable_toolset`, `list_available_toolsets`, `
336336

337337
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.
338338

339+
### Strict Toolset Validation
340+
341+
**Best for:** Locked-down environments where toolset allow-lists must fail closed.
342+
343+
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.
344+
345+
<table>
346+
<tr><th>Local Server Only</th></tr>
347+
<tr valign="top">
348+
<td>
349+
350+
```json
351+
{
352+
"type": "stdio",
353+
"command": "go",
354+
"args": [
355+
"run",
356+
"./cmd/github-mcp-server",
357+
"stdio",
358+
"--toolsets=repos,issues,typo",
359+
"--strict-toolsets"
360+
],
361+
"env": {
362+
"GITHUB_PERSONAL_ACCESS_TOKEN": "${input:github_token}"
363+
}
364+
}
365+
```
366+
367+
</td>
368+
</tr>
369+
</table>
370+
371+
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.
372+
339373
---
340374

341375
### Lockdown Mode

internal/ghmcp/server.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ func NewStdioMCPServer(ctx context.Context, cfg github.MCPServerConfig) (*mcp.Se
134134
WithDeprecatedAliases(github.DeprecatedToolAliases).
135135
WithReadOnly(cfg.ReadOnly).
136136
WithToolsets(github.ResolvedEnabledToolsets(cfg.DynamicToolsets, cfg.EnabledToolsets, cfg.EnabledTools)).
137+
WithStrictToolsetValidation(cfg.StrictToolsetValidation).
137138
WithTools(github.CleanTools(cfg.EnabledTools)).
138139
WithExcludeTools(cfg.ExcludeTools).
139140
WithServerInstructions().
@@ -181,6 +182,9 @@ type StdioServerConfig struct {
181182
// See: https://github.com/github/github-mcp-server?tab=readme-ov-file#tool-configuration
182183
EnabledToolsets []string
183184

185+
// StrictToolsetValidation fails startup when enabled toolsets include unknown names.
186+
StrictToolsetValidation bool
187+
184188
// EnabledTools is a list of specific tools to enable (additive to toolsets)
185189
// When specified, these tools are registered in addition to any specified toolset tools
186190
EnabledTools []string
@@ -265,22 +269,23 @@ func RunStdioServer(cfg StdioServerConfig) error {
265269
}
266270

267271
ghServer, err := NewStdioMCPServer(ctx, github.MCPServerConfig{
268-
Version: cfg.Version,
269-
Host: cfg.Host,
270-
Token: cfg.Token,
271-
EnabledToolsets: cfg.EnabledToolsets,
272-
EnabledTools: cfg.EnabledTools,
273-
EnabledFeatures: cfg.EnabledFeatures,
274-
DynamicToolsets: cfg.DynamicToolsets,
275-
ReadOnly: cfg.ReadOnly,
276-
Translator: t,
277-
ContentWindowSize: cfg.ContentWindowSize,
278-
LockdownMode: cfg.LockdownMode,
279-
InsidersMode: cfg.InsidersMode,
280-
ExcludeTools: cfg.ExcludeTools,
281-
Logger: logger,
282-
RepoAccessTTL: cfg.RepoAccessCacheTTL,
283-
TokenScopes: tokenScopes,
272+
Version: cfg.Version,
273+
Host: cfg.Host,
274+
Token: cfg.Token,
275+
EnabledToolsets: cfg.EnabledToolsets,
276+
StrictToolsetValidation: cfg.StrictToolsetValidation,
277+
EnabledTools: cfg.EnabledTools,
278+
EnabledFeatures: cfg.EnabledFeatures,
279+
DynamicToolsets: cfg.DynamicToolsets,
280+
ReadOnly: cfg.ReadOnly,
281+
Translator: t,
282+
ContentWindowSize: cfg.ContentWindowSize,
283+
LockdownMode: cfg.LockdownMode,
284+
InsidersMode: cfg.InsidersMode,
285+
ExcludeTools: cfg.ExcludeTools,
286+
Logger: logger,
287+
RepoAccessTTL: cfg.RepoAccessCacheTTL,
288+
TokenScopes: tokenScopes,
284289
})
285290
if err != nil {
286291
return fmt.Errorf("failed to create MCP server: %w", err)

internal/ghmcp/server_test.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,42 @@
11
package ghmcp
2+
3+
import (
4+
"context"
5+
"io"
6+
"log/slog"
7+
"testing"
8+
9+
"github.com/github/github-mcp-server/pkg/github"
10+
"github.com/github/github-mcp-server/pkg/inventory"
11+
"github.com/github/github-mcp-server/pkg/translations"
12+
"github.com/stretchr/testify/require"
13+
)
14+
15+
func TestNewStdioMCPServer_StrictToolsetValidation(t *testing.T) {
16+
t.Parallel()
17+
18+
_, err := NewStdioMCPServer(context.Background(), testMCPServerConfig([]string{"repos", "typo"}, true))
19+
require.Error(t, err)
20+
require.ErrorIs(t, err, inventory.ErrUnknownToolsets)
21+
require.Contains(t, err.Error(), "typo")
22+
}
23+
24+
func TestNewStdioMCPServer_AllowsUnknownToolsetsWhenNotStrict(t *testing.T) {
25+
t.Parallel()
26+
27+
server, err := NewStdioMCPServer(context.Background(), testMCPServerConfig([]string{"repos", "typo"}, false))
28+
require.NoError(t, err)
29+
require.NotNil(t, server)
30+
}
31+
32+
func testMCPServerConfig(toolsets []string, strict bool) github.MCPServerConfig {
33+
return github.MCPServerConfig{
34+
Version: "test",
35+
Token: "test-token",
36+
EnabledToolsets: toolsets,
37+
StrictToolsetValidation: strict,
38+
Translator: translations.NullTranslationHelper,
39+
ContentWindowSize: 5000,
40+
Logger: slog.New(slog.NewTextHandler(io.Discard, nil)),
41+
}
42+
}

pkg/github/server.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,9 @@ type MCPServerConfig struct {
3030
// See: https://github.com/github/github-mcp-server?tab=readme-ov-file#tool-configuration
3131
EnabledToolsets []string
3232

33+
// StrictToolsetValidation fails startup when EnabledToolsets contains unknown names.
34+
StrictToolsetValidation bool
35+
3336
// EnabledTools is a list of specific tools to enable (additive to toolsets)
3437
// When specified, these tools are registered in addition to any specified toolset tools
3538
EnabledTools []string

pkg/inventory/builder.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ import (
1212
var (
1313
// ErrUnknownTools is returned when tools specified via WithTools() are not recognized.
1414
ErrUnknownTools = errors.New("unknown tools specified in WithTools")
15+
// ErrUnknownToolsets is returned when toolsets specified via WithToolsets() are not recognized.
16+
ErrUnknownToolsets = errors.New("unknown toolsets specified in WithToolsets")
1517
)
1618

1719
// ToolFilter is a function that determines if a tool should be included.
@@ -49,6 +51,7 @@ type Builder struct {
4951
filters []ToolFilter // filters to apply to all tools
5052
generateInstructions bool
5153
insidersMode bool
54+
strictToolsets bool
5255
}
5356

5457
// NewBuilder creates a new Builder.
@@ -111,6 +114,13 @@ func (b *Builder) WithToolsets(toolsetIDs []string) *Builder {
111114
return b
112115
}
113116

117+
// WithStrictToolsetValidation controls whether unknown toolset IDs should fail Build().
118+
// When disabled, unknown toolsets are recorded on the inventory for warning-only behavior.
119+
func (b *Builder) WithStrictToolsetValidation(strict bool) *Builder {
120+
b.strictToolsets = strict
121+
return b
122+
}
123+
114124
// WithTools specifies additional tools that bypass toolset filtering.
115125
// These tools are additive - they will be included even if their toolset is not enabled.
116126
// Read-only filtering still applies to these tools.
@@ -222,6 +232,9 @@ func (b *Builder) Build() (*Inventory, error) {
222232

223233
// Process toolsets and pre-compute metadata in a single pass
224234
r.enabledToolsets, r.unrecognizedToolsets, r.toolsetIDs, r.toolsetIDSet, r.defaultToolsetIDs, r.toolsetDescriptions = b.processToolsets()
235+
if b.strictToolsets && len(r.unrecognizedToolsets) > 0 {
236+
return nil, fmt.Errorf("%w: %s", ErrUnknownToolsets, strings.Join(r.unrecognizedToolsets, ", "))
237+
}
225238

226239
// Build set of valid tool names for validation
227240
validToolNames := make(map[string]bool, len(tools))

pkg/inventory/registry_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,34 @@ func TestUnrecognizedToolsets(t *testing.T) {
280280
}
281281
}
282282

283+
func TestBuildErrorsOnUnrecognizedToolsetsWhenStrict(t *testing.T) {
284+
tools := []ServerTool{
285+
mockTool("tool1", "toolset1", true),
286+
}
287+
288+
_, err := NewBuilder().
289+
SetTools(tools).
290+
WithToolsets([]string{"toolset1", "typo"}).
291+
WithStrictToolsetValidation(true).
292+
Build()
293+
294+
require.Error(t, err, "expected error for unrecognized toolset in strict mode")
295+
require.ErrorIs(t, err, ErrUnknownToolsets)
296+
require.Contains(t, err.Error(), "typo")
297+
}
298+
299+
func TestBuildAllowsUnrecognizedToolsetsWhenNotStrict(t *testing.T) {
300+
tools := []ServerTool{
301+
mockTool("tool1", "toolset1", true),
302+
}
303+
304+
reg := mustBuild(t, NewBuilder().
305+
SetTools(tools).
306+
WithToolsets([]string{"toolset1", "typo"}))
307+
308+
require.Equal(t, []string{"typo"}, reg.UnrecognizedToolsets())
309+
}
310+
283311
func TestBuildErrorsOnUnrecognizedTools(t *testing.T) {
284312
tools := []ServerTool{
285313
mockTool("tool1", "toolset1", true),

0 commit comments

Comments
 (0)