From f9f8320972f6589dd5aac0877a1bc89c4f200028 Mon Sep 17 00:00:00 2001 From: Rithvik Mysore Suresh Date: Fri, 21 Aug 2026 11:47:04 -0400 Subject: [PATCH 01/10] fix(files): list unscoped managed files Read owner-scoped managed rows directly when no provider or model is supplied, avoiding an unauthenticated OpenAI fallback. Refs #35362 --- .../proxy/hooks/managed_files.py | 19 ++++-- litellm/llms/base_llm/files/transformation.py | 6 +- .../openai_files_endpoints/files_endpoints.py | 36 ++++++----- .../proxy/test_managed_files_hook.py | 33 +++++++++++ .../test_files_endpoint.py | 59 +++++++++++++++++++ 5 files changed, 130 insertions(+), 23 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index c986e835e4f..9b62284072d 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -1365,12 +1365,23 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): async def afile_list( self, - purpose: Optional[OpenAIFilesPurpose], + purpose: str | None, litellm_parent_otel_span: Optional[Span], + user_api_key_dict: UserAPIKeyAuth, **data: Dict, - ) -> List[OpenAIFileObject]: - """Handled in files_endpoints.py""" - return [] + ) -> Dict[str, object]: + owner_filter: Final = build_owner_filter(user_api_key_dict) + if owner_filter is None: + return build_list_page([]) + + rows: Final = await _managed_file_table(self.prisma_client).find_many(where=owner_filter) + files: Final = [ + parsed_file_object.model_copy(update={"id": row.unified_file_id}) + for row in rows + if (parsed_file_object := _parse_managed_file_object(row.file_object, row.unified_file_id)) is not None + and (purpose is None or parsed_file_object.purpose == purpose) + ] + return build_list_page(files) def _is_batch_polling_enabled(self) -> bool: """ diff --git a/litellm/llms/base_llm/files/transformation.py b/litellm/llms/base_llm/files/transformation.py index 174be93448b..7c19326b627 100644 --- a/litellm/llms/base_llm/files/transformation.py +++ b/litellm/llms/base_llm/files/transformation.py @@ -13,7 +13,6 @@ from litellm.types.llms.openai import ( FileContentRequest, OpenAICreateFileRequestOptionalParams, OpenAIFileObject, - OpenAIFilesPurpose, ) from litellm.types.utils import LlmProviders, ModelResponse @@ -240,10 +239,11 @@ class BaseFileEndpoints(ABC): @abstractmethod async def afile_list( self, - purpose: OpenAIFilesPurpose | None, + purpose: str | None, litellm_parent_otel_span: Span | None, + user_api_key_dict: UserAPIKeyAuth, **data: dict, - ) -> list[OpenAIFileObject]: + ) -> dict[str, object]: pass @abstractmethod diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index 37cfd9d073d..a482cc54748 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -1488,24 +1488,28 @@ async def list_files( or get_custom_llm_provider_from_request_headers(request=request) or get_custom_llm_provider_from_request_query(request=request) or await get_custom_llm_provider_from_request_body(request=request) - or "openai" ) + managed_files_obj: Final = proxy_logging_obj.get_proxy_hook("managed_files") + if custom_llm_provider is None and isinstance(managed_files_obj, BaseFileEndpoints): + response = await managed_files_obj.afile_list( + purpose=purpose, + litellm_parent_otel_span=user_api_key_dict.parent_otel_span, + user_api_key_dict=user_api_key_dict, + ) + else: + resolved_custom_llm_provider: Final = custom_llm_provider or "openai" + apply_team_provider_credentials( + data=data, + llm_router=llm_router, + user_api_key_dict=user_api_key_dict, + custom_llm_provider=resolved_custom_llm_provider, + ) - # No model/target_model_names pinned: resolve upstream credentials from - # the team's deployment for this provider so the call is authenticated - # against the team's own account (e.g. the team's openai deployment). - apply_team_provider_credentials( - data=data, - llm_router=llm_router, - user_api_key_dict=user_api_key_dict, - custom_llm_provider=custom_llm_provider, - ) - - response = await litellm.afile_list( - custom_llm_provider=custom_llm_provider, - purpose=purpose, - **data, - ) + response = await litellm.afile_list( + custom_llm_provider=resolved_custom_llm_provider, + purpose=purpose, + **data, + ) if response is None: raise HTTPException( 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 fcd03e77aa2..b39d2ef8559 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -190,6 +190,39 @@ async def test_get_user_created_file_ids_remaps_stored_raw_provider_id_to_unifie assert files[0].purpose == raw_provider_object.purpose +@pytest.mark.asyncio +async def test_afile_list_returns_owner_scoped_managed_files(): + managed_files = _make_managed_files_instance() + managed_files.prisma_client.db.litellm_managedfiletable.find_many = AsyncMock( + return_value=[ + MagicMock( + file_object=_make_file_object("file-provider-id").model_dump(), + unified_file_id="unified-file-id", + ), + MagicMock( + file_object=_make_file_object("file-other-purpose").model_copy( + update={"purpose": "batch"} + ).model_dump(), + unified_file_id="unified-other-purpose", + ), + ] + ) + + response = await managed_files.afile_list( + purpose="batch_output", + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + managed_files.prisma_client.db.litellm_managedfiletable.find_many.assert_awaited_once_with( + where={"created_by": "test-user"} + ) + assert [file.id for file in response["data"]] == ["unified-file-id"] + assert response["first_id"] == "unified-file-id" + assert response["last_id"] == "unified-file-id" + assert response["has_more"] is False + + @pytest.mark.asyncio async def test_parse_managed_file_object_warning_omits_rejected_values(caplog): from litellm_enterprise.proxy.hooks.managed_files import ( 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 bf9323cdc6a..e6101d3edd8 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 @@ -2468,6 +2468,65 @@ def test_list_files_without_target_model_names_uses_team_openai_deployment( proxy_logging_obj.post_call_failure_hook.assert_not_called() +def test_unscoped_list_files_uses_managed_file_store( + mocker: MockerFixture, monkeypatch, llm_router: Router +): + import litellm.proxy.proxy_server as ps + from litellm.llms.base_llm.files.transformation import BaseFileEndpoints + from litellm.proxy._types import LitellmUserRoles + + managed_file = OpenAIFileObject( + id="unified-file-id", + object="file", + bytes=100, + created_at=1700000000, + filename="output.jsonl", + purpose="batch_output", + status="processed", + ) + + proxy_logging_obj = setup_proxy_logging_object(monkeypatch, llm_router) + managed_files = mocker.MagicMock(spec=BaseFileEndpoints) + managed_files.afile_list = mocker.AsyncMock( + return_value={ + "object": "list", + "data": [managed_file], + "first_id": managed_file.id, + "last_id": managed_file.id, + "has_more": False, + } + ) + 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) + provider_list = 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", + ) + + try: + response = client.get( + "/v1/files", + headers={"Authorization": "Bearer test-key"}, + ) + finally: + app.dependency_overrides.pop(ps.user_api_key_auth, None) + + assert response.status_code == 200, response.text + assert response.json()["data"][0]["id"] == "unified-file-id" + managed_files.afile_list.assert_awaited_once() + assert managed_files.afile_list.await_args.kwargs["user_api_key_dict"].user_id == "test-user" + provider_list.assert_not_awaited() + proxy_logging_obj.post_call_failure_hook.assert_not_called() + + def test_list_files_restricted_team_does_not_leak_global_openai_credentials( mocker: MockerFixture, monkeypatch ): From 75fd4b1448551d5e9f5d076a4b881b523888c7b2 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:33:07 -0700 Subject: [PATCH 02/10] fix(files): paginate the unscoped managed file listing The owner-scoped listing read every row the caller owns in one query, so an admin key that owns every file on the proxy pulled the whole table into one response. Page it with a keyset cursor on unified_file_id instead, and accept limit and after on GET /v1/files so a client can walk the pages. limit follows what OpenAI documents for that route: 1 to 10000, default 10000. An after cursor is resolved inside the caller's own scope, so an id they do not own gets a 400 rather than a page, and has_more now reflects whether another row exists instead of always being false. Refs #37714 --- .../proxy/hooks/managed_files.py | 52 +++- litellm/llms/base_llm/files/transformation.py | 2 + .../openai_files_endpoints/common_utils.py | 22 ++ .../openai_files_endpoints/files_endpoints.py | 4 + .../proxy/test_managed_files_hook.py | 222 +++++++++++++++++- .../test_files_endpoint.py | 64 +++++ 6 files changed, 360 insertions(+), 6 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index 9b62284072d..ca73a0574da 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -45,6 +45,7 @@ from litellm.proxy._types import ( UserAPIKeyAuth, ) from litellm.proxy.openai_files_endpoints.common_utils import ( + MAX_FILE_LIST_LIMIT, _is_base64_encoded_unified_file_id, apply_unified_file_ids, ensure_batch_response_managed_file_ids, @@ -54,6 +55,7 @@ from litellm.proxy.openai_files_endpoints.common_utils import ( map_raw_file_ids_to_unified, normalize_mime_type_for_provider, resolve_managed_output_file_model_name, + validate_file_list_limit, ) from litellm.proxy.pass_through_endpoints.llm_provider_handlers.batch_attribution import ( request_tags_from_metadata, @@ -144,7 +146,14 @@ class _ManagedFileRow(Protocol): class _ManagedFileTableActions(Protocol): async def find_first(self, where: Mapping[str, object]) -> Optional[_ManagedFileRow]: ... - async def find_many(self, where: Mapping[str, object]) -> Sequence[_ManagedFileRow]: ... + async def find_many( + self, + where: Mapping[str, object], + take: int = ..., + order: Union[Mapping[str, str], Sequence[Mapping[str, str]]] = ..., + cursor: Mapping[str, str] = ..., + skip: int = ..., + ) -> Sequence[_ManagedFileRow]: ... async def upsert(self, where: Mapping[str, str], data: Mapping[str, Mapping[str, object]]) -> _ManagedFileRow: ... @@ -1365,23 +1374,56 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): async def afile_list( self, - purpose: str | None, + purpose: Optional[str], litellm_parent_otel_span: Optional[Span], user_api_key_dict: UserAPIKeyAuth, + limit: Optional[int] = None, + after: Optional[str] = None, **data: Dict, ) -> Dict[str, object]: + """List the managed files the caller owns, newest first. + + Pagination is keyset based on ``unified_file_id`` so a key that owns + every file on the proxy still reads one bounded page at a time. + ``purpose`` is applied after parsing because the managed file table + keeps it inside the ``file_object`` blob instead of a column. + """ + 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: return build_list_page([]) - rows: Final = await _managed_file_table(self.prisma_client).find_many(where=owner_filter) + if after: + cursor_row = await _managed_file_table(self.prisma_client).find_first( + 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}'.", + ) + + page_size: Final = min(limit or MAX_FILE_LIST_LIMIT, MAX_FILE_LIST_LIMIT) + cursor_args: _CursorPageArgs = {"cursor": {"unified_file_id": after}, "skip": 1} if after else {} + + rows: Final = await _managed_file_table(self.prisma_client).find_many( + where=owner_filter, + take=page_size + 1, + order=[{"created_at": "desc"}, {"unified_file_id": "desc"}], + **cursor_args, + ) + has_more: Final = len(rows) > page_size + files: Final = [ parsed_file_object.model_copy(update={"id": row.unified_file_id}) - for row in rows + for row in rows[:page_size] if (parsed_file_object := _parse_managed_file_object(row.file_object, row.unified_file_id)) is not None and (purpose is None or parsed_file_object.purpose == purpose) ] - return build_list_page(files) + return build_list_page(files, has_more=has_more) def _is_batch_polling_enabled(self) -> bool: """ diff --git a/litellm/llms/base_llm/files/transformation.py b/litellm/llms/base_llm/files/transformation.py index 7c19326b627..1576af41e76 100644 --- a/litellm/llms/base_llm/files/transformation.py +++ b/litellm/llms/base_llm/files/transformation.py @@ -242,6 +242,8 @@ class BaseFileEndpoints(ABC): purpose: str | None, litellm_parent_otel_span: Span | None, user_api_key_dict: UserAPIKeyAuth, + limit: int | None = None, + after: str | None = None, **data: dict, ) -> dict[str, object]: pass diff --git a/litellm/proxy/openai_files_endpoints/common_utils.py b/litellm/proxy/openai_files_endpoints/common_utils.py index b9af01e9aea..22058fe3844 100644 --- a/litellm/proxy/openai_files_endpoints/common_utils.py +++ b/litellm/proxy/openai_files_endpoints/common_utils.py @@ -6,6 +6,7 @@ from dataclasses import dataclass, field from types import MappingProxyType from typing import TYPE_CHECKING, Final, Literal, Optional, Protocol, runtime_checkable +from litellm.proxy._types import ProxyException from litellm.repositories.table_repositories import ( ManagedFileRepository, ManagedObjectRepository, @@ -22,6 +23,27 @@ if TYPE_CHECKING: from litellm.types.utils import LiteLLMBatch +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: + return + bound, expected, openai_code = ( + ("below minimum", ">= 0", "integer_below_min_value") + if limit < 0 + else ("above maximum", f"<= {MAX_FILE_LIST_LIMIT}", "integer_above_max_value") + ) + raise ProxyException( + message=f"Invalid 'limit': integer {bound} value. Expected a value {expected}, but got {limit} instead.", + type="invalid_request_error", + param="limit", + code=400, + openai_code=openai_code, + ) + + @runtime_checkable class ManagedResourceAccessChecker(Protocol): async def can_user_call_unified_file_id( diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index 90a32be9419..3645da12ec5 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -1410,6 +1410,8 @@ async def list_files( provider: str | None = None, target_model_names: str | None = None, purpose: str | None = None, + limit: int | None = None, + after: str | None = None, ): """ Returns information about a specific file. that can be used across - Assistants API, Batch API @@ -1507,6 +1509,8 @@ async def list_files( purpose=purpose, litellm_parent_otel_span=user_api_key_dict.parent_otel_span, user_api_key_dict=user_api_key_dict, + limit=limit, + after=after, ) else: resolved_custom_llm_provider: Final = custom_llm_provider or "openai" 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 b39d2ef8559..49a1119c7a0 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -66,6 +66,62 @@ def _make_user_api_key_dict() -> UserAPIKeyAuth: ) +def _make_managed_file_row( + unified_file_id: str, + purpose: str = "batch_output", + created_by: str = "test-user", +) -> MagicMock: + file_object = _make_file_object(f"file-provider-{unified_file_id}").model_copy( + update={"purpose": purpose} + ) + return MagicMock( + unified_file_id=unified_file_id, + file_object=file_object.model_dump(), + created_by=created_by, + ) + + +class _FakeManagedFileTable: + """In-memory stand-in for the managed file table, newest row first.""" + + def __init__(self, rows): + self.rows = list(rows) + self.find_many_calls = [] + self.find_first_calls = [] + + def _owned_rows(self, where): + created_by = where.get("created_by") + return [row for row in self.rows if created_by is None or row.created_by == created_by] + + async def find_first(self, where): + self.find_first_calls.append(where) + return next( + (row for row in self._owned_rows(where) if row.unified_file_id == where.get("unified_file_id")), + None, + ) + + async def find_many(self, where, take=None, order=None, cursor=None, skip=0): + self.find_many_calls.append( + {"where": where, "take": take, "order": order, "cursor": cursor, "skip": skip} + ) + rows = self._owned_rows(where) + if cursor is not None: + start = next( + index + for index, row in enumerate(rows) + if row.unified_file_id == cursor["unified_file_id"] + ) + rows = rows[start + skip :] + return rows if take is None else rows[:take] + + +def _make_managed_files_over_rows(rows): + managed_files = _make_managed_files_instance() + table = _FakeManagedFileTable(rows) + managed_files.prisma_client.db.litellm_managedfiletable = table + return managed_files, table + + def _make_managed_files_instance(): """Create a _PROXY_LiteLLMManagedFiles with storage methods mocked out.""" from litellm_enterprise.proxy.hooks.managed_files import ( @@ -215,7 +271,9 @@ async def test_afile_list_returns_owner_scoped_managed_files(): ) managed_files.prisma_client.db.litellm_managedfiletable.find_many.assert_awaited_once_with( - where={"created_by": "test-user"} + where={"created_by": "test-user"}, + take=10001, + order=[{"created_at": "desc"}, {"unified_file_id": "desc"}], ) assert [file.id for file in response["data"]] == ["unified-file-id"] assert response["first_id"] == "unified-file-id" @@ -223,6 +281,168 @@ async def test_afile_list_returns_owner_scoped_managed_files(): assert response["has_more"] is False +@pytest.mark.asyncio +async def test_afile_list_does_not_leak_another_callers_files(): + managed_files, table = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-mine-2"), + _make_managed_file_row("unified-theirs", created_by="other-user"), + _make_managed_file_row("unified-mine-1"), + ] + ) + + response = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + assert [file.id for file in response["data"]] == ["unified-mine-2", "unified-mine-1"] + assert table.find_many_calls[0]["where"] == {"created_by": "test-user"} + + +@pytest.mark.asyncio +async def test_afile_list_denies_a_caller_without_a_user_or_team(): + 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=UserAPIKeyAuth(api_key="sk-test", parent_otel_span=None), + ) + + assert response["data"] == [] + assert response["has_more"] is False + assert table.find_many_calls == [] + + +@pytest.mark.asyncio +async def test_afile_list_filters_by_purpose(): + managed_files, _ = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-batch-output"), + _make_managed_file_row("unified-batch", purpose="batch"), + ] + ) + + response = await managed_files.afile_list( + purpose="batch", + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + assert [file.id for file in response["data"]] == ["unified-batch"] + + +@pytest.mark.asyncio +async def test_afile_list_honors_limit_and_reports_more_pages(): + managed_files, table = _make_managed_files_over_rows( + [_make_managed_file_row(f"unified-{index}") for index in range(5)] + ) + + response = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + limit=2, + ) + + assert [file.id for file in response["data"]] == ["unified-0", "unified-1"] + assert response["has_more"] is True + assert table.find_many_calls[0]["take"] == 3 + + +@pytest.mark.asyncio +async def test_afile_list_pages_through_every_file_without_overlap(): + managed_files, table = _make_managed_files_over_rows( + [_make_managed_file_row(f"unified-{index}") for index in range(5)] + ) + user_api_key_dict = _make_user_api_key_dict() + + seen = [] + after = None + while True: + page = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=user_api_key_dict, + limit=2, + after=after, + ) + page_ids = [file.id for file in page["data"]] + assert not set(page_ids) & set(seen) + seen.extend(page_ids) + if not page["has_more"]: + break + after = page["last_id"] + + assert seen == [f"unified-{index}" for index in range(5)] + assert table.find_many_calls[1]["cursor"] == {"unified_file_id": "unified-1"} + assert table.find_many_calls[1]["skip"] == 1 + + +@pytest.mark.asyncio +async def test_afile_list_rejects_an_after_cursor_outside_the_callers_files(): + from fastapi import HTTPException + + managed_files, table = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-mine"), + _make_managed_file_row("unified-theirs", created_by="other-user"), + ] + ) + + with pytest.raises(HTTPException) 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", + ) + + assert exc_info.value.status_code == 400 + assert table.find_first_calls[0] == { + "created_by": "test-user", + "unified_file_id": "unified-theirs", + } + assert table.find_many_calls == [] + + +@pytest.mark.asyncio +async def test_afile_list_rejects_a_limit_above_the_openai_maximum(): + from litellm.proxy._types import ProxyException + + managed_files, table = _make_managed_files_over_rows([_make_managed_file_row("unified-mine")]) + + 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(), + limit=10001, + ) + + assert exc_info.value.code == "400" + assert exc_info.value.param == "limit" + assert table.find_many_calls == [] + + +@pytest.mark.asyncio +async def test_afile_list_returns_an_empty_page_for_a_zero_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, + ) + + assert response["data"] == [] + assert response["has_more"] is False + assert table.find_many_calls == [] + + @pytest.mark.asyncio async def test_parse_managed_file_object_warning_omits_rejected_values(caplog): from litellm_enterprise.proxy.hooks.managed_files import ( 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 e6101d3edd8..d671047debb 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 @@ -2523,6 +2523,70 @@ def test_unscoped_list_files_uses_managed_file_store( assert response.json()["data"][0]["id"] == "unified-file-id" managed_files.afile_list.assert_awaited_once() assert managed_files.afile_list.await_args.kwargs["user_api_key_dict"].user_id == "test-user" + assert managed_files.afile_list.await_args.kwargs["limit"] is None + assert managed_files.afile_list.await_args.kwargs["after"] is None + provider_list.assert_not_awaited() + proxy_logging_obj.post_call_failure_hook.assert_not_called() + + +def test_unscoped_list_files_forwards_limit_and_after_to_the_managed_file_store( + mocker: MockerFixture, monkeypatch, llm_router: Router +): + import litellm.proxy.proxy_server as ps + from litellm.llms.base_llm.files.transformation import BaseFileEndpoints + from litellm.proxy._types import LitellmUserRoles + + second_page_file = OpenAIFileObject( + id="unified-file-id-2", + object="file", + bytes=100, + created_at=1700000000, + filename="output.jsonl", + purpose="batch", + status="processed", + ) + + proxy_logging_obj = setup_proxy_logging_object(monkeypatch, llm_router) + managed_files = mocker.MagicMock(spec=BaseFileEndpoints) + managed_files.afile_list = mocker.AsyncMock( + return_value={ + "object": "list", + "data": [second_page_file], + "first_id": second_page_file.id, + "last_id": second_page_file.id, + "has_more": True, + } + ) + 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) + provider_list = 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", + ) + + try: + response = client.get( + "/v1/files?limit=2&after=unified-file-id-1&purpose=batch", + headers={"Authorization": "Bearer test-key"}, + ) + finally: + app.dependency_overrides.pop(ps.user_api_key_auth, None) + + assert response.status_code == 200, response.text + assert response.json()["data"][0]["id"] == "unified-file-id-2" + assert response.json()["has_more"] is True + call_kwargs = managed_files.afile_list.await_args.kwargs + assert call_kwargs["limit"] == 2 + assert call_kwargs["after"] == "unified-file-id-1" + assert call_kwargs["purpose"] == "batch" provider_list.assert_not_awaited() proxy_logging_obj.post_call_failure_hook.assert_not_called() From 138b0da21f58111107adb4560c9202cb0f84e2a6 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:33:30 -0700 Subject: [PATCH 03/10] chore(dashboard): regenerate api types for the files list params --- ui/litellm-dashboard/src/lib/http/schema.d.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ui/litellm-dashboard/src/lib/http/schema.d.ts b/ui/litellm-dashboard/src/lib/http/schema.d.ts index cf55dc69e86..d311bfa3cbc 100644 --- a/ui/litellm-dashboard/src/lib/http/schema.d.ts +++ b/ui/litellm-dashboard/src/lib/http/schema.d.ts @@ -43144,6 +43144,8 @@ export interface operations { provider?: string | null; target_model_names?: string | null; purpose?: string | null; + limit?: number | null; + after?: string | null; }; header?: never; path?: never; @@ -58015,6 +58017,8 @@ export interface operations { provider?: string | null; target_model_names?: string | null; purpose?: string | null; + limit?: number | null; + after?: string | null; }; header?: never; path?: never; @@ -64268,6 +64272,8 @@ export interface operations { query?: { target_model_names?: string | null; purpose?: string | null; + limit?: number | null; + after?: string | null; }; header?: never; path: { From a15b81d3d71e78fb9bbe7c67943eead99956e699 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 17:46:35 -0700 Subject: [PATCH 04/10] fix(files): keep the list cursor usable on a filtered page A page whose rows are all dropped by the purpose filter, or by a row that does not parse, used to come back with an empty data list, has_more true and last_id null, so the caller had no cursor to advance with and stopped one page short of files it owns. last_id now falls back to the last row the page read. Also drops the OpenAIFilesPurpose import that the widened purpose annotation left unused. --- .../proxy/hooks/managed_files.py | 15 +++++--- .../base_llm/managed_resources/isolation.py | 14 ++++++-- .../proxy/test_managed_files_hook.py | 34 +++++++++++++++++++ 3 files changed, 56 insertions(+), 7 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index ca73a0574da..4841fad2ec9 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -67,7 +67,6 @@ from litellm.types.llms.openai import ( # pyright: ignore[reportAttributeAccess CreateFileRequest, FileObject, OpenAIFileObject, - OpenAIFilesPurpose, ResponsesAPIResponse, ) from litellm.types.utils import ( @@ -1386,7 +1385,10 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): Pagination is keyset based on ``unified_file_id`` so a key that owns every file on the proxy still reads one bounded page at a time. ``purpose`` is applied after parsing because the managed file table - keeps it inside the ``file_object`` blob instead of a column. + keeps it inside the ``file_object`` blob instead of a column, so a + narrowed page can hold fewer files than ``limit``. ``last_id`` then + falls back to the last row the page read, which keeps the cursor + usable even when every file on the page was filtered out. """ validate_file_list_limit(limit) if limit == 0: @@ -1416,14 +1418,19 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): **cursor_args, ) has_more: Final = len(rows) > page_size + page_rows: Final = rows[:page_size] files: Final = [ parsed_file_object.model_copy(update={"id": row.unified_file_id}) - for row in rows[:page_size] + for row in page_rows if (parsed_file_object := _parse_managed_file_object(row.file_object, row.unified_file_id)) is not None and (purpose is None or parsed_file_object.purpose == purpose) ] - return build_list_page(files, has_more=has_more) + return build_list_page( + files, + has_more=has_more, + next_cursor_id=page_rows[-1].unified_file_id if page_rows else None, + ) def _is_batch_polling_enabled(self) -> bool: """ diff --git a/litellm/llms/base_llm/managed_resources/isolation.py b/litellm/llms/base_llm/managed_resources/isolation.py index e1b204214d7..f1a54943de0 100644 --- a/litellm/llms/base_llm/managed_resources/isolation.py +++ b/litellm/llms/base_llm/managed_resources/isolation.py @@ -19,15 +19,23 @@ from litellm.proxy._types import ( ) -def build_list_page(items: list[Any], has_more: bool = False) -> dict[str, Any]: +def build_list_page( + items: list[Any], + has_more: bool = False, + next_cursor_id: str | None = None, +) -> dict[str, Any]: """Build the OpenAI-style paginated list response shape used by managed file/batch/vector-store listings. ``first_id`` and ``last_id`` are - sourced from each item's ``.id`` attribute.""" + sourced from each item's ``.id`` attribute. + + A listing that filters rows out after reading them can pass + ``next_cursor_id`` so an empty page still carries the cursor the caller + needs to reach the rows behind it.""" return { "object": "list", "data": items, "first_id": items[0].id if items else None, - "last_id": items[-1].id if items else None, + "last_id": items[-1].id if items else next_cursor_id, "has_more": has_more, } 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 49a1119c7a0..68ded79199d 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -334,6 +334,40 @@ async def test_afile_list_filters_by_purpose(): assert [file.id for file in response["data"]] == ["unified-batch"] +@pytest.mark.asyncio +async def test_afile_list_keeps_a_usable_cursor_when_a_page_filters_everything_out(): + managed_files, _ = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-0"), + _make_managed_file_row("unified-1"), + _make_managed_file_row("unified-2", purpose="batch"), + ] + ) + user_api_key_dict = _make_user_api_key_dict() + + first_page = await managed_files.afile_list( + purpose="batch", + litellm_parent_otel_span=None, + user_api_key_dict=user_api_key_dict, + limit=2, + ) + + assert first_page["data"] == [] + assert first_page["has_more"] is True + assert first_page["last_id"] == "unified-1" + + second_page = await managed_files.afile_list( + purpose="batch", + litellm_parent_otel_span=None, + user_api_key_dict=user_api_key_dict, + limit=2, + after=first_page["last_id"], + ) + + assert [file.id for file in second_page["data"]] == ["unified-2"] + assert second_page["has_more"] is False + + @pytest.mark.asyncio async def test_afile_list_honors_limit_and_reports_more_pages(): managed_files, table = _make_managed_files_over_rows( 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 05/10] 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 ): From 6b63623ca0bbcc72a9647cb35351c775e0223928 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:14:40 -0700 Subject: [PATCH 06/10] fix(files): never hand the sdk an empty page while matches remain The managed file listing cut the page to `limit` first and applied the purpose filter in Python afterwards, so a page whose rows all failed the filter came back as `data: []` with `has_more: true`. openai-python stops paging the moment `data` is empty, so `files.list(purpose="batch", limit=1)` returned nothing at all instead of every batch file. Read successive keyset chunks until the page holds `limit + 1` matches or the caller's rows run out, then return at most `limit` of them. `data` is now non-empty whenever matching files remain, its last id is always a usable cursor, and `has_more: false` only ever means the caller has seen everything. Rows whose stored blob will not parse drop out in the same loop, so they cannot empty a page either. That also makes the `next_cursor_id` escape hatch on `build_list_page` dead, so it goes back to what it was for the batch and vector-store listings that share it. Also move `validate_file_list_limit` up into the list_files route, so the target_model_names and provider branches reject an out-of-range limit the same way the managed file store already did. --- .../proxy/hooks/managed_files.py | 53 ++++----- .../base_llm/managed_resources/isolation.py | 14 +-- .../openai_files_endpoints/files_endpoints.py | 3 + .../proxy/test_managed_files_hook.py | 104 +++++++++++++++++- .../test_files_endpoint.py | 56 ++++++++-- 5 files changed, 178 insertions(+), 52 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index 78d6674863c..be37f648397 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -1384,11 +1384,13 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): Pagination is keyset based on ``unified_file_id`` so a key that owns every file on the proxy still reads one bounded page at a time. - ``purpose`` is applied after parsing because the managed file table - keeps it inside the ``file_object`` blob instead of a column, so a - narrowed page can hold fewer files than ``limit``. ``last_id`` then - falls back to the last row the page read, which keeps the cursor - usable even when every file on the page was filtered out. + ``purpose`` is applied after parsing, because the managed file table + keeps it inside the ``file_object`` blob instead of a column, and rows + whose blob will not parse drop out there too, so a chunk of rows can + yield fewer matches than the page holds. Successive chunks are read + until the page is full or the caller's rows run out, which keeps + ``data`` non-empty while matches remain and its last id usable as the + next cursor. """ validate_file_list_limit(limit) @@ -1410,28 +1412,29 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): ) page_size: Final = min(limit or MAX_FILE_LIST_LIMIT, MAX_FILE_LIST_LIMIT) - cursor_args: _CursorPageArgs = {"cursor": {"unified_file_id": after}, "skip": 1} if after else {} + chunk_size: Final = page_size + 1 + matches: Final[List[OpenAIFileObject]] = [] + cursor_id = after - rows: Final = await _managed_file_table(self.prisma_client).find_many( - where=owner_filter, - take=page_size + 1, - order=[{"created_at": "desc"}, {"unified_file_id": "desc"}], - **cursor_args, - ) - has_more: Final = len(rows) > page_size - page_rows: Final = rows[:page_size] + while len(matches) <= page_size: + cursor_args: _CursorPageArgs = {"cursor": {"unified_file_id": cursor_id}, "skip": 1} if cursor_id else {} + chunk = await _managed_file_table(self.prisma_client).find_many( + where=owner_filter, + take=chunk_size, + order=[{"created_at": "desc"}, {"unified_file_id": "desc"}], + **cursor_args, + ) + matches.extend( + parsed_file_object.model_copy(update={"id": row.unified_file_id}) + for row in chunk + if (parsed_file_object := _parse_managed_file_object(row.file_object, row.unified_file_id)) is not None + and (purpose is None or parsed_file_object.purpose == purpose) + ) + if len(chunk) < chunk_size: + break + cursor_id = chunk[-1].unified_file_id - files: Final = [ - parsed_file_object.model_copy(update={"id": row.unified_file_id}) - for row in page_rows - if (parsed_file_object := _parse_managed_file_object(row.file_object, row.unified_file_id)) is not None - and (purpose is None or parsed_file_object.purpose == purpose) - ] - return build_list_page( - files, - has_more=has_more, - next_cursor_id=page_rows[-1].unified_file_id if page_rows else None, - ) + return build_list_page(matches[:page_size], has_more=len(matches) > page_size) def _is_batch_polling_enabled(self) -> bool: """ diff --git a/litellm/llms/base_llm/managed_resources/isolation.py b/litellm/llms/base_llm/managed_resources/isolation.py index f1a54943de0..e1b204214d7 100644 --- a/litellm/llms/base_llm/managed_resources/isolation.py +++ b/litellm/llms/base_llm/managed_resources/isolation.py @@ -19,23 +19,15 @@ from litellm.proxy._types import ( ) -def build_list_page( - items: list[Any], - has_more: bool = False, - next_cursor_id: str | None = None, -) -> dict[str, Any]: +def build_list_page(items: list[Any], has_more: bool = False) -> dict[str, Any]: """Build the OpenAI-style paginated list response shape used by managed file/batch/vector-store listings. ``first_id`` and ``last_id`` are - sourced from each item's ``.id`` attribute. - - A listing that filters rows out after reading them can pass - ``next_cursor_id`` so an empty page still carries the cursor the caller - needs to reach the rows behind it.""" + sourced from each item's ``.id`` attribute.""" return { "object": "list", "data": items, "first_id": items[0].id if items else None, - "last_id": items[-1].id if items else next_cursor_id, + "last_id": items[-1].id if items else None, "has_more": has_more, } diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index 6b460d0239b..c9c794d94e3 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -65,6 +65,7 @@ from litellm.proxy.openai_files_endpoints.common_utils import ( get_credentials_for_model, handle_model_based_routing, prepare_data_with_credentials, + validate_file_list_limit, validate_managed_files_requirement, validate_managed_id_requirement, ) @@ -1436,6 +1437,8 @@ async def list_files( data: dict = {} try: + validate_file_list_limit(limit) + # Include original request and headers in the data base_llm_response_processor: Final = ProxyBaseLLMRequestProcessing(data=data) ( 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 091e287a96c..40abcac1008 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -81,6 +81,14 @@ def _make_managed_file_row( ) +def _make_unparseable_managed_file_row( + unified_file_id: str, + created_by: str = "test-user", +) -> MagicMock: + """A row whose stored blob cannot be parsed back into a file object.""" + return MagicMock(unified_file_id=unified_file_id, file_object=None, created_by=created_by) + + class _FakeManagedFileTable: """In-memory stand-in for the managed file table, newest row first.""" @@ -334,13 +342,37 @@ async def test_afile_list_filters_by_purpose(): assert [file.id for file in response["data"]] == ["unified-batch"] +async def _walk_afile_list(managed_files, user_api_key_dict, purpose, limit): + """Page through the listing the way the official SDK does, off ``data[-1].id``.""" + seen = [] + after = None + while True: + page = await managed_files.afile_list( + purpose=purpose, + litellm_parent_otel_span=None, + user_api_key_dict=user_api_key_dict, + limit=limit, + after=after, + ) + page_ids = [file.id for file in page["data"]] + assert not set(page_ids) & set(seen) + seen.extend(page_ids) + if not page["has_more"]: + return seen + assert page_ids, "an SDK stops paging on an empty page, so has_more must never ride one" + after = page_ids[-1] + + @pytest.mark.asyncio -async def test_afile_list_keeps_a_usable_cursor_when_a_page_filters_everything_out(): +async def test_afile_list_fills_a_page_past_rows_the_purpose_filter_drops(): + """The newest rows do not match, so the page must reach past them rather than come back empty.""" managed_files, _ = _make_managed_files_over_rows( [ _make_managed_file_row("unified-0"), _make_managed_file_row("unified-1"), _make_managed_file_row("unified-2", purpose="batch"), + _make_managed_file_row("unified-3"), + _make_managed_file_row("unified-4", purpose="batch"), ] ) user_api_key_dict = _make_user_api_key_dict() @@ -349,25 +381,85 @@ async def test_afile_list_keeps_a_usable_cursor_when_a_page_filters_everything_o purpose="batch", litellm_parent_otel_span=None, user_api_key_dict=user_api_key_dict, - limit=2, + limit=1, ) - assert first_page["data"] == [] + assert [file.id for file in first_page["data"]] == ["unified-2"] assert first_page["has_more"] is True - assert first_page["last_id"] == "unified-1" + assert first_page["last_id"] == "unified-2" second_page = await managed_files.afile_list( purpose="batch", litellm_parent_otel_span=None, user_api_key_dict=user_api_key_dict, - limit=2, + limit=1, after=first_page["last_id"], ) - assert [file.id for file in second_page["data"]] == ["unified-2"] + assert [file.id for file in second_page["data"]] == ["unified-4"] assert second_page["has_more"] is False +@pytest.mark.parametrize("limit", [1, 2, 3]) +@pytest.mark.asyncio +async def test_afile_list_walks_every_purpose_match_at_any_limit(limit): + managed_files, _ = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-0"), + _make_managed_file_row("unified-1"), + _make_managed_file_row("unified-2", purpose="batch"), + _make_managed_file_row("unified-3"), + _make_managed_file_row("unified-4", purpose="batch"), + _make_managed_file_row("unified-5", purpose="batch"), + _make_managed_file_row("unified-6"), + ] + ) + + seen = await _walk_afile_list(managed_files, _make_user_api_key_dict(), "batch", limit) + + assert seen == ["unified-2", "unified-4", "unified-5"] + + +@pytest.mark.asyncio +async def test_afile_list_fills_a_page_past_rows_that_do_not_parse(): + managed_files, _ = _make_managed_files_over_rows( + [ + _make_unparseable_managed_file_row("unified-0"), + _make_unparseable_managed_file_row("unified-1"), + _make_managed_file_row("unified-2"), + ] + ) + + page = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + limit=1, + ) + + assert [file.id for file in page["data"]] == ["unified-2"] + assert page["has_more"] is False + + +@pytest.mark.asyncio +async def test_afile_list_reports_no_more_pages_when_nothing_matches(): + managed_files, _ = _make_managed_files_over_rows( + [_make_managed_file_row(f"unified-{index}") for index in range(5)] + ) + + page = await managed_files.afile_list( + purpose="batch", + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + limit=2, + ) + + assert page["data"] == [] + assert page["has_more"] is False + assert page["first_id"] is None + assert page["last_id"] is None + + @pytest.mark.asyncio async def test_afile_list_honors_limit_and_reports_more_pages(): managed_files, table = _make_managed_files_over_rows( 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 08df05cdc8b..5bc77f1bd7a 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 @@ -1,7 +1,7 @@ import json import os import sys -from typing import List +from typing import Final, List from unittest.mock import ANY, AsyncMock import pytest @@ -2617,27 +2617,39 @@ def _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router: Router, af return managed_files -def _get_unscoped_list_files(query: str): +def _get_list_files(path: str): try: - return client.get(f"/v1/files{query}", headers={"Authorization": "Bearer test-key"}) + return client.get(path, headers={"Authorization": "Bearer test-key"}) finally: import litellm.proxy.proxy_server as ps app.dependency_overrides.pop(ps.user_api_key_auth, None) +def _get_unscoped_list_files(query: str): + return _get_list_files(f"/v1/files{query}") + + +_EMPTY_FILE_LIST_PAGE: Final = { + "object": "list", + "data": [], + "first_id": None, + "last_id": None, + "has_more": False, +} + + 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, - } + return dict(_EMPTY_FILE_LIST_PAGE) + + +async def _permissive_afile_list(**kwargs): + """Stand in for a file store that validates nothing, so only the route can reject.""" + return dict(_EMPTY_FILE_LIST_PAGE) @pytest.mark.parametrize( @@ -2683,6 +2695,30 @@ def test_unscoped_list_files_accepts_the_ends_of_the_openai_limit_range( assert managed_files.afile_list.await_args.kwargs["limit"] == limit +@pytest.mark.parametrize( + "path", + [ + "/v1/files?limit=0", + "/v1/files?limit=0&target_model_names=gpt-3.5-turbo", + "/openai/v1/files?limit=0", + ], + ids=["managed-file-store", "target-model-names", "provider-route"], +) +def test_list_files_validates_the_limit_on_every_branch( + mocker: MockerFixture, monkeypatch, llm_router: Router, path +): + """The limit is a route-level contract, so the scoped and provider branches reject it too.""" + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _permissive_afile_list) + + response = _get_list_files(path) + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "limit" + assert response.json()["error"]["message"] == ( + "Invalid 'limit': integer below minimum value. Expected a value >= 1, but got 0 instead." + ) + + def test_unscoped_list_files_returns_400_for_an_unknown_after_cursor( mocker: MockerFixture, monkeypatch, llm_router: Router ): From b9f5c45aa848e8ea109ee5410674da65617e1ede Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:25:57 -0700 Subject: [PATCH 07/10] fix(files): bound the queries a filtered file page can cost The chunk loop read `limit + 1` rows at a time, so a small limit whose matches sit far behind the newest rows advanced a couple of rows per query. A `purpose` that matches only the last of 10000 owned rows at `limit=1` cost 5001 sequential find_many calls for one HTTP request, which any authenticated caller could ask for on purpose. Once a scan has to continue past its first chunk, widen the chunk to FILE_LIST_CONTINUATION_CHUNK_SIZE. That same case now costs 21 queries. The first chunk keeps its `limit + 1` size, so a page the newest rows already fill still costs exactly one query and reads nothing extra. Rows whose blob will not parse drop out of a page the way a filter does, so they get the bound too, not just the purpose filter. The floor only changes how many round trips a page costs, never what it returns: chunk boundaries do not affect a keyset scan, so the page is still `matches[:page_size]`, `has_more` is still `len(matches) > page_size`, and empty data still implies `has_more` false. --- .../proxy/hooks/managed_files.py | 9 ++- .../openai_files_endpoints/common_utils.py | 2 + .../proxy/test_managed_files_hook.py | 63 +++++++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index be37f648397..a70d85cf59c 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -45,6 +45,7 @@ from litellm.proxy._types import ( UserAPIKeyAuth, ) from litellm.proxy.openai_files_endpoints.common_utils import ( + FILE_LIST_CONTINUATION_CHUNK_SIZE, MAX_FILE_LIST_LIMIT, _is_base64_encoded_unified_file_id, apply_unified_file_ids, @@ -1390,7 +1391,10 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): yield fewer matches than the page holds. Successive chunks are read until the page is full or the caller's rows run out, which keeps ``data`` non-empty while matches remain and its last id usable as the - next cursor. + next cursor. A first chunk that fills the page costs one query; once a + scan has to continue past it, the chunk widens to + ``FILE_LIST_CONTINUATION_CHUNK_SIZE`` so a page whose matches sit far + behind the newest rows cannot degenerate into thousands of queries. """ validate_file_list_limit(limit) @@ -1412,9 +1416,9 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): ) page_size: Final = min(limit or MAX_FILE_LIST_LIMIT, MAX_FILE_LIST_LIMIT) - chunk_size: Final = page_size + 1 matches: Final[List[OpenAIFileObject]] = [] cursor_id = after + chunk_size = page_size + 1 while len(matches) <= page_size: cursor_args: _CursorPageArgs = {"cursor": {"unified_file_id": cursor_id}, "skip": 1} if cursor_id else {} @@ -1433,6 +1437,7 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): if len(chunk) < chunk_size: break cursor_id = chunk[-1].unified_file_id + chunk_size = max(chunk_size, FILE_LIST_CONTINUATION_CHUNK_SIZE) return build_list_page(matches[:page_size], has_more=len(matches) > page_size) diff --git a/litellm/proxy/openai_files_endpoints/common_utils.py b/litellm/proxy/openai_files_endpoints/common_utils.py index 605da435848..837b4c43652 100644 --- a/litellm/proxy/openai_files_endpoints/common_utils.py +++ b/litellm/proxy/openai_files_endpoints/common_utils.py @@ -25,6 +25,8 @@ if TYPE_CHECKING: MAX_FILE_LIST_LIMIT: Final = 10000 +FILE_LIST_CONTINUATION_CHUNK_SIZE: Final = 500 + def validate_file_list_limit(limit: int | None) -> None: """Reject a ``limit`` outside the range OpenAI documents for GET /v1/files.""" 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 40abcac1008..0b81c1d23e3 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -441,6 +441,69 @@ async def test_afile_list_fills_a_page_past_rows_that_do_not_parse(): assert page["has_more"] is False +_DEEP_SCAN_ROW_COUNT = 2000 +_DEEP_SCAN_QUERY_BUDGET = 10 + + +@pytest.mark.asyncio +async def test_afile_list_bounds_the_queries_a_deep_purpose_match_costs(): + """A tiny limit over rows the filter drops must not turn one request into thousands of queries.""" + managed_files, table = _make_managed_files_over_rows( + [_make_managed_file_row(f"unified-{index:05d}") for index in range(_DEEP_SCAN_ROW_COUNT)] + + [_make_managed_file_row("unified-match", purpose="batch")] + ) + + page = await managed_files.afile_list( + purpose="batch", + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + limit=1, + ) + + assert [file.id for file in page["data"]] == ["unified-match"] + assert page["has_more"] is False + assert len(table.find_many_calls) <= _DEEP_SCAN_QUERY_BUDGET + + +@pytest.mark.asyncio +async def test_afile_list_bounds_the_queries_a_deep_unparseable_run_costs(): + """Rows that will not parse drop out like a filter does, so they get the same bound.""" + managed_files, table = _make_managed_files_over_rows( + [_make_unparseable_managed_file_row(f"unified-{index:05d}") for index in range(_DEEP_SCAN_ROW_COUNT)] + + [_make_managed_file_row("unified-parses")] + ) + + page = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + limit=1, + ) + + assert [file.id for file in page["data"]] == ["unified-parses"] + assert page["has_more"] is False + assert len(table.find_many_calls) <= _DEEP_SCAN_QUERY_BUDGET + + +@pytest.mark.asyncio +async def test_afile_list_reads_one_chunk_when_the_first_one_fills_the_page(): + """The widened chunk must stay off the common path, where the newest rows already fill the page.""" + managed_files, table = _make_managed_files_over_rows( + [_make_managed_file_row(f"unified-{index:05d}") for index in range(_DEEP_SCAN_ROW_COUNT)] + ) + + page = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + limit=2, + ) + + assert [file.id for file in page["data"]] == ["unified-00000", "unified-00001"] + assert page["has_more"] is True + assert [call["take"] for call in table.find_many_calls] == [3] + + @pytest.mark.asyncio async def test_afile_list_reports_no_more_pages_when_nothing_matches(): managed_files, _ = _make_managed_files_over_rows( From 56953707767e591b5fefedf267321d85b6af69b4 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Fri, 21 Aug 2026 20:33:57 -0700 Subject: [PATCH 08/10] fix(files): hand post-call hooks a page object, not a bare dict The managed hook returned the plain dict build_list_page builds, while every other GET /v1/files path returns an SDK page object. A post-call success hook or a logging callback that reads response.data off the listing raised AttributeError as soon as a request took the managed path FileListPage is a pydantic model over the same five fields, so hooks read .data again and the response body does not move: jsonable_encoder gives the same keys in the same order for the model and for the dict. It sits in litellm.types.llms.openai because base_llm/files/transformation.py already imports from there and cannot import proxy modules. It is deliberately not subscriptable, since the provider-backed path returns a page object that is not either, and dict access would be a third contract to keep alive Also reject a purpose the Files API never accepts. An unknown purpose matches no row, so the listing answered an empty page for what is really a bad request, while the upload route in this same file already refuses those values against get_args(OpenAIFilesPurpose). The check runs before the first query, and only in the managed hook, so providers that define their own purposes keep them Also put back the route's original except tail. Sending every error through handle_exception_on_proxy changed error.type on a bad target_model_names from "None" to the exception class name, which a caller matching on the body would read as a break. create_file in this file already pairs base's tail with a ProxyException passthrough, so list_files does the same and the handle_exception_on_proxy import is gone --- .../proxy/hooks/managed_files.py | 9 +- litellm/llms/base_llm/files/transformation.py | 3 +- .../openai_files_endpoints/common_utils.py | 22 ++- .../openai_files_endpoints/files_endpoints.py | 20 ++- litellm/types/llms/openai.py | 16 +++ .../proxy/test_managed_files_hook.py | 134 +++++++++++++----- .../test_files_endpoint.py | 131 ++++++++++++++++- 7 files changed, 286 insertions(+), 49 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index a70d85cf59c..e71b520d27f 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -57,6 +57,7 @@ from litellm.proxy.openai_files_endpoints.common_utils import ( normalize_mime_type_for_provider, resolve_managed_output_file_model_name, validate_file_list_limit, + validate_file_list_purpose, ) from litellm.proxy.pass_through_endpoints.llm_provider_handlers.batch_attribution import ( request_tags_from_metadata, @@ -66,6 +67,7 @@ from litellm.types.llms.openai import ( # pyright: ignore[reportAttributeAccess AsyncCursorPage, ChatCompletionFileObject, CreateFileRequest, + FileListPage, FileObject, OpenAIFileObject, ResponsesAPIResponse, @@ -1380,7 +1382,7 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): limit: Optional[int] = None, after: Optional[str] = None, **data: Dict, - ) -> Dict[str, object]: + ) -> FileListPage: """List the managed files the caller owns, newest first. Pagination is keyset based on ``unified_file_id`` so a key that owns @@ -1397,10 +1399,11 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): behind the newest rows cannot degenerate into thousands of queries. """ validate_file_list_limit(limit) + validate_file_list_purpose(purpose) owner_filter: Final = build_owner_filter(user_api_key_dict) if owner_filter is None: - return build_list_page([]) + return FileListPage(**build_list_page([])) if after: cursor_row = await _managed_file_table(self.prisma_client).find_first( @@ -1439,7 +1442,7 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): cursor_id = chunk[-1].unified_file_id chunk_size = max(chunk_size, FILE_LIST_CONTINUATION_CHUNK_SIZE) - return build_list_page(matches[:page_size], has_more=len(matches) > page_size) + return FileListPage(**build_list_page(matches[:page_size], has_more=len(matches) > page_size)) def _is_batch_polling_enabled(self) -> bool: """ diff --git a/litellm/llms/base_llm/files/transformation.py b/litellm/llms/base_llm/files/transformation.py index 1576af41e76..b20fe0f1560 100644 --- a/litellm/llms/base_llm/files/transformation.py +++ b/litellm/llms/base_llm/files/transformation.py @@ -11,6 +11,7 @@ from litellm.types.llms.openai import ( AllMessageValues, CreateFileRequest, FileContentRequest, + FileListPage, OpenAICreateFileRequestOptionalParams, OpenAIFileObject, ) @@ -245,7 +246,7 @@ class BaseFileEndpoints(ABC): limit: int | None = None, after: str | None = None, **data: dict, - ) -> dict[str, object]: + ) -> FileListPage: pass @abstractmethod diff --git a/litellm/proxy/openai_files_endpoints/common_utils.py b/litellm/proxy/openai_files_endpoints/common_utils.py index 837b4c43652..c5213d842a3 100644 --- a/litellm/proxy/openai_files_endpoints/common_utils.py +++ b/litellm/proxy/openai_files_endpoints/common_utils.py @@ -4,13 +4,14 @@ import re from collections.abc import Mapping from dataclasses import dataclass, field from types import MappingProxyType -from typing import TYPE_CHECKING, Final, Literal, Optional, Protocol, runtime_checkable +from typing import TYPE_CHECKING, Final, Literal, Optional, Protocol, get_args, runtime_checkable from litellm.proxy._types import ProxyException from litellm.repositories.table_repositories import ( ManagedFileRepository, ManagedObjectRepository, ) +from litellm.types.llms.openai import OpenAIFilesPurpose from litellm.types.utils import SpecialEnums if TYPE_CHECKING: @@ -46,6 +47,25 @@ def validate_file_list_limit(limit: int | None) -> None: ) +def validate_file_list_purpose(purpose: str | None) -> None: + """Reject a ``purpose`` filter the Files API never accepts. + + An unknown purpose matches no file, so filtering on it would report an + empty page for what is really a bad request. Rejecting it keeps a managed + listing consistent with the upload route and with the provider-backed + listings, which both refuse the same values. + """ + valid_purposes: Final = get_args(OpenAIFilesPurpose) + if purpose is None or purpose in valid_purposes: + return + raise ProxyException( + message=f"Invalid purpose: {purpose}. Must be one of: {valid_purposes}", + type="invalid_request_error", + param="purpose", + code=400, + ) + + @runtime_checkable class ManagedResourceAccessChecker(Protocol): async def can_user_call_unified_file_id( diff --git a/litellm/proxy/openai_files_endpoints/files_endpoints.py b/litellm/proxy/openai_files_endpoints/files_endpoints.py index c9c794d94e3..92bbd58ed90 100644 --- a/litellm/proxy/openai_files_endpoints/files_endpoints.py +++ b/litellm/proxy/openai_files_endpoints/files_endpoints.py @@ -69,7 +69,7 @@ from litellm.proxy.openai_files_endpoints.common_utils import ( validate_managed_files_requirement, validate_managed_id_requirement, ) -from litellm.proxy.utils import ProxyLogging, handle_exception_on_proxy, is_known_model +from litellm.proxy.utils import ProxyLogging, is_known_model from litellm.repositories.table_repositories import ManagedFileRepository from litellm.router import Router from litellm.types.llms.openai import ( @@ -1572,4 +1572,20 @@ 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()) - raise handle_exception_on_proxy(e) + if isinstance(e, ProxyException): + raise + 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), + ) diff --git a/litellm/types/llms/openai.py b/litellm/types/llms/openai.py index 1588c650177..37de518b231 100644 --- a/litellm/types/llms/openai.py +++ b/litellm/types/llms/openai.py @@ -65,6 +65,7 @@ from pydantic import ( BaseModel, ConfigDict, Discriminator, + Field, PrivateAttr, field_serializer, field_validator, @@ -381,6 +382,21 @@ class OpenAIFileObject(BaseModel): return self.dict() +class FileListPage(BaseModel): + """A page of files, as `GET /v1/files` returns it. + + Post-call hooks and logging callbacks are handed the listing response, and + the provider SDKs hand them a page object rather than a mapping, so this + exposes the same ``.data`` attribute while serializing to an identical body. + """ + + object: Literal["list"] = "list" + data: list[OpenAIFileObject] = Field(default_factory=list) + first_id: str | None = None + last_id: str | None = None + has_more: bool = False + + CREATE_FILE_REQUESTS_PURPOSE = Literal["assistants", "batch", "fine-tune", "messages"] 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 0b81c1d23e3..1cd6813b065 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -14,8 +14,8 @@ import pytest from typing import Optional from unittest.mock import AsyncMock, MagicMock, patch -from litellm.proxy._types import UserAPIKeyAuth -from litellm.types.llms.openai import OpenAIFileObject +from litellm.proxy._types import ProxyException, UserAPIKeyAuth +from litellm.types.llms.openai import FileListPage, OpenAIFileObject from litellm.types.utils import LiteLLMBatch @@ -283,10 +283,70 @@ async def test_afile_list_returns_owner_scoped_managed_files(): take=10001, order=[{"created_at": "desc"}, {"unified_file_id": "desc"}], ) - assert [file.id for file in response["data"]] == ["unified-file-id"] - assert response["first_id"] == "unified-file-id" - assert response["last_id"] == "unified-file-id" - assert response["has_more"] is False + assert [file.id for file in response.data] == ["unified-file-id"] + assert response.first_id == "unified-file-id" + assert response.last_id == "unified-file-id" + assert response.has_more is False + + +@pytest.mark.asyncio +async def test_afile_list_returns_a_page_object_callbacks_can_read(): + """Post-call hooks receive the listing and read ``.data`` off it, the way the + provider SDK's page lets them. The body on the wire stays a plain list page.""" + from fastapi.encoders import jsonable_encoder + + managed_files, _ = _make_managed_files_over_rows([_make_managed_file_row("unified-file-id")]) + + page = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + assert isinstance(page, FileListPage) + assert [file.id for file in page.data] == ["unified-file-id"] + + body = jsonable_encoder(page) + assert list(body) == ["object", "data", "first_id", "last_id", "has_more"] + assert body["object"] == "list" + assert [file["id"] for file in body["data"]] == ["unified-file-id"] + assert body["first_id"] == "unified-file-id" + assert body["last_id"] == "unified-file-id" + assert body["has_more"] is False + + +@pytest.mark.asyncio +@pytest.mark.parametrize("purpose", ["nonexistent_purpose", "EVALS", "batch "]) +async def test_afile_list_rejects_a_purpose_the_files_api_never_accepts(purpose): + """No stored file can carry an undocumented purpose, so filtering on one is a + bad request rather than a legitimately empty page.""" + managed_files, table = _make_managed_files_over_rows([_make_managed_file_row("unified-file-id")]) + + with pytest.raises(ProxyException) as exc_info: + await managed_files.afile_list( + purpose=purpose, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + assert exc_info.value.code == "400" + assert exc_info.value.type == "invalid_request_error" + assert exc_info.value.param == "purpose" + assert table.find_many_calls == [] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("purpose", ["batch", "assistants", "fine-tune", None]) +async def test_afile_list_accepts_every_documented_purpose(purpose): + managed_files, _ = _make_managed_files_over_rows([_make_managed_file_row("unified-file-id")]) + + page = await managed_files.afile_list( + purpose=purpose, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + assert isinstance(page, FileListPage) @pytest.mark.asyncio @@ -305,7 +365,7 @@ async def test_afile_list_does_not_leak_another_callers_files(): user_api_key_dict=_make_user_api_key_dict(), ) - assert [file.id for file in response["data"]] == ["unified-mine-2", "unified-mine-1"] + assert [file.id for file in response.data] == ["unified-mine-2", "unified-mine-1"] assert table.find_many_calls[0]["where"] == {"created_by": "test-user"} @@ -319,8 +379,8 @@ async def test_afile_list_denies_a_caller_without_a_user_or_team(): user_api_key_dict=UserAPIKeyAuth(api_key="sk-test", parent_otel_span=None), ) - assert response["data"] == [] - assert response["has_more"] is False + assert response.data == [] + assert response.has_more is False assert table.find_many_calls == [] @@ -339,7 +399,7 @@ async def test_afile_list_filters_by_purpose(): user_api_key_dict=_make_user_api_key_dict(), ) - assert [file.id for file in response["data"]] == ["unified-batch"] + assert [file.id for file in response.data] == ["unified-batch"] async def _walk_afile_list(managed_files, user_api_key_dict, purpose, limit): @@ -354,10 +414,10 @@ async def _walk_afile_list(managed_files, user_api_key_dict, purpose, limit): limit=limit, after=after, ) - page_ids = [file.id for file in page["data"]] + page_ids = [file.id for file in page.data] assert not set(page_ids) & set(seen) seen.extend(page_ids) - if not page["has_more"]: + if not page.has_more: return seen assert page_ids, "an SDK stops paging on an empty page, so has_more must never ride one" after = page_ids[-1] @@ -384,20 +444,20 @@ async def test_afile_list_fills_a_page_past_rows_the_purpose_filter_drops(): limit=1, ) - assert [file.id for file in first_page["data"]] == ["unified-2"] - assert first_page["has_more"] is True - assert first_page["last_id"] == "unified-2" + assert [file.id for file in first_page.data] == ["unified-2"] + assert first_page.has_more is True + assert first_page.last_id == "unified-2" second_page = await managed_files.afile_list( purpose="batch", litellm_parent_otel_span=None, user_api_key_dict=user_api_key_dict, limit=1, - after=first_page["last_id"], + after=first_page.last_id, ) - assert [file.id for file in second_page["data"]] == ["unified-4"] - assert second_page["has_more"] is False + assert [file.id for file in second_page.data] == ["unified-4"] + assert second_page.has_more is False @pytest.mark.parametrize("limit", [1, 2, 3]) @@ -437,8 +497,8 @@ async def test_afile_list_fills_a_page_past_rows_that_do_not_parse(): limit=1, ) - assert [file.id for file in page["data"]] == ["unified-2"] - assert page["has_more"] is False + assert [file.id for file in page.data] == ["unified-2"] + assert page.has_more is False _DEEP_SCAN_ROW_COUNT = 2000 @@ -460,8 +520,8 @@ async def test_afile_list_bounds_the_queries_a_deep_purpose_match_costs(): limit=1, ) - assert [file.id for file in page["data"]] == ["unified-match"] - assert page["has_more"] is False + assert [file.id for file in page.data] == ["unified-match"] + assert page.has_more is False assert len(table.find_many_calls) <= _DEEP_SCAN_QUERY_BUDGET @@ -480,8 +540,8 @@ async def test_afile_list_bounds_the_queries_a_deep_unparseable_run_costs(): limit=1, ) - assert [file.id for file in page["data"]] == ["unified-parses"] - assert page["has_more"] is False + assert [file.id for file in page.data] == ["unified-parses"] + assert page.has_more is False assert len(table.find_many_calls) <= _DEEP_SCAN_QUERY_BUDGET @@ -499,8 +559,8 @@ async def test_afile_list_reads_one_chunk_when_the_first_one_fills_the_page(): limit=2, ) - assert [file.id for file in page["data"]] == ["unified-00000", "unified-00001"] - assert page["has_more"] is True + assert [file.id for file in page.data] == ["unified-00000", "unified-00001"] + assert page.has_more is True assert [call["take"] for call in table.find_many_calls] == [3] @@ -517,10 +577,10 @@ async def test_afile_list_reports_no_more_pages_when_nothing_matches(): limit=2, ) - assert page["data"] == [] - assert page["has_more"] is False - assert page["first_id"] is None - assert page["last_id"] is None + assert page.data == [] + assert page.has_more is False + assert page.first_id is None + assert page.last_id is None @pytest.mark.asyncio @@ -536,8 +596,8 @@ async def test_afile_list_honors_limit_and_reports_more_pages(): limit=2, ) - assert [file.id for file in response["data"]] == ["unified-0", "unified-1"] - assert response["has_more"] is True + assert [file.id for file in response.data] == ["unified-0", "unified-1"] + assert response.has_more is True assert table.find_many_calls[0]["take"] == 3 @@ -558,12 +618,12 @@ async def test_afile_list_pages_through_every_file_without_overlap(): limit=2, after=after, ) - page_ids = [file.id for file in page["data"]] + page_ids = [file.id for file in page.data] assert not set(page_ids) & set(seen) seen.extend(page_ids) - if not page["has_more"]: + if not page.has_more: break - after = page["last_id"] + after = page.last_id assert seen == [f"unified-{index}" for index in range(5)] assert table.find_many_calls[1]["cursor"] == {"unified_file_id": "unified-1"} @@ -648,8 +708,8 @@ async def test_afile_list_accepts_the_ends_of_the_openai_limit_range(limit): limit=limit, ) - assert [file.id for file in response["data"]] == ["unified-mine"] - assert response["has_more"] is False + assert [file.id for file in response.data] == ["unified-mine"] + assert response.has_more is False assert table.find_many_calls[0]["take"] == limit + 1 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 5bc77f1bd7a..fbee23108cf 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 @@ -24,7 +24,11 @@ from litellm.proxy.openai_files_endpoints.file_content_streaming_handler import FileContentStreamingHandler, ) from litellm.proxy.proxy_server import app -from litellm.types.llms.openai import HttpxBinaryResponseContent, OpenAIFileObject +from litellm.types.llms.openai import ( + FileListPage, + HttpxBinaryResponseContent, + OpenAIFileObject, +) client = TestClient(app) from litellm.caching.caching import DualCache @@ -2640,16 +2644,20 @@ _EMPTY_FILE_LIST_PAGE: Final = { 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 + """Stand in for the managed file store, applying the real request validation.""" + from litellm.proxy.openai_files_endpoints.common_utils import ( + validate_file_list_limit, + validate_file_list_purpose, + ) validate_file_list_limit(kwargs.get("limit")) - return dict(_EMPTY_FILE_LIST_PAGE) + validate_file_list_purpose(kwargs.get("purpose")) + return FileListPage(**_EMPTY_FILE_LIST_PAGE) async def _permissive_afile_list(**kwargs): """Stand in for a file store that validates nothing, so only the route can reject.""" - return dict(_EMPTY_FILE_LIST_PAGE) + return FileListPage(**_EMPTY_FILE_LIST_PAGE) @pytest.mark.parametrize( @@ -2748,6 +2756,119 @@ def test_unscoped_list_files_returns_400_for_an_unknown_after_cursor( } +def _managed_file(file_id: str) -> OpenAIFileObject: + return OpenAIFileObject( + id=file_id, + bytes=17, + created_at=1700000000, + filename="batch_input.jsonl", + object="file", + purpose="batch", + status="uploaded", + ) + + +def test_unscoped_list_files_hands_post_call_hooks_a_page_object( + mocker: MockerFixture, monkeypatch, llm_router: Router +): + """Logging callbacks read ``response.data`` off a listing, so the managed + branch has to hand them the same page shape the provider branch does. A bare + mapping turns every registered callback into a 500 on this route.""" + import litellm.proxy.proxy_server as ps + + seen_by_callback: list[list[str]] = [] + + async def _reads_response_data(data, user_api_key_dict, response): + seen_by_callback.append([file.id for file in response.data]) + return None + + async def _one_managed_file(**kwargs): + return FileListPage( + data=[_managed_file("unified-file-id")], + first_id="unified-file-id", + last_id="unified-file-id", + ) + + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _one_managed_file) + ps.proxy_logging_obj.post_call_success_hook = _reads_response_data + + response = _get_unscoped_list_files("") + + assert response.status_code == 200, response.text + assert seen_by_callback == [["unified-file-id"]] + body = response.json() + assert list(body) == ["object", "data", "first_id", "last_id", "has_more"] + assert body["object"] == "list" + assert [file["id"] for file in body["data"]] == ["unified-file-id"] + assert body["has_more"] is False + + +@pytest.mark.parametrize("purpose", ["nonexistent_purpose", "EVALS", "batch "]) +def test_unscoped_list_files_returns_400_for_a_purpose_the_api_never_accepts( + mocker: MockerFixture, monkeypatch, llm_router: Router, purpose +): + """An unknown purpose matches nothing, so reporting an empty page would dress + a bad request up as a successful one. The provider-backed branches reject the + same values, and so does the upload route.""" + from urllib.parse import quote + + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _validating_afile_list) + + response = _get_list_files(f"/v1/files?purpose={quote(purpose)}") + + assert response.status_code == 400, response.text + assert response.json()["error"]["param"] == "purpose" + assert response.json()["error"]["type"] == "invalid_request_error" + assert response.json()["error"]["message"].startswith(f"Invalid purpose: {purpose}. Must be one of: ") + + +@pytest.mark.parametrize("purpose", ["batch", "assistants", "fine-tune"]) +def test_unscoped_list_files_accepts_every_documented_purpose( + mocker: MockerFixture, monkeypatch, llm_router: Router, purpose +): + managed_files = _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _validating_afile_list) + + response = _get_list_files(f"/v1/files?purpose={purpose}") + + assert response.status_code == 200, response.text + assert managed_files.afile_list.await_args.kwargs["purpose"] == purpose + + +def test_list_files_reports_a_bad_target_model_names_as_a_400( + mocker: MockerFixture, monkeypatch, llm_router: Router +): + """The exception tail reports an HTTPException with its own status and error + type rather than relabelling it, so a client that branches on either keeps + reading the same thing off a bad request.""" + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _permissive_afile_list) + + response = _get_list_files("/v1/files?target_model_names=gpt-3.5-turbo,gpt-4o") + + assert response.status_code == 400, response.text + assert response.json() == { + "error": { + "message": "target_model_names on list files must be a list of one model name. Example: ['gpt-4o']", + "type": "None", + "param": "None", + "code": "400", + } + } + + +def test_list_files_reports_an_unexpected_file_store_error_as_a_500( + mocker: MockerFixture, monkeypatch, llm_router: Router +): + async def _blows_up(**kwargs): + raise RuntimeError("managed file table is unreachable") + + _setup_unscoped_list_files_route(mocker, monkeypatch, llm_router, _blows_up) + + response = _get_unscoped_list_files("") + + assert response.status_code == 500, response.text + assert response.json()["error"]["message"] == "managed file table is unreachable" + + def test_list_files_restricted_team_does_not_leak_global_openai_credentials( mocker: MockerFixture, monkeypatch ): From e71a48c57e97f9f13f9499986a5fa75def18c1cb Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 22 Aug 2026 10:16:45 -0700 Subject: [PATCH 09/10] fix(files): accept OpenAI's evals purpose on the files routes OpenAIFilesPurpose was missing evals, which OpenAI documents. The upload route validates against that set, so POST /v1/files with purpose=evals was already being rejected, and the new listing validator extended the same rejection to GET /v1/files?purpose=evals, turning a purpose OpenAI accepts into a hard 400. Nothing branches exhaustively on the type, so widening it changes no routing. The managed-file listing test fake only understood a created_by filter. The OR filter a key carrying both a user_id and a team_id produces, the team_id filter a service-account key produces, and the empty filter a proxy admin produces all fell through it and returned every row, so the shapes most real keys send went uncovered. The fake now applies the filter it is handed, and the listing is tested against all three, including paging an OR filter across a cursor. Two docstrings claimed the continuation chunk bounds what a filtered page costs. It bounds queries per row scanned; the walk is still linear in the rows the caller owns. --- .../proxy/hooks/managed_files.py | 7 +- .../openai_files_endpoints/common_utils.py | 8 +- litellm/types/llms/openai.py | 1 + .../proxy/test_managed_files_hook.py | 175 +++++++++++++++++- 4 files changed, 177 insertions(+), 14 deletions(-) diff --git a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py index e71b520d27f..39f8de0b0cc 100644 --- a/enterprise/litellm_enterprise/proxy/hooks/managed_files.py +++ b/enterprise/litellm_enterprise/proxy/hooks/managed_files.py @@ -1395,8 +1395,11 @@ class _PROXY_LiteLLMManagedFiles(CustomLogger, BaseFileEndpoints): ``data`` non-empty while matches remain and its last id usable as the next cursor. A first chunk that fills the page costs one query; once a scan has to continue past it, the chunk widens to - ``FILE_LIST_CONTINUATION_CHUNK_SIZE`` so a page whose matches sit far - behind the newest rows cannot degenerate into thousands of queries. + ``FILE_LIST_CONTINUATION_CHUNK_SIZE``, so the walk costs one query per + that many rows instead of one per page. That bound is per query, not + per request: the work is still linear in the rows the caller owns, and + a filter matching nothing reads every one of them, with no index + covering either the owner filter or the sort. """ validate_file_list_limit(limit) validate_file_list_purpose(purpose) diff --git a/litellm/proxy/openai_files_endpoints/common_utils.py b/litellm/proxy/openai_files_endpoints/common_utils.py index c5213d842a3..d4cb8fe2374 100644 --- a/litellm/proxy/openai_files_endpoints/common_utils.py +++ b/litellm/proxy/openai_files_endpoints/common_utils.py @@ -48,12 +48,14 @@ def validate_file_list_limit(limit: int | None) -> None: def validate_file_list_purpose(purpose: str | None) -> None: - """Reject a ``purpose`` filter the Files API never accepts. + """Reject a ``purpose`` filter no upload to this proxy could have stored. An unknown purpose matches no file, so filtering on it would report an empty page for what is really a bad request. Rejecting it keeps a managed - listing consistent with the upload route and with the provider-backed - listings, which both refuse the same values. + listing consistent with the upload route, which refuses the same values + against this same set. The provider-backed listings do not: they pass + ``purpose`` upstream, so a purpose OpenAI accepts before it is added here + is rejected on the managed path while still working on those. """ valid_purposes: Final = get_args(OpenAIFilesPurpose) if purpose is None or purpose in valid_purposes: diff --git a/litellm/types/llms/openai.py b/litellm/types/llms/openai.py index 37de518b231..50e47071012 100644 --- a/litellm/types/llms/openai.py +++ b/litellm/types/llms/openai.py @@ -276,6 +276,7 @@ OpenAIFilesPurpose = Literal[ "fine-tune-results", "vision", "user_data", + "evals", "messages", ] 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 1cd6813b065..eddfc4fbd34 100644 --- a/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py +++ b/tests/test_litellm/enterprise/proxy/test_managed_files_hook.py @@ -14,7 +14,7 @@ import pytest from typing import Optional from unittest.mock import AsyncMock, MagicMock, patch -from litellm.proxy._types import ProxyException, UserAPIKeyAuth +from litellm.proxy._types import LitellmUserRoles, ProxyException, UserAPIKeyAuth from litellm.types.llms.openai import FileListPage, OpenAIFileObject from litellm.types.utils import LiteLLMBatch @@ -66,10 +66,38 @@ def _make_user_api_key_dict() -> UserAPIKeyAuth: ) +def _make_team_member_api_key_dict() -> UserAPIKeyAuth: + """The shape most real virtual keys carry: a user_id and a team_id.""" + return UserAPIKeyAuth( + api_key="sk-test", + user_id="test-user", + team_id="test-team", + parent_otel_span=None, + ) + + +def _make_service_account_api_key_dict() -> UserAPIKeyAuth: + return UserAPIKeyAuth( + api_key="sk-service", + team_id="test-team", + parent_otel_span=None, + ) + + +def _make_admin_api_key_dict() -> UserAPIKeyAuth: + return UserAPIKeyAuth( + api_key="sk-admin", + user_id="admin-user", + user_role=LitellmUserRoles.PROXY_ADMIN, + parent_otel_span=None, + ) + + def _make_managed_file_row( unified_file_id: str, purpose: str = "batch_output", created_by: str = "test-user", + team_id: Optional[str] = None, ) -> MagicMock: file_object = _make_file_object(f"file-provider-{unified_file_id}").model_copy( update={"purpose": purpose} @@ -78,15 +106,35 @@ def _make_managed_file_row( unified_file_id=unified_file_id, file_object=file_object.model_dump(), created_by=created_by, + team_id=team_id, ) def _make_unparseable_managed_file_row( unified_file_id: str, created_by: str = "test-user", + team_id: Optional[str] = None, ) -> MagicMock: """A row whose stored blob cannot be parsed back into a file object.""" - return MagicMock(unified_file_id=unified_file_id, file_object=None, created_by=created_by) + return MagicMock( + unified_file_id=unified_file_id, + file_object=None, + created_by=created_by, + team_id=team_id, + ) + + +def _row_matches_where(row, where) -> bool: + """Apply the Prisma ``where`` shapes build_owner_filter actually emits: + ``{}``, a single equality, and the ``OR`` of equalities a key carrying + both a user_id and a team_id produces.""" + for field, expected in where.items(): + if field == "OR": + if not any(_row_matches_where(row, clause) for clause in expected): + return False + elif getattr(row, field) != expected: + return False + return True class _FakeManagedFileTable: @@ -98,15 +146,11 @@ class _FakeManagedFileTable: self.find_first_calls = [] def _owned_rows(self, where): - created_by = where.get("created_by") - return [row for row in self.rows if created_by is None or row.created_by == created_by] + return [row for row in self.rows if _row_matches_where(row, where)] async def find_first(self, where): self.find_first_calls.append(where) - return next( - (row for row in self._owned_rows(where) if row.unified_file_id == where.get("unified_file_id")), - None, - ) + return next(iter(self._owned_rows(where)), None) async def find_many(self, where, take=None, order=None, cursor=None, skip=0): self.find_many_calls.append( @@ -336,7 +380,7 @@ async def test_afile_list_rejects_a_purpose_the_files_api_never_accepts(purpose) @pytest.mark.asyncio -@pytest.mark.parametrize("purpose", ["batch", "assistants", "fine-tune", None]) +@pytest.mark.parametrize("purpose", ["batch", "assistants", "fine-tune", "evals", None]) async def test_afile_list_accepts_every_documented_purpose(purpose): managed_files, _ = _make_managed_files_over_rows([_make_managed_file_row("unified-file-id")]) @@ -369,6 +413,119 @@ async def test_afile_list_does_not_leak_another_callers_files(): assert table.find_many_calls[0]["where"] == {"created_by": "test-user"} +@pytest.mark.asyncio +async def test_afile_list_returns_own_and_team_files_for_a_key_carrying_both_ids(): + managed_files, table = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-mine"), + _make_managed_file_row("unified-teammates", created_by="other-user", team_id="test-team"), + _make_managed_file_row("unified-outsiders", created_by="outsider", team_id="other-team"), + ] + ) + + response = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_team_member_api_key_dict(), + ) + + assert [file.id for file in response.data] == ["unified-mine", "unified-teammates"] + assert table.find_many_calls[0]["where"] == { + "OR": [{"created_by": "test-user"}, {"team_id": "test-team"}] + } + + +@pytest.mark.asyncio +async def test_afile_list_scopes_a_service_account_key_to_its_team(): + managed_files, table = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-teams", created_by="other-user", team_id="test-team"), + _make_managed_file_row("unified-outsiders", created_by="outsider", team_id="other-team"), + ] + ) + + response = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_service_account_api_key_dict(), + ) + + assert [file.id for file in response.data] == ["unified-teams"] + assert table.find_many_calls[0]["where"] == {"team_id": "test-team"} + + +@pytest.mark.asyncio +async def test_afile_list_returns_every_callers_files_for_a_proxy_admin(): + managed_files, table = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-mine"), + _make_managed_file_row("unified-theirs", created_by="other-user", team_id="other-team"), + ] + ) + + response = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_admin_api_key_dict(), + ) + + assert [file.id for file in response.data] == ["unified-mine", "unified-theirs"] + assert table.find_many_calls[0]["where"] == {} + + +@pytest.mark.asyncio +async def test_afile_list_pages_a_team_key_across_both_halves_of_its_filter(): + """Keyset pagination has to walk an OR filter as one ordered set, without + repeating a row across pages or dropping one between them.""" + managed_files, table = _make_managed_files_over_rows( + [ + _make_managed_file_row("unified-0"), + _make_managed_file_row("unified-1", created_by="other-user", team_id="test-team"), + _make_managed_file_row("unified-2"), + _make_managed_file_row("unified-3", created_by="outsider", team_id="other-team"), + _make_managed_file_row("unified-4", created_by="other-user", team_id="test-team"), + ] + ) + user_api_key_dict = _make_team_member_api_key_dict() + + seen = [] + cursor = None + for _ in range(4): + response = await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=user_api_key_dict, + limit=2, + after=cursor, + ) + seen.extend(file.id for file in response.data) + if not response.has_more: + break + cursor = response.last_id + + assert seen == ["unified-0", "unified-1", "unified-2", "unified-4"] + assert all( + call["where"] == {"OR": [{"created_by": "test-user"}, {"team_id": "test-team"}]} + for call in table.find_many_calls + ) + + +@pytest.mark.asyncio +async def test_afile_list_orders_newest_first_and_breaks_ties_on_the_cursor_column(): + managed_files, table = _make_managed_files_over_rows([_make_managed_file_row("unified-mine")]) + + await managed_files.afile_list( + purpose=None, + litellm_parent_otel_span=None, + user_api_key_dict=_make_user_api_key_dict(), + ) + + assert table.find_many_calls[0]["order"] == [ + {"created_at": "desc"}, + {"unified_file_id": "desc"}, + ] + + @pytest.mark.asyncio async def test_afile_list_denies_a_caller_without_a_user_or_team(): managed_files, table = _make_managed_files_over_rows([_make_managed_file_row("unified-mine")]) From 4f4edea055f81147793fcbbfbde371381e93983f Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Sat, 22 Aug 2026 10:27:32 -0700 Subject: [PATCH 10/10] test(files): update the managed-files test doubles to the current afile_list Eleven DummyManagedFiles stubs still declared afile_list(self, purpose, litellm_parent_otel_span). The real hook grew user_api_key_dict, limit and after, so the doubles no longer stand in for the interface they replace. Their tests pass today only because every one of them takes a provider branch that never reaches the hook, which means a stub going stale is invisible until some later test does reach it and reads a TypeError as a behavior change. Signatures only; no test changes behavior. --- .../test_files_endpoint.py | 110 ++++++++++++++++-- 1 file changed, 99 insertions(+), 11 deletions(-) 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 3df7a6643cd..c15ba5bcedb 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 @@ -329,7 +329,15 @@ def test_mock_create_audio_file(mocker: MockerFixture, monkeypatch, llm_router: async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -903,7 +911,15 @@ def test_create_file_with_expires_after( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -1066,7 +1082,15 @@ def test_create_file_with_expires_after_valid_values( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -1154,7 +1178,15 @@ def test_create_file_without_expires_after( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -1251,7 +1283,15 @@ def test_managed_files_with_loadbalancing( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -1368,7 +1408,15 @@ def test_create_file_with_nested_litellm_metadata( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -1472,7 +1520,15 @@ def test_create_file_with_deep_nested_litellm_metadata( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError("Not implemented for test") - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError("Not implemented for test") async def afile_delete( @@ -1568,7 +1624,15 @@ def _make_capturing_managed_files(): async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError async def afile_delete( @@ -2051,7 +2115,15 @@ def test_require_managed_files_allows_managed_file_upload( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError async def afile_delete( @@ -2175,7 +2247,15 @@ def test_require_managed_files_accepts_target_model_names_bracket_form( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError async def afile_delete( @@ -2255,7 +2335,15 @@ def test_require_managed_files_accepts_repeated_target_model_names_bracket_form( async def afile_retrieve(self, file_id, litellm_parent_otel_span, llm_router): raise NotImplementedError - async def afile_list(self, purpose, litellm_parent_otel_span): + async def afile_list( + self, + purpose, + litellm_parent_otel_span, + user_api_key_dict, + limit=None, + after=None, + **data, + ): raise NotImplementedError async def afile_delete(