From f859503bb86f5ebd40f45c4ff4015d8a578fb4c3 Mon Sep 17 00:00:00 2001 From: Ribhu Siripurapu Date: Thu, 22 Oct 2026 16:10:21 +0000 Subject: [PATCH] mcp: restrict client transport redirects to the endpoint origin Wrap Streamable HTTP and SSE transport HTTP clients so that, when the caller has not set CheckRedirect, redirects stay on the configured endpoint origin and do not change the HTTP method. Callers that supply their own CheckRedirect keep full control. --- mcp/httpclient.go | 45 +++++++++ mcp/httpclient_test.go | 201 +++++++++++++++++++++++++++++++++++++++++ mcp/sse.go | 5 +- mcp/streamable.go | 8 +- 4 files changed, 252 insertions(+), 7 deletions(-) create mode 100644 mcp/httpclient.go create mode 100644 mcp/httpclient_test.go diff --git a/mcp/httpclient.go b/mcp/httpclient.go new file mode 100644 index 00000000..fe607253 --- /dev/null +++ b/mcp/httpclient.go @@ -0,0 +1,45 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package mcp + +import ( + "fmt" + "net/http" + "net/url" + "strings" +) + +// transportHTTPClient returns an HTTP client for MCP transports. When the +// caller does not set CheckRedirect, redirects are limited to the endpoint +// origin and must not change the HTTP method of the original request. +func transportHTTPClient(c *http.Client, base *url.URL) *http.Client { + if c == nil { + c = http.DefaultClient + } + userCheck := c.CheckRedirect + out := *c + out.CheckRedirect = func(req *http.Request, via []*http.Request) error { + if userCheck != nil { + return userCheck(req, via) + } + return checkTransportRedirect(base, req, via) + } + return &out +} + +func checkTransportRedirect(base *url.URL, req *http.Request, via []*http.Request) error { + if !sameOrigin(base, req.URL) { + return fmt.Errorf("mcp: redirect to %q blocked: not same origin as endpoint %q", req.URL, base) + } + if len(via) > 0 && req.Method != via[0].Method { + return fmt.Errorf("mcp: redirect changed HTTP method from %s to %s", via[0].Method, req.Method) + } + return nil +} + +func sameOrigin(base, other *url.URL) bool { + return strings.EqualFold(base.Scheme, other.Scheme) && + strings.EqualFold(base.Host, other.Host) +} diff --git a/mcp/httpclient_test.go b/mcp/httpclient_test.go new file mode 100644 index 00000000..3f8b93ae --- /dev/null +++ b/mcp/httpclient_test.go @@ -0,0 +1,201 @@ +// Copyright 2025 The Go MCP SDK Authors. All rights reserved. +// Use of this source code is governed by the license +// that can be found in the LICENSE file. + +package mcp + +import ( + "bytes" + "context" + "io" + "net/http" + "net/http/httptest" + "net/url" + "sync/atomic" + "testing" + "time" +) + +func TestCheckTransportRedirect(t *testing.T) { + base, err := url.Parse("http://example.com/mcp") + if err != nil { + t.Fatal(err) + } + newReq := func(raw string, method string) *http.Request { + t.Helper() + u, err := url.Parse(raw) + if err != nil { + t.Fatal(err) + } + return &http.Request{Method: method, URL: u} + } + + tests := []struct { + name string + req *http.Request + via []*http.Request + wantErr bool + }{ + { + name: "same origin path change allowed", + req: newReq("http://example.com/other", http.MethodPost), + via: []*http.Request{newReq("http://example.com/mcp", http.MethodPost)}, + }, + { + name: "cross origin blocked", + req: newReq("http://evil.example/mcp", http.MethodPost), + via: []*http.Request{newReq("http://example.com/mcp", http.MethodPost)}, + wantErr: true, + }, + { + name: "method change blocked", + req: newReq("http://example.com/mcp", http.MethodGet), + via: []*http.Request{newReq("http://example.com/mcp", http.MethodPost)}, + wantErr: true, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + err := checkTransportRedirect(base, tc.req, tc.via) + if (err != nil) != tc.wantErr { + t.Fatalf("checkTransportRedirect() error = %v, wantErr %v", err, tc.wantErr) + } + }) + } +} + +func TestTransportHTTPClientRespectsUserCheckRedirect(t *testing.T) { + base, err := url.Parse("http://example.com/mcp") + if err != nil { + t.Fatal(err) + } + var userCalled atomic.Bool + client := transportHTTPClient(&http.Client{ + CheckRedirect: func(*http.Request, []*http.Request) error { + userCalled.Store(true) + return nil + }, + }, base) + req, _ := http.NewRequest(http.MethodGet, "http://other.example/", nil) + if err := client.CheckRedirect(req, nil); err != nil { + t.Fatalf("CheckRedirect() = %v, want nil", err) + } + if !userCalled.Load() { + t.Fatal("user CheckRedirect was not called") + } +} + +func TestStreamableClientTransportBlocksCrossOriginRedirect(t *testing.T) { + ctx := context.Background() + received := make(chan struct{}, 1) + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + received <- struct{}{} + http.Error(w, "redirect target", http.StatusInternalServerError) + })) + t.Cleanup(target.Close) + + server := NewServer(testImpl, nil) + handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, nil) + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + body, err := io.ReadAll(req.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + req.Body = io.NopCloser(bytes.NewReader(body)) + if bytes.Contains(body, []byte(`"method":"ping"`)) { + http.Redirect(w, req, target.URL, http.StatusTemporaryRedirect) + return + } + handler.ServeHTTP(w, req) + })) + t.Cleanup(origin.Close) + + session, err := NewClient(testImpl, nil).Connect(ctx, &StreamableClientTransport{Endpoint: origin.URL}, nil) + if err != nil { + t.Fatalf("Connect() failed: %v", err) + } + defer session.Close() + + if err := session.Ping(ctx, nil); err == nil { + t.Fatal("Ping() succeeded, want redirect error") + } + + select { + case <-received: + t.Fatal("cross-origin redirect was followed") + case <-time.After(250 * time.Millisecond): + } +} + +func TestStreamableClientTransportAllowsSameOriginRedirect(t *testing.T) { + ctx := context.Background() + server := NewServer(testImpl, nil) + handler := NewStreamableHTTPHandler(func(*http.Request) *Server { return server }, nil) + mux := http.NewServeMux() + mux.Handle("/mcp", handler) + mux.Handle("/mcp2", handler) + httpServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.URL.Path == "/mcp" && req.Method == http.MethodPost { + http.Redirect(w, req, "/mcp2", http.StatusTemporaryRedirect) + return + } + mux.ServeHTTP(w, req) + })) + t.Cleanup(httpServer.Close) + + session, err := NewClient(testImpl, nil).Connect(ctx, &StreamableClientTransport{Endpoint: httpServer.URL + "/mcp"}, nil) + if err != nil { + t.Fatalf("Connect() failed: %v", err) + } + defer session.Close() + + if err := session.Ping(ctx, nil); err != nil { + t.Fatalf("Ping() after same-origin redirect: %v", err) + } +} + +func TestSSEClientTransportBlocksCrossOriginRedirect(t *testing.T) { + ctx := context.Background() + received := make(chan struct{}, 1) + target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + received <- struct{}{} + http.Error(w, "redirect target", http.StatusInternalServerError) + })) + t.Cleanup(target.Close) + + server := NewServer(testImpl, nil) + sseHandler := NewSSEHandler(func(*http.Request) *Server { return server }, nil) + origin := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + if req.Method == http.MethodPost { + body, err := io.ReadAll(req.Body) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + req.Body = io.NopCloser(bytes.NewReader(body)) + if bytes.Contains(body, []byte(`"method":"ping"`)) { + http.Redirect(w, req, target.URL, http.StatusTemporaryRedirect) + return + } + } + sseHandler.ServeHTTP(w, req) + })) + t.Cleanup(origin.Close) + + session, err := NewClient(testImpl, nil).Connect(ctx, &SSEClientTransport{Endpoint: origin.URL}, nil) + if err != nil { + t.Fatalf("Connect() failed: %v", err) + } + defer session.Close() + + if err := session.Ping(ctx, nil); err == nil { + t.Fatal("Ping() succeeded, want redirect error") + } + + select { + case <-received: + t.Fatal("cross-origin redirect was followed") + case <-time.After(250 * time.Millisecond): + } +} diff --git a/mcp/sse.go b/mcp/sse.go index b9eede0f..6062fd85 100644 --- a/mcp/sse.go +++ b/mcp/sse.go @@ -440,10 +440,7 @@ func (c *SSEClientTransport) Connect(ctx context.Context) (Connection, error) { if err != nil { return nil, err } - httpClient := c.HTTPClient - if httpClient == nil { - httpClient = http.DefaultClient - } + httpClient := transportHTTPClient(c.HTTPClient, parsedURL) req.Header.Set("Accept", "text/event-stream") resp, err := httpClient.Do(req) if err != nil { diff --git a/mcp/streamable.go b/mcp/streamable.go index 819bc08f..b8d5b99b 100644 --- a/mcp/streamable.go +++ b/mcp/streamable.go @@ -23,6 +23,7 @@ import ( "mime" "net" "net/http" + "net/url" "slices" "strconv" "strings" @@ -2274,10 +2275,11 @@ func init() { // When closed, the connection issues a DELETE request to terminate the // session, unless no session was established. func (t *StreamableClientTransport) Connect(ctx context.Context) (Connection, error) { - client := t.HTTPClient - if client == nil { - client = http.DefaultClient + baseURL, err := url.Parse(t.Endpoint) + if err != nil { + return nil, fmt.Errorf("invalid endpoint URL: %w", err) } + client := transportHTTPClient(t.HTTPClient, baseURL) maxRetries := t.MaxRetries if maxRetries == 0 { maxRetries = 5