diff --git a/pkg/cli/mcp_argument_validation.go b/pkg/cli/mcp_argument_validation.go index b415a004b2e..c05a0ce296f 100644 --- a/pkg/cli/mcp_argument_validation.go +++ b/pkg/cli/mcp_argument_validation.go @@ -77,7 +77,12 @@ func argumentValidationMiddleware(toolParams map[string]toolParamEntry) mcp.Midd toolName := extractMCPToolName(req) validParams, ok := toolParams[toolName] if !ok { - return nil, newMCPError(jsonrpc.CodeMethodNotFound, fmt.Sprintf("unknown MCP tool: %q", toolName), nil) + // The tool name is not in the hardcoded registry (mcpToolParams). + // This should not happen in practice because the registry is + // exhaustive. Escalating to a protocol-level internal error + // makes any registry gap fail loudly as a server bug instead of + // surfacing as an ordinary tool-level validation failure. + return nil, newMCPError(jsonrpc.CodeInternalError, fmt.Sprintf("unknown MCP tool: %q", toolName), nil) } mcpArgValidationLog.Printf("Intercepted unknown param error: tool=%s, unknown_params=%v", toolName, unknownParams) diff --git a/pkg/cli/mcp_argument_validation_test.go b/pkg/cli/mcp_argument_validation_test.go index dd10da4c6aa..a783639a239 100644 --- a/pkg/cli/mcp_argument_validation_test.go +++ b/pkg/cli/mcp_argument_validation_test.go @@ -296,9 +296,9 @@ func TestArgumentValidationMiddleware_PassesThroughSuccessResults(t *testing.T) assert.False(t, toolResult.IsError) } -// TestArgumentValidationMiddleware_UnknownToolReturnsMethodNotFound verifies -// that an unregistered tool name is reported as method-not-found. -func TestArgumentValidationMiddleware_UnknownToolReturnsMethodNotFound(t *testing.T) { +// TestArgumentValidationMiddleware_UnknownToolReturnsInternalError verifies +// that an unregistered tool name is reported as an internal server error. +func TestArgumentValidationMiddleware_UnknownToolReturnsInternalError(t *testing.T) { toolParams := map[string]toolParamEntry{ "compile": {"workflows"}, } @@ -318,7 +318,7 @@ func TestArgumentValidationMiddleware_UnknownToolReturnsMethodNotFound(t *testin var rpcErr *jsonrpc.Error require.ErrorAs(t, err, &rpcErr, "error should be a JSON-RPC error") - assert.Equal(t, int64(jsonrpc.CodeMethodNotFound), rpcErr.Code, "unknown tool should use method-not-found code") + assert.Equal(t, int64(jsonrpc.CodeInternalError), rpcErr.Code, "unknown tool should use internal-error code") } // TestArgumentValidationMiddleware_PassesThroughNonToolCallMethods verifies diff --git a/pkg/cli/mcp_server_http.go b/pkg/cli/mcp_server_http.go index 9f3e2b4f74d..faa691657d9 100644 --- a/pkg/cli/mcp_server_http.go +++ b/pkg/cli/mcp_server_http.go @@ -29,6 +29,19 @@ type responseWriter struct { statusCode int } +func (w *responseWriter) WriteHeader(statusCode int) { + w.statusCode = statusCode + w.ResponseWriter.WriteHeader(statusCode) +} + +func mcpHTTPServerAddr(port int) string { + return fmt.Sprintf("127.0.0.1:%d", port) +} + +func mcpHTTPServerDisplayURL(port int) string { + return fmt.Sprintf("http://127.0.0.1:%d", port) +} + func loggingHandler(handler http.Handler) http.Handler { return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { start := time.Now() @@ -69,21 +82,26 @@ func runHTTPServer(server *mcp.Server, port int) error { handler := mcp.NewStreamableHTTPHandler(func(req *http.Request) *mcp.Server { return server }, &mcp.StreamableHTTPOptions{ - SessionTimeout: 2 * time.Hour, // Close idle sessions after 2 hours - Logger: logger.NewSlogLoggerWithHandler(mcpLog), + SessionTimeout: 2 * time.Hour, // Close idle sessions after 2 hours + DisableLocalhostProtection: false, // Keep the SDK's localhost Host-header checks enabled. + Logger: logger.NewSlogLoggerWithHandler(mcpLog), }) handlerWithLogging := loggingHandler(handler) - // Create HTTP server - addr := fmt.Sprintf(":%d", port) + // Bind to loopback only. Since v1.6.0, the SDK no longer enables + // cross-origin protection by default, so binding to 127.0.0.1 (instead + // of all interfaces) is the safest default for a local MCP server — it + // prevents the server from being reachable on non-localhost interfaces. + // The SDK's localhost Host-header checks stay enabled as a second layer. + addr := mcpHTTPServerAddr(port) httpServer := &http.Server{ Addr: addr, Handler: handlerWithLogging, ReadHeaderTimeout: MCPServerHTTPTimeout, } - fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Starting MCP server on http://localhost"+addr)) + fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Starting MCP server on "+mcpHTTPServerDisplayURL(port))) mcpLog.Printf("HTTP server listening on %s", addr) // Run the HTTP server diff --git a/pkg/cli/mcp_server_http_test.go b/pkg/cli/mcp_server_http_test.go index 8438f8794af..931c089b610 100644 --- a/pkg/cli/mcp_server_http_test.go +++ b/pkg/cli/mcp_server_http_test.go @@ -8,6 +8,40 @@ import ( "testing" ) +func TestMCPHTTPServerAddr_BindsToLoopback(t *testing.T) { + testCases := []struct { + port int + want string + }{ + {port: 0, want: "127.0.0.1:0"}, + {port: 12345, want: "127.0.0.1:12345"}, + {port: 65535, want: "127.0.0.1:65535"}, + } + + for _, tc := range testCases { + if got := mcpHTTPServerAddr(tc.port); got != tc.want { + t.Errorf("mcpHTTPServerAddr(%d) = %q, want %q", tc.port, got, tc.want) + } + } +} + +func TestMCPHTTPServerDisplayURL_UsesLoopbackAddress(t *testing.T) { + testCases := []struct { + port int + want string + }{ + {port: 0, want: "http://127.0.0.1:0"}, + {port: 12345, want: "http://127.0.0.1:12345"}, + {port: 65535, want: "http://127.0.0.1:65535"}, + } + + for _, tc := range testCases { + if got := mcpHTTPServerDisplayURL(tc.port); got != tc.want { + t.Errorf("mcpHTTPServerDisplayURL(%d) = %q, want %q", tc.port, got, tc.want) + } + } +} + func TestSanitizeForLog_NoSpecialChars(t *testing.T) { input := "/api/v1/workflows" got := sanitizeForLog(input) @@ -71,6 +105,9 @@ func TestResponseWriter_EmbeddedResponseWriter(t *testing.T) { if rec.Code != http.StatusCreated { t.Errorf("embedded ResponseWriter code = %d, want %d", rec.Code, http.StatusCreated) } + if rw.statusCode != http.StatusCreated { + t.Errorf("responseWriter statusCode = %d, want %d", rw.statusCode, http.StatusCreated) + } } func TestLoggingHandler_PassesRequestToHandler(t *testing.T) {