From 9f46e445a59701a4a172d8e9cb4b8dce5a4b1e6c Mon Sep 17 00:00:00 2001 From: Mahmoud772122777 Date: Tue, 11 Aug 2026 11:54:45 +0530 Subject: [PATCH] Fix static tools validation fallback --- pkg/http/handler.go | 16 +++++++++------- pkg/http/handler_test.go | 29 ++++++++++++++++++++++++++++- 2 files changed, 37 insertions(+), 8 deletions(-) diff --git a/pkg/http/handler.go b/pkg/http/handler.go index ab229e55dc..459f28a77d 100644 --- a/pkg/http/handler.go +++ b/pkg/http/handler.go @@ -244,7 +244,7 @@ func DefaultGitHubMCPServerFactory(r *http.Request, deps github.ToolDependencies func DefaultInventoryFactory(cfg *ServerConfig, t translations.TranslationHelperFunc, featureChecker inventory.FeatureFlagChecker, scopeFetcher scopes.FetcherInterface) InventoryFactoryFunc { // Build the static tool/resource/prompt universe from CLI flags. // This is done once at startup and captured in the closure. - staticTools, staticResources, staticPrompts := buildStaticInventory(cfg, t) + staticTools, staticResources, staticPrompts, staticErr := buildStaticInventory(cfg, t) hasStaticFilters := hasStaticConfig(cfg) // Pre-compute valid tool names for filtering per-request tool headers. @@ -256,6 +256,10 @@ func DefaultInventoryFactory(cfg *ServerConfig, t translations.TranslationHelper } return func(r *http.Request) (*inventory.Inventory, error) { + if staticErr != nil { + return nil, staticErr + } + b := inventory.NewBuilder(). SetTools(staticTools). SetResources(staticResources). @@ -327,7 +331,7 @@ func hasStaticConfig(cfg *ServerConfig) bool { // non-granular siblings — must be carried through to the per-request // inventory, which then installs a checker and resolves the flag before // registering tools with the MCP server. -func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFunc) ([]inventory.ServerTool, []inventory.ServerResourceTemplate, []inventory.ServerPrompt) { +func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFunc) ([]inventory.ServerTool, []inventory.ServerResourceTemplate, []inventory.ServerPrompt, error) { // Tools with host-specific capabilities need to know the deployment they // will talk to. An unparseable host is not fatal here: NewAPIHost rejects // it later with a clearer error, so fall back to the dotcom default. @@ -338,7 +342,7 @@ func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFun opts := []github.ToolOption{github.WithHost(hostType)} if !hasStaticConfig(cfg) { - return github.AllTools(t, opts...), github.AllResources(t), github.AllPrompts(t) + return github.AllTools(t, opts...), github.AllResources(t), github.AllPrompts(t), nil } b := github.NewInventory(t, opts...). @@ -355,13 +359,11 @@ func buildStaticInventory(cfg *ServerConfig, t translations.TranslationHelperFun inv, err := b.Build() if err != nil { - // Fall back to all tools if there's an error (e.g. unknown tool names). - // The error will surface again at per-request time if relevant. - return github.AllTools(t, opts...), github.AllResources(t), github.AllPrompts(t) + return nil, nil, nil, err } ctx := context.Background() - return inv.AvailableTools(ctx), inv.AvailableResourceTemplates(ctx), inv.AvailablePrompts(ctx) + return inv.AvailableTools(ctx), inv.AvailableResourceTemplates(ctx), inv.AvailablePrompts(ctx), nil } // InventoryFiltersForRequest applies filters to the inventory builder diff --git a/pkg/http/handler_test.go b/pkg/http/handler_test.go index b4d509c3e5..0abacdcac6 100644 --- a/pkg/http/handler_test.go +++ b/pkg/http/handler_test.go @@ -634,6 +634,33 @@ func TestStaticConfigEnforcement(t *testing.T) { } } +func TestStaticInventoryInvalidEnabledToolsReturnsBadRequest(t *testing.T) { + apiHost, err := utils.NewAPIHost("https://api.github.com") + require.NoError(t, err) + + handler := NewHTTPMcpHandler( + context.Background(), + &ServerConfig{Version: "test", EnabledTools: []string{"nonexistent_tool"}}, + nil, + translations.NullTranslationHelper, + slog.Default(), + apiHost, + ) + + r := chi.NewRouter() + handler.RegisterMiddleware(r) + handler.RegisterRoutes(r) + + req := httptest.NewRequest(http.MethodPost, "/", nil) + req.Header.Set(headers.AuthorizationHeader, "Bearer ghp_testtoken") + + rr := httptest.NewRecorder() + r.ServeHTTP(rr, req) + + assert.Equal(t, http.StatusBadRequest, rr.Code) + assert.Contains(t, rr.Body.String(), "unknown tools specified") +} + func TestStaticInventoryPreservesPerRequestFeatureVariants(t *testing.T) { tools := []inventory.ServerTool{ mockToolWithFeatureFlag("list_issues", "issues", true, "", github.FeatureFlagCSVOutput), @@ -817,7 +844,7 @@ func TestStaticInventoryAppliesHostCapabilities(t *testing.T) { t.Parallel() cfg := &ServerConfig{Version: "test", Host: tt.host} - staticTools, _, _ := buildStaticInventory(cfg, translations.NullTranslationHelper) + staticTools, _, _, _ := buildStaticInventory(cfg, translations.NullTranslationHelper) var found bool for _, st := range staticTools {