From ba64a1c451e1a732f014599348aeea13e1166474 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 18:19:34 -0700 Subject: [PATCH] fix(files): return 400 for a limit outside the documented range The unscoped GET /v1/files limit check accepted 0, which OpenAI's minimum of 1 does not allow, and the route's except block rebuilt every error with getattr(e, "status_code", 500). ProxyException has no status_code, so the 400 it raises went out as a 500 and the OpenAI SDK retried it three times. Errors now go through handle_exception_on_proxy, the helper the sibling batches route already uses, and the unknown-cursor error is a ProxyException so it carries type invalid_request_error and param after instead of the literal "None". The cursor still 400s whether the file belongs to someone else or does not exist at all --- .../proxy/hooks/managed_files.py | 11 +- .../openai_files_endpoints/common_utils.py | 6 +- .../openai_files_endpoints/files_endpoints.py | 18 +-- .../proxy/test_managed_files_hook.py | 45 +++++-- .../test_files_endpoint.py | 121 ++++++++++++++++++ 5 files changed, 165 insertions(+), 36 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index 4841fad2ec9..78d6674863c 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -1391,8 +1391,6 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): usable even when every file on the page was filtered out. """ validate_file_list_limit(limit) - if limit == 0: - return build_list_page([]) owner_filter: Final = build_owner_filter(user_api_key_dict) if owner_filter is None: @@ -1403,9 +1401,12 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): where={**owner_filter, "unified_file_id": after} ) if cursor_row is None: - raise HTTPException( - status_code=400, - detail=f"Invalid 'after' cursor: no file found with id '{after}'.", + raise ProxyException( + message=f"Invalid 'after' cursor: no file found with id '{after}'.", + type="invalid_request_error", + param="after", + code=400, + openai_code="invalid_value", ) page_size: Final = min(limit or MAX_FILE_LIST_LIMIT, MAX_FILE_LIST_LIMIT) diff --git a/litellm/proxy/openai_files_endpoints/common_utils.py b/litellm/proxy/openai_files_endpoints/common_utils.py index 22058fe3844..605da435848 100644 --- a/litellm/proxy/openai_files_endpoints/common_utils.py +++ b/litellm/proxy/openai_files_endpoints/common_utils.py @@ -28,11 +28,11 @@ MAX_FILE_LIST_LIMIT: Final = 10000 def validate_file_list_limit(limit: int | None) -> None: """Reject a ``limit`` outside the range OpenAI documents for GET /v1/files.""" - if limit is None or 0 <= limit <= MAX_FILE_LIST_LIMIT: + if limit is None or 1 <= limit <= MAX_FILE_LIST_LIMIT: return bound, expected, openai_code = ( - ("below minimum", ">= 0", "integer_below_min_value") - if limit < 0 + ("below minimum", ">= 1", "integer_below_min_value") + if limit < 1 else ("above maximum", f"<= {MAX_FILE_LIST_LIMIT}", "integer_above_max_value") ) raise ProxyException( diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index 3645da12ec5..6b460d0239b 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -68,7 +68,7 @@ from litellm.proxy.openai_files_endpoints.common_utils import ( validate_managed_files_requirement, validate_managed_id_requirement, ) -from litellm.proxy.utils import ProxyLogging, is_known_model +from litellm.proxy.utils import ProxyLogging, handle_exception_on_proxy, is_known_model from litellm.repositories.table_repositories import ManagedFileRepository from litellm.router import Router from litellm.types.llms.openai import ( @@ -1569,18 +1569,4 @@ async def list_files( ) verbose_proxy_logger.error("litellm.proxy.proxy_server.list_files(): Exception occured - %s", e) verbose_proxy_logger.debug(traceback.format_exc()) - if isinstance(e, HTTPException): - raise ProxyException( - message=getattr(e, "message", str(e.detail)), - type=getattr(e, "type", "None"), - param=getattr(e, "param", "None"), - code=getattr(e, "status_code", status.HTTP_400_BAD_REQUEST), - ) - else: - error_msg: Final = f"{e}" - raise ProxyException( - message=getattr(e, "message", error_msg), - type=getattr(e, "type", "None"), - param=getattr(e, "param", "None"), - code=getattr(e, "status_code", 500), - ) + raise handle_exception_on_proxy(e) diff --git a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py index 68ded79199d..091e287a96c 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -415,9 +415,14 @@ async def test_afile_list_pages_through_every_file_without_overlap(): assert table.find_many_calls[1]["skip"] == 1 +@pytest.mark.parametrize( + "unknown_cursor", + ["unified-theirs", "unified-nowhere"], + ids=["another-users-file", "no-such-file"], +) @pytest.mark.asyncio -async def test_afile_list_rejects_an_after_cursor_outside_the_callers_files(): - from fastapi import HTTPException +async def test_afile_list_rejects_an_after_cursor_outside_the_callers_files(unknown_cursor): + from litellm.proxy._types import ProxyException managed_files, table = _make_managed_files_over_rows( [ @@ -426,24 +431,35 @@ async def test_afile_list_rejects_an_after_cursor_outside_the_callers_files(): ] ) - with pytest.raises(HTTPException) as exc_info: + with pytest.raises(ProxyException) as exc_info: await managed_files.afile_list( purpose=None, litellm_parent_otel_span=None, user_api_key_dict=_make_user_api_key_dict(), - after="unified-theirs", + after=unknown_cursor, ) - assert exc_info.value.status_code == 400 + assert exc_info.value.code == "400" + assert exc_info.value.type == "invalid_request_error" + assert exc_info.value.param == "after" + assert exc_info.value.message == f"Invalid 'after' cursor: no file found with id '{unknown_cursor}'." assert table.find_first_calls[0] == { "created_by": "test-user", - "unified_file_id": "unified-theirs", + "unified_file_id": unknown_cursor, } assert table.find_many_calls == [] +@pytest.mark.parametrize( + "limit, bound, expected_range", + [ + (0, "below minimum", ">= 1"), + (-1, "below minimum", ">= 1"), + (10001, "above maximum", "<= 10000"), + ], +) @pytest.mark.asyncio -async def test_afile_list_rejects_a_limit_above_the_openai_maximum(): +async def test_afile_list_rejects_a_limit_outside_the_openai_range(limit, bound, expected_range): from litellm.proxy._types import ProxyException managed_files, table = _make_managed_files_over_rows([_make_managed_file_row("unified-mine")]) @@ -453,28 +469,33 @@ async def test_afile_list_rejects_a_limit_above_the_openai_maximum(): purpose=None, litellm_parent_otel_span=None, user_api_key_dict=_make_user_api_key_dict(), - limit=10001, + limit=limit, ) assert exc_info.value.code == "400" + assert exc_info.value.type == "invalid_request_error" assert exc_info.value.param == "limit" + assert exc_info.value.message == ( + f"Invalid 'limit': integer {bound} value. Expected a value {expected_range}, but got {limit} instead." + ) assert table.find_many_calls == [] +@pytest.mark.parametrize("limit", [1, 10000]) @pytest.mark.asyncio -async def test_afile_list_returns_an_empty_page_for_a_zero_limit(): +async def test_afile_list_accepts_the_ends_of_the_openai_limit_range(limit): managed_files, table = _make_managed_files_over_rows([_make_managed_file_row("unified-mine")]) response = await managed_files.afile_list( purpose=None, litellm_parent_otel_span=None, user_api_key_dict=_make_user_api_key_dict(), - limit=0, + limit=limit, ) - assert response["data"] == [] + assert [file.id for file in response["data"]] == ["unified-mine"] assert response["has_more"] is False - assert table.find_many_calls == [] + assert table.find_many_calls[0]["take"] == limit + 1 @pytest.mark.asyncio diff --git a/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py b/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py index d671047debb..08df05cdc8b 100644 --- a/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py +++ b/tests/test_litellm/proxy/openai_files_endpoint/test_files_endpoint.py @@ -2591,6 +2591,127 @@ def test_unscoped_list_files_forwards_limit_and_after_to_the_managed_file_store( proxy_logging_obj.post_call_failure_hook.assert_not_called() +def _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router: Router, afile_list): + """Wire GET /v1/files to the managed file store, with afile_list as the store.""" + import litellm.proxy.proxy_server as ps + from litellm.llms.base_llm.files.transformation import BaseFileEndpoints + from litellm.proxy._types import LitellmUserRoles + + proxy_logging_obj = setup_proxy_logging_object(monkeypatch, llm_router) + managed_files = mocker.MagicMock(spec=BaseFileEndpoints) + managed_files.afile_list = mocker.AsyncMock(side_effect=afile_list) + proxy_logging_obj.proxy_hook_mapping["managed_files"] = managed_files + proxy_logging_obj.update_request_status = mocker.AsyncMock() + proxy_logging_obj.post_call_success_hook = mocker.AsyncMock(return_value=None) + proxy_logging_obj.post_call_failure_hook = mocker.AsyncMock() + monkeypatch.setattr("litellm.proxy.proxy_server.master_key", None) + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", None) + monkeypatch.setattr("litellm.proxy.proxy_server.llm_router", llm_router) + mocker.patch.object(litellm, "afile_list", new=mocker.AsyncMock()) + + app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth( + api_key="test-key", + user_role=LitellmUserRoles.INTERNAL_USER, + user_id="test-user", + ) + return managed_files + + +def _get_unscoped_list_files(query: str): + try: + return client.get(f"/v1/files{query}", headers={"Authorization": "Bearer test-key"}) + finally: + import litellm.proxy.proxy_server as ps + + app.dependency_overrides.pop(ps.user_api_key_auth, None) + + +async def _validating_afile_list(**kwargs): + """Stand in for the managed file store, applying the real limit validation.""" + from litellm.proxy.openai_files_endpoints.common_utils import validate_file_list_limit + + validate_file_list_limit(kwargs.get("limit")) + return { + "object": "list", + "data": [], + "first_id": None, + "last_id": None, + "has_more": False, + } + + +@pytest.mark.parametrize( + "limit, bound, expected_range", + [ + (0, "below minimum", ">= 1"), + (-1, "below minimum", ">= 1"), + (10001, "above maximum", "<= 10000"), + ], +) +def test_unscoped_list_files_returns_400_for_a_limit_outside_the_openai_range( + mocker: MockerFixture, monkeypatch, llm_router: Router, limit, bound, expected_range +): + """An out-of-range limit is the caller's mistake, so it must not read as a 500 the SDK retries.""" + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _validating_afile_list) + + response = _get_unscoped_list_files(f"?limit={limit}") + + assert response.status_code == 400, response.text + assert response.json() == { + "error": { + "message": ( + f"Invalid 'limit': integer {bound} value. " + f"Expected a value {expected_range}, but got {limit} instead." + ), + "type": "invalid_request_error", + "param": "limit", + "code": "400", + } + } + + +@pytest.mark.parametrize("limit", [1, 10000]) +def test_unscoped_list_files_accepts_the_ends_of_the_openai_limit_range( + mocker: MockerFixture, monkeypatch, llm_router: Router, limit +): + managed_files = _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _validating_afile_list) + + response = _get_unscoped_list_files(f"?limit={limit}") + + assert response.status_code == 200, response.text + assert response.json()["data"] == [] + assert managed_files.afile_list.await_args.kwargs["limit"] == limit + + +def test_unscoped_list_files_returns_400_for_an_unknown_after_cursor( + mocker: MockerFixture, monkeypatch, llm_router: Router +): + from litellm.proxy._types import ProxyException + + async def _unknown_cursor(**kwargs): + raise ProxyException( + message=f"Invalid 'after' cursor: no file found with id '{kwargs['after']}'.", + type="invalid_request_error", + param="after", + code=400, + openai_code="invalid_value", + ) + + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _unknown_cursor) + + response = _get_unscoped_list_files("?after=file-does-not-exist-xyz") + + assert response.status_code == 400, response.text + assert response.json() == { + "error": { + "message": "Invalid 'after' cursor: no file found with id 'file-does-not-exist-xyz'.", + "type": "invalid_request_error", + "param": "after", + "code": "400", + } + } + + def test_list_files_restricted_team_does_not_leak_global_openai_credentials( mocker: MockerFixture, monkeypatch ):