From 1f474d5bb3b835f76098b9d319dbb811575c0f24 Mon Sep 17 00:00:00 2001 From: shivam Date: Thu, 9 Apr 2026 18:34:23 -0700 Subject: [PATCH] =?UTF-8?q?fix(proxy):=20spend=20logs=20RBAC=E2=80=94avoid?= =?UTF-8?q?=20common=5Futils=20cycle,=20tighten=20ownership=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- .../spend_management_endpoints.py | 12 ++++--- .../test_spend_management_endpoints.py | 36 ++++++++----------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/litellm/proxy/spend_tracking/spend_management_endpoints.py b/litellm/proxy/spend_tracking/spend_management_endpoints.py index cd2ccda936e..16c20250c29 100644 --- a/litellm/proxy/spend_tracking/spend_management_endpoints.py +++ b/litellm/proxy/spend_tracking/spend_management_endpoints.py @@ -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 diff --git a/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py b/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py index 01171bc65ae..a64919438ad 100644 --- a/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py +++ b/tests/test_litellm/proxy/spend_tracking/test_spend_management_endpoints.py @@ -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