From 056e6e9d43a93d7af21e6bd82494867832bcfa60 Mon Sep 17 00:00:00 2001 From: Krrish Dholakia Date: Wed, 1 Oct 2025 14:28:24 -0700 Subject: [PATCH] fix(key_management_endpoints.py): fix security issue where user could update another user's key metadata --- litellm/proxy/_new_secret_config.yaml | 28 ++++++++- .../key_management_endpoints.py | 58 ++++++++++--------- 2 files changed, 58 insertions(+), 28 deletions(-) diff --git a/litellm/proxy/_new_secret_config.yaml b/litellm/proxy/_new_secret_config.yaml index 5d8052493a1..b7b30d36f99 100644 --- a/litellm/proxy/_new_secret_config.yaml +++ b/litellm/proxy/_new_secret_config.yaml @@ -1,8 +1,32 @@ model_list: - model_name: openai/gpt-4o litellm_params: - model: openai/gpt-4o - api_key: os.environ/OPENAI_API_KEY + model: openai/gpt-4o-mini + api_base: "https://webhook.site/2f385e05-00aa-402b-86d1-efc9261471a5" + api_key: dummy + - model_name: "byok-wildcard/*" + litellm_params: + model: openai/* + - model_name: xai-grok-3 + litellm_params: + model: xai/grok-3 + - model_name: hosted_vllm/whisper-v3 + litellm_params: + model: hosted_vllm/whisper-v3 + api_base: "https://webhook.site/2f385e05-00aa-402b-86d1-efc9261471a5" + api_key: dummy + +# mcp_servers: +# github_mcp: +# url: "https://api.githubcopilot.com/mcp" +# auth_type: oauth2 +# authorization_url: https://github.com/login/oauth/authorize +# token_url: https://github.com/login/oauth/access_token +# client_id: os.environ/GITHUB_OAUTH_CLIENT_ID +# client_secret: os.environ/GITHUB_OAUTH_CLIENT_SECRET +# scopes: ["public_repo", "user:email"] +# allowed_tools: ["list_tools"] +# # disallowed_tools: ["repo_delete"] litellm_settings: callbacks: ["prometheus"] diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index 7ce38c2d683..0eb2d27463f 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -90,10 +90,10 @@ def _get_user_in_team( def _calculate_key_rotation_time(rotation_interval: str) -> datetime: """ Helper function to calculate the next rotation time for a key based on the rotation interval. - + Args: rotation_interval: String representing the rotation interval (e.g., '30d', '90d', '1h') - + Returns: datetime: The calculated next rotation time in UTC """ @@ -102,28 +102,34 @@ def _calculate_key_rotation_time(rotation_interval: str) -> datetime: return now + timedelta(seconds=interval_seconds) -def _set_key_rotation_fields(data: dict, auto_rotate: bool, rotation_interval: Optional[str]) -> None: +def _set_key_rotation_fields( + data: dict, auto_rotate: bool, rotation_interval: Optional[str] +) -> None: """ Helper function to set rotation fields in key data if auto_rotate is enabled. - + Args: data: Dictionary to update with rotation fields auto_rotate: Whether auto rotation is enabled rotation_interval: The rotation interval string (required if auto_rotate is True) """ if auto_rotate and rotation_interval: - data.update({ - "auto_rotate": auto_rotate, - "rotation_interval": rotation_interval, - "key_rotation_at": _calculate_key_rotation_time(rotation_interval) - }) + data.update( + { + "auto_rotate": auto_rotate, + "rotation_interval": rotation_interval, + "key_rotation_at": _calculate_key_rotation_time(rotation_interval), + } + ) def _is_allowed_to_make_key_request( - user_api_key_dict: UserAPIKeyAuth, user_id: Optional[str], team_id: Optional[str] + user_api_key_dict: UserAPIKeyAuth, + user_id: Optional[str], + team_id: Optional[str], ) -> bool: """ - Assert user only creates keys for themselves + Assert user only creates/updates keys for themselves Relevant issue: https://github.com/BerriAI/litellm/issues/7336 """ @@ -332,6 +338,7 @@ def common_key_access_checks( data: Union[GenerateKeyRequest, UpdateKeyRequest], llm_router: Optional[Router], premium_user: bool, + user_id: Optional[str] = None, ) -> Literal[True]: """ Check if user is allowed to make a key request, for this key @@ -339,7 +346,7 @@ def common_key_access_checks( try: _is_allowed_to_make_key_request( user_api_key_dict=user_api_key_dict, - user_id=data.user_id, + user_id=user_id or data.user_id, team_id=data.team_id, ) except AssertionError as e: @@ -1136,13 +1143,6 @@ async def update_key_fn( if prisma_client is None: raise Exception("Not connected to DB!") - common_key_access_checks( - user_api_key_dict=user_api_key_dict, - data=data, - llm_router=llm_router, - premium_user=premium_user, - ) - existing_key_row = await prisma_client.get_data( token=data.key, table_name="key", query_type="find_unique" ) @@ -1153,6 +1153,14 @@ async def update_key_fn( detail={"error": f"Team not found, passed team_id={data.team_id}"}, ) + common_key_access_checks( + user_api_key_dict=user_api_key_dict, + data=data, + user_id=existing_key_row.user_id, + llm_router=llm_router, + premium_user=premium_user, + ) + # check if user has permission to update key await TeamMemberPermissionChecks.can_team_member_execute_key_management_endpoint( user_api_key_dict=user_api_key_dict, @@ -1198,9 +1206,9 @@ async def update_key_fn( # Handle rotation fields if auto_rotate is being enabled _set_key_rotation_fields( - non_default_values, - non_default_values.get("auto_rotate", False), - non_default_values.get("rotation_interval") + non_default_values, + non_default_values.get("auto_rotate", False), + non_default_values.get("rotation_interval"), ) _data = {**non_default_values, "token": key} @@ -1602,8 +1610,6 @@ def _check_model_access_group( return True - - async def generate_key_helper_fn( # noqa: PLR0915 request_type: Literal[ "user", "key" @@ -1766,12 +1772,12 @@ async def generate_key_helper_fn( # noqa: PLR0915 "allowed_routes": allowed_routes or [], "object_permission_id": object_permission_id, } - + # Add rotation fields if auto_rotate is enabled _set_key_rotation_fields( data=key_data, auto_rotate=auto_rotate or False, - rotation_interval=rotation_interval + rotation_interval=rotation_interval, ) if (