From 7b44576216d7a082c285cd38b5e7a50658ebc8ce Mon Sep 17 00:00:00 2001 From: "Jugal D. Bhatt" <55304795+jugaldb@users.noreply.github.com> Date: Fri, 11 Jul 2025 23:54:42 +0530 Subject: [PATCH] Validation to mcp server name (#12515) * added validation * add helper util --- .../mcp_server/mcp_server_manager.py | 2 + .../proxy/_experimental/mcp_server/utils.py | 23 +++++++++++ .../mcp_management_endpoints.py | 9 +++++ .../test_mcp_servers.py | 38 +++++++++++++++++++ .../mcp_tools/create_mcp_server.tsx | 10 ++++- .../components/mcp_tools/mcp_server_edit.tsx | 7 +++- 6 files changed, 86 insertions(+), 3 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index bf743a779b..1f9b09c0e8 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -25,6 +25,7 @@ from litellm.proxy._experimental.mcp_server.utils import ( get_server_name_prefix_tool_mcp, is_tool_name_prefixed, normalize_server_name, + validate_mcp_server_name, ) from litellm.proxy._types import ( LiteLLM_MCPServerTable, @@ -78,6 +79,7 @@ class MCPServerManager: """ verbose_logger.debug("Loading MCP Servers from config-----") for server_name, server_config in mcp_servers_config.items(): + validate_mcp_server_name(server_name) _mcp_info: dict = server_config.get("mcp_info", None) or {} mcp_info = MCPInfo(**_mcp_info) mcp_info["server_name"] = server_name diff --git a/litellm/proxy/_experimental/mcp_server/utils.py b/litellm/proxy/_experimental/mcp_server/utils.py index 241a3e20d3..f6ea8a89ae 100644 --- a/litellm/proxy/_experimental/mcp_server/utils.py +++ b/litellm/proxy/_experimental/mcp_server/utils.py @@ -74,3 +74,26 @@ def is_tool_name_prefixed(tool_name: str) -> bool: True if tool name is prefixed, False otherwise """ return MCP_TOOL_PREFIX_SEPARATOR in tool_name + +def validate_mcp_server_name(server_name: str, raise_http_exception: bool = False) -> None: + """ + Validate that MCP server name does not contain '-' (hyphen). + + Args: + server_name: The server name to validate + raise_http_exception: If True, raises HTTPException instead of generic Exception + + Raises: + Exception or HTTPException: If server name contains '-' + """ + if server_name and '-' in server_name: + error_message = f"Server name cannot contain '-' (hyphen). Please use '_' (underscore) instead. Found: {server_name}" + if raise_http_exception: + from fastapi import HTTPException + from starlette import status + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={"error": error_message} + ) + else: + raise Exception(error_message) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index f31612d600..351fe75291 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -24,6 +24,7 @@ import litellm from litellm._logging import verbose_logger, verbose_proxy_logger from litellm.constants import LITELLM_PROXY_ADMIN_NAME from litellm.proxy.auth.model_checks import get_mcp_server_ids +from litellm.proxy._experimental.mcp_server.utils import validate_mcp_server_name router = APIRouter(prefix="/v1/mcp", tags=["mcp"]) MCP_AVAILABLE: bool = True @@ -344,6 +345,10 @@ if MCP_AVAILABLE: "Database not connected. Connect a database to your proxy" ) + # Server name validation: disallow '-' + if payload.alias: + validate_mcp_server_name(payload.alias, raise_http_exception=True) + # AuthZ - restrict only proxy admins to create mcp servers if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role: raise HTTPException( @@ -484,6 +489,10 @@ if MCP_AVAILABLE: "Database not connected. Connect a database to your proxy - https://docs.litellm.ai/docs/simple_proxy#managing-auth---virtual-keys" ) + # Server name validation: disallow '-' + if payload.alias: + validate_mcp_server_name(payload.alias, raise_http_exception=True) + # Authz - restrict only admins to delete mcp servers if LitellmUserRoles.PROXY_ADMIN != user_api_key_dict.user_role: raise HTTPException( diff --git a/tests/store_model_in_db_tests/test_mcp_servers.py b/tests/store_model_in_db_tests/test_mcp_servers.py index 45f6f18d59..71db4c0a4b 100644 --- a/tests/store_model_in_db_tests/test_mcp_servers.py +++ b/tests/store_model_in_db_tests/test_mcp_servers.py @@ -254,3 +254,41 @@ async def test_create_mcp_server_auth_failure(): # Verify the exception details assert exc_info.value.status_code == 403 assert "permission" in str(exc_info.value.detail) + +@pytest.mark.asyncio +async def test_create_mcp_server_invalid_alias(): + """ + Test that creating an MCP server with a '-' in the alias fails with the correct error. + """ + with mock.patch("litellm.proxy.management_endpoints.mcp_management_endpoints.MCP_AVAILABLE", True), \ + mock.patch("litellm.proxy.management_endpoints.mcp_management_endpoints.get_prisma_client_or_throw") as mock_get_prisma, \ + mock.patch("litellm.proxy.management_endpoints.mcp_management_endpoints.get_mcp_server") as mock_get_server: + + from litellm.proxy.management_endpoints.mcp_management_endpoints import add_mcp_server + from fastapi import HTTPException + + mock_prisma = mock.Mock() + mock_get_prisma.return_value = mock_prisma + + # Set up test data with invalid alias + server_id = str(uuid.uuid4()) + mcp_server_request = generate_mcpserver_create_request(server_id=server_id) + mcp_server_request.alias = "invalid-alias" # This should trigger the validation error + + # Mock that server does not exist + mock_get_server.return_value = None + + user_auth = UserAPIKeyAuth( + api_key=TEST_MASTER_KEY, + user_id="test-user", + user_role=LitellmUserRoles.PROXY_ADMIN + ) + + with pytest.raises(HTTPException) as exc_info: + await add_mcp_server( + payload=mcp_server_request, + user_api_key_dict=user_auth + ) + + assert exc_info.value.status_code == 400 + assert "Server name cannot contain '-' (hyphen). Please use '_' (underscore) instead." in str(exc_info.value.detail) diff --git a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx index f9d10c98ec..22654fa67d 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/create_mcp_server.tsx @@ -145,7 +145,7 @@ const CreateMCPServer: React.FC = ({ label={ MCP Server Name - + @@ -153,10 +153,16 @@ const CreateMCPServer: React.FC = ({ name="alias" rules={[ { required: false, message: "Please enter a server name" }, + { + validator: (_, value) => + value && value.includes('-') + ? Promise.reject("Server name cannot contain '-' (hyphen). Please use '_' (underscore) instead.") + : Promise.resolve(), + }, ]} > diff --git a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx index a6a4f922a3..90b5fd6833 100644 --- a/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx +++ b/ui/litellm-dashboard/src/components/mcp_tools/mcp_server_edit.tsx @@ -110,7 +110,12 @@ const MCPServerEdit: React.FC = ({ mcpServer, accessToken, o
- + + value && value.includes('-') + ? Promise.reject("Server name cannot contain '-' (hyphen). Please use '_' (underscore) instead.") + : Promise.resolve(), + }]}>