-
Notifications
You must be signed in to change notification settings - Fork 3.7k
Fail closed on unknown toolsets in strict mode #2184
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -336,6 +336,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. | ||
|
Comment on lines
+339
to
+343
|
||
|
|
||
| <table> | ||
| <tr><th>Local Server Only</th></tr> | ||
| <tr valign="top"> | ||
| <td> | ||
|
|
||
| ```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}" | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| </td> | ||
| </tr> | ||
| </table> | ||
|
|
||
| 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 | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)), | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR description says strict-toolsets is documented in the README, but there don’t appear to be any README updates in this change set (and the only doc update is in this file). Either update README.md (tool configuration section) to mention
--strict-toolsets/GITHUB_STRICT_TOOLSETS, or adjust the PR description so it matches what’s actually being changed.