Rate limit the public search API

branch · main@8f1c2ab → cursor/rate-limit-search@d4e91f0 · claude-opus-5 (cursor)

One integration partner was issuing ~40 search requests a second, which saturated the read replica and slowed checkout for everyone. This adds a fixed-window limiter in front of /api/search only: 100 requests per minute per API key, 429 with Retry-After when exceeded.

It deliberately does not touch the other routes. The window is stored in Redis with INCR + EXPIRE, so it is shared across the four app instances rather than per-process.

riskwhole changemedium risk70%

Every limiter is a way to break traffic that used to work. If the 100/min number is wrong for a real partner, they start getting 429s in production with no warning.

  • "cap the search endpoint, partners are hammering it"
    the requirement, but no number was given
  • docs/partners.md:44
    the largest documented partner contract is 60 requests a minute, so 100 leaves headroom
  • unverifiednot verified
    no load test against production traffic shapes
  • log limiter rejections for a week before enforcing, if you want a safer rollout
90
Explained
44 of 49 added lines
86%
verified
sourced
8
inferred
1
stubs
0
high risk
1
todos
3
questions
1
  • npm test -- rate-limit
    11 tests, including the boundary at exactly 100
    pass
  • npm run typecheck
    pass
  • npm run test:e2e
    needs a staging Redis; not available on this machine
    skipped
annotations11 of 11 shown · j k to step through

src/lib/rate-limit.ts

+35 0read-only794%
@@ -0,0 +1,35 @@
1+import { redis } from "@/lib/redis";
2+ 
3+export interface Decision {
4+ allowed: boolean;
5+ remaining: number;
6+ resetSeconds: number;
7+}
8+ 
9+const LIMIT = 100;
10+const WINDOW_SECONDS = 60;
11+ 
12+/**
13+ * Fixed window per key. Two Redis commands per request, pipelined, so the
14+ * limiter costs one round trip.
15+ */
16+export async function take(key: string): Promise<Decision> {
17+ const bucket = `rl:${key}:${Math.floor(Date.now() / 1000 / WINDOW_SECONDS)}`;
18+ const [hits] = await redis
19+ .pipeline()
20+ .incr(bucket)
21+ .expire(bucket, WINDOW_SECONDS)
22+ .exec();
23+ 
24+ const count = Number(hits);
25+ return {
26+ allowed: count <= LIMIT,
27+ remaining: Math.max(0, LIMIT - count),
28+ resetSeconds:
29+ WINDOW_SECONDS - (Math.floor(Date.now() / 1000) % WINDOW_SECONDS),
30+ };
31+}
32+ 
33+export function bucketFor(key: string, at = Date.now()): string {
34+ return `rl:${key}:${Math.floor(at / 1000 / WINDOW_SECONDS)}`;
35+}
intent+1..795%

The module returns a Decision object rather than a boolean so the route can put remaining and resetSeconds into headers without recomputing the window arithmetic.

  • src/lib/search.ts:9
    the other lib modules also return a result object rather than a primitive
  • passedtypes
intent+9..10low risk90%

