deckhearth/.convoys/drop-public-setup.md
Randall Stillwell 792439b5eb architect(drop-public-setup): expand scope with brief 2 (CJS→ESM)
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 <cursoragent@cursor.com>
2026-05-23 17:02:44 -05:00

19 KiB
Raw Permalink Blame History

name classification success_metric skip status created parent addresses depends_on
drop-public-setup convoy 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.
role-design-system-auditor
role-a11y-auditor
role-ux-reviewer
role-ia-architect
browser-smoke
open 2026-05-23 ship-readiness P0
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).

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=<value> 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":"<value>"}' 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=<different-value>. 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)
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)

slice_dependencies:
  - brief: 1
    depends_on: []
    files:
      - scripts/setup-neon-db.js
      - README.md
  - brief: 2
    depends_on:
      - brief: 1
    files:
      - scripts/setup-neon-db.js

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

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.

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). 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.