From 31f6568bad775eb1090979659244ca40a15b8df0 Mon Sep 17 00:00:00 2001 From: Ishaan Jaffer Date: Tue, 10 Mar 2026 13:32:41 -0700 Subject: [PATCH] fix(mcp): validate field names in mcp_required_fields; surface backend error in submit UI --- .../mcp_management_endpoints.py | 18 +++++++++++++++++- .../test_mcp_management_endpoints.py | 17 +++++++++++++++++ .../components/mcp_tools/create_mcp_server.tsx | 5 ++++- 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 70060a92568..2ba8ae2ec62 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -161,6 +161,10 @@ if MCP_AVAILABLE: _base_validate_and_normalize_mcp_server_payload(payload) _validate_mcp_server_name_fields(payload) + _VALID_MCP_REQUIRED_FIELDS: frozenset = frozenset( + NewMCPServerRequest.model_fields + ) + def _validate_mcp_required_fields(payload: Any) -> None: """Validate submission payload against admin-configured mcp_required_fields.""" from litellm.proxy.proxy_server import ( @@ -173,6 +177,18 @@ if MCP_AVAILABLE: if not required_fields: return + # Fail fast on unknown field names — a typo in the config would silently + # block every submission with a confusing "missing fields" error. + unknown = [f for f in required_fields if f not in _VALID_MCP_REQUIRED_FIELDS] + if unknown: + raise HTTPException( + status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, + detail={ + "error": f"mcp_required_fields contains unknown field names: {unknown}. " + "Check general_settings.mcp_required_fields in your proxy config." + }, + ) + # Mirror the UI's compliance checks (MCPStandardsSettings.tsx FIELD_GROUPS): # auth_type requires a real value — "none" is treated as absent. _AUTH_TYPE_SENTINEL = "none" @@ -750,7 +766,7 @@ if MCP_AVAILABLE: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail={ - "error": "Admin users should use POST /v1/mcp/server to create servers directly instead of the submission workflow." + "error": "PROXY_ADMIN users should use POST /v1/mcp/server to create servers directly instead of the submission workflow." }, ) diff --git a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py index b53ece2b1a8..30b3be4a3eb 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_mcp_management_endpoints.py @@ -1837,3 +1837,20 @@ class TestValidateMCPRequiredFields: with patch_proxy_general_settings({}): # Should not raise when no required fields are configured _validate_mcp_required_fields(payload) + + def test_unknown_field_name_in_config_raises_500(self): + from litellm.proxy.management_endpoints.mcp_management_endpoints import ( + _validate_mcp_required_fields, + ) + + payload = NewMCPServerRequest( + alias="My Server", + url="https://example.com/mcp", + transport=MCPTransport.sse, + ) + # "source_Url" is a typo — not a real field on NewMCPServerRequest + with patch_proxy_general_settings({"mcp_required_fields": ["source_Url"]}): + with pytest.raises(HTTPException) as exc_info: + _validate_mcp_required_fields(payload) + assert exc_info.value.status_code == 500 + assert "source_Url" 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 c9dca035206..ec15c05dd45 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 @@ -391,7 +391,10 @@ const CreateMCPServer: React.FC = ({ onCreateSuccess(response); } } catch (error) { - NotificationsManager.fromBackend("Error creating MCP Server: " + error); + const reason = error instanceof Error ? error.message : String(error); + NotificationsManager.fromBackend( + isAdmin ? `Error creating MCP Server: ${reason}` : `Error submitting MCP Server: ${reason}` + ); } finally { setIsLoading(false); }