deckhearth/.convoys/add-rate-limiting/brief-1-extend-rate-limit-and-wire-routes.md

convoy	brief_number	depends_on	files
add-rate-limiting	1		lib/rate-limit.js pages/api/users/search.js pages/api/cards/search.js pages/api/user/avatar.js pages/api/user/avatar/generate.js pages/api/cards/import-mtg.js pages/api/cards/import-pokemon.js pages/api/cards/import-lorcana.js pages/admin/card-import.js .cursor/rules/api-routes.mdc

Brief 1: Extend lib/rate-limit.js to named per-class limiters + wire into the remaining abusable endpoints + gate the import routes

Goal (1 sentence)

Refactor lib/rate-limit.js from a single auth-only limiter into a Map-of-named-limiters shape (preserving the checkAuthRateLimit(req) contract that login.js + register.js depend on per Brief 4), add four new named exports — checkSearchRateLimit(req), checkUploadRateLimit(req, userId), checkGenerateRateLimit(req, userId), checkImportRateLimit(req, userId) — wire each into the appropriate handler at the documented gate-ordering (auth before rate-limit for user-keyed classes), add getUserFromRequest + admin-role check + import rate-limit to the three currently-anonymous pages/api/cards/import-*.js routes, fix pages/admin/card-import.js to send the Bearer token the newly-gated import routes require, and extend .cursor/rules/api-routes.mdc § Rate limiting with the per-class pattern + a per-class limit table.

Files in scope (do not edit anything else)

The 10 files listed in files: above (all modified, no new files, no deletions).

Files explicitly out of scope (do not touch even if it seems related):

• pages/api/auth/login.js, pages/api/auth/register.js — already wired by fix-auth-bypass Brief 4. Verify post-edit that they still work (call checkAuthRateLimit(req) against the refactored module), but do NOT modify them.
• lib/permission-middleware.js, lib/auth-secret.js, pages/api/auth-utils.js — auth surface is untouched by this convoy.
• package.json, package-lock.json — @upstash/ratelimit@^2.0.8 + @upstash/redis@^1.38.0 are already installed (Brief 4). No new dependencies. No version bumps.
• AGENTS.md — Gotcha #12 documents the env-var requirement; the doc-writer pass at convoy close will update the gotcha to reflect the new per-class limits. Do NOT preempt that edit here.
• .github/workflows/ci.yml — no new CI gate is added. The forbidden-endpoints + forbidden-cors-headers jobs already defend the API surface; per-class rate-limit wiring isn't grep-checkable.
• test/** — no new per-route handler tests in this convoy (Decision 6 below). Vitest 21/21 must still pass with no spec changes.
• tests/smoke/**, tests/visual/** — smoke + visual suites don't exercise any of these endpoints; do NOT modify.
• pages/api/cards/search.js's SQL — the file has a known god-function shape with 7+ conditional SQL branches (SELECT * FROM cards WHERE … repeated). That's god-function-split / refactor-cards-search-sql scope, NOT here. Do NOT touch any of the SQL branches; only add the rate-limit gate at the top.
• pages/api/user/avatar.js's parseMultipartFormData body-streaming behavior — the 5MB multipart body is consumed before any rate-limit gate could short-circuit, meaning an attacker can still exhaust the 5MB body even on a 429 path. That's harden-multipart-parser scope (queued as a follow-up); the gate-ordering in this brief is purely defensive (rate-limit BEFORE the method branches so the gate fires before the body parse).
• scripts/import-*.js — standalone scripts independent of the API routes; do NOT touch.
• Any other pages/api/**/*.js file. The convoy scope is the 7 surfaces listed in .convoys/add-rate-limiting.md § Scope.

Conventions to follow

Decisions from the convoy file (cite when implementing)

