rstillwell/deckhearth

Fork 0

fix(security): drop wildcard CORS + redundant OPTIONS from 24 API routes (P0 #5 PARTIAL → RESOLVED) #19

Merged

varutasu merged 3 commits from convoy/cors-tighten into main

2026-05-24 21:41:38 -04:00

varutasu commented

2026-05-24 21:38:51 -04:00

(Migrated from github.com)

Summary

Closes P0 #5 in .convoys/ship-readiness.md from PARTIAL → RESOLVED. fix-auth-bypass Brief 4 (commit 297afca) cleaned login + register; this convoy sweeps the remaining 24 pages/api/** handlers that carried the identical scaffolded wildcard-CORS + redundant-OPTIONS pattern, plus adds a blocking forbidden-cors-headers CI job to lock in the cleanup against future regression.

With this merged, the only remaining P0 is #6 (rate limiting on non-auth endpoints — add-rate-limiting convoy).

Convoy artifacts

Convoy file: .convoys/cors-tighten.md (architect ratified all 5 decisions self-ratifiable)
Single brief: .convoys/cors-tighten/brief-1-sweep-wildcard-cors.md
Architect commit: ec22b70
Implementer commit: a843736

Decisions ratified (all 5 architect-self-ratifiable)

	Decision	Choice
1	Narrow (verify.js only) vs Expanded (all 24)	Expanded — all 24
2	OPTIONS handler shape	Delete entirely (match Brief 4 precedent on login.js / register.js)
3	verify.js Allow-Methods tightening	Moot (subsumed by D2's deletion)
4	New per-route handler tests	No (queued for fill-vitest-handler-coverage convoy)
5	CI regression-lock job	Yes — new `forbidden-cors-headers` job, modeled on `forbidden-endpoints`

Pattern-drift audit

Parent + architect spot-checked 14 of 24 files (zero drift). All 24 share the identical 9-11-line scaffolded block. Two safe sub-shapes:

Pattern A (16 files) — top-level method gate immediately after the CORS block; deletion leaves clean top-of-handler
Pattern B (8 files) — method-branched inside try; deletion routes OPTIONS to the existing else { 405 } branch

Both patterns accept the same mechanical deletion. Architect's full pattern map in .convoys/cors-tighten/brief-1-sweep-wildcard-cors.md.

Architect's most non-obvious finding

.cursor/rules/api-routes.mdc documents withCollectionPermission(...) as a convention, but zero pages/api/** routes actually use it (architect grep returned empty). The stress-test concern about CORS headers being inside vs outside a permission-wrapper turned out to be moot. All 24 routes call getUserFromRequest directly inside the handler body.

Diff

25 files, +29 / -261. Pure deletion across 24 source files; 29 additions are the new CI job.

 .github/workflows/ci.yml                          | 29 +++++++++++++++++++++++
 pages/api/admin/index.js                          | 11 ---------
 pages/api/auth/verify.js                          | 11 ---------
 ... (22 more pages/api/** files, ~10-11 deletions each)
 25 files changed, 29 insertions(+), 261 deletions(-)

Precedent files explicitly untouched

pages/api/auth/login.js and pages/api/auth/register.js (already swept by Brief 4) are byte-identical at HEAD. Architect's R7 foot-gun avoided.

Verification

Check	Result
`npm run lint`	exit 1, 128 problems (baseline match)
`npm run test:run` (vitest)	21/21 pass
`git grep -nE "Access-Control-Allow-(Origin\|Methods\|Headers)" -- 'pages/api/**'`	zero matches
`git grep -nE "if \(req\.method === 'OPTIONS'\)" -- 'pages/api/**'`	zero matches
New CI job grep, local	exit 0, "OK: no Access-Control-Allow-* headers under pages/api/."
`ci.yml` YAML parse	OK
Precedent files diff	empty (untouched)

Expected CI behavior

forbidden-cors-headers (new job): pass (just swept; nothing to find)
forbidden-endpoints (existing): pass (no change in scope)
Lint: still 128 problems
Vitest: still 21/21
Playwright smoke: should pass 3/3 — smoke navigates to /login and /api/health which are same-origin from the Vercel preview perspective (CORS irrelevant). The login → JWT-verify flow doesn't break because the smoke spec doesn't actually submit credentials.
Screenshot diff: not triggered (path filter excludes API-only changes)

What did NOT change

lib/**, components/**, scripts/**, test/**, package.json, lockfile, AGENTS.md, .cursor/rules/, any other workflow YAML
pages/api/auth/login.js + register.js (Brief 4's earlier sweep preserved)
pages/api/auth/login.js's rate-limit gate (Brief 4) and pages/api/auth/register.js's rate-limit gate (Brief 4) — both untouched

Out of scope (deferred / flagged)

Real CORS layer for future mobile / 3rd-party API consumer — YAGNI; design from scratch when actually needed (probably middleware-based)
Per-route handler unit tests — queued for fill-vitest-handler-coverage (none of the 24 routes have vitest coverage today)
add-rate-limiting (P0 #6 PARTIAL) — last remaining P0; queued for next convoy

Test plan

Local lint matches 128 baseline
Local vitest 21/21
Grep gates clean
YAML valid
Precedent files untouched
CI forbidden-cors-headers job passes (verify post-push)
CI all other checks green (verify post-push)
Playwright smoke 3/3 against Vercel preview (verify post-push)

Made with Cursor

## Summary Closes **P0 #5** in `.convoys/ship-readiness.md` from PARTIAL → RESOLVED. `fix-auth-bypass` Brief 4 (commit `297afca`) cleaned login + register; this convoy sweeps the remaining 24 `pages/api/**` handlers that carried the identical scaffolded wildcard-CORS + redundant-OPTIONS pattern, plus adds a blocking `forbidden-cors-headers` CI job to lock in the cleanup against future regression. **With this merged, the only remaining P0 is #6 (rate limiting on non-auth endpoints — `add-rate-limiting` convoy).** ## Convoy artifacts - Convoy file: `.convoys/cors-tighten.md` (architect ratified all 5 decisions self-ratifiable) - Single brief: `.convoys/cors-tighten/brief-1-sweep-wildcard-cors.md` - Architect commit: `ec22b70` - Implementer commit: `a843736` ## Decisions ratified (all 5 architect-self-ratifiable) | | Decision | Choice | |---|---|---| | 1 | Narrow (verify.js only) vs Expanded (all 24) | **Expanded — all 24** | | 2 | OPTIONS handler shape | **Delete entirely** (match Brief 4 precedent on login.js / register.js) | | 3 | verify.js Allow-Methods tightening | **Moot** (subsumed by D2's deletion) | | 4 | New per-route handler tests | **No** (queued for fill-vitest-handler-coverage convoy) | | 5 | CI regression-lock job | **Yes** — new `forbidden-cors-headers` job, modeled on `forbidden-endpoints` | ## Pattern-drift audit Parent + architect spot-checked **14 of 24** files (zero drift). All 24 share the identical 9-11-line scaffolded block. Two safe sub-shapes: - **Pattern A** (16 files) — top-level method gate immediately after the CORS block; deletion leaves clean top-of-handler - **Pattern B** (8 files) — method-branched inside `try`; deletion routes OPTIONS to the existing `else { 405 }` branch Both patterns accept the same mechanical deletion. Architect's full pattern map in `.convoys/cors-tighten/brief-1-sweep-wildcard-cors.md`. ## Architect's most non-obvious finding `.cursor/rules/api-routes.mdc` documents `withCollectionPermission(...)` as a convention, but **zero `pages/api/**` routes actually use it** (architect grep returned empty). The stress-test concern about CORS headers being inside vs outside a permission-wrapper turned out to be moot. All 24 routes call `getUserFromRequest` directly inside the handler body. ## Diff **25 files, +29 / -261**. Pure deletion across 24 source files; 29 additions are the new CI job. ``` .github/workflows/ci.yml | 29 +++++++++++++++++++++++ pages/api/admin/index.js | 11 --------- pages/api/auth/verify.js | 11 --------- ... (22 more pages/api/** files, ~10-11 deletions each) 25 files changed, 29 insertions(+), 261 deletions(-) ``` ## Precedent files explicitly untouched `pages/api/auth/login.js` and `pages/api/auth/register.js` (already swept by Brief 4) are byte-identical at HEAD. Architect's R7 foot-gun avoided. ## Verification | Check | Result | |---|---| | `npm run lint` | exit 1, **128 problems** (baseline match) | | `npm run test:run` (vitest) | **21/21 pass** | | `git grep -nE "Access-Control-Allow-(Origin\|Methods\|Headers)" -- 'pages/api/**'` | **zero matches** | | `git grep -nE "if \(req\.method === 'OPTIONS'\)" -- 'pages/api/**'` | **zero matches** | | New CI job grep, local | exit 0, "OK: no Access-Control-Allow-* headers under pages/api/." | | `ci.yml` YAML parse | OK | | Precedent files diff | empty (untouched) | ## Expected CI behavior - **`forbidden-cors-headers`** (new job): pass (just swept; nothing to find) - **`forbidden-endpoints`** (existing): pass (no change in scope) - **Lint**: still 128 problems - **Vitest**: still 21/21 - **Playwright smoke**: should pass 3/3 — smoke navigates to `/login` and `/api/health` which are same-origin from the Vercel preview perspective (CORS irrelevant). The login → JWT-verify flow doesn't break because the smoke spec doesn't actually submit credentials. - **Screenshot diff**: not triggered (path filter excludes API-only changes) ## What did NOT change - `lib/**`, `components/**`, `scripts/**`, `test/**`, `package.json`, lockfile, `AGENTS.md`, `.cursor/rules/`, any other workflow YAML - `pages/api/auth/login.js` + `register.js` (Brief 4's earlier sweep preserved) - `pages/api/auth/login.js`'s rate-limit gate (Brief 4) and `pages/api/auth/register.js`'s rate-limit gate (Brief 4) — both untouched ## Out of scope (deferred / flagged) - **Real CORS layer for future mobile / 3rd-party API consumer** — YAGNI; design from scratch when actually needed (probably middleware-based) - **Per-route handler unit tests** — queued for `fill-vitest-handler-coverage` (none of the 24 routes have vitest coverage today) - **`add-rate-limiting`** (P0 #6 PARTIAL) — last remaining P0; queued for next convoy ## Test plan - [x] Local lint matches 128 baseline - [x] Local vitest 21/21 - [x] Grep gates clean - [x] YAML valid - [x] Precedent files untouched - [ ] CI `forbidden-cors-headers` job passes (verify post-push) - [ ] CI all other checks green (verify post-push) - [ ] Playwright smoke 3/3 against Vercel preview (verify post-push) Made with [Cursor](https://cursor.com)

vercel[bot] commented

2026-05-24 21:38:56 -04:00

(Migrated from github.com)

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
tcg-vault	Ready	Preview, Comment	May 25, 2026 1:38am

[vc]: #bbQFDBui1Xe8PlCNRZpJejjHYJcEmgIlNMOx6iPRfwc=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWNvbnZveS1jb3JzLXRpZ2h0ZW4tcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzLnZlcmNlbC5hcHAifSwiaW5zcGVjdG9yVXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3JhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy90Y2ctdmF1bHQvRUw4dlNhMzJXSGFicm5XRndoY2RlNDZReWdpcCIsInByZXZpZXdVcmwiOiJ0Y2ctdmF1bHQtZ2l0LWNvbnZveS1jb3JzLXRpZ2h0ZW4tcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzLnZlcmNlbC5hcHAiLCJuZXh0Q29tbWl0U3RhdHVzIjoiREVQTE9ZRUQifV0sInJlcXVlc3RSZXZpZXdVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vdmVyY2VsLWFnZW50L3JlcXVlc3QtcmV2aWV3P293bmVyPXZhcnV0YXN1JnJlcG89dGNnLXZhdWx0JnByPTE5In0= The latest updates on your projects. Learn more about [Vercel for GitHub](https://vercel.link/github-learn-more). | Project | Deployment | Actions | Updated (UTC) | | :--- | :----- | :------ | :------ | | [tcg-vault](https://vercel.com/randall-stillwells-projects/tcg-vault) | ![Ready](https://vercel.com/static/status/ready.svg) [Ready](https://vercel.com/randall-stillwells-projects/tcg-vault/EL8vSa32WHabrnWFwhcde46Qygip) | [Preview](https://tcg-vault-git-convoy-cors-tighten-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-convoy-cors-tighten-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 25, 2026 1:38am | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=19" rel="noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://agents-vade-review.vercel.sh/request-review-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://agents-vade-review.vercel.sh/request-review-light.svg"><img src="https://agents-vade-review.vercel.sh/request-review-light.svg" alt="Request Review"></picture></a>

github-actions[bot] commented

2026-05-24 21:39:03 -04:00

(Migrated from github.com)

Pipeline Health

Build + CI gates

Gate	Status
Vercel build (Preview)	✅ pass
CI: Lint	✅ pass
CI: Schema map fresh	❌ skipped
Preview smoke	✅ pass
Visual diff	✅ pass

Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build).

Role reports

Role	Status
Reviewer report	⏳ pending
A11y audit	⏳ pending
Design system audit	⏳ pending

See individual comments above for details. This rollup updates automatically.

## Pipeline Health ### Build + CI gates | Gate | Status | | --- | --- | | Vercel build (Preview) | ✅ pass | | CI: Lint | ✅ pass | | CI: Schema map fresh | ❌ skipped | | Preview smoke | ✅ pass | | Visual diff | ✅ pass | _Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build)._ ### Role reports | Role | Status | | --- | --- | | Reviewer report | ⏳ pending | | A11y audit | ⏳ pending | | Design system audit | ⏳ pending | See individual comments above for details. This rollup updates automatically.

github-actions[bot] commented

2026-05-24 21:39:57 -04:00

(Migrated from github.com)

Visual Diff

Screenshots and diffs uploaded as artifacts: view run

If intentional changes: update snapshots locally with npx playwright test --project=visual --update-snapshots and commit.

## Visual Diff Screenshots and diffs uploaded as artifacts: [view run](https://github.com/varutasu/tcg-vault/actions/runs/26378806554) If intentional changes: update snapshots locally with `npx playwright test --project=visual --update-snapshots` and commit.