fix(db): honor autoBackupEnabled setting for pre-write backups (#5871) (#5888)

This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-07-02 02:46:36 -03:00
committed by GitHub
parent 211e05ad41
commit 9dceb6289b
3 changed files with 188 additions and 0 deletions

View File

@@ -46,6 +46,8 @@
### 🔧 Bug Fixes
- **db (auto backup toggle):** full pre-write SQLite backups now honor the persisted `backup.autoBackupEnabled` dashboard setting — previously only the `DISABLE_SQLITE_AUTO_BACKUP` env var was checked, so disabling auto-backup in the UI had no effect and ~70MB pre-write snapshots kept firing. Manual and pre-restore backups still always run. Regression guard: `tests/unit/db-backup-autobackup-setting-5871.test.ts`. ([#5871](https://github.com/diegosouzapw/OmniRoute/issues/5871))
- **providers (auto/ routing for custom providers):** custom OpenAI-/Anthropic-compatible providers (dynamic `*-compatible-*` connection IDs) are no longer excluded from `auto/` routing — the Auto-Combo virtual factory previously skipped any connection whose provider was absent from the static registry. It now falls back to the connection's `defaultModel`. Regression guard: `tests/unit/auto-custom-provider-5873.test.ts`. ([#5873](https://github.com/diegosouzapw/OmniRoute/issues/5873))
- **middleware (hook sandbox):** operator-authored pre-request hook code now runs inside a hardened Node `vm` sandbox (minimal context, no ambient globals/`process.env`, execution timeout, no `require`) instead of `new Function()` in the main process — closing the Hard Rule #3 / SonarCloud S1523 exposure. Regression guard: `tests/unit/middleware-hook-sandbox-5872.test.ts`. ([#5872](https://github.com/diegosouzapw/OmniRoute/issues/5872))

View File

@@ -211,6 +211,102 @@ export function cleanupDbBackups(options?: { maxFiles?: number; retentionDays?:
};
}
function coerceBoolean(value: unknown): boolean | null {
if (typeof value === "boolean") return value;
if (typeof value === "string") {
const v = value.trim().toLowerCase();
if (v === "true" || v === "1") return true;
if (v === "false" || v === "0") return false;
}
return null;
}
function parseStoredJson(value: string | undefined): unknown {
if (value === undefined) return undefined;
try {
return JSON.parse(value);
} catch {
return value;
}
}
/**
* #5871: resolve the persisted `backup.autoBackupEnabled` dashboard setting.
*
* This mirrors the precedence used by `databaseSettings.getUserDatabaseSettings()`
* (default `false` → `settings.databaseSettings.backup` → `settings.backup` →
* `databaseSettings` namespace, last wins) but reads `key_value` rows directly so
* `backup.ts` does not create a static import cycle with `databaseSettings.ts`
* (which imports `backupDbFile` at module load). Returns `true` when auto backups
* are explicitly disabled by the operator, so callers can skip non-manual backups.
*/
export function isAutoBackupDisabledBySetting(): boolean {
try {
const db = getDbInstance();
const rows = db
.prepare("SELECT namespace, key, value FROM key_value WHERE namespace IN (?, ?)")
.all("settings", "databaseSettings") as Array<{
namespace: string;
key: string;
value: string;
}>;
// Candidate values by source, resolved regardless of DB row iteration order.
// Precedence (lowest → highest, last wins), mirroring getUserDatabaseSettings():
// settings.databaseSettings.backup → settings.backup → databaseSettings namespace.
let fromSettingsNested: boolean | null = null; // settings.databaseSettings.backup.autoBackupEnabled
let fromSettingsBackup: boolean | null = null; // settings.backup.autoBackupEnabled
let fromDbFlat: boolean | null = null; // databaseSettings namespace flat alias "autoBackupEnabled"
let fromDbNested: boolean | null = null; // databaseSettings namespace nested "backup.autoBackupEnabled"
for (const row of rows) {
const parsed = parseStoredJson(row.value);
if (row.namespace === "settings") {
if (row.key === "databaseSettings" && isPlainObject(parsed)) {
const backup = (parsed as Record<string, unknown>).backup;
if (isPlainObject(backup)) {
const b = coerceBoolean((backup as Record<string, unknown>).autoBackupEnabled);
if (b !== null) fromSettingsNested = b;
}
} else if (row.key === "backup" && isPlainObject(parsed)) {
const b = coerceBoolean((parsed as Record<string, unknown>).autoBackupEnabled);
if (b !== null) fromSettingsBackup = b;
}
} else if (row.namespace === "databaseSettings") {
if (row.key === "autoBackupEnabled") {
const b = coerceBoolean(parsed);
if (b !== null) fromDbFlat = b;
} else if (row.key === "backup.autoBackupEnabled") {
const b = coerceBoolean(parsed);
if (b !== null) fromDbNested = b;
}
}
}
// Apply precedence: last non-null wins (mirrors getUserDatabaseSettings — flat alias
// first, then nested key). Default (no persisted value) → not disabled.
let enabled: boolean | null = null;
for (const candidate of [
fromSettingsNested,
fromSettingsBackup,
fromDbFlat,
fromDbNested,
]) {
if (candidate !== null) enabled = candidate;
}
return enabled === false;
} catch {
// If the setting cannot be read (e.g. DB not ready), do not block backups.
return false;
}
}
function isPlainObject(value: unknown): value is Record<string, unknown> {
return typeof value === "object" && value !== null && !Array.isArray(value);
}
function isSqliteAutoBackupDisabled() {
const isTest =
typeof process !== "undefined" &&
@@ -260,6 +356,11 @@ export function backupDbFile(reason = "auto") {
if (isBuildPhase || isCloud) return null;
if (!SQLITE_FILE || !fs.existsSync(SQLITE_FILE)) return null;
if (reason !== "manual" && isSqliteAutoBackupDisabled()) return null;
// #5871: honor the persisted `backup.autoBackupEnabled` dashboard toggle. Only
// manual and pre-restore backups bypass this gate; automatic + pre-write safety
// snapshots must stop firing once the operator disables auto-backup in the UI.
if (reason !== "manual" && reason !== "pre-restore" && isAutoBackupDisabledBySetting())
return null;
const stat = fs.statSync(SQLITE_FILE);
if (stat.size < 4096) {

View File

@@ -0,0 +1,85 @@
/**
* Regression guard for #5871.
*
* Full pre-write SQLite backups (~70MB) must honor the persisted
* `backup.autoBackupEnabled` dashboard setting, not only the
* `DISABLE_SQLITE_AUTO_BACKUP` env var. Before the fix, disabling auto-backup in
* the UI had no effect and pre-write snapshots kept firing (bounded only by the
* 60-minute throttle).
*
* NOTE: `isSqliteAutoBackupDisabled()` short-circuits to `true` under the test
* runner, so `backupDbFile()` always returns null in tests regardless of the new
* gate. We therefore exercise the setting-gate logic directly via the exported
* `isAutoBackupDisabledBySetting()` helper, which is exactly what `backupDbFile()`
* consults for non-manual / non-pre-restore reasons.
*/
import test from "node:test";
import assert from "node:assert/strict";
import fs from "node:fs";
import os from "node:os";
import path from "node:path";
const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-backup-5871-"));
process.env.DATA_DIR = TEST_DATA_DIR;
process.env.DISABLE_SQLITE_AUTO_BACKUP = "true";
const core = await import("../../src/lib/db/core.ts");
const backup = await import("../../src/lib/db/backup.ts");
const databaseSettings = await import("../../src/lib/db/databaseSettings.ts");
function resetStorage() {
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
}
test.beforeEach(() => {
resetStorage();
core.getDbInstance();
});
test.after(() => {
core.resetDbInstance();
fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
});
test("fresh install (seeded default autoBackupEnabled=false) → auto backups disabled", () => {
// The DB seed persists databaseSettings.autoBackupEnabled=false by default, so the
// dashboard toggle reads "off" out of the box — the gate must honor that.
assert.equal(databaseSettings.getUserDatabaseSettings().backup.autoBackupEnabled, false);
assert.equal(backup.isAutoBackupDisabledBySetting(), true);
});
test("no persisted value at all → auto backups are NOT disabled (backups allowed)", () => {
// Remove every persisted autoBackupEnabled row to exercise the "absent" branch.
const db = core.getDbInstance();
db.prepare("DELETE FROM key_value WHERE key IN (?, ?)").run(
"autoBackupEnabled",
"backup.autoBackupEnabled"
);
db.prepare("DELETE FROM key_value WHERE namespace='settings' AND key IN (?, ?)").run(
"backup",
"databaseSettings"
);
assert.equal(backup.isAutoBackupDisabledBySetting(), false);
});
test("autoBackupEnabled=false → auto backups are disabled (gate trips)", () => {
databaseSettings.updateDatabaseSettings({ backup: { autoBackupEnabled: false } });
assert.equal(backup.isAutoBackupDisabledBySetting(), true);
});
test("autoBackupEnabled=true → auto backups are NOT disabled", () => {
databaseSettings.updateDatabaseSettings({ backup: { autoBackupEnabled: true } });
assert.equal(backup.isAutoBackupDisabledBySetting(), false);
});
test("persisted value survives a getUserDatabaseSettings round-trip", () => {
databaseSettings.updateDatabaseSettings({ backup: { autoBackupEnabled: false } });
assert.equal(databaseSettings.getUserDatabaseSettings().backup.autoBackupEnabled, false);
assert.equal(backup.isAutoBackupDisabledBySetting(), true);
databaseSettings.updateDatabaseSettings({ backup: { autoBackupEnabled: true } });
assert.equal(databaseSettings.getUserDatabaseSettings().backup.autoBackupEnabled, true);
assert.equal(backup.isAutoBackupDisabledBySetting(), false);
});