Skip to content

Commit 3df818a

Browse files
authored
fix(mcp-server): bind HTTP server to 127.0.0.1; document validation escalation (#43766)
1 parent 7a7e6c1 commit 3df818a

4 files changed

Lines changed: 70 additions & 10 deletions

File tree

pkg/cli/mcp_argument_validation.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,12 @@ func argumentValidationMiddleware(toolParams map[string]toolParamEntry) mcp.Midd
7777
toolName := extractMCPToolName(req)
7878
validParams, ok := toolParams[toolName]
7979
if !ok {
80-
return nil, newMCPError(jsonrpc.CodeMethodNotFound, fmt.Sprintf("unknown MCP tool: %q", toolName), nil)
80+
// The tool name is not in the hardcoded registry (mcpToolParams).
81+
// This should not happen in practice because the registry is
82+
// exhaustive. Escalating to a protocol-level internal error
83+
// makes any registry gap fail loudly as a server bug instead of
84+
// surfacing as an ordinary tool-level validation failure.
85+
return nil, newMCPError(jsonrpc.CodeInternalError, fmt.Sprintf("unknown MCP tool: %q", toolName), nil)
8186
}
8287

8388
mcpArgValidationLog.Printf("Intercepted unknown param error: tool=%s, unknown_params=%v", toolName, unknownParams)

pkg/cli/mcp_argument_validation_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -296,9 +296,9 @@ func TestArgumentValidationMiddleware_PassesThroughSuccessResults(t *testing.T)
296296
assert.False(t, toolResult.IsError)
297297
}
298298

