Team member permission /spend/logs for team-wide spend logs (UI + RBAC) by shivamrawat1 · Pull Request #25458 · BerriAI/litellm

shivamrawat1 · 2026-04-10T00:45:07Z

Relevant issues

Adds a dedicated team Member Permission for /spend/logs so internal users who are not team admins can view all spend logs for teams where that permission is granted. Team admins keep full team log access. Default (no permission) stays own logs only. Secures /spend/logs/ui/{request_id} for non-admins. UI shows the new permission in team settings.

Cause
RBAC gap — Only team admins could view team-scoped spend logs; members had no opt-in for team-wide logs.
Prisma vs helpers — members_with_roles from the DB is JSON (dicts); _is_user_team_admin / _team_member_has_permission expected Member objects → 'dict' object has no attribute 'user_id' when filtering by team_id.
Double filter — The UI sends user_id with “my logs” for internal users even when team_id is set; the handler applied user before auth, so queries became team + current user and hid other members’ logs.

Pre-Submission checklist

Please complete all items before asking a LiteLLM maintainer to review your PR

I have Added testing in the tests/test_litellm/ directory, Adding at least 1 test is a hard requirement - see details
My PR passes all unit tests on make test-unit
My PR's scope is as isolated as possible, it only solves 1 specific problem
I have requested a Greptile review by commenting @greptileai and received a Confidence Score of at least 4/5 before requesting a maintainer review

Delays 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)

CI status guideline:

50-55 passing tests: main is stable with minor issues.

45-49 passing tests: acceptable but needs attention

<= 40 passing tests: unstable; be careful with your merges and assess the risk.

Branch creation CI run
Link:
CI run for the last commit
Link:
Merge / cherry-pick CI run
Links:

Type

🆕 New Feature
✅ Test

Changes

KeyManagementRoutes.SPEND_LOGS (/spend/logs) in _types.py and team permission UI copy.
_can_team_member_view_log — Allow team admin or /spend/logs on the team; LiteLLM_TeamTable(**team_row.model_dump()) before admin/permission checks.
ui_view_spend_logs — For internal users without team_id, union own user + permitted teams via OR; on authorized team_id, where_conditions.pop("user", None) so team-wide rows aren’t narrowed by the UI’s user_id.
Detail endpoint — _assert_user_can_view_request_id for non-admins.
Tests updated for Prisma-shaped teams and new permission paths.

https://www.loom.com/share/af095d47b5bc4cc8ae88dd970c99e9c3

vercel · 2026-04-10T00:45:15Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
litellm	Ready	Preview, Comment	Apr 10, 2026 1:41am

shivamrawat1 · 2026-04-10T00:46:40Z

@greptile review

codspeed-hq · 2026-04-10T00:46:49Z

Merging this PR will not alter performance

✅ 16 untouched benchmarks

_{Comparing litellm_team_members_logs (b6357cd) with main (e7551a1)}

greptile-apps · 2026-04-10T00:54:30Z

Greptile Summary

This PR adds a SPEND_LOGS team-member permission (/spend/logs) that lets non-admin team members view team-wide spend logs when the permission is granted. It fixes three bugs identified in prior review rounds: the None == None ownership bypass in _assert_user_can_view_request_id, the silent RBAC skip when prisma_client is None in the detail endpoint, and the double-filter issue where the UI's user_id param narrowed team-scoped queries. The implementation looks structurally sound and well-tested; remaining concerns are P2 style/quality items.

Confidence Score: 5/5

Safe to merge; all three previously identified blocking bugs are fixed and the new RBAC logic is correct.

All P0/P1 concerns from prior review rounds (None==None bypass, prisma_client-None skip, double user filter) are addressed in the final commits. Remaining findings are P2 quality items that do not affect correctness or security.

No files require special attention for merge; spend_management_endpoints.py has a minor observability gap noted above.

Important Files Changed

