Skip to content

Relax legacy request metadata validation - #1852

Open
halter73 wants to merge 6 commits into
mainfrom
halter73-fix-metadata-protocol-version
Open

Relax legacy request metadata validation#1852
halter73 wants to merge 6 commits into
mainfrom
halter73-fix-metadata-protocol-version

Conversation

@halter73

@halter73 halter73 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

ChatGPT negotiates 2025-11-25 but sends _meta keys introduced in 2026-07-28. The SDK rejected those requests even though the legacy protocol leaves those reserved values opaque. This change accepts that traffic without using future metadata to change legacy session state.

Protocol metadata

  • Established legacy session or supported legacy header: future reserved _meta values are not parsed, validated, or projected, even when malformed. The authoritative transport version is still validated against server configuration and the established session.
  • initialize: future reserved _meta values remain opaque. An established modern session or a modern transport version rejects initialize with MethodNotFound, preventing a downgrade. A rejected discovery negotiation can still fall back to the legacy handshake.
  • Headerless requests: HTTP no longer rejects a request merely because its body contains a modern protocol version. The core resolves the version from established session state or readable body metadata. Supported modern metadata selects the modern path; unsupported nonlegacy versions are rejected. A legacy metadata version is advisory and does not establish a session version.
  • No established era and unreadable version metadata: treat the value as absent and use legacy handling, with a warning for the ambiguous fallback. Established legacy traffic does not produce that warning. Modern-only servers and server/discover still reject requests that cannot select a modern version.
  • Modern requests: retain strict required-metadata validation, header/body consistency, supported-version checks, and session-version stability. clientCapabilities is required; clientInfo and logLevel remain optional. Malformed metadata cannot establish a new session version.

Server-only metadata parsing moves out of McpSessionHandler into the server's built-in incoming filter, before application filters. The implementation removes blanket reserved-key rejection, redundant era checks, and duplicate protocol-version parsing rather than introducing tolerant deserialization.

Client capabilities and delivery

Request-bound McpServer.ClientCapabilities exposes modern per-request capabilities, including in stateless HTTP mode. Legacy requests continue to use initialized session capabilities, not future _meta values. This avoids capability comparisons between initialization and subsequent requests.

Capability declarations no longer double as a transport-availability sentinel. Sampling, roots, and elicitation remain blocked when the underlying stateless transport cannot deliver server-to-client requests safely. An outgoing-request interceptor provides an alternate delivery channel, so task-store-backed requests remain supported without session affinity, including through AsSamplingChatClient() and generic ElicitAsync<T>().

Logging capability advertisement

Omit deprecated logging from modern server/discover responses while retaining it for legacy initialize responses. Other advertised capabilities are preserved.

Regression coverage includes legacy identity preservation, headerless and malformed metadata, modern-only servers, initialization after modern requests, failed-negotiation fallback, and capability visibility versus stateless delivery safety.

Fixes #1783
Fixes #1774

Note

This pull request description was drafted with Copilot.

halter73 and others added 2 commits August 6, 2026 17:27
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Legacy requests with a malformed _meta/io.modelcontextprotocol/protocolVersion still throw InvalidParams due to unconditional parsing, which contradicts the intended “ignore malformed future metadata on legacy” behavior.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review tier: Lite
Findings: 1 Medium severity

New issues introduced by this change (1)
Severity Finding
Medium severity src/​ModelContextProtocol.Core/​Server/​McpServerImpl.cs — ReadRequestMetadata calls GetProtocolVersionMeta unconditionally, which throws InvalidParams if…
What changed in this PR

This PR relaxes validation of legacy per-request _meta so legacy clients (notably ChatGPT negotiating 2025-11-25) aren’t rejected when they include auxiliary metadata introduced in later protocol eras, while keeping modern (2026-07-28+) protocol/version claims and required metadata strict. It also adjusts server/discover capability advertisement to avoid claiming deprecated logging on modern discover responses.

Changes:

  • Reworks server-side _meta parsing/projection and protocol boundary validation to treat known legacy-era metadata as advisory while enforcing strictness for modern envelopes and version switching.
  • Omits deprecated logging capability from 2026-07-28 server/discover responses while retaining it for legacy initialize.
  • Adds regression coverage across server/client behavior, including legacy session isolation vs per-request metadata and malformed metadata handling.
