Skip to content

Commit 5ddece0

Browse files
JAORMXclaude
JAORMX
and
claude
committed
Cap webhook middleware request body at 1 MB
The validating and mutating webhook middlewares both called io.ReadAll on the inbound HTTP request body with no size cap before forwarding to the configured webhook server. The client side correctly limited the response body via io.LimitReader to MaxResponseSize, but the server side missed the symmetric limit on inbound requests, so the webhook package would buffer arbitrarily large bodies into memory. Wrap r.Body with http.MaxBytesReader at MaxRequestSize (1 MB, symmetric to MaxResponseSize) and return HTTP 413 with a JSON-RPC error envelope when the limit is exceeded. Reject the read before any forwarding. Note: this is the webhook-layer cap. mcp.ParsingMiddleware sits earlier in the proxy chain and currently reads the body unbounded; capping inbound bodies at the MCP parsing layer is tracked separately and is the load-bearing fix against upstream DoS. This change still bounds the webhook package's own re-read buffer and lays the symmetry groundwork. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 9cc163a commit 5ddece0

5 files changed

Lines changed: 194 additions & 2 deletions

File tree

‎pkg/webhook/mutating/middleware.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"bytes"
88
"context"
99
"encoding/json"
10+
"errors"
1011
"fmt"
1112
"io"
1213
"log/slog"
@@ -88,9 +89,16 @@ func createMutatingHandler(executors []clientExecutor, serverName, transport str
8889
return
8990
}
9091

91-
// Read the request body to get the raw MCP request.
92+
// Read the request body to get the raw MCP request, capped at MaxRequestSize
93+
// to prevent unbounded memory consumption from oversized inbound requests.
94+
r.Body = http.MaxBytesReader(w, r.Body, webhook.MaxRequestSize)
9295
bodyBytes, err := io.ReadAll(r.Body)
9396
if err != nil {
97+
var maxErr *http.MaxBytesError
98+
if errors.As(err, &maxErr) {
99+
sendErrorResponse(w, http.StatusRequestEntityTooLarge, "Request body exceeds maximum size", parsedMCP.ID)
100+
return
101+
}
94102
sendErrorResponse(w, http.StatusInternalServerError, "Failed to read request body", parsedMCP.ID)
95103
return
96104
}

‎pkg/webhook/mutating/middleware_test.go‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -405,6 +405,81 @@ func TestMutatingMiddleware_SkipNonMCPRequests(t *testing.T) {
405405
assert.Equal(t, http.StatusOK, rr.Code)
406406
}
407407

