From 7ab19adceadf33ed5b7fa28945144e767f7fa875 Mon Sep 17 00:00:00 2001 From: ScreenTinker Date: Tue, 9 Jun 2026 09:31:52 -0500 Subject: [PATCH] fix(db): observable migrations + fail-fast schema verification (#37) Self-hosters rebuilding could end up schema-behind-code, failing only at runtime (a missing users.must_change_password locked out all logins). Two root causes: 1. The migration loop swallowed EVERY error (catch {}), so a real ALTER failure was indistinguishable from the benign 'duplicate column' on an already-migrated DB. Now only 'duplicate column'/'already exists' is treated as a no-op; any other error is logged loudly, and a one-line summary reports how many new column migrations actually applied this boot. 2. Nothing verified the schema after migrations. Added lib/schema-check.js: verifyAndRepairSchema() checks the tables + columns the request path REQUIRES, idempotently repairs missing repairable columns (logging each), and if anything required is STILL missing, prints a loud FATAL block and exits - failing fast at boot instead of at the first authed request. Note: the reported 'audit_log missing' was a misdiagnosis - the code uses activity_log (0 refs to audit_log), created by schema.sql on every boot. Tests: healthy (no-op), auto-repair of must_change_password, missing-table report. --- server/db/database.js | 20 ++++++++- server/lib/schema-check.js | 69 ++++++++++++++++++++++++++++++++ server/test/schema-check.test.js | 49 +++++++++++++++++++++++ 3 files changed, 137 insertions(+), 1 deletion(-) create mode 100644 server/lib/schema-check.js create mode 100644 server/test/schema-check.test.js diff --git a/server/db/database.js b/server/db/database.js index d8a7448..a1cdba3 100644 --- a/server/db/database.js +++ b/server/db/database.js @@ -178,9 +178,23 @@ const migrations = [ // login. Default 0 so all existing users are unaffected. "ALTER TABLE users ADD COLUMN must_change_password INTEGER NOT NULL DEFAULT 0", ]; +// Apply each ALTER idempotently. A "duplicate column name" / "already exists" +// error means the column is already present (expected on a migrated DB) - benign. +// ANY OTHER error is a real, partial-migration failure: log it loudly so it's +// visible at boot rather than as a silent runtime failure later (issue #37, where +// a swallowed failure left users.must_change_password absent -> total auth lockout). +let _migApplied = 0; for (const sql of migrations) { - try { db.exec(sql); } catch (e) { /* already exists */ } + try { + db.exec(sql); + _migApplied++; + } catch (e) { + if (!/duplicate column name|already exists/i.test(e.message)) { + console.error(`[migrate] FAILED: ${sql}\n -> ${e.message}`); + } + } } +if (_migApplied > 0) console.log(`[migrate] applied ${_migApplied} new column migration(s)`); // Fix assignments table: make content_id nullable (SQLite requires table rebuild) try { @@ -700,4 +714,8 @@ function pruneScreenshots(deviceId) { `).run(deviceId, deviceId); } +// #37: fail fast (loud) if migrations left the DB missing schema the code needs. +const { verifyAndRepairSchema } = require('../lib/schema-check'); +verifyAndRepairSchema(db); + module.exports = { db, pruneTelemetry, pruneScreenshots }; diff --git a/server/lib/schema-check.js b/server/lib/schema-check.js new file mode 100644 index 0000000..dd0f5f7 --- /dev/null +++ b/server/lib/schema-check.js @@ -0,0 +1,69 @@ +'use strict'; + +// #37: verify the DB has the schema the running code REQUIRES, after all +// migrations have run. On a partial/stale DB (e.g. a Docker rebuild that missed a +// migration) it repairs missing repairable columns idempotently and logs clearly; +// if anything required is STILL missing it calls onMissing (default: loud log + +// process.exit(1)) so the server fails fast at boot instead of limping along and +// breaking at the first authed request. The #37 lockout was a silently-absent +// users.must_change_password, which the auth middleware gates every request on. + +// Tables the request path depends on (schema.sql creates them with CREATE TABLE +// IF NOT EXISTS on every boot; listed so their absence is still caught loudly). +const REQUIRED_TABLES = [ + 'users', 'organizations', 'organization_members', 'workspaces', 'workspace_members', + 'devices', 'content', 'playlists', 'activity_log', 'schema_migrations', +]; + +// [table, column, repairSQL] — columns the code SELECTs / gates on. repairSQL is +// the idempotent ALTER that adds it if missing (null = base column, assert only). +const REQUIRED_COLUMNS = [ + ['users', 'must_change_password', "ALTER TABLE users ADD COLUMN must_change_password INTEGER NOT NULL DEFAULT 0"], + ['users', 'role', null], + ['users', 'plan_id', "ALTER TABLE users ADD COLUMN plan_id TEXT DEFAULT 'free'"], +]; + +function defaultOnMissing(missing) { + const bar = '='.repeat(72); + console.error(`\n${bar}`); + console.error('[schema-check] FATAL: database is missing required schema:'); + for (const m of missing) console.error(` - ${m}`); + console.error('Migrations did not make the schema code-complete. The server is'); + console.error('refusing to start to avoid silent runtime failures (e.g. issue #37,'); + console.error('where a missing users.must_change_password failed every login).'); + console.error('Fix: restore the newest db/remote_display.pre-*.db snapshot, or add'); + console.error('the missing column/table manually, then restart.'); + console.error(`${bar}\n`); + process.exit(1); +} + +// Returns the list of still-missing items (empty when healthy). Calls +// opts.onMissing(missing) when non-empty (default exits the process). +function verifyAndRepairSchema(db, opts = {}) { + const onMissing = opts.onMissing || defaultOnMissing; + const tableSet = new Set(db.prepare("SELECT name FROM sqlite_master WHERE type='table'").all().map(r => r.name)); + const columns = (t) => new Set(db.prepare(`PRAGMA table_info(${t})`).all().map(c => c.name)); + + const missing = []; + for (const t of REQUIRED_TABLES) if (!tableSet.has(t)) missing.push(`table "${t}"`); + + for (const [t, c, repair] of REQUIRED_COLUMNS) { + if (!tableSet.has(t)) continue; // table-missing already recorded + if (columns(t).has(c)) continue; + if (repair) { + try { + console.warn(`[schema-check] required column ${t}.${c} is missing — applying repair...`); + db.exec(repair); + console.warn(`[schema-check] repaired ${t}.${c}`); + } catch (e) { + console.error(`[schema-check] repair of ${t}.${c} FAILED: ${e.message}`); + } + } + if (!columns(t).has(c)) missing.push(`column "${t}.${c}"`); + } + + if (missing.length) onMissing(missing); + return missing; +} + +module.exports = { verifyAndRepairSchema, REQUIRED_TABLES, REQUIRED_COLUMNS }; diff --git a/server/test/schema-check.test.js b/server/test/schema-check.test.js new file mode 100644 index 0000000..12736fa --- /dev/null +++ b/server/test/schema-check.test.js @@ -0,0 +1,49 @@ +'use strict'; + +// #37: verifyAndRepairSchema - repairs missing repairable columns, and reports +// (fail-fast hook) anything still missing. We inject onMissing so the fail path +// doesn't call process.exit during tests. Node v20 built-ins only. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const Database = require('better-sqlite3'); +const { verifyAndRepairSchema, REQUIRED_TABLES } = require('../lib/schema-check'); + +function freshDb(withMustChange = true) { + const db = new Database(':memory:'); + for (const t of REQUIRED_TABLES) { + if (t === 'users') { + db.exec(`CREATE TABLE users (id TEXT PRIMARY KEY, role TEXT, plan_id TEXT${withMustChange ? ', must_change_password INTEGER NOT NULL DEFAULT 0' : ''})`); + } else { + db.exec(`CREATE TABLE ${t} (id TEXT PRIMARY KEY)`); + } + } + return db; +} +const hasCol = (db, t, c) => db.prepare(`PRAGMA table_info(${t})`).all().some(x => x.name === c); + +test('healthy schema: nothing missing, onMissing not called', () => { + const db = freshDb(true); + let called = false; + const missing = verifyAndRepairSchema(db, { onMissing: () => { called = true; } }); + assert.deepEqual(missing, []); + assert.equal(called, false); +}); + +test('missing repairable column (must_change_password) is auto-repaired - no fail', () => { + const db = freshDb(false); + assert.equal(hasCol(db, 'users', 'must_change_password'), false, 'precondition: column absent'); + let called = false; + const missing = verifyAndRepairSchema(db, { onMissing: () => { called = true; } }); + assert.deepEqual(missing, [], 'no residual missing after repair'); + assert.equal(called, false, 'onMissing not called when repair succeeds'); + assert.equal(hasCol(db, 'users', 'must_change_password'), true, 'column was added'); +}); + +test('missing required table -> reported to onMissing (fail-fast hook fires)', () => { + const db = freshDb(true); + db.exec('DROP TABLE activity_log'); + let got = null; + verifyAndRepairSchema(db, { onMissing: (m) => { got = m; } }); + assert.ok(got && got.some(x => /activity_log/.test(x)), 'reported the missing table'); +});