Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 20 additions & 15 deletions litellm/proxy/_experimental/mcp_server/db.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
"""
Expand All @@ -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


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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 ─────────────────────────


Expand Down
Loading