Filename	Overview
litellm/proxy/spend_tracking/spend_management_endpoints.py	Core RBAC changes: adds `_can_team_member_view_log`, `_assert_user_can_view_request_id`, `_get_permitted_team_ids_for_spend_logs`, and wires them into `ui_view_spend_logs` and the detail endpoint; exception in team-ID fetch is swallowed without logging.
litellm/proxy/_types.py	Adds `SPEND_LOGS = "/spend/logs"` to `KeyManagementRoutes` and includes it in the allowed team-member permissions list; clean and correct.
tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py	Good unit and integration test coverage for new RBAC helpers; the `query_raw` mock reuses the Prisma count result instead of applying SQL conditions, so the raw SQL OR clause for permitted_team_ids is not independently verified.
ui/litellm-dashboard/src/components/team/permission_definitions.tsx	Adds `/spend/logs` description and GET method detection; straightforward UI-only change, no issues.
ui/litellm-dashboard/src/components/team/permission_definitions.test.tsx	Adds tests for `/spend/logs` permission description and GET method; complete and correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Request: GET /spend/logs/ui] --> B{is_admin_view?}
    B -- Yes --> G[Apply all query params as-is]
    B -- No --> C{team_id param provided?}

    C -- Yes --> D[_can_team_member_view_log\n DB: find team, check admin or permission]
    D -- Denied --> E[HTTP 403]
    D -- Allowed --> F[where: team_id=T, pop user filter]

    C -- No --> H{_can_user_view_spend_log?\nrole=INTERNAL_USER and user_id != None}
    H -- No --> G2[No user/team filter applied]
    H -- Yes --> I[_get_permitted_team_ids_for_spend_logs\n DB: get_user_object + find_many teams]
    I -- permitted_ids non-empty --> J[OR: user=me OR team_id IN permitted_ids]
    I -- empty --> K[where: user = my user_id only]

    F --> L[Prisma count + raw SQL query]
    G --> L
    J --> L
    K --> L
    G2 --> L
    L --> M[Return paginated response]

    subgraph Detail Endpoint
        N[GET /spend/logs/ui/request_id] --> O{is_admin_view?}
        O -- Yes --> P[Fetch payload]
        O -- No --> Q{prisma_client None?}
        Q -- Yes --> R[HTTP 403]
        Q -- No --> S[_assert_user_can_view_request_id]
        S -- Denied --> T[HTTP 403]
        S -- Allowed --> P
    end

_{Reviews (3): Last reviewed commit: "fix(proxy): reject non-admin spend log d..." | Re-trigger Greptile}

greptile-apps · 2026-04-10T00:54:33Z

+    if row.user == user_api_key_dict.user_id:
+        return


None == None bypasses the ownership check

When a service-account key (non-admin, user_id=None) calls /spend/logs/ui/{request_id} and the target log has user=None (system-level requests have no user field), the comparison None == None evaluates to True and returns immediately, granting full access to that log without any team-membership check. Service-account keys are valid non-admin callers of this endpoint.

Suggested change

if 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

Rule Used: What: Fail any PR which may contains a security in... (source)

greptile-apps · 2026-04-10T00:54:38Z

+    from litellm.proxy.auth.auth_checks import get_user_object
+    from litellm.proxy.proxy_server import proxy_logging_obj, user_api_key_cache


Imports inside function body

_get_permitted_team_ids_for_spend_logs imports get_user_object, proxy_logging_obj, and user_api_key_cache at call time. Per the project style guide, imports should live at module level; the only accepted exception is avoiding circular imports. If circular import avoidance is the reason here, add a brief comment explaining that — otherwise, move these to the top of the file alongside the other if TYPE_CHECKING / deferred imports.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

shivamrawat1 · 2026-04-10T01:21:04Z

@greptile review again

greptile-apps · 2026-04-10T01:28:20Z

+    if not _is_admin_view_safe(user_api_key_dict=user_api_key_dict):
+        if prisma_client is not None:
+            await _assert_user_can_view_request_id(
+                prisma_client=prisma_client,
+                user_api_key_dict=user_api_key_dict,
+                request_id=request_id,
+            )


Auth check silently skipped when prisma_client is None

When prisma_client is None (e.g. deployments that use only a custom logger such as S3 or GCS as the storage backend), the ownership/permission check is silently bypassed. The custom-logger loop that follows still executes and can return the full messages/response payload to any authenticated non-admin caller with no team-membership or user-ownership verification — defeating the RBAC introduced by this PR for those setups.

Consider raising HTTP 403 immediately when prisma_client is None and the caller is not an admin, or at minimum add a comment documenting that non-DB logging paths are intentionally unprotected so future reviewers understand the trade-off.

@greptile the change of the PR only applies to showing logs in UI. How is this relevant?

The endpoint returns the actual request and response payloads (messages, model output) for a given request_id — that's sensitive data regardless of whether it's labeled "UI."

When prisma_client is None (e.g., custom logger setups with S3/GCS), your new auth check is skipped:

if not _is_admin_view_safe(user_api_key_dict=user_api_key_dict): if prisma_client is not None: # ❌ skipped when None await _assert_user_can_view_request_id(...)

But the endpoint continues executing — it fetches data from custom_loggers and can still return the full payload to any authenticated non-admin user who knows/guesses a request_id. That bypasses the team-member RBAC you're adding in this PR.

@greptile how to fix it?

Add an else clause to raise 403 when prisma_client is None for non-admins:

Suggested change

if not _is_admin_view_safe(user_api_key_dict=user_api_key_dict):

if prisma_client is not None:

