#587 Add tenant-scoped rate limit for /bookings/search using leaky-bu…

.env.example

@@ -24,3 +24,11 @@ IDEMPOTENCY_REDIS_ENCRYPTION_ACTIVE_KEY_ID=
 IDEMPOTENCY_REDIS_ENCRYPTION_ACTIVE_KEY=
 # Comma-separated key-id:base64-key entries kept temporarily for decryption during rotation.
 IDEMPOTENCY_REDIS_ENCRYPTION_PREVIOUS_KEYS=
+
+# Per-tenant leaky-bucket rate limiting for /api/v1/bookings/search.
+# Sustained requests per second allowed per tenant (default 60).
+BOOKINGS_SEARCH_RATE_PER_SECOND=60
+# Maximum burst capacity per tenant (default 120).
+BOOKINGS_SEARCH_BURST=120
+# Hard timeout for the rate-limiter Redis call before failing open (default 250 ms).
+BOOKINGS_SEARCH_REDIS_TIMEOUT_MS=250

docs/api/bookings-search-test-notes.md

@@ -0,0 +1,63 @@
+# Test notes — per-tenant leaky-bucket for /bookings/search
+
+Captured on branch `feat/tenant-leaky-bucket-search`.
+Command per `package.json`: `npm test` (jest, `--runInBand`).
+
+## New suites (this change)
+
+```
+PASS src/middleware/__tests__/tenantLeakyBucket.redis.test.ts
+PASS src/middleware/__tests__/tenantLeakyBucket.test.ts
+PASS src/routes/__tests__/bookings.test.ts
+Test Suites: 3 passed, 3 total
+Tests:       62 passed, 62 total
+```
+
+Coverage (scoped to the two new modules, `--collectCoverageFrom`):
+
+| File | Stmts | Branch | Funcs | Lines |
+| --- | --- | --- | --- | --- |
+| src/middleware/tenantLeakyBucket.ts | 100% | 97.14% | 100% | 100% |
+| src/routes/bookings.ts | 100% | 98.27% | 100% | 100% |
+| **All files** | **100%** | **97.65%** | **100%** | **100%** |
+
+Residual uncovered branches are defensive by design: the production
+`require("ioredis")` fallback (unreachable under test injection) and a
+null-guard for `auth.userId` that the auth middleware makes unreachable over
+HTTP. Both exceed the 95% requirement.
+
+## Edge cases from the issue — where each is covered
+
+| Required edge case | Test |
+| --- | --- |
+| Burst then sustained | `tenantLeakyBucket.test.ts` → “burst then sustained: 120-request burst passes, 121st is throttled, bucket refills at 60 rps” (also E2E in `bookings.test.ts`) |
+| Tenant switch mid-connection | `tenantLeakyBucket.test.ts` → “tenant switch mid-connection charges the correct bucket each request” |
+| Redis latency spike | `tenantLeakyBucket.test.ts` → “enforces the timeout on a latency spike instead of hanging” + “a slow store fails open within the timeout and the response completes” |
+| Noisy tenant can't starve others | `tenantLeakyBucket.test.ts` → “noisy tenant cannot starve other tenants”; `bookings.test.ts` → E2E |
+| Atomicity (Lua on Redis) | `tenantLeakyBucket.redis.test.ts` → Lua ↔ JS parity on a real Lua VM (ioredis-mock): burst, throttle, drain, clock skew, EVALSHA caching, NOSCRIPT fallback |
+| 429 + Retry-After | middleware + route tests assert header and body contract |
+| Metric | `rate_limit_bucket_burn{tenant}` set on every request; `rate_limit_redis_failures_total` on fail-open |
+
+## Full-suite regression check (`npm test`)
+
+The repository's `main` is already heavily broken upstream (654 failing tests
+across 175 suites: bad upstream commits with syntax errors in
+`marketplaceSearchSchema.ts` / `marketplaceSearchService.ts`, missing exports
+like `FraudReasonCode`, undefined helpers like `resetSeniorPool`, etc.). To
+compare like-for-like, all 257 suites were executed in chunks on both
+`origin/main` and this branch in an otherwise memory-constrained sandbox (a
+single monolithic `--runInBand` process OOMs after ~175 suites even on main).
+
+Result:
+
+- Failing suites on `main`: 175
+- Failing suites on this branch: 171 (all also failing on `main`)
+- Suites failing **only** on this branch (regressions): **0**
+- Two suites (`reranker`, `db-instrumentation`) failed on `main`'s chunk run
+  but pass when re-run individually on `main` → flaky pre-existing tests,
+  unrelated to this change.
+
+`tsc --noEmit`: 7 errors before and after the change — all pre-existing
+syntax errors on `main`; this change adds zero compile errors. ESLint: clean
+for every new/modified file (2 pre-existing unused-function errors in
+`src/config/env.ts` remain on `main`, untouched).