299-
// TestArgumentValidationMiddleware_UnknownToolReturnsMethodNotFound verifies
300-
// that an unregistered tool name is reported as method-not-found.
301-
func TestArgumentValidationMiddleware_UnknownToolReturnsMethodNotFound(t *testing.T) {
299+
// TestArgumentValidationMiddleware_UnknownToolReturnsInternalError verifies
300+
// that an unregistered tool name is reported as an internal server error.
301+
func TestArgumentValidationMiddleware_UnknownToolReturnsInternalError(t *testing.T) {
302302
toolParams := map[string]toolParamEntry{
303303
"compile": {"workflows"},
304304
}
@@ -318,7 +318,7 @@ func TestArgumentValidationMiddleware_UnknownToolReturnsMethodNotFound(t *testin
318318

319319
var rpcErr *jsonrpc.Error
320320
require.ErrorAs(t, err, &rpcErr, "error should be a JSON-RPC error")
321-
assert.Equal(t, int64(jsonrpc.CodeMethodNotFound), rpcErr.Code, "unknown tool should use method-not-found code")
321+
assert.Equal(t, int64(jsonrpc.CodeInternalError), rpcErr.Code, "unknown tool should use internal-error code")
322322
}
323323

324324
// TestArgumentValidationMiddleware_PassesThroughNonToolCallMethods verifies

pkg/cli/mcp_server_http.go

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,19 @@ type responseWriter struct {
2929
statusCode int
3030
}
3131

32+
func (w *responseWriter) WriteHeader(statusCode int) {
33+
w.statusCode = statusCode
34+
w.ResponseWriter.WriteHeader(statusCode)
35+
}
36+
37+
func mcpHTTPServerAddr(port int) string {
38+
return fmt.Sprintf("127.0.0.1:%d", port)
39+
}
40+
41+
func mcpHTTPServerDisplayURL(port int) string {
42+
return fmt.Sprintf("http://127.0.0.1:%d", port)
43+
}
44+
3245
func loggingHandler(handler http.Handler) http.Handler {
3346
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
3447
start := time.Now()
@@ -69,21 +82,26 @@ func runHTTPServer(server *mcp.Server, port int) error {
6982
handler := mcp.NewStreamableHTTPHandler(func(req *http.Request) *mcp.Server {
7083
return server
7184
}, &mcp.StreamableHTTPOptions{
72-
SessionTimeout: 2 * time.Hour, // Close idle sessions after 2 hours
73-
Logger: logger.NewSlogLoggerWithHandler(mcpLog),
85+
SessionTimeout: 2 * time.Hour, // Close idle sessions after 2 hours
86+
DisableLocalhostProtection: false, // Keep the SDK's localhost Host-header checks enabled.
87+
Logger: logger.NewSlogLoggerWithHandler(mcpLog),
7488
})
7589

7690
handlerWithLogging := loggingHandler(handler)
7791

78-
// Create HTTP server
79-
addr := fmt.Sprintf(":%d", port)
92+
// Bind to loopback only. Since v1.6.0, the SDK no longer enables
93+
// cross-origin protection by default, so binding to 127.0.0.1 (instead
94+
// of all interfaces) is the safest default for a local MCP server — it
95+
// prevents the server from being reachable on non-localhost interfaces.
96+
// The SDK's localhost Host-header checks stay enabled as a second layer.
97+
addr := mcpHTTPServerAddr(port)
8098
httpServer := &http.Server{
8199
Addr: addr,
82100
Handler: handlerWithLogging,
83101
ReadHeaderTimeout: MCPServerHTTPTimeout,
84102
}
85103

86-
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Starting MCP server on http://localhost"+addr))
104+
fmt.Fprintln(os.Stderr, console.FormatInfoMessage("Starting MCP server on "+mcpHTTPServerDisplayURL(port)))
87105
mcpLog.Printf("HTTP server listening on %s", addr)
88106

89107
// Run the HTTP server

pkg/cli/mcp_server_http_test.go

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,40 @@ import (
88
"testing"
99
)
1010

11+
func TestMCPHTTPServerAddr_BindsToLoopback(t *testing.T) {
12+
testCases := []struct {
13+
port int
14+
want string
15+
}{
16+
{port: 0, want: "127.0.0.1:0"},
17+
{port: 12345, want: "127.0.0.1:12345"},
18+
{port: 65535, want: "127.0.0.1:65535"},
19+
}
20+
21+
for _, tc := range testCases {
22+
if got := mcpHTTPServerAddr(tc.port); got != tc.want {
23+
t.Errorf("mcpHTTPServerAddr(%d) = %q, want %q", tc.port, got, tc.want)
24+
}
25+
}
26+
}
27+
28+
func TestMCPHTTPServerDisplayURL_UsesLoopbackAddress(t *testing.T) {
29+
testCases := []struct {
30+
port int
31+
want string
32+
}{
33+
{port: 0, want: "http://127.0.0.1:0"},
34+
{port: 12345, want: "http://127.0.0.1:12345"},
35+
{port: 65535, want: "http://127.0.0.1:65535"},
36+
}
37+
38+
for _, tc := range testCases {
39+
if got := mcpHTTPServerDisplayURL(tc.port); got != tc.want {
40+
t.Errorf("mcpHTTPServerDisplayURL(%d) = %q, want %q", tc.port, got, tc.want)
41+
}
42+
}
43+
}
44+
1145
func TestSanitizeForLog_NoSpecialChars(t *testing.T) {
1246
input := "/api/v1/workflows"
1347
got := sanitizeForLog(input)
@@ -71,6 +105,9 @@ func TestResponseWriter_EmbeddedResponseWriter(t *testing.T) {
71105
if rec.Code != http.StatusCreated {
72106
t.Errorf("embedded ResponseWriter code = %d, want %d", rec.Code, http.StatusCreated)
73107
}
108+
if rw.statusCode != http.StatusCreated {
109+
t.Errorf("responseWriter statusCode = %d, want %d", rw.statusCode, http.StatusCreated)
110+
}
74111
}
75112

76113
func TestLoggingHandler_PassesRequestToHandler(t *testing.T) {

0 commit comments

Comments
 (0)