Limit HTTP request bodies before MCP middleware parsing

pkg/http/handler.go

@@ -132,6 +132,8 @@ func NewHTTPMcpHandler(
 
 func (h *Handler) RegisterMiddleware(r chi.Router) {
 	r.Use(
+		// Must run first: bounds the body before anything downstream reads it.
+		middleware.WithMaxBodySize(h.maxRequestBodyBytes()),
 		middleware.ExtractUserToken(h.oauthCfg),
 		middleware.WithRequestConfig,
 		middleware.WithMCPParse(),
@@ -143,6 +145,15 @@ func (h *Handler) RegisterMiddleware(r chi.Router) {
 	}
 }
 
+// maxRequestBodyBytes returns the effective request-body size limit, applied
+// both by the early middleware and by the MCP SDK handler.
+func (h *Handler) maxRequestBodyBytes() int64 {
+	if h.config != nil && h.config.MaxRequestBodyBytes > 0 {
+		return h.config.MaxRequestBodyBytes
+	}
+	return middleware.DefaultMaxRequestBodyBytes
+}
+
 // RegisterRoutes registers the routes for the MCP server
 // URL-based values take precedence over header-based values
 func (h *Handler) RegisterRoutes(r chi.Router) {
@@ -239,6 +250,9 @@ func (h *Handler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
 		return ghServer
 	}, &mcp.StreamableHTTPOptions{
 		Stateless: true,
+		// Required, not just belt-and-braces: the effective limit exceeds the
+		// SDK's own default, which would otherwise cap it.
+		MaxRequestBodyBytes: h.maxRequestBodyBytes(),
 	})
 
 	mcpHandler.ServeHTTP(w, r)

pkg/http/handler_test.go

@@ -3,6 +3,7 @@ package http
 import (
 	"context"
 	"encoding/json"
+	"fmt"
 	"log/slog"
 	"net/http"
 	"net/http/httptest"
@@ -14,6 +15,7 @@ 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/http/headers"
+	"github.com/github/github-mcp-server/pkg/http/middleware"
 	"github.com/github/github-mcp-server/pkg/inventory"
 	"github.com/github/github-mcp-server/pkg/scopes"
 	"github.com/github/github-mcp-server/pkg/translations"
@@ -1287,3 +1289,136 @@ func TestUIMetaStrippedWhenClientLacksCapability(t *testing.T) {
 	require.Len(t, unknown, 1)
 	require.NotNil(t, unknown[0].Tool.Meta["ui"], "_meta.ui should be preserved when capability is unknown and FF is on")
 }
+
+// TestMaxRequestBodyBytes checks the effective limit and, critically, that it
+// sits above the MCP SDK default — which is why it must also be passed to
+// StreamableHTTPOptions rather than left to the SDK.
+func TestMaxRequestBodyBytes(t *testing.T) {
+	t.Run("default leaves headroom above the MCP SDK limit", func(t *testing.T) {
+		h := &Handler{config: &ServerConfig{}}
+
+		assert.Equal(t, int64(5<<20), h.maxRequestBodyBytes())
+		assert.Greater(t, h.maxRequestBodyBytes(), int64(mcp.DefaultMaxRequestBodyBytes),
+			"the default intentionally exceeds the SDK limit, so the SDK must be told about it")
+	})
+
+	t.Run("configured value overrides the default", func(t *testing.T) {
+		h := &Handler{config: &ServerConfig{MaxRequestBodyBytes: 1234}}
+		assert.Equal(t, int64(1234), h.maxRequestBodyBytes())
+	})
+}
+
+// TestMaxRequestBodySizeEnforcement exercises both layers the limit is applied
+// at: the early middleware, and the MCP SDK handler the request is delegated to.
+func TestMaxRequestBodySizeEnforcement(t *testing.T) {
+	const limit = 256
+
+	apiHost, err := utils.NewAPIHost("https://api.github.com")
+	require.NoError(t, err)
+
+	buildBody := func(size int) string {
+		payload := `{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{"pad":"PADDING"}}`
+		if len(payload) >= size {
+			return payload
+		}
+		pad := strings.Repeat("x", size-len(payload))
+		return strings.Replace(payload, "PADDING", "PADDING"+pad, 1)
+	}
+
+	newHandler := func(t *testing.T, maxBytes int64, mcpServerFactoryCalled *bool) *Handler {
+		t.Helper()
+		return NewHTTPMcpHandler(
+			context.Background(),
+			&ServerConfig{Version: "test", MaxRequestBodyBytes: maxBytes},
+			nil,
+			translations.NullTranslationHelper,
+			slog.Default(),
+			apiHost,
+			WithInventoryFactory(func(_ *http.Request) (*inventory.Inventory, error) {
+				return inventory.NewBuilder().Build()
+			}),
+			WithGitHubMCPServerFactory(func(_ *http.Request, _ github.ToolDependencies, _ *inventory.Inventory, _ *github.MCPServerConfig) (*mcp.Server, error) {
+				if mcpServerFactoryCalled != nil {
+					*mcpServerFactoryCalled = true
+				}
+				return mcp.NewServer(&mcp.Implementation{Name: "test", Version: "0.0.1"}, nil), nil
+			}),
+			WithScopeFetcher(allScopesFetcher{}),
+		)
+	}
+
+	newRouter := func(h *Handler) http.Handler {
+		r := chi.NewRouter()
+		h.RegisterMiddleware(r)
+		h.RegisterRoutes(r)
+		return r
+	}
+
+	newRequest := func(body string) *http.Request {
+		req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
+		req.Header.Set(headers.ContentTypeHeader, headers.ContentTypeJSON)
+		req.Header.Set(headers.AcceptHeader, strings.Join([]string{headers.ContentTypeJSON, headers.ContentTypeEventStream}, ", "))
+		req.Header.Set(headers.AuthorizationHeader, strings.Join([]string{"ghs", "test-token"}, "_"))
+		return req
+	}
+
+	t.Run("middleware rejects an oversized request before the MCP server is built", func(t *testing.T) {
+		var mcpServerFactoryCalled bool
+		r := newRouter(newHandler(t, limit, &mcpServerFactoryCalled))
+
+		body := buildBody(limit + 1)
+		require.Greater(t, len(body), limit)
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, newRequest(body))
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
+		assert.Contains(t, rr.Body.String(), "request body too large")
+		assert.False(t, mcpServerFactoryCalled, "the MCP server should never be constructed for an oversized request")
+	})
+
+	t.Run("request at the configured limit succeeds", func(t *testing.T) {
+		var mcpServerFactoryCalled bool
+		r := newRouter(newHandler(t, limit, &mcpServerFactoryCalled))
+
+		body := buildBody(limit)
+		require.Len(t, body, limit)
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, newRequest(body))
+
+		assert.Equal(t, http.StatusOK, rr.Code, "response body: %s", rr.Body.String())
+		assert.True(t, mcpServerFactoryCalled, "the MCP server should be constructed for an allowed request")
+	})
+
+	t.Run("SDK handler enforces the configured limit when the middleware is bypassed", func(t *testing.T) {
+		h := newHandler(t, limit, nil)
+
+		body := buildBody(limit + 1)
+		require.Greater(t, len(body), limit)
+
+		rr := httptest.NewRecorder()
+		h.ServeHTTP(rr, newRequest(body))
+
+		assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
+		assert.Contains(t, rr.Body.String(), fmt.Sprintf("request body exceeds %d bytes", limit),
+			"the SDK should report the configured limit, not its own default")
+	})
+
+	// The default headroom only exists if it reaches the SDK as well; leaving
+	// the SDK on its own default would silently reject this request.
+	t.Run("unconfigured handler accepts a request above the MCP SDK limit", func(t *testing.T) {
+		var mcpServerFactoryCalled bool
+		r := newRouter(newHandler(t, 0, &mcpServerFactoryCalled))
+
+		body := buildBody(mcp.DefaultMaxRequestBodyBytes + 1024)
+		require.Greater(t, int64(len(body)), int64(mcp.DefaultMaxRequestBodyBytes))
+		require.Less(t, int64(len(body)), middleware.DefaultMaxRequestBodyBytes)
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, newRequest(body))
+
+		assert.Equal(t, http.StatusOK, rr.Code, "response body: %s", rr.Body.String())
+		assert.True(t, mcpServerFactoryCalled, "the MCP server should be constructed for an allowed request")
+	})
+}

