From 61962af02788f1748a9f6d1a386e2f35aeccf4b3 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 4 Jun 2026 03:11:09 +0000 Subject: [PATCH] fix(mcp): scope per-user env var overrides to currently user-scoped vars A stored per-user value for a var that the admin later switched to global scope was still able to override the admin's global value, because the header merge applied the full user blob over the globals. Filter the user blob to vars that are currently user-scoped and let admin globals win. --- .../mcp_server/mcp_server_manager.py | 8 ++++- .../mcp_server/test_mcp_env_vars.py | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) 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 ────────────────────────────────────────