mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-12 23:01:41 +00:00
fix(proxy)!: apply request-parameter checks consistently across body, path and form inputs (#36011)
Backport of the #36011 batch; picked from its merge commit on internal staging.
(cherry picked from commit c898d341c0)
This commit is contained in:
parent
ca9719459c
commit
dae46795c8
10 changed files with 332 additions and 26 deletions
|
|
@ -18,6 +18,7 @@ from litellm.litellm_core_utils.url_utils import (
|
|||
validate_url,
|
||||
)
|
||||
from litellm.proxy._types import *
|
||||
from litellm.proxy.common_utils.http_parsing_utils import extract_nested_form_metadata
|
||||
from litellm.types.router import CONFIGURABLE_CLIENTSIDE_AUTH_PARAMS
|
||||
from litellm.types.utils import CustomPricingLiteLLMParams
|
||||
|
||||
|
|
@ -440,6 +441,18 @@ def is_request_body_safe(
|
|||
metadata = _coerce_metadata_to_dict(request_body.get(metadata_key))
|
||||
if metadata is not None:
|
||||
_check_banned_params(metadata, general_settings, llm_router, model)
|
||||
if any(
|
||||
isinstance(key, str) and key.startswith(f"{metadata_key}[")
|
||||
for key in request_body
|
||||
):
|
||||
_check_banned_params(
|
||||
extract_nested_form_metadata(
|
||||
form_data=request_body, prefix=f"{metadata_key}["
|
||||
),
|
||||
general_settings,
|
||||
llm_router,
|
||||
model,
|
||||
)
|
||||
for target in iter_request_fallback_targets(request_body):
|
||||
if isinstance(target, dict):
|
||||
_check_banned_params(target, general_settings, llm_router, model)
|
||||
|
|
|
|||
|
|
@ -62,7 +62,10 @@ if TYPE_CHECKING:
|
|||
ProxyConfig = _ProxyConfig
|
||||
else:
|
||||
ProxyConfig = Any
|
||||
from litellm.proxy.litellm_pre_call_utils import add_litellm_data_to_request
|
||||
from litellm.proxy.litellm_pre_call_utils import (
|
||||
add_litellm_data_to_request,
|
||||
reject_url_valued_destination,
|
||||
)
|
||||
from litellm.types.utils import ModelResponse, ModelResponseStream, Usage
|
||||
|
||||
# Datadog streaming spans are a no-op when ddtrace is not enabled, but the
|
||||
|
|
@ -895,6 +898,9 @@ class ProxyBaseLLMRequestProcessing:
|
|||
"queue_time_seconds"
|
||||
] = queue_time_seconds
|
||||
|
||||
if isinstance(model, str):
|
||||
reject_url_valued_destination("model", model)
|
||||
|
||||
self.data["model"] = (
|
||||
general_settings.get("completion_model", None) # server default
|
||||
or user_model # model name passed via cli args
|
||||
|
|
|
|||
|
|
@ -6,7 +6,7 @@ import secrets
|
|||
import time
|
||||
import traceback
|
||||
from datetime import datetime, timedelta
|
||||
from typing import Any, Dict, Iterable, Literal, Optional, Union, cast
|
||||
from typing import Any, Dict, Final, Iterable, Literal, Mapping, Optional, Union, cast
|
||||
|
||||
import fastapi
|
||||
from fastapi import APIRouter, Depends, HTTPException, Request, Response, status
|
||||
|
|
@ -27,6 +27,9 @@ from litellm.proxy._types import (
|
|||
UserAPIKeyAuth,
|
||||
WebhookEvent,
|
||||
)
|
||||
from litellm.proxy.auth.auth_utils import (
|
||||
_BANNED_REQUEST_BODY_PARAMS, # pyright: ignore[reportPrivateUsage] # one canonical list, shared with the request-body check
|
||||
)
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.db.exception_handler import PrismaDBExceptionHandler
|
||||
from litellm.proxy.health_check import (
|
||||
|
|
@ -41,6 +44,10 @@ from litellm.proxy.middleware.in_flight_requests_middleware import (
|
|||
get_in_flight_requests,
|
||||
)
|
||||
from litellm.proxy.shutdown.graceful_shutdown_manager import GracefulShutdownManager
|
||||
from litellm.router_utils.clientside_credential_handler import (
|
||||
_ADMIN_CONFIG_FIELDS_TO_CLEAR_ON_BASE_OVERRIDE, # pyright: ignore[reportPrivateUsage] # one canonical list, shared with the router path
|
||||
clientside_credential_keys,
|
||||
)
|
||||
|
||||
#### Health ENDPOINTS ####
|
||||
|
||||
|
|
@ -80,6 +87,49 @@ def _reject_os_environ_references(params: dict) -> None:
|
|||
stack.append(value)
|
||||
|
||||
|
||||
_CONFIG_CONNECTION_FIELDS: Final[frozenset[str]] = frozenset(
|
||||
(
|
||||
*_ADMIN_CONFIG_FIELDS_TO_CLEAR_ON_BASE_OVERRIDE,
|
||||
*clientside_credential_keys,
|
||||
"litellm_credential_name",
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
def _config_base_for_health_check(
|
||||
config_params: Mapping[str, object],
|
||||
request_params: Mapping[str, object],
|
||||
allow_client_side_credentials: bool = False,
|
||||
) -> dict[str, object]:
|
||||
"""Return the configured parameters to merge under a connection-test request.
|
||||
|
||||
A request that sets its own connection fields describes a connection of its
|
||||
own, so the configuration's credentials are not carried into it: they belong
|
||||
to the endpoint the configuration names. Anything the request does not set
|
||||
still comes from the configuration, which is what lets a request name a
|
||||
configured model and test it as configured.
|
||||
|
||||
``litellm_credential_name`` is dropped alongside the literal credential
|
||||
fields: it names a stored credential that ``load_credentials_from_list``
|
||||
resolves into the same secrets further down the call, so leaving it in place
|
||||
would reintroduce them by reference.
|
||||
|
||||
``general_settings.allow_client_side_credentials`` is the existing proxy-wide
|
||||
opt-in for callers supplying their own connection parameters. Where an admin
|
||||
has enabled it, a request may pair its own endpoint with the configured
|
||||
credentials, as it could before.
|
||||
"""
|
||||
if allow_client_side_credentials:
|
||||
return dict(config_params)
|
||||
if not any(param in request_params for param in _BANNED_REQUEST_BODY_PARAMS):
|
||||
return dict(config_params)
|
||||
return {
|
||||
key: value
|
||||
for key, value in config_params.items()
|
||||
if key not in _CONFIG_CONNECTION_FIELDS
|
||||
}
|
||||
|
||||
|
||||
def get_callback_identifier(callback):
|
||||
"""
|
||||
Get the callback identifier string, handling both strings and objects.
|
||||
|
|
@ -1854,7 +1904,12 @@ async def test_model_connection(
|
|||
from litellm.proxy.management_endpoints.model_management_endpoints import (
|
||||
ModelManagementAuthChecks,
|
||||
)
|
||||
from litellm.proxy.proxy_server import llm_router, premium_user, prisma_client
|
||||
from litellm.proxy.proxy_server import (
|
||||
general_settings,
|
||||
llm_router,
|
||||
premium_user,
|
||||
prisma_client,
|
||||
)
|
||||
from litellm.types.router import Deployment, LiteLLM_Params
|
||||
|
||||
try:
|
||||
|
|
@ -1930,8 +1985,17 @@ async def test_model_connection(
|
|||
)
|
||||
|
||||
# Merge: config params (from proxy config) as base, request params override
|
||||
# This allows users to override specific params while using config for credentials
|
||||
litellm_params = {**config_litellm_params, **request_litellm_params}
|
||||
litellm_params = {
|
||||
**_config_base_for_health_check(
|
||||
config_litellm_params,
|
||||
request_litellm_params,
|
||||
allow_client_side_credentials=general_settings.get(
|
||||
"allow_client_side_credentials"
|
||||
)
|
||||
is True,
|
||||
),
|
||||
**request_litellm_params,
|
||||
}
|
||||
|
||||
## Auth check
|
||||
await ModelManagementAuthChecks.can_user_make_model_call(
|
||||
|
|
|
|||
|
|
@ -70,6 +70,7 @@ async def image_generation(
|
|||
user_api_key_dict: UserAPIKeyAuth = Depends(user_api_key_auth),
|
||||
model: Optional[str] = None,
|
||||
):
|
||||
from litellm.proxy.litellm_pre_call_utils import reject_url_valued_destination
|
||||
from litellm.proxy.proxy_server import (
|
||||
add_litellm_data_to_request,
|
||||
general_settings,
|
||||
|
|
@ -96,6 +97,9 @@ async def image_generation(
|
|||
proxy_config=proxy_config,
|
||||
)
|
||||
|
||||
if isinstance(model, str):
|
||||
reject_url_valued_destination("model", model)
|
||||
|
||||
data["model"] = (
|
||||
model
|
||||
or general_settings.get("image_generation_model", None) # server default
|
||||
|
|
|
|||
|
|
@ -4,7 +4,7 @@ import json
|
|||
import re
|
||||
import time
|
||||
from collections import OrderedDict
|
||||
from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union
|
||||
from typing import TYPE_CHECKING, Any, Dict, Final, List, Optional, Union
|
||||
|
||||
from fastapi import HTTPException, Request
|
||||
from pydantic import ValidationError as PydanticValidationError
|
||||
|
|
@ -214,29 +214,39 @@ def _reject_url_valued_destinations(data: Dict[str, Any]) -> None:
|
|||
are unaffected, while admins can opt specific hosts back in via
|
||||
``litellm.provider_url_destination_allowed_hosts``.
|
||||
"""
|
||||
allowed_hosts = getattr(litellm, "provider_url_destination_allowed_hosts", []) or []
|
||||
for field in _URL_DESTINATION_REQUEST_FIELDS:
|
||||
value = data.get(field)
|
||||
if not isinstance(value, str):
|
||||
if isinstance(value, str):
|
||||
reject_url_valued_destination(field, value)
|
||||
|
||||
|
||||
def reject_url_valued_destination(field: str, value: str) -> None:
|
||||
"""Reject a URL-valued destination identifier unless admin-allowlisted.
|
||||
|
||||
Operates on one field/value pair. ``_reject_url_valued_destinations`` applies
|
||||
it across ``_URL_DESTINATION_REQUEST_FIELDS`` for a request body.
|
||||
"""
|
||||
allowed_hosts: Final = (
|
||||
getattr(litellm, "provider_url_destination_allowed_hosts", []) or []
|
||||
)
|
||||
for candidate in provider_url_destination_candidates(value):
|
||||
if not candidate.lower().startswith(("http://", "https://")):
|
||||
continue
|
||||
for candidate in provider_url_destination_candidates(value):
|
||||
if not candidate.lower().startswith(("http://", "https://")):
|
||||
continue
|
||||
if is_url_destination_allowed_by_host(candidate, allowed_hosts):
|
||||
continue
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"error": "invalid_request",
|
||||
"param": field,
|
||||
"message": (
|
||||
f"URL-valued '{field}' is not allowed. Configure custom "
|
||||
"endpoints with api_base instead, or add the destination "
|
||||
"host to `provider_url_destination_allowed_hosts` in "
|
||||
"litellm_settings."
|
||||
),
|
||||
},
|
||||
)
|
||||
if is_url_destination_allowed_by_host(candidate, allowed_hosts):
|
||||
continue
|
||||
raise HTTPException(
|
||||
status_code=400,
|
||||
detail={
|
||||
"error": "invalid_request",
|
||||
"param": field,
|
||||
"message": (
|
||||
f"URL-valued '{field}' is not allowed. Configure custom "
|
||||
"endpoints with api_base instead, or add the destination "
|
||||
"host to `provider_url_destination_allowed_hosts` in "
|
||||
"litellm_settings."
|
||||
),
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def _strip_untrusted_request_header_controls(
|
||||
|
|
|
|||
|
|
@ -14,6 +14,7 @@ exclude = ["litellm/types/*", "litellm/__init__.py", "litellm/proxy/example_conf
|
|||
"litellm/llms/azure_ai/rerank/__init__.py" = ["F401"]
|
||||
"litellm/llms/bedrock/chat/__init__.py" = ["F401"]
|
||||
"litellm/proxy/utils.py" = ["F401", "PLR0915"]
|
||||
"litellm/proxy/common_request_processing.py" = ["PLR0915"]
|
||||
"litellm/proxy/guardrails/guardrail_hooks/litellm_content_filter/content_filter.py" = ["PLR0915"]
|
||||
"litellm/proxy/guardrails/guardrail_hooks/guardrail_benchmarks/test_eval.py" = ["PLR0915"]
|
||||
"litellm/responses/streaming_iterator.py" = ["PLR0915"]
|
||||
|
|
|
|||
|
|
@ -2405,3 +2405,80 @@ class TestIsRequestBodySafeBlocksModelList:
|
|||
)
|
||||
is True
|
||||
)
|
||||
|
||||
|
||||
class TestIsRequestBodySafeChecksBracketNotationMetadata:
|
||||
"""Bracket notation is how multipart callers express nested metadata; it is
|
||||
validated the same way the dict form is."""
|
||||
|
||||
@pytest.mark.parametrize("metadata_key", ["metadata", "litellm_metadata"])
|
||||
def test_bracket_notation_banned_param_is_rejected(self, metadata_key):
|
||||
with pytest.raises(ValueError, match="langfuse_host"):
|
||||
is_request_body_safe(
|
||||
request_body={
|
||||
"purpose": "assistants",
|
||||
f"{metadata_key}[langfuse_host]": "https://example.invalid",
|
||||
},
|
||||
general_settings={},
|
||||
llm_router=None,
|
||||
model="gpt-4",
|
||||
)
|
||||
|
||||
def test_bracket_notation_api_base_is_rejected(self):
|
||||
with pytest.raises(ValueError, match="api_base"):
|
||||
is_request_body_safe(
|
||||
request_body={"litellm_metadata[api_base]": "https://example.invalid"},
|
||||
general_settings={},
|
||||
llm_router=None,
|
||||
model="gpt-4",
|
||||
)
|
||||
|
||||
def test_bracket_notation_allowed_under_proxy_wide_opt_in(self):
|
||||
assert (
|
||||
is_request_body_safe(
|
||||
request_body={"litellm_metadata[langfuse_host]": "https://byok.example"},
|
||||
general_settings={"allow_client_side_credentials": True},
|
||||
llm_router=None,
|
||||
model="gpt-4",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_benign_bracket_notation_metadata_is_allowed(self):
|
||||
assert (
|
||||
is_request_body_safe(
|
||||
request_body={
|
||||
"purpose": "assistants",
|
||||
"litellm_metadata[spend_logs_metadata][owner]": "john",
|
||||
"litellm_metadata[tags]": "production",
|
||||
},
|
||||
general_settings={},
|
||||
llm_router=None,
|
||||
model="gpt-4",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
||||
def test_bracket_notation_matches_json_encoding_for_deeper_nesting(self):
|
||||
"""A value nested below the first level is treated the same either way:
|
||||
the check descends one level into metadata, for both encodings."""
|
||||
deep_bracket = {
|
||||
"litellm_metadata[spend_logs_metadata][langfuse_host]": "https://example.invalid"
|
||||
}
|
||||
deep_json = {
|
||||
"litellm_metadata": {"spend_logs_metadata": {"langfuse_host": "https://example.invalid"}}
|
||||
}
|
||||
kwargs = dict(general_settings={}, llm_router=None, model="gpt-4")
|
||||
assert is_request_body_safe(request_body=deep_bracket, **kwargs) is True
|
||||
assert is_request_body_safe(request_body=deep_json, **kwargs) is True
|
||||
|
||||
def test_body_without_bracket_keys_is_unaffected(self):
|
||||
assert (
|
||||
is_request_body_safe(
|
||||
request_body={"model": "gpt-4", "messages": [{"role": "user", "content": "hi"}]},
|
||||
general_settings={},
|
||||
llm_router=None,
|
||||
model="gpt-4",
|
||||
)
|
||||
is True
|
||||
)
|
||||
|
|
|
|||
|
|
@ -1823,3 +1823,96 @@ def test_clean_endpoint_data_strips_credentials_keeps_routing_fields():
|
|||
assert "aws_access_key_id" not in cleaned
|
||||
assert cleaned.get("api_base") == "https://example.test/v1"
|
||||
assert cleaned.get("api_version") == "2024-10-21"
|
||||
|
||||
|
||||
class TestConfigBaseForHealthCheck:
|
||||
"""A request that sets its own connection fields gets a base without the
|
||||
configuration's credentials; anything it leaves unset still comes from
|
||||
the configuration."""
|
||||
|
||||
CONFIG = {
|
||||
"model": "openai/gpt-4o",
|
||||
"api_key": "sk-configured",
|
||||
"api_base": "https://configured.example/v1",
|
||||
"vertex_credentials": "configured-creds",
|
||||
"rpm": 100,
|
||||
}
|
||||
|
||||
def _base(self, config, request, allow_client_side_credentials=False):
|
||||
from litellm.proxy.health_endpoints._health_endpoints import (
|
||||
_config_base_for_health_check,
|
||||
)
|
||||
|
||||
return _config_base_for_health_check(
|
||||
config, request, allow_client_side_credentials=allow_client_side_credentials
|
||||
)
|
||||
|
||||
def test_request_without_connection_fields_inherits_config(self):
|
||||
base = self._base(self.CONFIG, {"model": "openai/gpt-4o"})
|
||||
assert base["api_key"] == "sk-configured"
|
||||
assert base["api_base"] == "https://configured.example/v1"
|
||||
|
||||
def test_request_setting_api_base_does_not_inherit_config_credentials(self):
|
||||
base = self._base(self.CONFIG, {"api_base": "https://caller.example/v1"})
|
||||
assert "api_key" not in base
|
||||
assert "api_base" not in base
|
||||
assert "vertex_credentials" not in base
|
||||
assert base["rpm"] == 100
|
||||
|
||||
def test_add_model_flow_keeps_its_own_credentials(self):
|
||||
"""Adding a second deployment for an already-configured name sends a
|
||||
complete connection; it is tested as sent, not as configured."""
|
||||
request = {
|
||||
"model": "openai/gpt-4o",
|
||||
"api_base": "https://new-deployment.example/v1",
|
||||
"api_key": "sk-new-deployment",
|
||||
}
|
||||
merged = {**self._base(self.CONFIG, request), **request}
|
||||
assert merged["api_base"] == "https://new-deployment.example/v1"
|
||||
assert merged["api_key"] == "sk-new-deployment"
|
||||
assert "sk-configured" not in str(merged)
|
||||
|
||||
def test_destination_override_without_own_key_inherits_no_credential(self):
|
||||
"""A request that redirects the destination but supplies no credential
|
||||
of its own gets none from the configuration."""
|
||||
request = {"api_base": "https://elsewhere.example"}
|
||||
merged = {**self._base(self.CONFIG, request), **request}
|
||||
assert "api_key" not in merged
|
||||
assert "sk-configured" not in str(merged)
|
||||
|
||||
def test_non_api_base_destination_field_also_drops_credentials(self):
|
||||
base = self._base(
|
||||
{**self.CONFIG, "aws_secret_access_key": "configured-secret"},
|
||||
{"aws_bedrock_runtime_endpoint": "https://caller.example"},
|
||||
)
|
||||
assert "api_key" not in base
|
||||
assert "aws_secret_access_key" not in base
|
||||
|
||||
def test_opt_in_restores_configured_credentials_under_a_request_endpoint(self):
|
||||
"""With general_settings.allow_client_side_credentials enabled, a request
|
||||
may pair its own endpoint with the configured credentials, as before."""
|
||||
base = self._base(
|
||||
self.CONFIG,
|
||||
{"api_base": "https://caller.example/v1"},
|
||||
allow_client_side_credentials=True,
|
||||
)
|
||||
assert base["api_key"] == "sk-configured"
|
||||
|
||||
def test_stored_credential_reference_is_dropped_with_the_credentials(self):
|
||||
"""A stored-credential name resolves to the same secrets downstream, so a
|
||||
request that redirects the destination must not keep it either."""
|
||||
config = {**self.CONFIG, "litellm_credential_name": "OpenAI-prod"}
|
||||
base = self._base(config, {"api_base": "https://caller.example/v1"})
|
||||
assert "litellm_credential_name" not in base
|
||||
assert "api_key" not in base
|
||||
|
||||
def test_stored_credential_reference_kept_when_request_sets_no_connection(self):
|
||||
"""The Admin UI tests a configured model by naming it plus its stored
|
||||
credential and nothing else; that keeps working."""
|
||||
config = {**self.CONFIG, "litellm_credential_name": "OpenAI-prod"}
|
||||
base = self._base(
|
||||
config,
|
||||
{"model": "openai/gpt-4o", "litellm_credential_name": "OpenAI-prod", "custom_llm_provider": "openai"},
|
||||
)
|
||||
assert base["litellm_credential_name"] == "OpenAI-prod"
|
||||
assert base["api_key"] == "sk-configured"
|
||||
|
|
|
|||
|
|
@ -120,3 +120,28 @@ def test_azure_image_edit_route(client_no_auth):
|
|||
assert called_kwargs["prompt"] == "A cute baby sea otter"
|
||||
assert response.status_code == 200
|
||||
assert response.json()["data"]
|
||||
|
||||
|
||||
def test_azure_image_generation_route_rejects_url_valued_path_model(client_no_auth):
|
||||
"""A URL-valued deployment segment is refused before any provider call."""
|
||||
client, mock_aimage_generation, _ = client_no_auth
|
||||
response = client.post(
|
||||
"/openai/deployments/oobabooga/https://example.invalid/images/generations",
|
||||
json={"prompt": "A cute baby sea otter", "n": 1, "size": "1024x1024"},
|
||||
)
|
||||
|
||||
assert response.status_code == 400
|
||||
assert "URL-valued" in response.text
|
||||
mock_aimage_generation.assert_not_called()
|
||||
|
||||
|
||||
def test_azure_image_generation_route_allows_ordinary_path_model(client_no_auth):
|
||||
"""A deployment name that merely contains a provider prefix still routes."""
|
||||
client, mock_aimage_generation, _ = client_no_auth
|
||||
response = client.post(
|
||||
"/openai/deployments/dall-e-3/images/generations",
|
||||
json={"prompt": "A cute baby sea otter", "n": 1, "size": "1024x1024"},
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
mock_aimage_generation.assert_called_once()
|
||||
|
|
|
|||
|
|
@ -177,3 +177,16 @@ async def test_add_litellm_data_to_request_rejects_url_valued_model():
|
|||
)
|
||||
assert exc_info.value.status_code == 400
|
||||
assert exc_info.value.detail["param"] == "model"
|
||||
|
||||
|
||||
class TestNonStringDestinationValues:
|
||||
"""Only string identifiers are inspected. Anything else is left alone for the
|
||||
request's normal validation to handle."""
|
||||
|
||||
@pytest.mark.parametrize("value", [123, None, True, {"a": 1}, ["x"], 1.5])
|
||||
def test_non_string_model_is_ignored(self, value):
|
||||
_reject_url_valued_destinations({"model": value})
|
||||
|
||||
@pytest.mark.parametrize("value", [123, None, True, {"a": 1}, ["x"]])
|
||||
def test_non_string_file_id_is_ignored(self, value):
|
||||
_reject_url_valued_destinations({"file_id": value})
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue