mirror of
https://github.com/BerriAI/litellm.git
synced 2026-10-04 02:31:27 +00:00
fix(proxy): keep the submitted body out of 422 validation errors (#43231)
Co-authored-by: mateo-berri <277851410+mateo-berri@users.noreply.github.com>
This commit is contained in:
parent
474ab91c09
commit
e0fb89bc82
8 changed files with 168 additions and 21 deletions
13
litellm/proxy/common_utils/validation_error_body.py
Normal file
13
litellm/proxy/common_utils/validation_error_body.py
Normal file
|
|
@ -0,0 +1,13 @@
|
|||
from collections.abc import Sequence
|
||||
|
||||
from typing_extensions import ReadOnly, TypedDict
|
||||
|
||||
|
||||
class ValidationErrorDetail(TypedDict):
|
||||
type: ReadOnly[str]
|
||||
loc: ReadOnly[tuple[int | str, ...]]
|
||||
msg: ReadOnly[str]
|
||||
|
||||
|
||||
def public_validation_errors(errors: Sequence[ValidationErrorDetail]) -> tuple[ValidationErrorDetail, ...]:
|
||||
return tuple(ValidationErrorDetail(type=error["type"], loc=error["loc"], msg=error["msg"]) for error in errors)
|
||||
|
|
@ -8,8 +8,8 @@ from fastapi import Request
|
|||
from fastapi.dependencies.utils import get_flat_params
|
||||
from fastapi.params import ParamTypes
|
||||
from fastapi.responses import JSONResponse
|
||||
from typing_extensions import ReadOnly, TypedDict
|
||||
|
||||
from litellm.proxy.common_utils.validation_error_body import ValidationErrorDetail
|
||||
from litellm.types.proxy.management_endpoints.management_v1 import (
|
||||
ListLinks,
|
||||
PageLinks,
|
||||
|
|
@ -58,14 +58,6 @@ def escape_like(value: str) -> str:
|
|||
return value.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_")
|
||||
|
||||
|
||||
class ValidationErrorDetail(TypedDict):
|
||||
"""The keys of a pydantic/FastAPI validation error a problem document needs."""
|
||||
|
||||
type: ReadOnly[str]
|
||||
loc: ReadOnly[tuple[int | str, ...]]
|
||||
msg: ReadOnly[str]
|
||||
|
||||
|
||||
def _is_length_error_of_rejected_items(error: ValidationErrorDetail, errors: Sequence[ValidationErrorDetail]) -> bool:
|
||||
"""pydantic counts only items that validated, so a bad item also trips the parent's min_length."""
|
||||
return error["type"] == "too_short" and any(
|
||||
|
|
|
|||
|
|
@ -476,6 +476,7 @@ from litellm.proxy.common_utils.user_api_key_cache import (
|
|||
project_spend_counter_key,
|
||||
tag_cache_key,
|
||||
)
|
||||
from litellm.proxy.common_utils.validation_error_body import public_validation_errors
|
||||
from litellm.proxy.config_resolvers import (
|
||||
FieldSource,
|
||||
SettingsStore,
|
||||
|
|
@ -551,7 +552,6 @@ from litellm.proxy.hooks.proxy_track_cost_callback import _ProxyDBLogger, run_sp
|
|||
from litellm.proxy.image_endpoints.endpoints import router as image_router
|
||||
from litellm.proxy.list_api.common import (
|
||||
ManagementProblem,
|
||||
ValidationErrorDetail,
|
||||
problem_response,
|
||||
request_validation_problem,
|
||||
)
|
||||
|
|
@ -1983,16 +1983,14 @@ class _ExceptionRow(TypedDict, total=False):
|
|||
|
||||
@app.exception_handler(RequestValidationError)
|
||||
async def otel_request_validation_exception_handler(request: Request, exc: RequestValidationError):
|
||||
public_errors: Final = public_validation_errors(exc.errors())
|
||||
public_exc: Final = RequestValidationError(public_errors).with_traceback(exc.__traceback__)
|
||||
if request.url.path.startswith(MANAGEMENT_V1_PREFIX):
|
||||
validation_errors: Final[Sequence[ValidationErrorDetail]] = exc.errors()
|
||||
problem: Final = request_validation_problem(validation_errors)
|
||||
_close_dangling_otel_server_span(request, problem.status, exc=exc)
|
||||
problem: Final = request_validation_problem(public_errors)
|
||||
_close_dangling_otel_server_span(request, problem.status, exc=public_exc)
|
||||
return problem_response(problem)
|
||||
_close_dangling_otel_server_span(request, 422, exc=exc)
|
||||
return JSONResponse(
|
||||
status_code=422,
|
||||
content={"detail": jsonable_encoder(exc.errors())},
|
||||
)
|
||||
_close_dangling_otel_server_span(request, 422, exc=public_exc)
|
||||
return JSONResponse(status_code=422, content={"detail": public_errors})
|
||||
|
||||
|
||||
@app.exception_handler(Exception)
|
||||
|
|
|
|||
|
|
@ -25,6 +25,7 @@ from litellm.litellm_core_utils.sensitive_data_masker import mask_sensitive_keys
|
|||
from litellm.proxy._experimental.mcp_server.tool_search import MCP_TOOL_SEARCH_SETTINGS_KEY
|
||||
from litellm.proxy._types import *
|
||||
from litellm.proxy.auth.user_api_key_auth import user_api_key_auth
|
||||
from litellm.proxy.common_utils.validation_error_body import public_validation_errors
|
||||
from litellm.proxy.config_resolvers import FieldSource, SettingsStore, source_for
|
||||
from litellm.proxy.config_resolvers.settings_store import ConfigOwnedKeyError
|
||||
from litellm.proxy.config_resolvers.sso import (
|
||||
|
|
@ -1875,7 +1876,7 @@ async def update_ui_settings(
|
|||
try:
|
||||
settings: Final = effective_cls.model_validate(settings_body)
|
||||
except ValidationError as e:
|
||||
raise HTTPException(status_code=422, detail=e.errors())
|
||||
raise HTTPException(status_code=422, detail=public_validation_errors(e.errors()))
|
||||
|
||||
unsupported_team_fields: Final = sorted(
|
||||
frozenset(settings.team_admin_editable_team_fields) - SUPPORTED_TEAM_ADMIN_PERMISSIONS
|
||||
|
|
|
|||
|
|
@ -265,7 +265,7 @@ def test_close_dangling_otel_server_span_logger_raises_state_cleared_error(monke
|
|||
|
||||
@pytest.mark.asyncio
|
||||
async def test_otel_request_validation_exception_handler_returns_422_detail():
|
||||
errors = [{"loc": ["body", "model"], "msg": "field required", "type": "missing"}]
|
||||
errors = [{"loc": ["body", "model"], "msg": "field required", "type": "missing", "input": {"messages": []}}]
|
||||
exc = RequestValidationError(errors)
|
||||
request = _make_request()
|
||||
|
||||
|
|
@ -273,7 +273,69 @@ async def test_otel_request_validation_exception_handler_returns_422_detail():
|
|||
body = json.loads(response.body)
|
||||
|
||||
assert response.status_code == 422
|
||||
assert normalize(body) == {"detail": exc.errors()}
|
||||
assert body == {"detail": [{"type": "missing", "loc": ["body", "model"], "msg": "field required"}]}
|
||||
|
||||
|
||||
_SUBMITTED_PASSWORD: Final = "hunter2-Sup3rSecret!"
|
||||
_PASSWORD_LEAKING_ERRORS: Final = (
|
||||
{
|
||||
"type": "missing",
|
||||
"loc": ["body", "new_password"],
|
||||
"msg": "Field required",
|
||||
"input": {"current_password": _SUBMITTED_PASSWORD},
|
||||
},
|
||||
{
|
||||
"type": "value_error",
|
||||
"loc": ["body", "password"],
|
||||
"msg": "Value error, password cannot be set via /user/new",
|
||||
"input": _SUBMITTED_PASSWORD,
|
||||
"ctx": {"error": ValueError(_SUBMITTED_PASSWORD)},
|
||||
},
|
||||
)
|
||||
_PUBLIC_ERRORS: Final = (
|
||||
{"type": "missing", "loc": ["body", "new_password"], "msg": "Field required"},
|
||||
{"type": "value_error", "loc": ["body", "password"], "msg": "Value error, password cannot be set via /user/new"},
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_otel_request_validation_exception_handler_never_echoes_the_submitted_body():
|
||||
"""A pydantic error carries the offending value as ``input`` (the whole body for a
|
||||
``missing`` error) and input-derived values in ``ctx``; a caller who mistyped a
|
||||
request holding a password must not get that password back."""
|
||||
exc = RequestValidationError(list(_PASSWORD_LEAKING_ERRORS))
|
||||
|
||||
response = await otel_request_validation_exception_handler(request=_make_request(), exc=exc)
|
||||
|
||||
assert response.status_code == 422
|
||||
assert json.loads(response.body) == {"detail": list(_PUBLIC_ERRORS)}
|
||||
assert _SUBMITTED_PASSWORD.encode() not in response.body
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_otel_request_validation_exception_handler_hands_the_span_only_the_public_errors(monkeypatch):
|
||||
"""The OTEL SERVER span's error message is ``str(exc)``, which FastAPI builds from
|
||||
every error dict ``input`` included, so the span gets the same public-only errors
|
||||
the caller does, and keeps the traceback the original carried."""
|
||||
import litellm.proxy.proxy_server as ps
|
||||
|
||||
fake_logger = MagicMock()
|
||||
monkeypatch.setattr(ps, "open_telemetry_logger", fake_logger, raising=False)
|
||||
exc = RequestValidationError(list(_PASSWORD_LEAKING_ERRORS))
|
||||
try:
|
||||
raise exc
|
||||
except RequestValidationError as raised:
|
||||
original_traceback = raised.__traceback__
|
||||
request = _make_request(parent_otel_span=MagicMock())
|
||||
|
||||
await otel_request_validation_exception_handler(request=request, exc=exc)
|
||||
|
||||
(_span, span_exc, status_code) = fake_logger.record_error_attributes_on_span.call_args.args
|
||||
assert status_code == 422
|
||||
assert isinstance(span_exc, RequestValidationError)
|
||||
assert list(span_exc.errors()) == list(_PUBLIC_ERRORS)
|
||||
assert _SUBMITTED_PASSWORD not in str(span_exc)
|
||||
assert span_exc.__traceback__ is original_traceback
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
|
|
|||
|
|
@ -317,6 +317,25 @@ def test_claim_onboarding_link_missing_field_422(client, monkeypatch, mock_prism
|
|||
assert any("password" in str(item) for item in body["detail"])
|
||||
|
||||
|
||||
def test_claim_onboarding_link_422_never_echoes_the_submitted_password(client):
|
||||
"""A body that fails validation is answered with the field path and message only;
|
||||
pydantic's ``input`` (the whole submitted body for a missing field, password
|
||||
included) must never come back to the caller or land in whatever logs the response."""
|
||||
password = "hunter2-Sup3rSecret!"
|
||||
|
||||
response = client.post(
|
||||
"/onboarding/claim_token",
|
||||
json={"invitation_link": "abc", "password": password},
|
||||
)
|
||||
|
||||
assert response.status_code == 422
|
||||
assert password.encode() not in response.content
|
||||
detail = response.json()["detail"]
|
||||
assert detail[0]["loc"] == ["body", "user_id"]
|
||||
assert detail[0]["msg"]
|
||||
assert set(detail[0]) == {"type", "loc", "msg"}
|
||||
|
||||
|
||||
def test_claim_onboarding_link_bad_onboarding_jwt_401(
|
||||
client, monkeypatch, mock_prisma
|
||||
):
|
||||
|
|
|
|||
|
|
@ -3817,6 +3817,22 @@ class TestTeamAdminEditableTeamFieldsSetting:
|
|||
|
||||
assert response.status_code == 422
|
||||
|
||||
def test_patch_422_never_echoes_the_submitted_value(self, monkeypatch):
|
||||
self._as_proxy_admin(monkeypatch)
|
||||
submitted = "hunter2-Sup3rSecret!"
|
||||
|
||||
try:
|
||||
response = client.patch("/update/ui_settings", json={"team_admin_editable_team_fields": submitted})
|
||||
finally:
|
||||
app.dependency_overrides.clear()
|
||||
|
||||
assert response.status_code == 422
|
||||
assert submitted.encode() not in response.content
|
||||
detail = response.json()["detail"]
|
||||
assert detail[0]["loc"] == ["team_admin_editable_team_fields"]
|
||||
assert detail[0]["msg"]
|
||||
assert set(detail[0]) == {"type", "loc", "msg"}
|
||||
|
||||
def test_patch_persists_and_syncs_the_list_to_general_settings(self, monkeypatch):
|
||||
mock_prisma = self._as_proxy_admin(monkeypatch)
|
||||
general_settings: dict = {"team_admin_editable_team_fields": []}
|
||||
|
|
|
|||
46
tests/unit/proxy/common_utils/test_validation_error_body.py
Normal file
46
tests/unit/proxy/common_utils/test_validation_error_body.py
Normal file
|
|
@ -0,0 +1,46 @@
|
|||
from typing import Final
|
||||
|
||||
from litellm.proxy.common_utils.validation_error_body import public_validation_errors
|
||||
|
||||
_PASSWORD: Final = "hunter2-Sup3rSecret!"
|
||||
|
||||
|
||||
def test_public_validation_errors_drops_input_ctx_and_url():
|
||||
errors: Final = (
|
||||
{
|
||||
"type": "missing",
|
||||
"loc": ("body", "user_id"),
|
||||
"msg": "Field required",
|
||||
"input": {"invitation_link": "abc", "password": _PASSWORD},
|
||||
"url": "https://errors.pydantic.dev/2/v/missing",
|
||||
},
|
||||
{
|
||||
"type": "value_error",
|
||||
"loc": ("body", "password"),
|
||||
"msg": "Value error, password cannot be set here",
|
||||
"input": _PASSWORD,
|
||||
"ctx": {"error": ValueError(_PASSWORD)},
|
||||
},
|
||||
)
|
||||
|
||||
public: Final = public_validation_errors(errors)
|
||||
|
||||
assert public == (
|
||||
{"type": "missing", "loc": ("body", "user_id"), "msg": "Field required"},
|
||||
{"type": "value_error", "loc": ("body", "password"), "msg": "Value error, password cannot be set here"},
|
||||
)
|
||||
assert _PASSWORD not in repr(public)
|
||||
|
||||
|
||||
def test_public_validation_errors_keeps_type_loc_and_msg_verbatim_in_order():
|
||||
errors: Final = (
|
||||
{"type": "int_parsing", "loc": ("body", "litellm_params", "rpm"), "msg": "Input should be a valid integer"},
|
||||
{"type": "extra_forbidden", "loc": ("body", "users", 0, "user_emial"), "msg": "Extra inputs are not permitted"},
|
||||
{"type": "too_short", "loc": ("body", "users"), "msg": "List should have at least 1 item"},
|
||||
)
|
||||
|
||||
assert public_validation_errors(errors) == errors
|
||||
|
||||
|
||||
def test_public_validation_errors_empty_in_empty_out():
|
||||
assert public_validation_errors(()) == ()
|
||||
Loading…
Add table
Reference in a new issue