Limit HTTP request bodies before MCP middleware parsing

Add WithMaxBodySize middleware that bounds the request body via
http.MaxBytesReader (with a fast Content-Length rejection when known),
registered first in RegisterMiddleware so it runs before any other
middleware or the MCP SDK reads or buffers the body.

WithMCPParse and WithScopeChallenge now return a clear 413 "request
body too large" response when their body read hits the limit, instead
of silently continuing.

Defaults to 10 MiB, overridable via ServerConfig.MaxRequestBodyBytes.

Fixes #3102

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Author: SamMorrowDrums

pkg/http/handler.go

@@ -132,6 +132,9 @@ func NewHTTPMcpHandler(
 
 func (h *Handler) RegisterMiddleware(r chi.Router) {
 	r.Use(
+		// Must run first: bounds the request body before any other
+		// middleware (or the MCP SDK) reads or buffers it.
+		middleware.WithMaxBodySize(h.maxRequestBodyBytes()),
 		middleware.ExtractUserToken(h.oauthCfg),
 		middleware.WithRequestConfig,
 		middleware.WithMCPParse(),
@@ -143,6 +146,15 @@ func (h *Handler) RegisterMiddleware(r chi.Router) {
 	}
 }
 
+// maxRequestBodyBytes returns the configured request-body size limit, or
+// middleware.DefaultMaxRequestBodyBytes if none was configured.
+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) {

pkg/http/handler_test.go

@@ -1287,3 +1287,87 @@ 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")
 }
+
+// TestRegisterMiddleware_MaxRequestBodySize verifies that RegisterMiddleware
+// wires the body-size limit ahead of the body-consuming middleware, so an
+// oversized request never reaches the MCP server, and that requests within
+// the configured limit (including exactly at the boundary) still succeed.
+func TestRegisterMiddleware_MaxRequestBodySize(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, mcpServerFactoryCalled *bool) http.Handler {
+		t.Helper()
+		handler := NewHTTPMcpHandler(
+			context.Background(),
+			&ServerConfig{Version: "test", MaxRequestBodyBytes: limit},
+			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{}),
+		)
+
+		r := chi.NewRouter()
+		handler.RegisterMiddleware(r)
+		handler.RegisterRoutes(r)
+		return r
+	}
+
+	t.Run("oversized request is rejected before reaching the MCP server", func(t *testing.T) {
+		var mcpServerFactoryCalled bool
+		r := newHandler(t, &mcpServerFactoryCalled)
+
+		body := buildBody(limit + 1)
+		require.Greater(t, len(body), limit)
+
+		req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(body))
+		req.Header.Set(headers.AuthorizationHeader, strings.Join([]string{"ghs", "test-token"}, "_"))
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, req)
+
+		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("boundary-size request at the configured limit succeeds", func(t *testing.T) {
+		var mcpServerFactoryCalled bool
+		r := newHandler(t, &mcpServerFactoryCalled)
+
+		body := buildBody(limit)
+		require.Len(t, body, limit)
+
+		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"}, "_"))
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, req)
+
+		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,54 @@
+package middleware
+
+import (
+	"errors"
+	"net/http"
+)
+
+// DefaultMaxRequestBodyBytes bounds the size of HTTP request bodies accepted
+// by the MCP endpoints when no explicit limit is configured.
+const DefaultMaxRequestBodyBytes int64 = 10 << 20 // 10 MiB
+
+// WithMaxBodySize returns middleware that bounds the size of the request
+// body. It must be registered before any middleware that reads or buffers
+// the body (e.g. WithMCPParse, WithScopeChallenge) so that an oversized
+// payload is rejected before it is ever fully buffered in memory, rather than
+// relying on a size guard applied later by the MCP SDK or a downstream
+// handler.
+//
+// When Content-Length is known and already exceeds maxBytes, the request is
+// rejected immediately without touching the body. Otherwise the body is
+// wrapped with http.MaxBytesReader, so any subsequent read (including
+// chunked or unknown-length bodies) fails with a *http.MaxBytesError once
+// maxBytes have been consumed.
+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)
+		})
+	}
+}
+
+// writeRequestTooLarge writes the standard "request body too large" response.
+// Every middleware that reads the request body should use this so oversized
+// requests get a consistent, clear response regardless of which layer
+// detects the overflow.
+func writeRequestTooLarge(w http.ResponseWriter) {
+	http.Error(w, "request body too large", http.StatusRequestEntityTooLarge)
+}
+
+// isMaxBytesError reports whether err resulted from a body exceeding the
+// limit applied by WithMaxBodySize, as opposed to some other read failure.
+func isMaxBytesError(err error) bool {
+	var maxBytesErr *http.MaxBytesError
+	return errors.As(err, &maxBytesErr)
+}

