diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 377963f1b91..22f55406e37 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -4057,6 +4057,30 @@ def _swap_in_model_cost_map(new_model_cost_map: dict) -> int: return fetched_model_count +def _proxy_router_general_settings( + configured: RouterGeneralSettings | Mapping[str, Any] | None, +) -> RouterGeneralSettings: + """The proxy's RouterGeneralSettings, preserving operator config. + + ``async_only_mode`` is a proxy-runtime requirement -- only async clients are + initialised on this path -- so it is always forced on. Every other field the + operator set under ``router_settings.router_general_settings`` is kept. + + Before this existed the proxy passed a hardcoded + ``RouterGeneralSettings(async_only_mode=True)`` as an explicit keyword + alongside ``**router_params``, so an operator who set the key in config hit + "got multiple values for keyword argument" at startup and had no way to set + proxy-side router general settings at all (e.g. ``model_name_resolution``). + """ + if configured is None: + return RouterGeneralSettings(async_only_mode=True) + settings: Final = ( + RouterGeneralSettings(**configured) if isinstance(configured, Mapping) else configured.model_copy() + ) + settings.async_only_mode = True + return settings + + class ProxyConfig: """ Abstraction class on top of config loading/updating logic. Gives us one place to control all config updating logic. @@ -5332,13 +5356,18 @@ class ProxyConfig: verbose_proxy_logger.warning( "Key '%s' is not a valid argument for Router.__init__(). Ignoring this key.", k ) + # `async_only_mode` is a proxy-runtime requirement (only async clients are + # initialised here), so it is forced on. Everything else the operator set + # under `router_settings.router_general_settings` -- e.g. + # `model_name_resolution: strict` -- is preserved; passing the key in + # config used to collide with this keyword and raise TypeError at startup. + router_params["router_general_settings"] = _proxy_router_general_settings( + router_params.get("router_general_settings") + ) router = litellm.Router( **router_params, assistants_config=assistants_config, search_tools=search_tools, - router_general_settings=RouterGeneralSettings( - async_only_mode=True # only init async clients - ), ignore_invalid_deployments=True, # don't raise an error if a deployment is invalid ) @@ -5792,9 +5821,10 @@ class ProxyConfig: verbose_proxy_logger.debug("_model_list: %s", _model_list) llm_router = litellm.Router( model_list=_model_list, - router_general_settings=RouterGeneralSettings( - async_only_mode=True # only init async clients - ), + # DB-sourced model list: no config router_settings in scope + # here, so this is the proxy default (async_only_mode on, + # everything else at its RouterGeneralSettings default). + router_general_settings=_proxy_router_general_settings(None), search_tools=search_tools, ignore_invalid_deployments=True, ) diff --git a/litellm/proxy/route_llm_request.py b/litellm/proxy/route_llm_request.py index 507006d4c91..b9784de5bee 100644 --- a/litellm/proxy/route_llm_request.py +++ b/litellm/proxy/route_llm_request.py @@ -250,6 +250,15 @@ async def _canonical_target_is_allowed( ride the rewrite onto a deployment it was never granted. (Auth ran on the requested string before routing, when the target group was not yet known.) + Uses ``can_key_call_resolved_model`` -- the same helper every other + post-resolution auth site uses (model_group_alias rewrites, realtime + endpoints, auto-router) -- so the key, team, team-member, and project + allowlists are all re-checked against the target. Checking only the key + (``can_key_call_model``) would leave a team whose allowlist holds a stale + unserved name able to ride the rewrite onto a deployment it was never + granted, since an unrestricted *key* on that team passes the key-level + check on its own. + A denial returns False rather than raising, so the request falls through to the same 400 an unresolvable model gets today and the response reveals nothing about the target's existence. @@ -257,13 +266,13 @@ async def _canonical_target_is_allowed( if user_api_key_dict is None: return True from litellm.proxy.auth.auth_checks import ( - can_key_call_model, # pyright: ignore[reportUnknownVariableType] - auth_checks is partially typed + can_key_call_resolved_model, # pyright: ignore[reportUnknownVariableType] - auth_checks is partially typed ) try: - await can_key_call_model( + await can_key_call_resolved_model( model=canonical_target, - llm_model_list=llm_router.get_model_list(), + llm_model_list=llm_router.model_list, valid_token=user_api_key_dict, llm_router=llm_router, ) diff --git a/litellm/router_utils/canonical_model_resolution.py b/litellm/router_utils/canonical_model_resolution.py index 71a8a4add94..aa0a8341a6b 100644 --- a/litellm/router_utils/canonical_model_resolution.py +++ b/litellm/router_utils/canonical_model_resolution.py @@ -96,7 +96,7 @@ def _infer_provider(model: str) -> str | None: return custom_llm_provider or None -def canonicalize(model: str) -> tuple[str, str] | None: +def canonicalize(model: str, custom_llm_provider: str | None = None) -> tuple[str, str] | None: """Reduce ``model`` to a ``(provider, canonical_name)`` identity. The canonical name is the model string with any LiteLLM provider-route @@ -104,11 +104,19 @@ def canonicalize(model: str) -> tuple[str, str] | None: LiteLLM's own routing syntax rather than part of the model's identity. The provider is carried alongside so equality checks are always provider-scoped. + ``custom_llm_provider`` -- the deployment's explicit provider override -- + wins over whatever the model string implies. A deployment can carry a + first-party-looking id (``claude-haiku-4-5``) while actually being served + through Bedrock, Vertex, or OpenRouter; inferring the provider from the + string alone would index it as Anthropic and let an Anthropic-form request + be rewritten onto that other provider's credentials, quota, and bill -- + exactly the cross-provider hop rule 2 in the module docstring forbids. + Returns None when the provider cannot be inferred. """ if not model: return None - provider: Final = _infer_provider(model) + provider: Final = custom_llm_provider or _infer_provider(model) if provider is None: return None # get_llm_provider returns the model with its routing prefix stripped, which @@ -183,7 +191,14 @@ def build_canonical_index( if not isinstance(model_group, str) or not isinstance(underlying, str): continue - identity = canonicalize(underlying) + # An explicit provider override decides the provider; see canonicalize(). + provider_override = ( + litellm_params.get("custom_llm_provider") if isinstance(litellm_params, Mapping) else None + ) # rebind-ok: per-deployment loop variable + identity = canonicalize( + underlying, + custom_llm_provider=provider_override if isinstance(provider_override, str) else None, + ) if model_group in group_identity and group_identity[model_group] != identity: # Deployments in this group disagree about what they serve; the # group cannot stand for a single canonical identity. diff --git a/tests/test_litellm/router_utils/test_router_canonical_model_resolution.py b/tests/test_litellm/router_utils/test_router_canonical_model_resolution.py index 93109f56d04..8953e949b67 100644 --- a/tests/test_litellm/router_utils/test_router_canonical_model_resolution.py +++ b/tests/test_litellm/router_utils/test_router_canonical_model_resolution.py @@ -157,6 +157,47 @@ class TestBuildIndexAndLookup: assert lookup(index, DATED) is None assert lookup(index, UNDATED) is None + def test_custom_llm_provider_override_decides_provider(self): + """Regression (I2): a deployment's explicit custom_llm_provider wins over + whatever the model string implies. A first-party-looking id served via + OpenRouter/Bedrock/Vertex must not be indexed as Anthropic -- otherwise + an Anthropic-form request rides the rewrite onto that other provider's + credentials, quota, and bill, which is exactly the cross-provider hop + rule 2 forbids.""" + index = build_canonical_index( + [ + { + "model_name": "haiku-via-openrouter", + "litellm_params": { + "model": "claude-haiku-4-5", + "custom_llm_provider": "openrouter", + }, + }, + ] + ) + # Indexed under the real provider, not the one the string implies. + assert ("openrouter", UNDATED) in index + assert ("anthropic", UNDATED) not in index + # An Anthropic-form request must not reach the OpenRouter deployment. + assert lookup(index, DATED) is None + assert lookup(index, UNDATED) is None + + def test_custom_llm_provider_override_still_resolves_within_provider(self): + """The override narrows the provider, it does not disable resolution: + a request that infers to the same overridden provider still resolves.""" + index = build_canonical_index( + [ + { + "model_name": "haiku-via-bedrock", + "litellm_params": { + "model": "claude-haiku-4-5", + "custom_llm_provider": "bedrock", + }, + }, + ] + ) + assert index[("bedrock", UNDATED)] == "haiku-via-bedrock" + def test_team_owned_deployment_never_indexed(self): """Regression: a team-owned deployment (model_info.team_id set) must never enter the global canonical index. Without this, a no-team key @@ -410,3 +451,115 @@ class TestAuthAndOnTarget: ) is True ) + + +class TestCanonicalTargetReAuth: + """The rewrite's re-auth must use the *resolved-model* helper. + + Regression: the hook originally called ``can_key_call_model``, which checks + only the key's own allowlist. Team, team-member, and project allowlists were + therefore never re-checked against the resolved target, so an unrestricted + key on a team whose allowlist held only a stale unserved name could ride the + rewrite onto a deployment that team was never granted. + ``can_key_call_resolved_model`` is the helper every other post-resolution + auth site uses (model_group_alias rewrites, realtime endpoints, auto-router). + """ + + @pytest.mark.asyncio + async def test_uses_resolved_model_helper_so_team_scope_is_rechecked(self, anthropic_router: Router): + from unittest.mock import AsyncMock, patch + + from litellm.proxy._types import UserAPIKeyAuth + from litellm.proxy.route_llm_request import _canonical_target_is_allowed + + with patch( + "litellm.proxy.auth.auth_checks.can_key_call_resolved_model", + new=AsyncMock(return_value=None), + ) as resolved_check: + allowed = await _canonical_target_is_allowed( + canonical_target=ANTHROPIC_GROUP, + llm_router=anthropic_router, + user_api_key_dict=UserAPIKeyAuth(token="t", team_id="team-A"), + ) + + assert allowed is True + resolved_check.assert_awaited_once() + assert resolved_check.await_args.kwargs["model"] == ANTHROPIC_GROUP + + @pytest.mark.asyncio + async def test_denial_declines_rewrite_rather_than_raising(self, anthropic_router: Router): + """A denial must return False (request falls through to the usual 400), + never propagate an exception that would leak the target's existence.""" + from unittest.mock import AsyncMock, patch + + from litellm.proxy._types import UserAPIKeyAuth + + from litellm.proxy.route_llm_request import _canonical_target_is_allowed + + with patch( + "litellm.proxy.auth.auth_checks.can_key_call_resolved_model", + new=AsyncMock(side_effect=ProxyException(message="denied", type="auth_error", param=None, code=401)), + ): + allowed = await _canonical_target_is_allowed( + canonical_target=ANTHROPIC_GROUP, + llm_router=anthropic_router, + user_api_key_dict=UserAPIKeyAuth(token="t", team_id="team-A"), + ) + + assert allowed is False + + @pytest.mark.asyncio + async def test_no_auth_context_is_allowed(self, anthropic_router: Router): + """No key context (non-proxy Router use) leaves the rewrite unguarded by + key auth, matching the surrounding call path.""" + from litellm.proxy.route_llm_request import _canonical_target_is_allowed + + assert ( + await _canonical_target_is_allowed( + canonical_target=ANTHROPIC_GROUP, + llm_router=anthropic_router, + user_api_key_dict=None, + ) + is True + ) + + +class TestProxyRouterGeneralSettings: + """Operators must be able to set model_name_resolution on the proxy. + + Regression: the proxy passed a hardcoded RouterGeneralSettings(async_only_mode=True) + as an explicit keyword alongside **router_params, so setting + router_general_settings in config raised "got multiple values for keyword + argument" at startup -- leaving no way to select 'strict'. + """ + + def test_config_settings_preserved_and_async_only_forced(self): + from litellm.proxy.proxy_server import _proxy_router_general_settings + + settings = _proxy_router_general_settings({"model_name_resolution": "strict"}) + assert settings.model_name_resolution == "strict" + assert settings.async_only_mode is True + + def test_async_only_mode_cannot_be_disabled_by_config(self): + from litellm.proxy.proxy_server import _proxy_router_general_settings + + settings = _proxy_router_general_settings({"async_only_mode": False, "model_name_resolution": "strict"}) + assert settings.async_only_mode is True + assert settings.model_name_resolution == "strict" + + def test_none_yields_proxy_default(self): + from litellm.proxy.proxy_server import _proxy_router_general_settings + + settings = _proxy_router_general_settings(None) + assert settings.async_only_mode is True + assert settings.model_name_resolution == "canonical" + + def test_model_instance_is_not_mutated(self): + from litellm.proxy.proxy_server import _proxy_router_general_settings + from litellm.types.router import RouterGeneralSettings + + original = RouterGeneralSettings(async_only_mode=False, model_name_resolution="strict") + settings = _proxy_router_general_settings(original) + assert settings.async_only_mode is True + # The caller's object must not be rewritten at a distance. + assert original.async_only_mode is False