feat(phase-8): extract rate-limit module (Batch A, A1-A6)

[Nether403]

Summary

Implements Batch A (tasks A1-A6) of the Phase 8 deployment spec. Lifts the inline rateLimit(bucket, limit) factory and in-process Map out of apps/api/src/app.ts into a dedicated apps/api/src/rate-limit/ module, adds an Upstash Redis backend behind a RATE_LIMIT_BACKEND feature flag, and gates every backend through a fail-open wrapper per design § 9.

Module layout

File	Purpose
buckets.ts	generation = 30/60s, read = 100/60s
client-id.ts	x-forwarded-for → cf-connecting-ip → "local" (R4.4)
types.ts	RateLimitBackend, RateLimitDecision interfaces
memory.ts	Process-local backend, lazy rollover (no setInterval)
upstash.ts	@upstash/ratelimit sliding window, null on missing env
fail-open.ts	Wraps any backend, logs once per 60s on failure (R4.5)
index.ts	Public barrel + createRateLimitMiddleware factory
rate-limit.pbt.test.ts	fast-check Property 1: failures never yield 429

Backend selection

RATE_LIMIT_BACKEND env var picks the backend:

• Default memory — preserves current test and local-dev behavior.
• upstash — activates the Upstash client. If credentials are missing, silently falls back to memory with a single [rate-limit] upstash env missing, falling back to memory warn so flipping the flag ahead of provisioning never breaks boot (design § 9 step 2).

Any backend is wrapped in wrapFailOpen(...) so check() errors are swallowed, logged at most once per 60s, and the request is allowed through — the fail-open invariant is asserted by a fast-check property (design § 8 Property 1).

App integration (A6)

• apps/api/src/app.ts swaps the three rate-limit app.use() mounts to the new createRateLimitMiddleware factory. Removed GENERATION_LIMIT / READ_LIMIT / WINDOW_MS constants and the rateLimitBuckets export.
• apps/api/src/index.ts deletes the setInterval stale-key cleanup — memory backend now rolls over lazily (design § 9 step 1).

Contract tests

Four new cases in apps/api/src/app.test.ts (design § 8):

• admin 401 before rate-limit counter increments (R8.1)
• Retry-After only on 429 (R4.7, R4.8)
• /health and /openapi.json exempt from counting (R4.9)
• bucket count survives backend swap (R6.4)

Dependencies added

Package	Version	Kind
@upstash/ratelimit	^2.0.8	runtime
@upstash/redis	^1.38.0	runtime
fast-check	^4.8.0	dev

Verification

pnpm --filter @stackfast/api test         → 7 files, 83 tests, 0 failed
pnpm --filter @stackfast/api type-check   → exit 0
pnpm --filter @stackfast/api lint         → exit 0
pnpm --filter @stackfast/api build        → exit 0

Test count grew from 40 → 83 (+43 new cases: 14 buckets/client-id, 8 memory, 7 fail-open, 9 Upstash, 1 PBT property, 4 contract).

Closes

Tasks A1, A2, A3, A4, A5, A6 in .kiro/specs/phase-8-deployment/tasks.md.

Next up: Batch B (Sentry wiring behind SENTRY_DSN) and Batch C (auth fail-closed tightening) — both unblocked once this merges.

---

---

Summary by cubic

Extracted the rate limiter into a dedicated apps/api/src/rate-limit/ module with memory and optional Upstash backends, and wired a fail-open middleware across API routes. This completes Phase 8 Batch A (A1–A6) and readies the API for multi-instance deploys without changing local/test behavior.

• New Features

  • New module with buckets config, client-id helper, shared types, and createRateLimitMiddleware.
  • Memory backend with lazy rollover; removed the old Map cleanup timer.
  • Upstash Redis backend behind RATE_LIMIT_BACKEND=upstash; falls back to memory if Upstash env is missing; both backends run through fail-open so backend errors never return 429.
  • App mounts switched to the factory; /health and /openapi.json remain uncounted; added contract and property-based tests.

• Dependencies

  • Runtime: @upstash/ratelimit ^2.0.8, @upstash/redis ^1.38.0
  • Dev: fast-check ^4.8.0

^{Written for commit 39a40ab. Summary will update on new commits.}

[cubic-dev-ai]

4 issues found across 19 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="apps/api/src/rate-limit/rate-limit.pbt.test.ts">

<violation number="1" location="apps/api/src/rate-limit/rate-limit.pbt.test.ts:182">
P2: Don't truncate the generated request sequence to the shorter auxiliary array; it can skip backend failures at the tail of the run.</violation>
</file>

<file name="apps/api/src/rate-limit/fail-open.ts">

<violation number="1" location="apps/api/src/rate-limit/fail-open.ts:52">
P2: The fail-open log message is backend-specific (`upstash unavailable`) even though this wrapper is used for memory and other backends too, which can misreport the actual failing component.</violation>
</file>

<file name="apps/api/src/rate-limit/index.ts">

<violation number="1" location="apps/api/src/rate-limit/index.ts:157">
P1: The `rateLimitHealth` error path is unreachable. Since `getBackend()` returns the fail-open-wrapped backend, errors from the underlying backend are swallowed by the wrapper and never reach this try-catch. The health probe will always report `ok: true`, defeating its purpose of detecting backend failures.

Consider either storing a reference to the unwrapped backend for health checks, or adding a dedicated `ping()`/`health()` method to the `RateLimitBackend` interface that the fail-open wrapper passes through without catching.</violation>
</file>

