fix(scripts): require TEST_USERS_PASSWORD + purge weak literals from test-user helpers #27

Merged
varutasu merged 1 commit from convoy/purge-weak-creds-from-helpers into main 2026-05-26 23:53:25 -04:00
varutasu commented 2026-05-26 23:50:13 -04:00 (Migrated from github.com)

Summary

Closes the remaining scope of the queued purge-weak-creds-from-helpers follow-up after PR #25 (fix-reset-db-script, squash 3ab9bf8) closed the scripts/reset-db.js half. Mirrors the post-drop-public-setup (b63b509) + post-PR-#25 (3ab9bf8) fail-loud env-var pattern verbatim — third application of the same proven shape.

Bugs fixed

scripts/create-test-users.js

  • Hardcoded weak passwords (was lines 15 + 24): bcrypt.hash('alice123', 12) and bcrypt.hash('bob123', 12) literals removed. Both rows now use bcrypt.hash(testUsersPassword, 12) where testUsersPassword is read from process.env.TEST_USERS_PASSWORD with a fail-loud check at the top of createTestUsers() that exits 1 BEFORE opening any DB connection if the var is unset.
  • Passwords echoed to stdout (was lines 21, 30, 34-36): four console.log lines that printed the literal alice123 / bob123 / admin123 strings deleted. New summary line documents where the password comes from ((passwords from TEST_USERS_PASSWORD)) without ever printing the value.

TESTING_GUIDE.md

  • Test Accounts table still documented admin123 / alice123 / bob123 for admin + alice + bob. Rewritten to show password source per user (ADMIN_INITIAL_PASSWORD env var or TEST_USERS_PASSWORD env var, depending on the seed script). Added a paragraph documenting that both env vars must be set in .env.local before running their respective seed scripts, with a openssl rand -base64 24 generation tip and a pointer to README's "First-time admin setup" section.
  • Inline workflow snippets (sections 1 + 5): Password: alice123 / Password: bob123 replaced with Password: <value of TEST_USERS_PASSWORD from .env.local>.

Fix shape

Single env var (TEST_USERS_PASSWORD) covers both alice + bob — deliberate simplification vs. per-user vars (risk R2 in the convoy file argues this; these are fixture users for the collaboration demo, not independent identities, and per-user sprawl would double the env-var contract for zero security benefit). Fail-loud check uses the same wording template as setup-neon-db.js lines 20-26 and reset-db.js lines 29-35 (names the var, points at .env.local, suggests openssl rand -base64 24, references README). ON CONFLICT (email) DO NOTHING preserved (already in original) — defensive against double-run. No CJS→ESM conversion neededcreate-test-users.js was already top-level ESM, unlike the previous two convoys.

