[Fix] RBAC: Restore Admin Viewer Read Parity for Logs + Settings Pages - #26846
Merged
yuneng-berri merged 8 commits intoMay 1, 2026
Merged
Conversation
…reads
Admin Viewer (proxy_admin_viewer) was being blocked from endpoints it should
be able to read. Most visibly the UI Logs page rendered empty because every
filter and detail call (/spend/logs/ui, /spend/logs/ui/{id},
/spend/logs/session/ui, /customer/list) was rejected at the route_checks
layer even though the underlying handlers permit admin-viewer.
Backend:
- Extend admin_viewer_routes to include spend_tracking_routes,
/customer/{list,info}, /spend/logs/* detail routes, callback / config /
budget / alerting reads, and model cost map status/source.
- Replace bare `user_role != PROXY_ADMIN` checks in read-only handlers
(/budget/list, /budget/settings, /alerting/settings, /invitation/info,
/config/field/info, /config/list, /schedule/model_cost_map_reload/status,
/model/cost_map/source) with `_user_has_admin_view()`.
UI:
- Add `rolesAllowedToViewWriteScopedPages` (rolesWithWriteAccess + Admin
Viewer) and use it for the "Models + Endpoints" and "Agents" sidebar
items so admin viewers see them read-only. Playground stays gated by
rolesWithWriteAccess (cost-incurring).
- Hide Add / Edit / Delete buttons in the LLM Credentials panel for
non-proxy-admin viewers.
Tests:
- 31 parametrized route_checks cases for the Logs + settings endpoints,
with internal-user negative coverage to ensure the gate isn't widened.
- 9 handler-level integration tests (FastAPI TestClient) verifying
admin viewer is no longer blocked at the handler layer.
- New leftnav cases asserting Playground hidden / Models + Agents / Logs
visible to Admin Viewer.
- New roles + credentials test cases for the UI write-gate.
Contributor
Greptile SummaryThis PR restores read parity for the Confidence Score: 5/5Safe to merge — all previously flagged P1 issues are addressed and the new auth logic is well-tested Both previously flagged P1 findings (management-route write bypass, dead /invitation/info handler change) are resolved in this PR. The new default-allow-GET / default-deny-POST structure is well-reasoned and comprehensively tested with 31 route-check cases and 9 handler-level integration tests. The only remaining finding is a P2 test quality note about missing request.method = 'GET' in two parametrized test groups. No security regressions identified. tests/test_litellm/proxy/auth/test_route_checks.py — two parametrized test groups don't set request.method, so they silently exercise the explicit-allowlist branch instead of the GET default-allow path
|
| Filename | Overview |
|---|---|
| litellm/proxy/auth/route_checks.py | Replaces blanket management-routes fallback with a safe-method default-allow + explicit POST denylist; resolves the previously flagged management-route write bypass |
| litellm/proxy/_types.py | Expands admin_viewer_routes with spend tracking, customer, settings, guardrail, MCP and cost-map routes; adds clear comments about the new default-allow model |
| litellm/proxy/proxy_server.py | 7 handler-level checks changed from user_role != PROXY_ADMIN to _user_has_admin_view(), covering alerting, invitation, config, model cost map, Anthropic beta headers, and adaptive router endpoints |
| tests/test_litellm/proxy/auth/test_route_checks.py | 343 lines of new parametrized tests covering logs-page routes, settings routes, default GET-allow contract, POST blocking, and management-route write blocking; two test groups don't set request.method so they silently test the allowlist path rather than the GET default-allow path |
| tests/test_litellm/proxy/auth/test_admin_viewer_handler_access.py | New integration test file: 9 handler-level tests via TestClient confirming Admin Viewer is no longer role-blocked at the handler layer |
| ui/litellm-dashboard/src/utils/roles.ts | Adds rolesAllowedToViewWriteScopedPages constant extending rolesWithWriteAccess with both Admin Viewer role name variants |
| ui/litellm-dashboard/src/components/model_add/credentials.tsx | Add/Edit/Delete credential buttons hidden for non-proxy-admin roles; credential list itself remains visible (read parity) |
| litellm/proxy/management_endpoints/budget_management_endpoints.py | /budget/list and /budget/settings handler checks updated to use _user_has_admin_view() |
| litellm/proxy/guardrails/guardrail_endpoints.py | Guardrail submissions listing now uses _user_has_admin_view() so Admin Viewer can see all submissions |
| litellm/proxy/management_endpoints/jwt_key_mapping_endpoints.py | Read endpoints (list_jwt_key_mappings, info_jwt_key_mapping) now grant Admin Viewer access; write endpoints unchanged |
Reviews (3): Last reviewed commit: "[Fix] RBAC: Drop management_routes Write..." | Re-trigger Greptile
…itellm_/pensive-bartik-e24048 # Conflicts: # ui/litellm-dashboard/src/components/leftnav.tsx
yuneng-berri
changed the title
Greptile review caught that the /invitation/info handler relaxation was dead code: the route_checks layer rejects admin viewers before the handler runs because /invitation/info was never added to admin_viewer_routes. Add /invitation/info to admin_viewer_routes and extend the route-level parametrized test to cover it. The handler-level integration test passed previously because `app.dependency_overrides[user_api_key_auth]` bypasses route_checks; this new route-level test exercises the layer that production traffic hits.
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
| # Invitations). | ||
| "/callbacks/list", | ||
| "/callbacks/configs", | ||
| "/get/config/callbacks", |
Contributor
There was a problem hiding this comment.
High: Secrets exposure to read-only admin role
The /get/config/callbacks handler (proxy_server.py ~line 13282) has no handler-level role check and returns decrypted values of LANGFUSE_SECRET_KEY, AWS_SECRET_ACCESS_KEY, SMTP_PASSWORD, SLACK_WEBHOOK_URL, and other integration secrets. By adding this route to admin_viewer_routes, any PROXY_ADMIN_VIEW_ONLY user can now retrieve these secrets.
The other two callback routes (/callbacks/list, /callbacks/configs) are safe — they return only callback names and static schema. Consider removing /get/config/callbacks from this list, or adding a _user_has_admin_view guard inside the handler that redacts secret values for the view-only role.
Contributor
|
1 review comment(s) still open on this PR. Status: 1 open |
… Models page hard-blocks
User reported six more 403s and "still restricts access to keys + models" after
the first round. Root causes:
1. Six read endpoints were missing from admin_viewer_routes:
- /guardrails/list, /v2/guardrails/list (Guardrails page)
- /guardrails/submissions, /guardrails/submissions/{guardrail_id}
- /guardrails/usage/overview (Guardrails Monitor page)
- /policies/attachments/list (Policies page)
- /get/mcp_semantic_filter_settings (Settings page)
2. /guardrails/submissions handler treated admin viewer as non-admin, filtering
them to only their team submissions. Switch to _user_has_admin_view() so
admin viewer sees all submissions (read parity with Proxy Admin).
3. UI Keys page (user_dashboard.tsx) and Models page (ModelsAndEndpointsView.tsx)
each had a hard "Access Denied" block specifically for "Admin Viewer" — a
leftover from the pre-parity era. Remove the blocks; gate the "Create Key"
button on the Keys page so admin viewer can read keys but not mint them.
Also drop the post-login redirect that forced admin viewers to /usage on
sign-in (page.tsx).
Tests:
- Extend ADMIN_VIEWER_SETTINGS_ROUTES parametrize list to cover all 7 new
routes (route-checks layer is now the layer production traffic actually
hits, vs. the dependency-override-bypass that was masking the gap).
Root cause: admin_viewer_routes was an explicit allowlist, so every newly-added
GET endpoint anywhere in the codebase silently 403'd for admin viewer until
someone remembered to add it. We had whacked /spend/logs/ui, /customer/list,
/guardrails/list, /policies/attachments/list, /invitation/info, and several
others in serial — but the next round still surfaced /in_product_nudges,
/health/latest, /credentials, /v1/mcp/network/client-ip, /claude-code/plugins,
/policy/templates. This pattern keeps repeating because the model is wrong.
Structural fix in `_check_proxy_admin_viewer_access`:
- Default-allow safe HTTP methods (GET / HEAD / OPTIONS) on any
non-inference route. Admin Viewer's principle is read parity with
Proxy Admin; HTTP semantics already mark GET as side-effect-free, so
using the method as the allow signal is the correct primitive.
- Unsafe methods (POST/PUT/PATCH/DELETE) still go through the existing
explicit allowlists + the hard-blocked write set
(/user/new, /team/new, /key/generate, …).
- LLM/inference routes still 403 (cost-incurring).
The existing admin_viewer_routes list is retained as a backstop for the
small set of routes implemented as POST but semantically read (e.g.
/spend/calculate). Adding new GET endpoints no longer requires touching
this list.
Models page tab/panel off-by-one (UI bug for Admin Viewer):
Tremor's TabList filters falsy children but TabPanels does not, so
conditionally hiding "Add Model" with `{!shouldHideAddModelTab && ...}`
left a phantom panel slot — clicking "LLM Credentials" showed nothing,
and clicking "Pass-Through Endpoints" showed the credentials panel.
Refactor to a single source-of-truth `visibleTabs` array; tab and
panel indices now can never desync.
Tests:
- 12 parametrized tests covering the 6 user-reported endpoints + 4
hypothetical-future endpoints + 2 already-fixed ones, all asserting
Admin Viewer GET succeeds via the default-allow path (no allowlist
entry needed).
- 5 parametrized tests for POST writes still 403'ing
(random-future-write, /user/new, /team/new, /key/generate, /model/new).
- All 207 existing route_checks tests still pass — backward-compatible.
…r Admin Viewer
The default-allow-GET fix in route_checks unblocked the route layer, but a
second class of bug remained: handlers that gate on `user_role !=
PROXY_ADMIN` (or a private `_require_proxy_admin` helper) reject admin
viewer at the handler before the route's HTTP method even matters.
Backend: relax handler role checks on read endpoints to allow
PROXY_ADMIN_VIEW_ONLY (same `_user_has_admin_view` helper used elsewhere).
- /v1/access_group GET (list) + /v1/access_group/{id} GET — split
`_require_proxy_admin` into a parallel `_require_admin_view` for the
two read handlers; writes (POST / PUT / DELETE) keep the strict gate.
- /cloudzero/settings GET, /vantage/settings GET — read-only views.
- /config_overrides/hashicorp_vault GET — read-only config view.
- /team/permissions_list GET — let admin viewer see permissions like
a Proxy Admin would.
- /jwt/key/mapping/list, /jwt/key/mapping/info — JWT mapping reads.
- /v1/mcp/discover, /v1/mcp/openapi-registry — MCP picker views.
- /schedule/anthropic_beta_headers_reload/status — read-only status.
- /adaptive_router/state — read-only live snapshot.
UI: hide write buttons that admin viewer should not see (button click
would fail the backend write gate, but the UX expectation is no button).
- Internal Users: hide "Invite User" button.
- Access Groups: hide "Create Access Group" + Delete row action.
- Budgets: hide "+ Create Budget" + Edit/Delete row actions.
- Prompts: hide "+ Add New Prompt" / "Upload .prompt File"; gate the
prompt-table Edit/Delete actions on `isProxyAdminRole` (was
`isAdminRole` which incorrectly included admin viewer).
- Router Settings → Fallbacks: hide AddFallbacks panel + per-row Test
+ Delete actions.
- AI Hub: hide "Select Models / Agents / MCP Servers / Skills to Make
Public" + "Useful Links Management" (writes).
These pages remain VISIBLE for admin viewer (read parity); only the
write entry points are hidden.
…itellm_/pensive-bartik-e24048 # Conflicts: # ui/litellm-dashboard/src/app/(dashboard)/models-and-endpoints/ModelsAndEndpointsView.tsx
Contributor
Author
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Greptile P1: the unsafe-method branch of `_check_proxy_admin_viewer_access`
ended with a blanket `if route in management_routes: return`. That set is a
mix of reads (info/list — handled via the safe-method GET branch above) and
writes. The fallback let Admin Viewer POST to write endpoints not enumerated
in `_ADMIN_VIEWER_BLOCKED_WRITE_ROUTES`, including:
- /team/block, /team/unblock, /team/permissions_update
- /jwt/key/mapping/{new,update,delete}
- /key/bulk_update
- /key/{key_id}/reset_spend
Remove the fallback. The two remaining allow sets (admin_viewer_routes and
global_spend_tracking_routes) are both read-only, so removal does not affect
the legitimate POST-as-read cases (e.g. /spend/calculate, which is in
spend_tracking_routes ⊂ admin_viewer_routes).
Tests:
- 8 new parametrized cases pinning each previously-leaking management write
endpoint to 403 on POST for PROXY_ADMIN_VIEW_ONLY.
Contributor
Author
ryan-crabbe-berri
approved these changes
May 1, 2026
7 tasks
fzowl
pushed a commit
to fzowl/litellm
that referenced
this pull request
Jun 24, 2026
…-e24048 [Fix] RBAC: Restore Admin Viewer Read Parity for Logs + Settings Pages
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The Admin Viewer (
proxy_admin_viewer) role was broken: the UI Logs page rendered empty for admin viewers and several admin-only-visible settings pages 403'd, because endpoints they should read were blocked at the route_checks layer (or at the handler level via bareuser_role != PROXY_ADMINchecks). This PR restores read parity with Proxy Admin per the role principle (read parity, no writes, no cost-incurring actions).Changes
Backend route gate (
litellm/proxy/_types.py)admin_viewer_routesnow includesspend_tracking_routes(covers/spend/logs/ui, the main Logs page list endpoint), plus/customer/{list,info},/spend/logs/ui/{logId},/spend/logs/session/ui, callback/config/budget/alerting reads, and model cost map status/source.Backend handler gates
budget_management_endpoints.py:/budget/list,/budget/settingsnow use_user_has_admin_view().proxy_server.py:/alerting/settings,/invitation/info,/config/field/info,/config/list,/schedule/model_cost_map_reload/status,/model/cost_map/sourcenow use_user_has_admin_view().UI sidebar
rolesAllowedToViewWriteScopedPagesconstant inroles.ts(=rolesWithWriteAccess+ Admin Viewer) — used for "Models + Endpoints" and "Agents" so admin viewers see them read-only. Playground stays gated byrolesWithWriteAccess(cost-incurring action).UI write gating (
credentials.tsx)!isProxyAdminRole(userRole). The credential list itself remains visible.Tests
route_checkscases for the Logs + settings endpoints, with internal-user negative coverage to confirm the gate isn't widened beyond admin viewer.TestClient) verifying admin viewer is no longer blocked at the handler layer.roles.test.ts+credentials.test.tsxcases.Out of scope (deferred follow-up)
Deeper write-action audit on Models page tabs (Model Retry Settings, Model Group Alias, Price Data Reload) and the Agents page. These tabs are reachable under
all_admin_rolesbut their internaluserRole === "Admin"check correctly excludes Admin Viewer for actual delete/save calls — UX could be improved by hiding the buttons rather than letting clicks 403.Test plan
uv run pytest tests/test_litellm/proxy/auth/— 556 passeduv run pytest tests/test_litellm/proxy/auth/ tests/test_litellm/proxy/management_endpoints/test_{budget,callback_management,customer,internal_user}_endpoints.py tests/test_litellm/proxy/spend_tracking/— 752 passeduv run black+uv run ruff check+uv run mypy— clean on changed filesproxy_admin_viewer, open Logs page, confirm logs render and filters work; open Models + Endpoints; confirm Playground stays hidden