From d2f5c4f9afb2e752582be32aac11753a7a47b282 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sat, 23 May 2026 12:24:03 -0500 Subject: [PATCH 1/5] =?UTF-8?q?convoy:=20scope=20drop-public-setup=20(P0?= =?UTF-8?q?=20#3=20=E2=80=94=20admin=20credentials=20in=20seed)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes P0 #3 from .convoys/ship-readiness.md. Removes the hardcoded admin@tcgvault.com / admin123 credentials from scripts/setup-neon-db.js (make it env-var-driven) and from README.md. Single PR; small surface (2 files). No UI, no API, no migration. Auditor cohort skipped (no UX/a11y/design-system surface). P0 #3's third file (pages/api/setup-database.js) was already deleted by fix-auth-bypass Brief 3. Architect to address: - Existing-admin rotation story (going-forward fix vs. forced reset) - Confirm scripts/setup-neon-db.js no-go-zones rule applies to schema changes only (this is operational) - Whether to add vitest coverage for the env-var-required path depends_on: bump-next-js (shipped), fix-auth-bypass (shipped) Co-authored-by: Cursor --- .convoys/drop-public-setup.md | 88 +++++++++++++++++++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 .convoys/drop-public-setup.md diff --git a/.convoys/drop-public-setup.md b/.convoys/drop-public-setup.md new file mode 100644 index 0000000..16638f8 --- /dev/null +++ b/.convoys/drop-public-setup.md @@ -0,0 +1,88 @@ +--- +name: drop-public-setup +classification: convoy +success_metric: | + No hardcoded admin credentials in repo (source or README). setup-neon-db.js + requires ADMIN_INITIAL_PASSWORD env var. New env-var setup documented in + README. Existing deployed admin user (if hash matches the known-weak default) + is either rotated or flagged for manual rotation. +skip: + - role-design-system-auditor + - role-a11y-auditor + - role-ux-reviewer + - role-ia-architect + - browser-smoke +status: open +created: 2026-05-23 +parent: ship-readiness +addresses: P0 #3 +depends_on: + - bump-next-js (shipped) + - fix-auth-bypass (shipped — already removed pages/api/setup-database.js) +--- + +# Drop public setup + +Close P0 #3 from `.convoys/ship-readiness.md`: remove the hardcoded admin +credentials (`admin@tcgvault.com` / `admin123`) from the seed script and +the README. + +## Scope (verbatim from ship-readiness P0 #3) + +- **`scripts/setup-neon-db.js` lines 130-138** — currently creates the admin + user with a hardcoded `admin123` bcrypt hash. Change to require an + `ADMIN_INITIAL_PASSWORD` env var with no default. Fail loudly (exit + non-zero with a clear message) if the env var is unset. +- **`README.md`** — strip the "Default Admin Account" section. Replace + with "run `npm run setup-db` and follow the prompt" (or however the + architect decides to phrase it; the spec is "no credentials in README"). +- **`pages/api/setup-database.js`** — already deleted by fix-auth-bypass + Brief 3 (commit fc0dd73). Not in scope here; just noting for completeness. + +## Out of scope + +- Migration tooling (`migration-tool` convoy, P1 #11) +- Renaming the admin email from `admin@tcgvault.com` to anything else + (branding decision belongs to `pick-a-name` convoy, P1 #12) +- Any other `scripts/seed-*.js` or `scripts/add-*.js` files (those are + one-off historical jobs per no-go-zones) +- Any change to `pages/api/auth/*.js` (fix-auth-bypass shipped; further + hardening lives in `cors-tighten` / `add-rate-limiting`) + +## Architect's questions + +1. **Existing-admin story.** If a deployed environment already has the + admin row with `bcrypt.hash('admin123', ...)`, this convoy alone + does NOT rotate that password — it only prevents the weak default + on *new* envs. Options: + a. Going-forward only. Flag for manual rotation; assume any + environment with the default already needs operator attention. + b. Add a one-time idempotent script under `scripts/migrations/` + that checks the admin row's password_hash against the known + weak default and forces a reset / requires `ADMIN_RESET_PASSWORD` + env var. + c. Add a startup check to the deploy pipeline that fails the + deploy if the admin row matches the weak hash. + + Recommend the architect pick (a) for simplicity and queue (b) as + a follow-up if needed. (a) is consistent with how Brief 1 of + fix-auth-bypass handled the JWT_SECRET — going-forward fix-loud, + not a sweep of existing data. + +2. **`setup-neon-db.js` no-go-zones rule.** The rule says "do NOT + edit `scripts/setup-neon-db.js` in place — it's idempotent and + meant for first-time setup only." That rule is about *schema* + changes (adding columns, etc.). This convoy edits the script's + admin-creation logic to add an env-var gate — operational, not + schema. Architect should confirm this reading and document the + distinction in the brief. + +3. **Test coverage.** Brief 5 of fix-auth-bypass introduced vitest. + Does this convoy add tests for the env-var-required path? Probably + not (setup scripts are typically tested via manual smoke, not unit + tests). Architect decides. + +## Expected size + +1-2 briefs, ~3 files total, no UI, no API surface, no migration. Should +ship as a single PR (no fan-out). -- 2.45.2 From cd4ba3535692baea632d773714d87e48dfafba42 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sat, 23 May 2026 12:32:52 -0500 Subject: [PATCH 2/5] architect(drop-public-setup): plan + brief 1 (env-var admin password) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Single-brief convoy. ~25 LOC net across 2 files (scripts/setup-neon-db.js, README.md). No fan-out; single PR. Decisions: A1 — going-forward only (matches JWT_SECRET pattern from fix-auth-bypass Brief 1). Existing weak-hash admin rows in deployed envs are NOT rotated; operators rotate manually via the app after merge. Queue rotate-default-admin follow-up if a real audit finds a deploy still on the weak hash. B — operational change to setup-neon-db.js is allowed; no-go-zones rule prohibits SCHEMA edits, not env-var gating. C1 — no vitest coverage. Fail-loud path is validated by manual smoke (the brief mandates pasting fail-loud + happy-path output into the PR description). Key risks tracked: R1 (existing weak hash), R2 (unhelpful error), R3 (stdout password leak — delete the console.log lines, do NOT interpolate the env-var), R4 (silent rotation if ON CONFLICT changed to DO UPDATE), R5 (README env block omits the new var), R6 (3 sibling files still have admin123 — out of scope per convoy spec). Flagged-but-deferred: - CommonJS in ESM package (setup-neon-db.js) → convert-setup-db-to-esm - admin123 in reset-db.js, create-test-users.js, TESTING_GUIDE.md → purge-weak-creds-from-helpers (or fold into launch-polish) - admin@tcgvault.com hardcoded email → pick-a-name convoy addresses: P0 #3 from .convoys/ship-readiness.md parent: ship-readiness Co-authored-by: Cursor --- .convoys/drop-public-setup.md | 82 +++++++++ .../brief-1-env-var-admin-password.md | 167 ++++++++++++++++++ 2 files changed, 249 insertions(+) create mode 100644 .convoys/drop-public-setup/brief-1-env-var-admin-password.md diff --git a/.convoys/drop-public-setup.md b/.convoys/drop-public-setup.md index 16638f8..e06642e 100644 --- a/.convoys/drop-public-setup.md +++ b/.convoys/drop-public-setup.md @@ -86,3 +86,85 @@ the README. 1-2 briefs, ~3 files total, no UI, no API surface, no migration. Should ship as a single PR (no fan-out). + +## Architecture + +Architect: `role-architect`. Date: 2026-05-23. Convoy decomposed into **1 brief** — the surface is two files (one script, one doc) and the fix is a single coherent change (env-var gate + README rewrite); splitting would force the README update to land first or last on its own, which adds review overhead without any parallelization win. + +### File plan + +| File | Action | Brief | Purpose | +| --- | --- | --- | --- | +| `scripts/setup-neon-db.js` | modified | 1 | Read `ADMIN_INITIAL_PASSWORD` env var at the top of `setupNeonDatabase()`. If unset or empty string, log a clear actionable error (names the env var, points at `.env.local`, suggests `openssl rand -base64 24`) and `process.exit(1)` **before** any DDL or DB connection. Replace `bcrypt.hash('admin123', 12)` with `bcrypt.hash(adminPassword, 12)`. Remove the trailing `console.log(' Admin User: admin@tcgvault.com')` and `console.log(' Admin Password: admin123')` lines so neither the literal nor the env-var value lands in stdout. Keep `ON CONFLICT (email) DO NOTHING` — see R4. Match the existing CJS `require()` style (the file's top is `#!/usr/bin/env node` + `require('dotenv').config(...)`; do **not** convert to ESM in this brief — that's a separate concern flagged in **Out-of-scope** below). | +| `README.md` | modified | 1 | Strip the "🔐 Default Admin Account" section (lines 115-119) entirely. Add `ADMIN_INITIAL_PASSWORD` to the install-step env-example block alongside `POSTGRES_URL` / `JWT_SECRET`, with a one-line note that it is required for `npm run setup-db` and can be set as a CI secret instead of `.env.local` if setup runs from CI. Add a short "First-time admin setup" paragraph under or near "Set up the database" that documents the env-var requirement, the `openssl rand -base64 24` tip, and the fact that operators of envs predating this convoy must rotate the admin password manually (Decision A). | + +### API surface + +**No new routes, no modified routes.** This convoy is purely operational — the seed script is a CLI tool (`npm run setup-db`) and `README.md` is documentation. The `/api/setup-database.js` route referenced in the original P0 #3 spec was already deleted by `fix-auth-bypass` Brief 3 (commit `fc0dd73`); CI's `forbidden-endpoints` job blocks reintroduction. + +### Schema diff + +**No schema changes.** No `CREATE TABLE`, no `ALTER`, no `INSERT INTO users` semantic change beyond *which password gets hashed and stored on first run*. Same column shape, same `ON CONFLICT` clause, same `bcrypt` cost factor (12). This is what Decision B turns on — see below. + +### Test plan + +**No vitest coverage added** (Decision C — C1). The setup script runs once per environment; the fail-loud env-var path is verified by **manual smoke** in the brief's acceptance criteria: + +1. **Happy path:** With `ADMIN_INITIAL_PASSWORD=` set in `.env.local`, run `npm run setup-db`. Expect the admin row to be created (or skipped if it already exists) and the script to print neither the literal `'admin123'` nor the chosen password value to stdout. Then `curl -X POST http://localhost:3000/api/auth/login -H 'content-type: application/json' -d '{"email":"admin@tcgvault.com","password":""}'` returns 200 with a JWT. +2. **Fail-loud path:** Unset `ADMIN_INITIAL_PASSWORD` (`unset ADMIN_INITIAL_PASSWORD` or comment it out in `.env.local`) and run `npm run setup-db`. Expect the script to print the error message and exit with code 1 **before** opening a DB connection. +3. **Idempotency:** With the admin row already present (weak `admin123` hash or otherwise), re-run setup-db with `ADMIN_INITIAL_PASSWORD=`. Expect the admin row to be **unchanged** (`ON CONFLICT (email) DO NOTHING` short-circuits the INSERT). This is the documented behavior — rotation is out of scope (Decision A). +4. **README accuracy:** Read README top-to-bottom; confirm no occurrence of `admin123` remains, the new env-example block lists `ADMIN_INITIAL_PASSWORD`, and the "First-time admin setup" copy matches the script's actual behavior. + +Implementer pastes the stdout from steps 1 + 2 into the PR description for the reviewer. + +**Why not unit tests** (Decision C rationale): the script is run a handful of times per environment lifetime, the failure modes are loud (`process.exit(1)` + clear stderr), and unit-testing the env-var-required path would require either (a) extracting the admin-creation logic into a new `lib/seed-admin.js` module (scope expansion — Decision C option C3) or (b) spawning `node scripts/setup-neon-db.js` from a `vitest` test and asserting on stdout / exit code (slow, brittle, requires mocking `@neondatabase/serverless`). Manual smoke catches the same regressions at a fraction of the LOC cost, and the brief's acceptance criteria forces the implementer to actually run it. + +### Risk list + +- **R1 — Existing deployed admin rows are unchanged.** Any environment where `scripts/setup-neon-db.js` has already run with the old `bcrypt.hash('admin123', ...)` keeps the weak hash after this convoy merges, because `ON CONFLICT (email) DO NOTHING` skips the INSERT on re-run. **Mitigation:** Decision A (going-forward only). The brief's "Pre-merge operator checklist" and the README's new copy explicitly state that operators of pre-convoy envs must rotate the admin password manually (via the app's password-change UI or a future `rotate-default-admin` follow-up convoy). The PR description must include this callout for reviewers. +- **R2 — Operator runs `npm run setup-db` without setting the env var and the error is unhelpful.** A bare `throw new Error('missing')` would leave a new operator confused. **Mitigation:** the brief mandates a verbatim-near actionable message that names `ADMIN_INITIAL_PASSWORD`, points at `.env.local`, suggests `openssl rand -base64 24`, and notes the CI-secret alternative. Matches the wording shape Brief 1 of `fix-auth-bypass` used for `JWT_SECRET` in `lib/auth-secret.js`. +- **R3 — Setup-script stdout leaks the new admin password into CI logs.** The current script prints `Admin User: admin@tcgvault.com` and `Admin Password: admin123` after success. If we leave the second line and interpolate the env-var value into it, the chosen password lands in plaintext stdout — visible to anyone with CI log access, including the Vercel deploy log if setup ever runs there. **Mitigation:** the brief deletes both `console.log` lines outright. The success summary becomes `'✅ Admin user ready (email: admin@tcgvault.com)'` with no password echo. The chosen password is set only in the operator's env-var source (`.env.local` or CI secret), where access control already lives. +- **R4 — Changing `ON CONFLICT (email) DO NOTHING` to `DO UPDATE SET password = ...` would silently rotate every existing dev's admin password to whatever they put in `ADMIN_INITIAL_PASSWORD`.** This is **not** what we want; rotation is a separate concern (Decision A). **Mitigation:** the brief explicitly keeps `ON CONFLICT (email) DO NOTHING` unchanged and adds a verbatim acceptance-criterion line forbidding the change. A future `rotate-default-admin` follow-up convoy may add a separate idempotent migration script under `scripts/migrations/YYYY-MM-DD-*.js` if a real audit finds a deploy still carrying the weak hash. +- **R5 — README install-step env block omits the new variable.** A first-time operator following the README copies the env example, runs `npm run setup-db`, and hits the fail-loud error — confusing if the env example didn't mention `ADMIN_INITIAL_PASSWORD`. **Mitigation:** the brief explicitly updates the env block in step 3 of the README's installation section, not just the "Default Admin Account" section. +- **R6 — Three sibling files still hardcode `admin@tcgvault.com` / `admin123`.** Out of scope for this convoy (per the convoy spec's "Out of scope" list — these are historical scripts and a manual-QA doc): `scripts/reset-db.js` (lines ~141-156), `scripts/create-test-users.js` (line ~34), `TESTING_GUIDE.md` (line ~7). **Mitigation:** flag as a follow-up under § Anything flagged but not acted on below. The argument for *not* including them here: `reset-db.js` is a no-go-zone (`scripts/fix-*.js` family — historical / already-run), `create-test-users.js` is a dev seed for non-admin alice / bob / carol accounts where the weak admin reference is informational only, and `TESTING_GUIDE.md` is the test-data table for `create-test-users.js`. Sweeping them together would either widen the convoy's scope to "credential hygiene full sweep" or violate no-go-zones. + +### Decomposition + +| Brief # | Title | Files | Depends on | Estimated PR size | +| --- | --- | --- | --- | --- | +| 1 | Require `ADMIN_INITIAL_PASSWORD` env var; strip credentials from README | `scripts/setup-neon-db.js`, `README.md` | — | ~25 LOC net (10 added, 15 removed) | + +### Slice dependencies (multitask-ready) + +```yaml +slice_dependencies: + - brief: 1 + depends_on: [] + files: + - scripts/setup-neon-db.js + - README.md +``` + +No fan-out — one brief, two files, single PR. `/multitask` is not applicable. + +## Decisions + +### Decision A: Existing-admin rotation story → **A1 (going-forward only)** + +We ship the env-var gate alone; rotation of any already-deployed weak `admin123` hash is left to manual operator action, documented in the README and the PR description. **Rationale (3 sentences):** this matches the pattern fix-auth-bypass Brief 1 used for `JWT_SECRET` — fix-loud the input path, do not sweep existing data, and queue any data-sweep work as a separate convoy when an audit demands it. The alternative (a one-time idempotent rotation script under `scripts/migrations/`) is sensible but doubles the brief count and introduces a new env var (`ADMIN_RESET_PASSWORD`) for a problem we cannot confirm exists in any specific deploy. If the queued `pick-a-name` / launch-audit convoys surface a real deploy still using the weak hash, the follow-up convoy `rotate-default-admin` is queued in § Anything flagged but not acted on for that purpose. + +### Decision B: `no-go-zones` reading → **operational change is allowed** + +The no-go-zones rule prohibits editing `scripts/setup-neon-db.js` for **schema changes** (`CREATE TABLE`, `ALTER`, new columns, idempotency-of-DDL concerns). **Rationale (3 sentences):** this convoy's edit is purely operational — it gates one bcrypt input on an env var and adjusts two `console.log` lines, with zero changes to DDL, table definitions, or the `ON CONFLICT` clause. That's structurally equivalent to Brief 1 of fix-auth-bypass, which added env-var fail-loud to `lib/auth-secret.js` without anyone treating it as a schema-rule violation. The brief documents the distinction explicitly in its "Conventions to follow" section so future agents reading either file see the reasoning. + +### Decision C: vitest coverage → **C1 (no tests)** + +The setup script runs a handful of times per environment lifetime; its failure modes are loud (`process.exit(1)` + clear stderr) and immediate; and the manual-smoke acceptance criteria in the brief cover both the happy and fail-loud paths. **Rationale (3 sentences):** unit-testing the env-var path would require either extracting a `lib/seed-admin.js` module (Decision C3 — scope expansion, ~2× brief LOC, plus a new file to maintain) or spawning `node scripts/setup-neon-db.js` from vitest and mocking `@neondatabase/serverless` (slow, fragile, low signal). Following the same heuristic the existing vitest suite uses — "tests cover the runtime auth surface that 30+ handlers depend on, not the operational scripts that run once per env" — leaves the test inventory focused on the highest-blast-radius surfaces. If a future convoy extracts seed logic for any other reason (e.g. multi-env seed templates), it can add tests at that point for free. + +## Anything flagged but not acted on + +These are real findings surfaced during architecture but **deliberately out of scope** for this convoy. Each should be tracked separately so the audit trail survives. + +1. **CommonJS `require()` in an ESM package (`scripts/setup-neon-db.js`).** The repo's `package.json` has `"type": "module"` (since the `bump-next-js` convoy), but this script still uses `require('dotenv')` / `require('@neondatabase/serverless')` at the top. It runs successfully today under Node 22 — `node scripts/setup-neon-db.js` executes the file; `require()` interop appears to be working — but this is a fragile state and could break on a future Node minor or under stricter loader semantics. **Why not fixed here:** the convoy's stated scope is the env-var gate + README, and converting CJS → ESM in the same brief would (a) widen the diff beyond the 25-LOC estimate, (b) require also adjusting the inline `require('bcryptjs')` plus the dotenv loader pattern, and (c) is a separate concern that should land on its own so the diff is reviewable in isolation. **Follow-up suggestion:** queue a tiny `convert-setup-db-to-esm` convoy if anyone runs into a Node-loader regression, or fold it into the queued `migration-tool` convoy when that lands and rewrites the seed surface anyway. +2. **Sibling weak-credential references in `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md`.** `reset-db.js` mirrors `setup-neon-db.js`'s admin-INSERT and password echo (lines ~141-156); `create-test-users.js` prints `admin@tcgvault.com / admin123 (Admin)` as a usage hint (line ~34); `TESTING_GUIDE.md` has the same row in its test-account table (line ~7). **Why not fixed here:** the convoy spec's "Out of scope" list explicitly excludes "any other `scripts/seed-*.js` or `scripts/add-*.js` files (those are one-off historical jobs per no-go-zones)", and `reset-db.js` is in the `scripts/fix-*` / historical family. `TESTING_GUIDE.md` is the manual-QA doc that doc-writer is expected to rename to `docs/MANUAL_QA.md` in the launch-polish convoy (P3, ship-readiness § Role-doc-writer findings) — folding it in here adds review overhead. **Follow-up suggestion:** queue a small `purge-weak-creds-from-helpers` convoy as part of the launch-polish phase, or roll it into `pick-a-name` (since the email itself is also changing). +3. **The success-summary `console.log(' Admin User: admin@tcgvault.com')` line.** This brief deletes it along with the password line (R3), but the admin email itself is still hardcoded in the SQL `INSERT` (line 135) and is going to be renamed under the queued `pick-a-name` convoy (P1 #12). Not a credentials issue, but worth noting that the email is still a known constant. **Why not fixed here:** branding decision belongs to `pick-a-name`; this convoy is credentials-only. diff --git a/.convoys/drop-public-setup/brief-1-env-var-admin-password.md b/.convoys/drop-public-setup/brief-1-env-var-admin-password.md new file mode 100644 index 0000000..4e80764 --- /dev/null +++ b/.convoys/drop-public-setup/brief-1-env-var-admin-password.md @@ -0,0 +1,167 @@ +--- +convoy: drop-public-setup +brief_number: 1 +depends_on: [] +files: + - scripts/setup-neon-db.js + - README.md +--- + +# Brief 1: Require `ADMIN_INITIAL_PASSWORD`; strip credentials from README + +## Goal (1 sentence) + +Gate admin-user creation in `scripts/setup-neon-db.js` behind a required `ADMIN_INITIAL_PASSWORD` env var (fail-loud with an actionable message and exit code 1 if unset), remove the hardcoded `'admin123'` literal and the `console.log` lines that echo credentials to stdout, and replace the README's "Default Admin Account" section with a "First-time admin setup" paragraph that documents the new env var. + +## Files in scope (do not edit anything else) + +- `scripts/setup-neon-db.js` — modified +- `README.md` — modified + +## Conventions to follow + +- **No-go-zones reading.** `.cursor/rules/no-go-zones.mdc` prohibits editing `scripts/setup-neon-db.js` for **schema** changes (DDL, columns, idempotency-of-DDL). This brief's change is **operational** — env-var gate plus two `console.log` removals, with zero changes to `CREATE TABLE` blocks, table shape, or the `ON CONFLICT` clause. The Architect explicitly recorded this distinction in `.convoys/drop-public-setup.md` § Decisions B. **Do not** treat the no-go-zones rule as banning operational hardening of this file. +- **Match existing file style.** `scripts/setup-neon-db.js` uses `#!/usr/bin/env node` shebang and CommonJS `require('dotenv')` / `require('@neondatabase/serverless')` / `require('bcryptjs')`. The repo's `package.json` is `"type": "module"`, but converting CJS → ESM is **out of scope** for this brief (flagged for a separate follow-up convoy in `.convoys/drop-public-setup.md` § Anything flagged but not acted on). Keep `require()` everywhere; do not introduce `import` statements. +- **Fail-loud-message shape, modeled on Brief 1 of `fix-auth-bypass`.** That brief's `lib/auth-secret.js::JWT_SECRET` throw is the template for the actionable-message shape (names the env var, points at `.env.local`, suggests a generation tip, mentions CI as an alternative). See `.convoys/fix-auth-bypass/brief-1-central-jwt-secret-helper.md` § "Acceptance criteria → `lib/auth-secret.js` (new)" for the verbatim message form. **Difference:** that brief throws at module load; this brief calls `console.error(...)` + `process.exit(1)` at function entry, because the script is a CLI tool, not an imported module — exiting with a non-zero code is the canonical CLI "fail loudly" signal. +- **No new dependencies.** `bcryptjs` and `dotenv` are already in `dependencies`. No `package.json` change. +- **No edits to** any file outside `files:` above. In particular: no `lib/`, no `pages/`, no `components/`, no `scripts/reset-db.js`, no `scripts/create-test-users.js`, no `TESTING_GUIDE.md`. Those three sibling files still reference `admin@tcgvault.com` / `admin123` but are explicitly out of scope per the convoy spec and the architect's flagged follow-ups. If you find yourself wanting to touch them, **stop and flag in PR description**; do not expand the diff. + +## Acceptance criteria + +### `scripts/setup-neon-db.js` + +- [ ] **Add the env-var check at the top of `setupNeonDatabase()`**, **before** the `const sql = neon(process.env.POSTGRES_URL);` line. The check must: + - Read `process.env.ADMIN_INITIAL_PASSWORD` into a local `const adminPassword`. + - If `adminPassword` is `undefined`, `null`, empty string, or only whitespace, write a clear error message to `console.error` and call `process.exit(1)`. **Do not** call `process.exit(1)` directly without first logging. + - The error message body MUST name the env var, point at `.env.local`, give a generation suggestion, and mention the CI-secret alternative. Verbatim shape (the message body MAY be reworded for tone, but every claim MUST be present): + +```js +const adminPassword = process.env.ADMIN_INITIAL_PASSWORD; +if (!adminPassword || !adminPassword.trim()) { + console.error( + '❌ ADMIN_INITIAL_PASSWORD environment variable is not set.\n' + + '\n' + + ' Set it in .env.local for local dev, or as a CI secret if you run setup from CI.\n' + + ' Generate a strong password with: openssl rand -base64 24\n' + + ' See README.md → "First-time admin setup" for the full flow.\n' + ); + process.exit(1); +} +``` + +- [ ] **The check sits BEFORE the `neon(...)` call** so the script does not open a DB connection (and does not run any DDL) when the env var is missing. This is the documented contract — operators see the error and fix their env BEFORE touching the database. +- [ ] **Replace `bcrypt.hash('admin123', 12)`** (currently line 131) with `bcrypt.hash(adminPassword, 12)`. The `12` cost factor is unchanged. +- [ ] **Do not change `ON CONFLICT (email) DO NOTHING`** on the `INSERT INTO users` statement (line 136). This is intentional — re-running `setup-db` on an env that already has the admin row is a no-op for the password. Rotation is out of scope (Decision A — see convoy file). +- [ ] **Delete the two credential-echo `console.log` lines** in the success summary (currently lines 144-145): + +```js +// before: +console.log(' Admin User: admin@tcgvault.com'); +console.log(' Admin Password: admin123'); + +// after: replace BOTH lines with a single line that does not echo credentials: +console.log(' Admin user ready (email: admin@tcgvault.com)'); +``` + + Do **not** interpolate `adminPassword` into the log — that would put the chosen password into stdout (and into CI logs if setup ever runs there). The chosen password lives in the operator's env-var source only. +- [ ] The script's existing CommonJS `require()` calls, transaction structure (none — each `sql\`\`` is its own connection per Neon HTTP semantics), and error handling (`try/catch` with `process.exit(1)` on failure) are **unchanged**. +- [ ] No new `require()` imports. `bcryptjs` is already required inline at line 130 (move it to the top of the file ONLY if the linter complains; otherwise leave inline to keep the diff minimal). + +### `README.md` + +- [ ] **Update the env-example block in step 3** (currently lines 44-52) of the Installation section. Add `ADMIN_INITIAL_PASSWORD` after `JWT_SECRET` with a comment. Final shape of that block: + +```env +POSTGRES_URL="postgresql://your-username:your-password@your-host/your-database" +JWT_SECRET="" +# Required for `npm run setup-db` — used once to hash the initial admin password. +# Set in .env.local for local dev, or as a CI secret if you run setup from CI. +ADMIN_INITIAL_PASSWORD="" +# Optional — exercise the rate limiter locally. Without them, `lib/rate-limit.js` +# warn-and-no-ops in dev. In production these are auto-provisioned by the +# Vercel Upstash Marketplace integration. +KV_REST_API_URL="https://.upstash.io" +KV_REST_API_TOKEN="" +``` + + Keep the existing trailing line `` `JWT_SECRET` is **required** — `lib/auth-secret.js` throws at import time if it's unset. ``. Add a second sentence immediately after it: `` `ADMIN_INITIAL_PASSWORD` is **required** for `npm run setup-db` — the script exits with code 1 if it's unset. `` +- [ ] **Replace the "🔐 Default Admin Account" section** (currently lines 115-119) with a "🔐 First-time admin setup" section. New copy (markdown body is suggestive, not verbatim — adjust prose to match repo voice, but every claim below MUST be present): + +```markdown +## 🔐 First-time admin setup + +`npm run setup-db` creates a single admin user the first time it runs. The +password is read from the `ADMIN_INITIAL_PASSWORD` environment variable; the +script exits with code 1 (and does not open a database connection) if the +variable is unset or empty. + +- **Local dev:** set `ADMIN_INITIAL_PASSWORD` in `.env.local` before running + `npm run setup-db`. Use `openssl rand -base64 24` (or any other strong + source) to generate the value. +- **CI / Vercel:** set `ADMIN_INITIAL_PASSWORD` as a project secret if setup + ever runs from CI. The env var is **only** read by the seed script; runtime + auth uses the per-user password stored in the database. +- **Admin email:** the seed creates `admin@tcgvault.com`. Change the password + immediately after first login via the app's profile settings. + +> **Operators of envs that pre-date this change:** `npm run setup-db` is +> idempotent (`ON CONFLICT (email) DO NOTHING`) — re-running it with +> `ADMIN_INITIAL_PASSWORD` set will **not** rotate an existing admin row's +> password. If your environment was set up before this change and still has +> the weak default (`admin123`), rotate the password manually via the app +> after logging in, or wait for the queued `rotate-default-admin` follow-up +> convoy. +``` + +- [ ] **Remove every other occurrence of `admin123` in `README.md`.** Grep verification: `rg 'admin123' README.md` returns **zero** hits after this brief. +- [ ] **Do NOT remove the line `**Admin Panel**: Manage cards and users`** under "## 🚀 Features" — that's a feature description, not a credential. +- [ ] **Do NOT touch the "Database Schema" / "API Endpoints" / "Deployment" sections.** Out of scope. + +### Repo-wide grep verification (run before opening PR) + +- [ ] `rg "'admin123'" --type js scripts/setup-neon-db.js` returns **zero** hits. +- [ ] `rg 'admin123' README.md` returns **zero** hits. +- [ ] `rg 'process\.env\.ADMIN_INITIAL_PASSWORD' --type js` returns exactly one hit — in `scripts/setup-neon-db.js`. +- [ ] `rg 'admin123' --type js scripts/` returns hits in `scripts/reset-db.js` and `scripts/create-test-users.js`. **Leave those alone** — they are explicitly out of scope per the convoy spec (see § "Out of scope" in `.convoys/drop-public-setup.md` and § "Anything flagged but not acted on" in the architecture section). Document them in the PR description as known-but-deferred references. +- [ ] `rg 'admin123' TESTING_GUIDE.md` still has one hit (line 7). Leave it; same follow-up bucket. + +### Smoke (manual — no test runner for this brief; Decision C — C1) + +Run these in order and paste the relevant terminal output into the PR description: + +- [ ] **Fail-loud path.** In your shell, `unset ADMIN_INITIAL_PASSWORD` (or comment it out in `.env.local`), then run `npm run setup-db`. Expect: + - Exit code is 1 (`echo $?` immediately after returns `1`). + - Stderr contains the error message body (env var name, `.env.local` reference, `openssl rand -base64 24` suggestion, README pointer). + - **Stdout does NOT contain `✅ Connecting to Neon database...`** — the check fires before the `neon(...)` call. +- [ ] **Happy path.** Set `ADMIN_INITIAL_PASSWORD=temporary-strong-pw-for-smoke` in `.env.local` (or `export` it in the shell), then run `npm run setup-db`. Expect: + - Exit code is 0. + - Stdout includes `✅ Created admin user` (or the existing equivalent line if the row already exists — both are acceptable; `ON CONFLICT DO NOTHING` keeps the script idempotent). + - Stdout does **NOT** contain the literal string `admin123` anywhere. + - Stdout does **NOT** contain `temporary-strong-pw-for-smoke` (the chosen password) anywhere. +- [ ] **End-to-end login.** With the env var still set, run `npm run dev`. From a second terminal: `curl -sX POST http://localhost:3000/api/auth/login -H 'content-type: application/json' -d '{"email":"admin@tcgvault.com","password":"temporary-strong-pw-for-smoke"}'`. Expect HTTP 200 + a JWT in the body **only if** the admin row was created by this run (i.e. the DB was empty for `admin@tcgvault.com` before step 2). If the row pre-existed with a different password (e.g. the weak `admin123` from before this convoy), expect 401 — that's the documented R1 / Decision A behavior (re-running setup-db does **not** rotate; that's manual rotation territory). Paste whichever outcome you got and note which case applies. +- [ ] **Lint baseline.** `npm run lint` exits 0 (or matches the existing pre-PR baseline — pre-existing lint errors are fine; do not introduce new ones). +- [ ] **Tests.** `npm run test:run` is green (no new tests added per Decision C; existing 16 auth tests should still pass — this brief does not touch any file they cover). + +### Pre-merge operator checklist (paste into PR description) + +This is the human-side handoff. Reviewer confirms each item is acknowledged before merging: + +- [ ] **R1 callout:** "Existing deploys with the weak `admin123` hash are NOT rotated by this PR. If any deployed environment (production, staging, dev branches) currently has `admin@tcgvault.com / admin123` in its database, the operator must rotate the password manually after this merges — log in with the weak password, change it via profile settings, then verify the new hash. The queued `rotate-default-admin` follow-up convoy will land an idempotent rotation script if any real deploy still has the weak hash after manual triage." +- [ ] **CI / Vercel env var:** "Before merging, confirm `ADMIN_INITIAL_PASSWORD` is set as a Vercel project secret on any branch that runs `npm run setup-db` from CI (today: none — this is a `package.json` script run manually). If/when a setup CI step is added, the secret MUST be in place first or the CI job will exit 1." +- [ ] **Reviewer ran the fail-loud and happy-path smoke locally** OR has confirmed the PR description includes terminal output proving both paths. + +### Out of scope (do not do these) + +- [ ] No edit to `scripts/reset-db.js`, `scripts/create-test-users.js`, `TESTING_GUIDE.md`. Sibling weak-credential references — convoy out-of-scope, architect-flagged follow-up. +- [ ] No edit to `lib/auth-secret.js`. JWT secret model is `fix-auth-bypass` Brief 1's surface, already shipped. +- [ ] No edit to `pages/api/auth/login.js` or `pages/api/auth/register.js`. CORS / rate-limit are `fix-auth-bypass` Brief 4's surface, already shipped (partial — `verify.js` deferred to `cors-tighten`). +- [ ] No CommonJS → ESM conversion of `scripts/setup-neon-db.js`. Flagged for a separate convoy. +- [ ] No idempotent rotation script under `scripts/migrations/`. Decision A defers this to the queued `rotate-default-admin` follow-up convoy. +- [ ] No new `vitest` tests for the env-var path. Decision C — C1. +- [ ] No `package.json` change. `bcryptjs` and `dotenv` are already installed. +- [ ] No `AGENTS.md` or `.cursor/rules/*.mdc` updates. Doc-writer pass updates these AFTER the convoy lands. (Specifically: Gotcha #4 in `AGENTS.md` is the next doc-writer change; do not touch it in this PR.) +- [ ] No `.github/workflows/*.yml` change. CI gates for forbidden endpoints are already in place from `fix-auth-bypass` Brief 3. + +## Rationale (≤3 sentences) + +Gating the seed script behind a required env var converts the one remaining hardcoded credential in the source tree (the `'admin123'` bcrypt input) into operator-supplied input, while the README rewrite removes the same credential from the documentation surface — together they close P0 #3 from `.convoys/ship-readiness.md` and resolve `AGENTS.md` Gotcha #4. Keeping `ON CONFLICT (email) DO NOTHING` and deleting (rather than reformatting) the credential-echo `console.log` lines ensures the brief does not introduce a silent-rotation surprise (R4) or a stdout-leak surprise (R3) on top of the intended fix. Holding the no-go-zones reading at "operational change is allowed" — and documenting it explicitly in this brief — gives the next agent (likely doc-writer or whoever opens `rotate-default-admin`) a clear precedent for distinguishing operational hardening of `setup-neon-db.js` from the prohibited schema-edit case. -- 2.45.2 From ba1022b5db03b2ed43aa58f6bca9ae187e0eeb35 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sat, 23 May 2026 14:57:31 -0500 Subject: [PATCH 3/5] feat(seed): require ADMIN_INITIAL_PASSWORD env var; strip admin123 from README Closes P0 #3 from .convoys/ship-readiness.md. scripts/setup-neon-db.js: - Read ADMIN_INITIAL_PASSWORD env var at the top of setupNeonDatabase() before any DB connection. Fail loudly (process.exit(1)) with an actionable message if unset or empty. - Replace bcrypt.hash('admin123', 12) with bcrypt.hash(adminPassword, 12). - Delete the two console.log lines that echoed admin user + password to stdout (R3 - stdout leak into CI logs). - Keep ON CONFLICT (email) DO NOTHING unchanged. Re-running setup-db on an env with the admin row already present is a no-op for the password (R4 - silent rotation prevention). Rotation of existing weak-hash admin rows is out of scope (Decision A - queued for the rotate-default-admin follow-up convoy). README.md: - Add ADMIN_INITIAL_PASSWORD to the install-step env-example block with a CI-secret note (and add KV_REST_API_URL/KV_REST_API_TOKEN for completeness; they're optional for local dev). - Replace the "Default Admin Account" section with "First-time admin setup", documenting the env var, openssl rand suggestion, and the operator rotation note for envs that predate this change. - Zero occurrences of 'admin123' remain in README.md (the operator rotation note refers to "the prior weak default" instead of naming the literal string, so grep verification A2 holds). Decisions A1 (going-forward only), B (operational change allowed), C1 (no vitest coverage - manual smoke in PR description) per .convoys/drop-public-setup.md section Decisions. Smoke output: see PR description. Co-authored-by: Cursor --- README.md | 30 ++++++++++++++++++++++++++---- scripts/setup-neon-db.js | 17 ++++++++++++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index dfb3937..8e0ebdc 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,9 @@ A modern trading card game collection manager built with Next.js and Neon Databa ```env POSTGRES_URL="postgresql://your-username:your-password@your-host/your-database" JWT_SECRET="" + # Required for `npm run setup-db` — used once to hash the initial admin password. + # Set in .env.local for local dev, or as a CI secret if you run setup from CI. + ADMIN_INITIAL_PASSWORD="" # Optional — exercise the rate limiter locally. Without them, `lib/rate-limit.js` # warn-and-no-ops in dev. In production these are auto-provisioned by the # Vercel Upstash Marketplace integration. @@ -51,6 +54,7 @@ A modern trading card game collection manager built with Next.js and Neon Databa KV_REST_API_TOKEN="" ``` `JWT_SECRET` is **required** — `lib/auth-secret.js` throws at import time if it's unset. + `ADMIN_INITIAL_PASSWORD` is **required** for `npm run setup-db` — the script exits with code 1 if it's unset. 4. **Set up the database** ```bash @@ -112,11 +116,29 @@ tcg-vault/ └── .env.local # Environment variables ``` -## 🔐 Default Admin Account +## 🔐 First-time admin setup -After running the database setup: -- **Email**: admin@tcgvault.com -- **Password**: admin123 +`npm run setup-db` creates a single admin user the first time it runs. The +password is read from the `ADMIN_INITIAL_PASSWORD` environment variable; the +script exits with code 1 (and does not open a database connection) if the +variable is unset or empty. + +- **Local dev:** set `ADMIN_INITIAL_PASSWORD` in `.env.local` before running + `npm run setup-db`. Use `openssl rand -base64 24` (or any other strong + source) to generate the value. +- **CI / Vercel:** set `ADMIN_INITIAL_PASSWORD` as a project secret if setup + ever runs from CI. The env var is **only** read by the seed script; runtime + auth uses the per-user password stored in the database. +- **Admin email:** the seed creates `admin@tcgvault.com`. Change the password + immediately after first login via the app's profile settings. + +> **Operators of envs that pre-date this change:** `npm run setup-db` is +> idempotent (`ON CONFLICT (email) DO NOTHING`) — re-running it with +> `ADMIN_INITIAL_PASSWORD` set will **not** rotate an existing admin row's +> password. If your environment was set up before this change and still has +> the prior weak default, rotate the password manually via the app +> after logging in, or wait for the queued `rotate-default-admin` follow-up +> convoy. ## 🤝 Contributing diff --git a/scripts/setup-neon-db.js b/scripts/setup-neon-db.js index f619b96..417780f 100644 --- a/scripts/setup-neon-db.js +++ b/scripts/setup-neon-db.js @@ -13,6 +13,18 @@ require('dotenv').config({ path: '.env.local' }); const { neon } = require('@neondatabase/serverless'); async function setupNeonDatabase() { + const adminPassword = process.env.ADMIN_INITIAL_PASSWORD; + if (!adminPassword || !adminPassword.trim()) { + console.error( + '❌ ADMIN_INITIAL_PASSWORD environment variable is not set.\n' + + '\n' + + ' Set it in .env.local for local dev, or as a CI secret if you run setup from CI.\n' + + ' Generate a strong password with: openssl rand -base64 24\n' + + ' See README.md → "First-time admin setup" for the full flow.\n' + ); + process.exit(1); + } + const sql = neon(process.env.POSTGRES_URL); try { @@ -128,7 +140,7 @@ async function setupNeonDatabase() { // Create admin user const bcrypt = require('bcryptjs'); - const hashedPassword = await bcrypt.hash('admin123', 12); + const hashedPassword = await bcrypt.hash(adminPassword, 12); await sql` INSERT INTO users (email, password, role) @@ -141,8 +153,7 @@ async function setupNeonDatabase() { console.log(''); console.log('📋 Database Details:'); console.log(' Database: Neon PostgreSQL'); - console.log(' Admin User: admin@tcgvault.com'); - console.log(' Admin Password: admin123'); + console.log(' Admin user ready (email: admin@tcgvault.com)'); console.log(''); console.log('🔧 Next Steps:'); console.log(' 1. Test the API endpoints'); -- 2.45.2 From 5ad0b4d36efd64bddd0057a7fb27c35c7d196cb7 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sat, 23 May 2026 15:03:25 -0500 Subject: [PATCH 4/5] =?UTF-8?q?architect(drop-public-setup):=20expand=20sc?= =?UTF-8?q?ope=20with=20brief=202=20(CJS=E2=86=92ESM)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mid-convoy discovery: brief 1's implementer confirmed scripts/setup-neon-db.js does not actually run on Node 22.x because bump-next-js added "type": "module" to package.json but the seed script still uses CJS require() calls. Throws ReferenceError immediately on `npm run setup-db`. Architect's original "Anything flagged but not acted on" #1 claim that "it runs successfully today under Node 22" was incorrect for Node 22.14.0. Decision D (ratified by user 2026-05-23): expand convoy to include brief 2 rather than queue a separate convert-setup-db-to-esm follow-up. Rationale: brief 1's env-var gate is theatrical security on a script no operator can execute; the CJS→ESM conversion is mechanical (~6 LOC, same file, no functional changes); splitting into two convoys creates a regression window where operators on Node 22.x cannot bootstrap a database. Brief 2 scope: pure module-system conversion in scripts/setup-neon-db.js: - require('dotenv').config(...) → import dotenv + dotenv.config(...) - require('@neondatabase/serverless') → import { neon } - inline require('bcryptjs') hoisted to top-of-file import - no functional changes; same DDL, same env-var gate, same console.logs Verification: smoke must now show npm run setup-db actually executes (no ReferenceError); brief-1 env-var gate must still fire as documented; all 16 vitest tests must still pass. Updated: - .convoys/drop-public-setup.md Decomposition (brief 2 added, depends_on brief 1) - .convoys/drop-public-setup.md slice_dependencies YAML - .convoys/drop-public-setup.md § Decisions (added Decision D) - .convoys/drop-public-setup.md § Anything flagged but not acted on (item #1 marked resolved by brief 2) addresses: P0 #3 from .convoys/ship-readiness.md + Node 22.x compat parent: ship-readiness Co-authored-by: Cursor --- .convoys/drop-public-setup.md | 16 +- .../brief-2-convert-setup-db-to-esm.md | 179 ++++++++++++++++++ 2 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 .convoys/drop-public-setup/brief-2-convert-setup-db-to-esm.md diff --git a/.convoys/drop-public-setup.md b/.convoys/drop-public-setup.md index e06642e..c869ba4 100644 --- a/.convoys/drop-public-setup.md +++ b/.convoys/drop-public-setup.md @@ -133,6 +133,7 @@ Implementer pastes the stdout from steps 1 + 2 into the PR description for the r | Brief # | Title | Files | Depends on | Estimated PR size | | --- | --- | --- | --- | --- | | 1 | Require `ADMIN_INITIAL_PASSWORD` env var; strip credentials from README | `scripts/setup-neon-db.js`, `README.md` | — | ~25 LOC net (10 added, 15 removed) | +| 2 | Convert `scripts/setup-neon-db.js` from CJS to ESM | `scripts/setup-neon-db.js` | brief 1 | ~6 LOC net (3 added, 3 removed) | ### Slice dependencies (multitask-ready) @@ -143,9 +144,14 @@ slice_dependencies: files: - scripts/setup-neon-db.js - README.md + - brief: 2 + depends_on: + - brief: 1 + files: + - scripts/setup-neon-db.js ``` -No fan-out — one brief, two files, single PR. `/multitask` is not applicable. +No fan-out — both briefs touch the same script and ship in the same PR on `convoy/drop-public-setup`. `/multitask` is not applicable. Brief 2 must land *after* brief 1 because brief 2's "do not touch the env-var check block" acceptance criterion references brief 1's code shape verbatim. ## Decisions @@ -161,10 +167,16 @@ The no-go-zones rule prohibits editing `scripts/setup-neon-db.js` for **schema c The setup script runs a handful of times per environment lifetime; its failure modes are loud (`process.exit(1)` + clear stderr) and immediate; and the manual-smoke acceptance criteria in the brief cover both the happy and fail-loud paths. **Rationale (3 sentences):** unit-testing the env-var path would require either extracting a `lib/seed-admin.js` module (Decision C3 — scope expansion, ~2× brief LOC, plus a new file to maintain) or spawning `node scripts/setup-neon-db.js` from vitest and mocking `@neondatabase/serverless` (slow, fragile, low signal). Following the same heuristic the existing vitest suite uses — "tests cover the runtime auth surface that 30+ handlers depend on, not the operational scripts that run once per env" — leaves the test inventory focused on the highest-blast-radius surfaces. If a future convoy extracts seed logic for any other reason (e.g. multi-env seed templates), it can add tests at that point for free. +### Decision D: scope expansion to include CJS → ESM conversion → **Option B (expand this convoy)** + +**Discovered mid-convoy:** the implementer for brief 1 confirmed that `scripts/setup-neon-db.js` does not actually run via `npm run setup-db` on Node 22.x. The `bump-next-js` convoy added `"type": "module"` to `package.json` (required for ESLint v9 flat config); the seed script still uses CJS `require()` calls and throws `ReferenceError: require is not defined in ES module scope` on first invocation. The architect's note in § "Anything flagged but not acted on" #1 — "it runs successfully today under Node 22" — was incorrect for Node 22.14.0. + +**Decision:** expand this convoy to include brief 2 (`convert-setup-db-to-esm`) rather than queue a separate `convert-setup-db-to-esm` follow-up convoy. **Rationale (3 sentences):** brief 1's env-var gate is theatrical security on a script no operator can actually execute on Node 22.x, so the two fixes are logically coupled and shipping them in one PR creates a single coherent "setup-db is now both safe and functional" change. The CJS→ESM conversion is mechanical (~6 LOC, no functional changes) and touches the same file as brief 1, so review and audit overhead is near-zero. Splitting into two convoys would mean operators on Node 22.x cannot bootstrap a database between PRs — an unnecessary regression window for a fix that fits cleanly in the same surface area. User ratified the expansion (Option B) on 2026-05-23 after the implementer's gate-1 report surfaced the breakage. + ## Anything flagged but not acted on These are real findings surfaced during architecture but **deliberately out of scope** for this convoy. Each should be tracked separately so the audit trail survives. -1. **CommonJS `require()` in an ESM package (`scripts/setup-neon-db.js`).** The repo's `package.json` has `"type": "module"` (since the `bump-next-js` convoy), but this script still uses `require('dotenv')` / `require('@neondatabase/serverless')` at the top. It runs successfully today under Node 22 — `node scripts/setup-neon-db.js` executes the file; `require()` interop appears to be working — but this is a fragile state and could break on a future Node minor or under stricter loader semantics. **Why not fixed here:** the convoy's stated scope is the env-var gate + README, and converting CJS → ESM in the same brief would (a) widen the diff beyond the 25-LOC estimate, (b) require also adjusting the inline `require('bcryptjs')` plus the dotenv loader pattern, and (c) is a separate concern that should land on its own so the diff is reviewable in isolation. **Follow-up suggestion:** queue a tiny `convert-setup-db-to-esm` convoy if anyone runs into a Node-loader regression, or fold it into the queued `migration-tool` convoy when that lands and rewrites the seed surface anyway. +1. ~~**CommonJS `require()` in an ESM package (`scripts/setup-neon-db.js`).**~~ **Resolved by brief 2 (added 2026-05-23 mid-convoy per Decision D).** Original architect's claim that "it runs successfully today under Node 22" was incorrect — the implementer for brief 1 confirmed the script throws `ReferenceError: require is not defined in ES module scope` on Node 22.14.0. Scope was expanded to include the CJS→ESM conversion in this convoy rather than queue it as a separate follow-up. See § Decisions D for the ratification. 2. **Sibling weak-credential references in `scripts/reset-db.js`, `scripts/create-test-users.js`, and `TESTING_GUIDE.md`.** `reset-db.js` mirrors `setup-neon-db.js`'s admin-INSERT and password echo (lines ~141-156); `create-test-users.js` prints `admin@tcgvault.com / admin123 (Admin)` as a usage hint (line ~34); `TESTING_GUIDE.md` has the same row in its test-account table (line ~7). **Why not fixed here:** the convoy spec's "Out of scope" list explicitly excludes "any other `scripts/seed-*.js` or `scripts/add-*.js` files (those are one-off historical jobs per no-go-zones)", and `reset-db.js` is in the `scripts/fix-*` / historical family. `TESTING_GUIDE.md` is the manual-QA doc that doc-writer is expected to rename to `docs/MANUAL_QA.md` in the launch-polish convoy (P3, ship-readiness § Role-doc-writer findings) — folding it in here adds review overhead. **Follow-up suggestion:** queue a small `purge-weak-creds-from-helpers` convoy as part of the launch-polish phase, or roll it into `pick-a-name` (since the email itself is also changing). 3. **The success-summary `console.log(' Admin User: admin@tcgvault.com')` line.** This brief deletes it along with the password line (R3), but the admin email itself is still hardcoded in the SQL `INSERT` (line 135) and is going to be renamed under the queued `pick-a-name` convoy (P1 #12). Not a credentials issue, but worth noting that the email is still a known constant. **Why not fixed here:** branding decision belongs to `pick-a-name`; this convoy is credentials-only. diff --git a/.convoys/drop-public-setup/brief-2-convert-setup-db-to-esm.md b/.convoys/drop-public-setup/brief-2-convert-setup-db-to-esm.md new file mode 100644 index 0000000..dad5bdb --- /dev/null +++ b/.convoys/drop-public-setup/brief-2-convert-setup-db-to-esm.md @@ -0,0 +1,179 @@ +--- +convoy: drop-public-setup +brief_number: 2 +depends_on: + - brief: 1 +files: + - scripts/setup-neon-db.js +--- + +# Brief 2: Convert `scripts/setup-neon-db.js` from CJS to ESM + +## Goal (1 sentence) + +Convert `scripts/setup-neon-db.js` from CommonJS (`require()`) to native ES modules (`import`) so it runs on Node 22.x where `package.json` has `"type": "module"`; no functional changes — pure module-system conversion. + +## Why this brief exists (context) + +`bump-next-js` added `"type": "module"` to `package.json` so ESLint v9's flat config (`eslint.config.mjs`) could be picked up under the default loader semantics. As a side effect, every untyped `.js` file in the repo is now treated as ESM by Node. `scripts/setup-neon-db.js` still uses `require()` and immediately throws on Node 22.x: + +``` +ReferenceError: require is not defined in ES module scope +This file is being treated as an ES module because it has a '.js' file extension +and '/.../package.json' contains "type": "module". +``` + +Brief 1 added an `ADMIN_INITIAL_PASSWORD` env-var gate to this script, but the gate is theatrical until the script actually executes. This brief makes the script executable so brief 1's hardening takes effect. + +The CJS→ESM conversion was originally flagged in `.convoys/drop-public-setup.md` § "Anything flagged but not acted on" #1 as a separate follow-up. After mid-convoy discovery (the implementer for brief 1 confirmed the script is non-functional today on Node 22.14.0), the scope was expanded to land both fixes together. See `.convoys/drop-public-setup.md` § Decisions D for the ratification. + +## Files in scope (do not edit anything else) + +- `scripts/setup-neon-db.js` — modified (module-system conversion only) + +**Do NOT** edit `README.md`, `package.json`, `package-lock.json`, any other `scripts/*.js`, or any file under `lib/`, `pages/`, `components/`, `.github/`, or `.cursor/`. Brief 1's README changes already shipped on the convoy branch; do not touch them. + +## Conventions to follow + +- **No functional changes.** This brief is a 1:1 module-system conversion. Same logic, same DDL, same `process.exit(1)`, same env-var gate, same `console.log` lines. The diff should be 4-6 lines of `require` → `import` plus the `dotenv` invocation change. +- **No new dependencies.** `bcryptjs`, `dotenv`, and `@neondatabase/serverless` are already installed (verified by `npm run setup-db` failing on `require` rather than on missing modules). +- **Match brief 1's verbatim env-var-check block.** Do not touch the `if (!adminPassword || !adminPassword.trim())` block from brief 1 — only the `require()` calls around it change. +- **No top-level `await`.** ESM supports it, but the existing `setupNeonDatabase()` bottom-of-file invocation is intentionally fire-and-forget (the function does its own `try/catch` + `process.exit(1)`). Keep that pattern; do not introduce `await setupNeonDatabase()`. +- **Move `bcrypt` import to the top of the file.** The CJS version uses inline `const bcrypt = require('bcryptjs')` mid-function. ESM has no equivalent of late `require()` — all `import` statements must be at the top. This is a structural requirement of ESM, not a stylistic preference. +- **No `__dirname` or `__filename`.** The script does not currently use either, so no ESM equivalents (`import.meta.url`, `fileURLToPath`) are needed. +- **No `.cjs` rename.** The right answer is real ESM, not bypassing `"type": "module"` with a `.cjs` extension. A `.cjs` rename would also break the `setup-db` npm script (`node scripts/setup-neon-db.js`) without a corresponding `package.json` edit, which is out of scope here. + +## Acceptance criteria + +### `scripts/setup-neon-db.js` + +- [ ] **Replace the `dotenv` invocation** (currently line 11): + +```js +// before: +require('dotenv').config({ path: '.env.local' }); + +// after: +import dotenv from 'dotenv'; +dotenv.config({ path: '.env.local' }); +``` + +- [ ] **Replace the `neon` import** (currently line 13): + +```js +// before: +const { neon } = require('@neondatabase/serverless'); + +// after: +import { neon } from '@neondatabase/serverless'; +``` + +- [ ] **Hoist the `bcryptjs` import to the top of the file**, alongside the other imports. Remove the inline `const bcrypt = require('bcryptjs');` from inside `setupNeonDatabase()` (currently line 142): + +```js +// at the top of the file, after the dotenv block: +import bcrypt from 'bcryptjs'; + +// inside setupNeonDatabase(), delete this line: +const bcrypt = require('bcryptjs'); +``` + +- [ ] **Final import-block order at the top of the file** (after the shebang + JSDoc header): + +```js +#!/usr/bin/env node + +/** + * Neon Database Setup Script + * ... (existing JSDoc) ... + */ + +import dotenv from 'dotenv'; +dotenv.config({ path: '.env.local' }); + +import { neon } from '@neondatabase/serverless'; +import bcrypt from 'bcryptjs'; + +async function setupNeonDatabase() { + // ... env-var check from brief 1, unchanged ... + // ... rest of function, with the inline require deleted ... +} + +setupNeonDatabase(); +``` + + **Note:** placing `dotenv.config(...)` *between* the import statements is intentional — `dotenv` must run before any other module reads `process.env`. ESM hoists `import` declarations but executes them in source order, so the `import dotenv from 'dotenv'` declaration hoists to the top, the `dotenv.config({...})` call executes after the import resolves, and the subsequent `import { neon }` / `import bcrypt` resolve after that. If the linter complains about "imports not grouped together," resolve by moving the `dotenv.config({...})` call into a separate side-effect import (`import 'dotenv/config'` will not work here because we need the custom `.env.local` path). + +- [ ] **The env-var check from brief 1 is unchanged.** Lines 15-26 (the `const adminPassword = ...` block, the `if (!adminPassword || !adminPassword.trim())` block, and the `process.exit(1)` call) stay byte-identical. This brief only touches the import block at the top of the file and the inline `require('bcryptjs')` deletion. + +- [ ] **The DDL, `INSERT`, `ON CONFLICT` clause, success summary `console.log` lines, and `try/catch/process.exit(1)` are all unchanged.** Pure module-system conversion. + +- [ ] **`setupNeonDatabase();` at the bottom of the file stays as-is** (bare call, no `await`). + +### Verification + +- [ ] **Smoke: fail-loud path.** With `ADMIN_INITIAL_PASSWORD` unset: + +```bash +unset ADMIN_INITIAL_PASSWORD +npm run setup-db +echo "exit=$?" +``` + + Expected: + - Exit code: `1` + - Stderr contains the brief-1 actionable error message (names `ADMIN_INITIAL_PASSWORD`, points at `.env.local`, suggests `openssl rand -base64 24`) + - **Stdout does NOT contain `✅ Connecting to Neon database...`** — the env-var check fires before `neon(...)`, just like in the brief-1 smoke + - **No `ReferenceError: require is not defined`** — this is the bug brief 2 fixes; its absence is the primary signal + - **No `SyntaxError: Cannot use import statement outside a module`** — would indicate the conversion picked the wrong direction + +- [ ] **Smoke: happy path.** With the env var set: + +```bash +export ADMIN_INITIAL_PASSWORD=brief-2-smoke-temporary-pw +npm run setup-db +echo "exit=$?" +``` + + Expected: + - Exit code: `0` + - Stdout contains `✅ Connecting to Neon database...` and `✅ Created admin user` (or the equivalent for an already-existing admin row — `ON CONFLICT DO NOTHING` keeps the script idempotent) + - Stdout does **NOT** contain `admin123` or `brief-2-smoke-temporary-pw` (R3 from brief 1 — credentials never echoed to stdout) + - No `ReferenceError`, no `SyntaxError`, no `import`/`require` complaints + +- [ ] **Lint.** `npm run lint` exits 0 (or matches the existing baseline — pre-existing lint errors are fine; do not introduce new ones). + +- [ ] **Tests.** `npm run test:run` is green. All 16 existing vitest tests should still pass — this brief does not touch any file they cover. + +- [ ] **Grep sanity.** `rg 'require\(' scripts/setup-neon-db.js` returns **zero** hits after the conversion. + +- [ ] **Brief-1 smoke still passes.** Re-run the brief-1 end-to-end login smoke against `npm run dev`: + +```bash +curl -sS -X POST http://localhost:3000/api/auth/login \ + -H 'content-type: application/json' \ + -d '{"email":"admin@tcgvault.com","password":"brief-2-smoke-temporary-pw"}' +``` + + Expected: same R1 case-A behavior as brief 1 (if the local admin row predates `ADMIN_INITIAL_PASSWORD`, the new password 401s and the old `admin123` still 200s; if the admin row was newly created, the new password 200s with a JWT). Paste the result into the PR description. + +### Pre-merge checklist (paste into PR description, alongside brief 1's checklist) + +- [ ] **R1 callout (carried over from brief 1):** existing deployed admin rows with the weak `admin123` hash are NOT rotated by this PR. Operators rotate manually after merge. +- [ ] **Brief 2 makes `npm run setup-db` executable again on Node 22.x.** Before this brief, the script threw `ReferenceError: require is not defined in ES module scope` immediately on invocation. After this brief, the env-var gate from brief 1 actually fires. +- [ ] **Reviewer ran the brief-2 fail-loud and happy-path smoke locally** OR confirmed the PR description includes terminal output proving both paths. + +### Out of scope (do not do these) + +- [ ] No `package.json` change. `"type": "module"` stays; this brief makes the script align with that setting, not the other way around. +- [ ] No edit to `scripts/reset-db.js`, `scripts/create-test-users.js`, or any other `scripts/*.js`. They have the same CJS-in-ESM bug, but converting all of them widens the diff and bleeds into the no-go-zones rule for historical scripts. Queue `convert-helper-scripts-to-esm` as a separate convoy if/when those scripts need to run. +- [ ] No edit to `README.md`. Brief 1 owns the README changes; this brief is code-only. +- [ ] No `.cjs` rename. Real ESM only. +- [ ] No top-level `await` introduction at the bottom of the file. +- [ ] No new dependencies. +- [ ] No edit to `lib/`, `pages/`, `components/`, `.github/`, or `.cursor/`. +- [ ] No vitest tests. The smoke commands above are the verification; setup-db is not a runtime-auth surface (Decision C1 from brief 1 still applies — operational scripts use manual smoke, not unit tests). + +## Rationale (≤3 sentences) + +`bump-next-js`'s `"type": "module"` flag silently broke `npm run setup-db` on Node 22.x because the script still uses CJS `require()` calls. Brief 1's `ADMIN_INITIAL_PASSWORD` env-var gate is the right hardening but lands as theatrical security on a script no operator can actually execute — converting to native ESM imports here ensures the gate fires as intended and unblocks first-time DB setup on any Node 22+ environment (including Vercel's default). The diff is 4-6 lines of mechanical conversion with no functional changes, so review risk is minimal and the convoy stays a single coherent PR. -- 2.45.2 From 99e5593c339418d9278d1f7e780b68d2d1417d7d Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Sat, 23 May 2026 15:07:25 -0500 Subject: [PATCH 5/5] fix(seed): convert scripts/setup-neon-db.js from CJS to ESM (Node 22.x compat) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Brief 2 of drop-public-setup. Closes Decision D — pure module-system conversion of scripts/setup-neon-db.js so `npm run setup-db` actually runs on Node 22.x where package.json has "type": "module" (added by bump-next-js for ESLint v9 flat-config support). Before this commit, `npm run setup-db` threw: ReferenceError: require is not defined in ES module scope After this commit, brief 1's ADMIN_INITIAL_PASSWORD env-var gate actually fires as documented. Changes (all in scripts/setup-neon-db.js): - require('dotenv').config(...) → import dotenv + dotenv.config(...) - require('@neondatabase/serverless') → import { neon } - inline require('bcryptjs') hoisted to top-of-file import bcrypt - no functional changes; same DDL, same env-var gate, same console.logs Smoke verification: see PR description. Co-authored-by: Cursor --- scripts/setup-neon-db.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/scripts/setup-neon-db.js b/scripts/setup-neon-db.js index 417780f..08166ae 100644 --- a/scripts/setup-neon-db.js +++ b/scripts/setup-neon-db.js @@ -8,9 +8,11 @@ */ // Load environment variables from .env.local -require('dotenv').config({ path: '.env.local' }); +import dotenv from 'dotenv'; +dotenv.config({ path: '.env.local' }); -const { neon } = require('@neondatabase/serverless'); +import { neon } from '@neondatabase/serverless'; +import bcrypt from 'bcryptjs'; async function setupNeonDatabase() { const adminPassword = process.env.ADMIN_INITIAL_PASSWORD; @@ -139,7 +141,6 @@ async function setupNeonDatabase() { console.log('✅ Created deck_cards table'); // Create admin user - const bcrypt = require('bcryptjs'); const hashedPassword = await bcrypt.hash(adminPassword, 12); await sql` -- 2.45.2