From 4ea82c129111ae5a2a4fbe39fd4edf8449b72100 Mon Sep 17 00:00:00 2001 From: "MD. MOHIBUR RAHMAN" <35300157+mrpmohiburrahman@users.noreply.github.com> Date: Thu, 20 Aug 2026 08:58:56 +0600 Subject: [PATCH] fix(server): enforce Mcp-Param-* parity for numbers the header codec cannot represent --- .../mcp-param-unsafe-integer-header-check.md | 14 ++++++ .../src/shared/mcpParamHeaders.ts | 22 +++++---- .../test/shared/mcpParamHeaders.test.ts | 45 +++++++++++++++++++ .../test/server/mcpParamValidation.test.ts | 34 ++++++++++++++ 4 files changed, 107 insertions(+), 8 deletions(-) create mode 100644 .changeset/mcp-param-unsafe-integer-header-check.md diff --git a/.changeset/mcp-param-unsafe-integer-header-check.md b/.changeset/mcp-param-unsafe-integer-header-check.md new file mode 100644 index 0000000000..09f91d944f --- /dev/null +++ b/.changeset/mcp-param-unsafe-integer-header-check.md @@ -0,0 +1,14 @@ +--- +'@modelcontextprotocol/core-internal': patch +'@modelcontextprotocol/server': patch +--- + +Reject a `tools/call` whose `x-mcp-header`-annotated numeric argument is one the header codec cannot represent. `validateMcpParamHeaders` converts each annotated body value to its header string first, and treated a `undefined` conversion as "the body carries a non-primitive, so params validation owns this fault" — skipping the missing-header, invalid-encoding and value-comparison checks for that declaration entirely. But the codec also returns `undefined` for a number it cannot represent: a non-finite value, or an integer outside ±(2^53−1). Both are reachable over the wire, because `JSON.parse('{"a":9007199254740993,"b":1e400}')` yields `9007199254740992` and `Infinity`. + +Nothing downstream owned those values. An unsafe integer is a valid JSON Schema `integer` — Ajv's integer check has no safe-range bound, and `zod`'s `z.number()` accepts it — so a `tools/call` sending one while omitting the required `Mcp-Param-*` header returned `200` and ran the tool handler, instead of the spec's pre-dispatch rejection. A flatly contradictory header (body `9007199254740992`, header `1`) was swallowed the same way. + +Such a value now falls through to the ordinary header checks and is refused `400 Bad Request` with JSON-RPC `-32020` (`HeaderMismatch`) before dispatch, under the existing `param-header-missing` / `param-header-mismatch` cells — this is the spec's "client omits the header but the value is in the body → server MUST reject" row, which the codec's `undefined` was accidentally masking, not a new condition. + +Two behaviours are deliberately unchanged. An unsafe integer whose header matches numerically (`Mcp-Param-N: 9007199254740992`) is still accepted: header/body parity holds, and the spec's safe-range MUST is client-side and definition-scoped, so policing the value itself here would overreach. A number body against a `type: 'string'` declaration still skips the header check, because that is a genuine body-vs-schema fault `-32602` owns. + +This SDK's own client omits the header for exactly these values (`buildMcpParamHeaders`), so a client sending an unsafe integer for an annotated parameter now fails fast: `callTool` treats the `HEADER_MISMATCH` as a stale-schema miss, refetches `tools/list` and retries once, then rethrows. diff --git a/packages/core-internal/src/shared/mcpParamHeaders.ts b/packages/core-internal/src/shared/mcpParamHeaders.ts index 493cf50aeb..f8c148ebc8 100644 --- a/packages/core-internal/src/shared/mcpParamHeaders.ts +++ b/packages/core-internal/src/shared/mcpParamHeaders.ts @@ -352,16 +352,21 @@ export function validateMcpParamHeaders( continue; } const bodyString = mcpParamPrimitiveToString(bodyRaw); - if (bodyString === undefined) { - // Body carries a non-primitive where the schema declares one; - // params validation owns that fault. Skip the header check. + const numericBody = typeof bodyRaw === 'number' && (decl.type === 'integer' || decl.type === 'number'); + // The codec also refuses a number it cannot represent (non-finite, or outside the safe + // integer range). That is still a valid JSON Schema `integer`/`number`, so params + // validation never faults it and skipping would disable the check entirely. A genuine + // non-primitive keeps skipping — `-32602` owns that. + if (bodyString === undefined && !numericBody) { continue; } + // `JSON.stringify(Infinity)` is `'null'` — the one value these checks let through. + const bodyForMessage = typeof bodyRaw === 'number' && !Number.isFinite(bodyRaw) ? String(bodyRaw) : JSON.stringify(bodyRaw); if (headerValue === null) { return paramHeaderMismatchRejection( 'param-header-missing', headerKey, - `the body carries ${pathName(decl.path)}=${JSON.stringify(bodyRaw)} but the ${headerKey} header is absent` + `the body carries ${pathName(decl.path)}=${bodyForMessage} but the ${headerKey} header is absent` ); } const decoded = decodeMcpParamValue(headerValue); @@ -382,14 +387,15 @@ export function validateMcpParamHeaders( // body-vs-schema fault that params validation owns; fall back to // string comparison and let dispatch emit `-32602` instead so an // identical non-numeric pair never reports a mismatch. - const numericComparable = - (decl.type === 'integer' || decl.type === 'number') && CANONICAL_DECIMAL.test(decoded) && typeof bodyRaw === 'number'; - const equal = numericComparable ? Number(decoded) === bodyRaw : decoded === bodyString; + const numericComparable = numericBody && CANONICAL_DECIMAL.test(decoded); + // `bodyString` is undefined only for a refused number; fall back to `String(bodyRaw)` so an + // agreeing pair still matches when its spelling is not plain decimal (`1e21` → `'1e+21'`). + const equal = numericComparable ? Number(decoded) === bodyRaw : decoded === (bodyString ?? String(bodyRaw)); if (!equal) { return paramHeaderMismatchRejection( 'param-header-mismatch', headerKey, - `the ${headerKey} header decodes to ${JSON.stringify(decoded)} but the body carries ${pathName(decl.path)}=${JSON.stringify(bodyRaw)}` + `the ${headerKey} header decodes to ${JSON.stringify(decoded)} but the body carries ${pathName(decl.path)}=${bodyForMessage}` ); } } diff --git a/packages/core-internal/test/shared/mcpParamHeaders.test.ts b/packages/core-internal/test/shared/mcpParamHeaders.test.ts index 13d8ea582f..3a070c4fd8 100644 --- a/packages/core-internal/test/shared/mcpParamHeaders.test.ts +++ b/packages/core-internal/test/shared/mcpParamHeaders.test.ts @@ -248,6 +248,7 @@ describe('buildMcpParamHeaders', () => { describe('validateMcpParamHeaders — server-behavior table', () => { const DECLS = [{ path: ['region'], headerName: 'Region', type: 'string' }] as const; + const COUNT = [{ path: ['count'], headerName: 'Count', type: 'integer' }] as const; test('header present and matching → ok', () => { const headers = new Headers({ [`${MCP_PARAM_HEADER_PREFIX}Region`]: 'us-west1' }); @@ -272,6 +273,50 @@ describe('validateMcpParamHeaders — server-behavior table', () => { expect(r).toMatchObject({ kind: 'reject', httpStatus: 400, code: HEADER_MISMATCH_ERROR_CODE, cell: 'param-header-missing' }); }); + // sep-2243-server-validate-param-match — globally-untested manifest check, covered here. + // A number the codec refuses is still a valid JSON Schema `integer`, so params validation + // never faults it; the header rung stays responsible for these rows. + test('unsafe-integer body but the header is absent → reject 400/-32020', () => { + const r = validateMcpParamHeaders(COUNT, { count: 2 ** 53 }, new Headers()); + expect(r).toMatchObject({ kind: 'reject', httpStatus: 400, code: HEADER_MISMATCH_ERROR_CODE, cell: 'param-header-missing' }); + }); + + test('non-finite body but the header is absent → reject 400/-32020', () => { + const r = validateMcpParamHeaders(COUNT, { count: Number.POSITIVE_INFINITY }, new Headers()); + expect(r).toMatchObject({ kind: 'reject', httpStatus: 400, code: HEADER_MISMATCH_ERROR_CODE, cell: 'param-header-missing' }); + }); + + test('unsafe-integer body with a disagreeing header → reject 400/-32020', () => { + const r = validateMcpParamHeaders(COUNT, { count: 2 ** 53 }, new Headers({ [`${MCP_PARAM_HEADER_PREFIX}Count`]: '1' })); + expect(r).toMatchObject({ kind: 'reject', httpStatus: 400, code: HEADER_MISMATCH_ERROR_CODE, cell: 'param-header-mismatch' }); + }); + + test('unsafe-integer body whose header matches numerically → ok', () => { + const headers = new Headers({ [`${MCP_PARAM_HEADER_PREFIX}Count`]: '9007199254740992' }); + expect(validateMcpParamHeaders(COUNT, { count: 2 ** 53 }, headers)).toBeUndefined(); + }); + + // `String(1e21)` is `'1e+21'`, which the canonical-decimal gate refuses — so this agreeing + // pair takes the string branch and would otherwise be reported as disagreeing with itself. + test('unsafe-integer body whose header mirrors the exponent spelling → ok', () => { + const headers = new Headers({ [`${MCP_PARAM_HEADER_PREFIX}Count`]: '1e+21' }); + expect(validateMcpParamHeaders(COUNT, { count: 1e21 }, headers)).toBeUndefined(); + }); + + test('unsafe-integer body whose header spells the same value in full decimal → ok', () => { + const headers = new Headers({ [`${MCP_PARAM_HEADER_PREFIX}Count`]: `1${'0'.repeat(21)}` }); + expect(validateMcpParamHeaders(COUNT, { count: 1e21 }, headers)).toBeUndefined(); + }); + + test('non-finite body is named in the rejection message rather than rendered as null', () => { + const r = validateMcpParamHeaders(COUNT, { count: Number.POSITIVE_INFINITY }, new Headers()); + expect(r?.message).toContain('count=Infinity'); + }); + + test('a number body on a string-typed declaration still defers to params validation', () => { + expect(validateMcpParamHeaders(DECLS, { region: 2 ** 53 }, new Headers())).toBeUndefined(); + }); + test('header present but disagreeing → reject 400/-32020 with the mismatch in data', () => { const r = validateMcpParamHeaders(DECLS, { region: 'us-west1' }, new Headers({ [`${MCP_PARAM_HEADER_PREFIX}Region`]: 'eu' })); expect(r).toMatchObject({ diff --git a/packages/server/test/server/mcpParamValidation.test.ts b/packages/server/test/server/mcpParamValidation.test.ts index 531edf05b0..64147c51c4 100644 --- a/packages/server/test/server/mcpParamValidation.test.ts +++ b/packages/server/test/server/mcpParamValidation.test.ts @@ -34,6 +34,11 @@ const REGION_INPUT_SCHEMA = { properties: { region: { type: 'string', 'x-mcp-header': 'Region' }, query: { type: 'string' } } } as const; +const COUNT_INPUT_SCHEMA = { + type: 'object', + properties: { n: { type: 'integer', 'x-mcp-header': 'N' } } +} as const; + function makeFactory(): () => McpServer { return () => { const s = new McpServer({ name: 'param-server', version: '1.0.0' }); @@ -100,6 +105,35 @@ describe('SEP-2243 Mcp-Param-* server validation (createMcpHandler, modern era)' expect(body.error.code).toBe(-32_020); }); + // Nothing downstream faults an unsafe integer, so the rung must reject the missing header + // itself. The spy proves it happened before dispatch; the status alone would not. + it('an unsafe-integer body with no header is rejected 400/-32020 before the handler runs', async () => { + const toolHandler = vi.fn(async () => ({ content: [{ type: 'text' as const, text: 'ran' }] })); + const mcp = createMcpHandler(() => { + const s = new McpServer({ name: 'param-server', version: '1.0.0' }); + s.registerTool('count', { inputSchema: fromJsonSchema<{ n?: number }>(COUNT_INPUT_SCHEMA) }, toolHandler); + return s; + }); + // Raw JSON so 2^53 reaches the wire exactly as reported. + const body = `{"jsonrpc":"2.0","id":7,"method":"tools/call","params":{"name":"count","arguments":{"n":9007199254740992},"_meta":${JSON.stringify(ENVELOPE)}}}`; + const response = await mcp.fetch( + new Request('http://localhost/mcp', { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Accept: 'application/json, text/event-stream', + 'mcp-protocol-version': MODERN, + 'mcp-method': 'tools/call', + 'mcp-name': 'count' + }, + body + }) + ); + expect(response.status).toBe(400); + expect(((await response.json()) as { error: { code: number } }).error.code).toBe(-32_020); + expect(toolHandler).not.toHaveBeenCalled(); + }); + // sep-2243-server-not-expect-null (globally-untested manifest check). it('a null/absent body value passes regardless of any stray header', async () => { const handler = createMcpHandler(makeFactory());