The limit and window are constants rather than config because there is no runtime config mechanism in this service yet, and inventing one here would have made the change much larger than the problem.

  • src/lib/*.ts
    every other module in lib/ hard-codes its tuning values the same way
  • docs/partners.md:44
    100/min sits above the largest contract
  • rejected env vars — rejected for now; there is no schema or validation for env in this service, so a typo would fail silently at runtime
changing the limit needs a deploy, which is acceptable while there is exactly one consumer of this module
intent+11..1590%

The comment records the cost of the design (two commands, one round trip) because that is the thing a future reader will want to know before adding a third Redis call to this function.

  • passedread only
intent+16..24medium risk85%

A fixed window was chosen over a token bucket because INCR/EXPIRE are atomic on the Redis side, so no lock or Lua script is needed and the limiter stays correct across the four app instances.

  • passedcommand
    $ npm test -- rate-limit
    11 tests
  • passedtypes
  • rejected sliding window log — rejected, it needs one sorted-set entry per request, which is far more memory for a limit this coarse
  • rejected token bucket in process memory — rejected, four instances would each allow the full 100

The known cost of a fixed window is burstiness at the boundary: a client can send 100 requests at 11:59:59 and 100 more at 12:00:00. For protecting a read replica that is acceptable; for billing it would not be.

assumption+19..23medium risk50%

EXPIRE is issued on every request rather than only on creation, so a long-running window keeps having its TTL reset to a full 60 seconds.

  • no external source — inferred
    I could not find a codebase convention for this, and the Redis docs do not say EXPIRE-on-every-INCR is wrong, only that it is redundant
  • passedcommand
    $ npm test -- rate-limit
    (the test asserts the window resets at 60s, which passes because the clock is faked)
  • confirm against a real Redis before this ships
  • is refreshing the TTL on every hit acceptable, or should this be SET key 1 EX 60 NX followed by INCR?
intent+25..3185%

remaining is clamped at zero because INCR keeps counting past the limit; without the clamp a blocked client would see a negative number in the header. resetSeconds is derived from the wall clock rather than Redis TTL to avoid a second round trip.

  • src/app/api/search/route.ts:21
    the only consumer, which puts both values in the 429 response
  • passedcommand
    $ npm test -- rate-limit
    (asserts remaining never goes below 0)
  • rejected TTL bucket for the reset value — rejected, it costs another round trip for a number the process can already compute
test+32..3490%

bucketFor exists only so the test can compute the same key the limiter will use, without duplicating the arithmetic in the test file.

  • tests/rate-limit.test.ts:14
    the only caller
  • passedcommand
    $ npm test -- rate-limit

src/app/api/search/route.ts

+14 2read-only379%
@@ -1,9 +1,14 @@
11 import { NextResponse } from "next/server";
22 import { search } from "@/lib/search";
3+import { take } from "@/lib/rate-limit";
34  
45 export async function GET(request: Request) {
56 const url = new URL(request.url);
67 const query = url.searchParams.get("q") ?? "";
7 const results = await search(query);
8 return NextResponse.json({ results });
8+ const apiKey = request.headers.get("x-api-key");
9+ if (apiKey) {
10+ const decision = await take(apiKey);
11+ if (!decision.allowed) return tooMany(decision.resetSeconds);
12+ }
13+ return NextResponse.json({ results: await search(query) });
914 }
@@ -10,2 +15,9 @@
1015  
1116 export const runtime = "nodejs";
17+ 
18+function tooMany(resetSeconds: number) {
19+ return NextResponse.json(
20+ { error: "rate_limited", retryAfter: resetSeconds },
21+ { status: 429, headers: { "retry-after": String(resetSeconds) } },
22+ );
23+}
note-790%

The old code awaited search(query) into a variable that was used once. Inlining it keeps the happy path a single expression now that a branch sits above it.

  • passedread only
security+8..11high risk80%

Requests without an x-api-key header are not limited at all. Anonymous search traffic goes through the CDN and was already capped there, and adding an IP-based limit here would have hit shared-NAT users.

  • infra/cdn.tf:88
    the CDN rule caps anonymous search at 20/min/IP
  • "partners are hammering it"
    the reported problem is keyed traffic, not anonymous
  • passedcommand
    $ npm test -- rate-limit
    covers the missing-key path
  • unverifiedchecked by hand
    101st keyed request returns 429, anonymous request does not
    => pass
if the CDN rule is ever removed, this endpoint is unprotected for anonymous callers and nothing here will tell you.
  • add an alert on the CDN rule, or fail closed here
intent+17..2395%

Retry-After is sent in seconds rather than a date because the value is computed as a duration and the HTTP spec allows either; a duration avoids clock-skew between our servers and the client.

  • rfc9110#10.2.3
    Retry-After accepts delay-seconds
  • passedcommand
    $ npm test -- rate-limit
  • passedchecked by hand
    curl -i shows retry-after: 37