From 02344057a2911ebb03874f9265ac62de3a2ccdfa Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 5 Jun 2026 16:43:40 +0000 Subject: [PATCH] fix(mcp): treat empty-valued global env var as unset in header resolution An empty scope=global value was keyed by presence, so it masked a referenced per-user var (suppressing the 412 prompt) and won the merge over a value the user did supply, interpolating an empty string into the header. Drop empty globals so they neither cover a required user var nor override a user value; the unresolved ${NAME} is then left untouched like any undefined reference. --- .../mcp_server/mcp_server_manager.py | 5 ++ .../mcp_server/test_mcp_env_vars.py | 78 +++++++++++++++++++ 2 files changed, 83 insertions(+) diff --git a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py index 8932f321c36..0e8997265f5 100644 --- a/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py +++ b/litellm/proxy/_experimental/mcp_server/mcp_server_manager.py @@ -1722,6 +1722,11 @@ class MCPServerManager: return static_headers global_values, user_specs = parse_admin_env_vars(env_vars) + # An empty-valued global is treated as unset: it must not mask a per-user + # var the user still has to supply, nor override a value the user did + # supply. The unresolved ${NAME} is then left untouched, like any other + # undefined reference. + global_values = {name: value for name, value in global_values.items() if value} user_var_names = {spec["name"] for spec in user_specs} # If no env vars are configured, return static_headers as-is. 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 7b411ec4f53..19065ff816b 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 @@ -490,6 +490,84 @@ async def test_resolve_static_headers_dual_scope_var_uses_global_without_412( assert load_calls == [] +@pytest.mark.asyncio +async def test_resolve_static_headers_empty_global_does_not_cover_user_var( + monkeypatch, +): + """An empty-valued global must not cover a referenced per-user var. The + global carries no usable value, so the tool-call path still raises a 412 + when the user hasn't supplied one, instead of silently interpolating an + empty string into the header.""" + 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-6", + name="srv6", + transport="http", + url="https://example.com", + static_headers={"Authorization": "Bearer ${SHARED_TOKEN}"}, + env_vars=[ + {"name": "SHARED_TOKEN", "value": "", "scope": "global"}, + {"name": "SHARED_TOKEN", "value": "", "scope": "user"}, + ], + ) + + async def fake_load_user_env_vars( + server, user_api_key_auth, *, force_refresh=False + ): + return {} + + monkeypatch.setattr(manager, "_load_user_env_vars", fake_load_user_env_vars) + + with pytest.raises(_u("MCPMissingUserEnvVarsError")) as exc: + await manager._resolve_static_headers_with_env_vars( + server, user_api_key_auth=object() + ) + assert exc.value.missing == ["SHARED_TOKEN"] + + +@pytest.mark.asyncio +async def test_resolve_static_headers_user_value_wins_over_empty_global( + monkeypatch, +): + """When a global is empty, a value the user did supply must win the merge + rather than being clobbered by the empty global. The header resolves to the + user's value, not an empty string.""" + 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-7", + name="srv7", + transport="http", + url="https://example.com", + static_headers={"Authorization": "Bearer ${SHARED_TOKEN}"}, + env_vars=[ + {"name": "SHARED_TOKEN", "value": "", "scope": "global"}, + {"name": "SHARED_TOKEN", "value": "", "scope": "user"}, + ], + ) + + async def fake_load_user_env_vars( + server, user_api_key_auth, *, force_refresh=False + ): + return {"SHARED_TOKEN": "user-secret"} + + monkeypatch.setattr(manager, "_load_user_env_vars", fake_load_user_env_vars) + + headers = await manager._resolve_static_headers_with_env_vars( + server, user_api_key_auth=object() + ) + assert headers == {"Authorization": "Bearer user-secret"} + + # ── health-check skip for per-user-env-var-backed headers ──────────────────