deckhearth/.convoys/fix-auth-bypass/brief-4-tighten-auth-surface.md

convoy	brief_number	depends_on	files	cross_brief_commitments
fix-auth-bypass	4	1	package.json package-lock.json lib/rate-limit.js pages/api/auth/login.js pages/api/auth/register.js	brief description 1 Brief 1 already removed the `JWT_SECRET` literal and routed login.js + register.js through `auth-utils.generateToken`. This brief preserves those changes; do NOT reintroduce inline `jwt.sign` or `JWT_SECRET` references. brief description 5 Brief 5 (vitest) modifies `package.json` and `package-lock.json` after this brief. If Brief 5 lands first by accident, this brief's implementer MUST rebase on Brief 5's lockfile rather than regenerate from scratch. The sequenced order is Brief 4 → Brief 5; the convoy's slice_dependencies enforces this.

Brief 4: Tighten the public auth surface (CORS + rate limit)

Goal (1 sentence)

Drop the wide-open Access-Control-Allow-Origin: '*' header from /api/auth/login and /api/auth/register, and rate-limit both endpoints to 5 attempts per 15 minutes per IP via @upstash/ratelimit (with a graceful no-op fallback in non-production environments where Upstash isn't configured).

Files in scope (do not edit anything else)

• package.json — modified (add @upstash/ratelimit, @upstash/redis)
• package-lock.json — modified (regenerated by npm install)
• lib/rate-limit.js — new
• pages/api/auth/login.js — modified
• pages/api/auth/register.js — modified

Conventions to follow

• .cursor/rules/auth-and-permissions.mdc § "Token model" — auth flow shape stays unchanged. Only the request-acceptance gate (CORS, rate limit) changes.
• .cursor/rules/api-routes.mdc § "Method gating" + "Error handling" — the rate-limit check goes inside the existing try/catch, after the method gate, before the body parsing.
• .cursor/rules/no-go-zones.mdc — do not touch pages/api/auth/verify.js, pages/api/favorites.js, pages/api/users/search.js, or any other auth-adjacent file. The CORS sweep on the rest of the API is add-rate-limiting / future scope.
• package.json formatting: 2-space indent, alphabetical key order within dependencies / devDependencies (match the existing block from Brief 1's bump-next-js work).
• lib/rate-limit.js ESM export, kebab-case file name, 2-space indent, no top-level side effects beyond a const init.

Acceptance criteria

package.json changes

• dependencies gains "@upstash/ratelimit": "^2.0.8". (Verified at architect time: npm view @upstash/ratelimit version → 2.0.8. Peer dep: @upstash/redis: ^1.34.3.)
• dependencies gains "@upstash/redis": "^1.38.0". (Verified at architect time: npm view @upstash/redis version → 1.38.0. Satisfies @upstash/ratelimit@2.0.8's peer-dep range ^1.34.3. The only direct dep @upstash/redis itself pulls in is uncrypto@^0.1.3.)
• No other dependencies change. No devDependencies change in this brief (vitest is Brief 5).
• No engines block change. Both packages are pure-JS ESM with Node >=18 requirements; tcg-vault runs Node 20 on Vercel.

package-lock.json changes

• Regenerated via npm install (no hand edits).
• npm ls @upstash/ratelimit reports a single 2.0.x version. No duplicates.
• npm ls @upstash/redis reports a single 1.38.x version.
• npm install exits cleanly with no ERESOLVE errors and no npm warn deprecated for either package.

lib/rate-limit.js (new)

• File exports a single async function checkAuthRateLimit(req) that returns { allowed: boolean, remaining: number, reset: number }.
• On first call, the module initializes a singleton Ratelimit instance lazily. Do not initialize at module top level — top-level new Redis(...) would throw at import time in environments without Upstash env vars (including local dev where the developer hasn't onboarded Upstash yet, and any test that imports pages/api/auth/login.js transitively).
• Initialization rules:
  • If process.env.KV_REST_API_URL and process.env.KV_REST_API_TOKEN are both set: construct new Redis({ url, token }) and new Ratelimit({ redis, limiter: Ratelimit.slidingWindow(5, '15 m'), prefix: 'tcgvault:auth' }). (Env-var names match Vercel's Upstash Marketplace integration; see Post-merge addendum.)
  • If either env var is missing AND process.env.NODE_ENV === 'production': throw at first call with a message naming both env vars. (Fail-closed in prod — better to error a single login attempt than silently disable rate limiting.)
  • If either env var is missing AND NODE_ENV !== 'production': log one console.warn ("[rate-limit] KV_REST_API_URL / KV_REST_API_TOKEN not set — rate limiting disabled (dev/test only)"), cache a no-op limiter (return { allowed: true, remaining: Infinity, reset: 0 } from checkAuthRateLimit).
• IP extraction:

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

Use identifier as the rate-limit key. Do NOT use req.body.email (an attacker can rotate emails) or req.headers.authorization (login is unauthenticated by design — the header is absent).

• On Upstash quota error or network failure inside ratelimit.limit(...): catch and fail-open (return { allowed: true, ... }) with a single console.error('[rate-limit]', err). Reasoning: a hard outage at Upstash should not lock everyone out of login. Brute-force protection lives behind defense-in-depth (Vercel firewall, future fail2ban-style lockout). Document this trade-off in a comment.
• No default export. Only the named checkAuthRateLimit export.
• No top-level await (Next.js Pages Router serverless bundler handles ESM, but module-init time is the wrong place for I/O — keep it lazy).

pages/api/auth/login.js

• Drop CORS-*. Remove lines 9-17 (the setHeader('Access-Control-Allow-Origin', '*') and friends, plus the OPTIONS preflight). Same-origin requests on Vercel work without explicit CORS headers — the browser doesn't preflight a same-origin POST.
  • If a future cross-origin client appears (e.g. a separate marketing-site origin), pin via process.env.PUBLIC_FRONTEND_ORIGIN. Do NOT add this conditionally now — adding the env-var path "just in case" creates a code path no test will cover, and the current tcg-vault deploy is single-origin Vercel. The add-rate-limiting convoy or a follow-up cors-tighten convoy can add it when it actually has a consumer.
• No OPTIONS handler. With CORS-* gone, OPTIONS preflight isn't relevant for same-origin POST. If the front-end ever sends a preflight (it shouldn't on same-origin), Next.js will route it to this handler, which will hit the if (req.method !== 'POST') 405 branch — that's the correct response.
• Add the rate-limit gate between the method check and the body parsing. Verbatim shape:

import { checkAuthRateLimit } from '../../../lib/rate-limit.js';
// ... existing imports stay ...

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

  const { allowed, reset } = await checkAuthRateLimit(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 {
    // ... existing body unchanged ...
  } catch (error) {
    console.error('Login error:', error);
    res.status(500).json({ error: 'Internal server error' });
  }
}

Note Retry-After is in seconds, and reset from @upstash/ratelimit is a Unix-ms timestamp (per the SDK's Ratelimit.limit return shape).

• Brief 1 already replaced jwt.sign(...) with generateToken(user). Preserve that. Do not reintroduce inline jwt.sign or JWT_SECRET references.
• Brief 1 already removed import jwt from 'jsonwebtoken'. Keep it removed.

pages/api/auth/register.js

• Same CORS removal as login.js (drop the four setHeader calls + OPTIONS preflight at lines 10-18).
• Same rate-limit gate, same shape, between method check and try. The 429 response shape and Retry-After header are identical.
• Same import path: '../../../lib/rate-limit.js'. Verify by reading the existing '../../../lib/slug-utils.js' import on line 4.
• Brief 1's generateToken call is preserved.

Smoke (manual)

• In .env.local, set KV_REST_API_URL and KV_REST_API_TOKEN (if you have an Upstash free-tier account or have pulled them down from Vercel via vercel env pull). If you don't, leave both unset — the warn-and-continue branch should fire, and login still works.
• npm run dev; submit invalid login 6 times in quick succession (each with a typo). Expect: first 5 return 401, 6th returns 429 with Retry-After header. (Skipped if Upstash isn't configured.)
• Submit a valid login. Expect: token returned. (Successful logins also count against the limit per the sliding-window algo — that's intentional; a credential-stuffing attacker can't dodge by knowing one valid pair.)
• Open dev tools → network tab on the login submit. Confirm there is no Access-Control-Allow-Origin response header. Confirm there is no preflight OPTIONS request.
• Verify same behavior on /api/auth/register.
• Vercel preview deploy succeeds with both env vars unset → expect npm run build to succeed (lazy init means no import-time throw).

Pre-deploy checklist (call out in the PR description)

• Before merging to main, confirm KV_REST_API_URL and KV_REST_API_TOKEN are present in the Vercel project settings (Production + Preview environments). These are auto-provisioned the moment the Vercel Upstash Marketplace integration is enabled on the project — no manual paste-the-token step. (You can verify locally with vercel env ls or by inspecting Vercel's project → Settings → Environment Variables.) Without them, the prod auth endpoints will throw on first login attempt (intentional fail-closed). Free-tier Upstash Redis via the Marketplace is sufficient (10k commands/day; rate-limit traffic is single-digit commands per request).
• Add a note to .env.local.example (if it exists; otherwise to AGENTS.md "Running locally" — but defer to doc-writer pass).

Out of scope

• No CORS / rate-limit on pages/api/auth/verify.js. (verify.js is a GET on token presence; rate-limiting it would bounce legitimate page loads. The CORS-* on it is a smaller risk, deferred to cors-tighten or add-rate-limiting.)
• No CORS / rate-limit on pages/api/favorites.js, pages/api/users/search.js, pages/api/cards/import-*.js, avatar upload, etc. → add-rate-limiting convoy.
• No withRateLimit(handler) higher-order wrapper. The two endpoints in scope justify inline; a wrapper is premature abstraction until there are 3+ call sites.
• No middleware-based rate limit (Next.js middleware.js). Pages Router with serverless functions doesn't share the Edge runtime cleanly with @upstash/ratelimit's default Node-fetch path. Inline is simpler.
• No withCollectionPermission-style wrapper change.
• No AGENTS.md / .cursor/rules/auth-and-permissions.mdc updates — doc-writer pass.

Rationale (≤3 sentences)

Wrapping login + register with rate limiting closes the credential-stuffing window before public launch (P0 #6 partial), and dropping CORS-* removes a class of CSRF vectors that the wild-card header was masking (P0 #4). Choosing @upstash/ratelimit over a DIY-Postgres alternative respects the convoy's "no schema changes" rule, and choosing serverless-native over an in-memory limiter respects the Vercel deployment model (each cold start would otherwise reset its own counter). Bundling CORS and rate-limit into one brief — rather than splitting them across Brief 4 + Brief 5 as the convoy file initially suggested — avoids two PRs editing the same two handler files in sequence.

Post-merge addendum (2026-05-23)

Added retroactively by role-doc-writer during convoy close-out. The brief as originally written specified UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN — the two env-var names baked into @upstash/redis's generic README examples — and the verbatim shape above still reflects that. The shipped code in lib/rate-limit.js uses KV_REST_API_URL / KV_REST_API_TOKEN instead. This addendum records the deviation so future readers don't mistake the brief's original shape for the as-built behavior.

What changed and why. Mid-implementation, the implementer surfaced that this project already runs on Vercel's Upstash Marketplace integration, which auto-provisions a Redis instance under a project-scoped credential set named KV_* (alongside KV_URL, REDIS_URL, and KV_REST_API_READ_ONLY_TOKEN). Aliasing those to a new UPSTASH_REDIS_REST_* pair would have required either (a) a manual paste-the-token step on every environment (Production, Preview, Local) or (b) a duplicate set of env vars pointing at the same Upstash instance. Neither was worth the friction; the Marketplace's own naming is the lower-coordination path.

How the change was approved. Implementer paused, surfaced the discrepancy upward via parent-agent interrupt, parent agent approved the rename to KV_REST_API_* ("ship what Vercel hands you"), and the implementer continued with the renamed pair. The convoy file's "Pre-merge env-var checklist" line ("UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN") was not updated in the implementation PR — the close-out doc-writer pass (this addendum + the matching edit to .convoys/fix-auth-bypass.md § "Convoy outcome") is where the canonical record lives.

Not in scope for this brief. The other three KV_*-prefixed vars Vercel exposes (KV_URL, REDIS_URL, KV_REST_API_READ_ONLY_TOKEN) are intentionally unused. @upstash/redis's REST client reads only KV_REST_API_URL + KV_REST_API_TOKEN; the others are for the Redis-protocol client (@upstash/redis/cloudflare / ioredis) or for read-only consumers. Do not wire them up unless a downstream library specifically requires one.

Verbatim shape correction. The "Initialization rules" snippet above has been updated in-place to read KV_REST_API_URL / KV_REST_API_TOKEN. The "Smoke (manual)" and "Pre-deploy checklist" sections have been updated to match. Any other documentation that still mentions UPSTASH_REDIS_REST_* for this project (search-and-replace target) is stale.