From 1f1bf4c421fab741552e27275a9d4a8b5d26cfea Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 3 Jun 2026 20:06:10 +0000 Subject: [PATCH] fix(mcp): surface env-var fetch errors and authorize before existence check getMCPUserEnvVars now throws on non-2xx so UserEnvVarsModal reports the error instead of silently rendering the empty 'no per-user fields' state. The per-user env-var endpoints now run the access check before the server lookup so a non-admin cannot tell a missing server (404) apart from one they lack access to (403), closing a server-id enumeration leak. --- .../mcp_management_endpoints.py | 6 ++-- .../test_mcp_management_endpoints.py | 28 +++++++++++++++++++ .../src/components/networking.tsx | 12 ++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/litellm/proxy/management_endpoints/mcp_management_endpoints.py b/litellm/proxy/management_endpoints/mcp_management_endpoints.py index 44ff1695145..d224dd262c9 100644 --- a/litellm/proxy/management_endpoints/mcp_management_endpoints.py +++ b/litellm/proxy/management_endpoints/mcp_management_endpoints.py @@ -2248,13 +2248,13 @@ if MCP_AVAILABLE: status_code=status.HTTP_400_BAD_REQUEST, detail={"error": "User ID not found in token"}, ) + await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) server = await get_mcp_server(prisma_client, server_id) if server is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail={"error": f"MCP Server {server_id} not found"}, ) - await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) stored = await get_user_env_vars(prisma_client, user_id, server_id) return _compute_user_env_var_status(server=server, stored_values=stored) @@ -2279,13 +2279,13 @@ if MCP_AVAILABLE: status_code=status.HTTP_400_BAD_REQUEST, detail={"error": "User ID not found in token"}, ) + await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) server = await get_mcp_server(prisma_client, server_id) if server is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail={"error": f"MCP Server {server_id} not found"}, ) - await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) # Filter to only known per-user var names declared by the admin — # never persist arbitrary keys the user invents. _, user_specs = parse_admin_env_vars(getattr(server, "env_vars", None)) @@ -2316,13 +2316,13 @@ if MCP_AVAILABLE: status_code=status.HTTP_400_BAD_REQUEST, detail={"error": "User ID not found in token"}, ) + await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) server = await get_mcp_server(prisma_client, server_id) if server is None: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail={"error": f"MCP Server {server_id} not found"}, ) - await _authorize_mcp_server_access(prisma_client, user_api_key_dict, server_id) await delete_user_env_vars(prisma_client, user_id, server_id) return _compute_user_env_var_status(server=server, stored_values={}) 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 383f0d166c0..4a2581e872a 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 @@ -4141,3 +4141,31 @@ class TestMCPUserEnvVarsAccessControl: ) assert result.server_id == "srv-1" access_list_mock.assert_not_awaited() + + @pytest.mark.asyncio + async def test_non_admin_gets_403_not_404_for_inaccessible_server(self): + """Authorization must run before the existence check so a non-admin + cannot distinguish "server does not exist" (404) from "server exists but + you lack access" (403) and enumerate server IDs.""" + get_mcp_server_mock = AsyncMock(return_value=None) + with ( + patch.object( + mgmt_endpoints, "get_prisma_client_or_throw", return_value=MagicMock() + ), + patch.object(mgmt_endpoints, "get_mcp_server", get_mcp_server_mock), + patch.object( + mgmt_endpoints, + "get_all_mcp_servers_for_user", + AsyncMock(return_value=[]), + ), + ): + with pytest.raises(HTTPException) as exc: + await mgmt_endpoints.get_mcp_user_env_vars( + server_id="srv-1", + user_api_key_dict=generate_mock_user_api_key_auth( + user_id="alice", + user_role=LitellmUserRoles.INTERNAL_USER, + ), + ) + assert exc.value.status_code == 403 + get_mcp_server_mock.assert_not_awaited() diff --git a/ui/litellm-dashboard/src/components/networking.tsx b/ui/litellm-dashboard/src/components/networking.tsx index 0e9e262722d..8b6475cf58c 100644 --- a/ui/litellm-dashboard/src/components/networking.tsx +++ b/ui/litellm-dashboard/src/components/networking.tsx @@ -10119,7 +10119,7 @@ export const listMCPUserCredentials = async ( export const getMCPUserEnvVars = async ( accessToken: string, serverId: string, -): Promise => { +): Promise => { const url = proxyBaseUrl ? `${proxyBaseUrl}/v1/mcp/server/${serverId}/user-env-vars` : `/v1/mcp/server/${serverId}/user-env-vars`; @@ -10127,7 +10127,15 @@ export const getMCPUserEnvVars = async ( method: "GET", headers: { [globalLitellmHeaderName]: `Bearer ${accessToken}` }, }); - if (!response.ok) return null; + if (!response.ok) { + const err = await response.json().catch(() => ({})); + const detail = (err as { detail?: unknown })?.detail; + const message = + typeof detail === "string" + ? detail + : (detail as { error?: string })?.error || "Failed to load env vars"; + throw new Error(message); + } return response.json(); };