From 04599034d8716027911a4c44b35934931ea31bbb Mon Sep 17 00:00:00 2001 From: oopsoonk <462383+oopsmonk@users.noreply.github.com> Date: Tue, 19 May 2026 09:33:15 +0800 Subject: [PATCH] fix(spend-logs): deny detail view when row missing to block custom-logger leak MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_assert_user_can_view_request_id` previously returned silently when the `LiteLLM_SpendLogs` row was absent, after which the handler walked `get_request_response_payload` on every active custom logger (S3, GCS, langsmith, …) keyed by request_id — letting any non-admin read another user's messages/response when the DB row had expired or failed to write. Raise 403 on the missing-row branch so the lookup cannot fall through to external log storage without ownership being established first. --- .../spend_management_endpoints.py | 14 ++++++++- .../test_spend_management_endpoints.py | 29 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/litellm/proxy/spend_tracking/spend_management_endpoints.py b/litellm/proxy/spend_tracking/spend_management_endpoints.py index d030fabe8b5..cdc5af21ccb 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -3571,13 +3571,25 @@ async def _assert_user_can_view_request_id( Allowed when the log belongs to the user directly, or to one of their permitted teams (admin or ``/spend/logs`` permission). Raises HTTP 403 if not. + + If the row is missing (expired, never written, or unknown request_id) we + cannot establish ownership, so we must deny — the caller goes on to query + custom loggers (S3, GCS, …) by request_id, and silently allowing here + would let any non-admin read another user's request/response payload. """ row = await prisma_client.db.litellm_spendlogs.find_unique( where={"request_id": request_id}, include=None, ) if row is None: - return + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "error": "Not authorized to view spend log for request_id={}".format( + request_id + ) + }, + ) if row.user is not None and row.user == user_api_key_dict.user_id: return diff --git a/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py b/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py index 4bcabfe853a..c341e397bac 100644 --- a/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py +++ b/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py @@ -318,6 +318,35 @@ 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_when_row_missing(): + """ + If the spend-log row is absent (expired, never written, or unknown id) we + cannot establish ownership. Returning silently would let the handler fall + through to custom loggers (S3, GCS, langsmith, …) by request_id, leaking + another user's messages/response. Must deny instead. + """ + + class MockSpendLogs: + async def find_unique(self, where, include=None): + return None + + 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="user_1") + with pytest.raises(HTTPException) as exc_info: + await spend_management_endpoints._assert_user_can_view_request_id( + MockPrisma(), auth, "req-missing" + ) + 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