[Fix] Allow non-admin compliance path reads - #27234
Conversation
|
Michael Riad Zaky seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
Greptile SummaryThis PR fixes a repeated 15-second error on the logs page by allowing
Confidence Score: 5/5Safe to merge — the access change is narrow, the endpoints are stateless validators on caller-supplied data, and the role boundary for view-only users is preserved. The compliance endpoints take a ComplianceCheckRequest payload and run purely in-process checks with no database access or cross-tenant data retrieval, so granting internal_user access does not widen the data surface. The existing route-allowlist pattern is followed exactly, and both the positive and negative access cases are covered by new tests. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/proxy/_types.py | Adds compliance_check_routes list and appends it to internal_user_routes; internal_user_view_only_routes is intentionally unchanged, preserving the access boundary for that role. |
| tests/test_litellm/proxy/auth/test_route_checks.py | Adds two parametrized tests: one confirming INTERNAL_USER can reach the compliance routes, one confirming INTERNAL_USER_VIEW_ONLY is still blocked with the expected exception message. |
Reviews (3): Last reviewed commit: "Restrict compliance routes to internal u..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
🤖 litellm-agent: Merged into staging branch Triage Summary Merge Confidence: 5/5 ✅ READY All checks green. Greptile 5/5, no blocking pattern findings, CircleCI passed. |
|
This PR is wrong. the pr changes the available routes for view only users. @Michael-RZ-Berri do you mean internal_users or internal users with view only role? those are 2 separate roles. internal_user_viewer is deprecated - https://docs.litellm.ai/docs/proxy/access_control#internal-user-viewer---read-only-access |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Route expansion applied to deprecated view-only role
- Removed compliance routes from the deprecated internal_user_viewer whitelist and updated route-check tests to assert only internal_user can access them.
Preview (9355564a16)
diff --git a/litellm/proxy/_types.py b/litellm/proxy/_types.py
--- a/litellm/proxy/_types.py
+++ b/litellm/proxy/_types.py
@@ -656,6 +656,13 @@
"/health/services",
] + info_routes
+ # Stateless validators on caller-supplied log data; source logs are
+ # already accessible via spend_tracking_routes, so no scope expansion.
+ compliance_check_routes = [
+ "/compliance/eu-ai-act",
+ "/compliance/gdpr",
+ ]
+
# Routes in `global_spend_tracking_routes` return proxy-wide spend across
# every team, customer, and api_key. They are intentionally NOT included
# here — non-admin roles must not see other tenants' spend. Admin roles go
@@ -675,6 +682,7 @@
]
+ spend_tracking_routes
+ key_management_routes
+ + compliance_check_routes
)
internal_user_view_only_routes = spend_tracking_routes
diff --git a/tests/test_litellm/proxy/auth/test_route_checks.py b/tests/test_litellm/proxy/auth/test_route_checks.py
--- a/tests/test_litellm/proxy/auth/test_route_checks.py
+++ b/tests/test_litellm/proxy/auth/test_route_checks.py
@@ -53,6 +53,60 @@
assert "Your role=internal_user" in str(exc_info.value)
+@pytest.mark.parametrize(
+ "route",
+ ["/compliance/eu-ai-act", "/compliance/gdpr"],
+)
+def test_compliance_routes_open_to_internal_user(route):
+ """Compliance routes are stateless validators on caller-supplied log data
+ - non-admin internal_user roles can call them."""
+ role = LitellmUserRoles.INTERNAL_USER.value
+ user_obj = LiteLLM_UserTable(
+ user_id="test_user",
+ user_email="test@example.com",
+ user_role=role,
+ )
+ valid_token = UserAPIKeyAuth(user_id="test_user", user_role=role)
+ request = MagicMock(spec=Request)
+ request.query_params = {}
+
+ RouteChecks.non_proxy_admin_allowed_routes_check(
+ user_obj=user_obj,
+ _user_role=role,
+ route=route,
+ request=request,
+ valid_token=valid_token,
+ request_data={},
+ )
+
+
+@pytest.mark.parametrize(
+ "route",
+ ["/compliance/eu-ai-act", "/compliance/gdpr"],
+)
+def test_compliance_routes_blocked_for_internal_user_view_only(route):
+ """Deprecated internal_user_viewer role must not gain compliance route access."""
+ role = LitellmUserRoles.INTERNAL_USER_VIEW_ONLY.value
+ user_obj = LiteLLM_UserTable(
+ user_id="test_user",
+ user_email="test@example.com",
+ user_role=role,
+ )
+ valid_token = UserAPIKeyAuth(user_id="test_user", user_role=role)
+ request = MagicMock(spec=Request)
+ request.query_params = {}
+
+ with pytest.raises(HTTPException):
+ RouteChecks.non_proxy_admin_allowed_routes_check(
+ user_obj=user_obj,
+ _user_role=role,
+ route=route,
+ request=request,
+ valid_token=valid_token,
+ request_data={},
+ )
+
+
def test_proxy_admin_viewer_config_update_route_rejected():
"""Test that proxy admin viewer users are rejected when trying to call /config/update"""You can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 2993e45. Configure here.
|
🤖 litellm-agent: Reverted from staging branch |
Oh yeah didn't know they were deprecated, should be for internal_users only then. |
9355564 to
6c0a4ad
Compare
6c0a4ad to
46f5554
Compare
db8198f
into
litellm_internal_staging
* allow non-admin roles on /compliance/* read routes * Restrict compliance routes to internal users --------- Co-authored-by: Michael Riad Zaky <michaelr@Mac.localdomain> Co-authored-by: Cursor Agent <cursoragent@cursor.com>
5 participants
Relevant issues
The logs page logs an error every 15 seconds that a non-admin user stays on the page and has a row-information side panel open. This is because compliance, for relevant requests, was gated to only be viewable to admins on the backend, even though compliance is included in the request information for every user when present. This PR allows internal users to see compliance so there aren't a flood of errors because a page was left open.
Linear ticket
Resolves LIT-2760.
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 reviewScreenshots / Proof of Fix
Type
🐛 Bug Fix
✅ Test
Changes
_types file
Note
Medium Risk
Touches RBAC route allowlists; mistakes could unintentionally broaden access, though the change is narrowly scoped to two stateless compliance endpoints and covered by tests.
Overview
Allows non-admin
internal_usercallers to access the compliance validation endpoints (/compliance/eu-ai-act,/compliance/gdpr) by adding them toLiteLLMRoutes.internal_user_routes.Adds route-check tests to ensure these endpoints are permitted for
INTERNAL_USERand still denied for the deprecatedINTERNAL_USER_VIEW_ONLYrole.Reviewed by Cursor Bugbot for commit 46f5554. Bugbot is set up for automated code reviews on this repo. Configure here.