From b146de990f54a31e2c8c0b38a0e89966258877da Mon Sep 17 00:00:00 2001 From: Thomas Joise Date: Mon, 20 Jul 2026 21:54:15 +0530 Subject: [PATCH] fix(scripts): audit + guard apply-onboarding-pipeline's setting writes Final whole-branch review FIX 2. The live-deploy script upserted the onboarding template settings with raw SQL (ON CONFLICT DO UPDATE), bypassing setSetting -> no audit row, violating the append-only-audit rule -- and it unconditionally overwrote templates an owner may have already edited in the Modules onboarding-template editor, making the "Idempotent" header comment false and dangerous for a script that runs against ~167 real projects. Upserts now go through setSetting(db, 'system', key, value) (audited), and only write a key that is still absent, unless run with an explicit --force argument (documented). Header comment corrected to say idempotency holds only while the templates haven't been owner-edited, and that --force overwrites unconditionally. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/hq/scripts/apply-onboarding-pipeline.ts | 30 ++++++++++++++------ 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/apps/hq/scripts/apply-onboarding-pipeline.ts b/apps/hq/scripts/apply-onboarding-pipeline.ts index fd49cc9..ce0393f 100644 --- a/apps/hq/scripts/apply-onboarding-pipeline.ts +++ b/apps/hq/scripts/apply-onboarding-pipeline.ts @@ -1,10 +1,16 @@ -// One-off deploy step for the Client Detail redesign: overwrite the live onboarding-template -// settings with the composed quotation-led pipeline (shared front + per-module tail), then run +// One-off deploy step for the Client Detail redesign: writes the live onboarding-template +// settings with the composed quotation-led pipeline (shared front + per-module tail), then runs // the tick-preserving migration so existing SMS/RTGS/MobileApp projects adopt the new steps. -// Idempotent. Run from repo root: npx tsx apps/hq/scripts/apply-onboarding-pipeline.ts +// Idempotent ONLY as long as the templates below haven't been owner-edited since: each setting +// 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] import { openDb } from '../src/db' import { openPgDb } from '../src/db-pg' import { migrateOnboardingTemplates } from '../src/migrate-onboarding-templates' +import { getSetting, setSetting } from '../src/repos-reminders' const FRONT = [ { key: 'enquiry', label: 'Enquiry received' }, @@ -48,21 +54,27 @@ const TAILS: Record = { async function main(): Promise { const pgUrl = process.env['DATABASE_URL'] ?? '' const db = pgUrl !== '' ? await openPgDb(pgUrl) : openDb(process.env['HQ_DATA_DIR']) + const force = process.argv.includes('--force') + // Audited via setSetting (append-only-audit rule) and, unless --force, skips any key + // that's already set — so an owner's edit in the Modules onboarding-template editor is + // left alone on a re-run instead of being silently clobbered. const upsert = async (key: string, list: unknown): Promise => { - await db.run( - `INSERT INTO setting (key, value) VALUES (?, ?) ON CONFLICT (key) DO UPDATE SET value = excluded.value`, - key, JSON.stringify(list), - ) + if (!force && (await getSetting(db, key)) !== null) { + // eslint-disable-next-line no-console + console.log(`Skipped ${key} — already set (pass --force to overwrite an owner edit).`) + return + } + await setSetting(db, 'system', key, JSON.stringify(list)) } await upsert('project.milestone_template.front', FRONT) for (const [code, list] of Object.entries(TAILS)) await upsert(`project.milestone_template:${code}`, list) // eslint-disable-next-line no-console - console.log('Template settings updated: front + SMS/RTGS/MOBILEAPP tails.') + console.log('Template settings step done: front + SMS/RTGS/MOBILEAPP tails.') const res = await migrateOnboardingTemplates(db) // eslint-disable-next-line no-console - console.log(`Onboarding migration: ${res.projects} project(s) rebuilt, ${res.carried} tick(s) carried, ${res.dropped} dropped.`) + console.log(`Onboarding migration: ${res.projects} project(s) rebuilt, ${res.carried} tick(s) carried, ${res.dropped} dropped, ${res.preserved} custom step(s) preserved.`) } main().catch((e) => { console.error(e); process.exit(1) })