fix(persistence): preserve authoritative state on write failures
Keep SQLite authoritative after completed migration even when legacy JSON is later invalid, reject non-object config documents before any migration state is written, and guard async secret masking by input generation. Persist renderer-facing config and queue mutations before updating memory so SQLite errors reject IPC calls and retain the last durable queue snapshot.
This commit is contained in:
@@ -181,6 +181,42 @@ describe('migrateJsonToSqlite', () => {
|
||||
expect(JSON.parse(language!.value)).toBe('de');
|
||||
});
|
||||
|
||||
test('keeps SQLite config queue and secrets authoritative after a corrupted legacy config restart', () => {
|
||||
const configPath = writeJson('config.json', { language: 'de', client_secret: 'persisted-secret' });
|
||||
writeJson('download_queue.json', [{ id: 'q1', status: 'pending', title: 'Persisted queue item' }]);
|
||||
let secrets = createSecretStore(db, new MemorySecureStorage());
|
||||
|
||||
migrateJsonToSqlite({ db, appDataDir, secrets });
|
||||
db.close();
|
||||
db = openDatabase(path.join(tmpDir, 'app.db'));
|
||||
secrets = createSecretStore(db, new MemorySecureStorage());
|
||||
fs.writeFileSync(configPath, '{ no longer valid JSON', 'utf-8');
|
||||
|
||||
const restart = migrateJsonToSqlite({ db, appDataDir, secrets });
|
||||
|
||||
expect(restart).toMatchObject({ alreadyApplied: true, errors: [] });
|
||||
expect(JSON.parse(db.get<{ value: string }>('SELECT value FROM config_kv WHERE key = ?', ['language'])!.value)).toBe('de');
|
||||
expect(db.all<{ id: string }>('SELECT id FROM queue_items ORDER BY queue_position')).toEqual([{ id: 'q1' }]);
|
||||
expect(secrets.get('twitch_client_secret')).toBe('persisted-secret');
|
||||
});
|
||||
|
||||
test.each([
|
||||
['null', null],
|
||||
['false', false],
|
||||
['zero', 0],
|
||||
['empty string', ''],
|
||||
])('does not mark or partially migrate a syntactically valid but invalid %s config', (_, invalidConfig) => {
|
||||
writeJson('config.json', invalidConfig);
|
||||
writeJson('download_queue.json', [{ id: 'q1', status: 'pending' }]);
|
||||
|
||||
const result = migrateJsonToSqlite({ db, appDataDir });
|
||||
|
||||
expect(result.errors).toEqual([{ source: 'config.json', message: 'Config JSON must be an object' }]);
|
||||
expect(db.all('SELECT * FROM config_kv')).toEqual([]);
|
||||
expect(db.all('SELECT * FROM queue_items')).toEqual([]);
|
||||
expect(db.get('SELECT name FROM migrations_applied WHERE name = ?', ['authoritative-state-v1'])).toBeUndefined();
|
||||
});
|
||||
|
||||
test('does not let the former shadow-migration marker skip authoritative secret import', () => {
|
||||
const configPath = writeJson('config.json', { language: 'de', client_secret: 'legacy-secret' });
|
||||
db.run('INSERT INTO migrations_applied(name, payload) VALUES (?, ?)', ['v4-to-v5-jsons', '{}']);
|
||||
|
||||
@@ -82,11 +82,6 @@ export function migrateJsonToSqlite(opts: MigratorOptions): MigrationResult {
|
||||
const queuePath = path.join(appDataDir, 'download_queue.json');
|
||||
const existing = db.get<{ name: string }>('SELECT name FROM migrations_applied WHERE name = ?', [MIGRATION_NAME]);
|
||||
if (existing) {
|
||||
try {
|
||||
scrubExistingConfig(configPath);
|
||||
} catch (error) {
|
||||
return emptyResult(true, [{ source: 'config.json', message: error instanceof Error ? error.message : String(error) }]);
|
||||
}
|
||||
return emptyResult(true);
|
||||
}
|
||||
|
||||
@@ -99,7 +94,7 @@ export function migrateJsonToSqlite(opts: MigratorOptions): MigrationResult {
|
||||
errors.push({ source: 'download_queue.json', message: 'Queue JSON must be an array' });
|
||||
}
|
||||
if (errors.length > 0) return emptyResult(false, errors);
|
||||
if (config && (typeof config !== 'object' || Array.isArray(config))) {
|
||||
if (configExists && (!config || typeof config !== 'object' || Array.isArray(config))) {
|
||||
return emptyResult(false, [{ source: 'config.json', message: 'Config JSON must be an object' }]);
|
||||
}
|
||||
if (config && !secrets && [...SECRET_KEYS].some((key) => typeof config[key] === 'string' && config[key])) {
|
||||
|
||||
@@ -0,0 +1,62 @@
|
||||
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||
import * as fs from 'node:fs';
|
||||
import * as os from 'node:os';
|
||||
import * as path from 'node:path';
|
||||
import { openDatabase, type DbHandle } from '../infra/db';
|
||||
import { createAppStateStore } from './app-state-store';
|
||||
import { persistStateChange } from './persistence-commit';
|
||||
|
||||
let directory: string;
|
||||
let db: DbHandle;
|
||||
|
||||
beforeEach(() => {
|
||||
directory = fs.mkdtempSync(path.join(os.tmpdir(), 'persistence-commit-'));
|
||||
db = openDatabase(path.join(directory, 'app.db'));
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
db.close();
|
||||
fs.rmSync(directory, { recursive: true, force: true });
|
||||
});
|
||||
|
||||
describe('persistStateChange', () => {
|
||||
it('keeps runtime configuration at the persisted value when a SQLite write fails', () => {
|
||||
const previous = { language: 'de' };
|
||||
const next = { language: 'en' };
|
||||
createAppStateStore(db).saveConfig(previous);
|
||||
const failingDb: DbHandle = {
|
||||
...db,
|
||||
run(sql, params) {
|
||||
if (sql.includes('INSERT INTO config_kv')) throw new Error('SQLITE_IOERR config');
|
||||
db.run(sql, params);
|
||||
},
|
||||
};
|
||||
let runtime = previous;
|
||||
|
||||
expect(() => {
|
||||
runtime = persistStateChange(runtime, () => next, (candidate) => createAppStateStore(failingDb).saveConfig(candidate));
|
||||
}).toThrow('SQLITE_IOERR config');
|
||||
expect(runtime).toEqual(previous);
|
||||
expect(createAppStateStore(db).loadConfig()).toMatchObject(previous);
|
||||
});
|
||||
|
||||
it('keeps runtime queue at the persisted snapshot when a SQLite write fails', () => {
|
||||
const previous = [{ id: 'q1', status: 'pending' }];
|
||||
const next = [{ id: 'q2', status: 'pending' }];
|
||||
createAppStateStore(db).saveQueue(previous);
|
||||
const failingDb: DbHandle = {
|
||||
...db,
|
||||
run(sql, params) {
|
||||
if (sql.includes('INSERT OR REPLACE INTO queue_items')) throw new Error('SQLITE_IOERR queue');
|
||||
db.run(sql, params);
|
||||
},
|
||||
};
|
||||
let runtime = previous;
|
||||
|
||||
expect(() => {
|
||||
runtime = persistStateChange(runtime, () => next, (candidate) => createAppStateStore(failingDb).saveQueue(candidate));
|
||||
}).toThrow('SQLITE_IOERR queue');
|
||||
expect(runtime).toEqual(previous);
|
||||
expect(createAppStateStore(db).loadQueue()).toEqual(previous);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,5 @@
|
||||
export function persistStateChange<T>(current: T, createNext: (current: T) => T, persist: (next: T) => void): T {
|
||||
const next = createNext(current);
|
||||
persist(next);
|
||||
return next;
|
||||
}
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { resolveSecretInputUpdate } from './secret-input';
|
||||
import { createSecretInputRevision, isSecretInputRevisionCurrent, resolveSecretInputUpdate } from './secret-input';
|
||||
|
||||
describe('resolveSecretInputUpdate', () => {
|
||||
it('keeps a configured secret when the masked value is unchanged', () => {
|
||||
@@ -17,4 +17,23 @@ describe('resolveSecretInputUpdate', () => {
|
||||
it('ignores an empty field when no secret is configured', () => {
|
||||
expect(resolveSecretInputUpdate('', false)).toEqual({ action: 'unchanged' });
|
||||
});
|
||||
|
||||
it('does not apply a completed save mask after a newer secret input arrives', async () => {
|
||||
const revision = createSecretInputRevision();
|
||||
const requestRevision = revision.current();
|
||||
let resolveSave: (() => void) | undefined;
|
||||
let visibleValue = 'first-secret';
|
||||
const save = new Promise<void>((resolve) => {
|
||||
resolveSave = resolve;
|
||||
}).then(() => {
|
||||
if (isSecretInputRevisionCurrent(revision, requestRevision)) visibleValue = '••••••••';
|
||||
});
|
||||
|
||||
revision.advance();
|
||||
visibleValue = 'second-secret';
|
||||
resolveSave?.();
|
||||
await save;
|
||||
|
||||
expect(visibleValue).toBe('second-secret');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -5,6 +5,27 @@ export type SecretInputUpdate =
|
||||
|
||||
export const SECRET_INPUT_MASK = '••••••••';
|
||||
|
||||
export interface SecretInputRevision {
|
||||
current(): number;
|
||||
advance(): void;
|
||||
}
|
||||
|
||||
export function createSecretInputRevision(): SecretInputRevision {
|
||||
let value = 0;
|
||||
return {
|
||||
current() {
|
||||
return value;
|
||||
},
|
||||
advance() {
|
||||
value += 1;
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
export function isSecretInputRevisionCurrent(revision: SecretInputRevision, value: number): boolean {
|
||||
return revision.current() === value;
|
||||
}
|
||||
|
||||
export function resolveSecretInputUpdate(value: string, configured: boolean): SecretInputUpdate {
|
||||
if (configured && value === SECRET_INPUT_MASK) return { action: 'unchanged' };
|
||||
const normalized = value.trim();
|
||||
|
||||
Reference in New Issue
Block a user