Release: v0.2.3

apps/api/src/app.ts

@@ -13,8 +13,9 @@
  *                             is built from the post-reconciliation tree)
  *  6b. push-daemon plugin  → starts gitsheets push daemon
  *  6c. services plugin     → builds in-memory state + FTS
- *  7. rate-limit plugin    → in-memory counters keyed per-IP + per-account
- *  8. idempotency plugin   → in-memory map keyed by personId+key
+ *  7. session middleware   → parses the JWT cookie into request.session
+ *  8. rate-limit plugin    → in-memory counters keyed per-IP + per-account
+ *  8a. idempotency plugin  → in-memory map keyed by personId+key
  *  9. @fastify/swagger      → OpenAPI 3.1 doc generation
  * 10. @fastify/swagger-ui   → Swagger UI at /api/_docs
  * 11. routes               → registered last after all plumbing
@@ -151,15 +152,17 @@ export async function buildApp(opts: BuildAppOptions = {}): Promise<FastifyInsta
   await fastify.register(slugRedirectPlugin);
   await fastify.register(legacyRedirectPlugin);
 
-  // ----- 7. Rate limiting -----
+  // ----- 7. Session middleware (JWT auth) -----
+  // Before rate-limit and idempotency: all three use onRequest hooks, which run
+  // in registration order, and the other two key on request.session.person.
+  await fastify.register(sessionMiddlewarePlugin);
+
+  // ----- 8. Rate limiting -----
   await fastify.register(rateLimitPlugin);
 
-  // ----- 8. Idempotency -----
+  // ----- 8a. Idempotency -----
   await fastify.register(idempotencyPlugin);
 
