diff --git a/litellm/proxy/management_endpoints/common_utils.py b/litellm/proxy/management_endpoints/common_utils.py index 98155ad6839..8e1fd17ac8d 100644 --- a/litellm/proxy/management_endpoints/common_utils.py +++ b/litellm/proxy/management_endpoints/common_utils.py @@ -23,6 +23,28 @@ def validate_finite_spend(spend: float | None) -> None: ) +def validate_key_duration(duration: str | None) -> None: + """Reject key-expiry durations that can't be parsed, so a bad value 400s + at the write boundary instead of 500ing inside duration math. + + `None` (never expires) and `'-1'` (the update path's never-expire + sentinel) pass through. + """ + if duration is None or duration == "-1": + return + + from litellm.litellm_core_utils.duration_parser import duration_in_seconds + + try: + duration_in_seconds(duration) + except (ValueError, OverflowError): + message = f"Invalid duration '{duration}'. Use a format like '1h', '24h', '7d', or '30d'." + raise HTTPException( + status_code=400, + detail={"error": message}, # mutable-ok: single-shot error envelope + ) + + def validate_budget_duration(budget_duration: str | None, status_code: int = 400) -> None: """Reject budget durations that can't be parsed, are non-positive, or overflow date math, so a bad value can't be persisted and later crash the diff --git a/litellm/proxy/management_endpoints/key_management_endpoints.py b/litellm/proxy/management_endpoints/key_management_endpoints.py index db4467dda5b..0f93bd696db 100644 --- a/litellm/proxy/management_endpoints/key_management_endpoints.py +++ b/litellm/proxy/management_endpoints/key_management_endpoints.py @@ -92,6 +92,7 @@ from litellm.proxy.management_endpoints.common_utils import ( _user_has_admin_view, validate_budget_duration, validate_finite_spend, + validate_key_duration, ) from litellm.proxy.management_endpoints.model_management_endpoints import ( _add_model_to_db, @@ -1002,6 +1003,7 @@ async def _common_key_generation_helper( ) validate_budget_duration(data.budget_duration) + validate_key_duration(data.duration) raise_on_invalid_key_logging_config(data.metadata) if data.throttle_on_budget_exceeded is True and user_api_key_dict.user_role != LitellmUserRoles.PROXY_ADMIN.value: @@ -2265,6 +2267,11 @@ async def prepare_key_update_data( if "duration" in non_default_values: duration: Final = non_default_values.pop("duration") + if duration is None or isinstance(duration, str): + # Reject garbage here so bulk/team-bulk/regenerate callers that + # skip _validate_update_key_data still 400 instead of 500ing + # inside duration math below. + validate_key_duration(duration) if duration is None or duration == "-1": # Set expires to None to indicate the key never expires non_default_values["expires"] = None @@ -2275,6 +2282,9 @@ async def prepare_key_update_data( if "budget_duration" in non_default_values: budget_duration: Final = non_default_values.pop("budget_duration") + if budget_duration is None or isinstance(budget_duration, str): + # Same write-boundary guarantee as duration above. + validate_budget_duration(budget_duration) if budget_duration is None: non_default_values["budget_duration"] = None non_default_values["budget_reset_at"] = None @@ -2695,6 +2705,7 @@ async def _validate_update_key_data( # Reject NaN/±inf spend before it can reach the DB / spend counter. validate_finite_spend(data.spend) validate_budget_duration(data.budget_duration) + validate_key_duration(data.duration) _is_proxy_admin: Final = user_api_key_dict.user_role == LitellmUserRoles.PROXY_ADMIN.value 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 da8fc760787..b7a7feda69c 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_common_utils.py +++ b/tests/test_litellm/proxy/management_endpoints/test_common_utils.py @@ -29,6 +29,7 @@ from litellm.proxy.management_endpoints.common_utils import ( _user_has_admin_privileges, _user_has_admin_view, admin_can_invite_user, + validate_key_duration, ) from litellm.proxy.management_endpoints.common_utils import _has_non_empty_value @@ -1120,3 +1121,23 @@ class TestUpdateMetadataFieldsPremiumCheck: } _update_metadata_fields(updated_kv) mock_check.assert_called() + + +class TestValidateKeyDuration: + """Invalid key-expiry durations must 400, not 500 (#39710).""" + + def test_none_and_never_expire_sentinel_pass(self): + assert validate_key_duration(None) is None + assert validate_key_duration("-1") is None + + def test_valid_durations_pass(self): + assert validate_key_duration("7d") is None + assert validate_key_duration("1h") is None + + def test_garbage_durations_raise_400(self): + from fastapi import HTTPException + + for bad in ["banana", "1x", ""]: + with pytest.raises(HTTPException) as exc_info: + validate_key_duration(bad) + assert exc_info.value.status_code == 400 diff --git a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py index 65cc23ea67f..f06f40c98dd 100644 --- a/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py +++ b/tests/test_litellm/proxy/management_endpoints/test_key_management_endpoints.py @@ -1796,6 +1796,32 @@ async def test_update_service_account_works_with_team_id(): await prepare_key_update_data(data=data, existing_key_row=existing_key) +@pytest.mark.asyncio +async def test_update_key_garbage_duration_400s_instead_of_500(): + """Regression: bulk/team-bulk/regenerate skip _validate_update_key_data, so + prepare_key_update_data itself must reject garbage durations (#39711).""" + data = UpdateKeyRequest(key="sk-1", duration="not-a-duration") + existing_key = LiteLLM_VerificationToken(token="hashed") + + with pytest.raises(HTTPException) as exc_info: + await prepare_key_update_data(data=data, existing_key_row=existing_key) + assert exc_info.value.status_code == 400 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("duration", ["7d", None, "-1"]) +async def test_update_key_valid_durations_still_flow_through(duration): + data = UpdateKeyRequest(key="sk-1", duration=duration) + existing_key = LiteLLM_VerificationToken(token="hashed") + + updated = await prepare_key_update_data(data=data, existing_key_row=existing_key) + + if duration in (None, "-1"): + assert updated["expires"] is None + else: + assert updated["expires"] is not None + + @pytest.mark.asyncio @pytest.mark.parametrize("flag_value", [True, False]) async def test_update_key_enable_prompt_caching_folds_into_metadata(flag_value):