pkg/http/middleware/body_limit.go

@@ -0,0 +1,46 @@
+package middleware
+
+import (
+	"errors"
+	"net/http"
+)
+
+// DefaultMaxRequestBodyBytes bounds the total HTTP request, not just the tool
+// payload within it. It sits modestly above the MCP SDK's own default to leave
+// room for JSON-RPC and tool-call envelope overhead; because it is the larger
+// of the two, callers must also pass it to the SDK or the SDK would cap it.
+const DefaultMaxRequestBodyBytes int64 = 5 << 20 // 5 MiB
+
+// WithMaxBodySize bounds the size of the request body. It must be registered
+// before any middleware that reads or buffers the body (WithMCPParse,
+// WithScopeChallenge), so an oversized payload is rejected before it is
+// buffered in memory rather than by a later guard in the MCP SDK.
+//
+// A body of unknown length (chunked, HTTP/2) cannot be rejected upfront, so
+// the limit is instead enforced on read and surfaces as a *http.MaxBytesError
+// to whichever middleware reads the body first.
+func WithMaxBodySize(maxBytes int64) func(http.Handler) http.Handler {
+	return func(next http.Handler) http.Handler {
+		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			if r.ContentLength > maxBytes {
+				writeRequestTooLarge(w)
+				return
+			}
+
+			if r.Body != nil {
+				r.Body = http.MaxBytesReader(w, r.Body, maxBytes)
+			}
+
+			next.ServeHTTP(w, r)
+		})
+	}
+}
+
+func writeRequestTooLarge(w http.ResponseWriter) {
+	http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
+}
+
+func isMaxBytesError(err error) bool {
+	var maxBytesErr *http.MaxBytesError
+	return errors.As(err, &maxBytesErr)
+}

