mirror of
https://github.com/BerriAI/litellm.git
synced 2026-11-13 08:52:02 +00:00
fix(router): apply the clientside-override credential clearing to the in-flight request
_handle_clientside_credential cleared admin credential and endpoint fields via get_dynamic_litellm_params, but only the upserted shadow deployment saw the cleared dict; the request being served still merged the original deployment litellm_params under the caller's kwargs, so a client api_base override rode along with the admin's OAuth fields and a bearer minted from the admin client_secret was sent to the redirected upstream. _update_kwargs_with_deployment now returns the cleared litellm_params and the chat and generic call frames use them for the outbound call, so an override request gets only the caller's own credentials. Covered by router-level respx regression tests
This commit is contained in:
parent
0124daa8b7
commit
d0ddd9eb09
4 changed files with 113 additions and 16 deletions
|
|
@ -1961,7 +1961,7 @@ class Router:
|
|||
thread.start()
|
||||
|
||||
kwargs.setdefault("messages", messages)
|
||||
self._update_kwargs_with_deployment(deployment=deployment, kwargs=kwargs)
|
||||
clientside_litellm_params: Final = self._update_kwargs_with_deployment(deployment=deployment, kwargs=kwargs)
|
||||
kwargs.pop("silent_model", None) # Ensure it's not in kwargs either
|
||||
model_name = litellm_params["model"]
|
||||
potential_model_client: Final = self._get_client(deployment=deployment, kwargs=kwargs)
|
||||
|
|
@ -1982,12 +1982,13 @@ class Router:
|
|||
self.routing_strategy_pre_call_checks(deployment=deployment)
|
||||
|
||||
input_kwargs: Final = {
|
||||
**litellm_params,
|
||||
**(clientside_litellm_params if clientside_litellm_params is not None else litellm_params),
|
||||
"messages": messages,
|
||||
"caching": self.cache_responses,
|
||||
"client": model_client,
|
||||
**kwargs,
|
||||
}
|
||||
input_kwargs.pop("silent_model", None)
|
||||
response: Final = litellm.completion(**input_kwargs)
|
||||
verbose_router_logger.info("litellm.completion(model=%s)\x1b[32m 200 OK\x1b[0m", model_name)
|
||||
|
||||
|
|
@ -2991,7 +2992,7 @@ class Router:
|
|||
)
|
||||
|
||||
kwargs.setdefault("messages", messages)
|
||||
self._update_kwargs_with_deployment(deployment=deployment, kwargs=kwargs)
|
||||
clientside_litellm_params: Final = self._update_kwargs_with_deployment(deployment=deployment, kwargs=kwargs)
|
||||
kwargs.pop("silent_model", None) # Ensure it's not in kwargs either
|
||||
|
||||
model_name = litellm_params["model"]
|
||||
|
|
@ -3003,7 +3004,7 @@ class Router:
|
|||
self.total_calls[model_name] += 1
|
||||
|
||||
input_kwargs: Final = {
|
||||
**litellm_params,
|
||||
**(clientside_litellm_params if clientside_litellm_params is not None else litellm_params),
|
||||
"messages": messages,
|
||||
"caching": self.cache_responses,
|
||||
"client": model_client,
|
||||
|
|
@ -3193,9 +3194,13 @@ class Router:
|
|||
|
||||
def _handle_clientside_credential(
|
||||
self, deployment: dict, kwargs: dict, function_name: str | None = None
|
||||
) -> Deployment:
|
||||
) -> tuple[Deployment, dict]: # mutable-ok: the litellm_params dict is merged into per-request kwargs downstream
|
||||
"""
|
||||
Handle clientside credential
|
||||
|
||||
Returns the upserted deployment plus the effective litellm_params with
|
||||
admin credential and endpoint fields cleared on an api_base override,
|
||||
so the in-flight request uses the cleared params too.
|
||||
"""
|
||||
model_info: Final = deployment.get("model_info", {}).copy()
|
||||
litellm_params: Final = deployment["litellm_params"].copy()
|
||||
|
|
@ -3215,7 +3220,7 @@ class Router:
|
|||
model_info=model_info,
|
||||
)
|
||||
self.upsert_deployment(deployment=deployment_pydantic_obj) # add new deployment to router
|
||||
return deployment_pydantic_obj
|
||||
return deployment_pydantic_obj, dynamic_litellm_params
|
||||
|
||||
@staticmethod
|
||||
def _merge_tools_from_deployment(deployment: dict, kwargs: dict) -> None:
|
||||
|
|
@ -3242,12 +3247,16 @@ class Router:
|
|||
deployment: dict,
|
||||
kwargs: dict,
|
||||
function_name: str | None = None,
|
||||
) -> None:
|
||||
) -> dict | None: # mutable-ok: callers merge the returned litellm_params into per-request kwargs
|
||||
"""
|
||||
3 jobs:
|
||||
- Adds selected deployment, model_info and api_base to kwargs["metadata"] (used for logging)
|
||||
- Adds default litellm params to kwargs, if set.
|
||||
- Merges tools from deployment with request (proxy-configured tools + request tools).
|
||||
|
||||
Returns the cleared litellm_params when the caller supplied clientside
|
||||
credentials (admin credential fields must not ride along to a
|
||||
client-redirected upstream), else None so callers use the deployment's own.
|
||||
"""
|
||||
for key in self._forwarded_alias_marker_keys_the_deployment_sets(
|
||||
deployment=deployment, forwarded_keys=kwargs.pop(_ALIAS_MARKER_FORWARDED_PARAMS_KWARG, ())
|
||||
|
|
@ -3259,8 +3268,9 @@ class Router:
|
|||
deployment_litellm_model_name = deployment["litellm_params"]["model"]
|
||||
deployment_api_base = deployment["litellm_params"].get("api_base")
|
||||
deployment_model_name: Final = deployment["model_name"]
|
||||
clientside_litellm_params = None # rebind-ok: set only when the caller supplied clientside credentials
|
||||
if is_clientside_credential(request_kwargs=kwargs):
|
||||
deployment_pydantic_obj: Final = self._handle_clientside_credential(
|
||||
deployment_pydantic_obj, clientside_litellm_params = self._handle_clientside_credential(
|
||||
deployment=deployment, kwargs=kwargs, function_name=function_name
|
||||
)
|
||||
model_info = deployment_pydantic_obj.model_info.model_dump()
|
||||
|
|
@ -3325,6 +3335,7 @@ class Router:
|
|||
kwargs["timeout"] = self._get_timeout(kwargs=kwargs, data=deployment["litellm_params"])
|
||||
|
||||
self._update_kwargs_with_default_litellm_params(kwargs=kwargs, metadata_variable_name=metadata_variable_name)
|
||||
return clientside_litellm_params
|
||||
|
||||
def _get_async_openai_model_client(self, deployment: dict, kwargs: dict):
|
||||
"""
|
||||
|
|
@ -4676,9 +4687,15 @@ class Router:
|
|||
return await original_generic_function(model=model, **kwargs)
|
||||
raise e
|
||||
|
||||
self._update_kwargs_with_deployment(deployment=deployment, kwargs=kwargs, function_name=function_name)
|
||||
clientside_litellm_params: Final = self._update_kwargs_with_deployment(
|
||||
deployment=deployment, kwargs=kwargs, function_name=function_name
|
||||
)
|
||||
|
||||
data: Final = deployment["litellm_params"].copy()
|
||||
data: Final = (
|
||||
clientside_litellm_params
|
||||
if clientside_litellm_params is not None
|
||||
else deployment["litellm_params"].copy()
|
||||
)
|
||||
model_name: Final = data["model"]
|
||||
self.total_calls[model_name] += 1
|
||||
|
||||
|
|
|
|||
|
|
@ -411,7 +411,7 @@ def test_router_handle_clientside_credential():
|
|||
}
|
||||
router = Router(model_list=[deployment])
|
||||
|
||||
new_deployment = router._handle_clientside_credential(
|
||||
new_deployment, _ = router._handle_clientside_credential(
|
||||
deployment=deployment,
|
||||
kwargs={
|
||||
"api_key": "123",
|
||||
|
|
|
|||
|
|
@ -2062,7 +2062,7 @@ def test_handle_clientside_credential_metadata_loading(
|
|||
}
|
||||
|
||||
# Call the function
|
||||
result_deployment = router._handle_clientside_credential(
|
||||
result_deployment, _ = router._handle_clientside_credential(
|
||||
deployment=deployment, kwargs=kwargs, function_name=function_name
|
||||
)
|
||||
|
||||
|
|
@ -2139,7 +2139,7 @@ def test_handle_clientside_credential_metadata_variable_name(
|
|||
}
|
||||
|
||||
# Call the function
|
||||
result_deployment = router._handle_clientside_credential(
|
||||
result_deployment, _ = router._handle_clientside_credential(
|
||||
deployment=deployment, kwargs=kwargs, function_name=function_name
|
||||
)
|
||||
|
||||
|
|
@ -2176,7 +2176,7 @@ def test_handle_clientside_credential_no_metadata(model_list):
|
|||
# This should fail because there's no model_group in metadata
|
||||
# The function expects to find model_group in the metadata
|
||||
try:
|
||||
result_deployment = router._handle_clientside_credential(
|
||||
result_deployment, _ = router._handle_clientside_credential(
|
||||
deployment=deployment, kwargs=kwargs, function_name="acompletion"
|
||||
)
|
||||
# If we get here, the function should have used deployment.model_name as fallback
|
||||
|
|
@ -2194,7 +2194,7 @@ def test_handle_clientside_credential_no_metadata(model_list):
|
|||
}
|
||||
|
||||
try:
|
||||
result_deployment = router._handle_clientside_credential(
|
||||
result_deployment, _ = router._handle_clientside_credential(
|
||||
deployment=deployment,
|
||||
kwargs=kwargs_with_empty_metadata,
|
||||
function_name="acompletion",
|
||||
|
|
@ -2227,7 +2227,7 @@ def test_handle_clientside_credential_with_responses_function(model_list):
|
|||
}
|
||||
|
||||
# Call the function with _ageneric_api_call_with_fallbacks function name (which handles responses)
|
||||
result_deployment = router._handle_clientside_credential(
|
||||
result_deployment, _ = router._handle_clientside_credential(
|
||||
deployment=deployment,
|
||||
kwargs=kwargs,
|
||||
function_name="_ageneric_api_call_with_fallbacks",
|
||||
|
|
|
|||
|
|
@ -7,6 +7,12 @@ fields so the bearer token litellm mints with them is never sent to the caller's
|
|||
endpoint.
|
||||
"""
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
import respx
|
||||
|
||||
import litellm
|
||||
from litellm import Router
|
||||
from litellm.router_utils.clientside_credential_handler import (
|
||||
_ADMIN_CONFIG_FIELDS_TO_CLEAR_ON_BASE_OVERRIDE,
|
||||
get_dynamic_litellm_params,
|
||||
|
|
@ -51,3 +57,77 @@ def test_oauth_creds_preserved_without_base_override():
|
|||
params = get_dynamic_litellm_params(_deployment_params(), {"temperature": 0.5})
|
||||
for field in _OAUTH_FIELDS:
|
||||
assert field in params
|
||||
|
||||
|
||||
OPENAI_CHAT_RESPONSE = {
|
||||
"id": "chatcmpl-1",
|
||||
"object": "chat.completion",
|
||||
"created": 1677652288,
|
||||
"model": "gpt-4o",
|
||||
"choices": [
|
||||
{"index": 0, "message": {"role": "assistant", "content": "ok"}, "finish_reason": "stop"}
|
||||
],
|
||||
"usage": {"prompt_tokens": 1, "completion_tokens": 1, "total_tokens": 2},
|
||||
}
|
||||
|
||||
|
||||
def _router() -> Router:
|
||||
return Router(
|
||||
model_list=[
|
||||
{
|
||||
"model_name": "oauth-model",
|
||||
"litellm_params": {
|
||||
"model": "openai/gpt-4o",
|
||||
"api_key": "sk-admin",
|
||||
**_deployment_params(),
|
||||
},
|
||||
}
|
||||
]
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_base_override_mints_nothing_for_client_upstream(monkeypatch, respx_mock: respx.MockRouter):
|
||||
# The in-flight request must use the cleared litellm_params: no token grant
|
||||
# fires and the client-redirected upstream sees only the client's own key,
|
||||
# never a bearer minted with the admin's client_secret.
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||
litellm.in_memory_llm_clients_cache.flush_cache()
|
||||
token_route = respx_mock.post("https://idp.internal/oauth/token").mock(
|
||||
return_value=httpx.Response(200, json={"access_token": "tok-admin", "expires_in": 3600})
|
||||
)
|
||||
client_route = respx_mock.post("https://client.example/v1/chat/completions").mock(
|
||||
return_value=httpx.Response(200, json=OPENAI_CHAT_RESPONSE)
|
||||
)
|
||||
|
||||
await _router().acompletion(
|
||||
model="oauth-model",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
api_base="https://client.example/v1",
|
||||
api_key="sk-client",
|
||||
)
|
||||
|
||||
assert not token_route.called
|
||||
assert client_route.calls.last.request.headers["authorization"] == "Bearer sk-client"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_router_without_override_mints_bearer_for_admin_upstream(monkeypatch, respx_mock: respx.MockRouter):
|
||||
from litellm.llms.openai_like.oauth_authenticator import _token_cache
|
||||
|
||||
monkeypatch.setattr(litellm, "disable_aiohttp_transport", True)
|
||||
litellm.in_memory_llm_clients_cache.flush_cache()
|
||||
_token_cache.flush_cache()
|
||||
respx_mock.post("https://idp.internal/oauth/token").mock(
|
||||
return_value=httpx.Response(200, json={"access_token": "tok-admin", "expires_in": 3600})
|
||||
)
|
||||
gateway_route = respx_mock.post("https://gateway.internal/v1/chat/completions").mock(
|
||||
return_value=httpx.Response(200, json=OPENAI_CHAT_RESPONSE)
|
||||
)
|
||||
|
||||
await _router().acompletion(
|
||||
model="oauth-model",
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
)
|
||||
|
||||
assert gateway_route.calls.last.request.headers["authorization"] == "Bearer tok-admin"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue