Repository navigation
fix(docs-mcp): Limit request bodies and tool input elements - #1784
Conversation
Bound /mcp and /internal/docs bodies at 64 KiB before JSON parsing, using both the declared Content-Length and a streamed byte count. Configure the MCP SDK's maxToolInputElements at 1,000 (GHSA-46jr-4wx8-v453). Fixes #1601 Co-Authored-By: BYK <126780+BYK@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| new McpServer( | ||
| { | ||
| description: | ||
| "Generates tailored Sentry integration docs by reading docs.sentry.io.", | ||
| name: "sentry-docs", | ||
| version: "1.0.0", | ||
| }, | ||
| { maxToolInputElements: MAX_TOOL_INPUT_ELEMENTS }, | ||
| ), |
There was a problem hiding this comment.
High severity vulnerability may affect your project—review required:
Line 153 lists a dependency (@modelcontextprotocol/server) with a known High severity vulnerability.
ℹ️ Why this matters
Affected versions of @modelcontextprotocol/sdk and @modelcontextprotocol/server are vulnerable to Allocation of Resources Without Limits or Throttling. McpServer places no limit on how many array elements and object keys it validates in tool-call arguments. A single unauthenticated HTTP request within the default 4 MiB body limit can exhaust the Node.js heap during validation and crash the server process (denial of service). After upgrading, also set the opt-in maxToolInputElements McpServer option and lower maxRequestBodySize.
References: GHSA
To resolve this comment:
Check if you expose the McpServer over an HTTP transport (Streamable HTTP, SSE, or createMcpHandler) rather than stdio only and it registers tools that accept input, especially arrays or records.
- If you're affected, upgrade this dependency to at least version 2.3.0 at pnpm-lock.yaml.
- If you're not affected, comment
/fp we don't use this [condition]
💬 Ignore this finding
To ignore this, reply with:
/fp <comment>for false positive/ar <comment>for acceptable risk/other <comment>for all other reasons
You can view more details on this finding in the Semgrep AppSec Platform here.
There was a problem hiding this comment.
This doesn't apply to the Docs MCP Worker. The McpServer here comes from @modelcontextprotocol/sdk, which resolves to 1.32.2 through the workspace override, and this PR sets maxToolInputElements on it and caps request bodies at 64 KiB.
The @modelcontextprotocol/server@2.0.0 lockfile entry comes in through @mastra/core, which pins it to exactly 2.0.0. Mastra only imports it for types (dist/tools/types.d.ts), and none of its runtime JS references it. I bundled the Worker with wrangler deploy --dry-run --outdir to check: the only @modelcontextprotocol package in the bundle's source map is sdk@1.32.2. Upgrading the v2 server package wouldn't change what ships.
Leaving this thread open so a human can triage the finding in Semgrep.
BYK
left a comment
There was a problem hiding this comment.
Can we share this logic/code between the main MCP and this one?
Move the body-size guard into @sentry/toolkit-core/request-body so the hosted MCP server and Docs MCP use the same reader and JSON-RPC 413. Each server keeps its own byte budget.
Move MAX_MCP_REQUEST_BODY_BYTES and MAX_TOOL_INPUT_ELEMENTS into @sentry/toolkit-core/mcp-limits next to the body reader, so the hosted MCP server and Docs MCP use the same 1 MiB and 10,000-element budgets. Generic body-limit tests now live in toolkit-core; Docs MCP keeps only tests that its routes apply the limits.
Main's shared input limits (#1780, #1784) cover what this branch added for the hosted MCP, so they replace its MAX_MCP_* constants in mcp-core and the Code Mode request body limiter. The handler keeps main's boundMcpRequestBody() and only adds registerCodeModeTools(). Definitions are regenerated for submit_feedback_to_sentry_team. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017M6odW3kt5i65zATxfz9RX
Fixes #1601. This is the Docs MCP follow-up to #1780, which did the same hardening for the hosted MCP server (GHSA-46jr-4wx8-v453). Docs MCP already resolves
@modelcontextprotocol/sdk1.32.2 through the workspace override, so no SDK upgrade is needed. Both mitigations are opt-in, and this PR turns them on. It also moves the limits out of the hosted server so both servers share them.Shared limits:
@sentry/toolkit-core/mcp-limits. The new module holds:MAX_MCP_REQUEST_BODY_BYTES(1 MiB) andMAX_TOOL_INPUT_ELEMENTS(10,000), moved here frommcp-cloudflareandmcp-core. Their values and sizing rationale are unchanged from fix(mcp): Limit tool input elements and request body size #1780.readBoundedBody, which reads a body up to the cap. A declaredContent-Lengthover the cap is rejected without reading, and streamed bytes are always counted, so bodies withoutContent-Length(or with a wrong one) are bounded too.boundMcpRequestBodyandmcpPayloadTooLargeResponse, which return the same JSON-RPC 413 shape the SDK uses.The hosted server (
mcp-coreandmcp-cloudflare) now imports these instead of keeping private copies. Its behavior doesn't change, and its existing body-limit and element-cap tests pass unchanged.Docs MCP.
createMcpServerpassesmaxToolInputElements./mcpand/internal/docsbound the body before any JSON parsing. That matters for/mcpbecause@hono/mcpparses the body itself, so the SDK's own transport limit wouldn't apply. The access check still runs first, so callers outside the service binding still get 403. Docs MCP inputs (at most a 2,000-character query plus 40 context strings, slugs, and a stack summary) fit well within the shared limits.Error contracts:
/mcp: 413 with the JSON-RPC error (code: -32000,Payload Too Large: ...)./internal/docs: 413 with{ "error": "Request body must not exceed 1048576 bytes" }. Malformed JSON still gets the existing 400.isErrorresult before the handler runs, so no doctree fetch or model call happens.Tests. The body-limit tests live in toolkit-core: a body at the limit, oversized bodies with and without
Content-Length, a body larger than its declaredContent-Length, the replayed request, and the 413 shape. Docs MCP keeps one test per route to check that the route applies the limits. None of the tests call a model.I didn't add per-field
.max()limits tolibs/features. The element budget already bounds them, and a per-field cap would change the published tool schema.--
View Session in Sentry