<file name="apps/api/src/rate-limit/client-id.ts">

<violation number="1" location="apps/api/src/rate-limit/client-id.ts:27">
P2: Prioritizing `x-forwarded-for` before `cf-connecting-ip` makes client identity easy to spoof and can bypass rate-limit enforcement.</violation>
</file>

_{Tip: cubic can generate docs of your entire codebase and keep them up to date. Try it here.}

[cubic-dev-ai]

P1: The rateLimitHealth error path is unreachable. Since getBackend() returns the fail-open-wrapped backend, errors from the underlying backend are swallowed by the wrapper and never reach this try-catch. The health probe will always report ok: true, defeating its purpose of detecting backend failures.

Consider either storing a reference to the unwrapped backend for health checks, or adding a dedicated ping()/health() method to the RateLimitBackend interface that the fail-open wrapper passes through without catching.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/rate-limit/index.ts, line 157:

<comment>The `rateLimitHealth` error path is unreachable. Since `getBackend()` returns the fail-open-wrapped backend, errors from the underlying backend are swallowed by the wrapper and never reach this try-catch. The health probe will always report `ok: true`, defeating its purpose of detecting backend failures.

Consider either storing a reference to the unwrapped backend for health checks, or adding a dedicated `ping()`/`health()` method to the `RateLimitBackend` interface that the fail-open wrapper passes through without catching.</comment>

<file context>
@@ -0,0 +1,173 @@
+}> {
+  const backend = getBackend();
+  try {
+    await backend.check({ bucket: "read", clientId: "__health__" });
+    return { backend: backend.name, ok: true };
+  } catch (error) {
</file context>

[cubic-dev-ai]

P2: Don't truncate the generated request sequence to the shorter auxiliary array; it can skip backend failures at the tail of the run.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/rate-limit/rate-limit.pbt.test.ts, line 182:

<comment>Don't truncate the generated request sequence to the shorter auxiliary array; it can skip backend failures at the tail of the run.</comment>

<file context>
@@ -0,0 +1,217 @@
+          // Align lengths: if the generator produced mismatched
+          // lengths, truncate to the shorter so every failure bit has
+          // a matching client id.
+          const n = Math.min(failures.length, clientIndices.length);
+          const trimmedFailures = failures.slice(0, n);
+          const clientIds = clientIndices
</file context>

[cubic-dev-ai]

P2: The fail-open log message is backend-specific (upstash unavailable) even though this wrapper is used for memory and other backends too, which can misreport the actual failing component.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/rate-limit/fail-open.ts, line 52:

<comment>The fail-open log message is backend-specific (`upstash unavailable`) even though this wrapper is used for memory and other backends too, which can misreport the actual failing component.</comment>

<file context>
@@ -0,0 +1,137 @@
+} from "./types.js";
+
+const LOG_WINDOW_MS = 60_000;
+const LOG_MESSAGE = "[rate-limit] upstash unavailable";
+
+export interface FailOpenOptions {
</file context>

[cubic-dev-ai]

P2: Prioritizing x-forwarded-for before cf-connecting-ip makes client identity easy to spoof and can bypass rate-limit enforcement.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At apps/api/src/rate-limit/client-id.ts, line 27:

<comment>Prioritizing `x-forwarded-for` before `cf-connecting-ip` makes client identity easy to spoof and can bypass rate-limit enforcement.</comment>

<file context>
@@ -0,0 +1,70 @@
+const FALLBACK = "local";
+
+export function resolveClientId(headers: HeaderLookup): string {
+  const xff = readHeader(headers, XFF_HEADER);
+  if (xff) {
+    const leftMost = xff.split(",")[0]?.trim();
</file context>

[devin-ai-integration]

Devin Review found 1 potential issue.

View 6 additional findings in Devin Review.

[devin-ai-integration]

🟡 rateLimitHealth() can never detect backend failures because it probes through the fail-open wrapper

rateLimitHealth() calls getBackend() which always returns a wrapFailOpen()-wrapped backend (every path in selectBackend() at apps/api/src/rate-limit/index.ts:53-66 wraps with wrapFailOpen()). The fail-open wrapper's check() (apps/api/src/rate-limit/fail-open.ts:107-128) catches all inner backend errors and returns a synthetic allow decision. Consequently, the catch block at line 159 is dead code — in production, rateLimitHealth() always returns { ok: true }, and the error?: string field in the return type is never populated. The function cannot fulfill its stated purpose as a health probe for backend connectivity.

Prompt for agents

The rateLimitHealth() function in apps/api/src/rate-limit/index.ts is designed to probe the rate-limit backend's health, but it calls getBackend() which returns the fail-open-wrapped backend. The fail-open wrapper (apps/api/src/rate-limit/fail-open.ts) catches all errors from the inner backend and returns a synthetic allow decision, so rateLimitHealth() can never observe an error — it always returns ok: true.

To fix this, rateLimitHealth() needs to bypass the fail-open wrapper and probe the raw inner backend directly. Possible approaches:
1. Store the unwrapped backend alongside the wrapped one in a module-level variable, and have rateLimitHealth() call the unwrapped backend's check() directly.
2. Extend the RateLimitBackend interface or the fail-open wrapper to expose a healthCheck() method or an unwrap() accessor.
3. Have wrapFailOpen expose the last-error state so rateLimitHealth() can inspect it without re-probing.

---

Was this helpful? React with 👍 or 👎 to provide feedback.