Merge pull request #51 from screentinker/fix/migration-schema-verify

fix(db): observable migrations + fail-fast schema verification (#37)

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 };

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 };

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');
+});