-  // ----- 8a. Session middleware (JWT auth) -----
-  await fastify.register(sessionMiddlewarePlugin);
-
   // ----- 9-10. OpenAPI / Swagger UI -----
   await fastify.register(fastifySwagger, {
     openapi: {

apps/api/src/plugins/rate-limit.ts

@@ -2,10 +2,18 @@
  * In-memory rate-limit plugin.
  *
  * Enforces per-IP and per-account caps per specs/api/conventions.md#rate-limiting:
- *   - Unauthenticated reads: 60 req / min / IP
- *   - Authenticated reads:  300 req / min / account
- *   - Writes:                30 req / min / account
- *   - Auth endpoints:        10 req / min / IP
+ *   - Unauthenticated reads:  1200 req / min / IP
+ *   - Authenticated reads:     300 req / min / account
+ *   - Writes:                   30 req / min / account (120 / min / IP anonymous)
+ *   - Credential endpoints:    120 req / min / IP
+ *
+ * Only `/api/**` is counted. The SPA shell, its assets, and thumbnails come out
+ * of the same process and a single page load fetches dozens of them; counting
+ * those against the read cap is what produced a site-wide 429 storm at cutover.
+ *
+ * Per-IP caps are generous on purpose: production's load balancer does not yet
+ * preserve client addresses (cfp-live-cluster #201), so every visitor shares
+ * one "IP" until that lands.
  *
  * Counters are reset on restart (intentional — single replica, civic scale).
  * Exceeded limit → RateLimitedError(retryAfterSeconds).
@@ -23,6 +31,14 @@ interface BucketEntry {
 
 const WINDOW_MS = 60_000; // 1 minute
 
+export const RATE_LIMITS = {
+  unauthenticatedReadsPerIp: 1200,
+  authenticatedReadsPerAccount: 300,
+  writesPerAccount: 30,
+  anonymousWritesPerIp: 120,
+  credentialPerIp: 120,
+} as const;
+
 function getOrCreate(map: Map<string, BucketEntry>, key: string): BucketEntry {
   let entry = map.get(key);
   if (!entry) {
@@ -60,37 +76,60 @@ function clientIp(request: FastifyRequest): string {
 }
 
 const WRITE_METHODS = new Set(['POST', 'PUT', 'PATCH', 'DELETE']);
-const AUTH_PATH_PREFIX = '/api/auth';
+const API_PREFIX = '/api/';
+
+/**
+ * Routes that accept or mint a credential. Session reads (`/api/auth/me`,
+ * `/api/auth/refresh`, `/api/auth/sessions`, `/api/auth/logout`) are ordinary
+ * traffic — `/me` runs on every page load.
+ */
+const CREDENTIAL_PATHS = [
+  '/api/auth/login',
+  '/api/auth/github/start',
+  '/api/auth/github/callback',
+  '/api/auth/link-github',
+  '/api/auth/password-reset/',
+  '/api/account-claim/by-password',
+];
+
+function pathOf(url: string): string {
+  const q = url.indexOf('?');
+  return q === -1 ? url : url.slice(0, q);
+}
+
+export function isCredentialPath(path: string): boolean {
+  return CREDENTIAL_PATHS.some((p) => (p.endsWith('/') ? path.startsWith(p) : path === p));
+}
 
 async function rateLimitPlugin(fastify: FastifyInstance): Promise<void> {
   const ipBuckets = new Map<string, BucketEntry>();
   const accountBuckets = new Map<string, BucketEntry>();
 
   fastify.addHook('onRequest', (request, _reply, done) => {
     try {
+      const path = pathOf(request.url);
+      if (!path.startsWith(API_PREFIX)) {
+        done();
+        return;
+      }
+
       const ip = clientIp(request);
       const isWrite = WRITE_METHODS.has(request.method);
-      const isAuthEndpoint = request.url.startsWith(AUTH_PATH_PREFIX);
-
       const personId = request.session?.person?.id;
 
-      if (isAuthEndpoint) {
-        // Auth endpoints: 10 req / min / IP
-        check(ipBuckets, `auth:${ip}`, 10);
+      if (isCredentialPath(path)) {
+        check(ipBuckets, `credential:${ip}`, RATE_LIMITS.credentialPerIp);
       } else if (isWrite) {
         if (personId) {
-          // Authenticated writes: 30 req / min / account
-          check(accountBuckets, `write-account:${personId}`, 30);
+          check(accountBuckets, `write-account:${personId}`, RATE_LIMITS.writesPerAccount);
         } else {
-          check(ipBuckets, `write:${ip}`, 30);
+          check(ipBuckets, `write:${ip}`, RATE_LIMITS.anonymousWritesPerIp);
         }
       } else {
         if (personId) {
-          // Authenticated reads: 300 req / min / account
-          check(accountBuckets, `account:${personId}`, 300);
+          check(accountBuckets, `account:${personId}`, RATE_LIMITS.authenticatedReadsPerAccount);
         } else {
-          // Unauthenticated reads: 60 req / min / IP
-          check(ipBuckets, `read:${ip}`, 60);
+          check(ipBuckets, `read:${ip}`, RATE_LIMITS.unauthenticatedReadsPerIp);
         }
       }
     } catch (err) {

apps/api/tests/api-skeleton.test.ts

@@ -6,7 +6,7 @@
  *  - ValidationError surfaces as 422 validation_failed with expected shape
  *  - Unknown Error surfaces as 500 internal_error with no message leak
  *  - traceId appears in error responses
- *  - Per-IP rate limit: 61 anonymous reads → 429 with Retry-After
+ *  - Per-IP rate limit: one read past the cap → 429 with Retry-After
  *  - Idempotency-Key: repeat POST returns cached response
  *  - /api/_openapi.json returns a valid OpenAPI 3.1 document
  *  - /api/_docs renders (200 response)
@@ -16,6 +16,7 @@ import { afterEach, beforeEach, describe, expect, it } from 'vitest';
 
 import type { FastifyInstance } from 'fastify';
 import { buildApp } from '../src/app.js';
+import { RATE_LIMITS } from '../src/plugins/rate-limit.js';
 import { createFullDataRepo, createPrivateStorageDir } from './helpers/test-full-repo.js';
 
 // ---------------------------------------------------------------------------
@@ -169,18 +170,19 @@ describe('/api/_test/* route gating', () => {
 // ---------------------------------------------------------------------------
 
 describe('rate limiting', () => {
-  it('61 anonymous reads from the same IP → 429 with Retry-After on the 61st', async () => {
-    // Make 60 reads — all should succeed
-    for (let i = 0; i < 60; i++) {
-      const res = await app!.inject({
-        method: 'GET',
-        url: '/api/health',
-        remoteAddress: '10.0.0.1',
-      });
-      expect(res.statusCode, `Request ${i + 1} should succeed`).toBe(200);
-    }
+  it('one read past the per-IP cap → 429 with Retry-After', async () => {
+    // Prime the bucket to the cap rather than issuing 1200 requests; the last
+    // in-cap read must still succeed.
+    const limit = RATE_LIMITS.unauthenticatedReadsPerIp;
+    app!.rateLimitBuckets.ip.set('read:10.0.0.1', { count: limit - 1, windowStart: Date.now() });
+    const okRes = await app!.inject({
+      method: 'GET',
+      url: '/api/health',
+      remoteAddress: '10.0.0.1',
+    });
+    expect(okRes.statusCode).toBe(200);
 
-    // The 61st should be rate-limited
+    // The next one should be rate-limited
     const res = await app!.inject({
       method: 'GET',
       url: '/api/health',

apps/api/tests/auth.test.ts

@@ -22,6 +22,7 @@ import { type FastifyInstance } from 'fastify';
 import { SignJWT } from 'jose';
 
 import { buildApp } from '../src/app.js';
+import { RATE_LIMITS } from '../src/plugins/rate-limit.js';
 import { mintSessionFor } from '../src/auth/issue.js';
 import { verifyAccess, verifyRefresh } from '../src/auth/jwt.js';
 import { createFullDataRepo, createPrivateStorageDir } from './helpers/test-full-repo.js';
@@ -499,12 +500,13 @@ describe('account-based rate limits', () => {
   it('authenticated reads key on account bucket (300/min), separate from IP bucket', async () => {
     const { accessToken } = await mintSessionFor(personId, 'user', JWT_KEY);
 
-    // Exhaust the IP bucket with anonymous reads (60 limit)
-    for (let i = 0; i < 60; i++) {
-      await app.inject({ method: 'GET', url: '/api/health', remoteAddress: '10.99.0.1' });
-    }
+    // Exhaust the IP bucket (prime it to the cap rather than issuing 1200 reads)
+    app.rateLimitBuckets.ip.set('read:10.99.0.1', {
+      count: RATE_LIMITS.unauthenticatedReadsPerIp,
+      windowStart: Date.now(),
+    });
 
-    // 61st anonymous request → 429
+    // Next anonymous request → 429
     const anonRes = await app.inject({
       method: 'GET',
       url: '/api/health',

apps/api/tests/rate-limit.test.ts

@@ -0,0 +1,114 @@
+/**
+ * Rate limiter scope — specs/api/conventions.md#rate-limiting.
+ *
+ * Covers what broke at cutover:
+ *  - non-/api requests (SPA shell, assets, thumbnails) are never counted
+ *  - GET /api/auth/me is an ordinary read, not a credential call
+ *  - credential endpoints share one per-IP bucket at RATE_LIMITS.credentialPerIp
+ *  - the unauthenticated read cap still trips one past RATE_LIMITS.unauthenticatedReadsPerIp
+ */
+import { afterEach, beforeEach, describe, expect, it } from 'vitest';
+import type { FastifyInstance } from 'fastify';
+import { buildApp } from '../src/app.js';
+import { RATE_LIMITS, isCredentialPath } from '../src/plugins/rate-limit.js';
+import { createFullDataRepo, createPrivateStorageDir } from './helpers/test-full-repo.js';
+
+let dataRepo: { path: string; cleanup: () => Promise<void> };
+let privateStore: { path: string; cleanup: () => Promise<void> };
+let app: FastifyInstance | undefined;
+
+const IP = '203.0.113.7';
+
+beforeEach(async () => {
+  dataRepo = await createFullDataRepo();
+  privateStore = await createPrivateStorageDir();
+  app = await buildApp({
+    serverOptions: { logger: false },
+    overrideEnv: {
+      CFP_DATA_REPO_PATH: dataRepo.path,
+      STORAGE_BACKEND: 'filesystem',
+      CFP_PRIVATE_STORAGE_PATH: privateStore.path,
+      CFP_JWT_SIGNING_KEY: 'test-jwt-signing-key-at-least-32-chars!!',
+      NODE_ENV: 'test',
+    },
+  });
+});
+
+afterEach(async () => {
+  if (app) {
+    await app.close();
+    app = undefined;
+  }
+  await dataRepo.cleanup();
+  await privateStore.cleanup();
+});
+
+function bucket(key: string): number {
+  return app!.rateLimitBuckets.ip.get(key)?.count ?? 0;
+}
+
+describe('rate limiter scope', () => {
+  it('does not count non-/api requests (SPA shell, assets, thumbnails)', async () => {
+    for (const url of ['/', '/login', '/assets/index-abc123.js', '/thumbnail/1/100x100']) {
+      const res = await app!.inject({ method: 'GET', url, headers: { 'x-forwarded-for': IP } });
+      expect(res.statusCode).not.toBe(429);
+    }
+    expect(app!.rateLimitBuckets.ip.size).toBe(0);
+  });
+
+  it('treats GET /api/auth/me as a read, not a credential call', async () => {
+    const res = await app!.inject({
+      method: 'GET',
+      url: '/api/auth/me',
+      headers: { 'x-forwarded-for': IP },
+    });
+    expect(res.statusCode).toBe(200);
+    expect(bucket(`read:${IP}`)).toBe(1);
+    expect(bucket(`credential:${IP}`)).toBe(0);
+  });
+
+  it('classifies credential endpoints', () => {
+    for (const p of [
+      '/api/auth/login',
+      '/api/auth/github/start',
+      '/api/auth/github/callback',
+      '/api/auth/link-github',
+      '/api/auth/password-reset/request',
+      '/api/auth/password-reset/confirm',
+      '/api/account-claim/by-password',
+    ]) {
+      expect(isCredentialPath(p), p).toBe(true);
+    }
+    for (const p of ['/api/auth/me', '/api/auth/refresh', '/api/auth/sessions', '/api/auth/logout', '/api/people']) {
+      expect(isCredentialPath(p), p).toBe(false);
+    }
+  });
+
+  it('caps credential endpoints per IP at RATE_LIMITS.credentialPerIp', async () => {
+    const limit = RATE_LIMITS.credentialPerIp;
+    let last = 0;
+    for (let i = 0; i < limit + 1; i++) {
+      const res = await app!.inject({
+        method: 'POST',
+        url: '/api/auth/login',
+        headers: { 'x-forwarded-for': IP, 'content-type': 'application/json' },
+        payload: {},
+      });
+      last = res.statusCode;
+      if (i < limit) expect(res.statusCode).not.toBe(429);
+    }
+    expect(last).toBe(429);
+    expect(bucket(`read:${IP}`)).toBe(0);
+  });
+
+  it('caps unauthenticated reads per IP one past RATE_LIMITS.unauthenticatedReadsPerIp', async () => {
+    const limit = RATE_LIMITS.unauthenticatedReadsPerIp;
+    // Prime the bucket directly rather than issuing 1200 requests.
+    app!.rateLimitBuckets.ip.set(`read:${IP}`, { count: limit - 1, windowStart: Date.now() });
+    const ok = await app!.inject({ method: 'GET', url: '/api/health', headers: { 'x-forwarded-for': IP } });
+    expect(ok.statusCode).toBe(200);
+    const over = await app!.inject({ method: 'GET', url: '/api/health', headers: { 'x-forwarded-for': IP } });
+    expect(over.statusCode).toBe(429);
+    expect(over.headers['retry-after']).toBeDefined();
+  });
+});

plans/rate-limit-scope.md

@@ -0,0 +1,56 @@
+---
+status: done
+depends: [saml-login-return-path]
+specs:
+  - specs/api/conventions.md
+  - specs/api/auth.md
+issues: []
+pr: 179
+---
+
+# Plan: scope and loosen the rate limiter
+
+## Scope
+
+Minutes after the 2026-09-17 cutover, visitors (including the Slack SAML
+test) started getting `429 rate_limited`. Three compounding causes:
+
+1. The limiter's `onRequest` hook counted **every** request — SPA shell,
+   JS/CSS chunks, thumbnails — against the 60/min unauthenticated read cap.
+   One page load is dozens of requests.
+2. `GET /api/auth/me`, which the SPA calls on every page load, lived in the
+   10/min `/api/auth/*` bucket alongside the credential endpoints.
+3. Production's NodeBalancer does not preserve client addresses, so Envoy —
+   and therefore `X-Forwarded-For` — sees one address for all visitors. Every
+   per-IP bucket is effectively site-wide (cfp-live-cluster #201).
+
+In: spec the scoping and the new caps, implement, test. Out: PROXY protocol
+on the NodeBalancer (infra, tracked separately); tightening the credential
+cap back down once real client IPs arrive.
+
+## Implements
+
+- [api/conventions.md](../specs/api/conventions.md) — `## Rate limiting`:
+  `/api/**` only; credential endpoints enumerated; new caps and the
+  shared-address caveat.
+- [api/auth.md](../specs/api/auth.md) — the 429 line under GitHub start
+  references the credential-endpoint cap instead of a stale number.
+
+## Approach
+
+- `apps/api/src/plugins/rate-limit.ts`: early return for non-`/api/` paths;
+  `isCredentialPath()` replaces the `/api/auth` prefix test; caps in an
+  exported `RATE_LIMITS` table so tests assert against the source of truth.
+- Tests: assets and the SPA fallthrough are uncounted; `/api/auth/me` is a
+  read, not a credential call; credential paths share one IP bucket; the read
+  cap still trips at `RATE_LIMITS.unauthenticatedReadsPerIp + 1`.
+
+## Validation
+
+- `npm run type-check && npm run lint && npm test` clean.
+- After deploy: no 429s in the pod log during normal browsing; Slack SAML test
+  completes.
+
+## Follow-ups
+
+- Tracked as: cfp-live-cluster #201 (PROXY protocol so per-IP is per visitor).