From 4d2acafa43cdb94d07c01fbb1d61b11a076f9dd1 Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Thu, 23 Apr 2026 16:52:45 -0700 Subject: [PATCH 1/2] Split MCP routes into inference vs management categories MCP server CRUD endpoints (/v1/mcp/server*) were bundled with MCP tool-call / passthrough endpoints under llm_api_routes, so setting DISABLE_LLM_API_ENDPOINTS=true on admin-only nodes also blocked the Admin UI from listing, adding, or attaching MCP servers. Separate mcp_inference_routes (data-plane, gated by DISABLE_LLM_API_ENDPOINTS) from mcp_management_routes (control-plane, gated by DISABLE_ADMIN_ENDPOINTS). Keep mcp_routes as a union for backward compat with allowed_routes=["mcp_routes"] virtual key configs. Upgrade is_management_route to pattern-aware matching so /v1/mcp/server/{path:path} resolves for concrete IDs. --- litellm/proxy/_types.py | 15 ++++- litellm/proxy/auth/route_checks.py | 6 +- .../proxy/auth/test_route_checks.py | 58 +++++++++++++++++++ .../proxy/auth/test_route_checks.py | 32 ++++++++++ 4 files changed, 106 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index 84a9c4b793..cd033f6758 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -427,7 +427,8 @@ class LiteLLMRoutes(enum.Enum): "/v1/skills/{skill_id}", ] - mcp_routes = [ + # MCP tool-call / passthrough routes — data-plane. Gated by DISABLE_LLM_API_ENDPOINTS. + mcp_inference_routes = [ "/mcp", "/mcp/", "/mcp/{subpath}", @@ -436,10 +437,18 @@ class LiteLLMRoutes(enum.Enum): "/mcp/tools/call", "/mcp-rest/tools/list", "/mcp-rest/tools/call", + ] + + # MCP server CRUD routes — control-plane. Gated by DISABLE_ADMIN_ENDPOINTS. + mcp_management_routes = [ "/v1/mcp/server", "/v1/mcp/server/{path:path}", ] + # Backwards-compat union — virtual keys may be configured with + # allowed_routes=["mcp_routes"], which should cover both halves. + mcp_routes = mcp_inference_routes + mcp_management_routes + agent_routes = [ "/v1/agents", "/v1/agents/{agent_id}", @@ -477,7 +486,7 @@ class LiteLLMRoutes(enum.Enum): + mapped_pass_through_routes + passthrough_routes_wildcard + apply_guardrail_routes - + mcp_routes + + mcp_inference_routes + litellm_native_routes + agent_routes ) @@ -563,7 +572,7 @@ class LiteLLMRoutes(enum.Enum): "/jwt/key/mapping/delete", "/jwt/key/mapping/list", "/jwt/key/mapping/info", - ] + key_management_routes + ] + key_management_routes + mcp_management_routes spend_tracking_routes = [ # spend diff --git a/litellm/proxy/auth/route_checks.py b/litellm/proxy/auth/route_checks.py index 26bbdef309..6417307f69 100644 --- a/litellm/proxy/auth/route_checks.py +++ b/litellm/proxy/auth/route_checks.py @@ -300,7 +300,7 @@ class RouteChecks: return True if RouteChecks.check_route_access( - route=route, allowed_routes=LiteLLMRoutes.mcp_routes.value + route=route, allowed_routes=LiteLLMRoutes.mcp_inference_routes.value ): return True @@ -358,7 +358,9 @@ class RouteChecks: """ Check if route is a management route """ - return route in LiteLLMRoutes.management_routes.value + return RouteChecks.check_route_access( + route=route, allowed_routes=LiteLLMRoutes.management_routes.value + ) @staticmethod def is_info_route(route: str) -> bool: diff --git a/tests/enterprise/litellm_enterprise/proxy/auth/test_route_checks.py b/tests/enterprise/litellm_enterprise/proxy/auth/test_route_checks.py index 24e2797796..c147c7aae9 100644 --- a/tests/enterprise/litellm_enterprise/proxy/auth/test_route_checks.py +++ b/tests/enterprise/litellm_enterprise/proxy/auth/test_route_checks.py @@ -252,6 +252,64 @@ class TestEnterpriseRouteChecksModelListExemption: ) +@patch("litellm.proxy.proxy_server.premium_user", True) +class TestEnterpriseRouteChecksMcpManagement: + """Regression tests: MCP management routes (/v1/mcp/server*) must remain + reachable when DISABLE_LLM_API_ENDPOINTS is set on admin nodes, but must be + blocked when DISABLE_ADMIN_ENDPOINTS is set. Uses the real is_llm_api_route + / is_management_route classifiers (not mocks).""" + + @pytest.mark.parametrize( + "route", + [ + "/v1/mcp/server", + "/v1/mcp/server/abc-123", + "/v1/mcp/server/abc-123/approve", + ], + ) + def test_mcp_management_allowed_when_llm_api_disabled(self, route): + with patch.dict(os.environ, {"DISABLE_LLM_API_ENDPOINTS": "true"}, clear=False): + os.environ.pop("DISABLE_ADMIN_ENDPOINTS", None) + # Should not raise — MCP management is a management route, not llm_api. + EnterpriseRouteChecks.should_call_route(route) + + @pytest.mark.parametrize( + "route", + [ + "/v1/mcp/server", + "/v1/mcp/server/abc-123", + ], + ) + def test_mcp_management_blocked_when_admin_disabled(self, route): + with patch.dict(os.environ, {"DISABLE_ADMIN_ENDPOINTS": "true"}, clear=False): + os.environ.pop("DISABLE_LLM_API_ENDPOINTS", None) + with pytest.raises(HTTPException) as exc_info: + EnterpriseRouteChecks.should_call_route(route) + + assert exc_info.value.status_code == 403 + assert "Management routes are disabled for this instance." in str( + exc_info.value.detail + ) + + @pytest.mark.parametrize( + "route", + [ + "/mcp/tools/call", + "/mcp-rest/tools/call", + ], + ) + def test_mcp_inference_still_blocked_when_llm_api_disabled(self, route): + with patch.dict(os.environ, {"DISABLE_LLM_API_ENDPOINTS": "true"}, clear=False): + os.environ.pop("DISABLE_ADMIN_ENDPOINTS", None) + with pytest.raises(HTTPException) as exc_info: + EnterpriseRouteChecks.should_call_route(route) + + assert exc_info.value.status_code == 403 + assert "LLM API routes are disabled for this instance." in str( + exc_info.value.detail + ) + + class TestEnterpriseRouteChecksErrorMessages: """Test that error messages correctly identify which feature requires Enterprise license""" diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py index bca6b9e78d..bfebc7145d 100644 --- a/tests/test_litellm/proxy/auth/test_route_checks.py +++ b/tests/test_litellm/proxy/auth/test_route_checks.py @@ -152,6 +152,38 @@ def test_virtual_key_mcp_routes_allows_v1_mcp_server_subpaths(route): assert result is True +@pytest.mark.parametrize( + "route", + [ + "/v1/mcp/server", + "/v1/mcp/server/abc-123", + "/v1/mcp/server/abc-123/approve", + ], +) +def test_mcp_management_routes_classified_as_management_not_llm_api(route): + """MCP server CRUD must be management routes, not llm_api routes, so + DISABLE_LLM_API_ENDPOINTS on admin nodes does not block the Admin UI.""" + + assert RouteChecks.is_llm_api_route(route=route) is False + assert RouteChecks.is_management_route(route=route) is True + + +@pytest.mark.parametrize( + "route", + [ + "/mcp/tools/call", + "/mcp-rest/tools/call", + "/mcp/tools/list", + ], +) +def test_mcp_inference_routes_classified_as_llm_api(route): + """MCP tool-call / passthrough routes must remain llm_api routes so they + continue to be blocked by DISABLE_LLM_API_ENDPOINTS on admin nodes.""" + + assert RouteChecks.is_llm_api_route(route=route) is True + assert RouteChecks.is_management_route(route=route) is False + + def test_virtual_key_allowed_routes_with_litellm_routes_member_name_denied(): """Test that virtual key is denied when route is not in the allowed LiteLLMRoutes group""" From 35eef7d92ca244cfa9329d8083da36d9c108945f Mon Sep 17 00:00:00 2001 From: Ryan Crabbe Date: Thu, 23 Apr 2026 17:35:35 -0700 Subject: [PATCH 2/2] chore: apply black formatting to _types.py management_routes block --- litellm/proxy/_types.py | 72 ++++++++++++++++++++++------------------- 1 file changed, 38 insertions(+), 34 deletions(-) diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py index cd033f6758..f55489fde2 100644 --- a/litellm/proxy/_types.py +++ b/litellm/proxy/_types.py @@ -539,40 +539,44 @@ class LiteLLMRoutes(enum.Enum): KeyManagementRoutes.KEY_ALIASES.value, ] - management_routes = [ - # user - "/user/new", - "/user/update", - "/user/bulk_update", - "/user/delete", - "/user/info", - "/user/list", - "/user/daily/activity", - "/user/daily/activity/aggregated", - # team - "/team/new", - "/team/update", - "/team/delete", - "/team/list", - "/v2/team/list", - "/team/info", - "/team/block", - "/team/unblock", - "/team/available", - "/team/permissions_list", - "/team/permissions_update", - "/team/daily/activity", - # model - "/model/new", - "/model/update", - "/model/delete", - "/model/info", - "/jwt/key/mapping/new", - "/jwt/key/mapping/update", - "/jwt/key/mapping/delete", - "/jwt/key/mapping/list", - "/jwt/key/mapping/info", - ] + key_management_routes + mcp_management_routes + management_routes = ( + [ + # user + "/user/new", + "/user/update", + "/user/bulk_update", + "/user/delete", + "/user/info", + "/user/list", + "/user/daily/activity", + "/user/daily/activity/aggregated", + # team + "/team/new", + "/team/update", + "/team/delete", + "/team/list", + "/v2/team/list", + "/team/info", + "/team/block", + "/team/unblock", + "/team/available", + "/team/permissions_list", + "/team/permissions_update", + "/team/daily/activity", + # model + "/model/new", + "/model/update", + "/model/delete", + "/model/info", + "/jwt/key/mapping/new", + "/jwt/key/mapping/update", + "/jwt/key/mapping/delete", + "/jwt/key/mapping/list", + "/jwt/key/mapping/info", + ] + + key_management_routes + + mcp_management_routes + ) spend_tracking_routes = [ # spend