fix(mcp): drop orphaned per-user credential rows when an MCP server is deleted (#30141)

litellm/proxy/_experimental/mcp_server/db.py

@@ -568,10 +568,12 @@ async def delete_mcp_server(
     """
     Delete the mcp server from the db by server_id
 
-    The server-row delete is the commit point. Per-user env var rows have no FK
-    cascade, so they are cleaned up afterwards on a best-effort basis: a transient
-    failure there leaves only orphaned rows pointing at a now-missing server and
-    must not turn a successful delete into a caller-visible error.
+    The server-row delete is the commit point. Per-user credential and env var
+    rows have no FK cascade, so they are cleaned up afterwards on a best-effort
+    basis: a transient failure there leaves only orphaned rows pointing at a
+    now-missing server and must not turn a successful delete into a
+    caller-visible error. Each table is cleaned independently so a failure on one
+    still attempts the other.
 
     Returns the deleted mcp server record if it exists, otherwise None
     """
@@ -581,17 +583,20 @@ async def delete_mcp_server(
         },
     )
     if deleted_server is not None:
-        try:
-            await prisma_client.db.litellm_mcpuserenvvars.delete_many(
-                where={"server_id": server_id}
-            )
-        except Exception as e:
-            verbose_proxy_logger.warning(
-                "MCP server %s deleted but per-user env var cleanup failed; "
-                "orphaned rows can be removed on a later delete: %s",
-                server_id,
-                e,
-            )
+        for model, label in (
+            (prisma_client.db.litellm_mcpusercredentials, "credential"),
+            (prisma_client.db.litellm_mcpuserenvvars, "env var"),
+        ):
+            try:
+                await model.delete_many(where={"server_id": server_id})
+            except Exception as e:
+                verbose_proxy_logger.warning(
+                    "MCP server %s deleted but per-user %s cleanup failed; "
+                    "orphaned rows can be removed on a later delete: %s",
+                    server_id,
+                    label,
+                    e,
+                )
     return deleted_server
 
 

tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py

@@ -872,6 +872,7 @@ def _mock_env_vars_prisma(row=None):
     prisma.db.litellm_mcpuserenvvars.find_many = AsyncMock(return_value=[])
     prisma.db.litellm_mcpuserenvvars.upsert = AsyncMock()
     prisma.db.litellm_mcpuserenvvars.delete_many = AsyncMock()
+    prisma.db.litellm_mcpusercredentials.delete_many = AsyncMock()
     return prisma
 
 
@@ -1252,6 +1253,64 @@ async def test_delete_mcp_server_succeeds_when_orphan_cleanup_fails():
     prisma.db.litellm_mcpuserenvvars.delete_many.assert_awaited_once()
 
 
+@pytest.mark.asyncio
+async def test_delete_mcp_server_removes_orphaned_user_credentials():
+    """Deleting a server must also drop every user's stored BYOK/OAuth credential
+    rows for it; there is no FK cascade, so skipping this leaves encrypted secrets
+    pointing at a now-missing server."""
+    from unittest.mock import AsyncMock
+
+    from litellm.proxy._experimental.mcp_server.db import delete_mcp_server
+
+    prisma = _mock_env_vars_prisma()
+    prisma.db.litellm_mcpservertable.delete = AsyncMock(return_value=object())
+
+    await delete_mcp_server(prisma, "srv-1")
+
+    prisma.db.litellm_mcpusercredentials.delete_many.assert_awaited_once()
+    call = prisma.db.litellm_mcpusercredentials.delete_many.call_args
+    assert call.kwargs["where"] == {"server_id": "srv-1"}
+
+
+@pytest.mark.asyncio
+async def test_delete_mcp_server_skips_credential_cleanup_when_server_missing():
+    """A no-op delete (server not found) must not touch the credential table."""
+    from unittest.mock import AsyncMock
+
+    from litellm.proxy._experimental.mcp_server.db import delete_mcp_server
+
+    prisma = _mock_env_vars_prisma()
+    prisma.db.litellm_mcpservertable.delete = AsyncMock(return_value=None)
+
+    result = await delete_mcp_server(prisma, "srv-1")
+
+    assert result is None
+    prisma.db.litellm_mcpusercredentials.delete_many.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_delete_mcp_server_credential_cleanup_failure_still_cleans_env_vars():
+    """Each per-user table is cleaned independently: a failure dropping credential
+    rows must not skip the env var cleanup (or vice versa), and the delete must
+    still succeed for the caller."""
+    from unittest.mock import AsyncMock
+
+    from litellm.proxy._experimental.mcp_server.db import delete_mcp_server
+
+    deleted = object()
+    prisma = _mock_env_vars_prisma()
+    prisma.db.litellm_mcpservertable.delete = AsyncMock(return_value=deleted)
+    prisma.db.litellm_mcpusercredentials.delete_many = AsyncMock(
+        side_effect=Exception("connection pool exhausted")
+    )
+
+    result = await delete_mcp_server(prisma, "srv-1")
+
+    assert result is deleted
+    prisma.db.litellm_mcpusercredentials.delete_many.assert_awaited_once()
+    prisma.db.litellm_mcpuserenvvars.delete_many.assert_awaited_once()
+
+
 # ── DB helpers: global env vars encrypted at rest ─────────────────────────