mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-07 08:26:10 +00:00
fix(spend_logs): authorize every request_id lookup match, not just one
x-litellm-call-id is client-supplied and litellm_call_id is not unique, so a caller can seed their own row with a litellm_call_id that collides with another tenant's request_id. _assert_user_can_view_request_id used find_first, which authorized whichever single row came back, while the follow-up list and detail queries returned every matching row (or, via an unordered LIMIT 1, a different match). Read every row the OR clause resolves to and require the caller to own each one; any unowned match denies the request.
This commit is contained in:
parent
f1dea17be1
commit
1fbd2dbb69
2 changed files with 70 additions and 29 deletions
|
|
@ -243,9 +243,14 @@ def _request_id_or_call_id_clause(request_id: str) -> tuple[_RequestIdEquals, _L
|
|||
return (request_id_clause, call_id_clause)
|
||||
|
||||
|
||||
async def _find_spend_log_row(prisma_client: PrismaClient, request_id: str) -> _SpendLogOwnershipRow | None:
|
||||
"""Read the single spend log row identified by ``request_id`` or ``litellm_call_id``."""
|
||||
return await _spend_logs_table(prisma_client).find_first(
|
||||
async def _find_spend_log_rows(prisma_client: PrismaClient, request_id: str) -> Sequence[_SpendLogOwnershipRow]:
|
||||
"""Read every spend log row matching ``request_id`` or ``litellm_call_id``.
|
||||
|
||||
``litellm_call_id`` is client-supplied (``x-litellm-call-id``) and not unique,
|
||||
so a single id can address more than one row across tenants. Callers that
|
||||
need to authorize the id must inspect every matching row, not just one.
|
||||
"""
|
||||
return await _spend_logs_table(prisma_client).find_many(
|
||||
where={"OR": _request_id_or_call_id_clause(request_id)},
|
||||
include=None,
|
||||
)
|
||||
|
|
@ -4301,31 +4306,29 @@ async def _assert_user_can_view_request_id(
|
|||
request_id: str,
|
||||
) -> None:
|
||||
"""
|
||||
Verify the requesting non-admin user is allowed to view this spend-log row.
|
||||
Allowed when the log belongs to the user directly, or to one of their
|
||||
permitted teams (admin or ``/spend/logs`` permission).
|
||||
Verify the requesting non-admin user is allowed to view every spend log row
|
||||
the ``request_id`` lookup can resolve to. Allowed per row when the log
|
||||
belongs to the user directly, or to one of their permitted teams (admin or
|
||||
``/spend/logs`` permission). Because ``litellm_call_id`` is client-supplied
|
||||
and non-unique, one id can address rows across tenants, so authorization
|
||||
must hold for every matching row: any unowned match denies the request.
|
||||
Raises HTTP 403 if not.
|
||||
"""
|
||||
row: Final = await _find_spend_log_row(prisma_client, request_id)
|
||||
if row is None:
|
||||
return
|
||||
|
||||
if row.user is not None and row.user == user_api_key_dict.user_id:
|
||||
return
|
||||
|
||||
if row.team_id:
|
||||
can_view: Final = await _can_team_member_view_log(
|
||||
rows: Final = await _find_spend_log_rows(prisma_client, request_id)
|
||||
caller_user_id: Final = user_api_key_dict.user_id
|
||||
for row in rows:
|
||||
if caller_user_id is not None and row.user == caller_user_id:
|
||||
continue
|
||||
if row.team_id and await _can_team_member_view_log(
|
||||
prisma_client=prisma_client,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
team_id=row.team_id,
|
||||
):
|
||||
continue
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={"error": f"Not authorized to view spend log for request_id={request_id}"},
|
||||
)
|
||||
if can_view:
|
||||
return
|
||||
|
||||
raise HTTPException(
|
||||
status_code=status.HTTP_403_FORBIDDEN,
|
||||
detail={"error": f"Not authorized to view spend log for request_id={request_id}"},
|
||||
)
|
||||
|
||||
|
||||
async def _get_permitted_team_ids_for_spend_logs(
|
||||
|
|
|
|||
|
|
@ -413,8 +413,8 @@ async def test_assert_user_can_view_request_id_rejects_both_users_none():
|
|||
team_id = None
|
||||
|
||||
class MockSpendLogs:
|
||||
async def find_first(self, where=None, include=None):
|
||||
return MockRow()
|
||||
async def find_many(self, where=None, include=None):
|
||||
return [MockRow()]
|
||||
|
||||
class MockDB:
|
||||
def __init__(self):
|
||||
|
|
@ -432,6 +432,44 @@ async def test_assert_user_can_view_request_id_rejects_both_users_none():
|
|||
assert exc_info.value.status_code == 403
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_assert_user_can_view_request_id_denies_cross_tenant_call_id_collision():
|
||||
"""
|
||||
Regression: ``litellm_call_id`` is client-supplied (``x-litellm-call-id``) and
|
||||
not unique, so a caller can seed their own row with a ``litellm_call_id``
|
||||
that collides with another tenant's ``request_id``. The auth check must
|
||||
inspect every matching row rather than just the first one, otherwise it
|
||||
would pass on the caller's owned row and the follow-up list/detail query
|
||||
could return the unowned sibling.
|
||||
"""
|
||||
|
||||
class Row:
|
||||
def __init__(self, user, team_id=None):
|
||||
self.user = user
|
||||
self.team_id = team_id
|
||||
|
||||
class MockSpendLogs:
|
||||
async def find_many(self, where=None, include=None):
|
||||
return [Row("caller_user"), Row("victim_user")]
|
||||
|
||||
class MockDB:
|
||||
def __init__(self):
|
||||
self.litellm_spendlogs = MockSpendLogs()
|
||||
|
||||
class MockPrisma:
|
||||
def __init__(self):
|
||||
self.db = MockDB()
|
||||
|
||||
auth = UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER, user_id="caller_user"
|
||||
)
|
||||
with pytest.raises(HTTPException) as exc_info:
|
||||
await spend_management_endpoints._assert_user_can_view_request_id(
|
||||
MockPrisma(), auth, "colliding-id"
|
||||
)
|
||||
assert exc_info.value.status_code == 403
|
||||
|
||||
|
||||
def test_ui_view_request_response_forbids_non_admin_without_db(client, monkeypatch):
|
||||
"""
|
||||
Without prisma, non-admins cannot be authorized to read request/response
|
||||
|
|
@ -2334,8 +2372,8 @@ async def test_ui_view_spend_logs_request_id_blocks_non_owner(client, monkeypatc
|
|||
team_id = None
|
||||
|
||||
class _SpendLogs:
|
||||
async def find_first(self, where=None, include=None):
|
||||
return _ForeignRow()
|
||||
async def find_many(self, where=None, include=None):
|
||||
return [_ForeignRow()]
|
||||
|
||||
class _DB:
|
||||
def __init__(self):
|
||||
|
|
@ -2400,10 +2438,10 @@ async def test_ui_view_spend_logs_request_id_owner_scoped_by_id_only(
|
|||
user = "user_1"
|
||||
team_id = "team1"
|
||||
|
||||
async def _find_first(where=None, include=None):
|
||||
return _OwnedRow()
|
||||
async def _find_many(where=None, include=None):
|
||||
return [_OwnedRow()]
|
||||
|
||||
mock_prisma.db.find_first = _find_first
|
||||
mock_prisma.db.find_many = _find_many
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma)
|
||||
|
||||
# A 5-day window that EXCLUDES the 90-day-old log, as the dashboard sends.
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue