rstillwell/deckhearth

Fork 0

import + gate import routes (P0 #6 - closes last P0) #20

Merged

varutasu merged 3 commits from convoy/add-rate-limiting into main

2026-05-25 00:00:00 -04:00

varutasu commented

2026-05-24 23:49:40 -04:00

(Migrated from github.com)

Summary

Closes the LAST open P0 ship-blocker. With this merge, all 8 of 8 P0s in .convoys/ship-readiness.md are RESOLVED — the launch-readiness checklist is empty for P0s.

fix-auth-bypass Brief 4 shipped lib/rate-limit.js with a single 5/15min auth limiter wired into login + register. This convoy extends the module to 5 named limiters (auth/search/upload/generate/import) and wires them into the remaining abusable surface, plus closes a critical secondary discovery: the import-routes auth gap.

Convoy artifacts

Convoy file: .convoys/add-rate-limiting.md (architect ratified 6 decisions; D1 routed back for operator gate-1)
Single brief: .convoys/add-rate-limiting/brief-1-extend-rate-limit-and-wire-routes.md
Architect commit: 60b842e
Implementer commit: 51a3a97

Architect's most important discovery

Investigating Decision 1 (scope expansion for import routes), the architect found:

pages/api/cards/import-{mtg,pokemon,lorcana}.js had ZERO auth checks. Publicly callable, hit external APIs (Scryfall / Pokémon-TCG / Lorcana) with no caller throttling, performed unbounded DB writes.
pages/admin/card-import.js's fetch sent NO Authorization header today. Adding getUserFromRequest to the import APIs without atomically fixing the admin UI would have returned 401 on every "Import Cards" click — a broken-but-passing PR (the admin UI isn't smoke-tested).

The brief atomically does both: API gating + admin UI Bearer-header fix in one commit.

Decisions ratified

	Decision	Choice
1	Scope expansion for import routes	Option A — gate all 3 uniformly + atomic admin UI fix (operator-ratified)
2	Named-limiter shape	Hybrid — preserve `checkAuthRateLimit` contract; add 4 named functions; `Map<className, Ratelimit>` cache; per-class Redis prefix
3	Per-class limits	auth 5/15min IP (unchanged), search 60/1min IP (bumped from 30 — ShareModal has no debounce; 17-char email = 16 requests in <5s), upload 10/1hr user, generate 5/1hr user (kept at 5 — DiceBear is free), import 5/1hr user
4	Identifier extraction	Two extractors. `extractUserIdentifier` THROWS on null/undefined/empty/NaN userId to prevent silent fallback-to-IP that would convert per-user limits into household-IP lockouts
5	429 response shape	Uniform — `'Too many attempts. Try again later.'` matches login.js/register.js (anti-fingerprinting)
6	New per-route handler tests	No — defer to `fill-vitest-handler-coverage`

Diff

10 files, +185 / -23.

 .cursor/rules/api-routes.mdc      | 41 ++++++++++++++----
 lib/rate-limit.js                 | 90 +++++++++++++++++++++++++++++++++------
 pages/admin/card-import.js        |  1 +
 pages/api/cards/import-lorcana.js | 16 +++++++
 pages/api/cards/import-mtg.js     | 16 +++++++
 pages/api/cards/import-pokemon.js | 16 +++++++
 pages/api/cards/search.js         |  7 +++
 pages/api/user/avatar.js          |  7 +++
 pages/api/user/avatar/generate.js |  7 +++
 pages/api/users/search.js         |  7 +++
 10 files changed, 185 insertions(+), 23 deletions(-)

pages/api/auth/login.js + pages/api/auth/register.js byte-identical at HEAD (Brief 4's contract preserved).

Per-class limiter inventory

Limiter	Limit/window	Key	Routes
`checkAuthRateLimit` (unchanged)	5 / 15min	IP	`login.js`, `register.js`
`checkSearchRateLimit` (new)	60 / 1min	IP	`users/search.js`, `cards/search.js`
`checkUploadRateLimit` (new)	10 / 1hr	user	`user/avatar.js`
`checkGenerateRateLimit` (new)	5 / 1hr	user	`user/avatar/generate.js`
`checkImportRateLimit` (new)	5 / 1hr	user (admin-only)	`cards/import-{mtg,pokemon,lorcana}.js`

Verification

Check	Result
`npm run lint`	exit 1, 128 problems (baseline match)
`npm run test:run` (vitest)	21/21 pass
Per-class grep counts (auth=2, search=2, upload=1, generate=1, import=3)	All match
Admin-role check (`user.role !== 'admin'`) in `import-*.js`	3 matches (one per import route)
Bearer header in admin UI	1 match at line 47 (`'auth_token'` key, matching codebase convention)
Brief 4 files (`login.js`, `register.js`) byte-identical	empty diff ✓
Per-class Redis prefix uniqueness	5 distinct (`tcgvault:auth/search/upload/generate/import`)

CI results (post-push run 26382185019)

Playwright smoke: 3/3 in 3.8s against the post-rate-limit Vercel preview (home 431ms, sign-in 331ms, /api/health 193ms) — confirms the new search limiter (60/min) does NOT 429 the smoke spec
forbidden-cors-headers (from cors-tighten): pass
forbidden-endpoints (existing): pass
Lint: 128 baseline
Vitest: 21/21
Screenshot diff: continue-on-error swallow per adopt-playwright-smoke Decision 4 (no baseline yet)
All other gates green

Post-merge state

P0 status table will become:

Item	Status
P0 #1 — `getUserFromRequest` hardcoded admin	RESOLVED
P0 #2 — `JWT_SECRET` hardcoded fallback	RESOLVED
P0 #3 — Default admin credentials in seed	RESOLVED
P0 #4 — Dev-only test endpoints	RESOLVED
P0 #5 — Wildcard CORS on API surface	RESOLVED
P0 #6 — No rate limiting	RESOLVED ← this PR
P0 #7 — Layout default-prop leaks email	RESOLVED
P0 #8 — Next.js vulnerable version	RESOLVED

8 of 8 RESOLVED. P1 backlog (pick-a-name, single-auth-provider, single-sql-client, migration-tool, etc.) becomes the next focus.

Out of scope (queued)

delete-dead-lorcana-import — Lorcana is gated defensively even though dead in frontend
god-function-split / refactor-cards-search-sql — cards/search.js's 240-line SQL god-function untouched
harden-multipart-parser — user/avatar.js's body streaming defense
withAdmin(handler) wrapper extraction — queued adjacent to single-auth-provider
fill-vitest-handler-coverage — per-route handler tests
Global IP backstop middleware

Test plan

Local lint = 128 baseline
Local vitest 21/21
Per-class grep counts match
Admin role check on all 3 import routes
Admin UI sends Bearer token
Brief 4 contract preserved (login.js + register.js untouched)
CI all green
Playwright smoke 3/3 (no false 429 from the new search limiter)
Manual: admin user can still trigger imports via the UI (smoke-test on Vercel preview before merge — the atomic admin UI Bearer fix is testable here)

## Summary **Closes the LAST open P0 ship-blocker.** With this merge, all 8 of 8 P0s in `.convoys/ship-readiness.md` are RESOLVED — the launch-readiness checklist is empty for P0s. `fix-auth-bypass` Brief 4 shipped `lib/rate-limit.js` with a single 5/15min auth limiter wired into login + register. This convoy extends the module to **5 named limiters** (auth/search/upload/generate/import) and wires them into the remaining abusable surface, plus closes a critical secondary discovery: the import-routes auth gap. ## Convoy artifacts - Convoy file: `.convoys/add-rate-limiting.md` (architect ratified 6 decisions; D1 routed back for operator gate-1) - Single brief: `.convoys/add-rate-limiting/brief-1-extend-rate-limit-and-wire-routes.md` - Architect commit: `60b842e` - Implementer commit: `51a3a97` ## Architect's most important discovery Investigating Decision 1 (scope expansion for import routes), the architect found: 1. **`pages/api/cards/import-{mtg,pokemon,lorcana}.js` had ZERO auth checks.** Publicly callable, hit external APIs (Scryfall / Pokémon-TCG / Lorcana) with no caller throttling, performed unbounded DB writes. 2. **`pages/admin/card-import.js`'s fetch sent NO `Authorization` header today.** Adding `getUserFromRequest` to the import APIs without atomically fixing the admin UI would have returned 401 on every "Import Cards" click — a broken-but-passing PR (the admin UI isn't smoke-tested). The brief atomically does both: API gating + admin UI Bearer-header fix in one commit. ## Decisions ratified | | Decision | Choice | |---|---|---| | 1 | Scope expansion for import routes | **Option A — gate all 3 uniformly + atomic admin UI fix** (operator-ratified) | | 2 | Named-limiter shape | **Hybrid** — preserve `checkAuthRateLimit` contract; add 4 named functions; `Map<className, Ratelimit>` cache; per-class Redis prefix | | 3 | Per-class limits | auth 5/15min IP (unchanged), search **60**/1min IP (bumped from 30 — ShareModal has no debounce; 17-char email = 16 requests in <5s), upload 10/1hr user, generate **5**/1hr user (kept at 5 — DiceBear is free), import 5/1hr user | | 4 | Identifier extraction | Two extractors. **`extractUserIdentifier` THROWS** on null/undefined/empty/NaN userId to prevent silent fallback-to-IP that would convert per-user limits into household-IP lockouts | | 5 | 429 response shape | **Uniform** — `'Too many attempts. Try again later.'` matches login.js/register.js (anti-fingerprinting) | | 6 | New per-route handler tests | **No** — defer to `fill-vitest-handler-coverage` | ## Diff 10 files, **+185 / -23**. ``` .cursor/rules/api-routes.mdc | 41 ++++++++++++++---- lib/rate-limit.js | 90 +++++++++++++++++++++++++++++++++------ pages/admin/card-import.js | 1 + pages/api/cards/import-lorcana.js | 16 +++++++ pages/api/cards/import-mtg.js | 16 +++++++ pages/api/cards/import-pokemon.js | 16 +++++++ pages/api/cards/search.js | 7 +++ pages/api/user/avatar.js | 7 +++ pages/api/user/avatar/generate.js | 7 +++ pages/api/users/search.js | 7 +++ 10 files changed, 185 insertions(+), 23 deletions(-) ``` `pages/api/auth/login.js` + `pages/api/auth/register.js` byte-identical at HEAD (Brief 4's contract preserved). ## Per-class limiter inventory | Limiter | Limit/window | Key | Routes | |---|---|---|---| | `checkAuthRateLimit` (unchanged) | 5 / 15min | IP | `login.js`, `register.js` | | `checkSearchRateLimit` (new) | 60 / 1min | IP | `users/search.js`, `cards/search.js` | | `checkUploadRateLimit` (new) | 10 / 1hr | user | `user/avatar.js` | | `checkGenerateRateLimit` (new) | 5 / 1hr | user | `user/avatar/generate.js` | | `checkImportRateLimit` (new) | 5 / 1hr | user (admin-only) | `cards/import-{mtg,pokemon,lorcana}.js` | ## Verification | Check | Result | |---|---| | `npm run lint` | exit 1, **128 problems** (baseline match) | | `npm run test:run` (vitest) | **21/21 pass** | | Per-class grep counts (auth=2, search=2, upload=1, generate=1, import=3) | All match | | Admin-role check (`user.role !== 'admin'`) in `import-*.js` | 3 matches (one per import route) | | Bearer header in admin UI | 1 match at line 47 (`'auth_token'` key, matching codebase convention) | | Brief 4 files (`login.js`, `register.js`) byte-identical | empty diff ✓ | | Per-class Redis prefix uniqueness | 5 distinct (`tcgvault:auth/search/upload/generate/import`) | ## CI results (post-push run 26382185019) - **Playwright smoke: 3/3 in 3.8s** against the post-rate-limit Vercel preview (home 431ms, sign-in 331ms, /api/health 193ms) — confirms the new search limiter (60/min) does NOT 429 the smoke spec - `forbidden-cors-headers` (from `cors-tighten`): pass - `forbidden-endpoints` (existing): pass - Lint: 128 baseline - Vitest: 21/21 - Screenshot diff: continue-on-error swallow per `adopt-playwright-smoke` Decision 4 (no baseline yet) - All other gates green ## Post-merge state P0 status table will become: | Item | Status | |---|---| | P0 #1 — `getUserFromRequest` hardcoded admin | RESOLVED | | P0 #2 — `JWT_SECRET` hardcoded fallback | RESOLVED | | P0 #3 — Default admin credentials in seed | RESOLVED | | P0 #4 — Dev-only test endpoints | RESOLVED | | P0 #5 — Wildcard CORS on API surface | RESOLVED | | **P0 #6 — No rate limiting** | **RESOLVED ← this PR** | | P0 #7 — Layout default-prop leaks email | RESOLVED | | P0 #8 — Next.js vulnerable version | RESOLVED | **8 of 8 RESOLVED.** P1 backlog (`pick-a-name`, `single-auth-provider`, `single-sql-client`, `migration-tool`, etc.) becomes the next focus. ## Out of scope (queued) - `delete-dead-lorcana-import` — Lorcana is gated defensively even though dead in frontend - `god-function-split` / `refactor-cards-search-sql` — `cards/search.js`'s 240-line SQL god-function untouched - `harden-multipart-parser` — `user/avatar.js`'s body streaming defense - `withAdmin(handler)` wrapper extraction — queued adjacent to `single-auth-provider` - `fill-vitest-handler-coverage` — per-route handler tests - Global IP backstop middleware ## Test plan - [x] Local lint = 128 baseline - [x] Local vitest 21/21 - [x] Per-class grep counts match - [x] Admin role check on all 3 import routes - [x] Admin UI sends Bearer token - [x] Brief 4 contract preserved (login.js + register.js untouched) - [x] CI all green - [x] Playwright smoke 3/3 (no false 429 from the new search limiter) - [ ] Manual: admin user can still trigger imports via the UI (smoke-test on Vercel preview before merge — the atomic admin UI Bearer fix is testable here)

vercel[bot] commented

2026-05-24 23:49:45 -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 3:49am

[vc]: #bJQPfUO46gDyaeFYg5QmNkwKS+NHSmmczEV0P0hqiOk=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWNvbnZveS1hZGQtcmF0LTQ0ZTI4Ny1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCJ9LCJpbnNwZWN0b3JVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzL3RjZy12YXVsdC9DV0tXalBTMVpOYmZFUTQ1bkhXRDhYRXE0WXVFIiwicHJldmlld1VybCI6InRjZy12YXVsdC1naXQtY29udm95LWFkZC1yYXQtNDRlMjg3LXJhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy52ZXJjZWwuYXBwIiwibmV4dENvbW1pdFN0YXR1cyI6IkRFUExPWUVEIn1dLCJyZXF1ZXN0UmV2aWV3VXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3ZlcmNlbC1hZ2VudC9yZXF1ZXN0LXJldmlldz9vd25lcj12YXJ1dGFzdSZyZXBvPXRjZy12YXVsdCZwcj0yMCJ9 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/CWKWjPS1ZNbfEQ45nHWD8XEq4YuE) | [Preview](https://tcg-vault-git-convoy-add-rat-44e287-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-convoy-add-rat-44e287-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 25, 2026 3:49am | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=20" 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 23:49:49 -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 23:50:52 -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/26382185038) If intentional changes: update snapshots locally with `npx playwright test --project=visual --update-snapshots` and commit.