Skip to content

fix(core): reject a plain Mcp-Param or Mcp-Name value with invalid characters - #3063

Open
chrikrah wants to merge 1 commit into
modelcontextprotocol:mainfrom
chrikrah:fix/mcp-param-invalid-chars
Open

fix(core): reject a plain Mcp-Param or Mcp-Name value with invalid characters#3063
chrikrah wants to merge 1 commit into
modelcontextprotocol:mainfrom
chrikrah:fix/mcp-param-invalid-chars

Conversation

@chrikrah

@chrikrah chrikrah commented Oct 22, 2026

Copy link
Copy Markdown

The server accepts a plain Mcp-Param-* or Mcp-Name value that holds a character outside 0x20–0x7E and tab, because decodeMcpParamValue (packages/core-internal/src/shared/mcpParamHeaders.ts:274) returns any value without the Base64 sentinel as it is. The 2026-07-28 spec says servers MUST reject such an Mcp-Param-* header. For Mcp-Name, Server Validation lists "a header value contains invalid characters" among the 400 / -32020 failures. Its Value Encoding section says the client must Base64-wrap such a value.

Node's HTTP parser passes a raw 0xE9 byte through as é, so the value reaches createMcpHandler. Before, at c05225a, through http.createServer and a raw socket:

$ npx tsx repro-e2e.mts    # tools/call route {region: "café"}, header "Mcp-Param-Region: caf\xE9"
node parsed mcp-param-region = "café"
HTTP/1.1 200 OK
{"result":{"content":[{"type":"text","text":"routed café"}],...},"jsonrpc":"2.0","id":7}

After:

HTTP/1.1 400 Bad Request
{"jsonrpc":"2.0","error":{"code":-32020,"message":"Bad Request: the request headers and body disagree: the Mcp-Param-Region header carries invalid characters or an invalid Base64 sentinel value",...},"id":7}

The new check uses the same character set as needsBase64 in the encoder. Both server checks get it: Mcp-Param-* and Mcp-Name in validateStandardRequestHeaders. The cost: a client that sends raw Latin-1 in these headers now gets 400 / -32020, not 200. The client in this SDK already wraps such values. v1.x has no SEP-2243 code, so this is for main only. A changeset is included. There is no issue for this.

Verification

# base c05225a, node v22.23.3, vitest 4.1.2
$ vitest run --maxWorkers=4   # packages/core-internal
Tests  1836 passed (1836)   # base
Tests  1838 passed (1838)   # this branch
$ vitest run --maxWorkers=4   # packages/server
Tests  674 passed (674)   # base
Tests  675 passed (675)   # this branch
# both production files reverted, tests kept:
$ vitest run test/shared/mcpParamHeaders.test.ts test/shared/standardHeaderValidation.test.ts   # core-internal
Tests  2 failed | 90 passed (92)   # expected 'café' to be undefined; expected undefined to be defined
$ vitest run test/server/mcpParamValidation.test.ts   # server
Tests  1 failed | 8 passed (9)   # expected 200 to be 400
$ pnpm --filter ./packages/core-internal --filter ./packages/server run lint   # eslint 9.39.4, prettier 3.6.2
All matched files use Prettier code style!
$ pnpm --filter ./packages/core-internal --filter ./packages/server run typecheck   # tsgo 7.0.0-dev.20260327.2
rc 0
$ vitest run scenarios/sep2243.test.ts coverage.test.ts   # test/e2e
Tests  2 passed (2); Tests  6 passed (6)
# not run: test/conformance

@felixweinberger, I kept the *-invalid-encoding cells for this rejection because the wire answer is the same -32020. Do you want a separate *-invalid-characters cell for it?

@chrikrah
fix(core): reject a plain Mcp-Param or Mcp-Name value with invalid ch…
effb6d6 
…aracters

Signed-off-by: Christopher Krah <48338417+chrikrah@users.noreply.github.com>
@chrikrah
chrikrah requested a review from a team as a code owner October 22, 2026 02:55
@changeset-bot

changeset-bot Bot commented Oct 22, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: effb6d6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 6 packages
Name Type
@modelcontextprotocol/core-internal Patch
@modelcontextprotocol/server Patch
@modelcontextprotocol/client Patch
@modelcontextprotocol/codemod Patch
@modelcontextprotocol/core Patch
@modelcontextprotocol/server-legacy Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pkg-pr-new

pkg-pr-new Bot commented Oct 22, 2026

Copy link
Copy Markdown

Open in StackBlitz

@modelcontextprotocol/client

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/client@3063

@modelcontextprotocol/codemod

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/codemod@3063

@modelcontextprotocol/core

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/core@3063

@modelcontextprotocol/server

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server@3063

@modelcontextprotocol/server-legacy

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/server-legacy@3063

@modelcontextprotocol/express

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/express@3063

@modelcontextprotocol/fastify

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/fastify@3063

@modelcontextprotocol/hono

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/hono@3063

@modelcontextprotocol/node

npm i https://pkg.pr.new/modelcontextprotocol/typescript-sdk/@modelcontextprotocol/node@3063

commit: effb6d6

@claude claude Bot added the v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes label Oct 22, 2026
@0xamlab

0xamlab commented Oct 22, 2026

Copy link
Copy Markdown

Verified this against base c05225a plus this branch. On main, decodeMcpParamValue('café') returns 'café' because the non-sentinel path at packages/core-internal/src/shared/mcpParamHeaders.ts:276 hands back any plain value unchecked; this branch gates it at :279. Running pnpm vitest run test/shared/mcpParamHeaders.test.ts test/shared/standardHeaderValidation.test.ts in packages/core-internal gives 92 passed (62 + 30), while the same PR tests against base fail exactly once with "expected 'café' to be undefined". packages/server test/server/mcpParamValidation.test.ts passes 9/9. The interior-tab case ('a\tb') still decodes, matching the 0x09 allowance in the spec.

@arhancanli

arhancanli commented Oct 23, 2026

Copy link
Copy Markdown

Read the diff (not run here). The shared hasOnlyPlainFieldChars between needsBase64 and decodeMcpParamValue is the right way to keep encoder and decoder from drifting, and the repro with a raw 0xE9 byte makes the gap clear.

Two checks I could not see in the tests: (1) 0x7F (DEL) and a C0 control such as 0x01 in the plain value. The predicate rejects them (c < 0x20 || c > 0x7e), but both tests use café, so a regression to a "non-ASCII only" check would still pass. (2) The same Latin-1 byte path through a real http.createServer request: Node hands UTF-8 bytes over as mojibake (café), which also now gets a 400. That seems right to me, but is the -32020 message ("invalid characters or an invalid Base64 sentinel") enough for a client author to tell this case from a bad sentinel, since the cell name is still *-invalid-encoding for both?

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

v2 Ideas, requests and plans for v2 of the SDK which will incorporate major changes and fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@chrikrah @0xamlab @arhancanli