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.
This commit is contained in:
mateo-berri 2026-06-04 03:11:09 +00:00
parent cc6e6deb98
commit 61962af027
No known key found for this signature in database
2 changed files with 43 additions and 1 deletions

View file

@ -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)

View file

@ -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 ────────────────────────────────────────