mirror of
https://github.com/screentinker/screentinker.git
synced 2026-06-14 18:22:46 -06:00
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.
This commit is contained in:
parent
9deccf0a2f
commit
7ab19adcea
|
|
@ -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 };
|
||||
|
|
|
|||
69
server/lib/schema-check.js
Normal file
69
server/lib/schema-check.js
Normal file
|
|
@ -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 };
|
||||
49
server/test/schema-check.test.js
Normal file
49
server/test/schema-check.test.js
Normal file
|
|
@ -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');
|
||||
});
|
||||
Loading…
Reference in a new issue