168 lines
15 KiB
Markdown
168 lines
15 KiB
Markdown
|
|
---
|
||
|
|
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="<generate with: openssl rand -hex 32>"
|
||
|
|
# 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="<generate with: openssl rand -base64 24>"
|
||
|
|
# 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://<your-upstash-host>.upstash.io"
|
||
|
|
KV_REST_API_TOKEN="<your-upstash-rest-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.
|