From 82e69edd3e6ea6c8acab6dd404542fedaf75c809 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Thu, 4 Jun 2026 21:37:58 +0000 Subject: [PATCH] mcp: decrypt env_vars on Prisma create/update rows that return JSON strings Prisma may hand back env_vars as a raw JSON string on write paths instead of a parsed list. The previous decrypt_global_env_var_values call iterated the string character-by-character (silent no-op), leaving global secrets encrypted on the returned row. add_server then seeded the registry with ciphertext, and the immediately-following reload_servers_from_database reused that broken entry because updated_at matched, so headers forwarded ciphertext upstream until a later edit changed updated_at. Wrap the create/update decrypt in _decrypt_env_vars_on_returned_row, which parses a string payload back into a list before the in-place decrypt runs and writes it back onto the row. Also harden _reencrypt_global_env_var_values against the same string shape so master-key rotation doesn't crash on dict(v) over a string. --- litellm/proxy/_experimental/mcp_server/db.py | 40 +++++++- .../mcp_server/test_mcp_env_vars.py | 93 +++++++++++++++++++ 2 files changed, 131 insertions(+), 2 deletions(-) diff --git a/litellm/proxy/_experimental/mcp_server/db.py b/litellm/proxy/_experimental/mcp_server/db.py index c7ee56e37e6..da1baa1c128 100644 --- a/litellm/proxy/_experimental/mcp_server/db.py +++ b/litellm/proxy/_experimental/mcp_server/db.py @@ -91,6 +91,35 @@ def decrypt_global_env_var_values(env_vars: Optional[Iterable[Any]]) -> None: entry.value = decrypted +def _decrypt_env_vars_on_returned_row(row: Any) -> None: + """Decrypt ``scope="global"`` env var values on a row returned by Prisma create/update. + + Prisma may hand back ``env_vars`` either as a parsed list (the common case for + JSONB columns) or as a raw JSON string (observed for some write paths). The + in-place decrypt helper only mutates iterables of dicts/models, so a string + payload would silently skip decryption and ciphertext would leak into the + registry via ``add_server``/``update_server`` (which trust the caller). + Parse the string back to a list so the in-place decrypt actually runs, and + write the decrypted list back onto the row so downstream consumers see plain + values. + """ + env_vars = getattr(row, "env_vars", None) + if env_vars is None: + return + if isinstance(env_vars, str): + try: + env_vars = json.loads(env_vars) + except (json.JSONDecodeError, TypeError): + return + if not isinstance(env_vars, list): + return + try: + setattr(row, "env_vars", env_vars) + except (AttributeError, TypeError): + pass + decrypt_global_env_var_values(env_vars) + + def _reencrypt_global_env_var_values( env_vars: Optional[Iterable[Any]], new_encryption_key: str ) -> Optional[List[Dict[str, Any]]]: @@ -104,6 +133,13 @@ def _reencrypt_global_env_var_values( """ if not env_vars: return None + if isinstance(env_vars, str): + try: + env_vars = json.loads(env_vars) + except (json.JSONDecodeError, TypeError): + return None + if not env_vars: + return None rebuilt = [dict(v) for v in env_vars] rotated = False for entry in rebuilt: @@ -567,7 +603,7 @@ async def create_mcp_server( data=data_dict # type: ignore ) - decrypt_global_env_var_values(getattr(new_mcp_server, "env_vars", None)) + _decrypt_env_vars_on_returned_row(new_mcp_server) return new_mcp_server @@ -645,7 +681,7 @@ async def update_mcp_server( where={"server_id": data.server_id}, data=data_dict # type: ignore ) - decrypt_global_env_var_values(getattr(updated_mcp_server, "env_vars", None)) + _decrypt_env_vars_on_returned_row(updated_mcp_server) return updated_mcp_server diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py index f09bf7497ba..ebba154e37c 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_env_vars.py @@ -1315,6 +1315,99 @@ async def test_add_server_does_not_double_decrypt_global_env_vars(env_vars_salt_ assert headers == {"X-Db": "s3cr3t-p@ss"} +@pytest.mark.asyncio +async def test_create_mcp_server_decrypts_env_vars_when_prisma_returns_json_string( + env_vars_salt_key, +): + """Regression for the reload-reuse path: Prisma can hand back ``env_vars`` on + a write as the raw JSON string that was persisted, not a parsed list. The + create/update wrappers must still decrypt globals on the returned row, else + ``add_server`` (which trusts the caller) seeds the registry with ciphertext + and the subsequent ``reload_servers_from_database`` reuses that broken entry + (timestamps match), so headers forward ciphertext upstream.""" + import json + from unittest.mock import AsyncMock, MagicMock + + from litellm.proxy._experimental.mcp_server.db import ( + _prepare_mcp_server_data, + create_mcp_server, + update_mcp_server, + ) + from litellm.proxy._types import ( + MCPEnvVar, + NewMCPServerRequest, + UpdateMCPServerRequest, + ) + + req = _global_env_var_server_request( + [MCPEnvVar(name="DB_PASSWORD", value="s3cr3t-p@ss", scope="global")] + ) + encrypted_env_vars_str = _prepare_mcp_server_data(req)["env_vars"] + assert "s3cr3t-p@ss" not in encrypted_env_vars_str + + def _prisma_row_with_json_string_env_vars(): + row = MagicMock() + row.env_vars = encrypted_env_vars_str + return row + + mock_prisma = MagicMock() + mock_prisma.db.litellm_mcpservertable.create = AsyncMock( + return_value=_prisma_row_with_json_string_env_vars() + ) + + created = await create_mcp_server( + mock_prisma, + NewMCPServerRequest( + server_id="srv-create", + url="https://upstream.example.com/mcp", + transport="http", + ), + touched_by="test-user", + ) + assert isinstance(created.env_vars, list) + assert created.env_vars[0]["value"] == "s3cr3t-p@ss" + + mock_prisma_upd = MagicMock() + mock_prisma_upd.db.litellm_mcpservertable.update = AsyncMock( + return_value=_prisma_row_with_json_string_env_vars() + ) + updated = await update_mcp_server( + mock_prisma_upd, + UpdateMCPServerRequest(server_id="srv-update"), + touched_by="test-user", + ) + assert isinstance(updated.env_vars, list) + assert updated.env_vars[0]["value"] == "s3cr3t-p@ss" + + +def test_reencrypt_global_env_var_values_handles_json_string(env_vars_salt_key): + """``rotate_mcp_server_credentials_master_key`` reads ``mcp_server.env_vars`` + straight off the Prisma row, which can be a JSON string. The re-encrypt + helper must parse it instead of failing on ``dict(v)`` over a string.""" + import json + + from litellm.proxy._experimental.mcp_server.db import ( + _prepare_mcp_server_data, + _reencrypt_global_env_var_values, + ) + from litellm.proxy._types import MCPEnvVar + + req = _global_env_var_server_request( + [MCPEnvVar(name="DB_PASSWORD", value="s3cr3t-p@ss", scope="global")] + ) + encrypted_env_vars_str = _prepare_mcp_server_data(req)["env_vars"] + original_ciphertext = json.loads(encrypted_env_vars_str)[0]["value"] + + rebuilt = _reencrypt_global_env_var_values( + encrypted_env_vars_str, new_encryption_key="rotated-master-key-0000" + ) + + assert rebuilt is not None + assert rebuilt[0]["name"] == "DB_PASSWORD" + assert rebuilt[0]["value"] != original_ciphertext + assert rebuilt[0]["value"] != "s3cr3t-p@ss" + + def test_decrypt_global_env_var_drops_undecryptable_value( env_vars_salt_key, monkeypatch ):