feat(sdk): rate-limit counters can live somewhere shared - #29
Merged
Conversation
RateLimitRule hard-constructed an InMemoryRateLimiter -- a Map, with no
seam to replace it. On any deployment with more than one process the limit
was effectively max x instances, and on Vercel or Lambda it also reset on
every cold start. rateLimit({ max: 100, window: 60 }) across eight
replicas is an 800/min limit that an autoscaler can raise for you.
That was inconsistent with the rest of the SDK: the detection stores and
the captcha's challenge and token stores were already behind swappable
interfaces. The one piece of state that actually has to be shared was the
only one that could not be.
evaluate() is synchronous and making it async would turn every rule
evaluation into a promise for the sake of the one rule that needs it, so a
store declares which it is. A sync store is consumed inline -- the default
in-memory path, unchanged. An async store is consumed in a new optional
Rule.prepare(), which protect() awaits, the same pre-fetch already used
for IP enrichment and Web Bot Auth verdicts.
A rule whose async store was never prepared reports NOT_RUN rather than
allowing. A rate limiter that has quietly stopped limiting is
indistinguishable from one that works, which is the worst of the options.
The Upstash store lives in core and uses fetch, not @upstash/redis: two
commands in a pipeline is not worth a dependency, a version to track, or a
transitive node: import finding its way into an edge bundle. Fixed windows
put the window id in the key so expiry is the only cleanup and two
processes cannot disagree about which window they are in; sliding windows
give each request a random member suffix, because two requests in the same
millisecond would otherwise be one ZADD that overwrites rather than two
that count.
Closes WebDecoy/app#726
cport1
force-pushed
the
feat/rate-limit-store
branch
2 times, most recently
from
August 22, 2026 02:14
014e46a to
fba9c9a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #726. Stacked on #28 (typed decision) — the
NOT_RUNstate it reports comes from there.The bug
rate-limit-rule.ts:19hard-constructednew InMemoryRateLimiter()— aMap, with no seam to replace it. On any deployment with more than one process the limit is effectivelymax × instances, and on Vercel or Lambda it also resets on every cold start.rateLimit({ max: 100, window: 60 })across eight replicas is an 800/min limit that an autoscaler can raise for you.This was inconsistent with the rest of the SDK:
detection/stores.tsand the captcha's challenge and token stores were already behind swappable interfaces with in-memory defaults. The one piece of state that actually has to be shared was the only one that could not be.The sync/async problem
Rule.evaluate()is synchronous. Making it async would turn every rule evaluation into a promise for the sake of the one rule that might need it, and the in-memory path — which is most installs — would pay for a network store nobody configured.So a store declares which it is:
evaluate(). The default in-memory path, unchanged and allocation-free.Rule.prepare(context), whichprotect()awaits before evaluation. This is the pre-fetch pattern the SDK already uses for IP enrichment and Web Bot Auth verdicts, so it fits the existing architecture rather than inventing a second one.A rule whose async store was never prepared reports
NOT_RUN, not ALLOW. The synchronousevaluateRules()cannot consume a networked store, and a rate limiter that has quietly stopped limiting is indistinguishable from one that is working — that is the failure mode worth being loud about.The Upstash store
Ships in the core package, not a sixth workspace, and calls the REST API with
fetchrather than depending on@upstash/redis. Two commands in a pipeline is not worth a dependency, a version to track, or a transitivenode:import finding its way into an edge bundle.check:edgecovers it automatically.Upstash is the right first target because it speaks Redis over HTTP: Vercel Edge, Workers and Deno have no
node:net, so an ordinary Redis client cannot open a socket in exactly the serverless deployments that need shared counters most.Two details worth reviewing:
wd:rl:f:<key>:<windowStart>), so expiry is the only cleanup needed and two processes cannot disagree about which window they are in.EXPIRE … NX, or a long window gets extended into a sliding one by later requests.<now>-<rand>). Two requests in the same millisecond would otherwise be oneZADDthat overwrites rather than two that count — undercounting exactly when the limit matters.Fails open by default when Redis is unreachable, because a rate limiter that takes the site down when its datastore has a bad minute has done more damage than the traffic it was shaping.
onError: 'closed'for limits protecting something more expensive than availability. Either way the outcome lands indecision.resultsrather than looking like a normal evaluation.Verification
14 new tests, 341 total. The one that matters: two
WebDecoyinstances pointed at one store enforcemax: 2as two, where before each had its ownMapand it was four. Also asserts exactly oneconsume()per request — a second consume inevaluate()afterprepare()would double-count and halve every limit. Build, lint andcheck:edgegreen.