Enforce request body size limits on proxies and vMCP

[ChrisJBurns]

Summary

ToolHive's 1MB request-body cap (and 413 handling) existed only on the management API server, so the MCP proxies and the vMCP server could be driven to memory exhaustion by large or unbounded request bodies via io.ReadAll (pkg/mcp/parser.go, the three proxy handlers, vMCP) was unbounded.

This PR extracts the existing, trusted body-limit logic into a reusable pkg/bodylimit package and applies it to all inbound listeners, rejecting oversized bodies with 413 before any handler buffers them. The default cap is 1MB; making it configurable (CLI flag / RunConfig / operator CRD) is intentionally deferred to a follow-up PR, as is the slow-upload ReadTimeout mitigation (tracked in #5499).

Medium level

• New pkg/bodylimit package: holds the body-limit middleware (Content-Length early-reject + http.MaxBytesReader safety net + 413) moved out of pkg/api/server.go. Exposes a plain net/http middleware (Middleware) for the API server and vMCP, and a runner middleware factory (CreateMiddleware/MiddlewareType) for the proxies. A non-positive limit falls back to the default — zero never means "unlimited".
• Proxies: bodylimit is registered in GetSupportedMiddlewareFactories and prepended as the outermost entry in PopulateMiddlewareConfigs, so it runs before auth and the MCP parser for all three transports (streamable, httpsse, transparent) via the shared chain.
• vMCP: the same middleware is applied in Handler() inside recovery, before the MCP parser. The response-writer wrapper gained Unwrap()/Flush() so wrapping does not break long-lived SSE streams (their write-deadline clearing goes through http.ResponseController).
• API server: now consumes pkg/bodylimit with no behavior change.

Low level

File	Change
pkg/bodylimit/middleware.go	New package: Middleware, CreateMiddleware, MiddlewareType, DefaultMaxRequestBodySize (1MB), MiddlewareParams, and the moved maxBytesTracker/bodySizeResponseWriter helpers (now with Unwrap/Flush for SSE safety).
pkg/api/server.go	Removed the local middleware + helper types; imports and calls bodylimit.Middleware.
pkg/api/request_size_test.go	Deleted; migrated to pkg/bodylimit/middleware_test.go.
pkg/runner/middleware.go	Register bodylimit factory; prepend it first (outermost) in PopulateMiddlewareConfigs.
pkg/vmcp/server/server.go	Wrap the MCP handler with bodylimit.Middleware inside recovery.
pkg/bodylimit/middleware_test.go, pkg/runner/middleware_test.go, pkg/vmcp/server/body_limit_test.go, pkg/transport/proxy/streamable/body_limit_test.go	Tests.

Type of change

• Bug fix (non-breaking change which fixes an issue)
• New feature
• Breaking change
• Refactoring
• Documentation
• Other

Test plan

• task test passes (full unit suite, exit 0)
• task build passes
• New unit tests: 413 on oversized body (Content-Length and chunked/no-Content-Length), zero/negative limit → default fallback, CreateMiddleware factory.
• Ordering regression guard: TestHTTPProxy_BodyLimitBoundsDownstreamReader sends a chunked oversized body and asserts a parser-like downstream reader sees <= limit bytes — fails if body-limit is ever moved after the parser.
• TestPopulateMiddlewareConfigs_BodyLimitIsOutermost asserts body-limit is index 0 (before auth/parser).
• vMCP 413 (TestHandler_RejectsOversizedBody) and SSE survival (TestIntegration_SSEGetConnectionSurvivesWriteTimeout still passes with the wrapper in place).

Does this introduce a user-facing change?

Yes. Requests to the MCP proxies and vMCP with a body larger than 1MB are now rejected with 413 Request Entity Too Large (previously accepted and buffered). The limit is not yet configurable — a follow-up PR adds CLI/CRD configuration. Deployments sending legitimately large MCP messages (e.g. big base64 payloads) should be aware of the 1MB default.

Special notes for reviewers

• Chain ordering is the load-bearing correctness point. Execution order is slice order with index 0 outermost — verified empirically (the existing comment on the recovery middleware claiming last-appended is "outermost" is misleading; it is actually innermost). The bounded-downstream-read test is the ground-truth guard.
• Single-transport integration test: the three proxies share the identical PopulateMiddlewareConfigs chain and reverse-apply loop, so the streamable proxy test plus the runner-level ordering test cover the mechanism for all three. Happy to add per-transport tests if reviewers prefer strict parity.
• Out of scope (follow-ups): configurable limit (CLI flag / RunConfig / operator CRD), and the slow-upload ReadTimeout mitigation (tracked in Add request read timeouts to MCP proxy servers #5499) (deferred pending SSE-streaming verification).

Auth-server endpoint coverage: The embedded OAuth/auth-server routes (notably the unauthenticated POST /oauth/token) are mounted outside the MCP middleware chain, so they are capped separately by wrapping EmbeddedAuthServer.Handler() with the same body-limit middleware at a 64KB cap (matching the existing DCR limit). Routes()/RegisterHandlers() derive from that handler, so all three proxies and vMCP inherit the bound. With this, every inbound body-reading endpoint is capped.

Large PR Justification

This PR exceeds 1000 lines mostly because of tests; the production footprint is small and stays within the repo's code-size budget (CONTRIBUTING counts production code only — tests/docs/generated are exempt).

• Multiple related changes that would break if separated — a request-body DoS cap must cover every inbound listener atomically: the three proxy transports (streamable, httpsse, transparent), the vMCP server, and the embedded auth-server endpoints, behind one shared pkg/bodylimit package. Splitting would ship a partially-protected fix that leaves some listeners exposed.
• Refactor that must be atomic — pkg/bodylimit is extracted from pkg/api/server.go; the move and the API server's switch to the shared package must land together to keep the build green. Most of pkg/bodylimit/middleware.go is moved code, not net-new.
• Test-heavy — ~730 of the added lines are tests (unit, per-transport integration, plus SSE-contract and chain-ordering regression guards). Production code is ~258 added / ~100 removed across 8 files, within the 400-line / 10-file limit.
• The PR also grew while addressing two rounds of review feedback

[codecov]

Codecov Report

❌ Patch coverage is 89.10891% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.63%. Comparing base (2ccfae9) to head (7d8b3f7).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/runner/middleware.go	69.23%	2 Missing and 2 partials ⚠️
...g/transport/proxy/transparent/transparent_proxy.go	82.35%	2 Missing and 1 partial ⚠️
pkg/runner/runner.go	50.00%	1 Missing and 1 partial ⚠️
pkg/transport/proxy/streamable/streamable_proxy.go	33.33%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5492      +/-   ##
==========================================
+ Coverage   69.51%   69.63%   +0.11%     
==========================================
  Files         639      642       +3     
  Lines       65092    65397     +305     
==========================================
+ Hits        45248    45536     +288     
- Misses      16518    16521       +3     
- Partials     3326     3340      +14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[github-actions]

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.

---

This review will be automatically dismissed once you add the justification section.

[jhrozek]

Stepping back from the line level: this is a solid, worthwhile change. It closes a real unauthenticated memory-exhaustion gap — before this, only the management API capped request bodies, while the three MCP proxy transports, vMCP, thv proxy, and the auth server's non-DCR endpoints all read bodies via io.ReadAll with no bound. Consolidating the logic into pkg/bodylimit (and deleting the pkg/api copy) is a net simplification, the "always present, outermost" invariant is a clean model, and the chunked/no-Content-Length handling (surfacing IsRequestTooLarge as 413 rather than 500 at each io.ReadAll site) shows the bypass case was thought through. The SSE preservation (Unwrap() for SetWriteDeadline, Flush() forwarding) is a subtle correctness detail that's easy to miss and was handled deliberately. Build + tests pass and the per-transport tests exercise the real 413 path.

The one question that I think actually gates the design is the inline comment on the 1 MB cap — flagged below. Everything else is minor and non-blocking; listing here rather than as separate inline threads:

• Duplicate constant (pkg/api/server.go:69): maxRequestBodySize = 1 << 20 now duplicates bodylimit.DefaultMaxRequestBodySize. They agree today but can silently drift; consider referencing the package constant directly. (Coupled to the inline question — if the cap becomes configurable, this value should come from the same source.)
• Positional idempotency guard (pkg/runner/middleware.go:323): the guard checks only middlewares[0]. Safe today since every builder prepends via this helper, but a future builder that appends body-limit at a non-zero index would get a second copy. A membership check (slices.ContainsFunc) would make the "single instance, outermost" contract defensive rather than convention-dependent.
• MCP parser error path (pkg/mcp/parser.go:87, outside this diff): on an io.ReadAll error it forwards to next instead of checking IsRequestTooLarge. End-to-end result is still 413 via the response-writer rewrite, but the path relies on that rewrite rather than handling it explicitly like the proxies now do. Possibly worth a follow-up — not this PR.
• Test readiness wait (pkg/transport/proxy/streamable/body_limit_test.go:27): waitForReady hand-rolls a time.Sleep poll loop where the httpsse sibling uses require.Eventually. Minor consistency nit (your own earlier commit replaced fixed sleeps with readiness waits).

Nothing above blocks merge in my view — the only thing I'd want an explicit answer on is the limit value/configurability.