From f8b2cd85bce23915fc2a3de5027b4319a3f40efe Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sun, 24 May 2026 20:49:30 -0500 Subject: [PATCH] docs: post-convoy cleanup for cors-tighten Reflects the merged cors-tighten convoy (PR #19, squash commit da50d78) in repo documentation. Closes P0 #5 (Wildcard CORS on API surface) from PARTIAL -> RESOLVED, leaving only P0 #6 (full add-rate-limiting) open of the original P0 ship-blocker set. One brief in the convoy: Brief 1 shipped as planned with no scope expansions and no implementer deviations from the verbatim spec. .convoys/cors-tighten.md: - frontmatter status: in-progress -> shipped (added shipped: 2026-05-24) - new ## As-shipped section: all 5 architect-self-ratifiable decisions ratified verbatim (D1 Option B / D2 delete OPTIONS / D3 moot / D4 no new tests / D5 add CI lock); Pattern split (16 Pattern A + 8 Pattern B) per architect's 10-file audit + implementer's per-file diff review; diff size (25 files, +29/-261); empirical CI metrics from post-merge run 26378806555 (forbidden-cors-headers 4s PASS, Playwright smoke 56s 3/3 in 3.3s, Screenshot diff continue-on-error 0 with the documented Decision-4 missing-baseline failure beneath); cross-validation that Playwright smoke continues to pass post-CORS removal (the auth + public surfaces don't depend on the wildcard header); implementer subagent-retry footnote (HEAD already at a843736 when retry woke up - transient retry, work is canonical); operator-action-required-going-forward: none; What did NOT change audit trail. .convoys/ship-readiness.md: - new ## Status summary at the top (right after the code-graph line): P0 set is now 7/8 RESOLVED; only #6 (rate-limiting) remains. Table lists each P0 with its resolving convoy + squash commit for a quick scan of remaining work. - P0 #5 marked RESOLVED 2026-05-24. Added the cors-tighten as-shipped block (24 files swept, new CI job, 16/8 Pattern split, 5 decisions ratified, diff stat, post-merge CI metrics, transient retry footnote, operator-action: none). Brief 4's 2026-05-23 partial is preserved as the prior as-shipped layer above the cors-tighten layer to maintain the audit trail. - Queued convoys: removed the cors-tighten entry (no longer queued). Added a new tighten-visual-diff-path-filter entry (P3 polish) - Screenshot diff workflow triggered on API-only PR #19 because its paths: filter is pages/** which matches pages/api/** too. ~55s of CI waste per API-only PR; one-line YAML tweak; verify GitHub Actions' negated-glob semantics before merging. .cursor/rules/api-routes.mdc: - new ## CORS section near the existing ## Dev/test endpoints (removed) section. Documents the no-CORS-by-default convention, the brief-4 + cors-tighten lineage, the new forbidden-cors-headers CI gate, and three forward-conventions (no setHeader for CORS, no OPTIONS preflight handlers, design a proper middleware layer if a future cross-origin caller is needed - not wildcards in individual handlers). AGENTS.md intentionally untouched. Gotcha #5 (the public setup-database.js endpoint) is already RESOLVED by fix-auth-bypass Brief 3 and unrelated to this convoy. The new convention belongs in .cursor/rules/api-routes.mdc (where API conventions live) rather than AGENTS.md; the convoy file + the new CI gate are sufficient documentation for the audit trail. Per convoy spec, no new gotcha entry needed. No changes to: package.json, package-lock.json, pages/api/**, lib/**, components/**, scripts/**, test/**, tests/**, .github/workflows/**, README.md, TESTING_GUIDE.md, playwright.config.js. Co-authored-by: Cursor --- .convoys/cors-tighten.md | 178 ++++++++++++++++++++++++++++++++++- .convoys/ship-readiness.md | 41 +++++++- .cursor/rules/api-routes.mdc | 10 ++ 3 files changed, 224 insertions(+), 5 deletions(-) diff --git a/.convoys/cors-tighten.md b/.convoys/cors-tighten.md index b5f3b2e..a39a1cf 100644 --- a/.convoys/cors-tighten.md +++ b/.convoys/cors-tighten.md @@ -13,8 +13,9 @@ skip: - role-a11y-auditor - role-ux-reviewer - role-ia-architect -status: in-progress +status: shipped created: 2026-05-24 +shipped: 2026-05-24 parent: ship-readiness addresses: P0 #5 (PARTIAL → RESOLVED) depends_on: @@ -468,3 +469,178 @@ slice_dependencies: ``` Single-brief slice; conductor dispatches one implementer (no `/multitask` fan-out applicable). Architect complete. 1 brief created. Estimated PRs: 1. Awaiting human gate 1 (plan approval) before implementer runs. + +## As-shipped + +Shipped 2026-05-24 as squash commit `da50d78` (PR #19, architect-commit +`ec22b70`, implementer-commit `a843736`). Brief 1 shipped as planned — +single mechanical sweep + new CI regression-lock, no scope expansions, +no implementer deviations from the brief's verbatim shape. Capturing +the empirical CI metrics + the cross-validation finding + one +transient implementer subagent retry footnote here so the next +architect / reviewer has the audit trail. + +### Decisions ratified at gate 1 + +All five decisions were architect-self-ratifiable per the convoy spec +(see § Decisions above) and all five were ratified verbatim — no +operator gate-1 decision needed for any individual one: + +- **Decision D1 — Option B (expanded sweep, all 24 files).** Architect's + 10-file pattern-drift audit (parent spot-checked 3 + architect + spot-checked 7 additional) confirmed all 24 share the identical + scaffolded shape. Closed P0 #5 from PARTIAL → RESOLVED in one PR + rather than splitting into two convoys. +- **Decision D2 — delete the OPTIONS preflight handler entirely** + (Option (a)). Method-check at the top of the handler (Pattern A) or + branched inside the `try` block (Pattern B) safely returns 405 for + any future OPTIONS request. Matches Brief 4 precedent on + `login.js` + `register.js` (commit `297afca`). +- **Decision D3 — `verify.js` `Allow-Methods` tightening is moot.** + The over-permissive `'GET, POST, PUT, DELETE, OPTIONS'` verb list + was deleted along with the other two `setHeader` calls under D2. + No-op. +- **Decision D4 — no new per-route handler tests in this convoy.** + Vitest covers `lib/auth-secret.js`, `lib/permission-middleware.js`, + `pages/api/auth-utils.js`, and `components/Layout.js` (21 tests + total) — none of the 24 swept files. Smoke covers `/`, `/login`, + `/api/health` — also none. Handler-level coverage is the queued + `fill-vitest-handler-coverage` convoy (does not exist yet); this + convoy ships the cleanup now. +- **Decision D5 — add a new blocking `forbidden-cors-headers` CI job.** + Modeled verbatim on the existing `forbidden-endpoints` job in + `.github/workflows/ci.yml` (added by `fix-auth-bypass` Brief 3). + Greps `pages/api/` for `Access-Control-Allow-(Origin|Methods|Headers)`, + emits `::error file= line=::` annotations on hit, exits 1. No + `continue-on-error`, no `|| true` wrapper — fully blocking. Runs + in ~4 seconds on the runner; zero new dependencies. + +### As-shipped Pattern split + +The 24 files split **16 Pattern A + 8 Pattern B** across the sweep, +matching the architect's pre-sweep prediction by transitivity from +the 10-file pattern-drift audit. The architect-confirmed exemplars +of each shape (10 files spot-checked, listed in § Architecture → +Pattern-drift audit results above): + +**Pattern A — top-level method gate after the CORS block** (confirmed +exemplars): `pages/api/auth/verify.js`, `pages/api/admin/index.js`, +`pages/api/user/avatar/generate.js`, +`pages/api/cards/[id]/ownership.js`, `pages/api/invite/accept.js`, +`pages/api/public/collections.js`, `pages/api/cards/owned.js`, +`pages/api/community/collections.js`, `pages/api/cards/search.js`, +`pages/api/invite/decline.js`. + +**Pattern B — method-branched inside the `try` block** (confirmed +exemplars): `pages/api/collections.js`, +`pages/api/collections/[identifier].js`, +`pages/api/collections/[identifier]/permissions.js`, +`pages/api/user/avatar.js`. + +The remaining 10 unconfirmed-at-audit files fell into the 16/8 split +during the implementer's per-file diff review. Each file's per-file +diff is verifiable in the squash commit `da50d78`; nothing +improvised away from either pattern's brief reference shape. + +Each file's diff is a pure deletion of 9-11 lines (the leading +`// Set CORS headers` comment + 3 `setHeader` calls + the leading +`// Handle preflight requests` comment + the 4-line OPTIONS-if block ++ the blank line that already sat between the deleted block and what +follows). No additions per source file. No re-indentation. No +behavior change to any post-block code. + +### As-shipped metrics + +Diff size (per `git show --stat da50d78`): + +- **25 files modified, +29 / -261.** +- 24 source files: pure deletion, 10-11 lines each. +- `.github/workflows/ci.yml`: +29 lines (the new `forbidden-cors-headers` + job block; sits between `forbidden-endpoints` and `test`, modeled + verbatim on `forbidden-endpoints` per Decision D5). + +Post-merge CI run 26378806555 + subsequent runs on `main`: + +- **`forbidden-cors-headers` (new) — PASS in 4s.** First live exercise + of the regression-lock; greps clean against the post-sweep tree. +- **`Playwright smoke` — PASS in 56s, 3/3 tests in 3.3s** against the + post-CORS-removal Vercel preview. Same three checks + (`home redirects or renders without 5xx`, `sign-in page renders`, + `public health endpoint responds`) — all green. Cross-validates that + the CORS removal is safe for the auth surface (the smoke spec's + sign-in check still passes against `/login`, and `/api/health` + still serves anonymously). +- **`Screenshot diff` — workflow exited 0** because of + `continue-on-error: true`, but the actual visual test failed with + the documented "snapshot doesn't exist" error (Decision-4 end state + of `adopt-playwright-smoke`). PR comment posted correctly with run + link + update instructions. Triggered on PR #19 despite this being + an API-only change because the workflow's `paths:` filter is + `pages/**` which matches `pages/api/**` too — a minor false-positive + queued as `tighten-visual-diff-path-filter` in + `.convoys/ship-readiness.md` § Queued convoys. Cosmetic, no + merge-block. +- **All other gates** (`Lint`, `Vitest`, `Schema map up to date`, + `forbidden-endpoints`) — green. +- **Local at implementer commit:** lint baseline 128 problems + (unchanged), vitest 21/21 pass, zero grep matches for + `Access-Control-Allow-*` under `pages/api/`, YAML valid. + +### Cross-validation: Playwright smoke continues to pass post-CORS-removal + +The smoke spec's test 2 (`'sign-in page renders'`) navigates to +`/login` and asserts the "Sign in" CTA renders; test 3 +(`'public health endpoint responds'`) hits `/api/health` and asserts +2xx. Both still green after the 24-file sweep, which proves the +removal is safe for the live auth + public surfaces: the login flow +doesn't depend on the wildcard CORS header in any browser context the +smoke exercises (same-origin Vercel preview), and the `/api/health` +endpoint (not in the 24-file sweep) is unaffected. + +This was not a planned acceptance criterion of the convoy — Decision D4 +explicitly deferred per-route handler coverage to a future convoy — +but the smoke spec's existing assertions transitively defend the auth +surface against this convoy's deletions. Worth noting because the +convoy ships with no new per-route tests yet locks in real behavior +via the existing CI signal. + +### Implementer subagent retry footnote (transient) + +The implementer's PR report flagged that HEAD was already at the +implementer commit (`a843736`) when its retry subagent woke up — a +prior implementer run had completed the work, and the retry's +"STOP per branch mismatch" rule kicked in. The retry then ran +verification only (lint baseline, vitest 21/21, grep clean, YAML +valid) and reported success. **This is a transient subagent retry, +not a process gap.** The implementer commit `a843736` is canonical; +the squash commit `da50d78` rolls up the architect plan + Brief 1 ++ the implementer's work without any duplication. + +### Operator action required going forward + +**None.** No env vars to seed, no secrets to rotate, no infra changes. +The `forbidden-cors-headers` job is self-contained (plain bash grep +on the runner; no new dependencies, no caching, no `setup-node`). +Future PRs that accidentally re-scaffold a wildcard CORS header will +fail the build with a `::error file= line=::` pointer to the +offending line. + +### What did NOT change + +- `pages/api/auth/login.js`, `pages/api/auth/register.js` — already + cleaned by `fix-auth-bypass` Brief 4 (commit `297afca`). Re-verified + post-sweep that they remain CORS-free. +- `pages/api/health.js` — never had the wildcard block; not in scope. +- Any `pages/api/cards/import-*.js` file — listed under no-go zones + in `.cursor/rules/no-go-zones.mdc` (external API rate limits, + run-against-staging-only); didn't carry the wildcard anyway. +- `lib/permission-middleware.js`, `lib/rate-limit.js`, + `lib/auth-secret.js` — auth surface untouched. +- `test/**` — no new per-route handler tests per Decision D4. +- `tests/smoke/**`, `tests/visual/**`, `playwright.config.js` — + smoke + visual suite unaffected (same-origin Vercel preview). +- `.github/workflows/preview-smoke.yml`, `.github/workflows/visual-diff.yml` + — owned by `adopt-playwright-smoke` / + `fix-vercel-deployment-protection-in-ci`; this convoy added only + the new `forbidden-cors-headers` job in `ci.yml`. +- `package.json`, `package-lock.json` — no dependency change. diff --git a/.convoys/ship-readiness.md b/.convoys/ship-readiness.md index f9ca102..0979636 100644 --- a/.convoys/ship-readiness.md +++ b/.convoys/ship-readiness.md @@ -13,6 +13,27 @@ Umbrella convoy capturing the full agent-pipeline review of tcg-vault as of 2026 Code graph: 122 files, 628 nodes, 5602 edges, 11 communities. Indexed by `user-code-review-graph` MCP. +## Status summary (as of 2026-05-24) + +**P0 ship-blockers: 7 of 8 RESOLVED; 1 remains.** + +| Item | Status | Convoy | +| --- | --- | --- | +| P0 #1 — `getUserFromRequest` hardcoded admin | **RESOLVED** 2026-05-23 | `fix-auth-bypass` Brief 2 (`258e479`) | +| P0 #2 — `JWT_SECRET` hardcoded fallback | **RESOLVED** 2026-05-23 | `fix-auth-bypass` Brief 1 (`4a10dce`) | +| P0 #3 — Default admin credentials in seed | **RESOLVED** 2026-05-23 | `drop-public-setup` (`ff80753` + `b63b509`) | +| P0 #4 — Dev-only test endpoints | **RESOLVED** 2026-05-23 | `fix-auth-bypass` Brief 3 (`fc0dd73`) | +| P0 #5 — Wildcard CORS on API surface | **RESOLVED** 2026-05-24 | `fix-auth-bypass` Brief 4 (`297afca`) + `cors-tighten` (`da50d78`) | +| P0 #6 — No rate limiting | **PARTIAL** 2026-05-23 | `fix-auth-bypass` Brief 4 (login + register only); queued `add-rate-limiting` for the rest | +| P0 #7 — Layout default-prop leaks email | **RESOLVED** 2026-05-24 | `fix-layout-default-user` (`ca302a8`) | +| P0 #8 — Next.js 15.4.3 vulnerable version | **RESOLVED** 2026-05-23 | `bump-next-js` (`e57ea17`) | + +**Only P0 #6 remains open** — extending `lib/rate-limit.js` to +`/api/users/search`, `/api/cards/search`, all `/api/cards/import-*`, +and `/api/user/avatar*` (queued as `add-rate-limiting`, launch +sequence step 4 below). That convoy is the last P0 milestone before +the security gate closes. + ## P0 — ship-blockers (security) These MUST land before any anonymous traffic touches the production URL. @@ -71,13 +92,25 @@ These MUST land before any anonymous traffic touches the production URL. - **As-shipped:** All four files deleted. `.github/workflows/ci.yml` has a new `forbidden-endpoints` job (blocking) that fails the build if any of the four paths reappear OR if a new `pages/api/test-*.js` file is added. Local simulation in the implementer PR confirmed clean → OK, with `test-fake.js` → FAIL, post-cleanup → OK. - **Owns:** `role-implementer`. -### 5. CORS `Access-Control-Allow-Origin: *` on auth endpoints — **PARTIAL 2026-05-23** +### 5. CORS `Access-Control-Allow-Origin: *` on auth endpoints — **RESOLVED 2026-05-24** -- **Partially resolved by:** `fix-auth-bypass` Brief 4, commit `297afca` (PR #9). Login + register only; `pages/api/auth/verify.js` is **deferred** to the queued `cors-tighten` follow-up convoy. +- **Resolved by:** `fix-auth-bypass` Brief 4, commit `297afca` (PR #9, login + register) + `cors-tighten`, squash commit `da50d78` (PR #19, the remaining 24 handlers + CI regression-lock). - **Files:** at minimum `pages/api/auth/login.js`, `pages/api/auth/register.js`, `pages/api/setup-database.js` (verify others). - **Impact:** Any origin can submit credentials. Combined with the no-rate-limit problem below, credential stuffing is wide open. - **Fix:** Set `Access-Control-Allow-Origin` to the literal frontend origin (`https://tcgvault.com` / preview domain), or remove the header entirely if the API and the frontend are same-origin (they are, on Vercel). -- **As-shipped:** `pages/api/auth/login.js` and `pages/api/auth/register.js` drop the four `setHeader` calls + the OPTIONS preflight handler. `pages/api/setup-database.js` was deleted entirely by Brief 3. `pages/api/auth/verify.js` still has the wildcard header — see follow-up convoy `cors-tighten`. +- **As-shipped (Brief 4, 2026-05-23):** `pages/api/auth/login.js` and `pages/api/auth/register.js` dropped the four `setHeader` calls + the OPTIONS preflight handler. `pages/api/setup-database.js` was deleted entirely by Brief 3. +- **As-shipped (`cors-tighten`, 2026-05-24, squash commit `da50d78`, PR #19, architect-commit `ec22b70`, implementer-commit `a843736`):** + 1. **24 `pages/api/**` handlers swept** — `admin/index.js`, `auth/verify.js`, `cards/[id]/ownership.js`, `cards/owned.js`, `cards/search.js`, `collections.js`, `collections/[identifier].js`, `collections/[identifier]/activity.js`, `collections/[identifier]/cards.js`, `collections/[identifier]/permissions.js`, `collections/[identifier]/thumbnails.js`, `community/collections.js`, `favorites.js`, `invite/accept.js`, `invite/decline.js`, `public/collections.js`, `user/avatar.js`, `user/avatar/generate.js`, `user/delete.js`, `user/password.js`, `user/profile.js`, `user/settings.js`, `user/stats.js`, `users/search.js`. Each diff is a pure deletion of 9-11 lines (the leading `// Set CORS headers` comment + 3 `setHeader` calls + the leading `// Handle preflight requests` comment + the 4-line OPTIONS-if block + the trailing blank line). No additions per source file. **Pattern split: 16 Pattern A (top-level method gate after the CORS block) + 8 Pattern B (method-branched inside the `try` block).** Both shapes documented verbatim in `.convoys/cors-tighten/brief-1-sweep-wildcard-cors.md`. + 2. **New blocking `forbidden-cors-headers` CI job** in `.github/workflows/ci.yml`, modeled verbatim on the existing `forbidden-endpoints` job (added by `fix-auth-bypass` Brief 3). Greps `pages/api/` for `Access-Control-Allow-(Origin|Methods|Headers)`, emits `::error file= line=::` annotations on hit, exits 1. No `continue-on-error`, no `|| true` wrapper. Sits between `forbidden-endpoints` and `test` in the YAML for logical grouping (both `forbidden-*` checks are static-source guards before the runtime test job). Runs in ~4 seconds; zero new dependencies. + 3. **All five architect decisions self-ratified at gate 1** (no operator decisions needed) — D1 Option B (expanded sweep, all 24 files), D2 delete the OPTIONS preflight handler entirely (Option (a)), D3 `verify.js` `Allow-Methods` tightening moot (subsumed by D2), D4 no new per-route handler tests in this convoy (deferred to queued `fill-vitest-handler-coverage`), D5 add the new CI regression-lock job. + 4. **Diff: 25 files, +29 / -261** (pure deletion across 24 source files; 29 additions = the new CI job). +- **As-shipped metrics (post-merge run 26378806555 + subsequent runs):** + - `forbidden-cors-headers` (new) — PASS in **4s**. First live exercise of the regression-lock; greps clean against the post-sweep tree. + - `Playwright smoke` — PASS in **56s, 3/3 tests in 3.3s** against the post-CORS-removal Vercel preview. Cross-validates that CORS removal is safe for the auth surface (smoke's sign-in check still passes against `/login`; `/api/health` still serves anonymously). Surfaced as a real CI signal even though Decision D4 deferred per-route handler tests — the existing smoke spec transitively defends the auth + public surfaces against this convoy's deletions. + - `Screenshot diff` — workflow exited 0 because of `continue-on-error: true`, but the actual visual test failed with the documented "snapshot doesn't exist" error (Decision-4 end state of `adopt-playwright-smoke`). Triggered on PR #19 despite this being API-only because its `paths:` filter is `pages/**` which matches `pages/api/**` too — minor false-positive queued as `tighten-visual-diff-path-filter` (see § Queued convoys). + - All other gates (`Lint`, `Vitest`, `Schema map up to date`, `forbidden-endpoints`) — green. +- **Implementer subagent-retry footnote (transient).** The implementer's PR report flagged that HEAD was already at the implementer commit (`a843736`) when its retry subagent woke up — a prior implementer run had completed the work, and the retry's "STOP per branch mismatch" rule kicked in; the retry then ran verification only (lint baseline, vitest 21/21, grep clean, YAML valid) and reported success. This is a transient subagent retry, not a process gap. The implementer commit `a843736` is canonical; the squash `da50d78` rolls up the architect plan + Brief 1 + the implementer's work without duplication. +- **Operator action required going forward:** **none.** No env vars to seed, no secrets to rotate, no infra changes. The `forbidden-cors-headers` job is self-contained (plain bash grep on the runner); future PRs that accidentally re-scaffold a wildcard CORS header will fail the build with a file-and-line pointer to the offending line. - **Owns:** `role-implementer`. ### 6. No rate limiting anywhere — **PARTIAL 2026-05-23** @@ -293,8 +326,8 @@ Total: ~14 convoys to get from current state to public-launch-ready. Estimate 4- Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch sequence but need to land before public traffic. Listed in priority order; not all will be P0/P1 — most are CI / DX / hygiene polish. - **`rotate-default-admin`** (priority: P2 hygiene). Operator-rotation script for envs that ran `setup-neon-db.js` before `drop-public-setup` and still carry the weak `admin123` bcrypt hash. Surfaced in P0 #3 § Operator caveat. Optional: do nothing if no audit finds a deployed env with the weak hash. -- **`cors-tighten`** (priority: P1 quality). Drop the wildcard `Access-Control-Allow-Origin` header from `pages/api/auth/verify.js`. Surfaced in P0 #5 (deferred from `fix-auth-bypass` Brief 4). - **`add-rate-limiting`** (priority: P1 quality, also listed in launch sequence step 4). Extend `lib/rate-limit.js` to `/api/users/search`, `/api/cards/search`, all `/api/cards/import-*`, and `/api/user/avatar*`. Login + register already wired in `fix-auth-bypass` Brief 4. +- **`tighten-visual-diff-path-filter`** (priority: P3 polish). Tighten `.github/workflows/visual-diff.yml`'s `paths:` filter from `pages/**` to something like `pages/!(api)/**` so API-only PRs don't trigger the visual workflow. Surfaced on PR #19 (`cors-tighten`) which was API-only but triggered `Screenshot diff` anyway, wasting ~55s of CI time. Low priority — works as designed today, just chatty. One-line YAML tweak; verify the negated-glob syntax against GitHub Actions' [path-filter semantics](https://docs.github.com/en/actions/using-workflows/triggering-a-workflow#filter-pattern-cheat-sheet) before merging (the `!()` extglob may need to be expressed differently if Actions uses minimatch without extglob support). - **`purge-weak-creds-from-helpers`** (priority: P2 hygiene). Sweep `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md` for the literal `admin@tcgvault.com` / `admin123` references. May fold into `pick-a-name` since the email itself is changing. - **`single-auth-provider`** (priority: P1 quality, also listed as launch sequence step 9). Collapse `lib/auth-context.js` + `lib/admin-auth.js` into `lib/use-auth.js`. Surfaced again as a follow-up in P0 #7 § Flagged-but-deferred (4 pages still import the legacy `useAuth`). - **`cleanup-mobile-nav-dead-props`** (priority: P3 polish). `components/MobileNavigation.js` accepts a dead `user` prop; remove it. Surfaced in P0 #7 § Flagged-but-deferred. May fold into `god-component-split` (P2 #13) if that lands first. diff --git a/.cursor/rules/api-routes.mdc b/.cursor/rules/api-routes.mdc index 08b7dfc..db05771 100644 --- a/.cursor/rules/api-routes.mdc +++ b/.cursor/rules/api-routes.mdc @@ -137,3 +137,13 @@ Notes: ## Dev/test endpoints (removed) The four endpoints `pages/api/simple.js`, `pages/api/test-auth.js`, `pages/api/test-db.js`, and `pages/api/setup-database.js` used to exist as unauthenticated dev / diagnostic routes. They were **deleted** by `fix-auth-bypass` Brief 3 (commit `fc0dd73`) and `.github/workflows/ci.yml`'s `forbidden-endpoints` job now fails the build if any of them are re-introduced, or if any new file matching `pages/api/test-*.js` is added. **Do not re-create these files.** If a future agent searches for `test-db` or `setup-database` and finds them missing, this section is the explanation — diagnostics belong outside the public API surface (a CLI script, an admin-gated route, or `npm run` task). + +## CORS + +No `pages/api/**` route ships CORS headers. The frontend and the API are same-origin on Vercel (same project, same domain), and cross-origin reads serve no legitimate purpose on this API surface — the wildcard `Access-Control-Allow-Origin: *` that 24 handlers used to carry was scaffolding cruft, not a deliberate cross-origin design. `fix-auth-bypass` Brief 4 (commit `297afca`) removed it from `pages/api/auth/login.js` + `pages/api/auth/register.js`; the `cors-tighten` convoy (squash commit `da50d78`, PR #19) swept the remaining 24 handlers and added a new blocking `forbidden-cors-headers` job to `.github/workflows/ci.yml` (modeled on `forbidden-endpoints`) that fails the build if any `Access-Control-Allow-(Origin|Methods|Headers)` reference reappears under `pages/api/`. + +Conventions to follow: + +- **Do not add `res.setHeader('Access-Control-Allow-*', ...)` to any new route.** The CI gate will fail the build with a file-and-line pointer. +- **Do not add `if (req.method === 'OPTIONS')` preflight handlers.** Same-origin requests don't preflight; cross-origin requests are blocked at the browser CORS layer (the desired end state). If an OPTIONS request ever arrives, the existing method gate (`if (req.method !== '') return res.status(405)`) returns 405 — strictly safer than the pre-sweep 200-to-everyone. +- **If a future cross-origin caller is legitimately needed** (third-party app, mobile client, public API key program — none exist today), design a proper CORS layer — probably as Next.js middleware reading an allowed-origin list from env — rather than scaffolding wildcards back into individual handlers. That's a separate convoy (`add-cors-layer` or similar); flag it as a new follow-up in `.convoys/ship-readiness.md` § Queued convoys at the time the need surfaces.