.convoys/cors-tighten.md

@@ -0,0 +1,242 @@
+---
+name: cors-tighten
+classification: convoy
+success_metric: |
+  No `pages/api/**/*.js` handler ships an `Access-Control-Allow-Origin: *`
+  header (or any other wildcard CORS header), AND no handler ships a
+  same-origin redundant OPTIONS preflight handler. Browser-issued
+  cross-origin POSTs to the auth surface return a CORS error instead of
+  succeeding. `npm run test:smoke` continues to pass (the smoke spec is
+  same-origin via the Vercel preview URL, so it is unaffected).
+skip:
+  - role-design-system-auditor
+  - role-a11y-auditor
+  - role-ux-reviewer
+  - role-ia-architect
+status: queued
+created: 2026-05-24
+parent: ship-readiness
+addresses: P0 #5 (PARTIAL → RESOLVED)
+depends_on:
+  - fix-auth-bypass (Brief 4, shipped — login + register CORS removal is the precedent)
+---
+
+# Convoy: cors-tighten
+
+Drop wildcard `Access-Control-Allow-Origin: *` from the remaining
+`pages/api/**` handlers. The `fix-auth-bypass` Brief 4 already
+cleaned login + register; the documented follow-up was just
+`pages/api/auth/verify.js`, but a fresh audit (parent grep at
+convoy creation, 2026-05-24) found **24 files** repo-wide carrying
+the identical scaffolded pattern:
+
+```js
+// Set CORS headers
+res.setHeader('Access-Control-Allow-Origin', '*');
+res.setHeader('Access-Control-Allow-Methods', '...');
+res.setHeader('Access-Control-Allow-Headers', 'Content-Type, Authorization');
+
+// Handle preflight requests
+if (req.method === 'OPTIONS') {
+  res.status(200).end();
+  return;
+}
+```
+
+The 24 files (spot-checked against `pages/api/cards/search.js`,
+`pages/api/collections.js`, `pages/api/user/avatar.js` — all
+identical except for the `Allow-Methods` verb list):
+
+```
+pages/api/admin/index.js
+pages/api/auth/verify.js
+pages/api/cards/[id]/ownership.js
+pages/api/cards/owned.js
+pages/api/cards/search.js
+pages/api/collections.js
+pages/api/collections/[identifier].js
+pages/api/collections/[identifier]/activity.js
+pages/api/collections/[identifier]/cards.js
+pages/api/collections/[identifier]/permissions.js
+pages/api/collections/[identifier]/thumbnails.js
+pages/api/community/collections.js
+pages/api/favorites.js
+pages/api/invite/accept.js
+pages/api/invite/decline.js
+pages/api/public/collections.js
+pages/api/user/avatar.js
+pages/api/user/avatar/generate.js
+pages/api/user/delete.js
+pages/api/user/password.js
+pages/api/user/profile.js
+pages/api/user/settings.js
+pages/api/user/stats.js
+pages/api/users/search.js
+```
+
+## Why now
+
+P0 #5 in `.convoys/ship-readiness.md` was marked PARTIAL on
+2026-05-23 because Brief 4 only fixed the auth surface (login,
+register) under that convoy's narrow auth-bypass mandate. The
+"Queued convoys" entry assumed `cors-tighten` would be a one-file
+follow-up on `verify.js`. The 23-file gap is a fresh discovery.
+
+The wildcard `Access-Control-Allow-Origin: *` allows any origin to
+read API responses from authenticated browser sessions. Combined
+with the JWT-in-`Authorization`-header pattern this is less
+exploitable than cookie-based sessions would be (browsers won't
+attach the token automatically across origins), but the wildcard
+still:
+
+1. **Enables credential stuffing from third-party origins** —
+   attacker can serve a page that POSTs to `/api/auth/login` with
+   guessed credentials and read the response (success/failure +
+   token). The 5-attempt/15-min rate limit from
+   `lib/rate-limit.js` mitigates volume but not the
+   attack-class.
+2. **Enables arbitrary cross-origin reads of any authenticated GET
+   response** if a victim manually attaches a Bearer token in the
+   wrong browser context (or if a downstream consumer ever pivots
+   to cookies, which `single-auth-provider` may eventually do).
+3. **Defeats Vercel's same-origin-by-default deployment shape** —
+   `tcg-vault.com` (or whatever it ends up being post-`pick-a-name`)
+   and the API are served from the same Vercel project. There is
+   no legitimate cross-origin caller. The header is purely
+   scaffolding cruft from whatever generator created the original
+   route templates.
+
+This convoy is launch sequence step 4 in `.convoys/ship-readiness.md`'s
+"Proposed launch sequence" (originally `add-rate-limiting`'s slot,
+but `cors-tighten` was queued separately and is logically prior —
+fixing CORS first means rate-limiting's protection isn't
+side-stepped by a cross-origin caller).
+
+## Scope — TWO OPTIONS, architect picks at gate 1
+
+### Option A — Narrow (matches the documented queued convoy entry)
+
+- Drop wildcard CORS + OPTIONS handler from `pages/api/auth/verify.js`
+  ONLY. ~10 LOC deletion. Closes P0 #5 from PARTIAL → RESOLVED-on-auth-surface.
+- Queue a separate `cors-sweep-all-routes` convoy for the remaining
+  23 files. Adds friction (two PRs, two doc-writer cleanups), but
+  matches the convoy's original documented scope.
+
+### Option B — Expanded (recommended by parent, pending architect ratification)
+
+- Drop wildcard CORS + OPTIONS handler from **all 24 files** in one
+  PR. ~240 LOC deletion across 24 files, mechanically identical to
+  what Brief 4 did to login + register. Closes P0 #5 fully —
+  PARTIAL → RESOLVED.
+- Single mechanical sweep; no per-file design decisions; smoke +
+  vitest defend against regression.
+- Same precedent shape applies (Brief 4's commit `297afca` is the
+  reference diff).
+
+**Architect's responsibility at gate 1:** confirm Option B is
+mechanically safe (no file in the 24 has unique pre-OPTIONS body
+logic that depends on the wildcard, no file is doing a *narrow*
+CORS hint that should be preserved-but-tightened rather than
+deleted), or recommend Option A with explicit reasoning. Default
+recommendation is B — same-origin Vercel deployment means CORS
+headers serve no legitimate purpose anywhere on this API surface.
+
+## Operator action required
+
+**None.** No env vars, no secrets, no infra changes.
+
+## Decisions to ratify with operator
+
+1. **Option A (narrow) vs Option B (expanded).** See Scope §
+   above. Parent recommends Option B; architect investigates and
+   ratifies.
+2. **Should the OPTIONS preflight handler be replaced with a
+   `405 Method Not Allowed`?** Once the wildcard CORS is gone,
+   browsers will not send preflights to this API (same-origin
+   doesn't preflight). Three sub-options:
+   - **(a)** Delete the OPTIONS handler entirely. Method-check
+     at top of handler (`if (req.method !== 'POST')`) returns 405.
+     Same end-state as Brief 4 did to login + register.
+   - **(b)** Keep the OPTIONS handler, return 405. Slightly
+     friendlier to any future direct-CLI callers, but contradicts
+     the "no special-case OPTIONS" simplification.
+   - **(c)** Keep the OPTIONS handler, return 204. Strictly
+     correct per RFC 7231 for an empty success response.
+   Recommend (a). Matches Brief 4 precedent. Trivial to revisit
+   if a real cross-origin caller ever lands.
+3. **`pages/api/auth/verify.js` is a GET endpoint** — its
+   `Allow-Methods` line is `'GET, POST, PUT, DELETE, OPTIONS'`
+   (over-permissive). Should the method gate be tightened to
+   GET-only at the same time? Recommend YES — the handler
+   already has `if (req.method !== 'GET') return 405` at line 17,
+   so tightening the `Allow-Methods` line is moot once it's
+   deleted. No-op.
+4. **Any tests need updating?** Vitest suite covers
+   `pages/api/auth-utils.js` and `lib/permission-middleware.js`
+   but NOT the per-route handlers directly. Smoke suite covers
+   `home`, `sign-in`, `/api/health` — none of the 24 routes
+   in scope are smoke-covered today, and smoke uses same-origin
+   so CORS removal won't affect it. Recommend NO new tests in
+   this convoy (deferred to a per-route handler test convoy that
+   doesn't exist yet).
+
+## Known constraints
+
+- **All 24 files use the IDENTICAL scaffolded pattern** (parent
+  spot-checked 3 of 24; architect should spot-check 5+ more to
+  rule out drift). Same 3-line CORS block, same OPTIONS-guard
+  block. Differs only in the `Allow-Methods` verb list.
+- **Brief 4 of `fix-auth-bypass` (commit `297afca`)** is the
+  exact precedent. The shape there was: delete the comment, delete
+  the 3 setHeader calls, delete the if-OPTIONS block. That is the
+  edit to apply 24 times.
+- **No lib-level change.** No need to add a "CORS helper module"
+  or anything else — the right answer is "no CORS at all", same as
+  Brief 4 settled on.
+- **CI workflow `forbidden-endpoints` job** does NOT currently
+  forbid CORS headers. If we want a regression lock, the architect
+  could optionally add a new CI grep-gate. Recommend YES if Option
+  B is chosen (locks in the cleanup so it can't accumulate again).
+- **Same-origin assumption holds** — the API and frontend live on
+  the same Vercel project (same domain). If that ever changes
+  (separate API subdomain, mobile app calling the API directly),
+  a real CORS layer needs to be designed at that point. This
+  convoy explicitly does NOT design for that future — YAGNI.
+
+## Acceptance criteria
+
+The convoy is shippable when ALL of the following hold:
+
+1. The targeted files (`verify.js` for Option A; all 24 for
+   Option B) have zero `Access-Control-Allow-Origin` references.
+2. The targeted files have zero `if (req.method === 'OPTIONS')`
+   blocks.
+3. `npm run lint` exit code matches baseline (still 128 problems
+   per `fix-lint-baseline`; do NOT regress).
+4. `npm run test:run` (vitest) still passes 21/21 (no regression).
+5. `npm run test:smoke` (via CI on the PR) still passes 3/3 against
+   the Vercel preview — proves login + verify + /api/health flow
+   end-to-end after the CORS removal.
+6. `git grep -nE "Access-Control-Allow-Origin" -- 'pages/api/**'`
+   returns zero matches (Option B) OR exactly N-1 matches where
+   N=23 (Option A).
+7. If Option B AND the architect picks "add CI regression lock":
+   `.github/workflows/ci.yml`'s `forbidden-endpoints` job (or a
+   new `forbidden-cors-headers` job) fails when any new
+   `Access-Control-Allow-Origin` is reintroduced.
+
+## Anything flagged but not acted on (in advance)
+
+- **A real CORS layer for a future mobile / 3rd-party API
+  consumer.** Out of scope. If/when needed, design from scratch
+  (probably as middleware) rather than re-scaffolding wildcards.
+- **`cors-sweep-all-routes` follow-up** — only relevant if Option
+  A is chosen. Pre-queue the entry in ship-readiness's Queued
+  convoys section if Option A wins.
+- **API-route handler unit tests** — none of the 24 files have
+  vitest coverage today. Adding handler-level tests for each is
+  a separate convoy (probably `fill-vitest-handler-coverage`).
+- **OPTIONS / CORS via Next.js middleware** — `middleware.js`
+  doesn't exist. Adding a middleware layer to enforce same-origin
+  is an over-engineered fix for "remove unnecessary headers";
+  YAGNI. Note for posterity in case a future agent considers it.