From 7a85bf532039f0c112f9160a2bfa32f50b0f26e8 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Thu, 2 Jul 2026 16:26:50 -0700 Subject: [PATCH] refactor(spend): model the request_id spend-log lookup as an explicit point lookup (LIT-3981) The date-window relaxation for request_id lookups rode an apply_date_window flag threaded through the date validation and parsing. Model the two intents directly instead. A UI request_id query is a point lookup on the @id primary key that drops the time window and authorizes by row ownership; every other query, including the public /spend/logs/v2 route, takes the range-scan path that still requires a window Because the ownership check fully authorizes the single row, the general user/team scoping is now skipped for id lookups rather than layered on top redundantly. The confusing `is_v2 or request_id is None` guard is gone, and moving the date requirement into the range-scan branch lets the type checker narrow the dates it parses Behavior is preserved: the v2 contract still requires dates even when a request_id is supplied, and a non-owner is still rejected with 403. A regression test covers the non-admin owner id lookup, which resolves across all time and filters by the primary key alone --- .../spend_management_endpoints.py | 76 ++++++++++--------- .../test_spend_management_endpoints.py | 72 ++++++++++++++++++ 2 files changed, 113 insertions(+), 35 deletions(-) diff --git a/litellm/proxy/spend_tracking/spend_management_endpoints.py b/litellm/proxy/spend_tracking/spend_management_endpoints.py index d2a3169e6a9..122674d9105 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -1754,20 +1754,6 @@ async def ui_view_spend_logs( is_v2 = "/spend/logs/v2" in get_request_route(request) - # request_id is the @id primary key, so it identifies a single row and needs - # no time window. The internal UI always sends a 24h window, which is why a - # log id copied from an older page could not be found; drop the window for an - # id lookup on the UI route so it resolves across all time (LIT-3981). - apply_date_window = is_v2 or request_id is None - - if apply_date_window and (start_date is None or end_date is None): - raise ProxyException( - message="Start date and end date are required", - type="bad_request", - param="None", - code=status.HTTP_400_BAD_REQUEST, - ) - # Validate sort_by and sort_order valid_sort_fields = { "spend", @@ -1794,23 +1780,42 @@ async def ui_view_spend_logs( ) try: - formats = ["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"] if is_v2 else ["%Y-%m-%d %H:%M:%S"] + is_admin_view = _is_admin_view_safe(user_api_key_dict=user_api_key_dict) + is_request_id_lookup = request_id is not None and not is_v2 - def parse_date(date_str: str) -> datetime: - date_str = date_str.strip() - for fmt in formats: - try: - return datetime.strptime(date_str, fmt).replace(tzinfo=timezone.utc) - except ValueError: - continue - expected = "'YYYY-MM-DD' or 'YYYY-MM-DD HH:MM:SS'" if is_v2 else "'YYYY-MM-DD HH:MM:SS'" - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Invalid date format: {date_str}. Expected: {expected}", - ) + if is_request_id_lookup: + # request_id is the @id primary key: it identifies a single row, so a + # time window is meaningless. The dashboard always sends a default 24h + # window, which hid ids copied from an older page (LIT-3981). Drop the + # window for the id lookup so it resolves across all time; every other + # query, including the public v2 route, still requires one (below). + start_date_obj: datetime | None = None + end_date_obj: datetime | None = None + else: + if start_date is None or end_date is None: + raise ProxyException( + message="Start date and end date are required", + type="bad_request", + param="None", + code=status.HTTP_400_BAD_REQUEST, + ) + formats = ["%Y-%m-%d %H:%M:%S", "%Y-%m-%d"] if is_v2 else ["%Y-%m-%d %H:%M:%S"] - start_date_obj = parse_date(start_date) if apply_date_window else None - end_date_obj = parse_date(end_date) if apply_date_window else None + def parse_date(date_str: str) -> datetime: + date_str = date_str.strip() + for fmt in formats: + try: + return datetime.strptime(date_str, fmt).replace(tzinfo=timezone.utc) + except ValueError: + continue + expected = "'YYYY-MM-DD' or 'YYYY-MM-DD HH:MM:SS'" if is_v2 else "'YYYY-MM-DD HH:MM:SS'" + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"Invalid date format: {date_str}. Expected: {expected}", + ) + + start_date_obj = parse_date(start_date) + end_date_obj = parse_date(end_date) # Build where conditions where_conditions: dict[str, Any] = {} @@ -1887,18 +1892,19 @@ async def ui_view_spend_logs( where_conditions["spend"]["gte"] = min_spend if max_spend is not None: where_conditions["spend"]["lte"] = max_spend - is_admin_view = _is_admin_view_safe(user_api_key_dict=user_api_key_dict) - # The UI request_id lookup drops the date window above, so a non-admin could - # otherwise reach any single row by id; require they own it, mirroring the - # detail endpoint. Scoped to the UI route so the public v2 contract is unchanged. - if request_id is not None and not is_admin_view and not is_v2: + # A request_id lookup drops the date window, so a non-admin could otherwise + # reach any single row by id; require they own it, mirroring the detail + # endpoint. That ownership check fully authorizes the one row, so the + # general scoping below is skipped for id lookups. Scoped to the UI route + # so the public v2 contract is unchanged. + if request_id is not None and not is_v2 and not is_admin_view: await _assert_user_can_view_request_id( prisma_client=prisma_client, user_api_key_dict=user_api_key_dict, request_id=request_id, ) permitted_team_ids: List[str] | None = None - if not is_admin_view: + if not is_request_id_lookup and not is_admin_view: if team_id is not None: can_view_team = await _can_team_member_view_log( prisma_client=prisma_client, 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 0ac09d27010..02b4a373441 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 @@ -1640,6 +1640,78 @@ async def test_ui_view_spend_logs_request_id_blocks_non_owner(client, monkeypatc app.dependency_overrides.pop(ps.user_api_key_auth, None) +@pytest.mark.asyncio +async def test_ui_view_spend_logs_request_id_owner_scoped_by_id_only( + client, monkeypatch +): + """A non-admin owner looking up their own request_id resolves across all time. + The ownership check authorizes the single row, so the query drops both the date + window and the general user/team scoping and filters by the primary key alone; + without that skip an internal user would have a `user`/`OR` clause added.""" + today = datetime.datetime.now(timezone.utc) + mock_spend_logs = [ + { + "id": "log_old", + "request_id": "req-old", + "api_key": "sk-test-key", + "user": "user_1", + "team_id": "team1", + "spend": 0.05, + "startTime": (today - datetime.timedelta(days=90)).isoformat(), + "model": "gpt-4", + }, + ] + + captured: dict = {} + + def filter_fn(where): + captured["where"] = where + rows = _filter_logs_by_date_range(mock_spend_logs, where) + if where.get("request_id"): + rows = [r for r in rows if r["request_id"] == where["request_id"]] + return rows + + mock_prisma = make_ui_spend_logs_mock_prisma(mock_spend_logs, filter_fn) + + class _OwnedRow: + user = "user_1" + team_id = "team1" + + async def _find_unique(where, include=None): + return _OwnedRow() + + mock_prisma.db.find_unique = _find_unique + 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. + start_date = (today - datetime.timedelta(days=5)).strftime("%Y-%m-%d %H:%M:%S") + end_date = today.strftime("%Y-%m-%d %H:%M:%S") + + app.dependency_overrides[ps.user_api_key_auth] = lambda: UserAPIKeyAuth( + user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_1" + ) + try: + response = client.get( + "/spend/logs/ui", + params={ + "request_id": "req-old", + "start_date": start_date, + "end_date": end_date, + }, + headers={"Authorization": "Bearer sk-test"}, + ) + assert response.status_code == 200 + data = response.json() + assert data["total"] == 1 + assert data["data"][0]["request_id"] == "req-old" + assert "startTime" not in captured["where"] + assert captured["where"]["request_id"] == "req-old" + assert "user" not in captured["where"] + assert "OR" not in captured["where"] + finally: + app.dependency_overrides.pop(ps.user_api_key_auth, None) + + @pytest.mark.asyncio async def test_ui_view_spend_logs_unauthorized(client): # Test without authorization header