From 01323e890357f34ea39eab025e7fff55391ed26e Mon Sep 17 00:00:00 2001 From: user <70670632+stuxf@users.noreply.github.com> Date: Sun, 3 May 2026 09:18:17 +0000 Subject: [PATCH] fix(auth): per-param allow must continue, not return early MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pre-existing logic bug in ``_check_banned_params``: when the deployment-level ``configurable_clientside_auth_params`` permitted one banned field, the loop ``return``-ed on the first match instead of ``continue``-ing, so any other banned param later in the same body or metadata dict was never checked. This PR's metadata walk multiplies the surface where that bypass matters — a body pairing an allowed ``api_base`` with an observability credential like ``langfuse_host`` would silently pass. Proxy-wide ``allow_client_side_credentials`` keeps ``return`` (it's a global opt-in for every banned param). The per-param branch becomes ``continue`` so only the one explicitly-permitted field is skipped. Adds a regression test that exercises the api_base + langfuse_host pair. Co-Authored-By: Claude Opus 4.7 (1M context) --- litellm/proxy/auth/auth_utils.py | 9 +++++- .../proxy/auth/test_auth_utils.py | 30 +++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/auth/auth_utils.py b/litellm/proxy/auth/auth_utils.py index 84fb2f8a95e..9a6fc95f145 100644 --- a/litellm/proxy/auth/auth_utils.py +++ b/litellm/proxy/auth/auth_utils.py @@ -279,6 +279,8 @@ def _check_banned_params( if param not in body: continue if general_settings.get("allow_client_side_credentials") is True: + # Proxy-wide opt-in: every banned param is permitted, exit + # entirely so the rest of the loop doesn't waste work. return if ( _allow_model_level_clientside_configurable_parameters( @@ -289,7 +291,12 @@ def _check_banned_params( ) is True ): - return + # Per-param opt-in: only THIS param is permitted by the + # deployment's ``configurable_clientside_auth_params``. Skip + # to the next banned param so a body that pairs an allowed + # ``api_base`` with an unallowed ``langfuse_host`` is still + # rejected for the second field. + continue raise ValueError( f"Rejected Request: {param} is not allowed in request body. " "Clientside passthrough requires explicit admin opt-in via " diff --git a/tests/test_litellm/proxy/auth/test_auth_utils.py b/tests/test_litellm/proxy/auth/test_auth_utils.py index 58a55f8aa6f..7c04a4f61fb 100644 --- a/tests/test_litellm/proxy/auth/test_auth_utils.py +++ b/tests/test_litellm/proxy/auth/test_auth_utils.py @@ -1455,6 +1455,36 @@ class TestObservabilityCallbackBans: ) +def test_model_level_allow_does_not_skip_subsequent_banned_params(monkeypatch): + """Greptile P1: ``_check_banned_params`` previously ``return``-ed when a + deployment's ``configurable_clientside_auth_params`` permitted one + banned field, exiting before any later banned field in the same body + was checked. The metadata walk this PR adds multiplies the surface + where that bypass matters: a body pairing a model-level-allowed + ``api_base`` with an observability credential like ``langfuse_host`` + must still reject on the second field, not silently pass.""" + from litellm.proxy.auth import auth_utils + + monkeypatch.setattr( + auth_utils, + "_allow_model_level_clientside_configurable_parameters", + lambda model, param, request_body_value, llm_router: param == "api_base", + ) + + with pytest.raises(ValueError) as exc: + is_request_body_safe( + request_body={ + "model": "gpt-4", + "api_base": "https://allowed-by-deployment.example", + "langfuse_host": "https://attacker.example", + }, + general_settings={}, + llm_router=None, + model="gpt-4", + ) + assert "langfuse_host" in str(exc.value) + + def test_observability_ban_covers_canonical_supported_callback_params(): """Guard test: every entry in the canonical ``_supported_callback_params`` allow-list must end up either banned by