Fix static --tools validation fallback

pkg/http/handler.go

@@ -264,9 +264,24 @@ func DefaultGitHubMCPServerFactory(r *http.Request, deps github.ToolDependencies
 // a static inventory is built once at factory creation to pre-filter the tool
 // universe. Per-request headers can only narrow within these bounds.
 func DefaultInventoryFactory(cfg *ServerConfig, t translations.TranslationHelperFunc, featureChecker inventory.FeatureFlagChecker, scopeFetcher scopes.FetcherInterface) InventoryFactoryFunc {
+	factory, err := NewDefaultInventoryFactory(cfg, t, featureChecker, scopeFetcher)
+	if err != nil {
+		return func(_ *http.Request) (*inventory.Inventory, error) {
+			return nil, err
+		}
+	}
+	return factory
+}
+
+// NewDefaultInventoryFactory creates the default HTTP inventory factory and
+// validates static tool configuration before returning it.
+func NewDefaultInventoryFactory(cfg *ServerConfig, t translations.TranslationHelperFunc, featureChecker inventory.FeatureFlagChecker, scopeFetcher scopes.FetcherInterface) (InventoryFactoryFunc, error) {
 	// 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, err := buildStaticInventory(cfg, t)
+	if err != nil {
+		return nil, err
+	}
 	hasStaticFilters := hasStaticConfig(cfg)
 
 	// Pre-compute valid tool names for filtering per-request tool headers.
@@ -310,7 +325,7 @@ func DefaultInventoryFactory(cfg *ServerConfig, t translations.TranslationHelper
 		b.WithServerInstructions()
 
 		return b.Build()
-	}
+	}, nil
 }
 
 // filterRequestTools returns a shallow copy of the request with any per-request
@@ -349,7 +364,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.
@@ -360,7 +375,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...).
@@ -377,13 +392,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

pkg/http/handler_test.go

@@ -558,7 +558,8 @@ func TestStaticConfigEnforcement(t *testing.T) {
 			require.NoError(t, err)
 
 			// Build static tools the same way the production code does
-			staticTools, staticResources, staticPrompts := buildStaticInventoryFromTools(tt.config, tools)
+			staticTools, staticResources, staticPrompts, err := buildStaticInventoryFromTools(tt.config, tools)
+			require.NoError(t, err)
 			hasStatic := hasStaticConfig(tt.config)
 
 			validToolNames := make(map[string]bool, len(staticTools))
@@ -636,6 +637,68 @@ func TestStaticConfigEnforcement(t *testing.T) {
 	}
 }
 
