fix(proxy): spend logs RBAC—avoid common_utils cycle, tighten ownership 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
This commit is contained in:
shivam 2026-04-09 18:34:23 -07:00
parent 288ccb39c0
commit 1f474d5bb3
No known key found for this signature in database
2 changed files with 22 additions and 26 deletions

View file

@ -3439,12 +3439,16 @@ def _build_status_filter_condition(status_filter: Optional[str]) -> Dict[str, An
def _is_admin_view_safe(user_api_key_dict: UserAPIKeyAuth) -> bool:
"""
Safely determine if the current user has admin view permissions.
Wraps the underlying check and defaults to False on any exception.
Defaults to False on any exception.
"""
from litellm.proxy.management_endpoints.common_utils import _user_has_admin_view
try:
return _user_has_admin_view(user_api_key_dict=user_api_key_dict)
user_role = getattr(user_api_key_dict, "user_role", None)
if user_role is None:
return False
return user_role in (
LitellmUserRoles.PROXY_ADMIN,
LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY,
)
except Exception:
return False

View file

@ -112,38 +112,30 @@ from litellm.types.utils import BudgetConfig
@pytest.mark.asyncio
async def test_is_admin_view_safe_true(monkeypatch):
# Force underlying check to return True
monkeypatch.setattr(
common_utils,
"_user_has_admin_view",
lambda user_api_key_dict: True,
)
async def test_is_admin_view_safe_true():
auth = UserAPIKeyAuth(user_role=LitellmUserRoles.PROXY_ADMIN, user_id="admin_user")
assert spend_management_endpoints._is_admin_view_safe(auth) is True
@pytest.mark.asyncio
async def test_is_admin_view_safe_false(monkeypatch):
# Force underlying check to return False
monkeypatch.setattr(
common_utils,
"_user_has_admin_view",
lambda user_api_key_dict: False,
auth_view = UserAPIKeyAuth(
user_role=LitellmUserRoles.PROXY_ADMIN_VIEW_ONLY, user_id="admin_view"
)
assert spend_management_endpoints._is_admin_view_safe(auth_view) is True
@pytest.mark.asyncio
async def test_is_admin_view_safe_false():
auth = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_1")
assert spend_management_endpoints._is_admin_view_safe(auth) is False
@pytest.mark.asyncio
async def test_is_admin_view_safe_exception(monkeypatch):
async def test_is_admin_view_safe_exception():
# Ensure exceptions are swallowed and return False
def raise_err(*args, **kwargs):
raise RuntimeError("boom")
class ExplodingAuth:
@property
def user_role(self):
raise RuntimeError("boom")
monkeypatch.setattr(common_utils, "_user_has_admin_view", raise_err)
auth = UserAPIKeyAuth(user_role=LitellmUserRoles.INTERNAL_USER, user_id="user_1")
assert spend_management_endpoints._is_admin_view_safe(auth) is False
assert spend_management_endpoints._is_admin_view_safe(ExplodingAuth()) is False # type: ignore[arg-type]
@pytest.mark.asyncio