From f83d71abe062865ce13258718e23179802079b20 Mon Sep 17 00:00:00 2001 From: varutasu <104105839+varutasu@users.noreply.github.com> Date: Fri, 12 Jun 2026 23:23:44 -0500 Subject: [PATCH] =?UTF-8?q?convoy:=20scripts/rotate-admin-password.js=20?= =?UTF-8?q?=E2=80=94=20one-shot=20admin=20rotation=20(rotate-default-admin?= =?UTF-8?q?)=20(#141)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes the operator caveat from the `drop-public-setup` convoy: deployed envs that ran `npm run setup-db` BEFORE `ff80753` (2026-05-22) still carry the historical `admin123` bcrypt hash. The seed is idempotent (`ON CONFLICT (email) DO NOTHING`), so re-running setup-db is a no-op on existing rows. ## Design — D1: which option from the 3-option menu? | Option | Picked? | Why | |---|---|---| | A. Close as no-op (defer rotation to manual app login) | No | Leaves a real-world residue if any pre-drop-public-setup env still exists — and an audit is harder than just shipping the script. | | B. One-shot parameterized rotation script | **Yes** | Tightly scoped (~120 lines). Audit-trail-preserving (`updated_at` bump). Reusable for future rotations. No new auth surface in the app. | | C. First-login forced password reset flow in the app | No | Right product answer, but heavier scope (new route, new flag column, UI work). Deferred as the queued `force-admin-password-reset-flow` convoy. | ## Script shape `scripts/rotate-admin-password.js`: - Reads `POSTGRES_URL` + `ADMIN_NEW_PASSWORD` from env (or `.env.local`). - Optional `ADMIN_EMAIL` override; defaults to `admin@deckhearth.com`. Pass `admin@tcgvault.com` for envs that pre-date `pick-a-name` (squash `9abbab6`, 2026-05-24). - Fail-loud-exits BEFORE opening any DB connection if: - `POSTGRES_URL` is unset - `ADMIN_NEW_PASSWORD` is unset or empty - `ADMIN_NEW_PASSWORD` is shorter than 12 chars - Validates the target row EXISTS AND has `role = 'admin'` before touching it. Refuses to rotate non-admin rows even if `ADMIN_EMAIL` points at one. Refuses to rotate when multiple rows match (impossible given the UNIQUE(email) constraint, but checked anyway). - Hashes with bcryptjs at 12 rounds — same as `setup-neon-db.js`. - After UPDATE, re-fetches the row and runs `bcrypt.compare(newPassword, row.password_hash)`; exits non-zero if the compare fails (extremely unlikely, but catches silent UPDATE failures). - NEVER echoes the password to stdout / stderr / shell history. The only output is the row id, email, role, and updated_at. Same import shape as the existing `scripts/migrations/2026-05-24-rename-admin-email.js` (ESM, `dotenv.config({ path: '.env.local' })`, `import { neon } from '@neondatabase/serverless'`, tagged-template SQL) — keeps the "11 scripts/* using neon() directly" graveyard from gaining new patterns; fits the `purge-neondatabase-serverless-fully` follow-up convoy's existing audit shape. ## Out of scope - Sibling test users (alice / bob in `scripts/create-test-users.js`) — dev fixtures, not real auth surfaces. Documented inline + in AGENTS.md Gotcha #4. - First-login forced password reset flow — deferred as the queued `force-admin-password-reset-flow` convoy (it's the right product answer, but heavier scope than this hygiene PR). - Email rotation (already handled by `scripts/migrations/2026-05-24-rename-admin-email.js`). ## Test plan - [x] `node --check scripts/rotate-admin-password.js` — syntax OK - [x] `npm run lint` — clean (1 pre-existing unrelated warning) - [x] `npm run test:run` — 118 tests pass - [ ] CI on this PR - [ ] Operator-side smoke test (NOT covered by CI): - Set `ADMIN_NEW_PASSWORD=test-rotation-12chars` against a throwaway Neon branch DB, run the script, log in via the app with the new password, run the script again with a different password, log in again. Skip if there's no convenient throwaway DB. Co-authored-by: Cursor --- .convoys/.metrics.jsonl | 2 + .convoys/ship-readiness.md | 2 +- AGENTS.md | 10 +- scripts/rotate-admin-password.js | 167 +++++++++++++++++++++++++++++++ 4 files changed, 179 insertions(+), 2 deletions(-) create mode 100644 scripts/rotate-admin-password.js diff --git a/.convoys/.metrics.jsonl b/.convoys/.metrics.jsonl index 5f32388..b39f1b8 100644 --- a/.convoys/.metrics.jsonl +++ b/.convoys/.metrics.jsonl @@ -68,3 +68,5 @@ {"ts": "2026-06-13T02:21:43Z", "role": "role-implementer", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "brief": 1, "classification": "ci", "duration_s": 900, "outcome": "pr-open"} {"ts": "2026-06-13T02:57:31Z", "role": "role-implementer", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "brief": 2, "classification": "ci", "duration_s": 1200, "outcome": "pr-open"} {"ts": "2026-06-13T02:57:31Z", "role": "role-reviewer", "convoy": "harden-visual-diff-gate", "repo": "tcg-vault", "skip_flags": [], "brief": 2, "classification": "ci", "duration_s": 300, "outcome": "approved"} +{"ts": "2026-06-13T04:14:48Z", "role": "role-architect", "convoy": "rotate-default-admin", "repo": "tcg-vault", "skip_flags": [], "classification": "security", "duration_s": 240, "outcome": "option-B-chosen"} +{"ts": "2026-06-13T04:14:48Z", "role": "role-implementer", "convoy": "rotate-default-admin", "repo": "tcg-vault", "skip_flags": [], "classification": "security", "duration_s": 900, "outcome": "pr-open"} diff --git a/.convoys/ship-readiness.md b/.convoys/ship-readiness.md index 81e295d..377042f 100644 --- a/.convoys/ship-readiness.md +++ b/.convoys/ship-readiness.md @@ -415,7 +415,7 @@ Total: ~14 convoys to get from current state to public-launch-ready. Estimate 4- Follow-ups surfaced mid-convoy or mid-PR that didn't fit the original launch sequence but need to land before public traffic. Listed in priority order; not all will be P0/P1 — most are CI / DX / hygiene polish. -- **`rotate-default-admin`** (priority: P2 hygiene). Operator-rotation script for envs that ran `setup-neon-db.js` before `drop-public-setup` and still carry the weak `admin123` bcrypt hash. Surfaced in P0 #3 § Operator caveat. Optional: do nothing if no audit finds a deployed env with the weak hash. +- **`rotate-default-admin`** — **RESOLVED 2026-06-13** by PR #141 (`scripts/rotate-admin-password.js`). The convoy chose option B from the architect's three-option menu (close as no-op / build script / build forced-rotation flow): a parameterized one-shot rotation script that's safer than "manually change via app" (audit-trail-preserving via `updated_at`) and lighter than building a first-login forced-rotation flow in the app (that heavier option is the deferred `force-admin-password-reset-flow` convoy). Script reads `POSTGRES_URL` + `ADMIN_NEW_PASSWORD` env vars, validates target row exists + has `role='admin'`, refuses to rotate non-admin rows, verifies the new hash matches the supplied plaintext via `bcrypt.compare` post-update, never echoes the password. AGENTS.md Gotcha #4 documents the rotation workflow. Sibling test users (alice/bob in `scripts/create-test-users.js`) intentionally NOT rotated (dev fixtures, not real auth surfaces). - **`delete-dead-lorcana-import`** — **RESOLVED 2026-06-02** by PR #59 (`8262fec`). Deleted `pages/api/cards/import-lorcana.js` and `scripts/import-lorcana.js`; no dedicated convoy file (cleanup tracked here only). Entry kept for audit trail. - **`tighten-visual-diff-path-filter`** — **RESOLVED 2026-05-26** by `tighten-visual-diff-path-filter` convoy, squash commit `ba95462` (PR #26). Single-edit `paths:` filter change in `.github/workflows/visual-diff.yml`: inserted `'!pages/api/**'` immediately after `'pages/**'` (order-sensitive per GitHub Actions' minimatch path-filter semantics — exclusions only fire after a prior include matches). Verified the YAML deserialization order at gate time (`['pages/**', '!pages/api/**', 'components/**', 'styles/**', 'tailwind.config.js', 'postcss.config.js']`). `preview-smoke.yml` left untouched (no `paths:` filter; intentionally fires on every PR). Diff: 2 files, +279 / -0 (1 YAML entry + inline comment block + the planning convoy file). **Post-merge verification still pending** — the only true verification is that the next API-only PR after this merges does NOT trigger `Screenshot diff`. PR #30 (`single-sql-client`, squash `c403ea4`) was the **first API-only PR post-merge** and its CI Checks tab showed `Screenshot diff: not triggered` — empirical confirmation that the `!pages/api/**` exclusion fires correctly. The next-API-only-PR success line was originally specified in the convoy file's § Verification plan as the deferred-to-post-merge gate; this is that confirmation. Entry kept (not removed) to preserve the audit trail. See `.convoys/tighten-visual-diff-path-filter.md` § As-shipped. - **`purge-weak-creds-from-helpers`** — **RESOLVED 2026-05-26** by `purge-weak-creds-from-helpers` convoy, squash commit `5f2b234` (PR #27). The umbrella is now closed; both remaining halves shipped together. **Multi-convoy history:** (1) `drop-public-setup` Brief 1+2 (`ff80753` + `b63b509`) removed the first `admin123` literal from `scripts/setup-neon-db.js` and set the env-var + fail-loud + no-echo precedent. (2) `pick-a-name` Brief 2 (`9abbab6`) swept the `@tcgvault.com` literals in the three helper paths to `@deckhearth.com` together with the migration script. (3) `fix-reset-db-script` (`3ab9bf8`, PR #25) removed the second `admin123` from `scripts/reset-db.js` and the second `Admin Password:` echo. (4) **This convoy (PR #27)** closes the umbrella by sweeping the last two files: `scripts/create-test-users.js` (alice/bob fixtures, previously hardcoding `bcrypt.hash('alice123', 12)` + `bcrypt.hash('bob123', 12)` and echoing both literals to stdout) and `TESTING_GUIDE.md` (Test Accounts table previously documenting the weak literals). The post-convoy contract: single `TEST_USERS_PASSWORD` env var (intentional simplification per Risk R2 — these are collaboration-flow demo fixtures, not independent identities), fail-loud at the top of `createTestUsers()` BEFORE any DB connection, no password echo anywhere (`✅ Created Alice (alice@deckhearth.com / alice123)` → `✅ Created Alice (alice@deckhearth.com)`), `ON CONFLICT (email) DO NOTHING` preserved. Diff: 3 files, +249 / -22. Lint preserved at 125 (post-PR-#31 baseline); vitest 21/21. ESM-already (this was the first of the three weak-creds-shape convoys to skip the CJS→ESM half because `scripts/create-test-users.js` was already top-level ESM). **Operator caveat:** existing alice/bob rows in already-seeded envs are NOT rotated by re-running the script — `ON CONFLICT` preserves the old hashes; operators must rotate manually via the app or drop those rows and re-seed. Same caveat as the `drop-public-setup` admin-row guidance. **Surfaced out-of-scope follow-up:** `purge-quick-login-from-loginpage` — see new queue entry below. Entry kept (not removed) to preserve the audit trail. See `.convoys/purge-weak-creds-from-helpers.md` § As-shipped. diff --git a/AGENTS.md b/AGENTS.md index ada31e2..a4cd851 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -96,7 +96,15 @@ Code graph is indexed by `user-code-review-graph` MCP (122 files, 628 nodes, 560 - **#1 — Two SQL clients live in parallel. RESOLVED** by `single-sql-client` convoy (PR #30, squash commit `c403ea4`, 2026-05-26). `lib/database.js` is deleted; the 2 callers (`pages/api/auth-utils.js` source + `test/api/auth-utils.test.js` mock) migrated to `@vercel/postgres` tagged templates (byte-equivalent SQL semantics for the two single-parameter SELECT queries). The convoy's architect audit (D4) confirmed no current call site actually exercised the `sql.unsafe` injection vector — the `userId` callers passed a numeric SERIAL from a verified JWT — so this was foot-gun removal rather than a live security finding. **`@neondatabase/serverless` is still in `package.json` as a runtime dep** because 11 `scripts/*` helpers continue to use `neon()` directly (`setup-neon-db.js`, `migrations/2026-05-24-rename-admin-email.js`, `reset-db.js`, plus 8 historical `add-*` / `fix-*` / `seed-*` jobs). Those scripts use the safe tagged-template shape (`await sql\`...\``), not the deleted wrapper's unsafe `db.query(string, params)` shape. Full dep purge is tracked as the queued `purge-neondatabase-serverless-fully` follow-up (now unblocked by `migration-tool` PR #32 — the migration helpers all use `node-pg-migrate`'s `pg` client, not `@neondatabase/serverless`, so the only remaining direct `neon()` consumers are `setup-neon-db.js` (admin seed), `reset-db.js`, and the historical graveyard). Entry kept (not renumbered) to preserve cross-references. - **#2 — `getUserFromRequest` synthetic-admin fallback. RESOLVED** by `fix-auth-bypass` Brief 2 (commit `258e479`). The helper now returns `null` for unauthenticated requests; `pages/api/auth/verify.js` returns 401 on the no-token branch. The 16 unit tests in `test/lib/permission-middleware.test.js` lock in the contract, including a negative regression against the old synthetic-admin shape. Entry kept (not renumbered) to preserve the audit trail and stable cross-references. - **#3 — JWT_SECRET hardcoded across 7 files. RESOLVED** by `fix-auth-bypass` Brief 1 (commit `4a10dce`). `lib/auth-secret.js` is now the single source of truth and throws at module load when `JWT_SECRET` is unset. Canonical TTL is `JWT_TOKEN_TTL = '24h'`. The `'your-secret-key-change-in-production'` literal is gone from all 7 sites; CI lint passes against the post-fix tree. Entry kept (not renumbered) to preserve cross-references. -- **#4 — Default admin credentials in the seed. RESOLVED** by `drop-public-setup` Brief 1 (commit `ff80753`) + Brief 2 (commit `b63b509`). `scripts/setup-neon-db.js` no longer hardcodes `admin123`; it reads `ADMIN_INITIAL_PASSWORD` from the environment and exits with code 1 before opening a DB connection if the var is unset. README's "Default Admin Account" section is replaced with "First-time admin setup" copy that documents the env var, `openssl rand -base64 24` generation tip, and CI-secret alternative. Brief 2 converted the script from CJS to ESM so `npm run setup-db` actually runs on Node 22.x (the `bump-next-js` convoy's `"type": "module"` flag had silently broken it). **Operator caveat:** the seed is idempotent (`ON CONFLICT (email) DO NOTHING`); re-running setup-db on an env that already has the admin row does NOT rotate the password. Any deployed env that ran setup before this convoy still has the weak `admin123` hash — operators must rotate manually via the app, or wait for the queued `rotate-default-admin` follow-up convoy. Entry kept (not renumbered) to preserve cross-references. +- **#4 — Default admin credentials in the seed. RESOLVED** by `drop-public-setup` Brief 1 (commit `ff80753`) + Brief 2 (commit `b63b509`). `scripts/setup-neon-db.js` no longer hardcodes `admin123`; it reads `ADMIN_INITIAL_PASSWORD` from the environment and exits with code 1 before opening a DB connection if the var is unset. README's "Default Admin Account" section is replaced with "First-time admin setup" copy that documents the env var, `openssl rand -base64 24` generation tip, and CI-secret alternative. Brief 2 converted the script from CJS to ESM so `npm run setup-db` actually runs on Node 22.x (the `bump-next-js` convoy's `"type": "module"` flag had silently broken it). **Operator caveat:** the seed is idempotent (`ON CONFLICT (email) DO NOTHING`); re-running setup-db on an env that already has the admin row does NOT rotate the password. Any deployed env that ran setup before this convoy still has the weak `admin123` hash — operators rotate via the new `scripts/rotate-admin-password.js` (see `rotate-default-admin` resolution below). Entry kept (not renumbered) to preserve cross-references. + + **Rotation script — `rotate-default-admin` resolution** (2026-06-13). `scripts/rotate-admin-password.js` closes the operator caveat above with a one-shot, audit-trail-preserving rotation: + ```bash + POSTGRES_URL= \ + ADMIN_NEW_PASSWORD=$(openssl rand -base64 24) \ + node scripts/rotate-admin-password.js + ``` + Fail-loud-exits BEFORE opening a DB connection if `POSTGRES_URL` / `ADMIN_NEW_PASSWORD` are missing or the password is shorter than 12 chars. Validates the target row exists AND has `role = 'admin'` before touching it (refuses to rotate non-admin rows). Verifies the new bcrypt hash matches the supplied plaintext via `bcrypt.compare` post-update. Never echoes the password. Optional `ADMIN_EMAIL` override defaults to `admin@deckhearth.com`; pass `admin@tcgvault.com` to target a pre-`pick-a-name`-rename env. Sibling test users (alice / bob in `scripts/create-test-users.js`) are intentionally NOT rotated — they're dev fixtures. Post-`pick-a-name` (2026-05-24, squash `9abbab6`), the seeded admin email is `admin@deckhearth.com` (and alice/bob test users likewise diff --git a/scripts/rotate-admin-password.js b/scripts/rotate-admin-password.js new file mode 100644 index 0000000..e1bae5a --- /dev/null +++ b/scripts/rotate-admin-password.js @@ -0,0 +1,167 @@ +#!/usr/bin/env node + +/** + * One-shot admin-password rotation. + * + * Closes the operator caveat from the `drop-public-setup` convoy: any + * deployed env that ran `scripts/setup-neon-db.js` BEFORE that convoy + * (commit `ff80753`, 2026-05-22) still carries the historical + * `admin123` bcrypt hash in `users.password_hash` for the admin row. + * The seed is idempotent (`ON CONFLICT (email) DO NOTHING`), so + * re-running `npm run setup-db` does NOT rotate the password — it + * just no-ops on the existing row. + * + * This script is the deliberate, audit-trail-preserving alternative + * to "log into the app and change the password manually": it leaves + * an `updated_at` timestamp on the row, validates the target row is + * actually an admin before touching it, and verifies the hash matches + * post-update. + * + * Usage: + * + * POSTGRES_URL=... \ + * ADMIN_NEW_PASSWORD=$(openssl rand -base64 24) \ + * node scripts/rotate-admin-password.js + * + * # Or via .env.local with both vars set: + * node scripts/rotate-admin-password.js + * + * Required env: + * - POSTGRES_URL — Neon connection string (same shape as + * `npm run setup-db` uses). + * - ADMIN_NEW_PASSWORD — new plaintext password. The script + * fail-loud-exits BEFORE opening any DB + * connection if this is unset, empty, or + * shorter than 12 chars. + * + * Optional env: + * - ADMIN_EMAIL — defaults to `admin@deckhearth.com`. Use + * this if your env still has the legacy + * `admin@tcgvault.com` row (pre-`pick-a-name` + * convoy, 2026-05-24). Run the email-rename + * migration first if so: + * node scripts/migrations/2026-05-24-rename-admin-email.js + * + * Safety guarantees: + * - Validates the target row exists AND has `role = 'admin'` before + * touching it (refuses to rotate a non-admin row even if you point + * ADMIN_EMAIL at it). + * - Never echoes the password to stdout / stderr / shell history. + * The only stdout output is the rotation status + the row's id / + * email / role / updated_at. + * - Verifies the new hash matches the supplied plaintext via + * `bcrypt.compare` after the UPDATE, before exiting clean. If the + * compare fails (extremely unlikely), exits non-zero so the + * operator knows to investigate. + * + * NOT in scope (intentionally): + * - Rotating alice / bob test-user passwords (they live in + * `scripts/create-test-users.js` and are dev/test fixtures). + * - A "first-login forced rotation" feature in the app (heavier + * scope; would be the `force-admin-password-reset-flow` convoy). + */ + +import dotenv from 'dotenv'; +dotenv.config({ path: '.env.local' }); + +import { neon } from '@neondatabase/serverless'; +import bcrypt from 'bcryptjs'; + +const MIN_PASSWORD_LENGTH = 12; +const BCRYPT_ROUNDS = 12; + +async function main() { + const postgresUrl = process.env.POSTGRES_URL; + const newPassword = process.env.ADMIN_NEW_PASSWORD; + const targetEmail = process.env.ADMIN_EMAIL || 'admin@deckhearth.com'; + + if (!postgresUrl) { + console.error('❌ POSTGRES_URL is not set. Set it in .env.local or pass inline.'); + process.exit(1); + } + + if (!newPassword || newPassword.length === 0) { + console.error('❌ ADMIN_NEW_PASSWORD is not set. Generate one with:'); + console.error(' openssl rand -base64 24'); + console.error(' Then re-run:'); + console.error(' ADMIN_NEW_PASSWORD= node scripts/rotate-admin-password.js'); + process.exit(1); + } + + if (newPassword.length < MIN_PASSWORD_LENGTH) { + console.error(`❌ ADMIN_NEW_PASSWORD must be at least ${MIN_PASSWORD_LENGTH} characters. Got ${newPassword.length}.`); + process.exit(1); + } + + const sql = neon(postgresUrl); + + // `@neondatabase/serverless` returns rows directly (NOT wrapped in + // { rows, rowCount } like `@vercel/postgres`). Same pattern as + // scripts/migrations/2026-05-24-rename-admin-email.js. + const before = await sql` + SELECT id, email, role, updated_at + FROM users + WHERE email = ${targetEmail} + `; + + if (before.length === 0) { + console.error(`❌ No user found with email '${targetEmail}'.`); + console.error(' If you ran setup-db before the pick-a-name convoy (2026-05-24),'); + console.error(' the admin row may still be at admin@tcgvault.com. Run the'); + console.error(' email-rename migration first:'); + console.error(' node scripts/migrations/2026-05-24-rename-admin-email.js'); + console.error(' Or pass ADMIN_EMAIL=admin@tcgvault.com to target the legacy row.'); + process.exit(1); + } + + if (before.length > 1) { + console.error(`❌ Found ${before.length} rows matching '${targetEmail}'. The users.email column has a UNIQUE constraint so this should be impossible. Investigate manually.`); + process.exit(1); + } + + const row = before[0]; + if (row.role !== 'admin') { + console.error(`❌ Refusing to rotate password: row '${targetEmail}' (id=${row.id}) has role='${row.role}', not 'admin'. This script only rotates admin rows.`); + process.exit(1); + } + + console.log(`Found admin row: id=${row.id} email=${row.email} updated_at=${row.updated_at?.toISOString?.() ?? row.updated_at}`); + + const hashedPassword = await bcrypt.hash(newPassword, BCRYPT_ROUNDS); + + await sql` + UPDATE users + SET password_hash = ${hashedPassword}, + updated_at = CURRENT_TIMESTAMP + WHERE id = ${row.id} + AND email = ${targetEmail} + AND role = 'admin' + `; + + // Re-fetch + verify the new hash matches the supplied plaintext. + const after = await sql` + SELECT id, email, role, password_hash, updated_at + FROM users + WHERE id = ${row.id} + `; + + if (after.length !== 1) { + console.error(`❌ Post-update fetch returned ${after.length} rows. Investigate manually.`); + process.exit(1); + } + + const matches = await bcrypt.compare(newPassword, after[0].password_hash); + if (!matches) { + console.error('❌ Post-update bcrypt.compare returned false. The UPDATE may have failed silently. Investigate manually.'); + process.exit(1); + } + + console.log(`✅ Rotated password for admin row id=${row.id} email=${row.email}.`); + console.log(` New updated_at: ${after[0].updated_at?.toISOString?.() ?? after[0].updated_at}`); + console.log(' The new password is NOT echoed; store it in your secret manager.'); +} + +main().catch((err) => { + console.error('❌ Rotation failed:', err); + process.exit(1); +});