refactor(security): centralize rate-limit config into declarative policies (#366)

[Vyacheslav-Tomashevskiy]

Closes #366

What this changes

Every rate-limited route carried its own copy of the same four things: a limit, a window, a 429 message, and this line:

const ip = request.headers.get("x-forwarded-for")?.split(",")[0]?.trim() || "unknown";

Six routes, six copies (login, register, forgot-password, reset-password, analytics, stellar/send). This moves the config into src/lib/security.ts as declarative policies, exactly as the issue asks:

export const RATE_LIMIT_POLICIES = {
  login:              { limit: 10, windowMs: 60_000, scope: "ip",   message: "Too many attempts. Please try again later." },
  register:           { limit:  5, windowMs: 60_000, scope: "ip",   message: "Too many registration attempts. Please try again later." },
  "forgot-password":  { limit:  3, windowMs: 60_000, scope: "ip",   message: "Too many requests. Please try again later." },
  "reset-password":   { limit:  5, windowMs: 60_000, scope: "ip",   message: "Too many attempts. Please try again later." },
  analytics:          { limit: 60, windowMs: 60_000, scope: "ip",   message: "Too many requests. Please try again later." },
  "stellar-send":     { limit: 20, windowMs: 60_000, scope: "user", message: "Too many send requests. Please try again later." },
};

Each route is now two lines:

const limited = enforceRateLimit(request, "login");
if (limited) return limited;

All six limits, windows and messages are carried over unchanged — this refactor is behaviour-preserving except for the two fixes below.

Two defects the duplication was hiding

1. The rate limits could be bypassed outright. X-Forwarded-For is a list the client can seed — proxies only append to it. Reading split(",")[0] therefore reads an attacker-supplied value, so a caller gets a brand-new bucket on every request just by changing the header. Reproduced against origin/main's exact logic:

origin/main: 100 login attempts from ONE caller -> 100 allowed (limit is 10), buckets created: 100

That applies to all five IP-scoped endpoints, including login brute-force and forgot-password email spam.

getClientIp() now prefers x-real-ip (proxy-set, single-valued, cannot be appended to) and otherwise reads the entry RATE_LIMIT_TRUSTED_PROXY_HOPS from the right — the address the nearest trusted proxy actually saw. Default is 1 (one reverse proxy / platform edge, which is what Vercel provides); documented in .env.example for setups that stack a CDN in front of their own proxy.

2. The bucket map never shrank. rateBuckets entries were only ever overwritten, never deleted, so every distinct key leaked — and per defect 1 those keys were free for a caller to generate. Expired buckets are now swept, with a MAX_RATE_BUCKETS cap (10k) that evicts the entries closest to expiry if the sweep isn't enough.

Smaller things

• 429 responses now carry a Retry-After header. The value was already computed (retryAfterMs) and thrown away.
• /api/analytics returned a bare { success: false } on 429; it now uses errorResponse() like the other five, per the response-shape criterion. Additive — the success field is unchanged.
• When no address can be resolved (no proxy headers at all), callers share one bucket. That's deliberately fail-closed — the alternative is silently disabling the limit — but it does throttle unrelated callers, so it logs a rate_limit_unresolved_ip security event once per process instead of failing quietly.
• rateLimit() is still exported with its original signature; nothing that used it directly has to change.

Tests

src/lib/__tests__/rate-limit.test.ts, 15 tests, all green:

• getClientIp: prefers x-real-ip; ignores a client-seeded leftmost entry; single-entry header; padding/empty entries; returns null when no header
• enforceRateLimit: allows exactly limit then blocks (happy path + failure path); 429 body matches the api-response shape and carries Retry-After; rotating X-Forwarded-For no longer mints fresh buckets (regression test for defect 1); different callers stay isolated; policies stay independent for the same caller; user-scoped policies key by user id and not by address; throws if a user-scoped policy is used without a subject; unresolved-address callers are still limited
• bucket store: stays bounded under a flood of distinct keys; every declared policy is usable

Verification

• npx vitest run — 6 test files green / 79 tests pass. The 2 failures in validations.test.ts are pre-existing on main (a bad-checksum test address in stellarSendSchema), unrelated to this change.
• npm run build — ✓ Compiled successfully. The type-check step stops on src/lib/validations.ts:40: Cannot find name 'isValidStellarPublicKey', which is the known missing import already tracked in fix(send): validate the Stellar address checksum client-side (closes #424) #527/fix(validations): restore broken isValidStellarPublicKey import; feat: add /api/stellar/fee-estimate #529 and present on a clean main. With that import restored locally the full type check passes with no new errors; the import was not committed here.
• npx eslint on all seven touched files — clean.
• Live npm run dev + curl against /api/auth/forgot-password (limit 3), rotating the spoofed leftmost X-Forwarded-For on every request while keeping the same real client:

req1 -> HTTP 500   (passes the limiter, reaches the DB)
req2 -> HTTP 500   retry-after: 60
req3 -> HTTP 429   {"success":false,"error":"Too many requests. Please try again later."}   retry-after: 60
req4 -> HTTP 429   ...

A different real client and an x-real-ip caller both still passed the limiter while that bucket was exhausted, confirming isolation.

Note on scope

#365 asks for a rate limit on /api/stellar/submit, which still has none. I deliberately left that alone so it stays that issue's change — with this in place it's one policy entry plus the two-line call.

[vercel]

Someone is attempting to deploy a commit to the codex723's projects Team on Vercel.

A member of the Team first needs to authorize it.