pkg/http/middleware/body_limit_test.go

@@ -0,0 +1,114 @@
+package middleware
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// unknownLengthBody hides Len from httptest.NewRequest so ContentLength is -1,
+// as it is for a chunked or HTTP/2 request.
+func unknownLengthBody(s string) io.Reader {
+	return io.NopCloser(strings.NewReader(s))
+}
+
+func TestWithMaxBodySize(t *testing.T) {
+	const limit = 16
+
+	t.Run("allowed request under the limit passes through", func(t *testing.T) {
+		var nextCalled bool
+		var readBody string
+		var readErr error
+
+		next := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+			nextCalled = true
+			b, err := io.ReadAll(r.Body)
+			readBody, readErr = string(b), err
+		})
+
+		handler := WithMaxBodySize(limit)(next)
+
+		req := httptest.NewRequest(http.MethodPost, "/mcp", strings.NewReader("short"))
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+
+		assert.True(t, nextCalled, "next handler should be called for an allowed request")
+		require.NoError(t, readErr)
+		assert.Equal(t, "short", readBody)
+		assert.Equal(t, http.StatusOK, rr.Code)
+	})
+
+	t.Run("boundary size exactly at the limit is allowed", func(t *testing.T) {
+		body := strings.Repeat("a", limit)
+
+		var nextCalled bool
+		var readBody string
+		var readErr error
+
+		next := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+			nextCalled = true
+			b, err := io.ReadAll(r.Body)
+			readBody, readErr = string(b), err
+		})
+
+		handler := WithMaxBodySize(limit)(next)
+
+		req := httptest.NewRequest(http.MethodPost, "/mcp", strings.NewReader(body))
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+
+		assert.True(t, nextCalled, "next handler should be called when the body is exactly at the limit")
+		require.NoError(t, readErr, "reading exactly maxBytes should not error")
+		assert.Equal(t, body, readBody, "the full boundary-size body should be readable")
+	})
+
+	t.Run("oversized request with known Content-Length is rejected before next runs", func(t *testing.T) {
+		body := strings.Repeat("a", limit+1)
+
+		var nextCalled bool
+		next := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+			nextCalled = true
+		})
+
+		handler := WithMaxBodySize(limit)(next)
+
+		req := httptest.NewRequest(http.MethodPost, "/mcp", strings.NewReader(body))
+		require.Equal(t, int64(limit+1), req.ContentLength, "test setup: Content-Length should be known")
+
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+
+		assert.False(t, nextCalled, "next handler must not run for an oversized request")
+		assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
+		assert.Contains(t, rr.Body.String(), "request body too large")
+	})
+
+	t.Run("oversized request with unknown length fails on downstream read", func(t *testing.T) {
+		body := strings.Repeat("a", limit+1)
+
+		var nextCalled bool
+		var readErr error
+
+		next := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+			nextCalled = true
+			_, readErr = io.ReadAll(r.Body)
+		})
+
+		handler := WithMaxBodySize(limit)(next)
+
+		req := httptest.NewRequest(http.MethodPost, "/mcp", unknownLengthBody(body))
+		require.Equal(t, int64(-1), req.ContentLength, "test setup: Content-Length should be unknown")
+
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+
+		assert.True(t, nextCalled, "next handler still runs; the limit is enforced on read")
+		require.Error(t, readErr)
+		assert.True(t, isMaxBytesError(readErr), "expected a *http.MaxBytesError, got %v", readErr)
+	})
+}

pkg/http/middleware/mcp_parse.go

@@ -54,6 +54,10 @@ func WithMCPParse() func(http.Handler) http.Handler {
 			// Read the request body
 			body, err := io.ReadAll(r.Body)
 			if err != nil {
+				if isMaxBytesError(err) {
+					writeRequestTooLarge(w)
+					return
+				}
 				// Log but continue - don't block requests on parse errors
 				next.ServeHTTP(w, r)
 				return