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.
This commit is contained in:
mateo-berri 2026-06-03 20:06:10 +00:00
parent 102b73a261
commit 1f1bf4c421
No known key found for this signature in database
3 changed files with 41 additions and 5 deletions

View file

@ -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={})

View file

@ -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()

View file

@ -10119,7 +10119,7 @@ export const listMCPUserCredentials = async (
export const getMCPUserEnvVars = async (
accessToken: string,
serverId: string,
): Promise<MCPUserEnvVarsStatus | null> => {
): Promise<MCPUserEnvVarsStatus> => {
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();
};