diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index c4ad673b88f..dce63a39caf 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -118,6 +118,7 @@ from litellm.proxy.common_utils.encrypt_decrypt_utils import decrypt_value_helpe from litellm.proxy.common_utils.user_api_key_cache import get_management_object_ttl from litellm.proxy.utils import ProxyLogging, get_server_root_path from litellm.repositories.table_repositories import MCPServerRepository +from litellm.secret_managers.main import get_secret_str from litellm.types.llms.custom_http import httpxSpecialProvider from litellm.types.mcp import DEFAULT_SUBJECT_TOKEN_TYPE, MCPAuth, MCPStdioConfig from litellm.types.mcp_server.mcp_server_manager import ( @@ -536,6 +537,29 @@ def _deserialize_json_dict(data: Any) -> Optional[dict[str, str]]: return data +def _resolve_os_environ_in_stdio_env( + env: Optional[dict[str, str]], +) -> Optional[dict[str, str]]: + """Resolve ``os.environ/VAR`` references in a DB/API-registered stdio ``env`` map. + + config.yaml MCP servers resolve ``os.environ/`` at load via the proxy config + loader, but DB-backed servers deserialize ``env`` verbatim, so without this the + literal ``os.environ/...`` string reaches the subprocess and upstream auth fails. + A reference to an unset variable is dropped rather than forwarded as the literal + placeholder. + """ + if not env: + return env + + def _resolve(value: str) -> Optional[str]: + if value.startswith("os.environ/"): + return get_secret_str(value) + return value + + resolved = ((key, _resolve(value)) for key, value in env.items()) + return {key: value for key, value in resolved if value is not None} + + def _deserialize_json_list(data: Any) -> Optional[list[dict[str, Any]]]: """Deserialize a JSON array stored in the DB (``env_vars`` and friends). @@ -1310,7 +1334,7 @@ class MCPServerManager: 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)) + env_dict = _resolve_os_environ_in_stdio_env(_deserialize_json_dict(getattr(mcp_server, "env", None))) static_headers_dict = _deserialize_json_dict(getattr(mcp_server, "static_headers", None)) env_vars_list = self._resolve_env_vars_list( mcp_server, diff --git a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py index 12e22de195b..f848d00b677 100644 --- a/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py +++ b/tests/test_litellm/proxy/_experimental/mcp_server/test_mcp_server_manager.py @@ -4348,6 +4348,59 @@ class TestMCPServerTimestamps: assert mcp_server.created_at == created assert mcp_server.updated_at == updated + @pytest.mark.asyncio + async def test_build_mcp_server_from_table_resolves_os_environ_in_stdio_env(self, monkeypatch): + """DB/API-registered stdio servers must resolve os.environ/ in the env map (#32677). + + A literal os.environ/VAR reaching the subprocess is the bug: upstream auth fails + because the child sees the placeholder string instead of the secret. + """ + monkeypatch.setenv("GITHUB_TOKEN", "ghp_secret_value") + manager = MCPServerManager() + + record = LiteLLM_MCPServerTable( + server_id="stdio-osenviron-1", + server_name="stdio_osenviron", + url=None, + transport=MCPTransport.stdio, + command="npx", + args=["-y", "@modelcontextprotocol/server-github"], + env={ + "GITHUB_PERSONAL_ACCESS_TOKEN": "os.environ/GITHUB_TOKEN", + "PLAIN": "literal-value", + }, + ) + + server = await manager.build_mcp_server_from_table(record, credentials_are_encrypted=False) + + assert server.env == { + "GITHUB_PERSONAL_ACCESS_TOKEN": "ghp_secret_value", + "PLAIN": "literal-value", + } + + @pytest.mark.asyncio + async def test_build_mcp_server_from_table_drops_unset_os_environ_stdio_env(self, monkeypatch): + """An os.environ/ reference to an unset variable is dropped, never forwarded as the literal placeholder.""" + monkeypatch.delenv("DEFINITELY_UNSET_MCP_VAR", raising=False) + manager = MCPServerManager() + + record = LiteLLM_MCPServerTable( + server_id="stdio-osenviron-2", + server_name="stdio_osenviron_unset", + url=None, + transport=MCPTransport.stdio, + command="npx", + args=["-y", "@modelcontextprotocol/server-github"], + env={ + "MISSING": "os.environ/DEFINITELY_UNSET_MCP_VAR", + "PLAIN": "literal-value", + }, + ) + + server = await manager.build_mcp_server_from_table(record, credentials_are_encrypted=False) + + assert server.env == {"PLAIN": "literal-value"} + @pytest.mark.asyncio async def test_build_mcp_server_from_table_reads_token_endpoint_auth_method(self): """token_endpoint_auth_method stored in the credentials JSON is loaded onto the MCPServer (LIT-4091)."""