From 6f02e9aa3a96fd21654a075ecf8ce93e75ac53a8 Mon Sep 17 00:00:00 2001 From: Sucukdeluxe <259325684+Sucukdeluxe@users.noreply.github.com> Date: Wed, 12 Aug 2026 00:21:55 +0200 Subject: [PATCH] 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. --- src/main.ts | 189 +++++++++------------ src/main/domain/migrator.test.ts | 36 ++++ src/main/domain/migrator.ts | 7 +- src/main/domain/persistence-commit.test.ts | 62 +++++++ src/main/domain/persistence-commit.ts | 5 + src/main/domain/secret-input.test.ts | 21 ++- src/main/domain/secret-input.ts | 21 +++ src/renderer-settings.ts | 72 +++++--- 8 files changed, 275 insertions(+), 138 deletions(-) create mode 100644 src/main/domain/persistence-commit.test.ts create mode 100644 src/main/domain/persistence-commit.ts diff --git a/src/main.ts b/src/main.ts index 1dc4708..7b8efe6 100644 --- a/src/main.ts +++ b/src/main.ts @@ -52,6 +52,7 @@ import { registerTrustedIpcHandler } from './main/domain/privileged-ipc'; import { createRendererQueueItem, getMergeGroupCleanupPaths } from './main/domain/renderer-queue-input'; import { createAppStateStore, type AppStateStore } from './main/domain/app-state-store'; import { createExportableConfig } from './main/domain/config-export'; +import { persistStateChange } from './main/domain/persistence-commit'; import { resolveSecretInputUpdate } from './main/domain/secret-input'; import { createSecretStore, type SecretStore } from './main/domain/secret-store'; import { createElectronSecureStorage } from './main/infra/secure-storage'; @@ -456,17 +457,15 @@ function normalizeConfigTemplates(input: Config): Config { function recordDownloadedVodId(vodId: string): void { if (!vodId) return; - if (!Array.isArray(config.downloaded_vod_ids)) config.downloaded_vod_ids = []; - if (config.downloaded_vod_ids.includes(vodId)) return; - config.downloaded_vod_ids.push(vodId); - // Cap to keep config size bounded — drop oldest first. + const downloadedVodIds = Array.isArray(config.downloaded_vod_ids) ? config.downloaded_vod_ids : []; + if (downloadedVodIds.includes(vodId)) return; const DOWNLOADED_IDS_MAX = 4096; - if (config.downloaded_vod_ids.length > DOWNLOADED_IDS_MAX) { - config.downloaded_vod_ids = config.downloaded_vod_ids.slice( - config.downloaded_vod_ids.length - DOWNLOADED_IDS_MAX - ); - } - saveConfig(config); + const nextDownloadedVodIds = [...downloadedVodIds, vodId].slice(-DOWNLOADED_IDS_MAX); + config = persistStateChange(config, (current) => ({ ...current, downloaded_vod_ids: nextDownloadedVodIds }), saveConfig); +} + +function cloneConfig(value: Config): Config { + return JSON.parse(JSON.stringify(value)) as Config; } function loadConfig(): Config { @@ -481,12 +480,14 @@ function loadConfig(): Config { return normalizeConfigTemplates(defaultConfig); } -function saveConfig(config: Config): void { +function saveConfig(nextConfig: Config): void { + if (!appStateStore) throw new Error('Application state store is unavailable'); try { - if (!appStateStore) throw new Error('Application state store is unavailable'); - appStateStore.saveConfig(config); - } catch (e) { - console.error('Error saving config:', e); + appStateStore.saveConfig(nextConfig); + lastPersistedConfig = cloneConfig(nextConfig); + } catch (error) { + if (nextConfig === config) config = cloneConfig(lastPersistedConfig); + throw error; } } @@ -635,12 +636,14 @@ function loadQueue(): QueueItem[] { let queueSaveTimer: NodeJS.Timeout | null = null; let pendingQueueSnapshot: QueueItem[] | null = null; +function cloneQueue(queue: QueueItem[]): QueueItem[] { + return JSON.parse(JSON.stringify(queue)) as QueueItem[]; +} + function clearQueueFileFromDisk(): void { - try { - appStateStore?.saveQueue([]); - } catch (e) { - console.error('Error clearing queue file:', e); - } + if (!appStateStore) throw new Error('Application state store is unavailable'); + appStateStore.saveQueue([]); + lastPersistedQueueSnapshot = []; } function writeQueueToDisk(queue: QueueItem[]): void { @@ -649,26 +652,14 @@ function writeQueueToDisk(queue: QueueItem[]): void { return; } - try { - if (!appStateStore) throw new Error('Application state store is unavailable'); - appStateStore.saveQueue(queue); - } catch (e) { - console.error('Error saving queue:', e); - } + if (!appStateStore) throw new Error('Application state store is unavailable'); + appStateStore.saveQueue(queue); + lastPersistedQueueSnapshot = cloneQueue(queue); } function saveQueue(queue: QueueItem[], force = false): void { - if (config.persist_queue_on_restart === false) { - pendingQueueSnapshot = null; - if (queueSaveTimer) { - clearTimeout(queueSaveTimer); - queueSaveTimer = null; - } - clearQueueFileFromDisk(); - return; - } - - pendingQueueSnapshot = queue; + const snapshot = cloneQueue(queue); + pendingQueueSnapshot = snapshot; if (appShutdownStarted && !force) { if (queueSaveTimer) { @@ -678,28 +669,19 @@ function saveQueue(queue: QueueItem[], force = false): void { return; } - if (force) { - if (queueSaveTimer) { - clearTimeout(queueSaveTimer); - queueSaveTimer = null; - } - - writeQueueToDisk(pendingQueueSnapshot); - pendingQueueSnapshot = null; - return; - } - if (queueSaveTimer) { - return; + clearTimeout(queueSaveTimer); + queueSaveTimer = null; } - queueSaveTimer = setTimeout(() => { - queueSaveTimer = null; - if (pendingQueueSnapshot) { - writeQueueToDisk(pendingQueueSnapshot); - pendingQueueSnapshot = null; - } - }, QUEUE_SAVE_DEBOUNCE_MS); + try { + writeQueueToDisk(snapshot); + pendingQueueSnapshot = null; + } catch (error) { + pendingQueueSnapshot = null; + if (queue === downloadQueue) downloadQueue = cloneQueue(lastPersistedQueueSnapshot); + throw error; + } } function flushQueueSave(): void { @@ -731,10 +713,12 @@ function startDevelopmentReload(): void { let appStateStore: AppStateStore | null = null; let appSecretStore: SecretStore | null = null; let config = normalizeConfigTemplates(defaultConfig); +let lastPersistedConfig = cloneConfig(config); let twitchClientSecret = ''; let discordWebhookUrl = ''; let accessToken: string | null = null; let downloadQueue: QueueItem[] = []; +let lastPersistedQueueSnapshot: QueueItem[] = []; let queueIdCounter = 0; let lastQueueBroadcastFingerprint = ''; let isDownloading = false; @@ -7289,7 +7273,8 @@ ipcMain.handle('save-config', (event, newConfig: Partial, fileCapability delete acceptedConfig.download_path; } } - config = normalizeConfigTemplates({ ...config, ...acceptedConfig }); + const nextConfig = normalizeConfigTemplates({ ...config, ...acceptedConfig }); + config = persistStateChange(config, () => nextConfig, saveConfig); if (config.client_id !== previousClientId) { accessToken = null; @@ -7304,8 +7289,6 @@ ipcMain.handle('save-config', (event, newConfig: Partial, fileCapability nativeTheme.themeSource = config.theme === 'light' ? 'light' : 'dark'; } - saveConfig(config); - if (config.persist_queue_on_restart === false) { pendingQueueSnapshot = null; if (queueSaveTimer) { @@ -7408,8 +7391,7 @@ ipcMain.handle('start-live-recording', async (event, streamerName: string) => { return { success: false, error: 'ALREADY_RECORDING', streamer: login }; } - downloadQueue.push(liveItem); - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, (current) => [...current, liveItem], saveQueue); emitQueueUpdated(); if (!isDownloading) scheduleQueueProcessing(); appendDebugLog('live-recording-queued', { streamer: login, title: liveItem.title }); @@ -7434,8 +7416,7 @@ registerTrustedIpcHandler(ipcMain, 'add-to-queue', isTrustedRendererEvent, () => return downloadQueue; } - downloadQueue.push(item); - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, (current) => [...current, item], saveQueue); emitQueueUpdated(); return downloadQueue; }); @@ -7461,16 +7442,14 @@ registerTrustedIpcHandler(ipcMain, 'remove-from-queue', isTrustedRendererEvent, try { if (fs.existsSync(cleanupPath)) fs.unlinkSync(cleanupPath); } catch { } } - downloadQueue = downloadQueue.filter(item => item.id !== id); - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, (current) => current.filter((item) => item.id !== id), saveQueue); emitQueueUpdated(); return downloadQueue; }); ipcMain.handle('clear-completed', (event) => { if (!isTrustedRendererEvent(event)) return downloadQueue; - downloadQueue = downloadQueue.filter(item => item.status !== 'completed'); - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, (current) => current.filter((item) => item.status !== 'completed'), saveQueue); emitQueueUpdated(); return downloadQueue; }); @@ -7484,8 +7463,7 @@ ipcMain.handle('reorder-queue', (event, orderIds: string[]) => { return ai - bi; }); - downloadQueue = withOrder; - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, () => withOrder, saveQueue); emitQueueUpdated(); return downloadQueue; }); @@ -7495,18 +7473,18 @@ ipcMain.handle('retry-failed-downloads', async (event) => { const failedIds = downloadQueue.filter((item) => item.status === 'error').map((item) => item.id); await Promise.all(failedIds.map((id) => queueProcessRegistry.cancelItem(id))); for (const id of failedIds) queueProcessRegistry.resetItem(id); - downloadQueue = downloadQueue.map((item) => { + const nextQueue: QueueItem[] = downloadQueue.map((item) => { if (item.status !== 'error') return item; return { ...item, - status: 'pending', + status: 'pending' as const, progress: 0, last_error: '' }; }); - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue); emitQueueUpdated(); if (!isDownloading) { @@ -7527,14 +7505,12 @@ ipcMain.handle('retry-queue-item', async (event, id: string) => { await queueProcessRegistry.cancelItem(id); queueProcessRegistry.resetItem(id); - downloadQueue[idx] = { - ...item, + downloadQueue = persistStateChange(downloadQueue, (current) => current.map((candidate) => candidate.id === id ? { + ...candidate, status: 'pending', progress: 0, last_error: '' - }; - - saveQueue(downloadQueue); + } : candidate), saveQueue); emitQueueUpdated(); appendDebugLog('queue-item-retry-single', { id, title: item.title }); @@ -7615,10 +7591,9 @@ ipcMain.handle('create-merge-group', (event, itemIds: string[]) => { const firstIndex = downloadQueue.findIndex(item => itemIds.includes(item.id)); // Remove selected items and insert merged item at first position - downloadQueue = downloadQueue.filter(item => !itemIds.includes(item.id)); - downloadQueue.splice(firstIndex >= 0 ? Math.min(firstIndex, downloadQueue.length) : downloadQueue.length, 0, mergedItem); - - saveQueue(downloadQueue); + const nextQueue = downloadQueue.filter((item) => !itemIds.includes(item.id)); + nextQueue.splice(firstIndex >= 0 ? Math.min(firstIndex, nextQueue.length) : nextQueue.length, 0, mergedItem); + downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue); emitQueueUpdated(); return downloadQueue; }); @@ -7626,26 +7601,24 @@ ipcMain.handle('create-merge-group', (event, itemIds: string[]) => { ipcMain.handle('start-download', async (event) => { if (!isTrustedRendererEvent(event)) return false; if (isDownloading && queuePaused) { + const nextQueue = downloadQueue.map((item) => item.status === 'paused' ? { ...item, status: 'downloading' as const } : item); + downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue); queuePaused = false; - for (const item of downloadQueue) { - if (item.status === 'paused') item.status = 'downloading'; - } await Promise.all(queueProcessRegistry.activeItemIds().map((id) => queueProcessRegistry.resumeItem(id))); - saveQueue(downloadQueue); emitQueueUpdated(true); mainWindow?.webContents.send('download-started'); return true; } - downloadQueue = downloadQueue.map((item) => item.status === 'paused' ? { ...item, status: 'pending' } : item); + const nextQueue = downloadQueue.map((item) => item.status === 'paused' ? { ...item, status: 'pending' as const } : item); - const hasPendingItems = downloadQueue.some(item => item.status === 'pending'); + const hasPendingItems = nextQueue.some(item => item.status === 'pending'); if (!hasPendingItems) { emitQueueUpdated(); return false; } - saveQueue(downloadQueue); + downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue); emitQueueUpdated(); if (!isDownloading) { @@ -7658,17 +7631,16 @@ ipcMain.handle('pause-download', async (event) => { if (!isTrustedRendererEvent(event)) return false; if (!isDownloading || queuePaused) return false; - queuePaused = true; await Promise.all(queueProcessRegistry.activeItemIds().map((id) => queueProcessRegistry.pauseItem(id))); - for (const item of downloadQueue) { - if (item.status === 'downloading') { - item.status = 'paused'; - item.speed = ''; - item.eta = ''; - item.progressStatus = tBackend('downloadPaused'); - } - } - saveQueue(downloadQueue); + const nextQueue = downloadQueue.map((item) => item.status === 'downloading' ? { + ...item, + status: 'paused' as const, + speed: '', + eta: '', + progressStatus: tBackend('downloadPaused') + } : item); + downloadQueue = persistStateChange(downloadQueue, () => nextQueue, saveQueue); + queuePaused = true; emitQueueUpdated(true); mainWindow?.webContents.send('download-paused'); return true; @@ -8170,16 +8142,17 @@ ipcMain.handle('export-runtime-metrics', async (event) => { ipcMain.handle('mark-vod-downloaded', (event, vodId: string, mark: boolean): { success: boolean } => { if (!isTrustedRendererEvent(event)) return { success: false }; if (typeof vodId !== 'string' || !vodId) return { success: false }; - if (!Array.isArray(config.downloaded_vod_ids)) config.downloaded_vod_ids = []; - const has = config.downloaded_vod_ids.includes(vodId); + const downloadedVodIds = Array.isArray(config.downloaded_vod_ids) ? config.downloaded_vod_ids : []; + const has = downloadedVodIds.includes(vodId); + let nextDownloadedVodIds: string[]; if (mark && !has) { - config.downloaded_vod_ids.push(vodId); + nextDownloadedVodIds = [...downloadedVodIds, vodId]; } else if (!mark && has) { - config.downloaded_vod_ids = config.downloaded_vod_ids.filter((id) => id !== vodId); + nextDownloadedVodIds = downloadedVodIds.filter((id) => id !== vodId); } else { return { success: true }; } - saveConfig(config); + config = persistStateChange(config, (current) => ({ ...current, downloaded_vod_ids: nextDownloadedVodIds }), saveConfig); appendDebugLog('mark-vod-downloaded', { vodId, mark }); return { success: true }; }); @@ -8187,8 +8160,7 @@ ipcMain.handle('mark-vod-downloaded', (event, vodId: string, mark: boolean): { s ipcMain.handle('reset-downloaded-vod-ids', (event) => { if (!isTrustedRendererEvent(event)) return { success: false, removedCount: 0 }; const count = Array.isArray(config.downloaded_vod_ids) ? config.downloaded_vod_ids.length : 0; - config.downloaded_vod_ids = []; - saveConfig(config); + config = persistStateChange(config, (current) => ({ ...current, downloaded_vod_ids: [] }), saveConfig); appendDebugLog('reset-downloaded-vod-ids', { previousCount: count }); return { success: true, removedCount: count }; }); @@ -8254,8 +8226,7 @@ ipcMain.handle('import-config', async (event) => { delete imported.__exportedAt; const merged = normalizeConfigTemplates({ ...config, ...imported } as Config); - config = merged; - saveConfig(config); + config = persistStateChange(config, () => merged, saveConfig); appendDebugLog('config-import-applied', { source: importPath }); return { success: true, filePath: importPath }; } catch (e) { @@ -8481,8 +8452,10 @@ app.whenReady().then(() => { if (result.errors.length > 0) throw new Error(result.errors.map((entry: { source: string; message: string }) => `${entry.source}: ${entry.message}`).join('; ')); appStateStore = createAppStateStore(database); config = loadConfig(); + lastPersistedConfig = cloneConfig(config); downloadQueue = config.persist_queue_on_restart === false ? [] : loadQueue(); if (config.persist_queue_on_restart === false) appStateStore.saveQueue([]); + lastPersistedQueueSnapshot = cloneQueue(downloadQueue); twitchClientSecret = appSecretStore.get('twitch_client_secret') ?? ''; discordWebhookUrl = appSecretStore.get('discord_webhook_url') ?? ''; } catch (e) { @@ -8494,7 +8467,9 @@ app.whenReady().then(() => { appStateStore = null; appSecretStore = null; config = normalizeConfigTemplates(defaultConfig); + lastPersistedConfig = cloneConfig(config); downloadQueue = []; + lastPersistedQueueSnapshot = []; twitchClientSecret = ''; discordWebhookUrl = ''; } diff --git a/src/main/domain/migrator.test.ts b/src/main/domain/migrator.test.ts index 16166a2..047824b 100644 --- a/src/main/domain/migrator.test.ts +++ b/src/main/domain/migrator.test.ts @@ -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', '{}']); diff --git a/src/main/domain/migrator.ts b/src/main/domain/migrator.ts index fcc4eff..e6dfedc 100644 --- a/src/main/domain/migrator.ts +++ b/src/main/domain/migrator.ts @@ -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])) { diff --git a/src/main/domain/persistence-commit.test.ts b/src/main/domain/persistence-commit.test.ts new file mode 100644 index 0000000..36d349e --- /dev/null +++ b/src/main/domain/persistence-commit.test.ts @@ -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); + }); +}); diff --git a/src/main/domain/persistence-commit.ts b/src/main/domain/persistence-commit.ts new file mode 100644 index 0000000..e523988 --- /dev/null +++ b/src/main/domain/persistence-commit.ts @@ -0,0 +1,5 @@ +export function persistStateChange(current: T, createNext: (current: T) => T, persist: (next: T) => void): T { + const next = createNext(current); + persist(next); + return next; +} diff --git a/src/main/domain/secret-input.test.ts b/src/main/domain/secret-input.test.ts index ce4d04d..c663d9e 100644 --- a/src/main/domain/secret-input.test.ts +++ b/src/main/domain/secret-input.test.ts @@ -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((resolve) => { + resolveSave = resolve; + }).then(() => { + if (isSecretInputRevisionCurrent(revision, requestRevision)) visibleValue = '••••••••'; + }); + + revision.advance(); + visibleValue = 'second-secret'; + resolveSave?.(); + await save; + + expect(visibleValue).toBe('second-secret'); + }); }); diff --git a/src/main/domain/secret-input.ts b/src/main/domain/secret-input.ts index d3bef88..a1bff47 100644 --- a/src/main/domain/secret-input.ts +++ b/src/main/domain/secret-input.ts @@ -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(); diff --git a/src/renderer-settings.ts b/src/renderer-settings.ts index f7f1aac..32592a3 100644 --- a/src/renderer-settings.ts +++ b/src/renderer-settings.ts @@ -12,6 +12,11 @@ let secretStatus: SecretStatus = { clientSecretConfigured: false, discordWebhookConfigured: false }; +type SecretInputId = 'clientSecret' | 'discordWebhookUrl'; +const secretInputGenerations: Record = { + clientSecret: 0, + discordWebhookUrl: 0 +}; function canRunSettingsAutoRefresh(): boolean { if (document.hidden) { @@ -586,30 +591,44 @@ function collectCredentialsPayload(): Partial { }; } +function secretConfigured(inputId: SecretInputId): boolean { + return inputId === 'clientSecret' ? secretStatus.clientSecretConfigured : secretStatus.discordWebhookConfigured; +} + +function syncSecretField(inputId: SecretInputId): void { + byId(inputId).value = secretConfigured(inputId) ? SECRET_INPUT_MASK : ''; +} + function syncSecretFields(): void { - byId('clientSecret').value = secretStatus.clientSecretConfigured ? SECRET_INPUT_MASK : ''; - byId('discordWebhookUrl').value = secretStatus.discordWebhookConfigured ? SECRET_INPUT_MASK : ''; + syncSecretField('clientSecret'); + syncSecretField('discordWebhookUrl'); +} + +async function persistSecretInput(inputId: SecretInputId): Promise { + const requestGeneration = secretInputGenerations[inputId]; + const value = byId(inputId).value; + if (value === SECRET_INPUT_MASK) return; + + const configured = secretConfigured(inputId); + let nextStatus = secretStatus; + if (value.trim()) { + nextStatus = inputId === 'clientSecret' + ? await window.api.setClientSecret(value.trim()) + : await window.api.setDiscordWebhook(value.trim()); + } else if (configured) { + nextStatus = inputId === 'clientSecret' + ? await window.api.clearClientSecret() + : await window.api.clearDiscordWebhook(); + } + + if (secretInputGenerations[inputId] !== requestGeneration) return; + secretStatus = nextStatus; + syncSecretField(inputId); } async function persistSecretInputs(): Promise { - const clientValue = byId('clientSecret').value; - if (clientValue !== SECRET_INPUT_MASK) { - secretStatus = clientValue.trim() - ? await window.api.setClientSecret(clientValue.trim()) - : secretStatus.clientSecretConfigured - ? await window.api.clearClientSecret() - : secretStatus; - } - - const webhookValue = byId('discordWebhookUrl').value; - if (webhookValue !== SECRET_INPUT_MASK) { - secretStatus = webhookValue.trim() - ? await window.api.setDiscordWebhook(webhookValue.trim()) - : secretStatus.discordWebhookConfigured - ? await window.api.clearDiscordWebhook() - : secretStatus; - } - syncSecretFields(); + await persistSecretInput('clientSecret'); + await persistSecretInput('discordWebhookUrl'); } function syncPartMinutesFieldState(): void { @@ -725,9 +744,9 @@ function getSettingsFingerprint(payload: Partial): string { ]); } -function syncSettingsFormFromConfig(): void { +function syncSettingsFormFromConfig(syncSecrets = true): void { byId('clientId').value = config.client_id ?? ''; - syncSecretFields(); + if (syncSecrets) syncSecretFields(); byId('sidebarSplitViewToggle').checked = config.sidebar_split_view !== false; applySidebarLayoutPreference(config.sidebar_split_view !== false); byId('downloadMode').value = (config.download_mode as 'parts' | 'full') ?? 'full'; @@ -790,7 +809,7 @@ async function persistSettings(options: { await persistSecretInputs(); config = await window.api.saveConfig(payload); - syncSettingsFormFromConfig(); + syncSettingsFormFromConfig(false); pendingCredentialsReconnect = false; if (options.reconnectAfterSave) { @@ -830,7 +849,6 @@ async function flushSettingsAutoSave(reconnectAfterSave = false): Promise try { await persistSecretInputs(); config = await window.api.saveConfig(payload); - syncSecretFields(); lastPersistedSettingsFingerprint = getSettingsFingerprint({}); if (reconnectAfterSave && pendingCredentialsReconnect) { pendingCredentialsReconnect = false; @@ -936,6 +954,12 @@ function initSettingsAutoSave(): void { }); } + for (const id of ['clientSecret', 'discordWebhookUrl'] as const) { + byId(id).addEventListener('input', () => { + secretInputGenerations[id] += 1; + }); + } + for (const id of ['clientSecret', 'discordWebhookUrl'] as const) { byId(id).addEventListener('focus', (event) => { const input = event.currentTarget as HTMLInputElement;