diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 38ed6ffd67f..4af4a6aff07 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -1591,7 +1591,13 @@ class MCPServerManager: setup_url=build_env_var_setup_url(server.server_id), ) - merged_vars: Dict[str, str] = {**global_values, **user_values} + # Only honor stored user values for currently user-scoped vars, and let + # admin globals win, so a stale row from when a var was user-scoped can + # never override the global value the admin set after switching it. + scoped_user_values = { + name: value for name, value in user_values.items() if name in user_var_names + } + merged_vars: Dict[str, str] = {**scoped_user_values, **global_values} if not static_headers: return static_headers return interpolate_headers(static_headers, merged_vars) 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 d48beeb9069..52db2f4de9a 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 @@ -366,6 +366,42 @@ async def test_resolve_static_headers_unreferenced_user_var_is_not_blocking( assert headers == {"X-Static": "ok"} +@pytest.mark.asyncio +async def test_resolve_static_headers_stale_user_value_cannot_override_global( + monkeypatch, +): + """A var that used to be user-scoped (so the user has a stored value) but is + now global must resolve to the admin's global value, not the stale per-user + row. Otherwise a user could override admin-configured headers indefinitely.""" + from litellm.proxy._experimental.mcp_server.mcp_server_manager import ( + MCPServerManager, + ) + from litellm.types.mcp_server.mcp_server_manager import MCPServer + + manager = MCPServerManager() + server = MCPServer( + server_id="srv-4", + name="srv4", + transport="http", + url="https://example.com", + static_headers={"X-DB-URL": "${DB_HOST}/${CORP_USERNAME}"}, + env_vars=[ + # DB_HOST is now global; it used to be user-scoped. + {"name": "DB_HOST", "value": "admin-db", "scope": "global"}, + {"name": "CORP_USERNAME", "value": "", "scope": "user"}, + ], + ) + + async def fake_load_user_env_vars(server, user_api_key_auth): + # Stale DB_HOST row left over from when it was user-scoped. + return {"DB_HOST": "evil-db", "CORP_USERNAME": "alice"} + + monkeypatch.setattr(manager, "_load_user_env_vars", fake_load_user_env_vars) + + headers = await manager._resolve_static_headers_with_env_vars(server, object()) + assert headers == {"X-DB-URL": "admin-db/alice"} + + # ── _load_user_env_vars guard paths ────────────────────────────────────────