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

This commit is contained in:
Matthew Meszaros
2026-09-04 21:12:27 -07:00
parent b35e111705
commit c7da25d126
2 changed files with 23 additions and 2 deletions
+18 -1
View File
@@ -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) {
+5 -1
View File
@@ -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