Compare commits

...

1 Commits

Author SHA1 Message Date
J
e0752c9c9f fix(webhook): use unique ZSET member in Redis rate limiter
The sliding-window Lua script used the nanosecond timestamp as both the
ZSET score and member. Two requests landing in the same nanosecond
collided on an identical member, so ZADD updated in place instead of
inserting and the window under-counted — letting requests through past
the limit. This surfaced as a flaky CI failure in
TestRedisWebhookIPRateLimiter_HasSeparateBudgetFromTokenLimiter.

Keep the timestamp as the score (so ZREMRANGEBYSCORE trimming is
unchanged) and use a per-request UUID as the member so each admitted
request is counted exactly once.

Co-authored-by: multica-agent <github@multica.ai>
2026-06-25 01:16:42 +08:00

View File

@@ -5,6 +5,7 @@ import (
"sync"
"time"
"github.com/google/uuid"
"github.com/redis/go-redis/v9"
)
@@ -86,10 +87,12 @@ func (l *memoryWebhookRateLimiter) Allow(_ context.Context, key string) bool {
// ── Redis implementation ────────────────────────────────────────────────────
// webhookLimiterKey:<token> is the ZSET we keep timestamps in. Members are
// nanosecond timestamps as strings. Score = same value, so ZREMRANGEBYSCORE
// can drop everything older than the cutoff, then ZCARD tells us the
// remaining count.
// webhookLimiterKey:<token> is the ZSET we keep timestamps in. The score is
// the request's nanosecond timestamp so ZREMRANGEBYSCORE can drop everything
// older than the cutoff and ZCARD tells us the remaining count. The member is
// a per-request unique id (NOT the timestamp): two requests landing in the
// same nanosecond would otherwise collide on an identical member, ZADD would
// update-in-place instead of inserting, and the window would under-count.
const (
webhookLimiterKeyPrefix = "mul:webhook:rate:"
webhookIPLimiterKeyPrefix = "mul:webhook:ip:"
@@ -98,10 +101,11 @@ const (
// webhookLimiterAllowSrc runs the slide-window check atomically on Redis:
//
// KEYS[1] = ZSET key
// ARGV[1] = now (unix nanos as string)
// ARGV[1] = now (unix nanos as string, used as the entry score)
// ARGV[2] = cutoff (unix nanos as string)
// ARGV[3] = limit
// ARGV[4] = expiry seconds (TTL refresh, larger than window)
// ARGV[5] = unique member id for this request
//
// Returns 1 when the request is admitted, 0 when it should be rejected.
//
@@ -114,12 +118,13 @@ local now = tonumber(ARGV[1])
local cutoff = tonumber(ARGV[2])
local limit = tonumber(ARGV[3])
local ttl = tonumber(ARGV[4])
local member = ARGV[5]
redis.call('ZREMRANGEBYSCORE', key, '-inf', cutoff)
local count = redis.call('ZCARD', key)
if count >= limit then
return 0
end
redis.call('ZADD', key, now, tostring(now))
redis.call('ZADD', key, now, member)
redis.call('EXPIRE', key, ttl)
return 1
`
@@ -169,11 +174,15 @@ func (l *redisWebhookRateLimiter) Allow(ctx context.Context, key string) bool {
if prefix == "" {
prefix = webhookLimiterKeyPrefix
}
// Unique member per request: the score carries the timestamp for the
// sliding-window trim, but two requests in the same nanosecond must not
// collapse onto one ZSET member, or the window under-counts.
member := uuid.NewString()
res, err := webhookLimiterAllowScript.Run(
ctx,
l.rdb,
[]string{prefix + key},
now, cutoff, l.cfg.Limit, ttlSeconds,
now, cutoff, l.cfg.Limit, ttlSeconds, member,
).Int()
if err != nil {
// Fail open on Redis errors — webhook ingress should keep working