mirror of
https://github.com/langgenius/dify.git
synced 2026-10-16 15:35:32 +08:00
fix(api): return protocol errors for unsupported MCP requests (#44197)
This commit is contained in:
@@ -1,4 +1,4 @@
|
||||
from typing import Any
|
||||
from typing import Any, Literal, get_args
|
||||
|
||||
from flask import Response, request
|
||||
from flask_restx import Resource
|
||||
@@ -11,12 +11,20 @@ from controllers.mcp import mcp_ns
|
||||
from core.mcp import types as mcp_types
|
||||
from core.mcp.server.streamable_http import handle_mcp_request, negotiate_protocol_version
|
||||
from extensions.ext_database import db
|
||||
from fields.base import ResponseModel
|
||||
from graphon.variables.input_entities import VariableEntity, VariableEntityType
|
||||
from libs import helper
|
||||
from models.enums import EndUserType
|
||||
from models.model import App, AppMCPServer, AppMode, EndUser
|
||||
from services.app.mcp_server_service import AppMCPServerStatus
|
||||
|
||||
_MCP_METHODS = frozenset(
|
||||
method
|
||||
for message_type in (mcp_types.ClientRequest, mcp_types.ClientNotification)
|
||||
for model in get_args(message_type.model_fields["root"].annotation)
|
||||
for method in get_args(model.model_fields["method"].annotation)
|
||||
)
|
||||
|
||||
|
||||
class MCPRequestError(Exception):
|
||||
"""Custom exception for MCP request processing errors"""
|
||||
@@ -34,7 +42,13 @@ class MCPRequestPayload(BaseModel):
|
||||
id: int | str | None = Field(default=None, description="Request ID for tracking responses")
|
||||
|
||||
|
||||
class MCPJSONRPCResponse(RootModel[mcp_types.JSONRPCResponse | mcp_types.JSONRPCError]):
|
||||
class MCPJSONRPCErrorResponse(ResponseModel):
|
||||
jsonrpc: Literal["2.0"]
|
||||
id: int | str | None
|
||||
error: mcp_types.ErrorData
|
||||
|
||||
|
||||
class MCPJSONRPCResponse(RootModel[mcp_types.JSONRPCResponse | MCPJSONRPCErrorResponse]):
|
||||
pass
|
||||
|
||||
|
||||
@@ -50,7 +64,7 @@ class MCPAppApi(Resource):
|
||||
@mcp_ns.doc(params={"server_code": "Unique identifier for the MCP server"})
|
||||
@mcp_ns.response(200, "MCP JSON-RPC response", mcp_ns.models[MCPJSONRPCResponse.__name__])
|
||||
@mcp_ns.response(202, "MCP notification accepted")
|
||||
@mcp_ns.response(400, "Invalid MCP request or parameters")
|
||||
@mcp_ns.response(400, "Invalid MCP request or parameters", mcp_ns.models[MCPJSONRPCErrorResponse.__name__])
|
||||
@mcp_ns.response(404, "Server or app not found")
|
||||
def post(self, server_code: str):
|
||||
"""Handle MCP requests for a specific server.
|
||||
@@ -70,7 +84,21 @@ class MCPAppApi(Resource):
|
||||
# response-contract:ignore MCP route returns Flask Response from JSON-RPC handler
|
||||
args = MCPRequestPayload.model_validate(mcp_ns.payload or {})
|
||||
request_id: int | str | None = args.id
|
||||
mcp_request = self._parse_mcp_request(args.model_dump(exclude_none=True))
|
||||
try:
|
||||
mcp_request = self._parse_mcp_request(args.model_dump(exclude_none=True))
|
||||
except MCPRequestError as e:
|
||||
if "id" not in args.model_fields_set:
|
||||
return Response("", status=202)
|
||||
response = helper.compact_generate_response(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
"id": request_id,
|
||||
"error": {"code": e.error_code, "message": e.message},
|
||||
}
|
||||
)
|
||||
# A legacy HTTP error lets modern clients fall back to initialize.
|
||||
response.status_code = 400
|
||||
return response
|
||||
|
||||
# Resolve the negotiated protocol version from the MCP-Protocol-Version header.
|
||||
is_initialize = isinstance(mcp_request.root, mcp_types.InitializeRequest)
|
||||
@@ -229,13 +257,15 @@ class MCPAppApi(Resource):
|
||||
|
||||
def _parse_mcp_request(self, args: dict[str, Any]) -> mcp_types.ClientRequest | mcp_types.ClientNotification:
|
||||
"""Parse and validate MCP request"""
|
||||
if args.get("method") not in _MCP_METHODS:
|
||||
raise MCPRequestError(mcp_types.METHOD_NOT_FOUND, "Method not found")
|
||||
try:
|
||||
return mcp_types.ClientRequest.model_validate(args)
|
||||
except ValidationError:
|
||||
try:
|
||||
return mcp_types.ClientNotification.model_validate(args)
|
||||
except ValidationError as e:
|
||||
raise MCPRequestError(mcp_types.INVALID_PARAMS, f"Invalid MCP request: {str(e)}")
|
||||
raise MCPRequestError(mcp_types.INVALID_PARAMS, "Invalid MCP request parameters") from e
|
||||
|
||||
def _retrieve_end_user(self, tenant_id: str, mcp_server_id: str, session: Session) -> EndUser | None:
|
||||
"""Get end user in the caller session"""
|
||||
|
||||
@@ -41,11 +41,11 @@ def app() -> Iterator[Flask]:
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def mock_mcp_ns():
|
||||
def mock_mcp_ns(monkeypatch: pytest.MonkeyPatch):
|
||||
fake_ns = types.SimpleNamespace()
|
||||
fake_ns.payload = None
|
||||
fake_ns.models = {}
|
||||
module.mcp_ns = fake_ns
|
||||
monkeypatch.setattr(module, "mcp_ns", fake_ns)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
@@ -208,14 +208,15 @@ class TestMCPAppApi:
|
||||
|
||||
post_fn = unwrap(api.post)
|
||||
|
||||
with pytest.raises(module.MCPRequestError):
|
||||
post_fn("server-1")
|
||||
response = post_fn("server-1")
|
||||
assert response.status_code == 202
|
||||
assert response.data == b""
|
||||
|
||||
def test_inactive_server(self):
|
||||
fake_payload(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
"method": "test",
|
||||
"method": "ping",
|
||||
"id": 1,
|
||||
"params": {},
|
||||
}
|
||||
@@ -241,7 +242,7 @@ class TestMCPAppApi:
|
||||
with pytest.raises(ValidationError):
|
||||
post_fn("server-1")
|
||||
|
||||
def test_missing_request_id(self):
|
||||
def test_unknown_method_without_request_id_is_ignored(self):
|
||||
fake_payload(
|
||||
{
|
||||
"jsonrpc": "2.0",
|
||||
@@ -258,8 +259,9 @@ class TestMCPAppApi:
|
||||
|
||||
post_fn = unwrap(api.post)
|
||||
|
||||
with pytest.raises(module.MCPRequestError):
|
||||
post_fn("server-1")
|
||||
response = post_fn("server-1")
|
||||
assert response.status_code == 202
|
||||
assert response.data == b""
|
||||
|
||||
def test_server_not_found(self):
|
||||
"""Test when MCP server doesn't exist"""
|
||||
@@ -469,9 +471,13 @@ class TestMCPAppApi:
|
||||
|
||||
post_fn = unwrap(api.post)
|
||||
|
||||
with pytest.raises(module.MCPRequestError) as exc_info:
|
||||
post_fn("server-1")
|
||||
assert "Invalid MCP request" in str(exc_info.value)
|
||||
response = post_fn("server-1")
|
||||
assert response.status_code == 400
|
||||
assert response.get_json() == {
|
||||
"jsonrpc": "2.0",
|
||||
"id": 1,
|
||||
"error": {"code": module.mcp_types.METHOD_NOT_FOUND, "message": "Method not found"},
|
||||
}
|
||||
|
||||
def test_validate_server_status_active(self):
|
||||
"""Test successful server status validation"""
|
||||
|
||||
@@ -0,0 +1,188 @@
|
||||
"""MCP request parsing errors exercised through the real HTTP blueprint."""
|
||||
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from flask import Flask
|
||||
from flask.testing import FlaskClient
|
||||
from sqlalchemy import select
|
||||
from sqlalchemy.engine import Engine
|
||||
from sqlalchemy.orm import Session
|
||||
|
||||
import controllers.mcp as mcp_controller
|
||||
import controllers.mcp.mcp as module
|
||||
from models.engine import db
|
||||
from models.enums import EndUserType
|
||||
from models.model import App, AppMCPServer, AppMode, AppModelConfig, EndUser
|
||||
from services.app.mcp_server_service import AppMCPServerStatus
|
||||
|
||||
_URL = "/mcp/server/test-server/mcp"
|
||||
_MODERN_HEADERS = {
|
||||
"Accept": "application/json, text/event-stream",
|
||||
"MCP-Protocol-Version": "2026-07-28",
|
||||
"Mcp-Method": "server/discover",
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def app(sqlite_engine: Engine, monkeypatch: pytest.MonkeyPatch) -> Flask:
|
||||
app = Flask(__name__)
|
||||
app.config["SQLALCHEMY_DATABASE_URI"] = str(sqlite_engine.url)
|
||||
db.init_app(app)
|
||||
app.register_blueprint(mcp_controller.bp)
|
||||
monkeypatch.setattr(module, "mcp_ns", mcp_controller.mcp_ns)
|
||||
return app
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def client(app: Flask) -> FlaskClient:
|
||||
return app.test_client()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def published_server(sqlite_session: Session) -> AppMCPServer:
|
||||
app_id = str(uuid4())
|
||||
tenant_id = str(uuid4())
|
||||
model_config = AppModelConfig(app_id=app_id, user_input_form="[]")
|
||||
app = App(
|
||||
id=app_id,
|
||||
tenant_id=tenant_id,
|
||||
name="test_app",
|
||||
mode=AppMode.CHAT,
|
||||
enable_site=False,
|
||||
enable_api=True,
|
||||
app_model_config_id=model_config.id,
|
||||
)
|
||||
server = AppMCPServer(
|
||||
tenant_id=tenant_id,
|
||||
app_id=app_id,
|
||||
name="test_app",
|
||||
description="Test MCP app",
|
||||
server_code="test-server",
|
||||
status=AppMCPServerStatus.ACTIVE,
|
||||
parameters="{}",
|
||||
)
|
||||
sqlite_session.add_all([app, model_config, server])
|
||||
sqlite_session.commit()
|
||||
return server
|
||||
|
||||
|
||||
@pytest.mark.parametrize("request_id", [1, 0, "discover-1", None])
|
||||
def test_server_discover_returns_method_not_found(client: FlaskClient, request_id: int | str | None) -> None:
|
||||
response = client.post(
|
||||
_URL,
|
||||
json={
|
||||
"jsonrpc": "2.0",
|
||||
"id": request_id,
|
||||
"method": "server/discover",
|
||||
"params": {
|
||||
"_meta": {
|
||||
"io.modelcontextprotocol/protocolVersion": "2026-07-28",
|
||||
"io.modelcontextprotocol/clientInfo": {"name": "modern-client", "version": "1.0"},
|
||||
"io.modelcontextprotocol/clientCapabilities": {
|
||||
"extensions": {"io.modelcontextprotocol/ui": {"mimeTypes": ["text/html;profile=mcp-app"]}}
|
||||
},
|
||||
}
|
||||
},
|
||||
},
|
||||
headers=_MODERN_HEADERS,
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = response.get_json()
|
||||
assert body == {
|
||||
"jsonrpc": "2.0",
|
||||
"id": request_id,
|
||||
"error": {"code": -32601, "message": "Method not found"},
|
||||
}
|
||||
assert type(body["id"]) is type(request_id)
|
||||
|
||||
|
||||
def test_invalid_tool_params_return_safe_jsonrpc_error(client: FlaskClient) -> None:
|
||||
response = client.post(
|
||||
_URL,
|
||||
json={
|
||||
"jsonrpc": "2.0",
|
||||
"id": "tool-call-1",
|
||||
"method": "tools/call",
|
||||
"params": {"arguments": {"secret": "private-input-do-not-echo"}},
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
body = response.get_json()
|
||||
assert body["jsonrpc"] == "2.0"
|
||||
assert body["id"] == "tool-call-1"
|
||||
assert body["error"]["code"] == -32602
|
||||
assert "private-input-do-not-echo" not in response.get_data(as_text=True)
|
||||
assert "validation error" not in body["error"]["message"].lower()
|
||||
|
||||
|
||||
@pytest.mark.parametrize("method", ["server/discover", "notifications/unknown", "tools/call"])
|
||||
def test_unparseable_notification_has_no_jsonrpc_response(client: FlaskClient, method: str) -> None:
|
||||
response = client.post(
|
||||
_URL,
|
||||
json={"jsonrpc": "2.0", "method": method, "params": {}},
|
||||
headers={**_MODERN_HEADERS, "Mcp-Method": method},
|
||||
)
|
||||
|
||||
assert response.status_code == 202
|
||||
assert response.data == b""
|
||||
|
||||
|
||||
def test_legacy_initialize_and_list_tools_remain_compatible(
|
||||
client: FlaskClient, published_server: AppMCPServer, sqlite_session: Session
|
||||
) -> None:
|
||||
response = client.post(
|
||||
_URL,
|
||||
json={
|
||||
"jsonrpc": "2.0",
|
||||
"id": 0,
|
||||
"method": "initialize",
|
||||
"params": {
|
||||
"protocolVersion": "2024-11-05",
|
||||
"capabilities": {},
|
||||
"clientInfo": {"name": "legacy-client", "version": "1.0"},
|
||||
},
|
||||
},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
initialized = response.get_json()
|
||||
assert initialized["id"] == 0
|
||||
assert initialized["result"]["protocolVersion"] == "2024-11-05"
|
||||
end_user = sqlite_session.scalar(select(EndUser).where(EndUser.session_id == published_server.id))
|
||||
assert end_user is not None
|
||||
assert end_user.tenant_id == published_server.tenant_id
|
||||
assert end_user.app_id == published_server.app_id
|
||||
assert end_user.type == EndUserType.MCP
|
||||
assert end_user.name == "legacy-client@1.0"
|
||||
|
||||
headers = {"MCP-Protocol-Version": "2024-11-05"}
|
||||
notification = client.post(
|
||||
_URL,
|
||||
json={"jsonrpc": "2.0", "method": "notifications/initialized"},
|
||||
headers=headers,
|
||||
)
|
||||
assert notification.status_code == 202
|
||||
assert notification.data == b""
|
||||
|
||||
listed = client.post(
|
||||
_URL,
|
||||
json={"jsonrpc": "2.0", "id": "list-1", "method": "tools/list"},
|
||||
headers=headers,
|
||||
)
|
||||
assert listed.status_code == 200
|
||||
body = listed.get_json()
|
||||
assert body["id"] == "list-1"
|
||||
assert body["result"]["tools"] == [
|
||||
{
|
||||
"name": "test_app",
|
||||
"description": "Test MCP app",
|
||||
"inputSchema": {
|
||||
"type": "object",
|
||||
"properties": {"query": {"type": "string", "description": "User Input/Question content"}},
|
||||
"required": ["query"],
|
||||
},
|
||||
}
|
||||
]
|
||||
Reference in New Issue
Block a user