docs/api/bookings-search.md

@@ -0,0 +1,171 @@
+# Bookings Search API — `/api/v1/bookings/search`
+
+Search endpoint for booking records, protected by a **per-tenant leaky-bucket
+rate limiter** (60 rps sustained, 120 burst) instead of the coarse global
+fixed-window limiter used elsewhere. This document covers the endpoint itself
+and the rate-limiting behavior clients and operators must understand.
+
+Implementation:
+
+| Piece | Location |
+| --- | --- |
+| Router | `src/routes/bookings.ts` |
+| Per-tenant leaky-bucket middleware | `src/middleware/tenantLeakyBucket.ts` |
+| Metrics | `src/metrics.ts` (`rate_limit_bucket_burn`, `rate_limit_redis_failures_total`) |
+| Config | `src/config/env.ts` (`BOOKINGS_SEARCH_*`) |
+
+---
+
+## Why per-tenant instead of the global limiter?
+
+The previous configuration shared one fixed-window budget across traffic. A
+single misbehaving tenant (retry storm, broken sync loop, aggressive scraper)
+could exhaust the window and starve every other tenant — the classic *noisy
+neighbor* failure. The per-tenant leaky bucket gives **each tenant its own
+independent budget**, so an abusive tenant can only ever throttle itself:
+
+```
+tenant A: [████████████████████████████] 429 — throttled, burns alone
+tenant B: [█░░░░░░░░░░░░░░░░░░░░░░░░] 200 — unaffected
+```
+
+## Algorithm
+
+Leaky bucket, evaluated atomically inside Redis (Lua script):
+
+- Each admitted request adds **1 token** to the caller's bucket.
+- The bucket **drains at a constant rate**: `BOOKINGS_SEARCH_RATE_PER_SECOND`
+  tokens/second (default **60**).
+- Requests are admitted while `level + 1 <= BOOKINGS_SEARCH_BURST` (default
+  **120**).
+
+Consequences:
+
+- **Burst:** up to 120 requests may land simultaneously.
+- **Sustained:** long-run throughput converges to exactly 60 rps.
+- **Recovery:** after throttling, capacity returns continuously at 60 rps
+  (no hard window boundary, so no thundering-herd at window reset).
+
+The Lua script runs via `EVALSHA` with an automatic `EVAL` fallback on
+`NOSCRIPT`, so the read-modify-write of bucket state is atomic across all app
+instances — concurrent requests cannot double-spend capacity.
+
+### Redis state
+
+- Key: `rlb:bookings:search:<principal-type>:<id>` (hash `{level, ts}`).
+- TTL: `2 × (burst / rate) + 1` seconds (5 s with defaults), refreshed on
+  every request. Buckets of quiet tenants expire automatically — no memory
+  leak from one-off tenants.
+
+## Tenant identity resolution
+
+**Tenant keys are derived from trusted auth context only.** Resolution order:
+
+1. `req.auth.tenantId` / `req.user.tenantId` — true tenant claims.
+2. `req.auth.userId` / `req.user.sub || req.user.id` — the user is the tenant boundary.
+3. `req.apiKeyId` — partner API keys map 1:1 to tenants (SHA-256 hashed).
+4. Client IP (SHA-256 hashed) — anonymous traffic gets its own bucket and
+   cannot starve authenticated tenants.
+
+> [!IMPORTANT]
+> The `x-tenant-id` **request header is never consulted**: trusting it would
+> let any caller mint a fresh, empty bucket per request (limit evasion) and
+> would flood Redis and Prometheus with unbounded keys. Identifiers are also
+> canonicalized (strict charset + 128-char cap, otherwise replaced by a
+> deterministic hash) before being embedded in keys, preventing Redis
+> key-injection across logical namespaces.
+
+## Headers
+
+Every response (admitted or rejected) includes:
+
+| Header | Meaning |
+| --- | --- |
+| `X-RateLimit-Limit` | Burst capacity (120). |
+| `X-RateLimit-Remaining` | Tokens remaining in the tenant's bucket right now. |
+| `X-RateLimit-Reset` | Epoch seconds when the bucket will have fully drained. |
+| `Retry-After` | **Only on 429.** Whole seconds until the request can be retried successfully. Computed from live bucket state, not a constant. |
+
+## Responses
+
+**Admitted**
+
+```json
+{ "success": true, "data": { "results": [], "total": 0, "limit": 50, "offset": 0 } }
+```
+
+**Throttled — HTTP 429**
+
+```json
+{ "success": false, "error": "Too many requests, please try again later.", "retryAfter": 1 }
+```
+
+Client guidance: respect `Retry-After`, add jitter, and never retry 429s in a
+tight loop — a retry storm only re-fills your own bucket with condemned
+requests.
+
+## Failure modes
+
+The limiter is **fail-open**: if Redis is unreachable or slower than
+`BOOKINGS_SEARCH_REDIS_TIMEOUT_MS` (default 250 ms), the request is admitted
+without accounting and `rate_limit_redis_failures_total` increments.
+
+Rationale: rate limiting exists to protect the endpoint; it must never take
+the endpoint down with it. During a Redis outage, traffic degrades to the
+pre-fix (unlimited) behavior and protection recovers automatically when Redis
+returns. If `rate_limit_redis_failures_total` is sustained non-zero, treat it
+as a production incident (Redis health), not as an application bug.
+
+A too-slow Redis additionally cannot hang request handling — every store call
+is wrapped in the hard `BOOKINGS_SEARCH_REDIS_TIMEOUT_MS` budget.
+
+## Metrics
+
+| Metric | Type | Labels | Meaning |
+| --- | --- | --- | --- |
+| `rate_limit_bucket_burn` | Gauge | `tenant` | Live bucket fill level per tenant. Rising ⇒ about to be throttled; pinned at 120 ⇒ being throttled. Cardinality-budgeted (256) — excess tenants fold into the shared overflow label instead of harming the metrics pipeline. |
+| `rate_limit_redis_failures_total` | Counter | — | Fail-open events (Redis error/timeout). Healthy steady state is 0. |
+
+## Configuration
+
+| Env var | Default | Purpose |
+| --- | --- | --- |
+| `BOOKINGS_SEARCH_RATE_PER_SECOND` | `60` | Sustained rps per tenant (leak rate). |
+| `BOOKINGS_SEARCH_BURST` | `120` | Per-tenant burst capacity. |
+| `BOOKINGS_SEARCH_REDIS_TIMEOUT_MS` | `250` | Hard budget for the limiter Redis call before failing open. |
+
+Tuning notes: keep `burst >= 2 × rate` so dashboards and batch syncs can
+burst naturally; raise `rate` only together with capacity planning on the
+search backing store.
+
+## Endpoint
+
+`GET /api/v1/bookings/search`
+
+Auth: `x-chronopay-user-id` + role header (`customer`, `professional`,
+`admin`, `support`) — callers only ever see **their own** bookings.
+
+| Query param | Type | Description |
+| --- | --- | --- |
+| `q` | string ≤ 200 chars | Case-insensitive match over id, slotId, professional, note. |
+| `status` | enum | `pending`, `confirmed`, `firm`, `cancelled`, `expired`, `hold_placed`, `hold_refunded`. |
+| `slotId` | string | Exact slot filter. |
+| `from`, `to` | ISO 8601 | Only bookings overlapping `[from, to]`. One-sided ranges allowed. `from` must not be after `to`. |
+| `limit` | int 1–100 (default 50) | Page size. |
+| `offset` | int ≥ 0 (default 0) | Page offset. |
+
+Validation failures return `400 { success: false, error }`.
+
+## Tests
+
+- `src/middleware/__tests__/tenantLeakyBucket.test.ts` — decision engine,
+  tenant resolution/security, middleware behavior, burst-then-sustained,
+  tenant switch mid-connection, fail-open, latency-spike timeout.
+- `src/middleware/__tests__/tenantLeakyBucket.redis.test.ts` — proves the
+  Lua script on a real Lua VM (`ioredis-mock`) behaves identically to the JS
+  decision engine (burst, throttle, drain, clock skew), plus EVALSHA caching
+  and store lifecycle.
+- `src/routes/__tests__/bookings.test.ts` — endpoint search/filter/pagination
+  contract and the end-to-end noisy-neighbor scenario.
+
+Coverage for both new modules: 100% lines / 100% functions / ≥97% branches.

