From af505d533c579c0c321665b064f12b6912cd3b06 Mon Sep 17 00:00:00 2001 From: Randall Stillwell Date: Tue, 26 May 2026 22:49:38 -0500 Subject: [PATCH] fix(scripts): require TEST_USERS_PASSWORD + purge weak literals from test-user helpers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `scripts/create-test-users.js` hardcoded `bcrypt.hash('alice123', 12)` + `bcrypt.hash('bob123', 12)` and echoed those literals back to stdout both per-user and in a final summary block. `TESTING_GUIDE.md`'s Test Accounts table documented the same `admin123` / `alice123` / `bob123` trio. These were the last two weak-credential surfaces left in the helper-script + manual-QA-doc tree after `drop-public-setup` (commits `ff80753` + `b63b509`) and `fix-reset-db-script` (squash `3ab9bf8`, PR #25) closed the `setup-neon-db.js` and `reset-db.js` halves of the umbrella `purge-weak-creds-from-helpers` queued follow-up. The fix mirrors the post-`drop-public-setup` `setup-neon-db.js` pattern and the post-PR-#25 `reset-db.js` pattern verbatim, with one deliberate simplification: a single `TEST_USERS_PASSWORD` env var covers both alice + bob rather than per-user env vars (risk R2 in the convoy file argues this — these are fixture users for the collaboration demo flow, not independent identities, and per-user sprawl would double the env-var contract for zero security benefit). `createTestUsers()` now reads `process.env.TEST_USERS_PASSWORD` at the top of the function body and exits with code 1 BEFORE opening any DB connection if the var is unset or whitespace-only, with the same helpful-error wording template the other two scripts use (names the var, points at `.env.local`, suggests `openssl rand -base64 24`, references README's "First-time admin setup" section). All four password-echo `console.log` lines are deleted; the new summary documents *where* the password comes from without ever printing it. `TESTING_GUIDE.md`'s Test Accounts table is rewritten to show password source per user instead of the literal value; the two inline `Password: alice123` / `Password: bob123` workflow snippets are replaced with placeholder text. Unlike the previous two convoys, no CJS→ESM conversion was needed — `create-test-users.js` was already top-level ESM. Verification (all static — script is destructive and not live-tested): `node --check scripts/create-test-users.js` exit 0; `npm run lint` 128 problems (baseline preserved, no regression); `npm run test:run` 21/21 pass; grep `scripts/ TESTING_GUIDE.md` for `admin123|password123|test123|alice123|bob123` → 0 hits; `TEST_USERS_PASSWORD` referenced 10 times total (5 script + 5 doc). Operator caveat: anyone running `node scripts/create-test-users.js` post-merge must add `TEST_USERS_PASSWORD=` to their `.env.local` first; existing alice + bob rows in already-seeded environments are NOT rotated by re-running this script (`ON CONFLICT (email) DO NOTHING` preserves the old hashes). Same caveat that applies to the `drop-public-setup` admin row. Co-authored-by: Cursor --- .convoys/purge-weak-creds-from-helpers.md | 187 ++++++++++++++++++++++ TESTING_GUIDE.md | 26 ++- scripts/create-test-users.js | 58 +++++-- 3 files changed, 249 insertions(+), 22 deletions(-) create mode 100644 .convoys/purge-weak-creds-from-helpers.md diff --git a/.convoys/purge-weak-creds-from-helpers.md b/.convoys/purge-weak-creds-from-helpers.md new file mode 100644 index 0000000..4c84ba9 --- /dev/null +++ b/.convoys/purge-weak-creds-from-helpers.md @@ -0,0 +1,187 @@ +# purge-weak-creds-from-helpers (P2 hygiene — final scope close) + +**Status:** IN-FLIGHT 2026-05-26 +**Priority:** P2 hygiene (not a security blocker; the test-fixture +script is dev-only and the documented passwords were never reachable +from a production code path — but the bug pattern is the same as the +P0-grade weak-creds shape that `drop-public-setup` removed from +`setup-neon-db.js`, so closing it brings the helper-script surface to +zero weak literals) +**Convoy owner:** parent (no architect — proven-pattern mirror; +single-script + single-doc fix following two already-shipped +applications of the same pattern) +**Opened:** 2026-05-26 +**Classification:** hygiene + +## Background — the multi-convoy history that led here + +The original `purge-weak-creds-from-helpers` convoy was queued in +`.convoys/ship-readiness.md` as the umbrella for sweeping every +helper-script + manual-QA-doc reference to the legacy `admin123` / +`alice123` / `bob123` literals and the legacy `@tcgvault.com` email +domain. Its scope has been progressively whittled down by three +already-shipped convoys: + +1. **`drop-public-setup`** (squash `ff80753` Brief 1 + `b63b509` + Brief 2): replaced the hardcoded `admin123` in + `scripts/setup-neon-db.js` with the fail-loud + `ADMIN_INITIAL_PASSWORD` env-var gate; converted the script from + CJS to ESM so `npm run setup-db` actually runs on Node 22.x. Set + the precedent for the env-var + fail-loud + no-echo pattern that + the next two convoys (and this one) mirror verbatim. +2. **`pick-a-name` Brief 2** (squash `9abbab6`, 2026-05-24): swept + every `@tcgvault.com` literal in scripts + docs to `@deckhearth.com` + together with the one-shot migration script. **Email half done.** +3. **`fix-reset-db-script`** (squash `3ab9bf8`, PR #25, 2026-05-26): + second application of the post-`drop-public-setup` pattern, this + time to `scripts/reset-db.js`. Removed the second `admin123` literal + from the codebase, removed the only remaining `Admin Password:` + echo, converted the third CJS-in-ESM script. + +After those three convoys, the remaining weak-credential surface is +exactly two files — the alice/bob test-user fixture script and the +manual-QA doc that pairs with it. Both are addressed here. + +## Remaining scope (this convoy) + +1. **`scripts/create-test-users.js`** — alice + bob fixtures still + hardcode `bcrypt.hash('alice123', 12)` + `bcrypt.hash('bob123', 12)` + and echo the literal passwords to stdout (`console.log('✅ Created + Alice (alice@deckhearth.com / alice123)')`). +2. **`TESTING_GUIDE.md`** — Test Accounts table still documents the + literal passwords for admin + alice + bob. + +## The fix shape — single env var, no echo, ESM-already + +The fix is a verbatim mirror of the post-`drop-public-setup` +`scripts/setup-neon-db.js` pattern and the post-`fix-reset-db-script` +`scripts/reset-db.js` pattern, with one deliberate simplification: + +- **Single env var: `TEST_USERS_PASSWORD`.** Both alice and bob get + the same hashed value. Per-user env vars (`ALICE_PASSWORD`, + `BOB_PASSWORD`) would be unnecessary sprawl for what is a test + fixture surface — these aren't independent identities, they're a + collaborator-flow demo pair. Risk R2 below argues this explicitly. +- **Fail-loud at the top of `createTestUsers()`.** Reads + `process.env.TEST_USERS_PASSWORD`; if unset or whitespace-only, + prints an actionable error (names the var, points at `.env.local`, + suggests `openssl rand -base64 24`, references README's "First-time + admin setup" section) and `process.exit(1)` BEFORE opening any DB + connection. Same wording template as `setup-neon-db.js` lines 20-26 + and `reset-db.js` lines 29-35. +- **No password echo to stdout.** The previous file logged the + literal `alice123` / `bob123` strings in both the per-user creation + line and the final summary block. All four echo lines are deleted; + the new summary line documents *where* the password comes from + (`(passwords from TEST_USERS_PASSWORD)`) without ever printing the + value. +- **ESM already.** Unlike `setup-neon-db.js` and `reset-db.js` at the + start of their respective convoys, `create-test-users.js` was + already top-level ESM (it imports `{ config } from 'dotenv'`, + `{ sql } from '@vercel/postgres'`, `bcrypt from 'bcryptjs'` at the + top of the file). **No CJS→ESM conversion needed.** This convoy is + the first of the three to skip that half of the pattern. +- **`ON CONFLICT (email) DO NOTHING`** is preserved (already in the + original file at lines 19, 28) — defensive against double-run. + +`TESTING_GUIDE.md`'s Test Accounts table is rewritten to (a) remove +the literal passwords from the table, (b) document the env-var source +for each user, and (c) point at README's "First-time admin setup" +section for the `openssl rand -base64 24` generation tip. The two +inline `Password: alice123` / `Password: bob123` snippets later in the +workflow are replaced with `Password: `. + +## Verification plan (static-grep only — script is destructive) + +This convoy does NOT live-test `create-test-users.js`. The script +opens a DB connection and inserts rows; running it against a Neon +branch in CI or in the boot-the-brief loop would be a side-effect +cost we don't need to incur. The verification surface is entirely +static: + +- `node --check scripts/create-test-users.js` → exit 0 +- `npm run lint` → 128 problems (baseline preserved; no regression) +- `npm run test:run` → 21/21 pass +- Grep `scripts/ TESTING_GUIDE.md` for `admin123|password123|test123| + alice123|bob123` → 0 hits on literal passwords +- Grep `scripts/create-test-users.js` for `require(` → 0 hits + (mirror-the-pattern preserves ESM-only; trivially satisfied here + because the file was already ESM) +- Grep `scripts/ TESTING_GUIDE.md` for `TEST_USERS_PASSWORD` → + expect 10 hits (5 in script: docstring + const + error message + body; 5 in TESTING_GUIDE.md: table + two inline workflow snippets + + explanatory paragraph) + +**Live verification deferred to operator.** Optional post-merge +action: set `TEST_USERS_PASSWORD` in `.env.local`, run +`node scripts/create-test-users.js` against a non-prod Neon branch, +verify alice + bob rows insert; then unset the env var and re-run, +verify the script exits 1 with the helpful error message before +opening the DB connection. + +## Risks + +- **R1 — A CI step or doc dep on the literal passwords.** If + `.github/workflows/**` or any other doc (`docs/**`, `TESTING_GUIDE.md` + sibling files, the agent-context-pipeline docs) references + `alice123` / `bob123` / `admin123` as part of an automated test + flow, removing the literal would break it. **Mitigation:** the grep + hunt covered `scripts/` + `TESTING_GUIDE.md`. The broader hits in + `.convoys/**` and `AGENTS.md` are historical convoy narrative and + must NOT be edited (rewriting history). The only live reference + outside this convoy's scope is `pages/login.js` — see "Surfaced + out-of-scope follow-up" below. +- **R2 — Env-var sprawl.** Using a single `TEST_USERS_PASSWORD` for + both alice and bob is intentional. These are test-fixture users + for the collaboration demo flow in TESTING_GUIDE.md; they aren't + modeled as independent identities anywhere in the auth surface, and + giving them per-user passwords would (a) double the env-var + contract for zero security benefit (anyone running this script + already has full DB access) and (b) drift from the + ADMIN_INITIAL_PASSWORD shape that the operator is already trained + on. If future test-user additions need distinct passwords for + realistic concurrency testing, that's a separate concern and a + separate convoy. + +## Surfaced out-of-scope follow-up + +- **`pages/login.js` "Quick Login" buttons still hardcode the legacy + literals.** Lines 172 + 184 invoke + `handleQuickLogin('alice@deckhearth.com', 'alice123')` and + `handleQuickLogin('bob@deckhearth.com', 'bob123')`. These are + client-side dev convenience buttons that ship to production HTML + and reveal the legacy passwords directly to anyone viewing the + login page source. **NOT in scope for this convoy** (the convoy + spec is "scripts + docs only; do NOT touch `pages/**`"). Queue a + follow-up convoy: `purge-quick-login-from-loginpage` (P2 hygiene) + to either (a) delete the Quick Login section entirely or + (b) gate it behind `process.env.NODE_ENV === 'development'`. The + latter still requires a credential source that doesn't ship to + prod HTML — likely a `.env.local`-only `NEXT_PUBLIC_DEV_*` + convention or a dev-only proxy endpoint. Architect-worth. + +## Operator action required + +- **Pre-merge:** none. No schema change. No new infra. +- **Post-merge:** anyone running `node scripts/create-test-users.js` + (or `npm run create-test-users` if such a script exists) must add + `TEST_USERS_PASSWORD=` to their `.env.local` first. + Existing alice + bob rows in already-seeded environments are + **not** rotated by re-running this script — `ON CONFLICT (email) + DO NOTHING` preserves the old hashes. Any environment that ran + `create-test-users.js` before this convoy still has the weak + `alice123` / `bob123` hashes in its DB; operators must rotate + manually via the app (or drop those rows and re-seed). Same + caveat that applies to the `drop-public-setup` admin row — + `setup-neon-db.js`'s and `create-test-users.js`'s idempotency + means they do NOT rotate; they only seed. + +## Owns + +Parent (single-script + single-doc proven-pattern fix; no architect +or implementer subagent required). + +## As-shipped + +(stub — fill in post-merge) diff --git a/TESTING_GUIDE.md b/TESTING_GUIDE.md index 502f45e..816879c 100644 --- a/TESTING_GUIDE.md +++ b/TESTING_GUIDE.md @@ -2,11 +2,21 @@ ## 👥 Test Accounts -| User | Email | Password | Role | -|------|-------|----------|------| -| Admin | `admin@deckhearth.com` | `admin123` | Admin | -| Alice | `alice@deckhearth.com` | `alice123` | User | -| Bob | `bob@deckhearth.com` | `bob123` | User | +| User | Email | Role | Password source | +|------|-------|------|-----------------| +| Admin | `admin@deckhearth.com` | Admin | `ADMIN_INITIAL_PASSWORD` env var (seeded by `npm run setup-db`) | +| Alice | `alice@deckhearth.com` | User | `TEST_USERS_PASSWORD` env var (seeded by `node scripts/create-test-users.js`) | +| Bob | `bob@deckhearth.com` | User | `TEST_USERS_PASSWORD` env var (seeded by `node scripts/create-test-users.js`) | + +Both env vars must be set in `.env.local` before running the +corresponding seed script — each script fails loud (exit 1, no DB +connection opened) if its env var is unset. Generate strong values +with `openssl rand -base64 24`; see README.md → "First-time admin +setup" for the canonical env-var pattern. + +Alice + Bob share a single `TEST_USERS_PASSWORD` value because this is +a test-fixture surface; that's intentional and documented in the +`purge-weak-creds-from-helpers` convoy. ## 🃏 Sample Cards Available @@ -21,8 +31,8 @@ ### 1. **Login as Alice** ``` -Email: alice@deckhearth.com -Password: alice123 +Email: alice@deckhearth.com +Password: ``` ### 2. **Create a Collection** @@ -51,7 +61,7 @@ Password: alice123 ### 5. **Switch to Bob's Account** - Logout and login as Bob - Email: `bob@deckhearth.com` -- Password: `bob123` +- Password: `` ### 6. **Accept Invitation (Simulated)** Since we're testing locally, simulate email acceptance: diff --git a/scripts/create-test-users.js b/scripts/create-test-users.js index e0c45ed..abb26cb 100755 --- a/scripts/create-test-users.js +++ b/scripts/create-test-users.js @@ -1,39 +1,69 @@ #!/usr/bin/env node +/** + * Create Test Users Script + * + * Seeds the alice + bob test-user fixtures used by the manual QA flows + * in TESTING_GUIDE.md. Both users share a single password supplied via + * the TEST_USERS_PASSWORD environment variable — this is a test-fixture + * helper, not a prod auth surface, so per-user env vars would be + * unnecessary sprawl. + * + * Required env (in .env.local): + * POSTGRES_URL — Neon connection string + * TEST_USERS_PASSWORD — strong password applied to every test user + * (generate with `openssl rand -base64 24`) + * + * Mirrors the post-`drop-public-setup` shape of `setup-neon-db.js` + * (commit b63b509) and the post-`fix-reset-db-script` shape of + * `reset-db.js` (commit 3ab9bf8) — same ESM imports, same fail-loud + * env-var check, same no-password-echo convention. Convoy: + * `purge-weak-creds-from-helpers` (2026-05-26). + */ + import { config } from 'dotenv'; import { sql } from '@vercel/postgres'; import bcrypt from 'bcryptjs'; -// Load environment variables config({ path: '.env.local' }); async function createTestUsers() { + const testUsersPassword = process.env.TEST_USERS_PASSWORD; + if (!testUsersPassword || !testUsersPassword.trim()) { + console.error( + '❌ TEST_USERS_PASSWORD environment variable is not set.\n' + + '\n' + + ' Set it in .env.local before running `node scripts/create-test-users.js`.\n' + + ' Generate a strong password with: openssl rand -base64 24\n' + + ' See README.md → "First-time admin setup" for the env-var pattern.\n' + ); + process.exit(1); + } + try { - console.log('�� Creating test users...\n'); + console.log('👥 Creating test users...\n'); + + const hashedPassword = await bcrypt.hash(testUsersPassword, 12); - // Create Alice (collaborator) - const alicePassword = await bcrypt.hash('alice123', 12); await sql` INSERT INTO users (email, password, role) - VALUES ('alice@deckhearth.com', ${alicePassword}, 'user') + VALUES ('alice@deckhearth.com', ${hashedPassword}, 'user') ON CONFLICT (email) DO NOTHING `; - console.log('✅ Created Alice (alice@deckhearth.com / alice123)'); + console.log('✅ Created Alice (alice@deckhearth.com)'); - // Create Bob (collaborator) - const bobPassword = await bcrypt.hash('bob123', 12); await sql` INSERT INTO users (email, password, role) - VALUES ('bob@deckhearth.com', ${bobPassword}, 'user') + VALUES ('bob@deckhearth.com', ${hashedPassword}, 'user') ON CONFLICT (email) DO NOTHING `; - console.log('✅ Created Bob (bob@deckhearth.com / bob123)'); + console.log('✅ Created Bob (bob@deckhearth.com)'); console.log('\n🎉 Test users created successfully!'); - console.log('\n👥 Available Test Accounts:'); - console.log(' 1. admin@deckhearth.com / admin123 (Admin)'); - console.log(' 2. alice@deckhearth.com / alice123 (User)'); - console.log(' 3. bob@deckhearth.com / bob123 (User)'); + console.log('\n👥 Available Test Accounts (passwords from TEST_USERS_PASSWORD):'); + console.log(' 1. admin@deckhearth.com (Admin — seeded by setup-neon-db.js)'); + console.log(' 2. alice@deckhearth.com (User)'); + console.log(' 3. bob@deckhearth.com (User)'); } catch (error) { console.error('❌ Failed to create test users:', error.message); -- 2.45.2