180 lines
11 KiB
Markdown
180 lines
11 KiB
Markdown
|
|
---
|
||
|
|
convoy: drop-public-setup
|
||
|
|
brief_number: 2
|
||
|
|
depends_on:
|
||
|
|
- brief: 1
|
||
|
|
files:
|
||
|
|
- scripts/setup-neon-db.js
|
||
|
|
---
|
||
|
|
|
||
|
|
# Brief 2: Convert `scripts/setup-neon-db.js` from CJS to ESM
|
||
|
|
|
||
|
|
## Goal (1 sentence)
|
||
|
|
|
||
|
|
Convert `scripts/setup-neon-db.js` from CommonJS (`require()`) to native ES modules (`import`) so it runs on Node 22.x where `package.json` has `"type": "module"`; no functional changes — pure module-system conversion.
|
||
|
|
|
||
|
|
## Why this brief exists (context)
|
||
|
|
|
||
|
|
`bump-next-js` added `"type": "module"` to `package.json` so ESLint v9's flat config (`eslint.config.mjs`) could be picked up under the default loader semantics. As a side effect, every untyped `.js` file in the repo is now treated as ESM by Node. `scripts/setup-neon-db.js` still uses `require()` and immediately throws on Node 22.x:
|
||
|
|
|
||
|
|
```
|
||
|
|
ReferenceError: require is not defined in ES module scope
|
||
|
|
This file is being treated as an ES module because it has a '.js' file extension
|
||
|
|
and '/.../package.json' contains "type": "module".
|
||
|
|
```
|
||
|
|
|
||
|
|
Brief 1 added an `ADMIN_INITIAL_PASSWORD` env-var gate to this script, but the gate is theatrical until the script actually executes. This brief makes the script executable so brief 1's hardening takes effect.
|
||
|
|
|
||
|
|
The CJS→ESM conversion was originally flagged in `.convoys/drop-public-setup.md` § "Anything flagged but not acted on" #1 as a separate follow-up. After mid-convoy discovery (the implementer for brief 1 confirmed the script is non-functional today on Node 22.14.0), the scope was expanded to land both fixes together. See `.convoys/drop-public-setup.md` § Decisions D for the ratification.
|
||
|
|
|
||
|
|
## Files in scope (do not edit anything else)
|
||
|
|
|
||
|
|
- `scripts/setup-neon-db.js` — modified (module-system conversion only)
|
||
|
|
|
||
|
|
**Do NOT** edit `README.md`, `package.json`, `package-lock.json`, any other `scripts/*.js`, or any file under `lib/`, `pages/`, `components/`, `.github/`, or `.cursor/`. Brief 1's README changes already shipped on the convoy branch; do not touch them.
|
||
|
|
|
||
|
|
## Conventions to follow
|
||
|
|
|
||
|
|
- **No functional changes.** This brief is a 1:1 module-system conversion. Same logic, same DDL, same `process.exit(1)`, same env-var gate, same `console.log` lines. The diff should be 4-6 lines of `require` → `import` plus the `dotenv` invocation change.
|
||
|
|
- **No new dependencies.** `bcryptjs`, `dotenv`, and `@neondatabase/serverless` are already installed (verified by `npm run setup-db` failing on `require` rather than on missing modules).
|
||
|
|
- **Match brief 1's verbatim env-var-check block.** Do not touch the `if (!adminPassword || !adminPassword.trim())` block from brief 1 — only the `require()` calls around it change.
|
||
|
|
- **No top-level `await`.** ESM supports it, but the existing `setupNeonDatabase()` bottom-of-file invocation is intentionally fire-and-forget (the function does its own `try/catch` + `process.exit(1)`). Keep that pattern; do not introduce `await setupNeonDatabase()`.
|
||
|
|
- **Move `bcrypt` import to the top of the file.** The CJS version uses inline `const bcrypt = require('bcryptjs')` mid-function. ESM has no equivalent of late `require()` — all `import` statements must be at the top. This is a structural requirement of ESM, not a stylistic preference.
|
||
|
|
- **No `__dirname` or `__filename`.** The script does not currently use either, so no ESM equivalents (`import.meta.url`, `fileURLToPath`) are needed.
|
||
|
|
- **No `.cjs` rename.** The right answer is real ESM, not bypassing `"type": "module"` with a `.cjs` extension. A `.cjs` rename would also break the `setup-db` npm script (`node scripts/setup-neon-db.js`) without a corresponding `package.json` edit, which is out of scope here.
|
||
|
|
|
||
|
|
## Acceptance criteria
|
||
|
|
|
||
|
|
### `scripts/setup-neon-db.js`
|
||
|
|
|
||
|
|
- [ ] **Replace the `dotenv` invocation** (currently line 11):
|
||
|
|
|
||
|
|
```js
|
||
|
|
// before:
|
||
|
|
require('dotenv').config({ path: '.env.local' });
|
||
|
|
|
||
|
|
// after:
|
||
|
|
import dotenv from 'dotenv';
|
||
|
|
dotenv.config({ path: '.env.local' });
|
||
|
|
```
|
||
|
|
|
||
|
|
- [ ] **Replace the `neon` import** (currently line 13):
|
||
|
|
|
||
|
|
```js
|
||
|
|
// before:
|
||
|
|
const { neon } = require('@neondatabase/serverless');
|
||
|
|
|
||
|
|
// after:
|
||
|
|
import { neon } from '@neondatabase/serverless';
|
||
|
|
```
|
||
|
|
|
||
|
|
- [ ] **Hoist the `bcryptjs` import to the top of the file**, alongside the other imports. Remove the inline `const bcrypt = require('bcryptjs');` from inside `setupNeonDatabase()` (currently line 142):
|
||
|
|
|
||
|
|
```js
|
||
|
|
// at the top of the file, after the dotenv block:
|
||
|
|
import bcrypt from 'bcryptjs';
|
||
|
|
|
||
|
|
// inside setupNeonDatabase(), delete this line:
|
||
|
|
const bcrypt = require('bcryptjs');
|
||
|
|
```
|
||
|
|
|
||
|
|
- [ ] **Final import-block order at the top of the file** (after the shebang + JSDoc header):
|
||
|
|
|
||
|
|
```js
|
||
|
|
#!/usr/bin/env node
|
||
|
|
|
||
|
|
/**
|
||
|
|
* Neon Database Setup Script
|
||
|
|
* ... (existing JSDoc) ...
|
||
|
|
*/
|
||
|
|
|
||
|
|
import dotenv from 'dotenv';
|
||
|
|
dotenv.config({ path: '.env.local' });
|
||
|
|
|
||
|
|
import { neon } from '@neondatabase/serverless';
|
||
|
|
import bcrypt from 'bcryptjs';
|
||
|
|
|
||
|
|
async function setupNeonDatabase() {
|
||
|
|
// ... env-var check from brief 1, unchanged ...
|
||
|
|
// ... rest of function, with the inline require deleted ...
|
||
|
|
}
|
||
|
|
|
||
|
|
setupNeonDatabase();
|
||
|
|
```
|
||
|
|
|
||
|
|
**Note:** placing `dotenv.config(...)` *between* the import statements is intentional — `dotenv` must run before any other module reads `process.env`. ESM hoists `import` declarations but executes them in source order, so the `import dotenv from 'dotenv'` declaration hoists to the top, the `dotenv.config({...})` call executes after the import resolves, and the subsequent `import { neon }` / `import bcrypt` resolve after that. If the linter complains about "imports not grouped together," resolve by moving the `dotenv.config({...})` call into a separate side-effect import (`import 'dotenv/config'` will not work here because we need the custom `.env.local` path).
|
||
|
|
|
||
|
|
- [ ] **The env-var check from brief 1 is unchanged.** Lines 15-26 (the `const adminPassword = ...` block, the `if (!adminPassword || !adminPassword.trim())` block, and the `process.exit(1)` call) stay byte-identical. This brief only touches the import block at the top of the file and the inline `require('bcryptjs')` deletion.
|
||
|
|
|
||
|
|
- [ ] **The DDL, `INSERT`, `ON CONFLICT` clause, success summary `console.log` lines, and `try/catch/process.exit(1)` are all unchanged.** Pure module-system conversion.
|
||
|
|
|
||
|
|
- [ ] **`setupNeonDatabase();` at the bottom of the file stays as-is** (bare call, no `await`).
|
||
|
|
|
||
|
|
### Verification
|
||
|
|
|
||
|
|
- [ ] **Smoke: fail-loud path.** With `ADMIN_INITIAL_PASSWORD` unset:
|
||
|
|
|
||
|
|
```bash
|
||
|
|
unset ADMIN_INITIAL_PASSWORD
|
||
|
|
npm run setup-db
|
||
|
|
echo "exit=$?"
|
||
|
|
```
|
||
|
|
|
||
|
|
Expected:
|
||
|
|
- Exit code: `1`
|
||
|
|
- Stderr contains the brief-1 actionable error message (names `ADMIN_INITIAL_PASSWORD`, points at `.env.local`, suggests `openssl rand -base64 24`)
|
||
|
|
- **Stdout does NOT contain `✅ Connecting to Neon database...`** — the env-var check fires before `neon(...)`, just like in the brief-1 smoke
|
||
|
|
- **No `ReferenceError: require is not defined`** — this is the bug brief 2 fixes; its absence is the primary signal
|
||
|
|
- **No `SyntaxError: Cannot use import statement outside a module`** — would indicate the conversion picked the wrong direction
|
||
|
|
|
||
|
|
- [ ] **Smoke: happy path.** With the env var set:
|
||
|
|
|
||
|
|
```bash
|
||
|
|
export ADMIN_INITIAL_PASSWORD=brief-2-smoke-temporary-pw
|
||
|
|
npm run setup-db
|
||
|
|
echo "exit=$?"
|
||
|
|
```
|
||
|
|
|
||
|
|
Expected:
|
||
|
|
- Exit code: `0`
|
||
|
|
- Stdout contains `✅ Connecting to Neon database...` and `✅ Created admin user` (or the equivalent for an already-existing admin row — `ON CONFLICT DO NOTHING` keeps the script idempotent)
|
||
|
|
- Stdout does **NOT** contain `admin123` or `brief-2-smoke-temporary-pw` (R3 from brief 1 — credentials never echoed to stdout)
|
||
|
|
- No `ReferenceError`, no `SyntaxError`, no `import`/`require` complaints
|
||
|
|
|
||
|
|
- [ ] **Lint.** `npm run lint` exits 0 (or matches the existing baseline — pre-existing lint errors are fine; do not introduce new ones).
|
||
|
|
|
||
|
|
- [ ] **Tests.** `npm run test:run` is green. All 16 existing vitest tests should still pass — this brief does not touch any file they cover.
|
||
|
|
|
||
|
|
- [ ] **Grep sanity.** `rg 'require\(' scripts/setup-neon-db.js` returns **zero** hits after the conversion.
|
||
|
|
|
||
|
|
- [ ] **Brief-1 smoke still passes.** Re-run the brief-1 end-to-end login smoke against `npm run dev`:
|
||
|
|
|
||
|
|
```bash
|
||
|
|
curl -sS -X POST http://localhost:3000/api/auth/login \
|
||
|
|
-H 'content-type: application/json' \
|
||
|
|
-d '{"email":"admin@tcgvault.com","password":"brief-2-smoke-temporary-pw"}'
|
||
|
|
```
|
||
|
|
|
||
|
|
Expected: same R1 case-A behavior as brief 1 (if the local admin row predates `ADMIN_INITIAL_PASSWORD`, the new password 401s and the old `admin123` still 200s; if the admin row was newly created, the new password 200s with a JWT). Paste the result into the PR description.
|
||
|
|
|
||
|
|
### Pre-merge checklist (paste into PR description, alongside brief 1's checklist)
|
||
|
|
|
||
|
|
- [ ] **R1 callout (carried over from brief 1):** existing deployed admin rows with the weak `admin123` hash are NOT rotated by this PR. Operators rotate manually after merge.
|
||
|
|
- [ ] **Brief 2 makes `npm run setup-db` executable again on Node 22.x.** Before this brief, the script threw `ReferenceError: require is not defined in ES module scope` immediately on invocation. After this brief, the env-var gate from brief 1 actually fires.
|
||
|
|
- [ ] **Reviewer ran the brief-2 fail-loud and happy-path smoke locally** OR confirmed the PR description includes terminal output proving both paths.
|
||
|
|
|
||
|
|
### Out of scope (do not do these)
|
||
|
|
|
||
|
|
- [ ] No `package.json` change. `"type": "module"` stays; this brief makes the script align with that setting, not the other way around.
|
||
|
|
- [ ] No edit to `scripts/reset-db.js`, `scripts/create-test-users.js`, or any other `scripts/*.js`. They have the same CJS-in-ESM bug, but converting all of them widens the diff and bleeds into the no-go-zones rule for historical scripts. Queue `convert-helper-scripts-to-esm` as a separate convoy if/when those scripts need to run.
|
||
|
|
- [ ] No edit to `README.md`. Brief 1 owns the README changes; this brief is code-only.
|
||
|
|
- [ ] No `.cjs` rename. Real ESM only.
|
||
|
|
- [ ] No top-level `await` introduction at the bottom of the file.
|
||
|
|
- [ ] No new dependencies.
|
||
|
|
- [ ] No edit to `lib/`, `pages/`, `components/`, `.github/`, or `.cursor/`.
|
||
|
|
- [ ] No vitest tests. The smoke commands above are the verification; setup-db is not a runtime-auth surface (Decision C1 from brief 1 still applies — operational scripts use manual smoke, not unit tests).
|
||
|
|
|
||
|
|
## Rationale (≤3 sentences)
|
||
|
|
|
||
|
|
`bump-next-js`'s `"type": "module"` flag silently broke `npm run setup-db` on Node 22.x because the script still uses CJS `require()` calls. Brief 1's `ADMIN_INITIAL_PASSWORD` env-var gate is the right hardening but lands as theatrical security on a script no operator can actually execute — converting to native ESM imports here ensures the gate fires as intended and unblocks first-time DB setup on any Node 22+ environment (including Vercel's default). The diff is 4-6 lines of mechanical conversion with no functional changes, so review risk is minimal and the convoy stays a single coherent PR.
|