fix(mcp): don't re-decrypt already-decrypted global env vars in registry build

The db.py read/write helpers (get_mcp_server, create_mcp_server,
update_mcp_server) decrypt global env var values in place before returning,
while leaving credentials encrypted. add_server/update_server then passed
those records to build_mcp_server_from_table with the default
credentials_are_encrypted=True, which decrypted the global env var values a
second time. Decrypting an already-plaintext value (e.g. "postgresql")
fails and zeroes it, so the registry entry forwarded the raw ${NAME}
placeholder upstream instead of the interpolated secret, and reload's
updated_at-equality reuse kept the broken entry.

Add an env_vars_are_encrypted flag to build_mcp_server_from_table (defaulting
to credentials_are_encrypted) and have add_server/update_server pass
env_vars_are_encrypted=False so global env var values are decrypted exactly
once.
This commit is contained in:
mateo-berri 2026-06-04 12:43:43 +00:00
parent ad07d1476f
commit 4576c48660
No known key found for this signature in database
2 changed files with 68 additions and 5 deletions

View file

@ -904,10 +904,10 @@ class MCPServerManager:
self,
mcp_server: LiteLLM_MCPServerTable,
*,
credentials_are_encrypted: bool,
env_vars_are_encrypted: bool,
) -> Optional[List[Dict[str, Any]]]:
env_vars_list = _deserialize_json_list(getattr(mcp_server, "env_vars", None))
if credentials_are_encrypted:
if env_vars_are_encrypted:
from litellm.proxy._experimental.mcp_server.db import ( # noqa: PLC0415
decrypt_global_env_var_values,
)
@ -920,6 +920,7 @@ class MCPServerManager:
mcp_server: LiteLLM_MCPServerTable,
*,
credentials_are_encrypted: bool = True,
env_vars_are_encrypted: Optional[bool] = None,
) -> MCPServer:
_mcp_info: MCPInfo = mcp_server.mcp_info or {}
env_dict = _deserialize_json_dict(getattr(mcp_server, "env", None))
@ -927,7 +928,12 @@ class MCPServerManager:
getattr(mcp_server, "static_headers", None)
)
env_vars_list = self._resolve_env_vars_list(
mcp_server, credentials_are_encrypted=credentials_are_encrypted
mcp_server,
env_vars_are_encrypted=(
credentials_are_encrypted
if env_vars_are_encrypted is None
else env_vars_are_encrypted
),
)
credentials_dict = _deserialize_json_dict(
getattr(mcp_server, "credentials", None)
@ -1126,7 +1132,14 @@ class MCPServerManager:
return
try:
if mcp_server.server_id not in self.registry:
new_server = await self.build_mcp_server_from_table(mcp_server)
# Callers hand us a record returned by the db.py read/write
# helpers, which already decrypt global env var values (the
# `credentials` field is the only one still encrypted here).
# Re-decrypting plaintext would zero the values, so build with
# env_vars_are_encrypted=False.
new_server = await self.build_mcp_server_from_table(
mcp_server, env_vars_are_encrypted=False
)
self._assign_unique_short_prefix(new_server)
self.registry[mcp_server.server_id] = new_server
await self._maybe_register_openapi_tools(new_server)
@ -1149,7 +1162,11 @@ class MCPServerManager:
return
try:
if mcp_server.server_id in self.registry:
new_server = await self.build_mcp_server_from_table(mcp_server)
# See add_server: db.py helpers already decrypted env var
# values, so don't decrypt them a second time here.
new_server = await self.build_mcp_server_from_table(
mcp_server, env_vars_are_encrypted=False
)
# Carry the previously-resolved short prefix across so the
# tool names stay stable for clients holding cached lists.
existing_prefix = self.registry[mcp_server.server_id].short_prefix

View file

@ -889,6 +889,52 @@ async def test_build_mcp_server_from_table_decrypts_global_env_vars(env_vars_sal
assert headers == {"X-Db": "s3cr3t-p@ss"}
@pytest.mark.asyncio
async def test_add_server_does_not_double_decrypt_global_env_vars(env_vars_salt_key):
"""The create/fetch endpoints hand ``add_server`` a record whose global env
var values were already decrypted by the db.py helpers (only ``credentials``
stays encrypted). Building the registry entry must not decrypt them a second
time: a second decrypt of an already-plaintext value (e.g. ``postgresql``)
fails and zeroes it, which would forward the raw ``${NAME}`` placeholder
upstream instead of the interpolated secret."""
import json
from litellm.proxy._experimental.mcp_server.db import (
_prepare_mcp_server_data,
decrypt_global_env_var_values,
)
from litellm.proxy._experimental.mcp_server.mcp_server_manager import (
MCPServerManager,
)
from litellm.proxy._types import LiteLLM_MCPServerTable, MCPEnvVar
req = _global_env_var_server_request(
[MCPEnvVar(name="DB_PASSWORD", value="s3cr3t-p@ss", scope="global")]
)
env_vars = json.loads(_prepare_mcp_server_data(req)["env_vars"])
# Mirror what create_mcp_server / get_mcp_server return to add_server.
decrypt_global_env_var_values(env_vars)
assert env_vars[0]["value"] == "s3cr3t-p@ss"
table = LiteLLM_MCPServerTable(
server_id="srv-add",
alias="echo",
url="https://upstream.example.com/mcp",
transport="http",
auth_type="none",
static_headers={"X-Db": "${DB_PASSWORD}"},
env_vars=env_vars,
approval_status="active",
)
manager = MCPServerManager()
await manager.add_server(table)
server = manager.registry["srv-add"]
headers = await manager._resolve_static_headers_with_env_vars(server, None)
assert headers == {"X-Db": "s3cr3t-p@ss"}
def test_decrypt_global_env_var_drops_undecryptable_value(
env_vars_salt_key, monkeypatch
):