Merge 188e3a7cbc into 9071ca503e

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

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
 

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

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):