diff --git a/litellm/proxy/common_utils/validation_error_body.py b/litellm/proxy/common_utils/validation_error_body.py new file mode 100644 index 00000000000..b21f33a2434 --- /dev/null +++ b/litellm/proxy/common_utils/validation_error_body.py @@ -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) diff --git a/litellm/proxy/list_api/common.py b/litellm/proxy/list_api/common.py index daa6414fd94..efa8a271459 100644 --- a/litellm/proxy/list_api/common.py +++ b/litellm/proxy/list_api/common.py @@ -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( diff --git a/litellm/proxy/proxy_server.py b/litellm/proxy/proxy_server.py index 61304f0d919..646eca071d1 100644 --- a/litellm/proxy/proxy_server.py +++ b/litellm/proxy/proxy_server.py @@ -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) diff --git a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py index c91b1afd64a..227f0e7f795 100644 --- a/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py +++ b/litellm/proxy/ui_crud_endpoints/proxy_setting_endpoints.py @@ -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 diff --git a/tests/test_litellm/proxy/proxy_server/test_exception_handlers.py b/tests/test_litellm/proxy/proxy_server/test_exception_handlers.py index 089c2d57594..16cb1146ff5 100644 --- a/tests/test_litellm/proxy/proxy_server/test_exception_handlers.py +++ b/tests/test_litellm/proxy/proxy_server/test_exception_handlers.py @@ -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 diff --git a/tests/test_litellm/proxy/proxy_server/test_routes_onboarding.py b/tests/test_litellm/proxy/proxy_server/test_routes_onboarding.py index 778acc1baab..6c1d869d113 100644 --- a/tests/test_litellm/proxy/proxy_server/test_routes_onboarding.py +++ b/tests/test_litellm/proxy/proxy_server/test_routes_onboarding.py @@ -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 ): diff --git a/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py b/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py index 08d542df16c..0d7a713a380 100644 --- a/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py +++ b/tests/test_litellm/proxy/ui_crud_endpoints/test_proxy_setting_endpoints.py @@ -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": []} diff --git a/tests/unit/proxy/common_utils/test_validation_error_body.py b/tests/unit/proxy/common_utils/test_validation_error_body.py new file mode 100644 index 00000000000..a86f17b7461 --- /dev/null +++ b/tests/unit/proxy/common_utils/test_validation_error_body.py @@ -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(()) == ()