deckhearth/.convoys/purge-weak-creds-from-helpers.md

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: <value of TEST_USERS_PASSWORD from .env.local>.

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=<value> 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

Single squash commit 5f2b234 (PR #27, merged 2026-05-27T03:53:25Z UTC / local 2026-05-26). Parent-owned end-to-end per the convoy spec — no architect, no implementer subagent dispatched. Mirror-the-pattern fix exactly as planned; no mid-execution surprises that would have forced an architect bounce. The umbrella purge-weak-creds-from-helpers is now fully closed — both remaining files swept, weak-cred surface of the helper-script + manual-QA-doc lane is at zero.

Diff: 3 files, +249 / -22. scripts/create-test-users.js (the alice/bob fixture script) + TESTING_GUIDE.md (the manual-QA doc that pairs with it) + .convoys/purge-weak-creds-from-helpers.md (the planning document, committed atomically with the fix).

The two files atomically resolved:

1. scripts/create-test-users.js — alice + bob fixtures previously hardcoded bcrypt.hash('alice123', 12) + bcrypt.hash('bob123', 12) and echoed both literals to stdout (✅ Created Alice (alice@deckhearth.com / alice123) + matching bob line + a final summary block listing both passwords). All four echo lines are gone; the new shape reads a single TEST_USERS_PASSWORD env var at the top of createTestUsers() with the same fail-loud + actionable error message template as setup-neon-db.js's post-DPS shape (names the var, points at .env.local, suggests openssl rand -base64 24, references README's "First-time admin setup" section, process.exit(1) BEFORE opening any DB connection). Both alice and bob get the same hashed value per Risk R2 (these are collaboration-flow demo fixtures, not independent identities — per-user env vars would be sprawl). ON CONFLICT (email) DO NOTHING preserved (already in the original file). The two per-user creation lines are rewritten to not echo the password: ✅ Created Alice (alice@deckhearth.com)
   • ✅ Created Bob (bob@deckhearth.com); the final summary block includes (passwords from TEST_USERS_PASSWORD) to document where the password came from without printing it.
2. TESTING_GUIDE.md — Test Accounts table rewritten to (a) remove the literal passwords from the table, (b) document the env-var source for each user (admin ← ADMIN_INITIAL_PASSWORD; alice / bob ← TEST_USERS_PASSWORD), 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: <value of TEST_USERS_PASSWORD from .env.local>.

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). This was the first of the three weak-creds-shape convoys to skip the CJS→ESM conversion half of the pattern.

Verification (all gates green at merge):

• node --check scripts/create-test-users.js → exit 0
• npm run lint → 125 problems (post-PR-#31 baseline preserved; no regression introduced)
• npm run test:run → 21/21 pass
• Grep scripts/ TESTING_GUIDE.md for admin123|password123|test123|alice123|bob123 → 0 hits on literal passwords (the umbrella weak-cred surface is at zero across the helper-script + manual-QA-doc lane)
• Grep scripts/create-test-users.js for require( → 0 hits (ESM-already; trivially satisfied)
• Grep scripts/ TESTING_GUIDE.md for TEST_USERS_PASSWORD → expected ~10 hits (docstring + const + error message body in script; table + two inline workflow snippets + explanatory paragraph in doc)
• CI on PR #27: Lint ✓ | Vitest 21/21 ✓ | Playwright smoke 3/3 ✓ | forbidden-endpoints ✓ | forbidden-cors-headers ✓ | Vercel preview deploy ✓ | Aggregate gate ✓
• Screenshot diff: not triggered (script + docs only — paths: filter excludes scripts/** and TESTING_GUIDE.md; the post-PR-#26 !pages/api/** exclusion is not even relevant here)

Live verification deferred per convoy spec. Optional post-merge operator 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.

Operator caveat (going forward). 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.

Surfaced follow-up (newly queued in .convoys/ship-readiness.md): purge-quick-login-from-loginpage (P2 hygiene / security). Out-of-scope sibling bug: pages/login.js lines ~172 + ~184 still hardcode alice123 / bob123 in client-side "Quick Login" button handlers that ship to production HTML. The convoy spec scope was "scripts + docs only; do NOT touch pages/**", so this was deliberately left for a follow-up. Architect-worth (the right shape — delete entirely, gate behind process.env.NODE_ENV === 'development', or proxy through a dev-only endpoint — is a design decision).

Spec deviation: none.

Cross-validation finding. The Playwright smoke 3/3 PASS on a script + docs-only PR is the seventh consecutive convoy where the same 3-test smoke spec defends the auth surface through a sweeping change (PR #15 → #19 → #20 → #21 → #25 → #32 → this PR). The lineage continues.