• D1 (operator-ratified): Option A — gate all three pages/api/cards/import-*.js routes in this convoy with getUserFromRequest + admin-role check + per-user rate-limit. ALSO update pages/admin/card-import.js to send the Authorization: Bearer ${localStorage.getItem('auth_token')} header on the import fetch (necessary scope expansion — without it, the gated APIs immediately break the admin UI). Lorcana is gated defensively even though no current frontend caller exists; future cleanup convoy can delete if it stays unused.
• D2 (architect-self-ratified): Hybrid named-limiter shape — preserve checkAuthRateLimit(req) (Brief 4 contract, used by login + register), add four new named functions (checkSearchRateLimit, checkUploadRateLimit, checkGenerateRateLimit, checkImportRateLimit). Internal Map<className, Ratelimit> cache, distinct Redis prefix per class.
• D3 (architect-self-ratified): Per-class limits — auth 5/15min IP (unchanged), search 60/1min IP, upload 10/1hour user, generate 5/1hour user, import 5/1hour user. Search raised from parent's 30 because components/ShareModal.js's handleSearch fires on every keystroke (no debounce); typing a 17-char email = 17 requests in <5s, which would 429 at 30/1min. Generate raised from parent's 3 because pages/api/user/avatar/generate.js calls DiceBear (free public API), not OpenAI/Replicate; cost is just Vercel blob storage + DiceBear-side throttling.
• D4 (architect-self-ratified): Two-extractor shape — extractIpIdentifier(req) (existing) + extractUserIdentifier(userId) (new). extractUserIdentifier throws when userId is null/undefined/'' (defensive — if a future handler accidentally calls a user-keyed limiter before the auth check, the throw surfaces the misordering immediately rather than silently falling back to IP and quietly converting a per-user limit into a per-IP limit, which would lock out other household members for one user's behavior). Documented in the verbatim shape below.
• D5 (architect-self-ratified): Uniform 429 message — 'Too many attempts. Try again later.' matches login.js + register.js verbatim. Per-class variation would fingerprint which routes have which limits to an attacker.
• D6 (architect-self-ratified): No new per-route handler tests in this convoy. Deferred to queued fill-vitest-handler-coverage (same reasoning as cors-tighten Decision D4). Vitest 21/21 MUST still pass after the lib refactor — verified at architect time that no current vitest spec transitively imports lib/rate-limit.js (only login.js + register.js import it, and neither is covered by vitest; the convoy file's claim that "auth-utils tests transitively load this module" is stale).

Repo conventions (cite + match)

• .cursor/rules/no-go-zones.mdc. None of the 10 source files are listed under no-go zones. The "Card-import jobs" entry warns "Don't run them ad-hoc against prod data; use staging" — this brief gates them with admin-role enforcement which directly answers that no-go-zones warning (only admins can trigger imports; non-admins get 403).
• .cursor/rules/api-routes.mdc § Rate limiting. The existing pattern documents the auth class (login/register). This brief extends it with the four new classes; the verbatim updated content is in the Acceptance criteria § for .cursor/rules/api-routes.mdc below. Keep the existing § Authentication, § Request validation, § Method gating, § Error handling, § Database access, § Response shape, § Activity logging, § Dev/test endpoints, and § CORS subsections byte-identical — only § Rate limiting changes.
• .cursor/rules/auth-and-permissions.mdc. Admin-role check uses if (user.role !== 'admin') directly (per the rule's "Admin-only" pattern: "check user.role === 'admin' directly; consider extracting withAdmin() if a third call site appears"). The three import routes are the third+fourth+fifth call sites in the codebase, but extracting withAdmin() is its own convoy — for this brief, inline the check.
• Brief 4 precedent shape (.convoys/fix-auth-bypass/brief-4-tighten-auth-surface.md). The verbatim 429 response shape comes from there:

  const { allowed, reset } = await checkXxxRateLimit(req[, userId]);
  if (!allowed) {
    res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
    return res.status(429).json({ error: 'Too many attempts. Try again later.' });
  }
  

  Apply this shape at each call site. Do not deviate — same error message, same Retry-After calculation, same status code.
• @upstash/ratelimit per-class prefix isolation. Each class gets a distinct Redis key prefix (tcgvault:auth, tcgvault:search, tcgvault:upload, tcgvault:generate, tcgvault:import). Without distinct prefixes, hits on one class would consume the budget of another (e.g., a search hit would eat the auth budget for the same IP). Verified against @upstash/ratelimit@2.0.8's prefix: option which scopes all keys with the given string.
• Lazy init() + fail-closed-in-prod / warn-and-noop-in-dev. Both behaviors carry through unchanged from Brief 4. New limiters inherit them via the shared init() function. Do NOT reintroduce module-top-level new Redis(...) — it would throw at import time in any environment without KV_REST_API_URL / KV_REST_API_TOKEN, breaking local dev, vitest, and Vercel build-time bundling.
• @vercel/postgres tagged-templates only. None of the per-route edits touch SQL. (cards/search.js is excluded from SQL refactoring per § Files explicitly out of scope.)

Acceptance criteria

lib/rate-limit.js (modified)

Replace the current 70-line module with the verbatim shape below. The diff is mostly net-additive (~70 lines added, ~5 lines reshaped); the existing init(), extractIdentifier(), and checkAuthRateLimit() functions are conceptually preserved but restructured to share infrastructure across all five classes.

Verbatim new module shape:

import { Ratelimit } from '@upstash/ratelimit';
import { Redis } from '@upstash/redis';

// Per-class limiter configuration. Distinct Redis prefix per class is
// REQUIRED — without it, a search-class hit would consume the auth-class
// budget for the same identifier. `slidingWindow` chosen across all
// classes to match Brief 4's existing algorithm; switching to
// `tokenBucket` per-class would be its own convoy.
const LIMITER_CONFIG = {
  auth:     { limit: 5,  window: '15 m', prefix: 'tcgvault:auth' },
  search:   { limit: 60, window: '1 m',  prefix: 'tcgvault:search' },
  upload:   { limit: 10, window: '1 h',  prefix: 'tcgvault:upload' },
  generate: { limit: 5,  window: '1 h',  prefix: 'tcgvault:generate' },
  import:   { limit: 5,  window: '1 h',  prefix: 'tcgvault:import' },
};

// Lazy singleton. Module-load init would throw in environments without
// Upstash env vars (local dev pre-onboarding, tests that transitively
// import the auth handlers, Vercel build-time bundling). Defer
// construction until the first request actually arrives.
let cached = null;

function init() {
  // Env-var names match Vercel's Upstash Marketplace integration, which
  // auto-provisions KV_REST_API_URL and KV_REST_API_TOKEN. See
  // https://upstash.com/docs/redis/howto/vercelintegration. Single-source-
  // of-truth — do NOT alias to UPSTASH_REDIS_REST_*.
  const url = process.env.KV_REST_API_URL;
  const token = process.env.KV_REST_API_TOKEN;

  if (url && token) {
    const redis = new Redis({ url, token });
    const instances = new Map();
    for (const [name, cfg] of Object.entries(LIMITER_CONFIG)) {
      instances.set(
        name,
        new Ratelimit({
          redis,
          limiter: Ratelimit.slidingWindow(cfg.limit, cfg.window),
          prefix: cfg.prefix,
        })
      );
    }
    return { mode: 'live', instances };
  }

  if (process.env.NODE_ENV === 'production') {
    // Fail-closed in production. A single failed login is a better outcome
    // than silently disabling brute-force protection on the live site.
    throw new Error(
      '[rate-limit] Upstash not configured. Set KV_REST_API_URL and KV_REST_API_TOKEN in the deployment environment (auto-provisioned by the Vercel Upstash Marketplace integration) before serving auth traffic.'
    );
  }

  console.warn(
    '[rate-limit] KV_REST_API_URL / KV_REST_API_TOKEN not set — rate limiting disabled (dev/test only)'
  );
  return { mode: 'noop' };
}

function extractIpIdentifier(req) {
  const xff = req.headers?.['x-forwarded-for'];
  const firstHop = Array.isArray(xff) ? xff[0] : xff?.split(',')[0]?.trim();
  return firstHop || req.socket?.remoteAddress || 'anonymous';
}

// THROWS on missing userId. Per-user limiters MUST sit AFTER the auth
// check in the handler body — silently falling back to IP here would
// convert a per-user limit into a per-IP limit, locking out other
// household members for one user's behavior. The throw surfaces the
// misordering immediately during development rather than at first
// production incident.
function extractUserIdentifier(userId) {
  if (
    userId === null ||
    userId === undefined ||
    userId === '' ||
    (typeof userId === 'number' && Number.isNaN(userId))
  ) {
    throw new Error(
      '[rate-limit] extractUserIdentifier called without an authenticated userId. Place the rate-limit gate AFTER the auth check, never before.'
    );
  }
  return `user:${userId}`;
}

async function check(className, identifier) {
  if (!cached) {
    cached = init();
  }

  if (cached.mode === 'noop') {
    return { allowed: true, remaining: Infinity, reset: 0 };
  }

  const limiter = cached.instances.get(className);
  if (!limiter) {
    throw new Error(`[rate-limit] Unknown limiter class: ${className}`);
  }

  try {
    const { success, remaining, reset } = await limiter.limit(identifier);
    return { allowed: success, remaining, reset };
  } catch (err) {
    // Fail-open on Upstash outage. A hard outage at the rate-limit backend
    // should not lock the entire user base out. Brute-force protection
    // lives behind defense-in-depth (Vercel firewall, etc.).
    console.error('[rate-limit]', err);
    return { allowed: true, remaining: Infinity, reset: 0 };
  }
}

export async function checkAuthRateLimit(req) {
  return check('auth', extractIpIdentifier(req));
}

export async function checkSearchRateLimit(req) {
  return check('search', extractIpIdentifier(req));
}

export async function checkUploadRateLimit(req, userId) {
  return check('upload', extractUserIdentifier(userId));
}

export async function checkGenerateRateLimit(req, userId) {
  return check('generate', extractUserIdentifier(userId));
}

export async function checkImportRateLimit(req, userId) {
  return check('import', extractUserIdentifier(userId));
}

Acceptance:

• File ends up as the verbatim shape above (whitespace and comments preserved). 2-space indent. ESM. No default export.
• checkAuthRateLimit(req) return shape is byte-identical to Brief 4's — { allowed: boolean, remaining: number, reset: number }. login.js + register.js MUST continue to work without any change to their import or call shape.
• No top-level await. No module-load new Redis(...). The cached = null declaration is the only top-level side effect.
• LIMITER_CONFIG keys are exactly auth, search, upload, generate, import — five entries, no more, no less.
• Each LIMITER_CONFIG[*].prefix is unique and follows the tcgvault:<class> pattern.
• extractUserIdentifier(userId) THROWS the documented error message on null, undefined, empty string, or NaN. (Numeric 0 is technically valid — there's no user with ID 0 in the schema, but the check is defensive against future ID types; the conditional explicitly does NOT throw on 0 because 0 === null is false and 0 === undefined is false. This is intentional — if a future change introduces user ID 0 the limiter still keys correctly.)
• check('unknown-class', ...) throws [rate-limit] Unknown limiter class: unknown-class (defensive; should never fire in shipped code).
• The five exported check*RateLimit functions are the ONLY exports. No legacy extractIdentifier re-export — it's been renamed to extractIpIdentifier and is module-private.

pages/api/users/search.js (modified)

Add a new import and a new rate-limit gate after the JWT verify success, before the query-length validation. The route uses inline jwt.verify (not getUserFromRequest) but that doesn't matter — the search class is IP-keyed, not user-keyed, so the gate doesn't need the user id.

Verbatim post-edit shape:

import { sql } from '@vercel/postgres';
import jwt from 'jsonwebtoken';
import { JWT_SECRET } from '../../../lib/auth-secret.js';
import { checkSearchRateLimit } from '../../../lib/rate-limit.js';

export default async function handler(req, res) {
  if (req.method !== 'GET') {
    return res.status(405).json({ error: 'Method not allowed' });
  }

  // Verify authentication
  const authHeader = req.headers.authorization;
  if (!authHeader || !authHeader.startsWith('Bearer ')) {
    return res.status(401).json({ error: 'Authentication required' });
  }

  const token = authHeader.substring(7);
  try {
    jwt.verify(token, JWT_SECRET);
  } catch (error) {
    return res.status(401).json({ error: 'Invalid token' });
  }

  const { allowed, reset } = await checkSearchRateLimit(req);
  if (!allowed) {
    res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
    return res.status(429).json({ error: 'Too many attempts. Try again later.' });
  }

  const { q: query } = req.query;

  if (!query || query.length < 2) {
    return res.status(400).json({ error: 'Query must be at least 2 characters' });
  }

  try {
    // Search users by email (partial match)
    const result = await sql`
      SELECT id, email, role, created_at
      FROM users 
      WHERE email ILIKE ${`%${query}%`}
      ORDER BY email
      LIMIT 10
    `;

    res.status(200).json({
      users: result.rows
    });

  } catch (error) {
    console.error('User search error:', error);
    res.status(500).json({ error: 'Internal server error' });
  }
} 

Acceptance:

• One new import line: import { checkSearchRateLimit } from '../../../lib/rate-limit.js'; (relative path matches the existing ../../../lib/auth-secret.js precedent on line 3).
• Gate sits between the JWT-verify try/catch (lines 17-21) and the query-length validation (line 25). NOT inside the JWT try block.
• Net diff: +1 import, +5 lines (the gate block), 0 deletions, 0 reorderings.

pages/api/cards/search.js (modified)

The route is anonymous-by-design (cards are a public catalogue). Gate at the very top of the handler, after the method check, before the existing try block. IP-keyed.

Verbatim post-edit shape (top of file only):

import { sql } from '@vercel/postgres';
import { checkSearchRateLimit } from '../../../lib/rate-limit.js';

export default async function handler(req, res) {
  if (req.method !== 'GET') {
    return res.status(405).json({ error: 'Method not allowed' });
  }

  const { allowed, reset } = await checkSearchRateLimit(req);
  if (!allowed) {
    res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
    return res.status(429).json({ error: 'Too many attempts. Try again later.' });
  }

  try {
    const { 
      query = '', 
      // ... rest of the file unchanged ...

Acceptance:

• One new import line. Relative path '../../../lib/rate-limit.js'.
• Gate sits between the method check (lines 4-6) and the try block (current line 8).
• The 240-line SQL god-function inside the try block is BYTE-IDENTICAL post-edit. Do NOT touch any of the 7 conditional SQL branches, the filter object, the response shape, or the closing catch. The only diff is +1 import and +5 lines for the gate block.
• Do NOT add getUserFromRequest to this route. It's anonymous-by-design per the convoy file's "Known constraints" § "Card-search is anonymous-by-design — do NOT add a getUserFromRequest check. The IP-keyed limit is the correct defense (search is a public catalogue feature)."

pages/api/user/avatar.js (modified)

The handler has a structure with NO top-level method gate; method-branches inside the outer try block. Auth check sits inside the try (lines 16-19). Gate goes AFTER the auth check, BEFORE the method-branching (if (req.method === 'POST') at line 21), so both the POST upload AND the DELETE branches inherit the limit. User-keyed, passing user.userId.

Verbatim post-edit shape (auth + gate region only):

import { put, del } from '@vercel/blob';
import { sql } from '@vercel/postgres';
import { getUserFromRequest } from '../../../lib/permission-middleware';
import { checkUploadRateLimit } from '../../../lib/rate-limit.js';

export const config = {
  api: {
    bodyParser: {
      sizeLimit: '5mb',
    },
  },
};

export default async function handler(req, res) {
  try {
    // Get authenticated user
    const user = await getUserFromRequest(req);
    if (!user) {
      return res.status(401).json({ error: 'Authentication required' });
    }

    const { allowed, reset } = await checkUploadRateLimit(req, user.userId);
    if (!allowed) {
      res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
      return res.status(429).json({ error: 'Too many attempts. Try again later.' });
    }

    if (req.method === 'POST') {
      // Handle avatar upload
      // ... rest of the file unchanged ...

Acceptance:

• One new import line: import { checkUploadRateLimit } from '../../../lib/rate-limit.js';.
• Gate sits between the if (!user) 401 (line 17-19) and the if (req.method === 'POST') branch (line 21).
• Gate fires BEFORE parseMultipartFormData(req) runs. The body-streaming bypass concern (5MB consumed before the gate) is acknowledged out-of-scope (see § Files explicitly out of scope) — but the gate ordering itself MUST be correct so that future hardening of the body parser doesn't need to also reorder the gate.
• Net diff: +1 import, +5 lines, 0 deletions. The POST branch, DELETE branch, helper functions (parseMultipartFormData, deleteOldAvatar), and the config export are byte-identical.

pages/api/user/avatar/generate.js (modified)

The handler HAS a top-level method gate (if (req.method !== 'POST') at line 6). Auth check sits inside the try block (lines 12-15). Gate goes AFTER the auth check, BEFORE the SQL query that fetches user data (line 18). User-keyed, passing user.userId.

Verbatim post-edit shape (top of handler only):

import { put } from '@vercel/blob';
import { sql } from '@vercel/postgres';
import { getUserFromRequest } from '../../../../lib/permission-middleware';
import { checkGenerateRateLimit } from '../../../../lib/rate-limit.js';

export default async function handler(req, res) {
  if (req.method !== 'POST') {
    return res.status(405).json({ error: 'Method not allowed' });
  }

  try {
    // Get authenticated user
    const user = await getUserFromRequest(req);
    if (!user) {
      return res.status(401).json({ error: 'Authentication required' });
    }

    const { allowed, reset } = await checkGenerateRateLimit(req, user.userId);
    if (!allowed) {
      res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
      return res.status(429).json({ error: 'Too many attempts. Try again later.' });
    }

    // Get user information for avatar generation
    const userResult = await sql`
      SELECT email, first_name, last_name, username FROM users WHERE id = ${user.userId}
    `;
    // ... rest of the file unchanged ...

Acceptance:

• One new import line: import { checkGenerateRateLimit } from '../../../../lib/rate-limit.js'; (note FOUR ../ levels — this file is at pages/api/user/avatar/generate.js).
• Gate sits between the if (!user) 401 (lines 13-15) and the user-data SQL query (current line 18).
• Net diff: +1 import, +5 lines, 0 deletions.

pages/api/cards/import-mtg.js (modified)

Currently has NO auth, NO rate-limit. Add three gates in order: method check (already present), auth check (NEW), admin-role check (NEW), import rate-limit (NEW). User-keyed, passing user.userId.

Verbatim post-edit shape (top of handler only):

import { sql } from '@vercel/postgres';
import { getUserFromRequest } from '../../../lib/permission-middleware';
import { checkImportRateLimit } from '../../../lib/rate-limit.js';

export default async function handler(req, res) {
  if (req.method !== 'POST') {
    return res.status(405).json({ error: 'Method not allowed' });
  }

  const user = await getUserFromRequest(req);
  if (!user) {
    return res.status(401).json({ error: 'Authentication required' });
  }
  if (user.role !== 'admin') {
    return res.status(403).json({ error: 'Admin access required' });
  }

  const { allowed, reset } = await checkImportRateLimit(req, user.userId);
  if (!allowed) {
    res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
    return res.status(429).json({ error: 'Too many attempts. Try again later.' });
  }

  try {
    const { setCode } = req.body;
    // ... rest of the file unchanged ...

Acceptance:

• Two new import lines (one for getUserFromRequest, one for checkImportRateLimit). Relative paths '../../../lib/permission-middleware' and '../../../lib/rate-limit.js' — verified at architect time against the directory depth.
• All three gates sit BEFORE the existing try block (current line 8). Order: method → auth → admin → rate-limit.
• Net diff: +2 imports, +14 lines, 0 deletions. The Scryfall fetch + INSERT loop + response shape are byte-identical.

pages/api/cards/import-pokemon.js (modified)

Same shape as import-mtg.js — three new gates added before the existing try block (current line 50). The delay + fetchWithRetry helpers above the handler stay unchanged.

Acceptance:

• Two new import lines, same paths as import-mtg.js.
• All three gates sit BEFORE the try block (current line 50), AFTER the method check (current lines 46-48).
• Net diff: +2 imports, +14 lines, 0 deletions. The fetchWithRetry + delay helpers + Pokemon-TCG fetch + INSERT loop + response shape are byte-identical.

pages/api/cards/import-lorcana.js (modified)

Same shape as import-mtg.js — three new gates added before the existing try block (current line 50). Despite having NO frontend caller today (architect-verified: rg 'import-lorcana' pages/ components/ returns zero matches in source code), gate defensively so a future Lorcana admin UI addition inherits the protection automatically. The delay + fetchWithRetry helpers above the handler stay unchanged.

Acceptance:

• Two new import lines, same paths as import-mtg.js.
• All three gates sit BEFORE the try block (current line 50), AFTER the method check (current lines 46-48).
• Net diff: +2 imports, +14 lines, 0 deletions.

pages/admin/card-import.js (modified — scope expansion for D1)

The admin UI currently calls fetch(endpoint, { method: 'POST', headers: { 'Content-Type': 'application/json' } }) with NO Authorization header (line 43-49). Adding getUserFromRequest to the import APIs would 401 the admin UI on first run. Add the Bearer token to the fetch call. This is the only edit to this file — do NOT refactor the 309-line god-component, do NOT add Lorcana to the <select>, do NOT touch the popularSets or any other UI logic.

Verbatim post-edit shape (fetch call only):

      const response = await fetch(endpoint, {
        method: 'POST',
        headers: {
          'Content-Type': 'application/json',
          'Authorization': `Bearer ${localStorage.getItem('auth_token')}`,
        },
        body: JSON.stringify({ setCode: setCode.trim() }),
      });

Acceptance:

• Net diff: +1 line (the Authorization header entry inside the headers: object on line 45-47). 0 deletions.
• The 'auth_token' localStorage key matches every other authenticated fetch in the codebase (components/ShareModal.js line 44 + 67, etc.). Do NOT use a different key.
• The 309-line component otherwise stays byte-identical. No refactor of the <AdminProtected> wrapper, the dynamic(... { ssr: false }) export, the useState block, the popular-sets grid, or the result-display logic.

.cursor/rules/api-routes.mdc (modified)

Replace the existing § "Rate limiting" subsection (lines 104-135 in the current file). Keep every other section byte-identical. The updated subsection:

## Rate limiting

`lib/rate-limit.js` exposes five named limiters, one per route class. Each named export takes `req` (and `userId` for user-keyed classes) and returns `{ allowed, remaining, reset }`.

| Class | Limit | Window | Key | Used by | Helper |
| --- | --- | --- | --- | --- | --- |
| `auth` | 5 | 15 min | IP | `/api/auth/login`, `/api/auth/register` | `checkAuthRateLimit(req)` |
| `search` | 60 | 1 min | IP | `/api/users/search`, `/api/cards/search` | `checkSearchRateLimit(req)` |
| `upload` | 10 | 1 hour | user | `/api/user/avatar` | `checkUploadRateLimit(req, userId)` |
| `generate` | 5 | 1 hour | user | `/api/user/avatar/generate` | `checkGenerateRateLimit(req, userId)` |
| `import` | 5 | 1 hour | user | `/api/cards/import-mtg`, `/api/cards/import-pokemon`, `/api/cards/import-lorcana` | `checkImportRateLimit(req, userId)` |

**Verbatim call shape** (identical across all five classes — only the helper name and the optional `userId` argument differ):

```js
import { checkSearchRateLimit } from '../../../lib/rate-limit.js';

export default async function handler(req, res) {
  if (req.method !== 'GET') {
    return res.status(405).json({ error: 'Method not allowed' });
  }

  // For user-keyed classes, auth check goes HERE first; see "Gate ordering" below.

  const { allowed, reset } = await checkSearchRateLimit(req);
  if (!allowed) {
    res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
    return res.status(429).json({ error: 'Too many attempts. Try again later.' });
  }

  try {
    // ... handler body ...
  } catch (err) {
    // ...
  }
}
```

**Gate ordering rules:**

1. **Method check first.** Reject the wrong verb with 405 before doing any limiter work.
2. **Auth check before any user-keyed limiter.** `extractUserIdentifier(userId)` THROWS when `userId` is null/undefined/empty (defensive). For `upload`, `generate`, and `import`, the handler MUST call `getUserFromRequest(req)` (or equivalent JWT verification) and confirm a non-null user BEFORE calling the limiter. Wrong order = anonymous user bypasses (the THROW surfaces immediately during dev; do not catch and silently fall back to IP).
3. **For IP-keyed limiters (`auth`, `search`), gate placement is flexible** — either at the top of the handler (after the method check) or after a separate auth check that the route happens to also have (e.g. `users/search` JWT-verifies before rate-limiting, both are correct). The limiter only needs `req` for IP extraction.
4. **Admin-role check, if applicable, goes between auth and rate-limit.** Used by all three `/api/cards/import-*` routes: `if (user.role !== 'admin') return res.status(403).json({ error: 'Admin access required' })` sits between the `if (!user)` 401 and the import rate-limit call.

**Identifier extraction:**

- `extractIpIdentifier(req)` (module-private) — first hop in `x-forwarded-for` (Vercel's edge), falling back to `req.socket.remoteAddress`, falling back to the literal `'anonymous'`. Do NOT key off `req.body.email` (rotates) or `req.headers.authorization` (unauthenticated endpoints don't have one).
- `extractUserIdentifier(userId)` (module-private) — formats as `user:${userId}`. Throws on null/undefined/empty/NaN to surface gate-ordering bugs at dev time rather than silently falling back to IP and creating a per-IP-not-per-user limit.

**Env vars (unchanged from Brief 4):** `KV_REST_API_URL` + `KV_REST_API_TOKEN` (auto-provisioned by Vercel's Upstash Marketplace integration). In prod, missing either var is a **fail-closed throw** on the first call. In dev / test, the module warn-and-no-ops so local work isn't blocked. See `AGENTS.md` Gotcha #12 for the full env-var contract.

**429 response shape is uniform across all five classes.** Same error message (`'Too many attempts. Try again later.'`) and same `Retry-After` header calculation. Per-class variation would fingerprint the limits to an attacker.

**Fail-open on Upstash outage.** A network failure inside `ratelimit.limit(...)` returns `{ allowed: true, remaining: Infinity, reset: 0 }` with a single `console.error('[rate-limit]', err)`. Reasoning: a hard Upstash outage should not lock the entire user base out of every gated route. Brute-force / abuse protection lives behind defense-in-depth (Vercel firewall, future fail2ban-style lockout).

Acceptance:

• Replace lines 104-135 (the existing § "Rate limiting" subsection — starts with ## Rate limiting and ends right before ## Dev/test endpoints (removed)). Use the verbatim content above.
• Every OTHER section in the file stays byte-identical. No edits to § Authentication & Authorization, § Request validation, § Method gating, § Error handling, § Database access, § Response shape, § Activity logging, § Dev/test endpoints, or § CORS.
• The Markdown table renders cleanly (5 columns: Class | Limit | Window | Key | Used by | Helper — 6 columns actually, count the pipes; ensure alignment).
• No mention of the now-stale Sweeping the rest of the API ... is the queued add-rate-limiting convoy line — that sentence in the current rule gets replaced by the full new content.

Cross-file checks

• npm run lint exit code unchanged. The current baseline is ✖ 128 problems (81 errors, 47 warnings) (per bump-next-js Decision D + fix-lint-baseline tracking). Each per-route edit is an import + a small gate block — no new react-hooks/* paths, no new unused vars, no new no-img-element triggers. If the count grows, investigate before commit.

• npm run test:run (vitest) passes 21/21. No test file is touched in this convoy. The lib refactor preserves checkAuthRateLimit(req)'s return shape so any indirect dependency is irrelevant; architect verified at brief time that no current vitest spec actually imports lib/rate-limit.js (the convoy file's stale claim about transitive loading is corrected in Decision 6).

• npm run build exit 0. Turbopack compile time should be unchanged. The 10 modified files still compile to the same shape.

• npm run test:smoke against the Vercel preview passes 3/3. None of the 3 smoke tests ('home redirects or renders without 5xx', 'sign-in page renders', 'public health endpoint responds') hit any of the 7 gated endpoints, so no smoke regression. Verify in CI on PR push.

• Repo-wide grep clean. After the sweep:

  rg "checkAuthRateLimit" pages/api/
  

  Expected: 2 matches (login.js + register.js) — same as before this convoy.

  rg "checkSearchRateLimit|checkUploadRateLimit|checkGenerateRateLimit|checkImportRateLimit" pages/api/
  

  Expected: 6 matches total (search-2, upload-1, generate-1, import-3 — matching the 7 surfaces; users/search counts as search-1, cards/search counts as search-2).

• Per-class prefix uniqueness check.

  rg "tcgvault:" lib/rate-limit.js | sort -u
  

  Expected: 5 distinct lines, one per class (tcgvault:auth, tcgvault:search, tcgvault:upload, tcgvault:generate, tcgvault:import). No duplicates.

• Auth-then-rate-limit ordering check for user-keyed routes. In import-mtg.js, import-pokemon.js, import-lorcana.js, avatar.js, avatar/generate.js: visually confirm via git diff that getUserFromRequest (or equivalent) appears BEFORE the check*RateLimit call. If the order is reversed, the extractUserIdentifier throw fires on every anonymous request — the test would surface as a 500 in dev, but better to never ship that shape.

Manual verification (in addition to CI on push)

Run these in order. Paste relevant output (with secrets redacted) into the PR description.

• Local dev boot.

  npm run dev
  

  Expected: clean boot, no [rate-limit] warn-spam at startup (the limiter is lazy-init; no warn until first gated call). If KV_REST_API_* env vars are absent in .env.local, the first request to ANY gated route will emit one [rate-limit] KV_REST_API_URL / KV_REST_API_TOKEN not set — rate limiting disabled (dev/test only) line — that's correct dev-mode behavior.

• Auth limiter regression check — Brief 4's contract must be preserved.

  for i in 1 2 3 4 5 6; do
    curl -sS -o /dev/null -w "POST /api/auth/login attempt $i: %{http_code}\n" \
      -X POST -H "Content-Type: application/json" \
      -d '{"email":"nobody@example.com","password":"wrong"}' \
      http://localhost:3000/api/auth/login
  done
  

  Expected (only meaningful with KV_REST_API_* set):

  • Attempts 1-5: 401
  • Attempt 6: 429 with Retry-After header

  Without Upstash configured locally, all 6 will return 401 — the dev-mode no-op limiter — and that's also correct. Either outcome confirms checkAuthRateLimit still works through the refactored module.

• Search limiter (anonymous, IP-keyed). Without Upstash, this should never 429 in dev:

  for i in $(seq 1 5); do
    curl -sS -o /dev/null -w "GET /api/cards/search: %{http_code}\n" \
      "http://localhost:3000/api/cards/search?query=test"
  done
  

  Expected: 200 each call (dev-mode noop). The gate is wired but won't fire without Upstash. To exercise the live path, set KV_REST_API_* and burst >60 in <60s.

• User-keyed limiter — verify gate-ordering throws on misuse. This is a one-shot sanity check that the extractUserIdentifier throw fires when called pre-auth. Boot dev, then:

  node -e "
    const { checkUploadRateLimit } = require('./lib/rate-limit.js');
    checkUploadRateLimit({ headers: {} }, null).catch(err => {
      console.log('OK - throw fired:', err.message.startsWith('[rate-limit] extractUserIdentifier'));
    });
  "
  

  Expected: OK - throw fired: true. (If you get an ERR_REQUIRE_ESM error, use node --experimental-vm-modules or write a tiny .mjs wrapper — the module is ESM. The point is the throw, not the invocation shape.)

• Admin-only enforcement on import routes — anonymous → 401, authenticated-non-admin → 403, authenticated-admin → 200 (or whatever the import returns).

  # 1. Anonymous:
  curl -sS -o /dev/null -w "anonymous import-mtg: %{http_code}\n" \
    -X POST -H "Content-Type: application/json" \
    -d '{"setCode":"neo"}' \
    http://localhost:3000/api/cards/import-mtg
  # Expected: 401
  
  # 2. Auth'd non-admin (use a regular user's token):
  curl -sS -o /dev/null -w "user import-mtg: %{http_code}\n" \
    -X POST -H "Content-Type: application/json" \
    -H "Authorization: Bearer <regular-user-token>" \
    -d '{"setCode":"neo"}' \
    http://localhost:3000/api/cards/import-mtg
  # Expected: 403
  
  # 3. Auth'd admin: (optional — actually triggers Scryfall fetch + DB writes; skip
  #    unless you're staging-pointed and want to exercise the full happy path):
  curl -sS -w "admin import-mtg: %{http_code}\n" \
    -X POST -H "Content-Type: application/json" \
    -H "Authorization: Bearer <admin-token>" \
    -d '{"setCode":"neo"}' \
    http://localhost:3000/api/cards/import-mtg
  # Expected: 200 with {imported, skipped, total}
  

  Repeat for import-pokemon and import-lorcana. The first two checks (anonymous + non-admin) are the meaningful security check; the admin check is optional smoke and SHOULD ONLY run against a staging DB per .cursor/rules/no-go-zones.mdc.

• Admin UI smoke — log in as admin in the browser, visit /admin/card-import, type a set code (e.g. neo for MTG), click "Import Cards". Expected: the request succeeds (or returns whatever Scryfall would return). If the request 401s, the pages/admin/card-import.js Bearer-token edit didn't land — check the browser's network tab for the Authorization header on the POST.

• Avatar upload smoke — log in, visit /profile (or wherever the avatar uploader lives), upload an image. Expected: success. Then submit the form 11 times in <1 hour to verify the gate fires (with Upstash configured); without Upstash, no 429 in dev.

• Vitest pass count unchanged.

  npm run test:run 2>&1 | tail -5
  

  Expected: Tests 21 passed (21). If the count or any individual test changes, the lib refactor broke a contract — investigate immediately. Most likely culprit: checkAuthRateLimit's return shape drifted from { allowed, remaining, reset }.

• Diff hygiene. git diff main..HEAD --stat should show:

  • lib/rate-limit.js: ~70 lines + / ~5 lines - (net add of ~65 lines).
  • 7 source-file additions (~14 lines + / ~0 lines - each): users/search.js (~6/0), cards/search.js (~6/0), user/avatar.js (~6/0), user/avatar/generate.js (~6/0), cards/import-mtg.js (~16/0), cards/import-pokemon.js (~16/0), cards/import-lorcana.js (~16/0).
  • pages/admin/card-import.js: +1 / 0 lines.
  • .cursor/rules/api-routes.mdc: ~70 lines + / ~32 lines - (replacing the existing § Rate limiting subsection).
  • No whitespace-only changes elsewhere.

Boot-the-brief findings (preempted by the architect; do not re-investigate)

Finding 1 — pages/admin/card-import.js does NOT currently send the Bearer token

Architect read the file at brief time (309 lines). Line 43-49 calls fetch(endpoint, { method: 'POST', headers: { 'Content-Type': 'application/json' } }) with NO Authorization header. Without the fix in this brief, the moment the import APIs gain getUserFromRequest, the admin UI starts returning 401 on every import attempt. This is the documented scope expansion under Decision 1 — it's not optional, the brief is internally consistent only with both edits (API gate + admin UI fix) shipping together.

Finding 2 — import-lorcana.js has zero frontend callers

Architect ran rg 'import-lorcana' pages/ components/ and only pages/admin/card-import.js matched — but that match is in a comment / file-listing context, not a code-execution call (the <select> in card-import.js only has mtg and pokemon options). So gating Lorcana is purely defensive. A future cleanup convoy may delete pages/api/cards/import-lorcana.js if it's never wired up; for now, gating with the same shape as mtg/pokemon is the smaller-diff path.

Finding 3 — components/ShareModal.js's user-search has NO debounce

Architect read components/ShareModal.js::handleSearch (lines 56-77). It calls fetch('/api/users/search?q=...') on every keystroke when query.length >= 2. Typing a 17-char email like alice@example.com fires 16 requests within ~3 seconds (one per char after the 2-char minimum). Parent's recommendation of search: 30 / 1min would 429 on a single legitimate email entry. Tuned up to 60 / 1min in Decision 3 to fit the realistic burst pattern without blocking the search-as-you-type UX. A future client-side fix (adding debounce in ShareModal) would let us re-tighten this; queue as debounce-share-modal-search if it surfaces.

Finding 4 — pages/api/user/avatar/generate.js uses DiceBear, not a paid AI service

Architect read the file (133 lines). It calls https://api.dicebear.com/7.x/${avatarStyle}/svg?... — free public API for SVG initials avatars. No OpenAI / Anthropic / Replicate cost. The "cost" of abuse is Vercel blob storage (the generated SVG gets put() into blob storage on every successful call) + DiceBear's own rate-limiting if we hammer them. Tuned generate up to 5 / 1hour from parent's 3 — still catches accidental loops (user mashing "regenerate avatar" button) without blocking legitimate "I want to try 4 different seeds" workflow.

Finding 5 — pages/api/cards/search.js is a 240-line SQL god-function (do NOT refactor)

Architect read the file in full. Lines 40-186 are seven conditional SQL branches plus a fallback JS-filter path. Already flagged in .convoys/ship-readiness.md as god-function-split / refactor-cards-search-sql scope. The brief is explicit: only add the rate-limit gate at the top, do NOT touch any SQL. The implementer MUST resist the urge to "clean up while I'm in here" — that's a separate convoy with its own architect pass.

Finding 6 — pages/api/user/avatar.js has NO top-level method gate; method-branches inside the outer try

Architect read the file (209 lines). Handler structure is try { getUserFromRequest; if POST {...} else if DELETE {...} else 405 } — the method check is the LAST branch, after both POST and DELETE bodies. This is unusual but the brief accommodates it by placing the rate-limit gate AFTER the auth check, BEFORE the method-branching. Both POST and DELETE branches inherit the limit. (DELETE is rare — only fires on "remove my avatar" — so the upload limit applying to both is fine.) Do NOT restructure the handler to add a top-level method gate; that's a cosmetic refactor and out of scope.

Finding 7 — pages/api/users/search.js uses inline jwt.verify, not getUserFromRequest

Architect read the file. Lines 17-21 do const token = authHeader.substring(7); try { jwt.verify(token, JWT_SECRET); } catch { 401; } — but the verified decoded payload is discarded (the route doesn't need the user ID, only the proof of auth). For the IP-keyed search limiter, we don't need the user either — the gate just goes after the JWT-verify catch block, before the query-length validation. Do NOT refactor to use getUserFromRequest — that's its own auth-surface convoy (queued single-auth-provider).

Finding 8 — Vitest does NOT currently transitively import lib/rate-limit.js

The convoy file claims lib/rate-limit.js is "unit-tested transitively via the existing vitest suite" — that's stale. Architect ran rg 'rate-limit|@upstash' test/ → zero matches. Only pages/api/auth/login.js + register.js import lib/rate-limit.js, and neither has a vitest spec. The lib refactor is therefore strictly safer than the convoy file implies — there's no transitive test path to break. (test/api/auth-utils.test.js only imports pages/api/auth-utils.js + lib/auth-secret.js; no handler imports.) Decision 6 still holds: no NEW tests this convoy.

Finding 9 — Test setup file doesn't set KV_REST_API_* (intentionally)

test/setup.js sets only JWT_SECRET and NODE_ENV=test. With NODE_ENV=test, lib/rate-limit.js's init() falls into the warn-and-noop branch (NODE_ENV !== 'production'), so vitest never tries to construct a real Redis client. Even if a future vitest spec adds a handler import, the limiter no-ops in test. This is the correct shape; do NOT add KV_REST_API_* to test/setup.js.

Finding 10 — parseMultipartFormData body-streaming is acknowledged out-of-scope but the gate ordering still matters

pages/api/user/avatar.js::parseMultipartFormData consumes the multipart body via req.on('data') + req.on('end'). If the rate-limit gate were placed AFTER parseMultipartFormData, an attacker could flood the 5MB ceiling even on a 429 path. The brief places the gate BEFORE the method-branching (which calls parseMultipartFormData inside the POST branch), so the gate fires before the body parse. This is the correct ordering even though the body-streaming defense is out of scope — when harden-multipart-parser eventually lands, the gate ordering will already be correct and won't need adjustment.

Finding 11 — Per-class Redis prefix isolation is required for correctness

@upstash/ratelimit@2.0.8's prefix: option scopes all keys for that limiter. Without distinct prefixes, two limiters sharing a prefix would share a sliding-window counter, meaning a search hit would consume the auth budget for the same identifier (or, for user-keyed classes, a search hit from user X would consume their upload budget). The brief enforces five distinct prefixes (tcgvault:auth, tcgvault:search, tcgvault:upload, tcgvault:generate, tcgvault:import). Verified at architect time against the @upstash/ratelimit README's prefix semantics.

Out of scope (do not do these)

• Do NOT add new vitest or playwright tests. Deferred to fill-vitest-handler-coverage (Decision 6).
• Do NOT add a withRateLimit(handler) higher-order wrapper. The 7 call sites justify inline; a wrapper is premature abstraction.
• Do NOT migrate lib/rate-limit.js to Next.js middleware (Edge runtime). Pages Router serverless functions don't share the Edge runtime cleanly with @upstash/ratelimit's default Node-fetch path; inline is simpler.
• Do NOT add rate-limit headers to SUCCESS responses (X-RateLimit-Remaining, X-RateLimit-Reset). Honoring the existing login.js / register.js convention — only the 429 path sets Retry-After.
• Do NOT vary the 429 error message per class (Decision 5). Uniform message minimizes attacker fingerprinting.
• Do NOT add a global IP-based backstop limiter (Next.js middleware). Queued as add-global-rate-limit-middleware if a future audit shows non-listed routes being abused.
• Do NOT touch pages/api/cards/search.js's 240-line SQL god-function. Only add the rate-limit gate at the top.
• Do NOT touch pages/api/user/avatar.js's parseMultipartFormData. Body-streaming defense is harden-multipart-parser scope.
• Do NOT delete pages/api/cards/import-lorcana.js. Gating with the same shape as mtg/pokemon is the chosen path under Decision 1 (Option A applied uniformly to all three).
• Do NOT add Lorcana to the <select> in pages/admin/card-import.js. The only edit to that file is the Bearer-token addition on line 45-47.
• Do NOT extract a withAdmin(handler) wrapper from the three new admin-role checks. The convention rule says to extract when a third call site appears; these ARE the third+fourth+fifth, but extraction is its own auth-surface refactor (queued single-auth-provider adjacent). Inline for this convoy.
• Do NOT touch AGENTS.md Gotcha #12. Doc-writer pass at convoy close owns the update; preempting here creates merge conflicts.
• Do NOT touch .github/workflows/ci.yml. No new CI gate is added in this convoy (per-class rate-limit wiring isn't grep-checkable; the existing forbidden-endpoints + forbidden-cors-headers jobs suffice for the API surface).
• Do NOT bump @upstash/ratelimit or @upstash/redis versions. Pins stay at ^2.0.8 and ^1.38.0 from Brief 4.
• Do NOT add KV_REST_API_* to test/setup.js. The warn-and-noop branch is the correct test behavior.

Rationale (≤3 sentences)

Extending lib/rate-limit.js from one auth-only limiter to five named per-class limiters closes the last open P0 (#6 PARTIAL → RESOLVED) by wiring rate-limit + auth+admin gates into the remaining abusable surface; the hybrid named-export shape preserves Brief 4's checkAuthRateLimit(req) contract so login.js + register.js stay untouched. Auth-gating the three pages/api/cards/import-*.js routes (currently anonymous, hitting external Scryfall / Pokémon-TCG / Lorcana APIs with no caller throttling AND performing unbounded DB writes) is the security-critical scope expansion under Decision 1; adding the matching Bearer-token send to pages/admin/card-import.js is the necessary admin-UI fix to keep the gated APIs callable. Once this lands, the launch-readiness ship-blocker list is empty (8 of 8 RESOLVED), and the per-class shape is documented in .cursor/rules/api-routes.mdc for any future route to follow without architect re-derivation.