diff --git a/src/mcp/types/jsonrpc.py b/src/mcp/types/jsonrpc.py index 84304a37c..9f9e4318f 100644 --- a/src/mcp/types/jsonrpc.py +++ b/src/mcp/types/jsonrpc.py @@ -4,7 +4,7 @@ from typing import Annotated, Any, Literal -from pydantic import BaseModel, Field, TypeAdapter +from pydantic import BaseModel, Field, TypeAdapter, model_validator RequestId = Annotated[int, Field(strict=True)] | str """The ID of a JSON-RPC request.""" @@ -26,6 +26,20 @@ class JSONRPCNotification(BaseModel): method: str params: dict[str, Any] | None = None + @model_validator(mode="before") + @classmethod + def _reject_id_field(cls, data: dict[str, Any]) -> dict[str, Any]: + """Reject payloads containing an 'id' field (notifications must not have one).""" + # Per JSON-RPC 2.0, notifications must not include an "id" member. + # Without this check, Pydantic's union resolution silently absorbs + # invalid "id": null requests as notifications (#2057). + if "id" in data: + raise ValueError( + "Notifications must not include an 'id' field. " + "A JSON-RPC message with 'id' is a request, not a notification." + ) + return data + # TODO(Marcelo): This is actually not correct. A JSONRPCResponse is the union of a successful response and an error. class JSONRPCResponse(BaseModel): diff --git a/tests/issues/test_2057_null_id_rejected.py b/tests/issues/test_2057_null_id_rejected.py new file mode 100644 index 000000000..7f27fde54 --- /dev/null +++ b/tests/issues/test_2057_null_id_rejected.py @@ -0,0 +1,45 @@ +"""Tests for issue #2057: Requests with "id": null silently misclassified as notifications. + +When a JSON-RPC request arrives with ``"id": null``, the SDK should reject it +rather than silently reclassifying it as a ``JSONRPCNotification``. Both +JSON-RPC 2.0 and the MCP spec restrict request IDs to strings or integers. + +See: https://github.com/modelcontextprotocol/python-sdk/issues/2057 +""" + +import json + +import pytest +from pydantic import ValidationError + +from mcp.types import ( + JSONRPCNotification, + jsonrpc_message_adapter, +) + + +def test_notification_rejects_id_field() -> None: + """JSONRPCNotification must not accept messages with an 'id' field.""" + with pytest.raises(ValidationError, match="must not include an 'id' field"): + JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "initialize", "id": None}) + + +@pytest.mark.parametrize("id_value", [None, 0, 1, "", "abc"]) +def test_notification_rejects_any_id_value(id_value: object) -> None: + """Notification rejects 'id' regardless of value — null, int, or str.""" + with pytest.raises(ValidationError): + JSONRPCNotification.model_validate({"jsonrpc": "2.0", "method": "test", "id": id_value}) + + +def test_message_adapter_rejects_null_id() -> None: + """JSONRPCMessage union must not accept ``"id": null``.""" + raw = {"jsonrpc": "2.0", "method": "initialize", "id": None} + with pytest.raises(ValidationError): + jsonrpc_message_adapter.validate_python(raw) + + +def test_message_adapter_rejects_null_id_json() -> None: + """Same test but via validate_json (the path used by transports).""" + raw_json = json.dumps({"jsonrpc": "2.0", "method": "initialize", "id": None}) + with pytest.raises(ValidationError): + jsonrpc_message_adapter.validate_json(raw_json)