From 9256b5e6206ee4ec6c53033c4c32a0d33ce9c2cf Mon Sep 17 00:00:00 2001 From: Tin Chi Lo Date: Fri, 19 Jun 2026 10:10:54 -0700 Subject: [PATCH] refactor(mcp/v2): drop mis-modeled PerUserEnvVar api_key key_source Per-user env-vars are templated static headers (resolved alongside auth via the static-headers path), not an api_key credential mode; the PerUserEnvVar key_source was unreachable (no server config maps to it) and contradicted that model. ApiKeySource is now SharedKey | Byok and the api_key arm matches the two real sources. Static/env-var header resolution reuses v1's _resolve_static_headers_with_env_vars at the v2 manager (step 6); the clean v2 rewrite lands at v1 retirement alongside the caching model. --- .../gateway/mcp/outbound_credentials/resolver.py | 13 ------------- .../gateway/mcp/outbound_credentials/types.py | 13 +------------ tests/mcp_tests/gateway/test_resolver.py | 15 ++------------- 3 files changed, 3 insertions(+), 38 deletions(-) diff --git a/litellm/proxy/gateway/mcp/outbound_credentials/resolver.py b/litellm/proxy/gateway/mcp/outbound_credentials/resolver.py index 5231f952f2c..f04ad64f157 100644 --- a/litellm/proxy/gateway/mcp/outbound_credentials/resolver.py +++ b/litellm/proxy/gateway/mcp/outbound_credentials/resolver.py @@ -44,7 +44,6 @@ from .types import ( CredError, NoneConfig, PassthroughConfig, - PerUserEnvVar, ServerSpec, SharedKey, Subject, @@ -112,18 +111,6 @@ class UpstreamCredentialProvider: # The shared key is read straight from ServerSpec.config, not the per-user # store; it is the same credential for every caller. return Ok(_api_key_auth(config, source.value.get_secret_value())) - case PerUserEnvVar(): - fetched = await self._per_user_value(subject, server) - if isinstance(fetched, Error): - return Error(fetched.error) # store/DB down -> 503 - value = fetched.ok - if value is None: - return Error( - CredError.of_precondition_required( - "api_key: per-user env var not set for this subject" - ) - ) - return Ok(_api_key_auth(config, value)) case Byok(): fetched = await self._per_user_value(subject, server) if isinstance(fetched, Error): diff --git a/litellm/proxy/gateway/mcp/outbound_credentials/types.py b/litellm/proxy/gateway/mcp/outbound_credentials/types.py index fc36334bb6e..2ee085e9f8d 100644 --- a/litellm/proxy/gateway/mcp/outbound_credentials/types.py +++ b/litellm/proxy/gateway/mcp/outbound_credentials/types.py @@ -188,15 +188,6 @@ class SharedKey(BaseModel): value: SecretStr -class PerUserEnvVar(BaseModel): - """A per-user value the admin templated as an env var; the user fills it in. Pulled from - the credential store at resolve time. Missing means the user has not completed setup, a - precondition (412) rather than an auth failure.""" - - model_config = ConfigDict(frozen=True) - source: Literal["per_user_env_var"] = "per_user_env_var" - - class Byok(BaseModel): """A key the user brings via the entry flow, stored per-user. Pulled from the credential store at resolve time. Missing means the user must provide it, a 401 + WWW-Authenticate @@ -206,9 +197,7 @@ class Byok(BaseModel): source: Literal["byok"] = "byok" -ApiKeySource = Annotated[ - SharedKey | PerUserEnvVar | Byok, Field(discriminator="source") -] +ApiKeySource = Annotated[SharedKey | Byok, Field(discriminator="source")] class ApiKeyConfig(BaseModel): diff --git a/tests/mcp_tests/gateway/test_resolver.py b/tests/mcp_tests/gateway/test_resolver.py index e599396257d..964d3546020 100644 --- a/tests/mcp_tests/gateway/test_resolver.py +++ b/tests/mcp_tests/gateway/test_resolver.py @@ -59,7 +59,6 @@ from litellm.proxy.gateway.mcp.outbound_credentials.types import ( CredError, NoneConfig, PassthroughConfig, - PerUserEnvVar, ServerSpec, SharedKey, StaticKeys, @@ -295,13 +294,12 @@ def test_secret_fields_are_masked_in_serialization(): assert config.key_source.value.get_secret_value() == "SUPER-SECRET" -@pytest.mark.parametrize("source", [Byok(), PerUserEnvVar()]) -async def test_api_key_per_user_pulls_the_subject_credential(source: object): +async def test_api_key_byok_pulls_the_subject_credential(): store = InMemoryCredentialStore( {CredentialKey(tenant_id="t1", subject_id="u1", server_id="s1"): "user-secret"} ) provider = _provider(credential_store=store) - result = await provider.resolve(SUBJECT, _spec(ApiKeyConfig(key_source=source))) # type: ignore[arg-type] + result = await provider.resolve(SUBJECT, _spec(ApiKeyConfig(key_source=Byok()))) assert isinstance(result, Ok) assert _applied_headers(result.ok)["Authorization"] == "Bearer user-secret" @@ -313,15 +311,6 @@ async def test_api_key_byok_missing_returns_unauthorized(): assert result.error.tag == "unauthorized" -async def test_api_key_env_var_missing_returns_precondition_required(): - # Missing per-user env var -> 412 (a setup precondition), distinct from BYOK's 401. - result = await PROVIDER.resolve( - SUBJECT, _spec(ApiKeyConfig(key_source=PerUserEnvVar())) - ) - assert isinstance(result, Error) - assert result.error.tag == "precondition_required" - - async def test_api_key_per_user_isolated_by_subject(): # The stored key belongs to (t1,u1,s1); a different subject must not receive it. store = InMemoryCredentialStore(