fix(proxy): write failed-login counters and their expiry in one Redis call

Use RedisCache.async_increment_with_floor (a single Lua INCRBY + EXPIRE) for the
shared login counters instead of the two-step INCRBYFLOAT then EXPIRE, so a
counter can never be committed to Redis without its expiry. The repair in
_remaining_window now only covers expiries stripped out of band (PERSIST, a
restore) and uses the same atomic call

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

litellm/proxy/auth/login_throttle.py

@@ -194,11 +194,11 @@ class LoginThrottle:
         return max(local, _as_count(await self._outcome(redis_cache.async_get_cache(key))))
 
     async def _remaining_window(self, key: str) -> int:
-        """Seconds until this counter expires, repairing a counter left without an expiry.
+        """Seconds until this counter expires.
 
-        Redis commits the increment before setting the TTL, so a failure in between can
-        leave a counter that never expires. Nothing increments the key again once the
-        limit is reached, so without the repair the key would stay refused indefinitely.
+        Counters are only ever written together with their expiry, so a counter without one
+        was stripped out of band (PERSIST, a restore). It is given the full window again,
+        since nothing increments a key once the limit is reached.
         """
         redis_cache: Final = self.redis_cache
         if redis_cache is None:
@@ -206,7 +206,7 @@ class LoginThrottle:
         ttl: Final = await self._outcome(redis_cache.async_get_ttl(key))
         if isinstance(ttl, int) and ttl > 0:
             return min(ttl, self.window_seconds)
-        await self._outcome(redis_cache.async_increment(key, 0, ttl=self.window_seconds))
+        await self._outcome(redis_cache.async_increment_with_floor(key, 0, self.window_seconds))
         return self.window_seconds
 
     def _refused(self, retry_after: int, param: str) -> ProxyException:
@@ -260,8 +260,9 @@ class LoginThrottle:
         redis_cache: Final = self.redis_cache
         if redis_cache is None:
             return local
-        shared: Final = _as_count(await self._outcome(redis_cache.async_increment(key, 1, ttl=self.window_seconds)))
-        await self._remaining_window(key)
+        shared: Final = _as_count(
+            await self._outcome(redis_cache.async_increment_with_floor(key, 1, self.window_seconds))
+        )
         return max(local, shared)
 
     async def record_failure(self, username: str) -> FailureCounts:

tests/test_litellm/proxy/auth/test_login_utils.py

@@ -1142,58 +1142,65 @@ async def test_disabling_the_control_removes_the_delay_as_well(monkeypatch, logi
     assert login_delays.seconds == []
 
 
-class _NoExpiryRedis:
-    """Redis that stores the counter but never records an expiry for it.
+class _FakeRedis:
+    """Redis whose only counter write is the atomic INCRBY-plus-EXPIRE Lua call.
 
-    Models the window between INCRBYFLOAT committing and the TTL call failing.
+    `async_increment` is deliberately absent: a two-step increment would fail the test
+    with AttributeError, because Redis could then commit a count without its expiry.
     """
 
     def __init__(self):
         self.values: dict = {}
-        self.expiry_repairs = 0
+        self.ttls: dict = {}
 
     async def async_get_cache(self, key, **kwargs):
         return self.values.get(key)
 
-    async def async_increment(self, key, value, ttl=None, **kwargs):
-        if int(value) == 0:
-            self.expiry_repairs += 1
-        self.values[key] = self.values.get(key, 0) + int(value)
+    async def async_increment_with_floor(self, key, value, ttl):
+        self.values[key] = self.values.get(key, 0) + value
+        self.ttls.setdefault(key, ttl)
         return self.values[key]
 
     async def async_get_ttl(self, key):
-        return None
+        return self.ttls.get(key)
 
     async def async_delete_cache(self, key):
         self.values.pop(key, None)
+        self.ttls.pop(key, None)
+
+    def persist(self):
+        self.ttls.clear()
 
 
 @pytest.mark.asyncio
-async def test_a_counter_left_without_an_expiry_is_repaired(monkeypatch):
+async def test_counters_are_written_with_their_expiry_and_re_armed_if_stripped(monkeypatch):
     """Regression: a counter with no TTL would refuse the pair forever.
 
-    Redis commits the increment before setting the expiry, and nothing increments the key
-    again once the limit is reached, so a TTL that never landed is never repaired on its
-    own and the username and source pair stays refused with no way back.
+    Nothing increments a key once the limit is reached, so a counter that ever exists
+    without an expiry stays refused with no way back. Every write must therefore carry the
+    expiry, and a refusal that finds it stripped (PERSIST) must put the window back.
     """
     from litellm.proxy._types import ProxyException
 
     monkeypatch.setenv("UI_USERNAME", "admin")
     monkeypatch.setenv("UI_PASSWORD", "right")
-    redis = _NoExpiryRedis()
-    throttle = _throttle(max_attempts=2, redis_cache=redis)
+    redis = _FakeRedis()
+    throttle = _throttle(max_attempts=2, window_seconds=77, redis_cache=redis)
 
     for _ in range(2):
         with pytest.raises(ProxyException):
             await _guess(throttle)
 
-    assert redis.expiry_repairs >= 1, "each recorded failure must leave the counter with an expiry"
+    assert redis.values, "failures must land in the shared counter"
+    assert set(redis.ttls) == set(redis.values), "no counter may exist without its expiry"
+    assert set(redis.ttls.values()) == {77}
 
-    repairs_before_block = redis.expiry_repairs
+    redis.persist()
     with pytest.raises(ProxyException) as blocked:
         await _guess(throttle)
     assert blocked.value.code == "429"
-    assert redis.expiry_repairs > repairs_before_block, "the refusal path must repair a missing expiry too"
+    assert blocked.value.headers.get("Retry-After") == "77"
+    assert set(redis.ttls) >= {k for k in redis.values if ":user:" in k}, "the refusal must re-arm a stripped expiry"
 
 
 @pytest.mark.asyncio