File Description
tests/​ModelContextProtocol.Tests/​Server/​TaskProtocolGatingTests.cs Updates legacy task-gating expectation to tolerate per-request capability metadata without enabling tasks.
tests/​ModelContextProtocol.Tests/​Server/​NegotiatedProtocolVersionTests.cs Expands coverage for advisory legacy version metadata, legacy session isolation, malformed metadata handling, and modern version switching.
tests/​ModelContextProtocol.Tests/​Server/​McpServerTests.cs Validates legacy transport header remains authoritative over advisory _meta protocol version; asserts legacy initialize includes logging capability.
tests/​ModelContextProtocol.Tests/​Client/​McpClientMetaTests.cs Adds coverage that legacy per-request client metadata does not override initialized session identity/capabilities.
tests/​ModelContextProtocol.AspNetCore.Tests/​RawHttpConformanceTests.cs Adds conformance checks for discover capability suppression and legacy raw POST tolerance of auxiliary per-request metadata.
src/​ModelContextProtocol.Core/​Server/​McpServerImpl.cs Implements the relaxed legacy _meta semantics, strict modern validation, and logging capability suppression on discover.
src/​ModelContextProtocol.Core/​Protocol/​JsonRpcMessageContext.cs Updates documentation to clarify authoritative protocol version semantics and advisory legacy _meta behavior.
src/​ModelContextProtocol.Core/​McpSessionHandler.cs Removes session-handler _meta projection now handled by server pipeline filter logic.
Suppressed comments (1)

src/ModelContextProtocol.Core/Server/McpServerImpl.cs:318

  • ProjectInitializeRequestMetadata also uses GetProtocolVersionMeta, so an initialize request carrying a malformed _meta/io.modelcontextprotocol/protocolVersion (or other invalid value type) will currently fail with InvalidParams. Since initialize is a legacy handshake path and _meta is meant to be extensible there, consider treating a malformed protocolVersion entry as advisory/absent rather than rejecting the request.
        JsonObject? meta = GetRequestMeta(request);
        string? metadataProtocolVersion = GetProtocolVersionMeta(meta, out bool hasProtocolVersionMeta);
        string? transportProtocolVersion = request.Context?.ProtocolVersion;

        ValidateProtocolVersionMatch(transportProtocolVersion, metadataProtocolVersion);

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/ModelContextProtocol.Core/Server/McpServerImpl.cs Outdated
halter73 and others added 2 commits September 3, 2026 17:42
Separate client capability visibility from whether the transport can safely issue server-to-client requests, preserving stateless sampling, roots, and elicitation guards.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Skip parsing reserved per-request metadata once legacy semantics are authoritative, while retaining strict modern and metadata-only validation. Keep modern client capabilities request-scoped and independently gate stateless server-to-client requests.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@tarekgh

tarekgh commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Nit (optional): In ReadRequestMetadata, GetProtocolVersionMeta throws InvalidParams for a malformed (non-string) _meta/io.modelcontextprotocol/protocolVersion before the legacy-vs-modern classification runs, so a legacy request is rejected rather than tolerated. This is a minor inconsistency with the PR's "ignores malformed future metadata only on legacy requests" wording (the tests only cover malformed clientInfo/clientCapabilities/logLevel). It is arguably fine to stay strict since protocolVersion is the reserved key that drives version selection, but a short test pinning the intended behavior either way would be nice.

tarekgh
tarekgh previously approved these changes Sep 4, 2026
An outgoing-request interceptor replaces the session's server-to-client
channel, so it can deliver sampling, roots, and elicitation requests even
when the underlying transport cannot. That is what lets background task
execution park a request in an IMcpTaskStore and have the client answer it
on a later, unrelated request, which does not depend on session affinity.

OutgoingRequestInterceptingMcpServer relied on inheriting the base
SupportsServerToClientRequests default to get this, while forwarding every
other member to the wrapped server. Make the override explicit so the
behavior is stated rather than implied, and add stateless coverage for
AsSamplingChatClient() and the generic ElicitAsync<T>(), which are the two
APIs that check the capability guard eagerly instead of routing through the
interceptor short-circuit.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@halter73

halter73 commented Sep 8, 2026

Copy link
Copy Markdown
Contributor Author

@tarekgh Can you give this another look? Since your approval, I added two things beyond the original fix. _meta is now fully opaque under legacy semantics rather than leniently parsed (which is what deleted most of the code), and McpServer.ClientCapabilities is request-scoped everywhere it can be, with transport delivery gated separately so stateless still blocks sampling/roots/elicitation.

Comment thread src/ModelContextProtocol.Core/Server/McpServerImpl.cs
Comment thread src/ModelContextProtocol.AspNetCore/StreamableHttpHandler.cs
Reject initialize after a modern session is established, while preserving
fallback after a failed discovery probe. Let headerless requests reach the
core era decision and tolerate unreadable metadata when no modern version
is established, with a diagnostic warning for that ambiguous case.

Remove unreachable era checks, redundant helpers, and duplicate version
parsing. Preserve strict modern metadata validation, error precedence,
and delayed version establishment for malformed first requests.

Expand regression coverage for malformed values, authoritative transport
versions, modern-only servers, and warning behavior.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@halter73

halter73 commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

initialize now rejects an established modern session, while failed discovery negotiation can still fall back to the legacy handshake.

I removed the HTTP-only missing-header rejection. Headerless requests now use core version resolution: readable modern versions are validated; unreadable version metadata is ignored with a warning when legacy handling is allowed.

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

Labels

None yet

Projects

None yet

4 participants