deckhearth/.convoys/drop-public-setup.md
Randall Stillwell 4d32659545 architect(drop-public-setup): plan + brief 1 (env-var admin password)
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 <cursoragent@cursor.com>
2026-05-23 17:02:44 -05:00

18 KiB
Raw 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)

Slice dependencies (multitask-ready)

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.