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.
This commit is contained in:
mateo-berri 2026-08-22 10:16:45 -07:00
parent 7d61d9d71e
commit e71a48c57e
4 changed files with 177 additions and 14 deletions

View file

@ -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)

View file

@ -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:

View file

@ -276,6 +276,7 @@ OpenAIFilesPurpose = Literal[
"fine-tune-results",
"vision",
"user_data",
"evals",
"messages",
]

View file

@ -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")])