rstillwell/deckhearth
1
0
Fork 0
Code Issues 1 Pull requests Projects Releases Packages Wiki Activity Actions
deckhearth/AGENTS.md
Randall Stillwell c7ad0ffa29 docs: post-convoy cleanup for fix-auth-bypass 
Closes out the fix-auth-bypass convoy (PRs #6–#11, merged through
1629afb) on the docs side. Code already on main; this PR is docs only.

Updates:

AGENTS.md
  - §1 auth bullet refreshed (auth-secret SoT, 24h TTL, no synthetic
    admin, login/register rate limit)
  - §3 conventions point at lib/auth-secret.js + lib/rate-limit.js
  - §4 gotchas #2/#3/#5 converted to "Resolved" notes in place
    (NOT renumbered, to preserve cross-references)
  - new #12 documents the KV_REST_API_* env-var convention
  - §5 setup list adds the rate-limit env vars
  - §6 testing rewritten for Vitest (16 unit tests, blocking CI gate)

.cursor/rules/auth-and-permissions.mdc
  - canonical-surface table gains lib/auth-secret.js + lib/rate-limit.js
  - token model now 24h (was 7d) with fail-loud explanation
  - server-side authorization patterns lead with null → 401 contract

.cursor/rules/api-routes.mdc
  - removes the "CRITICAL — known bug" callout (resolved by Brief 2)
  - adds a "Rate limiting" section with verbatim shape + env-var notes
  - "Dev/test endpoints" → "Removed" historical note so future agents
    searching for test-db understand why it's gone

.convoys/fix-auth-bypass.md (restored — was on convoy branch only)
  - frontmatter → status: shipped
  - new "Convoy outcome" section: briefs + commits + resolved gotchas,
    R1-R12 risk walk, env-var-rename deviation record, queued follow-up
    convoys, lessons learned

.convoys/fix-auth-bypass/brief-{1..5}-*.md (restored from convoy branch)
  - audit-trail completeness; convoy plan references them by name
  - brief 4 additionally updated: UPSTASH_REDIS_REST_* → KV_REST_API_*
    across init rules, smoke, pre-deploy checklist
  - brief 4 has a new "Post-merge addendum" explaining the rename

.convoys/ship-readiness.md
  - P0 #1, #2, #4 → RESOLVED with merge-commit citations
  - P0 #5 (CORS), #6 (rate limit) → PARTIAL with deferral pointers
    (cors-tighten and add-rate-limiting convoys)
  - each item gains an "As-shipped" line for self-containment

README.md
  - Next.js 15 → 16, TypeScript claim corrected to JS-with-devDep
  - auth + rate-limit + testing bullets updated
  - env-var template extended with KV_REST_API_*
  - deleted dev-endpoints note added to the API list
  - "Default Admin Account" section LEFT ALONE — drop-public-setup territory

Verified: build exit 0 (with JWT_SECRET set), 16/16 vitest tests pass,
lint baseline unchanged (128/81/47).

Convoy: fix-auth-bypass / role-doc-writer (closeout)
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-23 12:22:50 -05:00

83 lines
12 KiB
Markdown
Raw Blame History

