mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
feat: SEP-986
This commit is contained in:
Yuta Saito
parent
30f2c09401
commit
05d9fb6fd6
6 changed files with 211 additions and 6 deletions
|
|
@ -38,6 +38,7 @@ from litellm.proxy._experimental.mcp_server.auth.user_api_key_auth_mcp import (
|
|||
MCPRequestHandler,
|
||||
)
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
MCP_TOOL_PREFIX_SEPARATOR,
|
||||
add_server_prefix_to_name,
|
||||
get_server_prefix,
|
||||
is_tool_name_prefixed,
|
||||
|
|
@ -61,6 +62,45 @@ from litellm.types.mcp_server.mcp_server_manager import (
|
|||
MCPOAuthMetadata,
|
||||
MCPServer,
|
||||
)
|
||||
from mcp.shared.tool_name_validation import SEP_986_URL, validate_tool_name
|
||||
|
||||
|
||||
# Probe includes characters on both sides of the separator to mimic real prefixed tool names.
|
||||
_separator_probe_tool_name = f"litellm{MCP_TOOL_PREFIX_SEPARATOR}probe"
|
||||
_separator_probe = validate_tool_name(_separator_probe_tool_name)
|
||||
if not _separator_probe.is_valid:
|
||||
verbose_logger.warning(
|
||||
"MCP tool prefix separator '%s' violates SEP-986. See %s",
|
||||
MCP_TOOL_PREFIX_SEPARATOR,
|
||||
SEP_986_URL,
|
||||
)
|
||||
|
||||
|
||||
def _warn_on_server_name_fields(
|
||||
*,
|
||||
server_id: str,
|
||||
alias: Optional[str],
|
||||
server_name: Optional[str],
|
||||
):
|
||||
def _warn(field_name: str, value: Optional[str]) -> None:
|
||||
if not value:
|
||||
return
|
||||
result = validate_tool_name(value)
|
||||
if result.is_valid:
|
||||
return
|
||||
|
||||
warning_text = "; ".join(result.warnings) if result.warnings else "Validation failed"
|
||||
verbose_logger.warning(
|
||||
"MCP server '%s' has invalid %s '%s': %s",
|
||||
server_id,
|
||||
field_name,
|
||||
value,
|
||||
warning_text,
|
||||
)
|
||||
|
||||
_warn("alias", alias)
|
||||
_warn("server_name", server_name)
|
||||
|
||||
|
||||
|
||||
def _deserialize_json_dict(data: Any) -> Optional[Dict[str, str]]:
|
||||
|
|
@ -209,6 +249,12 @@ class MCPServerManager:
|
|||
alias=alias,
|
||||
)
|
||||
|
||||
_warn_on_server_name_fields(
|
||||
server_id=server_id,
|
||||
alias=alias,
|
||||
server_name=server_name,
|
||||
)
|
||||
|
||||
auth_type = server_config.get("auth_type", None)
|
||||
if server_url and auth_type is not None and auth_type == MCPAuth.oauth2:
|
||||
mcp_oauth_metadata = await self._descovery_metadata(
|
||||
|
|
@ -2099,6 +2145,11 @@ class MCPServerManager:
|
|||
new_registry[server.server_id] = existing_server
|
||||
continue
|
||||
|
||||
_warn_on_server_name_fields(
|
||||
server_id=server.server_id,
|
||||
alias=getattr(server, "alias", None),
|
||||
server_name=getattr(server, "server_name", None),
|
||||
)
|
||||
verbose_logger.debug(
|
||||
f"Building server from DB: {server.server_id} ({server.server_name})"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -786,7 +786,6 @@ if MCP_AVAILABLE:
|
|||
add_prefix=add_prefix,
|
||||
raw_headers=raw_headers,
|
||||
)
|
||||
|
||||
filtered_tools = filter_tools_by_allowed_tools(tools, server)
|
||||
|
||||
filtered_tools = await filter_tools_by_key_team_permissions(
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@ from litellm._uuid import uuid
|
|||
from litellm.constants import LITELLM_PROXY_ADMIN_NAME
|
||||
from litellm.proxy._experimental.mcp_server.utils import (
|
||||
get_server_prefix,
|
||||
validate_and_normalize_mcp_server_payload,
|
||||
validate_and_normalize_mcp_server_payload as _base_validate_and_normalize_mcp_server_payload,
|
||||
)
|
||||
|
||||
router = APIRouter(prefix="/v1/mcp", tags=["mcp"])
|
||||
|
|
@ -56,6 +56,7 @@ except ImportError as e:
|
|||
MCP_AVAILABLE = False
|
||||
|
||||
if MCP_AVAILABLE:
|
||||
from mcp.shared.tool_name_validation import validate_tool_name
|
||||
from litellm.proxy._experimental.mcp_server.db import (
|
||||
create_mcp_server,
|
||||
delete_mcp_server,
|
||||
|
|
@ -97,6 +98,43 @@ if MCP_AVAILABLE:
|
|||
server: MCPServer
|
||||
expires_at: datetime
|
||||
|
||||
def _validate_mcp_server_name_fields(payload: Any) -> None:
|
||||
candidates: List[tuple[str, Optional[str]]] = []
|
||||
|
||||
server_name = getattr(payload, "server_name", None)
|
||||
alias = getattr(payload, "alias", None)
|
||||
|
||||
if server_name:
|
||||
candidates.append(("server_name", server_name))
|
||||
if alias:
|
||||
candidates.append(("alias", alias))
|
||||
|
||||
for field_name, value in candidates:
|
||||
if not value:
|
||||
continue
|
||||
|
||||
validation_result = validate_tool_name(value)
|
||||
if validation_result.is_valid:
|
||||
continue
|
||||
|
||||
error_messages_text = (
|
||||
f"Invalid MCP tool prefix '{value}' provided via {field_name}"
|
||||
)
|
||||
if validation_result.warnings:
|
||||
error_messages_text = (
|
||||
error_messages_text
|
||||
+ "\n"
|
||||
+ "\n".join(validation_result.warnings)
|
||||
)
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_400_BAD_REQUEST,
|
||||
detail={"error": error_messages_text},
|
||||
)
|
||||
|
||||
def validate_and_normalize_mcp_server_payload(payload: Any) -> None:
|
||||
_base_validate_and_normalize_mcp_server_payload(payload)
|
||||
_validate_mcp_server_name_fields(payload)
|
||||
|
||||
def _is_public_registry_enabled() -> bool:
|
||||
from litellm.proxy.proxy_server import (
|
||||
general_settings as proxy_general_settings,
|
||||
|
|
|
|||
|
|
@ -1,3 +1,6 @@
|
|||
import importlib
|
||||
import logging
|
||||
import os
|
||||
import sys
|
||||
from datetime import datetime
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
|
@ -29,6 +32,15 @@ from litellm.types.mcp import MCPAuth
|
|||
from litellm.types.mcp_server.mcp_server_manager import MCPOAuthMetadata, MCPServer
|
||||
|
||||
|
||||
def _reload_mcp_manager_module():
|
||||
utils_module = sys.modules["litellm.proxy._experimental.mcp_server.utils"]
|
||||
manager_module = sys.modules[
|
||||
"litellm.proxy._experimental.mcp_server.mcp_server_manager"
|
||||
]
|
||||
importlib.reload(utils_module)
|
||||
return importlib.reload(manager_module)
|
||||
|
||||
|
||||
class TestMCPServerManager:
|
||||
"""Test MCP Server Manager stdio functionality"""
|
||||
|
||||
|
|
@ -148,6 +160,90 @@ class TestMCPServerManager:
|
|||
# When the header isn't provided, the key is omitted entirely
|
||||
assert env == {}
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_servers_from_config_warns_on_invalid_alias(self, caplog):
|
||||
"""Invalid aliases from config should emit warnings during load."""
|
||||
|
||||
manager = MCPServerManager()
|
||||
config = {
|
||||
"validserver": {
|
||||
"alias": "bad/name",
|
||||
"url": "https://example.com",
|
||||
"transport": MCPTransport.http,
|
||||
}
|
||||
}
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
await manager.load_servers_from_config(config)
|
||||
|
||||
assert any(
|
||||
"invalid alias 'bad/name'" in message for message in caplog.messages
|
||||
)
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_load_servers_from_config_accepts_valid_alias(self, caplog):
|
||||
"""Valid aliases should be accepted and populate the registry."""
|
||||
|
||||
manager = MCPServerManager()
|
||||
config = {
|
||||
"validserver": {
|
||||
"alias": "friendly_alias",
|
||||
"url": "https://example.com",
|
||||
"transport": MCPTransport.http,
|
||||
}
|
||||
}
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
await manager.load_servers_from_config(config)
|
||||
|
||||
# No warnings logged for the valid alias
|
||||
assert all("invalid alias" not in message for message in caplog.messages)
|
||||
|
||||
server = next(iter(manager.config_mcp_servers.values()))
|
||||
assert server.alias == "friendly_alias"
|
||||
assert server.server_name == "validserver"
|
||||
|
||||
def test_warns_when_custom_separator_invalid(self, monkeypatch, caplog):
|
||||
"""Invalid MCP_TOOL_PREFIX_SEPARATOR values should log a warning."""
|
||||
|
||||
original_value = os.environ.get("MCP_TOOL_PREFIX_SEPARATOR")
|
||||
monkeypatch.setenv("MCP_TOOL_PREFIX_SEPARATOR", "/")
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
_reload_mcp_manager_module()
|
||||
|
||||
assert any("violates SEP-986" in message for message in caplog.messages)
|
||||
|
||||
# Restore original setting and ensure warning disappears
|
||||
if original_value is None:
|
||||
monkeypatch.delenv("MCP_TOOL_PREFIX_SEPARATOR", raising=False)
|
||||
else:
|
||||
monkeypatch.setenv("MCP_TOOL_PREFIX_SEPARATOR", original_value)
|
||||
|
||||
caplog.clear()
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
_reload_mcp_manager_module()
|
||||
|
||||
assert all("violates SEP-986" not in message for message in caplog.messages)
|
||||
|
||||
def test_accepts_valid_custom_separator(self, monkeypatch, caplog):
|
||||
"""Valid separators should not emit warnings during module import."""
|
||||
|
||||
original_value = os.environ.get("MCP_TOOL_PREFIX_SEPARATOR")
|
||||
monkeypatch.setenv("MCP_TOOL_PREFIX_SEPARATOR", "_")
|
||||
|
||||
with caplog.at_level(logging.WARNING, logger="LiteLLM"):
|
||||
_reload_mcp_manager_module()
|
||||
|
||||
assert all("violates SEP-986" not in message for message in caplog.messages)
|
||||
|
||||
if original_value is None:
|
||||
monkeypatch.delenv("MCP_TOOL_PREFIX_SEPARATOR", raising=False)
|
||||
else:
|
||||
monkeypatch.setenv("MCP_TOOL_PREFIX_SEPARATOR", original_value)
|
||||
|
||||
_reload_mcp_manager_module()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_list_tools_with_server_specific_auth_headers(self):
|
||||
"""Test list_tools method with server-specific auth headers"""
|
||||
|
|
|
|||
|
|
@ -2,14 +2,18 @@ import json
|
|||
import os
|
||||
import sys
|
||||
import types
|
||||
from types import SimpleNamespace
|
||||
from datetime import datetime, timedelta
|
||||
from typing import List, Optional
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
from fastapi import FastAPI
|
||||
from fastapi import FastAPI, HTTPException
|
||||
from fastapi.testclient import TestClient
|
||||
from litellm._uuid import uuid
|
||||
from litellm.proxy.management_endpoints import (
|
||||
mcp_management_endpoints as mgmt_endpoints,
|
||||
)
|
||||
|
||||
sys.path.insert(
|
||||
0, os.path.abspath("../../../..")
|
||||
|
|
@ -726,8 +730,6 @@ class TestTemporaryMCPSessionEndpoints:
|
|||
"litellm.proxy.management_endpoints.mcp_management_endpoints.get_cached_temporary_mcp_server",
|
||||
return_value=None,
|
||||
):
|
||||
from fastapi import HTTPException
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
_get_cached_temporary_mcp_server_or_404("missing")
|
||||
|
||||
|
|
@ -1195,6 +1197,25 @@ class TestMCPRegistryEndpoint:
|
|||
assert result[0]["server_id"] == "server-1"
|
||||
assert result[0]["status"] == "healthy"
|
||||
|
||||
|
||||
class TestManagementPayloadValidation:
|
||||
def test_rejects_invalid_alias(self):
|
||||
payload = SimpleNamespace(server_name="valid_server", alias="bad/name")
|
||||
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
mgmt_endpoints.validate_and_normalize_mcp_server_payload(payload)
|
||||
|
||||
assert exc_info.value.status_code == 400
|
||||
error_message = exc_info.value.detail["error"]
|
||||
assert "bad/name" in error_message
|
||||
|
||||
def test_accepts_valid_names(self):
|
||||
payload = SimpleNamespace(server_name="valid_server", alias=None)
|
||||
|
||||
mgmt_endpoints.validate_and_normalize_mcp_server_payload(payload)
|
||||
|
||||
assert payload.alias == "valid_server"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_health_check_view_all_mode(self):
|
||||
"""view_all mode should return health info for all MCP servers."""
|
||||
|
|
|
|||
|
|
@ -425,7 +425,7 @@ const CreateMCPServer: React.FC<CreateMCPServerProps> = ({
|
|||
label={
|
||||
<span className="text-sm font-medium text-gray-700 flex items-center">
|
||||
MCP Server Name
|
||||
<Tooltip title="Best practice: Use a descriptive name that indicates the server's purpose (e.g., 'GitHub_MCP', 'Email_Service'). Hyphens '-' are not allowed; use underscores '_' instead.">
|
||||
<Tooltip title="Best practice: Use a descriptive name that indicates the server's purpose (e.g., 'GitHub_MCP', 'Email_Service'). Hyphens '-' are not allowed; use underscores '_' instead. Names must comply with SEP-986 and will be rejected if invalid (https://modelcontextprotocol.io/specification/2025-11-25/server/tools#tool-names).">
|
||||
<InfoCircleOutlined className="ml-2 text-blue-400 hover:text-blue-600 cursor-help" />
|
||||
</Tooltip>
|
||||
</span>
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue