Closes P0 #6 (no rate limiting) from PARTIAL → RESOLVED. With
this merge, all 8 P0 ship-blockers are RESOLVED. fix-auth-bypass
Brief 4 shipped lib/rate-limit.js with a single 5/15min auth
limiter wired into login + register; this brief extends the
module to 5 named limiters (auth/search/upload/generate/import)
and wires them into the remaining abusable surface.
Per architect Decision 1 — Option A (gate all 3 import routes
uniformly). The architect's investigation found a critical
secondary bug: pages/admin/card-import.js's fetch sends NO
Authorization header today. Adding getUserFromRequest to the
import APIs without fixing the admin UI atomically would have
returned 401 on every "Import Cards" click. Both edits ship in
this single commit — API gating + admin UI Bearer fix — for
atomic safety. Lorcana is dead in frontend today (only
scripts/import-lorcana.js uses that path) but gated uniformly
to future-proof per AGENTS.md § 1 status; a
delete-dead-lorcana-import follow-up convoy is queued for later
if we decide to drop Lorcana entirely.
Per Decision 2 — hybrid named-limiter shape in lib/rate-limit.js.
checkAuthRateLimit(req) signature + return shape preserved
verbatim (don't break Brief 4's contract); 4 new named functions
added (checkSearchRateLimit, checkUploadRateLimit,
checkGenerateRateLimit, checkImportRateLimit). Map<className,
Ratelimit> cache, per-class Redis prefix (tcgvault:auth,
tcgvault:search, tcgvault:upload, tcgvault:generate,
tcgvault:import) so each class has its own budget.
Per Decision 3 — per-class limit values tuned with evidence:
auth 5 / 15min IP-keyed (unchanged from Brief 4)
search 60 / 1min IP-keyed (bumped from 30 — ShareModal
has no debounce; 17-char email
= 16 requests in <5s)
upload 10 / 1hr user-keyed
generate 5 / 1hr user-keyed (DiceBear is free, kept at 5)
import 5 / 1hr user-keyed (admin-only; external APIs
have their own limits)
Per Decision 4 — two extractors. extractIpIdentifier (existing,
unchanged) and extractUserIdentifier (new). The new one THROWS on
null/undefined/empty/NaN userId to prevent silent fallback-to-IP
(which would convert per-user limits into per-IP and lock out
households). Architect's R-finding: places the gate AFTER the
auth check on every per-user-keyed route, never before.
Per Decision 5 — uniform 429 response shape verbatim matching
login.js/register.js: Retry-After header + JSON
{ error: 'Too many attempts. Try again later.' }. Anti-
fingerprinting (per-class messages would tell an attacker which
classes have which limits).
Per Decision 6 — no new per-route handler tests this convoy.
Vitest 21/21 unchanged at merge.
Verification:
- npm run lint: 128 problems (baseline match)
- npm run test:run: 21/21 vitest pass (no regression;
auth-utils tests don't transitively load rate-limit per
architect D6 evidence)
- 5 named limiter exports verified via per-route grep counts
- Admin UI sends Authorization: Bearer <token> from
localStorage in the import fetch (matching pattern from
other admin pages)
- Brief 4's login.js + register.js byte-identical at HEAD
- .cursor/rules/api-routes.mdc § Rate limiting extended with
per-class table + gate-ordering rules
No new dependencies (Brief 4's @upstash/ratelimit + @upstash/redis
suffice). No workflow YAML changes. No AGENTS.md edits (doc-
writer pass at convoy close handles Gotcha #12 update + § 6
testing update + ship-readiness Status summary 7/8 → 8/8).
Co-authored-by: Cursor <cursoragent@cursor.com>
Six decisions ratified — five architect-self-ratifiable, one
(D1, scope expansion to gate the three import routes with auth +
admin-role + import rate-limit, plus the matching Bearer-token fix in
pages/admin/card-import.js) routed back for operator approval before
implementer dispatch.
Architecture: single brief, ~180 LOC across 10 files. Lib refactor
preserves Brief 4's checkAuthRateLimit(req) contract; adds four named
exports (search/upload/generate/import) with a Map<className, Ratelimit>
cache and distinct Redis prefix per class. Per-class limits tuned
against real client behavior — search bumped from 30 to 60/min after
finding ShareModal handleSearch has no debounce; generate bumped from 3
to 5/hour after confirming the generator uses DiceBear (free) not a
paid AI service.
Brief includes verbatim new lib/rate-limit.js shape so the implementer
has zero design discretion, plus per-route verbatim post-edit shapes
for all seven gated handlers and the admin-UI fetch fix.
Decision 1 investigation finding: pages/admin/card-import.js currently
fetches without an Authorization header. Without the matching fix in
this brief, adding getUserFromRequest to the import APIs immediately
breaks the admin UI on first run — the brief is internally consistent
only with both edits shipping together.
Decision 6 correction: the convoy file's claim that vitest transitively
loads lib/rate-limit.js is stale (rg verified no test imports it). The
lib refactor is strictly safer than the convoy file implied.
Co-authored-by: Cursor <cursoragent@cursor.com>
Scaffolds the add-rate-limiting convoy. fix-auth-bypass Brief 4
shipped lib/rate-limit.js with a single 5/15min auth-only limiter
wired into login + register; this convoy extends the surface to
search, upload, and (pending Decision 1) import routes.
Parent's pre-architect audit surfaced a critical secondary
finding beyond "missing rate limit": pages/api/cards/import-*.js
(3 files) have ZERO auth checks. They are publicly callable,
hit external APIs (Scryfall / Pokémon / Lorcana) with no caller
throttling, and perform unbounded DB writes. Rate-limit alone
won't close P0 #6 cleanly.
Architect Decision 1 routes the scope choice:
Option A — in-scope: add auth gates + rate limit to import
routes in this convoy. ~30 LOC across 3 files. Parent
recommends — precedent from drop-public-setup Brief 2 (mid-
convoy CJS/ESM expansion).
Option B — spin out: stay narrow on the 4 listed routes; queue
gate-import-routes follow-up. Mark P0 #6 RESOLVED-with-caveat.
Option C — rate-limit-only on imports: worst option (leaves
abusive anonymous endpoint live).
Six decisions queued (scope expansion, named-limiter shape,
per-class limits, identifier extraction, 429 response shape,
test coverage). No operator action required — Upstash env vars
already auto-provisioned by Brief 4.
If Option A wins, this convoy closes the last open P0 ship-
blocker (8/8 RESOLVED) and the launch checklist becomes empty.
Co-authored-by: Cursor <cursoragent@cursor.com>
