mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-03 02:22:24 +00:00
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.
This commit is contained in:
parent
16bc5272ec
commit
82e69edd3e
2 changed files with 131 additions and 2 deletions
|
|
@ -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
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
):
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue