From 29c81d50d8253aa6d93620ef1c41aa068317da6d Mon Sep 17 00:00:00 2001 From: Genmin Date: Fri, 1 May 2026 07:36:59 -0700 Subject: [PATCH] chore: clarify clientside credential cleanup --- litellm/router.py | 6 +++++- .../router_utils/clientside_credential_handler.py | 9 ++++----- tests/test_litellm/proxy/auth/test_auth_utils.py | 12 ++++-------- 3 files changed, 13 insertions(+), 14 deletions(-) diff --git a/litellm/router.py b/litellm/router.py index 54c920ad3d1..30da9db9c82 100644 --- a/litellm/router.py +++ b/litellm/router.py @@ -2379,7 +2379,11 @@ class Router: """ dep_params = deployment.get("litellm_params", {}) or {} for key in clientside_credential_keys: - if kwargs.get(key) is None and dep_params.get(key) is not None: + if ( + key in kwargs + and kwargs.get(key) is None + and dep_params.get(key) is not None + ): kwargs.pop(key, None) def _update_kwargs_with_deployment( diff --git a/litellm/router_utils/clientside_credential_handler.py b/litellm/router_utils/clientside_credential_handler.py index 4a36893c8b9..e8645db8aaa 100644 --- a/litellm/router_utils/clientside_credential_handler.py +++ b/litellm/router_utils/clientside_credential_handler.py @@ -92,11 +92,10 @@ def get_dynamic_litellm_params(litellm_params: dict, request_kwargs: dict) -> di # don't forward the admin's organization / extra_body / region / token / # vertex / aws fields — those were meant for the original upstream. # Always drop the admin's value first, then write the caller's value back - # if they resupplied the field. The naive - # ``if field not in request_kwargs: pop`` shape lets a caller *echo* a - # field name (with any value, including an empty string) to keep the - # admin's value in ``litellm_params`` and have it forwarded to the - # redirected upstream. + # when they resupplied a concrete, non-None field value. The old + # ``if field not in request_kwargs: pop`` shape let a caller echo a + # field name to keep the admin's value in ``litellm_params`` and have + # it forwarded to the redirected upstream. if ( request_kwargs.get("api_base") is not None or request_kwargs.get("base_url") is not None diff --git a/tests/test_litellm/proxy/auth/test_auth_utils.py b/tests/test_litellm/proxy/auth/test_auth_utils.py index b82cb355192..d3983535286 100644 --- a/tests/test_litellm/proxy/auth/test_auth_utils.py +++ b/tests/test_litellm/proxy/auth/test_auth_utils.py @@ -811,11 +811,7 @@ class TestGetDynamicLitellmParamsClearsAdminConfigOnBaseOverride: def test_caller_resupplied_value_overrides_admin_value_on_base_override(self): # When the caller redirects ``api_base`` and *also* supplies their # own value for one of the admin fields (e.g. ``organization``), - # the caller's value must win — never the admin's. The naive - # ``if field not in request_kwargs: pop`` shape lets a caller echo - # the field name with any value (or empty string) to keep the - # admin's value forwarded, which is the exfiltration vector this - # test guards against. + # the caller's value must win — never the admin's. from litellm.router_utils.clientside_credential_handler import ( get_dynamic_litellm_params, ) @@ -836,9 +832,9 @@ class TestGetDynamicLitellmParamsClearsAdminConfigOnBaseOverride: assert out["extra_body"] == {"attacker": "value"} def test_field_echo_does_not_preserve_admin_value(self): - # Regression: a caller that echoes an admin-config field name with - # an *empty* value (or any value) must not be able to keep the - # admin's value in ``litellm_params``. + # Regression: a caller that resupplies an admin-config field name + # with an empty value must not be able to keep the admin's value in + # ``litellm_params``. from litellm.router_utils.clientside_credential_handler import ( get_dynamic_litellm_params, )