From c435263744cf6a23b6cd9e0466cc8af3f6c4894a Mon Sep 17 00:00:00 2001 From: Luca <102960244+Luca-Timo@users.noreply.github.com> Date: Sun, 31 May 2026 21:35:48 +0200 Subject: [PATCH] fix(restore): default restore_allow_force=true + auto-upgrade existing installs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Root cause of the persistent "Force restore is not allowed by system settings" error even on fresh installs after `docker compose down -v`: migrations/core/032_add_restore_runs_table.js seeded the row with `JSON.stringify(false)` = the literal string 'false'. So every install (fresh OR upgraded) wrote restore_allow_force=false at migration time. The boot self-heal added earlier today saw the row and respected "admin policy" per its safety design — never noticing that the row was the deprecated migration default, not an explicit admin choice. Cure follows [[feedback_migration_no_compensation]] + [[feedback_self_heal_pattern]]: 1. Edit migration 032 IN PLACE — flip seed value from false to true. Fresh installs forward get the correct default at install time, no boot helper needed. 2. One-time auto-upgrade in _restoreSettingsBoot.js for installs that already ran the OLD migration. Bumps restore_allow_force to 'true' iff the current value is the deprecated literal 'false' AND the new tracking key `restore_allow_force_auto_upgraded` doesn't yet exist. The tracking flag is always written after the first boot pass, so subsequent admin choices (e.g. deliberately disabling force) are preserved on every boot after. 3. Defensive: adminRestore.js getRestoreSettings() now normalizes 'true'/'false'/'"true"'/'"false"' string shapes to JS booleans, not just '1'/'0'. Belt-and-suspenders so any future seeder that uses a different boolean serialization doesn't silently break the !settings.restore_allow_force gate. Net effect: any picpeak install pulling this image — fresh or existing — gets restore_allow_force=true on first boot after the upgrade. The catch-22 that forced every disaster-recovery admin to hand-write SQL before their FIRST restore is closed. --- .../core/032_add_restore_runs_table.js | 12 ++- backend/src/routes/adminRestore.js | 18 +++-- backend/src/services/_restoreSettingsBoot.js | 76 +++++++++++++++++-- 3 files changed, 93 insertions(+), 13 deletions(-) diff --git a/backend/migrations/core/032_add_restore_runs_table.js b/backend/migrations/core/032_add_restore_runs_table.js index 1bc06db4..0b4ce256 100644 --- a/backend/migrations/core/032_add_restore_runs_table.js +++ b/backend/migrations/core/032_add_restore_runs_table.js @@ -100,8 +100,18 @@ exports.up = async function(knex) { // Add restore-related settings to app_settings const restoreSettings = [ { + // Default ON so fresh installs can recover from disaster + // without the catch-22 documented in _restoreSettingsBoot.js + // (fresh-install admin user trips the "1 active admin" warning, + // which can only be overridden with force=true, which the wizard + // refused if this setting was false — exactly the moment an + // admin can least afford a SQL incantation). Flipped from false + // to true 2026-05-30. Existing installs that ran this migration + // with the OLD value will get auto-upgraded once by the boot + // self-heal in _restoreSettingsBoot.js — see the + // `restore_allow_force_auto_upgraded` guard there. setting_key: 'restore_allow_force', - setting_value: JSON.stringify(false), + setting_value: JSON.stringify(true), setting_type: 'restore' }, { diff --git a/backend/src/routes/adminRestore.js b/backend/src/routes/adminRestore.js index 305c6b91..0042608c 100644 --- a/backend/src/routes/adminRestore.js +++ b/backend/src/routes/adminRestore.js @@ -718,17 +718,23 @@ async function getRestoreSettings() { const settings = await db('app_settings') .where('setting_type', 'restore') .select('setting_key', 'setting_value'); - + const result = {}; settings.forEach(setting => { - // Convert boolean strings to actual booleans - if (setting.setting_value === '1' || setting.setting_value === '0') { - result[setting.setting_key] = setting.setting_value === '1'; + // Boolean-string normalization. Historically only handled '1'/'0', + // but other code paths (boot self-heal, admin UI, direct SQL) write + // 'true' / 'false' or JSON-encoded "true" / "false". Accept all four + // shapes so `!settings.` evaluates correctly downstream. + const raw = setting.setting_value; + if (raw === '1' || raw === 'true' || raw === '"true"') { + result[setting.setting_key] = true; + } else if (raw === '0' || raw === 'false' || raw === '"false"') { + result[setting.setting_key] = false; } else { - result[setting.setting_key] = setting.setting_value; + result[setting.setting_key] = raw; } }); - + return result; } diff --git a/backend/src/services/_restoreSettingsBoot.js b/backend/src/services/_restoreSettingsBoot.js index 592d258e..c7965dc8 100644 --- a/backend/src/services/_restoreSettingsBoot.js +++ b/backend/src/services/_restoreSettingsBoot.js @@ -57,6 +57,27 @@ const SEEDS = [ }, ]; +/** + * Installs that ran migration 032 BEFORE the 2026-05-30 in-place edit + * have a `restore_allow_force` row with the deprecated `false` default + * (literal string `'false'` from `JSON.stringify(false)`). Per + * [[feedback_self_heal_pattern]] knex won't re-run the corrected + * migration on those installs, so we have to bump the row to `true` + * here ONCE at boot. + * + * The bump is guarded by a tracking row `restore_allow_force_auto_upgraded` + * so we don't fight an admin who explicitly disables force later: + * - Tracking row absent → bump if the value is the deprecated `'false'` + * - Tracking row present → never touch `restore_allow_force` again + * + * The bump applies ONLY when the existing value EXACTLY equals the old + * migration default. Any other value (`'true'`, admin-set anything, + * empty, null) is left alone — those reflect either the fixed + * migration's output or a deliberate admin choice. + */ +const DEPRECATED_DEFAULT_VALUE = 'false'; +const AUTO_UPGRADE_FLAG_KEY = 'restore_allow_force_auto_upgraded'; + let booted = false; /** @@ -64,24 +85,27 @@ let booted = false; * * @param {object} db knex instance * @param {object} logger app logger (must expose .info / .warn) - * @returns {Promise<{ seeded: string[] }>} + * @returns {Promise<{ seeded: string[], upgraded: string[] }>} */ async function seedRestoreSettingsAtBoot(db, logger) { const log = logger || { info: () => {}, warn: () => {} }; - if (booted) return { seeded: [] }; + if (booted) return { seeded: [], upgraded: [] }; if (!(await db.schema.hasTable('app_settings'))) { log.warn('app_settings table missing at boot — restore-settings self-heal skipped'); - return { seeded: [] }; + return { seeded: [], upgraded: [] }; } const seeded = []; + const upgraded = []; + + // Step 1: fresh-install seeding. Insert rows that don't exist at all. for (const seed of SEEDS) { try { const existing = await db('app_settings') .where('setting_key', seed.setting_key) .first(); - if (existing) continue; // admin policy already in effect + if (existing) continue; await db('app_settings').insert({ setting_key: seed.setting_key, @@ -90,14 +114,54 @@ async function seedRestoreSettingsAtBoot(db, logger) { updated_at: new Date(), }); seeded.push(seed.setting_key); - log.info(`Seeded restore-meta setting ${seed.setting_key}=${seed.setting_value} (${seed.rationale.slice(0, 80)}...)`); + log.info(`Seeded restore-meta setting ${seed.setting_key}=${seed.setting_value}`); } catch (err) { log.warn(`Failed to seed restore-meta setting ${seed.setting_key}: ${err.message}`); } } + // Step 2: one-time auto-upgrade for installs that ran the OLD + // migration 032 (which seeded restore_allow_force='false'). Bump + // to 'true' iff the value is still the deprecated default AND the + // auto-upgrade tracking flag hasn't already been set. + try { + const guard = await db('app_settings') + .where('setting_key', AUTO_UPGRADE_FLAG_KEY) + .first(); + + if (!guard) { + const row = await db('app_settings') + .where('setting_key', 'restore_allow_force') + .first(); + + if (row && row.setting_value === DEPRECATED_DEFAULT_VALUE) { + await db('app_settings') + .where('setting_key', 'restore_allow_force') + .update({ + setting_value: 'true', + updated_at: new Date(), + }); + upgraded.push('restore_allow_force'); + log.info('Auto-upgraded restore_allow_force from deprecated migration-032 default \'false\' to \'true\' ' + + '(fresh-install disaster recovery now works without SQL incantation)'); + } + + // Always set the guard, even if no upgrade happened — prevents + // the bump from firing later if an admin sets the value to + // false on purpose. + await db('app_settings').insert({ + setting_key: AUTO_UPGRADE_FLAG_KEY, + setting_value: 'true', + setting_type: 'restore', + updated_at: new Date(), + }); + } + } catch (err) { + log.warn(`restore_allow_force auto-upgrade failed: ${err.message}`); + } + booted = true; - return { seeded }; + return { seeded, upgraded }; } // Test-only: reset the module-level boot flag so jest can re-exercise