# AGENTS.md — AI collaboration (tcg-vault)
Guidance for agents and humans working in this repo. Prefer existing patterns over new abstractions.
> Branding note: the repo, README, and seed data say "TCG Vault" and `admin@tcgvault.com`, but the Layout component renders "Deck Hearth". Pick one before launch — see `.convoys/` for tracking.
## 1. Project overview
A web app for managing trading-card-game collections (Magic, Pokémon, Lorcana). Users authenticate, build collections + decks, scan physical cards via a camera+AI-OCR flow, and share publicly. Admin users curate the card database.
- **Framework:** Next.js 16 (Pages router) + React 18, JavaScript (not TypeScript — see Gotcha #9)
- **Data:** Neon Postgres, accessed two different ways — `@neondatabase/serverless` (`lib/database.js`) AND raw `@vercel/postgres` (`pages/api/**`). Pick ONE; see Gotcha #1.
- **Auth:** Custom JWT (jsonwebtoken + bcryptjs), token stored in `localStorage`, sent as `Authorization: Bearer …`. No NextAuth. The secret + canonical 24h TTL come from `lib/auth-secret.js` (single source of truth; throws at module load if `JWT_SECRET` is unset). `getUserFromRequest` returns `null` for unauthenticated requests — no synthetic admin fallback — and login + register are rate-limited (5 attempts / 15 min via `@upstash/ratelimit`). The seed admin row (`admin@tcgvault.com` / `admin123`) still ships in `scripts/setup-neon-db.js`; see Gotcha #4.
- **UI:** Tailwind CSS + custom CSS variables for theming (light/dark via `lib/theme-context.js`)
- **Hosting:** Vercel (`vercel.json`, `.vercel/` present)
## 2. Architecture quick reference
| Area | Path | Notes |
| --- | --- | --- |
| Pages router views | `pages/*.js` | Public + auth views; uses `components/Layout.js` |
| API routes | `pages/api/**/*.js` | Express-style `handler(req, res)`. **30+ handlers depend on `lib/permission-middleware.js::getUserFromRequest`** |
| Shared UI | `components/*.js` | `Layout`, `CardItem`, `CameraScanner`, modal family |
| Auth + DB libs | `lib/*.js` | `auth-context`, `admin-auth`, `use-auth` (three parallel auth surfaces), `database`, `permission-middleware` |
| Migration scripts | `scripts/*.js` | 27+ one-off "add column" / "seed" scripts. No formal migration tool |
| Card-import jobs | `pages/api/cards/import-*.js`, `scripts/import-*.js` | Scryfall / Lorcana / Pokémon TCG APIs |
| Database schema | `scripts/setup-neon-db.js` | Bootstrap SQL DDL — the source of truth until a real migration tool lands |
| Schema map | `docs/SCHEMA_MAP.md` | Hand-curated; regenerate after schema changes |
Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 5602 edges). Ask: *"what calls `getUserFromRequest`?"* before refactoring auth.
## 3. Key conventions
- **Auth (server):** `import { getUserFromRequest } from '../../lib/permission-middleware'` → returns `{ userId, email, role }` or `null`. `null` means "send 401" — always early-return when the user is null before doing any work that depends on their identity.
- **Auth (client):** `import { useAuth } from '../lib/use-auth'`. Avoid `lib/auth-context.js` and `lib/admin-auth.js` for new code — they are legacy parallel implementations.
- **JWT secret + TTL:** `import { JWT_SECRET, JWT_TOKEN_TTL } from '../../lib/auth-secret.js'`. This is the only place either value is defined; do not reintroduce literal fallbacks. `JWT_TOKEN_TTL = '24h'` is canonical.
- **Auth helper (token mint / verify / password hash):** `import { ... } from '../../pages/api/auth-utils'` (`generateToken`, `verifyToken`, `hashPassword`, `verifyPassword`). Reads the secret + TTL from `lib/auth-secret.js` under the hood.
- **Rate limiting:** `import { checkAuthRateLimit } from '../../lib/rate-limit.js'` for any new auth-surface endpoint (`/api/auth/login` + `/api/auth/register` already wired). Returns `{ allowed, remaining, reset }`; on `!allowed` return 429 with a `Retry-After` header. See `.cursor/rules/api-routes.mdc` § "Rate limiting" for the verbatim shape.
- **Permission gate for collection routes:** wrap handlers with `withCollectionPermission('viewer' | 'editor' | 'owner')` from `lib/permission-middleware.js`.
- **DB access:** Use **tagged-template** style — `import { sql } from '@vercel/postgres'`. Avoid the legacy `lib/database.js` `db.query(string, params)` API; its parameter interpolation uses `sql.unsafe` and is a SQL-injection vector.
- **Activity logging:** `logCollectionActivity(collectionId, userId, action, details)` — call it from any handler that mutates a collection.
- **File names:** `kebab-case.js` for libs/scripts; `PascalCase.js` for React components.
- **Imports:** No path aliases configured; use relative imports.
- **Slugs:** `lib/slug-utils.js::generateUniqueSlug` for any user-facing identifier (collections, decks).
- **CSS theme tokens:** Components read `var(--bg-primary)`, `var(--text-primary)`, `var(--accent-ember)`, etc. — defined in `styles/`. Don't hardcode hex colors.
## 4. Common gotchas
- **#1 — Two SQL clients live in parallel.** `@neondatabase/serverless` (used by `lib/database.js`) and `@vercel/postgres` (used by most `pages/api/**` handlers). New code: prefer `@vercel/postgres` tagged templates. Migration to a single client is tracked in `.convoys/`.
- **#2 — `getUserFromRequest` synthetic-admin fallback. RESOLVED** by `fix-auth-bypass` Brief 2 (commit `258e479`). The helper now returns `null` for unauthenticated requests; `pages/api/auth/verify.js` returns 401 on the no-token branch. The 16 unit tests in `test/lib/permission-middleware.test.js` lock in the contract, including a negative regression against the old synthetic-admin shape. Entry kept (not renumbered) to preserve the audit trail and stable cross-references.
- **#3 — JWT_SECRET hardcoded across 7 files. RESOLVED** by `fix-auth-bypass` Brief 1 (commit `4a10dce`). `lib/auth-secret.js` is now the single source of truth and throws at module load when `JWT_SECRET` is unset. Canonical TTL is `JWT_TOKEN_TTL = '24h'`. The `'your-secret-key-change-in-production'` literal is gone from all 7 sites; CI lint passes against the post-fix tree. Entry kept (not renumbered) to preserve cross-references.
- **#4 — Default admin credentials are in the seed.** `admin@tcgvault.com` / `admin123` from `scripts/setup-neon-db.js`. Change the password immediately after running setup. Tracked by the queued `drop-public-setup` convoy.
- **#5 — `pages/api/setup-database.js` public endpoint. RESOLVED** by `fix-auth-bypass` Brief 3 (commit `fc0dd73`). The file is deleted along with the other three dev endpoints (`/api/simple`, `/api/test-auth`, `/api/test-db`), and `.github/workflows/ci.yml`'s new `forbidden-endpoints` job fails the build if any of them are re-introduced (or if a new `pages/api/test-*.js` file appears). Entry kept (not renumbered) to preserve cross-references.
- **#6 — Migrations are bare scripts.** `scripts/add-*.js` and `scripts/fix-*.js` are run-once jobs with no idempotency tracking. Adopt `node-pg-migrate`, `kysely`, or `drizzle-kit` before more schema changes.
- **#7 — Dual `is_public` semantics.** Collections and decks both have `is_public` columns; check which controls discovery vs. anonymous read in the relevant route.
- **#8 — Layout has hardcoded default user.** `Layout({ user = { email: 'me@randallstillwell.com', role: 'user' } })`. Anything rendering Layout without passing `user` will impersonate the maintainer. Pass `user` explicitly from every page.
- **#9 — `typescript` is a devDep, but the source is still JavaScript-only.** `package.json` lists `typescript@^5.9.3` purely so `eslint-config-next@16`'s bundled `typescript-eslint` chain can satisfy its hard `require('typescript')` at module load (the `peerDependenciesMeta.typescript.optional: true` flag in `eslint-config-next` only suppresses npm's install-time warning, not the runtime require). There is no `tsconfig.json`, no `.ts`/`.tsx` files, and no `// @ts-check` directives. Do not rename `.js` files to `.ts` or add a `tsconfig.json` without an explicit convoy decision — TypeScript adoption is its own scope. See `.convoys/bump-next-js.md` § Decisions C.
- **#10 — ESLint pinned to v9 (maintenance), not v10 (latest).** `devDependencies.eslint` is `^9.39.4` even though `latest` is `10.4.0`. We tried v10 and `npm run lint` crashed with `TypeError: scopeManager.addGlobals is not a function` because `eslint-config-next@16`'s bundled `typescript-eslint@8.x` predates ESLint v10's redesigned global-ingestion path. Reverted to v9 under Decision D. **Do NOT bump ESLint independently** — wait for the queued `bump-eslint-10` follow-up convoy, which is upstream-blocked until `typescript-eslint` ships a v10-tested release that `eslint-config-next` bundles. See `.convoys/bump-next-js.md` § Decisions D + "Follow-up convoys queued".
- **#11 — Turbopack is now the default bundler.** `next dev` and `next build` use Turbopack by default in Next.js 16. The fallback per command is `--webpack` (e.g. `next build --webpack`). We have no custom `webpack:` block in `next.config.js`, no custom loaders/aliases, and no Sass tilde imports, so Turbopack should "just work" — but if a build/runtime regression appears, reproduce on both bundlers before deciding whether to revert or pin a script to webpack. Do not pre-emptively switch to `--webpack`.
- **#12 — Rate-limit env vars are `KV_REST_API_URL` / `KV_REST_API_TOKEN`, not `UPSTASH_REDIS_REST_*`.** `lib/rate-limit.js` reads the Vercel Upstash Marketplace integration's auto-provisioned names. Three other Upstash-shaped vars exist in the Vercel-managed env (`KV_URL`, `REDIS_URL`, `KV_REST_API_READ_ONLY_TOKEN`) but our `@upstash/redis` REST client does not use them — do not wire to them. In prod, the rate-limit module **fails closed** if either of the two REST vars is missing (a single failed login is a better outcome than silently disabling brute-force protection). In dev / test, it warn-and-continues as a no-op so local work is unaffected when Upstash isn't wired up.
## 5. Running locally
- **Runtime:** Node 20 (Vercel default).
- **Setup:** `npm install`, copy `.env.local` template (POSTGRES_URL + JWT_SECRET + RESEND_API_KEY + BLOB_READ_WRITE_TOKEN; optionally KV_REST_API_URL + KV_REST_API_TOKEN to exercise the rate limiter locally — without them, `lib/rate-limit.js` warn-and-no-ops in dev), then `npm run setup-db` once.
- **Dev server:** `npm run dev` → http://localhost:3000.
## 6. Testing
- **Unit-test runner:** `vitest@^3.2.4` (installed via `fix-auth-bypass` Brief 5, commit `1629afb`). `npm test` for watch mode; `npm run test:run` for the CI / single-shot mode. Config in `vitest.config.js`, setup in `test/setup.js` (sets `JWT_SECRET` + `NODE_ENV=test` before any module loads). Specs live under `test/` mirroring source layout (`test/lib/*.test.js`, `test/api/*.test.js`).
- **Coverage today:** 16 unit tests covering the post-`fix-auth-bypass` auth surface — `lib/auth-secret.js` (3), `lib/permission-middleware.js::getUserFromRequest` (8, incl. a negative regression against the old synthetic-admin shape — Gotcha #2), and `pages/api/auth-utils.js` (5). These tests lock in the contracts established by Briefs 1 and 2; do not weaken them when refactoring auth.
- **CI:** the `test:` job in `.github/workflows/ci.yml` runs `npm run test:run` on every PR and push to `main` and is **blocking** (no `|| true`, no `continue-on-error`). A red test job blocks merge.
- **E2E / smoke runner:** `@playwright/test` is **still pending** — queued for the `adopt-playwright-smoke` convoy (see `.convoys/ship-readiness.md` § Proposed launch sequence step 10). Until it lands, `preview-smoke.yml` and `visual-diff.yml` are no-ops on the smoke side.
- **Manual QA:** `TESTING_GUIDE.md` still applies for surfaces not yet covered by automated tests (UI flows, scanner camera path, import jobs).
## 7. Deployment
- **Vercel** auto-deploys `main` and creates Preview deployments for every PR. `vercel.json` and `.vercel/` are committed. CI in `.github/workflows/` runs lint + types (no duplicate build — Vercel handles it).
- **Preview protection bypass for automation.** The project has a Protection Bypass for Automation token exposed locally as `VERCEL_AUTOMATION_BYPASS_SECRET` in `.env.local` (not committed). Smoke/visual-diff workflows pass this header (`x-vercel-protection-bypass`) when hitting password-protected preview URLs. Needed for the queued `adopt-playwright-smoke` convoy; do not log or echo the value.
## 8. Code graph
A local code-knowledge-graph MCP server (`user-code-review-graph`) is set up for this repo. Ask "what calls X?" or "show me the flow from /api/auth/login" instead of grepping. See [`docs/agent-context/README.md`](docs/agent-context/README.md).
Reference in a new issue View git blame Copy permalink