src/app.ts

@@ -48,6 +48,7 @@ function isTruthyEnvValue(value: string | undefined): boolean {
 
 // Import routers
 import checkoutRouter from "./routes/checkout.js";
+import bookingsRouter from "./routes/bookings.js";
 import buyerProfileRouter from "./buyer-profile/buyer-profile.routes.js";
 import oauth2Router from "./routes/oauth2.js";
 import adminRouter from "./routes/admin.js";
@@ -640,6 +641,9 @@ export function createApp(options: AppFactoryOptions = {}) {
     },
   );
 
+  // 4b. Bookings Search Routes (per-tenant leaky-bucket: 60 rps sustained / 120 burst)
+  app.use("/api/v1/bookings", bookingsRouter);
+
   // 5. Webhooks Routes
   registerWebhookRoutes(app);
   app.use("/api/v1", webhookRoutes);

src/config/env.ts

@@ -45,6 +45,12 @@
   internalOverrideSecretPrev?: string;
   /** Acceptable clock skew (ms) for the bypass timestamp. Default 30 000. */
   internalBypassToleranceMs: number;
+  /** Sustained requests per second per tenant on /api/v1/bookings/search. Default 60. */
+  bookingsSearchRatePerSecond: number;
+  /** Burst capacity per tenant on /api/v1/bookings/search. Default 120. */
+  bookingsSearchBurst: number;
+  /** Hard timeout (ms) for the leaky-bucket Redis call before failing open. Default 250. */
+  bookingsSearchRedisTimeoutMs: number;
 }
 
 export class EnvValidationError extends Error {
@@ -91,6 +97,25 @@
     issues,
   );
 
