fix(auth): tighten public auth surface — CORS + rate limit (Brief 4 of fix-auth-bypass) #9

Merged
varutasu merged 1 commit from brief/fix-auth-bypass/4-tighten-auth-surface into main 2026-05-23 11:57:50 -04:00
varutasu commented 2026-05-23 11:51:59 -04:00 (Migrated from github.com)

Summary

Convoy fix-auth-bypass / Brief 4 of 5. Adds rate limiting to /api/auth/login and /api/auth/register (5/15min per IP) and removes their wide-open CORS allowlist. See .convoys/fix-auth-bypass.md for the full convoy plan.

Rate limiting

  • New lib/rate-limit.js — lazy singleton, single source of truth, 5 attempts per 15-minute sliding window per IP, key prefix tcgvault:auth.
  • Backend: @upstash/ratelimit@^2.0.8 + @upstash/redis@^1.38.0.
  • Reads KV_REST_API_URL / KV_REST_API_TOKEN (Vercel Upstash Marketplace convention — auto-provisioned, no manual env-var setup needed).
  • Fail-closed in production if env vars are missing — better to error one login attempt than silently disable brute-force protection on the live site.
  • Fail-open in dev/test if env vars are missing — single console.warn, returns { allowed: true, remaining: Infinity, reset: 0 }.
  • Fail-open on Upstash backend outage — defense-in-depth; don't lock the entire userbase out if Upstash has a hiccup. Vercel firewall provides additional brute-force protection.
  • IP extracted from x-forwarded-for first hop, with req.socket.remoteAddress fallback. NOT req.body.email (rotates) or Authorization header (absent on unauthenticated login).
  • Returns HTTP 429 with Retry-After header (seconds until reset) when rate-limited.

CORS

  • Removed Access-Control-Allow-Origin: * + companion headers + OPTIONS preflight from login.js and register.js. These are first-party endpoints called from the same-origin SPA; the * allowlist was a dev convenience that shipped to prod.
  • verify.js CORS is out of scope per architect's cors-tighten deferral (see convoy plan § Architect's calls). Will land in a future cors-tighten convoy.

Files changed

  • package.json — added @upstash/ratelimit@^2.0.8 and @upstash/redis@^1.38.0 to dependencies (alphabetical)
  • package-lock.json — regenerated via npm install
  • lib/rate-limit.js (new, ~70 lines)
  • pages/api/auth/login.js — removed CORS block + OPTIONS preflight; added rate-limit check after method gate
  • pages/api/auth/register.js — same treatment as login

Net diff: 5 files, +126 / −22 LOC.

Test plan

  • npm run build exits 0
  • npm run lint matches baseline (128 problems / 81 errors / 47 warnings); zero new lint issues
  • npm ls @upstash/ratelimit reports single 2.0.x; npm ls @upstash/redis reports single 1.38.x
  • Module-load smoke (production env, no Upstash vars): throws with message naming KV_REST_API_URL and KV_REST_API_TOKEN
  • Module-load smoke (test env, no Upstash vars): logs one console.warn, returns { allowed: true, remaining: Infinity, reset: 0 }
  • grep -E "Access-Control-Allow-Origin" pages/api/auth/login.js pages/api/auth/register.js returns zero hits
  • grep -E "Access-Control-Allow-Origin" pages/api/auth/verify.js still returns hits (correctly out of scope per cors-tighten deferral)
  • Reviewer to verify on preview deployment:
    • 6 successive failed logins from the same IP — the 6th should return 429 with Retry-After header
    • First login from a fresh IP after the reset should return 200 (or 401 if creds are wrong, but not 429)
    • OPTIONS /api/auth/login should now return 405 (Method not allowed) instead of 200 (since we removed the preflight handler — this is correct for a same-origin endpoint)

⚠️ Pre-merge requirements

Vercel env vars (auto-provisioned by Upstash Marketplace integration):

  • KV_REST_API_URL provisioned
  • KV_REST_API_TOKEN provisioned

Confirmed live by maintainer (2026-05-23) — Upstash database created via Vercel Marketplace. No manual env-var setup required.

Other Vercel-injected vars (not used, listed for reference):

  • KV_REST_API_READ_ONLY_TOKEN (read-only, not needed for rate-limit writes)
  • KV_URL (Vercel KV-native, not used by @upstash/redis REST client)
  • REDIS_URL (TCP, not used by @upstash/redis REST client)