Verification (static-only — script touches the DB, 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 ✓
  • Grep scripts/create-test-users.js for require( → 0 hits ✓ (was already ESM)
  • Grep scripts/ TESTING_GUIDE.md for TEST_USERS_PASSWORD → 10 hits (5 in script + 5 in doc) ✓

Test plan

  • CI: Lint job green (baseline 128 problems preserved)
  • CI: Vitest job green (21/21)
  • CI: Playwright smoke green (script-only PR; should be insensitive — 5th convoy in a row to test this)
  • CI: forbidden-endpoints + forbidden-cors-headers regression-locks green
  • CI: Vercel preview deploy green
  • CI: Screenshot diff NOT triggered (script + doc only — paths filter excludes both)
  • Optional post-merge: operator sets TEST_USERS_PASSWORD in .env.local, runs node scripts/create-test-users.js against a non-prod Neon branch, verifies alice + bob seed; then unsets the var and re-runs, verifies fail-loud exit 1 with the helpful error message.

Operator action required

  • Pre-merge: none.
  • Post-merge: anyone running node scripts/create-test-users.js 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 old hashes); any environment seeded before this PR still has the weak alice123 / bob123 hashes and must rotate manually via the app, or drop those rows and re-seed.

Follow-ups

  • The purge-weak-creds-from-helpers queued entry in .convoys/ship-readiness.md can be marked RESOLVED post-merge. NOT updating ship-readiness in this PR — flagged for the role-doc-writer post-convoy pass.
  • Surfaced out-of-scope, newly queued: purge-quick-login-from-loginpage (P2 hygiene). pages/login.js lines 172 + 184 still invoke handleQuickLogin('alice@deckhearth.com', 'alice123') and handleQuickLogin('bob@deckhearth.com', 'bob123') — client-side dev-convenience buttons that ship the legacy literals to production HTML. NOT in scope here (convoy spec is "scripts + docs only; do NOT touch pages/**"). Either delete the section or gate it behind NODE_ENV === 'development' with a non-shipping credential source; either path is architect-worth.
  • Live verification deferred to operator per convoy spec — the script is destructive and we don't exercise it in CI.

Made with Cursor

## Summary Closes the remaining scope of the queued `purge-weak-creds-from-helpers` follow-up after PR #25 (`fix-reset-db-script`, squash `3ab9bf8`) closed the `scripts/reset-db.js` half. Mirrors the post-`drop-public-setup` (`b63b509`) + post-PR-#25 (`3ab9bf8`) fail-loud env-var pattern verbatim — third application of the same proven shape. ## Bugs fixed ### `scripts/create-test-users.js` - **Hardcoded weak passwords** (was lines 15 + 24): `bcrypt.hash('alice123', 12)` and `bcrypt.hash('bob123', 12)` literals removed. Both rows now use `bcrypt.hash(testUsersPassword, 12)` where `testUsersPassword` is read from `process.env.TEST_USERS_PASSWORD` with a fail-loud check at the top of `createTestUsers()` that exits 1 BEFORE opening any DB connection if the var is unset. - **Passwords echoed to stdout** (was lines 21, 30, 34-36): four `console.log` lines that printed the literal `alice123` / `bob123` / `admin123` strings deleted. New summary line documents *where* the password comes from (`(passwords from TEST_USERS_PASSWORD)`) without ever printing the value. ### `TESTING_GUIDE.md` - **Test Accounts table** still documented `admin123` / `alice123` / `bob123` for admin + alice + bob. Rewritten to show password *source* per user (`ADMIN_INITIAL_PASSWORD` env var or `TEST_USERS_PASSWORD` env var, depending on the seed script). Added a paragraph documenting that both env vars must be set in `.env.local` before running their respective seed scripts, with a `openssl rand -base64 24` generation tip and a pointer to README's "First-time admin setup" section. - **Inline workflow snippets** (sections 1 + 5): `Password: alice123` / `Password: bob123` replaced with `Password: <value of TEST_USERS_PASSWORD from .env.local>`. ## Fix shape Single env var (`TEST_USERS_PASSWORD`) covers both alice + bob — deliberate simplification vs. per-user vars (risk R2 in the convoy file argues this; these are fixture users for the collaboration demo, not independent identities, and per-user sprawl would double the env-var contract for zero security benefit). Fail-loud check uses the same wording template as `setup-neon-db.js` lines 20-26 and `reset-db.js` lines 29-35 (names the var, points at `.env.local`, suggests `openssl rand -base64 24`, references README). `ON CONFLICT (email) DO NOTHING` preserved (already in original) — defensive against double-run. **No CJS→ESM conversion needed** — `create-test-users.js` was already top-level ESM, unlike the previous two convoys. ## Verification (static-only — script touches the DB, 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 ✓ - Grep `scripts/create-test-users.js` for `require(` → 0 hits ✓ (was already ESM) - Grep `scripts/ TESTING_GUIDE.md` for `TEST_USERS_PASSWORD` → 10 hits (5 in script + 5 in doc) ✓ ## Test plan - [ ] CI: Lint job green (baseline 128 problems preserved) - [ ] CI: Vitest job green (21/21) - [ ] CI: Playwright smoke green (script-only PR; should be insensitive — 5th convoy in a row to test this) - [ ] CI: `forbidden-endpoints` + `forbidden-cors-headers` regression-locks green - [ ] CI: Vercel preview deploy green - [ ] CI: `Screenshot diff` NOT triggered (script + doc only — paths filter excludes both) - [ ] Optional post-merge: operator sets `TEST_USERS_PASSWORD` in `.env.local`, runs `node scripts/create-test-users.js` against a non-prod Neon branch, verifies alice + bob seed; then unsets the var and re-runs, verifies fail-loud exit 1 with the helpful error message. ## Operator action required - **Pre-merge:** none. - **Post-merge:** anyone running `node scripts/create-test-users.js` 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 old hashes); any environment seeded before this PR still has the weak `alice123` / `bob123` hashes and must rotate manually via the app, or drop those rows and re-seed. ## Follow-ups - The `purge-weak-creds-from-helpers` queued entry in `.convoys/ship-readiness.md` can be marked **RESOLVED** post-merge. **NOT updating ship-readiness in this PR** — flagged for the role-doc-writer post-convoy pass. - **Surfaced out-of-scope, newly queued:** `purge-quick-login-from-loginpage` (P2 hygiene). `pages/login.js` lines 172 + 184 still invoke `handleQuickLogin('alice@deckhearth.com', 'alice123')` and `handleQuickLogin('bob@deckhearth.com', 'bob123')` — client-side dev-convenience buttons that ship the legacy literals to production HTML. NOT in scope here (convoy spec is "scripts + docs only; do NOT touch pages/**"). Either delete the section or gate it behind `NODE_ENV === 'development'` with a non-shipping credential source; either path is architect-worth. - **Live verification deferred to operator** per convoy spec — the script is destructive and we don't exercise it in CI. Made with [Cursor](https://cursor.com)
vercel[bot] commented 2026-05-26 23:50:18 -04:00 (Migrated from github.com)

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
tcg-vault Ready Ready Preview, Comment May 27, 2026 3:50am

Request Review

[vc]: #KmR7RUkek8r8gx/R65UJ7hV4dVrusYRAtxDrD1sWqkg=:eyJpc01vbm9yZXBvIjp0cnVlLCJ0eXBlIjoiZ2l0aHViIiwicHJvamVjdHMiOlt7Im5hbWUiOiJ0Y2ctdmF1bHQiLCJwcm9qZWN0SWQiOiJwcmpfRjZXOEVvRkd3Y0g3aWVGcnRvRlNlOXdVVkFhNSIsImxpdmVGZWVkYmFjayI6eyJyZXNvbHZlZCI6MCwidW5yZXNvbHZlZCI6MCwidG90YWwiOjAsImxpbmsiOiJ0Y2ctdmF1bHQtZ2l0LWNvbnZveS1wdXJnZS13LTMzMWYxNy1yYW5kYWxsLXN0aWxsd2VsbHMtcHJvamVjdHMudmVyY2VsLmFwcCJ9LCJpbnNwZWN0b3JVcmwiOiJodHRwczovL3ZlcmNlbC5jb20vcmFuZGFsbC1zdGlsbHdlbGxzLXByb2plY3RzL3RjZy12YXVsdC8yQ0ZqZ1ZONEVOS1hYdmpCcU1NMjV4NGZiQm5FIiwicHJldmlld1VybCI6InRjZy12YXVsdC1naXQtY29udm95LXB1cmdlLXctMzMxZjE3LXJhbmRhbGwtc3RpbGx3ZWxscy1wcm9qZWN0cy52ZXJjZWwuYXBwIiwibmV4dENvbW1pdFN0YXR1cyI6IkRFUExPWUVEIn1dLCJyZXF1ZXN0UmV2aWV3VXJsIjoiaHR0cHM6Ly92ZXJjZWwuY29tL3ZlcmNlbC1hZ2VudC9yZXF1ZXN0LXJldmlldz9vd25lcj12YXJ1dGFzdSZyZXBvPXRjZy12YXVsdCZwcj0yNyJ9 The latest updates on your projects. Learn more about [Vercel for GitHub](https://vercel.link/github-learn-more). | Project | Deployment | Actions | Updated (UTC) | | :--- | :----- | :------ | :------ | | [tcg-vault](https://vercel.com/randall-stillwells-projects/tcg-vault) | ![Ready](https://vercel.com/static/status/ready.svg) [Ready](https://vercel.com/randall-stillwells-projects/tcg-vault/2CFjgVN4ENKXXvjBqMM25x4fbBnE) | [Preview](https://tcg-vault-git-convoy-purge-w-331f17-randall-stillwells-projects.vercel.app), [Comment](https://vercel.live/open-feedback/tcg-vault-git-convoy-purge-w-331f17-randall-stillwells-projects.vercel.app?via=pr-comment-feedback-link) | May 27, 2026 3:50am | <a href="https://vercel.com/vercel-agent/request-review?owner=varutasu&repo=tcg-vault&pr=27" rel="noreferrer"><picture><source media="(prefers-color-scheme: dark)" srcset="https://agents-vade-review.vercel.sh/request-review-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://agents-vade-review.vercel.sh/request-review-light.svg"><img src="https://agents-vade-review.vercel.sh/request-review-light.svg" alt="Request Review"></picture></a>
github-actions[bot] commented 2026-05-26 23:50:23 -04:00 (Migrated from github.com)

Pipeline Health

Build + CI gates

Gate Status
Vercel build (Preview) pass
CI: Lint pass
CI: Schema map fresh skipped
Preview smoke pass
Visual diff ⏭ skipped or pending

Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build).

Role reports

Role Status
Reviewer report pending
A11y audit pending
Design system audit pending

See individual comments above for details. This rollup updates automatically.

<!-- pipeline-rollup --> ## Pipeline Health ### Build + CI gates | Gate | Status | | --- | --- | | Vercel build (Preview) | ✅ pass | | CI: Lint | ✅ pass | | CI: Schema map fresh | ❌ skipped | | Preview smoke | ✅ pass | | Visual diff | ⏭ skipped or pending | _Build runs on Vercel; this CI runs lint and schema-map drift only (no duplicate build)._ ### Role reports | Role | Status | | --- | --- | | Reviewer report | ⏳ pending | | A11y audit | ⏳ pending | | Design system audit | ⏳ pending | See individual comments above for details. This rollup updates automatically.
Sign in to join this conversation.
No description provided.