diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index b0ea6b41ac..8ad44b5300 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -1,5 +1,7 @@ from typing import TYPE_CHECKING, Any, Dict, List, Optional, Union +from fastapi import HTTPException, status + from litellm._logging import verbose_proxy_logger from litellm.caching import DualCache from litellm.proxy._types import ( @@ -29,6 +31,34 @@ def _user_has_admin_view(user_api_key_dict: UserAPIKeyAuth) -> bool: ) +def require_caller_user_id_for_non_admin( + user_api_key_dict: UserAPIKeyAuth, +) -> str: + """Return the caller's user_id, or raise 403 if missing. + + Non-admin analytics endpoints scope queries by the caller's own user_id. + Service-account keys are deliberately created with user_id=None + (key_management_endpoints.py forces ``data.user_id = None`` at key + creation). Without this guard, that None value flows through to the + daily-activity builder, which treats ``entity_id is None`` as "no filter" + and returns every tenant's data. + + Callers must check is_admin first; this helper is only valid on the + non-admin scoping branch. + """ + if user_api_key_dict.user_id is None: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail={ + "error": ( + "Service-account keys cannot query user analytics. " + "Use a user-bound key, or call as a proxy admin." + ) + }, + ) + return user_api_key_dict.user_id + + def _is_user_team_admin( user_api_key_dict: UserAPIKeyAuth, team_obj: LiteLLM_TeamTable ) -> bool: diff --git a/litellm/proxy/management_endpoints/internal_user_endpoints.py b/litellm/proxy/management_endpoints/internal_user_endpoints.py index 921d24da04..ce257a26b7 100644 --- a/litellm/proxy/management_endpoints/internal_user_endpoints.py +++ b/litellm/proxy/management_endpoints/internal_user_endpoints.py @@ -35,6 +35,7 @@ from litellm.proxy.management_endpoints.common_daily_activity import ( from litellm.proxy.management_endpoints.common_utils import ( _is_user_team_admin, _user_has_admin_view, + require_caller_user_id_for_non_admin, ) from litellm.proxy.management_endpoints.key_management_endpoints import ( generate_key_helper_fn, @@ -2587,9 +2588,10 @@ async def get_user_daily_activity( if is_admin: entity_id = user_id # None means global view, otherwise filter by user else: + caller_user_id = require_caller_user_id_for_non_admin(user_api_key_dict) if user_id is None: - user_id = user_api_key_dict.user_id - if user_id != user_api_key_dict.user_id: + user_id = caller_user_id + if user_id != caller_user_id: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail={ @@ -2684,9 +2686,10 @@ async def get_user_daily_activity_aggregated( if is_admin: entity_id = user_id # None means global view, otherwise filter by user else: + caller_user_id = require_caller_user_id_for_non_admin(user_api_key_dict) if user_id is None: - user_id = user_api_key_dict.user_id - if user_id != user_api_key_dict.user_id: + user_id = caller_user_id + if user_id != caller_user_id: raise HTTPException( status_code=status.HTTP_403_FORBIDDEN, detail={ diff --git a/litellm/proxy/management_endpoints/usage_endpoints/ai_usage_chat.py b/litellm/proxy/management_endpoints/usage_endpoints/ai_usage_chat.py index 4de29e0409..a50ce1d3c4 100644 --- a/litellm/proxy/management_endpoints/usage_endpoints/ai_usage_chat.py +++ b/litellm/proxy/management_endpoints/usage_endpoints/ai_usage_chat.py @@ -440,6 +440,15 @@ def _resolve_fetch_kwargs( kwargs: Dict[str, Any] = {"start_date": start_date, "end_date": end_date} if fn_name == "get_usage_data": if not is_admin: + if user_id is None: + # Defense-in-depth: the endpoint guard in usage_endpoints/endpoints.py + # should have already rejected this. If we ever reach here it means + # a future caller invoked the helper without scoping — fail loudly + # rather than issuing an unfiltered global query. + raise ValueError( + "Non-admin caller has user_id=None; refusing to issue an " + "unscoped query. Endpoint-level guard missing." + ) kwargs["user_id"] = user_id elif fn_args.get("user_id"): kwargs["user_id"] = fn_args["user_id"] diff --git a/litellm/proxy/management_endpoints/usage_endpoints/endpoints.py b/litellm/proxy/management_endpoints/usage_endpoints/endpoints.py index 0dbe518afb..d0df80fed0 100644 --- a/litellm/proxy/management_endpoints/usage_endpoints/endpoints.py +++ b/litellm/proxy/management_endpoints/usage_endpoints/endpoints.py @@ -44,13 +44,17 @@ async def usage_ai_chat( """ from litellm.proxy.management_endpoints.common_utils import ( _user_has_admin_view, + require_caller_user_id_for_non_admin, ) from litellm.proxy.management_endpoints.usage_endpoints.ai_usage_chat import ( stream_usage_ai_chat, ) is_admin = _user_has_admin_view(user_api_key_dict) - user_id = user_api_key_dict.user_id + if is_admin: + user_id = user_api_key_dict.user_id + else: + user_id = require_caller_user_id_for_non_admin(user_api_key_dict) messages = [{"role": m.role, "content": m.content} for m in data.messages] return StreamingResponse( diff --git a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py index a1e7fe59ca..f898763d2c 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py +++ b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py @@ -481,3 +481,39 @@ class TestSetObjectMetadataField: ): _set_object_metadata_field(team, "model_rpm_limit", {"x": 1}) assert team.metadata == {"model_rpm_limit": {"x": 1}} + + +class TestRequireCallerUserIdForNonAdmin: + """ + Security regression: service-account keys (user_id=None) must not bypass + the non-admin scoping branch on analytics endpoints. + """ + + def test_returns_user_id_when_present(self): + from litellm.proxy.management_endpoints.common_utils import ( + require_caller_user_id_for_non_admin, + ) + + key_dict = UserAPIKeyAuth( + user_id="user-abc", + user_role=LitellmUserRoles.INTERNAL_USER, + ) + assert require_caller_user_id_for_non_admin(key_dict) == "user-abc" + + def test_raises_403_when_user_id_is_none(self): + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.common_utils import ( + require_caller_user_id_for_non_admin, + ) + + # Simulates a service-account key (user_id forced to None at key creation) + service_account_key = UserAPIKeyAuth( + user_id=None, + user_role=LitellmUserRoles.INTERNAL_USER, + ) + with pytest.raises(HTTPException) as exc_info: + require_caller_user_id_for_non_admin(service_account_key) + + assert exc_info.value.status_code == 403 + assert "Service-account keys" in str(exc_info.value.detail) diff --git a/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py index a1ba7ecd67..b12d20a83b 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_internal_user_endpoints.py @@ -1732,6 +1732,107 @@ async def test_get_user_daily_activity_non_admin_cannot_view_other_users(monkeyp assert call_kwargs.kwargs["entity_id"] == "regular-user-123" +@pytest.mark.asyncio +async def test_get_user_daily_activity_rejects_service_account_caller(monkeypatch): + """ + Security regression: a non-admin caller with user_id=None (a service-account + key, where user_id is forced to None at key creation) must not be able to + bypass the entity filter and read every tenant's daily spend. + + Before the fix, the endpoint silently defaulted user_id to + user_api_key_dict.user_id, which is itself None for service-account keys. + None != None is False, the same-user check passed, and entity_id=None + flowed into get_daily_activity, where the SQL builder treats None as + "no filter". + """ + from unittest.mock import AsyncMock, MagicMock + + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.internal_user_endpoints import ( + get_user_daily_activity, + ) + + mock_prisma_client = MagicMock() + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) + + # Tripwire: ensure get_daily_activity is never reached + mock_get_daily = AsyncMock() + monkeypatch.setattr( + "litellm.proxy.management_endpoints.internal_user_endpoints.get_daily_activity", + mock_get_daily, + ) + + service_account_key = UserAPIKeyAuth( + user_id=None, # service-account keys have user_id forced to None + user_role=LitellmUserRoles.INTERNAL_USER, + ) + + with pytest.raises(HTTPException) as exc_info: + await get_user_daily_activity( + start_date="2025-01-01", + end_date="2025-01-31", + model=None, + api_key=None, + user_id=None, + page=1, + page_size=50, + timezone=None, + user_api_key_dict=service_account_key, + ) + + assert exc_info.value.status_code == 403 + assert "Service-account keys" in str(exc_info.value.detail) + mock_get_daily.assert_not_called() + + +@pytest.mark.asyncio +async def test_get_user_daily_activity_aggregated_rejects_service_account_caller( + monkeypatch, +): + """ + Same security regression as + test_get_user_daily_activity_rejects_service_account_caller, on the + aggregated route. Same shape, raw-SQL builder, same fix. + """ + from unittest.mock import AsyncMock, MagicMock + + from fastapi import HTTPException + + from litellm.proxy.management_endpoints.internal_user_endpoints import ( + get_user_daily_activity_aggregated, + ) + + mock_prisma_client = MagicMock() + monkeypatch.setattr("litellm.proxy.proxy_server.prisma_client", mock_prisma_client) + + mock_get_daily_agg = AsyncMock() + monkeypatch.setattr( + "litellm.proxy.management_endpoints.internal_user_endpoints.get_daily_activity_aggregated", + mock_get_daily_agg, + ) + + service_account_key = UserAPIKeyAuth( + user_id=None, + user_role=LitellmUserRoles.INTERNAL_USER, + ) + + with pytest.raises(HTTPException) as exc_info: + await get_user_daily_activity_aggregated( + start_date="2025-01-01", + end_date="2025-01-31", + model=None, + api_key=None, + user_id=None, + timezone=None, + user_api_key_dict=service_account_key, + ) + + assert exc_info.value.status_code == 403 + assert "Service-account keys" in str(exc_info.value.detail) + mock_get_daily_agg.assert_not_called() + + @pytest.mark.asyncio async def test_get_user_daily_activity_aggregated_admin_global_view(monkeypatch): """ diff --git a/tests/test_litellm/proxy/management_endpoints/usage_endpoints/test_ai_usage_chat.py b/tests/test_litellm/proxy/management_endpoints/usage_endpoints/test_ai_usage_chat.py index 66a18e2edb..e8a74e41da 100644 --- a/tests/test_litellm/proxy/management_endpoints/usage_endpoints/test_ai_usage_chat.py +++ b/tests/test_litellm/proxy/management_endpoints/usage_endpoints/test_ai_usage_chat.py @@ -409,3 +409,60 @@ class TestStreamUsageAiChat: end_date="2025-01-31", user_id="my-user-id", ) + + +class TestUsageAiChatServiceAccountGuard: + """ + Security regression: a non-admin caller with user_id=None (service-account + key) must be rejected at the endpoint boundary, before any tool dispatch. + """ + + @pytest.mark.asyncio + async def test_non_admin_with_user_id_none_is_rejected(self): + from fastapi import HTTPException + + from litellm.proxy._types import LitellmUserRoles, UserAPIKeyAuth + from litellm.proxy.management_endpoints.usage_endpoints.endpoints import ( + ChatMessage, + UsageAIChatRequest, + usage_ai_chat, + ) + + service_account_key = UserAPIKeyAuth( + user_id=None, + user_role=LitellmUserRoles.INTERNAL_USER, + ) + request = MagicMock() + body = UsageAIChatRequest( + messages=[ChatMessage(role="user", content="hi")], + model="gpt-4o-mini", + ) + + with pytest.raises(HTTPException) as exc_info: + await usage_ai_chat( + data=body, + request=request, + user_api_key_dict=service_account_key, + ) + + assert exc_info.value.status_code == 403 + assert "Service-account keys" in str(exc_info.value.detail) + + def test_resolve_fetch_kwargs_tripwire_fires_on_none_user_id(self): + """ + Defense-in-depth: if a future endpoint forgets the entry guard and + a non-admin caller with user_id=None reaches _resolve_fetch_kwargs, + the tripwire must fire rather than issuing an unscoped query. + """ + from litellm.proxy.management_endpoints.usage_endpoints.ai_usage_chat import ( + _resolve_fetch_kwargs, + ) + + with pytest.raises(ValueError) as exc_info: + _resolve_fetch_kwargs( + fn_name="get_usage_data", + fn_args={"start_date": "2025-01-01", "end_date": "2025-01-31"}, + user_id=None, + is_admin=False, + ) + assert "Endpoint-level guard missing" in str(exc_info.value)