await _assert_user_can_view_request_id(

prisma_client=prisma_client,

user_api_key_dict=user_api_key_dict,

request_id=request_id,

)

from litellm.proxy.proxy_server import prisma_client

if not _is_admin_view_safe(user_api_key_dict=user_api_key_dict):

if prisma_client is not None:

await _assert_user_can_view_request_id(

prisma_client=prisma_client,

user_api_key_dict=user_api_key_dict,

request_id=request_id,

)

else:

raise HTTPException(

status_code=403,

detail="Request details unavailable - database client not configured",

)

…ip check - Drop module-level common_utils import; import team helpers inside callers. - Inline admin-view role check in _is_admin_view_safe to break import cycle. - Require non-null row.user before treating spend log as owned by the key (fixes None==None bypass for service keys). - Document deferred proxy_server imports in _get_permitted_team_ids_for_spend_logs. - Update tests (common_utils patches, regression test, ruff cleanups). Made-with: Cursor

shivamrawat1 · 2026-04-10T01:35:21Z

@greptile review again with the new commit

Non-admins previously skipped RBAC when prisma_client was None but could still read payloads from custom loggers. Return 403 unless admin view. Add test_ui_view_request_response_forbids_non_admin_without_db. Made-with: Cursor

krrish-berri-2 · 2026-04-11T16:32:15Z

@shivamrawat1 can you include a video showing how an admin grants this permission + proof before permission user can only see their spend vs. after where they can see all team spend

added option to allow team user to see logs of team

31f7501

github-advanced-security AI found potential problems Apr 10, 2026

View reviewed changes

Comment thread litellm/proxy/spend_tracking/spend_management_endpoints.py Fixed

greptile-apps bot reviewed Apr 10, 2026

View reviewed changes

resolved greptile comments

288ccb3

shivamrawat1 temporarily deployed to integration-postgres April 10, 2026 01:20 — with GitHub Actions Inactive

shivamrawat1 had a problem deploying to integration-postgres April 10, 2026 01:20 — with GitHub Actions Error

vercel bot deployed to Preview April 10, 2026 01:21 View deployment

github-advanced-security AI found potential problems Apr 10, 2026

View reviewed changes

Comment thread litellm/proxy/spend_tracking/spend_management_endpoints.py Fixed

greptile-apps bot reviewed Apr 10, 2026

View reviewed changes

shivamrawat1 temporarily deployed to integration-postgres April 10, 2026 01:35 — with GitHub Actions Inactive

shivamrawat1 had a problem deploying to integration-postgres April 10, 2026 01:35 — with GitHub Actions Error

shivamrawat1 temporarily deployed to integration-postgres April 10, 2026 01:35 — with GitHub Actions Inactive

vercel bot deployed to Preview April 10, 2026 01:36 View deployment

shivamrawat1 temporarily deployed to integration-postgres April 10, 2026 01:40 — with GitHub Actions Inactive

shivamrawat1 had a problem deploying to integration-postgres April 10, 2026 01:40 — with GitHub Actions Error

shivamrawat1 temporarily deployed to integration-postgres April 10, 2026 01:40 — with GitHub Actions Inactive

vercel bot deployed to Preview April 10, 2026 01:41 View deployment

yuneng-berri self-requested a review April 11, 2026 20:54

yuneng-berri approved these changes Apr 11, 2026

View reviewed changes

yuneng-berri merged commit 56e8245 into main Apr 11, 2026
104 of 107 checks passed

yuneng-berri deleted the litellm_team_members_logs branch April 11, 2026 20:55

		from litellm.proxy.auth.auth_checks import get_user_object
		from litellm.proxy.proxy_server import proxy_logging_obj, user_api_key_cache

Uh oh!

Conversation

shivamrawat1 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Relevant issues

Pre-Submission checklist

Delays in PR merge?

CI (LiteLLM team)

Type

Changes

Uh oh!

vercel bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shivamrawat1 commented Apr 10, 2026

Uh oh!

codspeed-hq bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Merging this PR will not alter performance

Uh oh!

Uh oh!

greptile-apps bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

greptile-apps bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 commented Apr 10, 2026

Uh oh!

Uh oh!

greptile-apps bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 Apr 10, 2026

Choose a reason for hiding this comment

Uh oh!

shivamrawat1 commented Apr 10, 2026

Uh oh!

krrish-berri-2 commented Apr 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shivamrawat1 commented Apr 10, 2026 •

edited

Loading

vercel bot commented Apr 10, 2026 •

edited

Loading

codspeed-hq bot commented Apr 10, 2026 •

edited

Loading

greptile-apps bot commented Apr 10, 2026 •

edited

Loading

shivamrawat1 Apr 10, 2026 •

edited

Loading