408+
// makeJSONBodyOfSize builds a syntactically-valid JSON body of exactly `size`
409+
// bytes by padding the value of a "data" field with ASCII characters. The
410+
// resulting bytes are valid JSON-RPC for use as an MCP request body.
411+
func makeJSONBodyOfSize(tb testing.TB, size int) []byte {
412+
tb.Helper()
413+
const envelope = `{"jsonrpc":"2.0","method":"tools/call","id":1,"data":""}`
414+
if size < len(envelope) {
415+
tb.Fatalf("requested size %d is smaller than minimum envelope size %d", size, len(envelope))
416+
}
417+
padding := bytes.Repeat([]byte("a"), size-len(envelope))
418+
body := []byte(`{"jsonrpc":"2.0","method":"tools/call","id":1,"data":"`)
419+
body = append(body, padding...)
420+
body = append(body, []byte(`"}`)...)
421+
if len(body) != size {
422+
tb.Fatalf("constructed body length %d != requested size %d", len(body), size)
423+
}
424+
return body
425+
}
426+
427+
func TestMutatingMiddleware_RequestBodySizeLimit(t *testing.T) {
428+
t.Parallel()
429+
430+
t.Run("body exceeding MaxRequestSize returns 413", func(t *testing.T) {
431+
t.Parallel()
432+
433+
cfg := makeConfig(closedServerURL, webhook.FailurePolicyIgnore)
434+
mw := createMutatingHandler(makeExecutors(t, []webhook.Config{cfg}), "srv", "stdio")
435+
436+
body := makeJSONBodyOfSize(t, webhook.MaxRequestSize+1)
437+
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body))
438+
ctx := context.WithValue(req.Context(), mcp.MCPRequestContextKey, &mcp.ParsedMCPRequest{Method: "tools/call", ID: 1})
439+
req = req.WithContext(ctx)
440+
441+
var nextCalled bool
442+
nextHandler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { nextCalled = true })
443+
444+
rr := httptest.NewRecorder()
445+
mw(nextHandler).ServeHTTP(rr, req)
446+
447+
assert.False(t, nextCalled, "next must not be called for oversized requests")
448+
assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
449+
450+
var errResp map[string]interface{}
451+
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &errResp))
452+
errObj, ok := errResp["Error"].(map[string]interface{})
453+
require.True(t, ok)
454+
assert.Equal(t, float64(http.StatusRequestEntityTooLarge), errObj["code"])
455+
assert.Equal(t, "Request body exceeds maximum size", errObj["message"])
456+
})
457+
458+
t.Run("body at MaxRequestSize boundary is accepted", func(t *testing.T) {
459+
t.Parallel()
460+
461+
// Use FailurePolicyIgnore against a closed port: the outbound webhook
462+
// call fails and is ignored per fail-open, so next is invoked. This
463+
// isolates the test from depending on a working webhook server.
464+
cfg := makeConfig(closedServerURL, webhook.FailurePolicyIgnore)
465+
mw := createMutatingHandler(makeExecutors(t, []webhook.Config{cfg}), "srv", "stdio")
466+
467+
body := makeJSONBodyOfSize(t, webhook.MaxRequestSize)
468+
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body))
469+
ctx := context.WithValue(req.Context(), mcp.MCPRequestContextKey, &mcp.ParsedMCPRequest{Method: "tools/call", ID: 1})
470+
req = req.WithContext(ctx)
471+
472+
var nextCalled bool
473+
nextHandler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { nextCalled = true })
474+
475+
rr := httptest.NewRecorder()
476+
mw(nextHandler).ServeHTTP(rr, req)
477+
478+
assert.True(t, nextCalled, "next should be called for boundary-size body (fail-open ignores webhook error)")
479+
assert.Equal(t, http.StatusOK, rr.Code)
480+
})
481+
}
482+
408483
func TestMiddlewareParams_Validate(t *testing.T) {
409484
t.Parallel()
410485
tests := []struct {

‎pkg/webhook/types.go‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ const MinTimeout = 1 * time.Second
3030
// MaxResponseSize is the maximum allowed size in bytes for webhook responses (1 MB).
3131
const MaxResponseSize = 1 << 20
3232

33+
// MaxRequestSize is the maximum allowed size in bytes for inbound webhook
34+
// middleware request bodies (1 MB). Requests exceeding this size are
35+
// rejected with HTTP 413 before the body is buffered or forwarded.
36+
const MaxRequestSize = 1 << 20
37+
3338
// Type indicates whether a webhook is validating or mutating.
3439
type Type string
3540

‎pkg/webhook/validating/middleware.go‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"bytes"
88
"context"
99
"encoding/json"
10+
"errors"
1011
"fmt"
1112
"io"
1213
"log/slog"
@@ -97,9 +98,16 @@ func createValidatingHandler(executors []clientExecutor, serverName, transport s
9798
return
9899
}
99100

100-
// Read the request body to get the raw MCP request
101+
// Read the request body to get the raw MCP request, capped at MaxRequestSize
102+
// to prevent unbounded memory consumption from oversized inbound requests.
103+
r.Body = http.MaxBytesReader(w, r.Body, webhook.MaxRequestSize)
101104
bodyBytes, err := io.ReadAll(r.Body)
102105
if err != nil {
106+
var maxErr *http.MaxBytesError
107+
if errors.As(err, &maxErr) {
108+
sendErrorResponse(w, http.StatusRequestEntityTooLarge, "Request body exceeds maximum size", parsedMCP.ID)
109+
return
110+
}
103111
sendErrorResponse(w, http.StatusInternalServerError, "Failed to read request body", parsedMCP.ID)
104112
return
105113
}

‎pkg/webhook/validating/middleware_test.go‎

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,102 @@ func TestValidatingMiddleware(t *testing.T) {
270270
})
271271
}
272272