+  const bookingsSearchRatePerSecond = parsePositiveInteger(
+    env.BOOKINGS_SEARCH_RATE_PER_SECOND,
+    "BOOKINGS_SEARCH_RATE_PER_SECOND",
+    60,
+    issues,
+  );
+  const bookingsSearchBurst = parsePositiveInteger(
+    env.BOOKINGS_SEARCH_BURST,
+    "BOOKINGS_SEARCH_BURST",
+    120,
+    issues,
+  );
+  const bookingsSearchRedisTimeoutMs = parsePositiveInteger(
+    env.BOOKINGS_SEARCH_REDIS_TIMEOUT_MS,
+    "BOOKINGS_SEARCH_REDIS_TIMEOUT_MS",
+    250,
+    issues,
+  );
+
   if (issues.length > 0) {
     throw new EnvValidationError(issues);
   }
@@ -113,6 +138,9 @@
     internalOverrideSecret,
     internalOverrideSecretPrev,
     internalBypassToleranceMs,
+    bookingsSearchRatePerSecond,
+    bookingsSearchBurst,
+    bookingsSearchRedisTimeoutMs,
   };
 }
 
@@ -260,7 +288,7 @@
   }
 }
 
 function parseReplicaId(rawValue: string | undefined): string {
   if (rawValue === undefined || rawValue.trim().length === 0) {
     // Fall back to the OS hostname so each pod/container gets a distinct ID
     // without requiring explicit config.
@@ -273,7 +301,7 @@
   return rawValue.trim();
 }
 
 function parseFloat01(rawValue: string | undefined, key: string, defaultValue: number, issues: string[]): number {
   if (rawValue === undefined) return defaultValue;
   const value = rawValue.trim();
   if (value.length === 0) {

src/metrics.ts

@@ -701,6 +701,38 @@ export const fairQueueBypassAttempts = createBudgetedCounter({
   registers: [register],
 });
 
+// ─── Per-tenant leaky-bucket rate limiter metrics ────────────────────────────
+
+/**
+ * Gauge of the current leaky-bucket fill level per tenant. Rising towards the
+ * configured burst capacity means the tenant is about to be throttled;
+ * a level pinned at capacity means the tenant is actively limited.
+ * Cardinality-budgeted so a flood of distinct tenants degrades to the
+ * shared overflow label rather than exhausting the metrics pipeline.
+ */
+export const rateLimitBucketBurn = createBudgetedGauge({
+  name: "rate_limit_bucket_burn",
+  help: "Current leaky-bucket fill level (tokens charged) per tenant on rate-limited search endpoints",
+  labels: ["tenant"],
+  budget: 256,
+  buckets: [],
+  registers: [register],
+});
+
+/**
+ * Counter incremented each time the per-tenant leaky-bucket limiter fails
+ * open because Redis errored or exceeded the store timeout. Zero is the
+ * healthy steady state; a sustained non-zero rate means rate limiting is
+ * currently degraded (requests allowed without accounting).
+ */
+export const rateLimitRedisFailuresTotal = createBudgetedCounter({
+  name: "rate_limit_redis_failures_total",
+  help: "Total number of times the tenant leaky-bucket limiter failed open due to Redis errors or timeouts",
+  labels: [],
+  budget: 0,
+  registers: [register],
+});
+
 /**
  * Histogram tracking per-request SQL wall-clock time in milliseconds.
  * Labels: route, outcome (ok | breached).