mirror of
https://github.com/BerriAI/litellm.git
synced 2026-09-11 22:51:28 +00:00
fix(proxy): let internal users read request/response for their own spend logs
The Logs drawer gets messages/response from GET /spend/logs/ui/{request_id};
the list endpoint omits those heavy columns for every caller, admins included.
That detail route was missing from LiteLLMRoutes.spend_tracking_routes, and
check_route_access anchors patterns, so /spend/logs/ui never matched it. Every
internal_user got a 403 before the handler ran and the UI fell back to the
"Request/Response Data Not Available" banner, even on their own requests
Adds the route to spend_tracking_routes so internal_user, internal_user_view_only,
admin_viewer and org_admin all inherit it, and drops the now-redundant explicit
entry from admin_viewer_routes. The handler already authorizes non-admins per row
via _assert_user_can_view_request_id, so no handler-side scoping change is needed
That helper returned silently when no spend-log row existed, which the detail
handler treats as authorized before asking every custom logger for the payload by
raw request_id. With retention pruning the row can be gone while the payload is
still in cold storage, so opening the route would have let a non-admin read
another tenant's prompt out of S3/GCS. A missing row now falls through to the
same 403 as a foreign row, which also removes the exists-but-not-yours oracle
Fixes #34099
This commit is contained in:
parent
6b264815ac
commit
6632e8b74f
4 changed files with 273 additions and 10 deletions
|
|
@ -703,6 +703,7 @@ class LiteLLMRoutes(enum.Enum):
|
|||
"/spend/logs",
|
||||
"/spend/logs/v2",
|
||||
"/spend/logs/ui",
|
||||
"/spend/logs/ui/{request_id}",
|
||||
"/spend/logs/session/ui",
|
||||
"/key/spend/report",
|
||||
"/user/spend/report",
|
||||
|
|
@ -932,10 +933,10 @@ class LiteLLMRoutes(enum.Enum):
|
|||
# PROXY_ADMIN_VIEW_ONLY — the route gate must match).
|
||||
"/customer/list",
|
||||
"/customer/info",
|
||||
# UI Logs page detail drawer (single + session) and the filter facets.
|
||||
# The list endpoint `/spend/logs/ui` is covered via
|
||||
# spend_tracking_routes below.
|
||||
"/spend/logs/ui/{logId}",
|
||||
# UI Logs page session detail drawer and the end-user filter facet.
|
||||
# The list endpoint `/spend/logs/ui` and the single-log detail route
|
||||
# `/spend/logs/ui/{request_id}` are covered via spend_tracking_routes
|
||||
# below.
|
||||
"/spend/logs/session/ui",
|
||||
"/management/v1/spend_logs/end_users",
|
||||
"/management/v1/spend_logs/users",
|
||||
|
|
|
|||
|
|
@ -4633,16 +4633,16 @@ async def _assert_user_can_view_request_id(
|
|||
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).
|
||||
Raises HTTP 403 if not.
|
||||
Raises HTTP 403 if not, including when no spend-log row exists for the
|
||||
request_id (e.g. it was pruned by retention), so a missing row can't be
|
||||
used to read a payload out of cold storage via the detail endpoint.
|
||||
"""
|
||||
row: Final = await _find_spend_log_row(prisma_client, request_id)
|
||||
if row is None:
|
||||
|
||||
if row is not None and row.user is not None and row.user == user_api_key_dict.user_id:
|
||||
return
|
||||
|
||||
if row.user is not None and row.user == user_api_key_dict.user_id:
|
||||
return
|
||||
|
||||
if row.team_id:
|
||||
if row is not None and row.team_id:
|
||||
can_view: Final = await _can_team_member_view_log(
|
||||
prisma_client=prisma_client,
|
||||
user_api_key_dict=user_api_key_dict,
|
||||
|
|
|
|||
|
|
@ -2033,6 +2033,82 @@ def test_proxy_admin_viewer_can_access_logs_page_endpoints(route):
|
|||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"user_role",
|
||||
[LitellmUserRoles.INTERNAL_USER, LitellmUserRoles.INTERNAL_USER_VIEW_ONLY],
|
||||
)
|
||||
def test_internal_user_can_access_logs_drawer_detail_route(user_role):
|
||||
"""
|
||||
The Logs drawer detail fetch (GET /spend/logs/ui/{request_id}) must pass
|
||||
route_checks for plain internal users, not just admins — the handler
|
||||
itself already self-authorizes row ownership via
|
||||
_assert_user_can_view_request_id.
|
||||
"""
|
||||
route = "/spend/logs/ui/abc-request-id"
|
||||
user_obj = LiteLLM_UserTable(
|
||||
user_id="internal_user",
|
||||
user_email="user@example.com",
|
||||
user_role=user_role.value,
|
||||
)
|
||||
valid_token = UserAPIKeyAuth(
|
||||
user_id="internal_user",
|
||||
user_role=user_role.value,
|
||||
)
|
||||
request = MagicMock(spec=Request)
|
||||
request.query_params = {}
|
||||
|
||||
try:
|
||||
RouteChecks.non_proxy_admin_allowed_routes_check(
|
||||
user_obj=user_obj,
|
||||
_user_role=user_role.value,
|
||||
route=route,
|
||||
request=request,
|
||||
valid_token=valid_token,
|
||||
request_data={},
|
||||
)
|
||||
except Exception as e:
|
||||
pytest.fail(f"{user_role.value} should be able to access {route}. Got error: {str(e)}")
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"route_group_name",
|
||||
[
|
||||
"spend_tracking_routes",
|
||||
"internal_user_routes",
|
||||
"internal_user_view_only_routes",
|
||||
"admin_viewer_routes",
|
||||
"org_admin_allowed_routes",
|
||||
],
|
||||
)
|
||||
def test_logs_drawer_detail_route_in_every_route_group(route_group_name):
|
||||
"""
|
||||
/spend/logs/ui/{request_id} must be reachable through
|
||||
RouteChecks.check_route_access under each role's own route group, so a
|
||||
partial revert (removing the route from `spend_tracking_routes` while
|
||||
leaving `non_proxy_admin_allowed_routes_check` alone) is also caught.
|
||||
"""
|
||||
from litellm.proxy._types import LiteLLMRoutes
|
||||
|
||||
allowed_routes = getattr(LiteLLMRoutes, route_group_name).value
|
||||
assert RouteChecks.check_route_access(
|
||||
route="/spend/logs/ui/req-34099", allowed_routes=allowed_routes
|
||||
)
|
||||
|
||||
|
||||
def test_logs_drawer_detail_route_allowed_for_scoped_virtual_key():
|
||||
"""
|
||||
A virtual key scoped to `allowed_routes=["spend_tracking_routes"]` must be
|
||||
able to reach the Logs drawer detail route.
|
||||
"""
|
||||
valid_token = UserAPIKeyAuth(
|
||||
user_id="scoped_key_user",
|
||||
allowed_routes=["spend_tracking_routes"],
|
||||
)
|
||||
assert RouteChecks.is_virtual_key_allowed_to_call_route(
|
||||
route="/spend/logs/ui/req-34099", valid_token=valid_token
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("route", ADMIN_VIEWER_LOGS_PAGE_ROUTES)
|
||||
def test_internal_user_blocked_from_admin_viewer_logs_routes(route):
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -455,6 +455,34 @@ 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_rejects_missing_row():
|
||||
"""
|
||||
A request_id with no spend-log row (e.g. pruned by retention) must not
|
||||
authorize reading the payload from cold storage; a missing row is not
|
||||
the same as an owned row.
|
||||
"""
|
||||
|
||||
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-row"
|
||||
)
|
||||
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
|
||||
|
|
@ -6777,3 +6805,161 @@ async def test_ui_view_spend_logs_search_returns_flat_rows_when_grouping_by_sess
|
|||
assert "next_session_cursor" not in data
|
||||
finally:
|
||||
app.dependency_overrides.pop(ps.user_api_key_auth, None)
|
||||
|
||||
|
||||
def _fake_prisma_with_owned_spend_log(owner_user_id, messages_json, response_json):
|
||||
class _Row:
|
||||
user = owner_user_id
|
||||
team_id = None
|
||||
|
||||
class _SpendLogs:
|
||||
async def find_unique(self, where, include=None):
|
||||
return _Row()
|
||||
|
||||
class _DB:
|
||||
def __init__(self):
|
||||
self.litellm_spendlogs = _SpendLogs()
|
||||
|
||||
async def query_raw(self, _sql, *_args):
|
||||
return [
|
||||
{
|
||||
"messages": messages_json,
|
||||
"response": response_json,
|
||||
"proxy_server_request": "{}",
|
||||
"metadata": "{}",
|
||||
}
|
||||
]
|
||||
|
||||
class _Prisma:
|
||||
def __init__(self):
|
||||
self.db = _DB()
|
||||
|
||||
return _Prisma()
|
||||
|
||||
|
||||
def test_ui_view_request_response_internal_user_owner_gets_payload(client, monkeypatch):
|
||||
"""
|
||||
An internal_user who owns the spend-log row can fetch the Logs drawer
|
||||
detail payload for their own request (regression for #34099, where the
|
||||
route was blocked for INTERNAL_USER before reaching this ownership check).
|
||||
"""
|
||||
messages_json = json.dumps([{"role": "user", "content": "hi"}])
|
||||
response_json = json.dumps({"choices": [{"message": {"content": "hello"}}]})
|
||||
monkeypatch.setattr(
|
||||
"litellm.proxy.proxy_server.prisma_client",
|
||||
_fake_prisma_with_owned_spend_log("user_a", messages_json, response_json),
|
||||
)
|
||||
app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_a"
|
||||
)
|
||||
try:
|
||||
response = client.get(
|
||||
"/spend/logs/ui/req-owned-by-user-a",
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
body = response.json()
|
||||
assert json.loads(body["messages"]) == [{"role": "user", "content": "hi"}]
|
||||
assert json.loads(body["response"]) == {
|
||||
"choices": [{"message": {"content": "hello"}}]
|
||||
}
|
||||
finally:
|
||||
app.dependency_overrides.pop(ps.user_api_key_auth, None)
|
||||
|
||||
|
||||
class _RecordingAdditionalLoggingUtils:
|
||||
"""Injectable custom logger that records every request_id it's asked for."""
|
||||
|
||||
def __init__(self, payload):
|
||||
self._payload = payload
|
||||
self.requested_ids = []
|
||||
|
||||
async def get_request_response_payload(self, request_id, start_time_utc, end_time_utc):
|
||||
self.requested_ids.append(request_id)
|
||||
return self._payload
|
||||
|
||||
|
||||
def test_ui_view_request_response_internal_user_non_owner_forbidden(client, monkeypatch):
|
||||
"""
|
||||
A different internal_user requesting someone else's row is forbidden;
|
||||
guards against _assert_user_can_view_request_id being skipped in the
|
||||
detail-drawer handler. Also proves the handler stops before it ever asks
|
||||
a custom logger or the DB for the payload.
|
||||
"""
|
||||
messages_json = json.dumps([{"role": "user", "content": "hi"}])
|
||||
response_json = json.dumps({"choices": [{"message": {"content": "hello"}}]})
|
||||
fake_prisma = _fake_prisma_with_owned_spend_log("user_a", messages_json, response_json)
|
||||
original_query_raw = fake_prisma.db.query_raw
|
||||
query_raw_calls = []
|
||||
|
||||
async def _spy_query_raw(*args, **kwargs):
|
||||
query_raw_calls.append((args, kwargs))
|
||||
return await original_query_raw(*args, **kwargs)
|
||||
|
||||
fake_prisma.db.query_raw = _spy_query_raw
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", fake_prisma)
|
||||
|
||||
custom_logger = _RecordingAdditionalLoggingUtils({"messages": "should-not-be-returned"})
|
||||
monkeypatch.setattr(
|
||||
litellm.logging_callback_manager,
|
||||
"get_active_additional_logging_utils_from_custom_logger",
|
||||
lambda: [custom_logger],
|
||||
)
|
||||
|
||||
app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_b"
|
||||
)
|
||||
try:
|
||||
response = client.get(
|
||||
"/spend/logs/ui/req-owned-by-user-a",
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
assert custom_logger.requested_ids == []
|
||||
assert query_raw_calls == []
|
||||
finally:
|
||||
app.dependency_overrides.pop(ps.user_api_key_auth, None)
|
||||
|
||||
|
||||
def test_ui_view_request_response_internal_user_missing_row_forbidden(client, monkeypatch):
|
||||
"""
|
||||
Regression for the fail-open in _assert_user_can_view_request_id: a
|
||||
request_id with no spend-log row (e.g. pruned by retention) must be
|
||||
denied before the handler ever consults a custom logger, otherwise a
|
||||
non-admin who guesses/obtains a request_id could read another tenant's
|
||||
payload out of cold storage. Fails if `if row is None: return` is
|
||||
reintroduced.
|
||||
"""
|
||||
|
||||
class _SpendLogs:
|
||||
async def find_unique(self, where, include=None):
|
||||
return None
|
||||
|
||||
class _DB:
|
||||
def __init__(self):
|
||||
self.litellm_spendlogs = _SpendLogs()
|
||||
|
||||
from types import SimpleNamespace
|
||||
|
||||
fake_prisma = SimpleNamespace(db=_DB())
|
||||
monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", fake_prisma)
|
||||
|
||||
custom_logger = _RecordingAdditionalLoggingUtils({"messages": "should-not-be-returned"})
|
||||
monkeypatch.setattr(
|
||||
litellm.logging_callback_manager,
|
||||
"get_active_additional_logging_utils_from_custom_logger",
|
||||
lambda: [custom_logger],
|
||||
)
|
||||
|
||||
app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth(
|
||||
user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_a"
|
||||
)
|
||||
try:
|
||||
response = client.get(
|
||||
"/spend/logs/ui/req-pruned",
|
||||
headers={"Authorization": "Bearer sk-test"},
|
||||
)
|
||||
assert response.status_code == 403
|
||||
assert custom_logger.requested_ids == []
|
||||
finally:
|
||||
app.dependency_overrides.pop(ps.user_api_key_auth, None)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue