mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(auth): per-param allow must continue, not return early
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) <noreply@anthropic.com>
This commit is contained in:
parent
37a22acf6f
commit
01323e8903
2 changed files with 38 additions and 1 deletions
|
|
@ -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 "
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue