fix(scripts): document the DATABASE_URL-prefixed invocation, make SQLite an explicit opt-in

apply-onboarding-pipeline.ts documented its own command with no `DATABASE_URL=`
prefix -- the only hardened data script missing it. Followed literally on the
production host it resolves to SQLite, not the real database. Header corrected to
match its siblings.

The deeper problem is that landing on SQLite was a DEFAULT, not a decision: with no
DATABASE_URL, resolution quietly returned '' and the run was only saved by the
"never CREATE a database" guard -- which passes as soon as any stale ./data/hq.db
exists under the operator shell's cwd. requireDbUrlForEnv now refuses a SQLite
outcome unless it was explicitly asked for (HQ_DB_TARGET=sqlite, a --sqlite flag,
or HQ_DATA_DIR=:memory:), and the error names both escape hatches. The create guard
is unchanged and still applies after the opt-in. server.ts calls resolveDbUrl
directly, so first-boot DB creation is untouched.

All five maintenance-script headers now state the SQLite opt-in.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
main
Thomas Joise 22 hours ago
parent 1717900e02
commit 77f135f0e2

@ -4,6 +4,8 @@
// skips a client whose name already contains that number, and only touches found=true rows.
// Does NOT overwrite the name's spelling/casing — only appends the number.
// LTD_JSON=<results.json> DATABASE_URL=postgres://… npx tsx apps/hq/scripts/apply-ltd-numbers.ts
// SQLite is never the default: to target a local SQLite database, opt in explicitly with
// HQ_DB_TARGET=sqlite (or the --sqlite flag). Without either, the script refuses to run.
import { readFileSync } from 'node:fs'
import { openDb, sqliteFilePath } from '../src/db'
import { openPgDb, pgTargetDescription, requireDbUrlForEnv } from '../src/db-pg'

@ -1,6 +1,8 @@
// apps/hq/scripts/apply-name-fixes.ts — one-off (D35 data correction): apply the approved
// client-name corrections from name-review.json via the audited updateClient path.
// REVIEW_JSON=<path> DATABASE_URL=postgres://… npx tsx apps/hq/scripts/apply-name-fixes.ts
// SQLite is never the default: to target a local SQLite database, opt in explicitly with
// HQ_DB_TARGET=sqlite (or the --sqlite flag). Without either, the script refuses to run.
import { readFileSync } from 'node:fs'
import { openDb, sqliteFilePath } from '../src/db'
import { openPgDb, pgTargetDescription, requireDbUrlForEnv } from '../src/db-pg'

@ -5,8 +5,12 @@
// is written only if it is still ABSENT, so a plain re-run never clobbers an owner's edit made
// in the Modules onboarding-template editor. Pass --force to unconditionally overwrite every
// template with the settings below — this DOES clobber owner edits, use with care.
// Run from repo root:
// npx tsx apps/hq/scripts/apply-onboarding-pipeline.ts [--force]
// Run from repo root, naming the database explicitly (this header previously omitted the
// DATABASE_URL prefix — followed literally on the production host it resolves to SQLite, not
// the real database, and the script now refuses rather than migrating the wrong DB):
// DATABASE_URL=postgres://… npx tsx apps/hq/scripts/apply-onboarding-pipeline.ts [--force]
// To deliberately run against a local SQLite database instead, opt in explicitly:
// HQ_DB_TARGET=sqlite npx tsx apps/hq/scripts/apply-onboarding-pipeline.ts (or --sqlite)
import { openDb, sqliteFilePath } from '../src/db'
import { openPgDb, pgTargetDescription, requireDbUrlForEnv } from '../src/db-pg'
import { migrateOnboardingTemplates } from '../src/migrate-onboarding-templates'

@ -3,6 +3,8 @@
// and must_change_password=1 (they change it on first login). Active per ACTIVE_STAT.
// Idempotent: skips a username/email that already exists.
// DATABASE_URL=postgres://… npx tsx apps/hq/scripts/import-employees.ts
// SQLite is never the default: to target a local SQLite database, opt in explicitly with
// HQ_DB_TARGET=sqlite (or the --sqlite flag). Without either, the script refuses to run.
import { openDb, sqliteFilePath } from '../src/db'
import { openPgDb, pgTargetDescription, requireDbUrlForEnv } from '../src/db-pg'
import { createEmployee } from '../src/repos-employees'

