deckhearth/.convoys/drop-public-setup.md

185 lines
20 KiB
Markdown
Raw Permalink Normal View History

---
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: shipped
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
**As-shipped:** Brief 1 `ff80753` + Brief 2 `b63b509` (2026-05-23). Closes P0 #3.
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).
2026-05-23 13:32:52 -04:00
## 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) |
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 16:03:25 -04:00
| 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) |
2026-05-23 13:32:52 -04:00
### Slice dependencies (multitask-ready)
```yaml
slice_dependencies:
- brief: 1
depends_on: []
files:
- scripts/setup-neon-db.js
- README.md
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 16:03:25 -04:00
- brief: 2
depends_on:
- brief: 1
files:
- scripts/setup-neon-db.js
2026-05-23 13:32:52 -04:00
```
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 16:03:25 -04:00
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.
2026-05-23 13:32:52 -04:00
## 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.
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 16:03:25 -04:00
### 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.
2026-05-23 13:32:52 -04:00
## 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.
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 16:03:25 -04:00
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.
2026-05-23 13:32:52 -04:00
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.