From 8852c0df9580efb35f1d273bfc54ab12e0304d27 Mon Sep 17 00:00:00 2001
From: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
Date: Wed, 3 Jun 2026 18:17:50 -0700
Subject: [PATCH] fix(mcp): preserve env_vars in registry to table conversion

The GET /v1/mcp/server list and health endpoints build LiteLLM_MCPServerTable
from the in-memory registry via _build_mcp_server_table and health_check_server.
Both copied static_headers but dropped env_vars, so the list always returned
env_vars: null. The admin edit form is populated from that list data, so it
loaded an empty env-var list; saving any edit then persisted env_vars: [],
silently wiping the stored variables. With nothing left to interpolate, the
${VAR} static headers were forwarded upstream verbatim as literal text.

Carry env_vars through both conversions, mirroring static_headers. Add
regression tests asserting both paths round-trip name, scope, value, and
description.
---
 .../mcp_server/mcp_server_manager.py          |  2 +
 .../mcp_server/test_mcp_server_manager.py     | 60 +++++++++++++++++++
 2 files changed, 62 insertions(+)

diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
index 32aa273c548..b395fc9ed40 100644
--- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
+++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py
@@ -3957,6 +3957,7 @@ class MCPServerManager:
             extra_headers=server.extra_headers or [],
             mcp_info=server.mcp_info,
             static_headers=server.static_headers,
+            env_vars=server.env_vars,
             status=status,
             last_health_check=datetime.now(),
             health_check_error=health_check_error,
@@ -4049,6 +4050,7 @@ class MCPServerManager:
             extra_headers=server.extra_headers or [],
             mcp_info=server.mcp_info,
             static_headers=server.static_headers,
+            env_vars=server.env_vars,
             status=None,  # No health check performed
             last_health_check=None,  # No health check performed
             health_check_error=None,
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 a333930a149..18fac1c72be 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
@@ -4038,5 +4038,65 @@ class TestApprovalStatusGate:
         assert "never-seen" not in manager.registry
 
 
+class TestRegistryTableConversionPreservesEnvVars:
+    """The registry ``MCPServer`` -> ``LiteLLM_MCPServerTable`` conversions back
+    the GET /v1/mcp/server list and health responses, which populate the admin
+    edit form. When they dropped ``env_vars`` the form loaded an empty list and
+    saving any edit silently wiped the stored vars, so ``${VAR}`` static headers
+    were forwarded upstream un-interpolated.
+    """
+
+    @staticmethod
+    def _server_with_env_vars() -> MCPServer:
+        return MCPServer(
+            server_id="env-vars-server",
+            name="env_vars_server",
+            url="https://example.com/mcp",
+            transport=MCPTransport.http,
+            auth_type=MCPAuth.oauth2,
+            static_headers={"X-Db-Url": "${DB_PROTOCOL}://${CORP_USER}@${DB_HOST}"},
+            env_vars=[
+                {
+                    "name": "DB_PROTOCOL",
+                    "value": "postgresql",
+                    "scope": "global",
+                    "description": None,
+                },
+                {
+                    "name": "CORP_USER",
+                    "value": "",
+                    "scope": "user",
+                    "description": "Your DB username",
+                },
+            ],
+        )
+
+    @staticmethod
+    def _assert_env_vars_round_tripped(table: LiteLLM_MCPServerTable) -> None:
+        assert table.env_vars is not None
+        by_name = {entry.name: entry for entry in table.env_vars}
+        assert set(by_name) == {"DB_PROTOCOL", "CORP_USER"}
+        assert by_name["DB_PROTOCOL"].scope == MCPEnvVarScope.global_
+        assert by_name["DB_PROTOCOL"].value == "postgresql"
+        assert by_name["CORP_USER"].scope == MCPEnvVarScope.user
+        assert by_name["CORP_USER"].description == "Your DB username"
+
+    def test_build_mcp_server_table_preserves_env_vars(self):
+        manager = MCPServerManager()
+        table = manager._build_mcp_server_table(self._server_with_env_vars())
+        self._assert_env_vars_round_tripped(table)
+
+    async def test_health_check_server_preserves_env_vars(self):
+        # OAuth2 without client credentials needs a per-user token, so the
+        # health check is skipped (no network) and we exercise the table
+        # construction path directly.
+        manager = MCPServerManager()
+        server = self._server_with_env_vars()
+        assert server.requires_per_user_auth is True
+        manager.registry[server.server_id] = server
+        table = await manager.health_check_server(server.server_id)
+        self._assert_env_vars_round_tripped(table)
+
+
 if __name__ == "__main__":
     pytest.main([__file__])