pkg/http/middleware/body_limit_test.go

@@ -0,0 +1,115 @@
+package middleware
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// unknownLengthBody wraps a reader without exposing a Len method, so
+// httptest.NewRequest cannot infer Content-Length from it. This mirrors a
+// chunked-transfer-encoded request, where the body size is unknown upfront.
+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

pkg/http/middleware/mcp_parse_test.go

@@ -189,3 +189,68 @@ func TestWithMCPParse_BodyRestoration(t *testing.T) {
 
 	assert.Equal(t, originalBody, capturedBody, "body should be restored for downstream handlers")
 }
+
+// TestWithMCPParse_WithMaxBodySize composes the body-size limit with
+// WithMCPParse, mirroring the production middleware ordering where
+// WithMaxBodySize runs first. It verifies that an oversized body is rejected
+// with a clear 413 before parsing runs, while requests within the limit
+// (including exactly at the boundary) still parse and preserve the body.
+func TestWithMCPParse_WithMaxBodySize(t *testing.T) {
+	const limit = 128
+
+	buildBody := func(size int) string {
+		payload := `{"jsonrpc":"2.0","method":"tools/call","params":{"name":"test_tool","arguments":{"pad":"PADDING"}}}`
+		if len(payload) >= size {
+			return payload
+		}
+		// Pad the JSON with a longer string value so we can hit an exact byte size.
+		pad := strings.Repeat("x", size-len(payload))
+		return strings.Replace(payload, "PADDING", "PADDING"+pad, 1)
+	}
+
+	t.Run("oversized body is rejected before parsing", func(t *testing.T) {
+		body := buildBody(limit + 1)
+		require.Greater(t, len(body), limit)
+
+		var nextCalled bool
+		nextHandler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) {
+			nextCalled = true
+		})
+
+		handler := WithMaxBodySize(limit)(WithMCPParse()(nextHandler))
+
+		req := httptest.NewRequest(http.MethodPost, "/mcp", strings.NewReader(body))
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+
+		assert.False(t, nextCalled, "downstream 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("boundary-size body is parsed and preserved", func(t *testing.T) {
+		body := buildBody(limit)
+		require.Len(t, body, limit)
+
+		var capturedInfo *ghcontext.MCPMethodInfo
+		var capturedBody string
+		nextHandler := http.HandlerFunc(func(_ http.ResponseWriter, r *http.Request) {
+			capturedInfo, _ = ghcontext.MCPMethod(r.Context())
+			b, err := io.ReadAll(r.Body)
+			require.NoError(t, err)
+			capturedBody = string(b)
+		})
+
+		handler := WithMaxBodySize(limit)(WithMCPParse()(nextHandler))
+
+		req := httptest.NewRequest(http.MethodPost, "/mcp", strings.NewReader(body))
+		rr := httptest.NewRecorder()
+		handler.ServeHTTP(rr, req)
+
+		assert.Equal(t, http.StatusOK, rr.Code)
+		require.NotNil(t, capturedInfo, "MCPMethodInfo should be parsed for an allowed request")
+		assert.Equal(t, "tools/call", capturedInfo.Method)
+		assert.Equal(t, "test_tool", capturedInfo.ItemName)
+		assert.Equal(t, body, capturedBody, "body should be preserved for downstream handlers")
+	})
+}

pkg/http/middleware/scope_challenge.go

@@ -54,6 +54,10 @@ func WithScopeChallenge(oauthCfg *oauth.Config, scopeFetcher scopes.FetcherInter
 				// Fallback: parse the request body directly
 				body, err := io.ReadAll(r.Body)
 				if err != nil {
+					if isMaxBytesError(err) {
+						writeRequestTooLarge(w)
+						return
+					}
 					next.ServeHTTP(w, r)
 					return
 				}