+func TestNewDefaultInventoryFactoryRejectsInvalidEnabledTools(t *testing.T) {
+	tests := []struct {
+		name         string
+		enabledTools []string
+		unknownTools []string
+	}{
+		{
+			name:         "mixed valid and invalid tools",
+			enabledTools: []string{"get_file_contents", "nonexistent_tool"},
+			unknownTools: []string{"nonexistent_tool"},
+		},
+		{
+			name:         "all invalid tools",
+			enabledTools: []string{"nonexistent_tool", "another_nonexistent_tool"},
+			unknownTools: []string{"nonexistent_tool", "another_nonexistent_tool"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			factory, err := NewDefaultInventoryFactory(
+				&ServerConfig{Version: "test", EnabledTools: tt.enabledTools},
+				translations.NullTranslationHelper,
+				nil,
+				allScopesFetcher{},
+			)
+			require.ErrorIs(t, err, inventory.ErrUnknownTools)
+			for _, unknownTool := range tt.unknownTools {
+				assert.Contains(t, err.Error(), unknownTool)
+			}
+			assert.Nil(t, factory, "invalid static configuration must not produce a widened inventory factory")
+		})
+	}
+}
+
+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),
@@ -644,7 +707,8 @@ func TestStaticInventoryPreservesPerRequestFeatureVariants(t *testing.T) {
 	cfg := &ServerConfig{Version: "test", EnabledToolsets: []string{"issues"}}
 	featureChecker := createHTTPFeatureChecker(nil, false)
 
-	staticTools, _, _ := buildStaticInventoryFromTools(cfg, tools)
+	staticTools, _, _, err := buildStaticInventoryFromTools(cfg, tools)
+	require.NoError(t, err)
 	require.Len(t, staticTools, 2, "static upper bounds should preserve both feature variants")
 
 	inv, err := inventory.NewBuilder().
@@ -758,9 +822,9 @@ func TestContentTypeHandling(t *testing.T) {
 
 // buildStaticInventoryFromTools is a test helper that mirrors buildStaticInventory
 // but uses the provided mock tools instead of calling github.AllTools.
-func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTool) ([]inventory.ServerTool, []inventory.ServerResourceTemplate, []inventory.ServerPrompt) {
+func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTool) ([]inventory.ServerTool, []inventory.ServerResourceTemplate, []inventory.ServerPrompt, error) {
 	if !hasStaticConfig(cfg) {
-		return tools, nil, nil
+		return tools, nil, nil, nil
 	}
 
 	b := inventory.NewBuilder().
@@ -778,11 +842,11 @@ func buildStaticInventoryFromTools(cfg *ServerConfig, tools []inventory.ServerTo
 
 	inv, err := b.Build()
 	if err != nil {
-		return tools, nil, nil
+		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
 }
 
 // TestStaticInventoryAppliesHostCapabilities guards against HTTP deployments
@@ -819,7 +883,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 {

pkg/http/server.go

@@ -142,6 +142,11 @@ func RunHTTPServer(cfg ServerConfig) error {
 	}
 
 	featureChecker := createHTTPFeatureChecker(cfg.EnabledFeatures, cfg.InsidersMode)
+	scopeFetcher := scopes.NewFetcher(apiHost, scopes.FetcherOptions{})
+	inventoryFactory, err := NewDefaultInventoryFactory(&cfg, t, featureChecker, scopeFetcher)
+	if err != nil {
+		return fmt.Errorf("failed to build inventory: %w", err)
+	}
 
 	obs, err := observability.NewExporters(logger, metrics.NewNoopMetrics())
 	if err != nil {
@@ -172,10 +177,9 @@ func RunHTTPServer(cfg ServerConfig) error {
 		TrustProxyHeaders: cfg.TrustProxyHeaders,
 	}
 
-	serverOptions := []HandlerOption{}
-	if cfg.ScopeChallenge {
-		scopeFetcher := scopes.NewFetcher(apiHost, scopes.FetcherOptions{})
-		serverOptions = append(serverOptions, WithScopeFetcher(scopeFetcher))
+	serverOptions := []HandlerOption{
+		WithInventoryFactory(inventoryFactory),
+		WithScopeFetcher(scopeFetcher),
 	}
 
 	r := chi.NewRouter()

pkg/http/server_test.go

@@ -6,11 +6,41 @@ import (
 
 	ghcontext "github.com/github/github-mcp-server/pkg/context"
 	"github.com/github/github-mcp-server/pkg/github"
+	"github.com/github/github-mcp-server/pkg/inventory"
 	"github.com/github/github-mcp-server/pkg/utils"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+func TestRunHTTPServerRejectsInvalidStaticTools(t *testing.T) {
+	tests := []struct {
+		name         string
+		enabledTools []string
+	}{
+		{
+			name:         "mixed valid and invalid tools",
+			enabledTools: []string{"get_file_contents", "nonexistent_tool"},
+		},
+		{
+			name:         "all invalid tools",
+			enabledTools: []string{"nonexistent_tool", "another_nonexistent_tool"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := RunHTTPServer(ServerConfig{
+				Version:      "test",
+				Host:         "https://github.com",
+				EnabledTools: tt.enabledTools,
+			})
+
+			require.ErrorIs(t, err, inventory.ErrUnknownTools)
+			assert.ErrorContains(t, err, "failed to build inventory")
+		})
+	}
+}
+
 func TestInitGlobalToolScopeMapUsesHost(t *testing.T) {
 	tests := []struct {
 		name     string