If the env vars are NOT set in a production deploy, the first auth call will return 500 (fail-closed by design — see R8 in the convoy plan).

⚠️ Brief deviation from literal text

The brief specified env var names UPSTASH_REDIS_REST_URL / UPSTASH_REDIS_REST_TOKEN (the upstream Upstash convention). Mid-implementation we discovered the user provisioned Upstash via Vercel Marketplace, which uses KV_REST_API_* names. This PR uses the KV_REST_API_* names per Upstash's official Vercel integration guide and to eliminate manual env-var aliasing. The convoy brief on convoy/fix-auth-bypass will be updated to match by the doc-writer pass at end of convoy.

Out of scope (intentionally — see convoy plan)

  • CORS on pages/api/auth/verify.jscors-tighten convoy (architect's call)
  • Rate limiting on other unauthenticated endpoints (/api/cards/import-*, /api/admin/*) → add-rate-limiting convoy
  • Vitest harness + rate-limit unit tests → Brief 5

Audit cohort

Reviewer dispatch follows post-push. No UI surface → design-system + a11y + browser-smoke auditors do not run.


🤖 Pipeline metadata: convoy=fix-auth-bypass, brief=4, depends_on=[1], audit=reviewer-only

Made with Cursor

## Summary Convoy `fix-auth-bypass` / Brief 4 of 5. Adds rate limiting to `/api/auth/login` and `/api/auth/register` (5/15min per IP) and removes their wide-open CORS allowlist. See [`.convoys/fix-auth-bypass.md`](https://github.com/varutasu/tcg-vault/blob/convoy/fix-auth-bypass/.convoys/fix-auth-bypass.md) for the full convoy plan. ### Rate limiting - New `lib/rate-limit.js` — lazy singleton, single source of truth, 5 attempts per 15-minute sliding window per IP, key prefix `tcgvault:auth`. - Backend: `@upstash/ratelimit@^2.0.8` + `@upstash/redis@^1.38.0`. - Reads `KV_REST_API_URL` / `KV_REST_API_TOKEN` (Vercel Upstash Marketplace convention — auto-provisioned, no manual env-var setup needed). - **Fail-closed in production** if env vars are missing — better to error one login attempt than silently disable brute-force protection on the live site. - **Fail-open in dev/test** if env vars are missing — single `console.warn`, returns `{ allowed: true, remaining: Infinity, reset: 0 }`. - **Fail-open on Upstash backend outage** — defense-in-depth; don't lock the entire userbase out if Upstash has a hiccup. Vercel firewall provides additional brute-force protection. - IP extracted from `x-forwarded-for` first hop, with `req.socket.remoteAddress` fallback. NOT `req.body.email` (rotates) or `Authorization` header (absent on unauthenticated login). - Returns HTTP 429 with `Retry-After` header (seconds until reset) when rate-limited. ### CORS - Removed `Access-Control-Allow-Origin: *` + companion headers + `OPTIONS` preflight from `login.js` and `register.js`. These are first-party endpoints called from the same-origin SPA; the `*` allowlist was a dev convenience that shipped to prod. - `verify.js` CORS is **out of scope** per architect's `cors-tighten` deferral (see convoy plan § Architect's calls). Will land in a future `cors-tighten` convoy. ## Files changed - `package.json` — added `@upstash/ratelimit@^2.0.8` and `@upstash/redis@^1.38.0` to dependencies (alphabetical) - `package-lock.json` — regenerated via `npm install` - `lib/rate-limit.js` (**new**, ~70 lines) - `pages/api/auth/login.js` — removed CORS block + OPTIONS preflight; added rate-limit check after method gate - `pages/api/auth/register.js` — same treatment as login Net diff: 5 files, +126 / −22 LOC. ## Test plan - [x] `npm run build` exits 0 - [x] `npm run lint` matches baseline (128 problems / 81 errors / 47 warnings); zero new lint issues - [x] `npm ls @upstash/ratelimit` reports single `2.0.x`; `npm ls @upstash/redis` reports single `1.38.x` - [x] Module-load smoke (production env, no Upstash vars): throws with message naming `KV_REST_API_URL` and `KV_REST_API_TOKEN` - [x] Module-load smoke (test env, no Upstash vars): logs one `console.warn`, returns `{ allowed: true, remaining: Infinity, reset: 0 }` - [x] `grep -E "Access-Control-Allow-Origin" pages/api/auth/login.js pages/api/auth/register.js` returns zero hits - [x] `grep -E "Access-Control-Allow-Origin" pages/api/auth/verify.js` still returns hits (correctly out of scope per `cors-tighten` deferral) - [ ] **Reviewer to verify on preview deployment:** - 6 successive failed logins from the same IP — the 6th should return 429 with `Retry-After` header - First login from a fresh IP after the reset should return 200 (or 401 if creds are wrong, but not 429) - `OPTIONS /api/auth/login` should now return 405 (Method not allowed) instead of 200 (since we removed the preflight handler — this is correct for a same-origin endpoint) ## ⚠️ Pre-merge requirements **Vercel env vars (auto-provisioned by Upstash Marketplace integration):** - `KV_REST_API_URL` ✅ provisioned - `KV_REST_API_TOKEN` ✅ provisioned Confirmed live by maintainer (2026-05-23) — Upstash database created via Vercel Marketplace. No manual env-var setup required. **Other Vercel-injected vars (not used, listed for reference):** - `KV_REST_API_READ_ONLY_TOKEN` (read-only, not needed for rate-limit writes) - `KV_URL` (Vercel KV-native, not used by `@upstash/redis` REST client) - `REDIS_URL` (TCP, not used by `@upstash/redis` REST client) If the env vars are NOT set in a production deploy, the first auth call will return 500 (fail-closed by design — see R8 in the convoy plan). ## ⚠️ Brief deviation from literal text The brief specified env var names `UPSTASH_REDIS_REST_URL` / `UPSTASH_REDIS_REST_TOKEN` (the upstream Upstash convention). Mid-implementation we discovered the user provisioned Upstash via Vercel Marketplace, which uses `KV_REST_API_*` names. This PR uses the `KV_REST_API_*` names per Upstash's [official Vercel integration guide](https://upstash.com/docs/redis/howto/vercelintegration) and to eliminate manual env-var aliasing. The convoy brief on `convoy/fix-auth-bypass` will be updated to match by the doc-writer pass at end of convoy. ## Out of scope (intentionally — see convoy plan) - CORS on `pages/api/auth/verify.js` → `cors-tighten` convoy (architect's call) - Rate limiting on other unauthenticated endpoints (`/api/cards/import-*`, `/api/admin/*`) → `add-rate-limiting` convoy - Vitest harness + rate-limit unit tests → **Brief 5** ## Audit cohort Reviewer dispatch follows post-push. No UI surface → design-system + a11y + browser-smoke auditors do not run. --- 🤖 Pipeline metadata: convoy=fix-auth-bypass, brief=4, depends_on=[1], audit=reviewer-only Made with [Cursor](https://cursor.com)
vercel[bot] commented 2026-05-23 11:52:05 -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 Ready Preview, Comment May 23, 2026 3:52pm

Request Review

[vc]: #ILQinVPRxnBgBLMpHgHFE12beqHkMs5L244vrRl2DAY=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWJyaWVmLWZpeC1hdXRoLWQ4MWJkOS1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCJ9LCJpbnNwZWN0b3JVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzL3RjZy12YXVsdC9HU2JhUHdZZU1tTHpYNEhBc0FISDFYR0pYUVJKIiwicHJldmlld1VybCI6InRjZy12YXVsdC1naXQtYnJpZWYtZml4LWF1dGgtZDgxYmQ5LXJhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy52ZXJjZWwuYXBwIiwibmV4dENvbW1pdFN0YXR1cyI6IkRFUExPWUVEIn1dLCJyZXF1ZXN0UmV2aWV3VXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3ZlcmNlbC1hZ2VudC9yZXF1ZXN0LXJldmlldz9vd25lcj12YXJ1dGFzdSZyZXBvPXRjZy12YXVsdCZwcj05In0= 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/GSbaPwYeMmLzX4HAsAHH1XGJXQRJ) | [Preview](https://tcg-vault-git-brief-fix-auth-d81bd9-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-brief-fix-auth-d81bd9-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 23, 2026 3:52pm | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=9" 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-23 11:52:10 -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 failure
Visual diff failure

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-rollup --> ## Pipeline Health ### Build + CI gates | Gate | Status | | --- | --- | | Vercel build (Preview) | ✅ pass | | CI: Lint | ✅ pass | | CI: Schema map fresh | ❌ skipped | | Preview smoke | ❌ failure | | Visual diff | ❌ failure | _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.
Sign in to join this conversation.
No description provided.