feat: Allow Admin Viewers to Access Spend Logs - #26583
Conversation
Add /spend/logs, /spend/logs/ui, /spend/logs/v2, /spend/logs/session/ui,
/spend/logs/ui/{request_id} to admin_viewer_routes so proxy_admin_viewer
can view spend logs across the platform.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR grants the Confidence Score: 5/5Safe to merge — changes are additive route allowlist entries with no risk of privilege escalation or data mutation. The PR only appends read-only endpoint strings to two allowlists and adds mock-only unit tests. All three affected roles (PROXY_ADMIN_VIEW_ONLY, INTERNAL_USER, INTERNAL_USER_VIEW_ONLY) receive access to the correct subset of routes, consistent with the existing design. No network calls, DB queries, or security boundaries are affected. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_types.py | Adds /spend/logs/v2 and /spend/logs/ui/{request_id} to spend_tracking_routes (INTERNAL_USER parity) and all five spend log routes to admin_viewer_routes (PROXY_ADMIN_VIEW_ONLY fix); changes are minimal and surgical. |
| tests/test_litellm/proxy/auth/test_route_checks.py | Adds two parametrized unit tests (mock-only, no network calls) covering the new route access for PROXY_ADMIN_VIEW_ONLY and for INTERNAL_USER/INTERNAL_USER_VIEW_ONLY; follows the existing test pattern in this file. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[Request to /spend/logs/*] --> B{non_proxy_admin_allowed_routes_check}
B --> C{Role?}
C -->|PROXY_ADMIN_VIEW_ONLY| D[_check_proxy_admin_viewer_access]
D --> E{In management_routes?}
E -->|No| F{In admin_viewer_routes?}
F -->|YES after this PR| G[✅ Allow]
F -->|No| H{In global_spend_tracking_routes?}
H -->|No| I[❌ 403 Forbidden]
C -->|INTERNAL_USER| J{In internal_user_routes?}
J -->|YES - includes spend_tracking_routes| G
J -->|No| I
C -->|INTERNAL_USER_VIEW_ONLY| K{In internal_user_view_only_routes = spend_tracking_routes?}
K -->|YES - /spend/logs/v2 added| G
K -->|No| I
Reviews (3): Last reviewed commit: "Add /spend/logs/v2 and /spend/logs/ui/{r..." | Re-trigger Greptile
| "/audit/{id}", | ||
| "/spend/logs", | ||
| "/spend/logs/ui", | ||
| "/spend/logs/v2", |
There was a problem hiding this comment.
/spend/logs/v2 missing from spend_tracking_routes
/spend/logs/v2 is added to admin_viewer_routes but its three sibling routes (/spend/logs, /spend/logs/ui, /spend/logs/session/ui) are already present in spend_tracking_routes, making them accessible to INTERNAL_USER and INTERNAL_USER_VIEW_ONLY roles as well. The new public-API endpoint /spend/logs/v2 shares the same handler as /spend/logs/ui but will be inaccessible to those roles, creating an inconsistency where the internal UI path works but the public API path does not for the same users. Consider adding /spend/logs/v2 to spend_tracking_routes for parity.
…routes These routes share their handler with /spend/logs/ui (already in spend_tracking_routes), so they must be reachable by INTERNAL_USER and INTERNAL_USER_VIEW_ONLY for parity. Per-user data filtering is still enforced inside the endpoint by _can_user_view_spend_log.
|
Thank you for this PR. This appears to fix the exact issue we are currently hitting in production. In our setup, a team member with We have already confirmed that Since this PR adds We are deploying from a prebuilt Docker image, so we cannot easily patch this locally. If possible, we would greatly appreciate a review/merge and inclusion in a near-term stable image release. Thanks again. |
|
We’re hitting this issue as well. This PR looks like the right fix for Admin Viewer/read-only access to spend logs. Any chance this can be reviewed and included in an upcoming stable release? |
Relevant issues
Fixes #26582
Related: #17086 (closed by stale-bot without fix)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
CI (LiteLLM team)
Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:
Screenshots / Proof of Fix
AS-IS
403 Forbidden with
Admin ViewerroleTO-BE
User with
Admin Viewerrole can retrieve logs correctly.Type
🆕 New Feature
✅ Test
Changes
litellm/proxy/_types.py: 5 entries added toadmin_viewer_routestests/test_litellm/proxy/auth/test_route_checks.py: parametrized testtest_proxy_admin_viewer_can_access_spend_logs(5 cases)