273+
// makeJSONBodyOfSize builds a syntactically-valid JSON body of exactly `size`
274+
// bytes by padding the value of a "data" field with ASCII characters. The
275+
// resulting bytes are valid JSON-RPC for use as an MCP request body.
276+
func makeJSONBodyOfSize(tb testing.TB, size int) []byte {
277+
tb.Helper()
278+
// Envelope structure: {"jsonrpc":"2.0","method":"tools/call","id":1,"data":"<padding>"}
279+
const envelope = `{"jsonrpc":"2.0","method":"tools/call","id":1,"data":""}`
280+
if size < len(envelope) {
281+
tb.Fatalf("requested size %d is smaller than minimum envelope size %d", size, len(envelope))
282+
}
283+
padding := bytes.Repeat([]byte("a"), size-len(envelope))
284+
body := []byte(`{"jsonrpc":"2.0","method":"tools/call","id":1,"data":"`)
285+
body = append(body, padding...)
286+
body = append(body, []byte(`"}`)...)
287+
if len(body) != size {
288+
tb.Fatalf("constructed body length %d != requested size %d", len(body), size)
289+
}
290+
return body
291+
}
292+
293+
func TestValidatingMiddleware_RequestBodySizeLimit(t *testing.T) {
294+
t.Parallel()
295+
296+
t.Run("body exceeding MaxRequestSize returns 413", func(t *testing.T) {
297+
t.Parallel()
298+
299+
// Use fail-open so that even if the middleware reached the webhook call
300+
// (it must NOT, since the size check fires first), the test would still
301+
// be unambiguous about the size rejection vs. a downstream error.
302+
cfg := webhook.Config{
303+
Name: "test-webhook",
304+
URL: closedServerURL,
305+
Timeout: webhook.DefaultTimeout,
306+
FailurePolicy: webhook.FailurePolicyIgnore,
307+
TLSConfig: &webhook.TLSConfig{InsecureSkipVerify: true},
308+
}
309+
client, err := webhook.NewClient(cfg, webhook.TypeValidating, nil)
310+
require.NoError(t, err)
311+
mw := createValidatingHandler([]clientExecutor{{client: client, config: cfg}}, "srv", "stdio")
312+
313+
body := makeJSONBodyOfSize(t, webhook.MaxRequestSize+1)
314+
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body))
315+
ctx := context.WithValue(req.Context(), mcp.MCPRequestContextKey, &mcp.ParsedMCPRequest{Method: "tools/call", ID: 1})
316+
req = req.WithContext(ctx)
317+
318+
var nextCalled bool
319+
nextHandler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { nextCalled = true })
320+
321+
rr := httptest.NewRecorder()
322+
mw(nextHandler).ServeHTTP(rr, req)
323+
324+
assert.False(t, nextCalled, "next must not be called for oversized requests")
325+
assert.Equal(t, http.StatusRequestEntityTooLarge, rr.Code)
326+
327+
// The error response is a JSON-RPC envelope.
328+
var errResp map[string]interface{}
329+
require.NoError(t, json.Unmarshal(rr.Body.Bytes(), &errResp))
330+
errObj, ok := errResp["Error"].(map[string]interface{})
331+
require.True(t, ok)
332+
assert.Equal(t, float64(http.StatusRequestEntityTooLarge), errObj["code"])
333+
assert.Equal(t, "Request body exceeds maximum size", errObj["message"])
334+
})
335+
336+
t.Run("body at MaxRequestSize boundary is accepted", func(t *testing.T) {
337+
t.Parallel()
338+
339+
// Point at a closed port with FailurePolicyIgnore so that the outbound
340+
// webhook call fails and is ignored per fail-open. This isolates the
341+
// test from depending on a working webhook server.
342+
cfg := webhook.Config{
343+
Name: "test-webhook",
344+
URL: closedServerURL,
345+
Timeout: webhook.DefaultTimeout,
346+
FailurePolicy: webhook.FailurePolicyIgnore,
347+
TLSConfig: &webhook.TLSConfig{InsecureSkipVerify: true},
348+
}
349+
client, err := webhook.NewClient(cfg, webhook.TypeValidating, nil)
350+
require.NoError(t, err)
351+
mw := createValidatingHandler([]clientExecutor{{client: client, config: cfg}}, "srv", "stdio")
352+
353+
body := makeJSONBodyOfSize(t, webhook.MaxRequestSize)
354+
req := httptest.NewRequest(http.MethodPost, "/", bytes.NewReader(body))
355+
ctx := context.WithValue(req.Context(), mcp.MCPRequestContextKey, &mcp.ParsedMCPRequest{Method: "tools/call", ID: 1})
356+
req = req.WithContext(ctx)
357+
358+
var nextCalled bool
359+
nextHandler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { nextCalled = true })
360+
361+
rr := httptest.NewRecorder()
362+
mw(nextHandler).ServeHTTP(rr, req)
363+
364+
assert.True(t, nextCalled, "next should be called for boundary-size body (fail-open ignores webhook error)")
365+
assert.Equal(t, http.StatusOK, rr.Code)
366+
})
367+
}
368+
273369
func TestMiddlewareParams_Validate(t *testing.T) {
274370
t.Parallel()
275371
tests := []struct {

0 commit comments

Comments
 (0)