@ -3,6 +3,8 @@
// checklist onto their new per-module template, carrying mapped ticks. Idempotent — safe
// to re-run (a second run does nothing).
// DATABASE_URL=postgres://… npx tsx apps/hq/scripts/migrate-onboarding-templates.ts
// SQLite is never the default: to target a local SQLite database, opt in explicitly with
// HQ_DB_TARGET=sqlite (or the --sqlite flag). Without either, the script refuses to run.
import { openDb, sqliteFilePath } from '../src/db'
import { openPgDb, pgTargetDescription, requireDbUrlForEnv } from '../src/db-pg'
import { migrateOnboardingTemplates } from '../src/migrate-onboarding-templates'

@ -142,15 +142,37 @@ export function resolveDbUrl(env: NodeJS.ProcessEnv = process.env, hardcoded = H
* migration/maintenance script must never CREATE a database, regardless of what NODE_ENV is or
* isn't set to. `hardcoded` and `sqliteExists` are both injectable so tests never touch a real
* Postgres URL or the real filesystem.
*
* Second guard (same family of bug, one step earlier): landing on SQLite at all must be a
* DECISION, never a default. An operator who follows a script's documented command with no
* `DATABASE_URL=` in front of it should be told so, not quietly pointed at whatever `./data/hq.db`
* happens to exist under their shell's cwd. So SQLite additionally requires an explicit opt-in
* `HQ_DB_TARGET=sqlite` or a `--sqlite` argv flag (`HQ_DATA_DIR=:memory:` counts too: nobody sets
* that by accident). Without one, the script refuses and the error names both options.
* `argv` is injectable for the same reason as the rest: tests never depend on the real process.
*/
export function requireDbUrlForEnv(
env: NodeJS.ProcessEnv = process.env,
hardcoded = HARDCODED_DATABASE_URL,
sqliteExists: (file: string) => boolean = fs.existsSync,
argv: readonly string[] = process.argv,
): string {
const pgUrl = resolveDbUrl(env, hardcoded)
if (pgUrl === '') {
const file = sqliteFilePath(env['HQ_DATA_DIR'])
const optedIn =
file === ':memory:' ||
(env['HQ_DB_TARGET'] ?? '').trim().toLowerCase() === 'sqlite' ||
argv.includes('--sqlite')
if (!optedIn) {
throw new Error(
'[db] refusing to run: no Postgres target resolved (DATABASE_URL is unset), and SQLite ' +
'has not been explicitly requested — running against the wrong database is worse than ' +
'not running at all. Either set DATABASE_URL=postgres://… to target the real database, ' +
`or opt into SQLite explicitly with HQ_DB_TARGET=sqlite (or the --sqlite flag), which ` +
`would use ${file}.`,
)
}
if (file !== ':memory:' && !sqliteExists(file)) {
throw new Error(
`[db] refusing to run: this would CREATE a new SQLite database at ${file}. ` +

@ -35,10 +35,10 @@ describe('requireDbUrlForEnv (deploy-script guard — OUTCOME-based, not NODE_EN
expect(requireDbUrlForEnv({ NODE_ENV: 'production' }, HARDCODED)).toBe(HARDCODED)
})
it('resolves to SQLite without hard-failing when the target file already exists (mocked fs check)', () => {
expect(requireDbUrlForEnv({}, undefined, () => true)).toBe('')
it('resolves to SQLite without hard-failing when it is opted into and the target file exists (mocked fs check)', () => {
expect(requireDbUrlForEnv({ HQ_DB_TARGET: 'sqlite' }, undefined, () => true)).toBe('')
// Even NODE_ENV=production is fine as long as the SQLite file it would land on already exists.
expect(requireDbUrlForEnv({ NODE_ENV: 'production' }, '', () => true)).toBe('')
expect(requireDbUrlForEnv({ NODE_ENV: 'production', HQ_DB_TARGET: 'sqlite' }, '', () => true)).toBe('')
})
it(':memory: never hard-fails — the guard never even calls the file-existence check', () => {
@ -52,10 +52,52 @@ describe('requireDbUrlForEnv (deploy-script guard — OUTCOME-based, not NODE_EN
it('refuses when resolution selects SQLite and the target does not exist — regardless of NODE_ENV', () => {
// This is the original prod bug: an operator shell has no NODE_ENV set at all, so the old
// NODE_ENV==='production' gate never fired. The guard must fire on the OUTCOME instead.
expect(() => requireDbUrlForEnv({}, undefined, () => false)).toThrow(/refusing to run/i)
expect(() => requireDbUrlForEnv({}, undefined, () => false)).toThrow(/CREATE a new SQLite database/)
const optIn = { HQ_DB_TARGET: 'sqlite' }
expect(() => requireDbUrlForEnv(optIn, undefined, () => false)).toThrow(/refusing to run/i)
expect(() => requireDbUrlForEnv(optIn, undefined, () => false)).toThrow(/CREATE a new SQLite database/)
// Also still refuses in the NODE_ENV=production case (hardcoded emptied out + no existing file).
expect(() => requireDbUrlForEnv({ NODE_ENV: 'production' }, '', () => false)).toThrow(/refusing to run/i)
expect(() => requireDbUrlForEnv({ NODE_ENV: 'production', HQ_DB_TARGET: 'sqlite' }, '', () => false))
.toThrow(/refusing to run/i)
})
})
describe('requireDbUrlForEnv — SQLite must be an explicit opt-in, never an accident', () => {
// The bug this closes: apply-onboarding-pipeline.ts documented its own invocation WITHOUT a
// `DATABASE_URL=` prefix. An operator who copy-pastes that on the prod box resolves to SQLite;
// if a stale ./data/hq.db happens to exist under their cwd, the file-existence guard passes and
// the migration "succeeds" against the wrong database. Landing on SQLite must be a decision.
const existingFile = (): boolean => true
it('refuses when nothing selects a target, even though the SQLite file exists', () => {
expect(() => requireDbUrlForEnv({}, undefined, existingFile)).toThrow(/refusing to run/i)
})
it('names BOTH escape hatches in the error — DATABASE_URL and the SQLite opt-in', () => {
expect(() => requireDbUrlForEnv({}, undefined, existingFile)).toThrow(/DATABASE_URL=postgres/)
expect(() => requireDbUrlForEnv({}, undefined, existingFile)).toThrow(/HQ_DB_TARGET=sqlite/)
expect(() => requireDbUrlForEnv({}, undefined, existingFile)).toThrow(/--sqlite/)
})
it('accepts HQ_DB_TARGET=sqlite (case/space tolerant)', () => {
expect(requireDbUrlForEnv({ HQ_DB_TARGET: 'sqlite' }, undefined, existingFile)).toBe('')
expect(requireDbUrlForEnv({ HQ_DB_TARGET: ' SQLite ' }, undefined, existingFile)).toBe('')
})
it('accepts a --sqlite argv flag (alongside the script\'s own flags)', () => {
expect(requireDbUrlForEnv({}, undefined, existingFile, ['node', 'script.ts', '--force', '--sqlite'])).toBe('')
})
it('rejects an unrelated HQ_DB_TARGET value rather than guessing', () => {
expect(() => requireDbUrlForEnv({ HQ_DB_TARGET: 'postgres' }, undefined, existingFile)).toThrow(/refusing to run/i)
})
it('the opt-in does NOT weaken the "never CREATE a database" guard', () => {
expect(() => requireDbUrlForEnv({ HQ_DB_TARGET: 'sqlite' }, undefined, () => false))
.toThrow(/CREATE a new SQLite database/)
})
it('DATABASE_URL needs no opt-in — Postgres is the intended target and never touches the flag', () => {
expect(requireDbUrlForEnv({ DATABASE_URL: 'postgres://x/y' }, undefined, () => false, [])).toBe('postgres://x/y')
})
})
@ -63,17 +105,20 @@ describe('requireDbUrlForEnv — real filesystem (the actual script entry path,
it('refuses a non-existent SQLite path', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'hq-db-resolve-'))
try {
expect(() => requireDbUrlForEnv({ HQ_DATA_DIR: dir })).toThrow(/refusing to run/i)
expect(() => requireDbUrlForEnv({ HQ_DATA_DIR: dir, HQ_DB_TARGET: 'sqlite' }))
.toThrow(/CREATE a new SQLite database/)
} finally {
fs.rmSync(dir, { recursive: true, force: true })
}
})
it('accepts an existing SQLite path', () => {
it('accepts an existing SQLite path once SQLite is opted into', () => {
const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'hq-db-resolve-'))
try {
fs.writeFileSync(path.join(dir, 'hq.db'), '')
expect(requireDbUrlForEnv({ HQ_DATA_DIR: dir })).toBe('')
expect(requireDbUrlForEnv({ HQ_DATA_DIR: dir, HQ_DB_TARGET: 'sqlite' })).toBe('')
// …and still refuses the same existing path with no opt-in at all.
expect(() => requireDbUrlForEnv({ HQ_DATA_DIR: dir })).toThrow(/HQ_DB_TARGET=sqlite/)
} finally {
fs.rmSync(dir, { recursive: true, force: true })
}

Loading…
Cancel
Save