From c7da25d12680f899cfc9325aa8c4a2dc4e12f74e Mon Sep 17 00:00:00 2001
From: Matthew Meszaros <meszmatew@gmail.com>
Date: Fri, 4 Sep 2026 21:12:27 -0700
Subject: [PATCH] fix: never leave a per-IP rate-limit counter without a TTL,
 which would block that address forever once it passed the limit, by dropping
 the key and failing open when EXPIRE fails and repairing a missing expiry on
 the reject path, and assert the exit status as well as the message when the
 CLI installer rejects a flag

---
 internal/api/middleware/ratelimit_ip.go | 19 ++++++++++++++++++-
 scripts/check-cli-installer.sh          |  6 +++++-
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/internal/api/middleware/ratelimit_ip.go b/internal/api/middleware/ratelimit_ip.go
index 71c60e23..579009f9 100644
--- a/internal/api/middleware/ratelimit_ip.go
+++ b/internal/api/middleware/ratelimit_ip.go
@@ -85,8 +85,25 @@ func (h *Handler) ipRateLimiter(prefix string, defaultLimit int, env string, win
 			c.Next()
 			return
 		}
+		// A counter with no TTL never resets, so the address it belongs to
+		// stays blocked forever once it passes the limit. That is a worse
+		// outcome than not counting at all, so a failed EXPIRE drops the key
+		// and lets the request through, matching how the rest of this
+		// middleware handles a cache it cannot trust.
 		if n == 1 {
-			_ = h.Cache.Expire(c.Request.Context(), key, window).Err()
+			if err := h.Cache.Expire(c.Request.Context(), key, window).Err(); err != nil {
+				_ = h.Cache.Del(c.Request.Context(), key).Err()
+				c.Next()
+				return
+			}
+		} else if n > int64(limit) {
+			// Repair a key that lost its expiry some other way (an older
+			// build, a restore, an eviction between the INCR and the EXPIRE
+			// above). Only on the reject path, which is rare, so it costs a
+			// round trip nobody feels.
+			if ttl, terr := h.Cache.TTL(c.Request.Context(), key).Result(); terr == nil && ttl < 0 {
+				_ = h.Cache.Expire(c.Request.Context(), key, window).Err()
+			}
 		}
 
 		if n > int64(limit) {
diff --git a/scripts/check-cli-installer.sh b/scripts/check-cli-installer.sh
index e4991901..ee7c322c 100755
--- a/scripts/check-cli-installer.sh
+++ b/scripts/check-cli-installer.sh
@@ -65,7 +65,11 @@ pass "--help works"
 # are set and `set -u` turns an unset variable into a fatal error. Only --help
 # and --dry-run were exercised, so nothing caught it.
 for bad in --nonsense --dir; do
-  out=$(sh "$SCRIPT" "$bad" 2>&1 || true)
+  # The exit status matters as much as the message: a script that explains the
+  # problem and then exits 0 tells every caller the install succeeded.
+  if out=$(sh "$SCRIPT" "$bad" 2>&1); then
+    fail "$bad exited 0; a rejected flag has to fail"
+  fi
   case "$out" in
     *"unbound variable"*) fail "$bad aborted with an unbound variable instead of an error message" ;;
   esac