Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/cli/mcp_argument_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions pkg/cli/mcp_argument_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
}
Expand All @@ -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
Expand Down
28 changes: 23 additions & 5 deletions pkg/cli/mcp_server_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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
Expand Down
37 changes: 37 additions & 0 deletions pkg/cli/mcp_server_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) {
Expand Down
Loading