diff --git a/src/main/browser/browser-cookie-import.test.ts b/src/main/browser/browser-cookie-import.test.ts index 27663017097..55e48f54394 100644 --- a/src/main/browser/browser-cookie-import.test.ts +++ b/src/main/browser/browser-cookie-import.test.ts @@ -442,6 +442,14 @@ describe('importCookiesFromBrowser Chromium', () => { expect( copyFileSyncMock.mock.calls.some(([source]) => source === `${sourceCookiesPath}-wal`) ).toBe(true) + expect( + copyFileSyncMock.mock.calls.some( + ([source, destination]) => + source === sourceCookiesPath && + typeof destination === 'string' && + destination.includes(join('userData', 'cookie-import-snapshots')) + ) + ).toBe(true) expect( ['', '-wal', '-shm'].map((suffix) => readFileSync(sourceCookiesPath + suffix)) ).toEqual(sourceFilesBefore) diff --git a/src/main/browser/browser-cookie-import.ts b/src/main/browser/browser-cookie-import.ts index 3191509d30c..de52cfbdc84 100644 --- a/src/main/browser/browser-cookie-import.ts +++ b/src/main/browser/browser-cookie-import.ts @@ -1522,7 +1522,12 @@ export async function importCookiesFromBrowser( try { // Why: the browser can commit cookies only to WAL while it remains open; // snapshot retries prevent pairing the main DB with a racing WAL generation. - sourceSnapshot = createChromiumCookieSnapshot(browser.cookiesPath) + // Why: snapshot bytes are sensitive — keep them under userData, not os.tmpdir(). + const snapshotTempRoot = join(app.getPath('userData'), 'cookie-import-snapshots') + mkdirSync(snapshotTempRoot, { recursive: true }) + sourceSnapshot = createChromiumCookieSnapshot(browser.cookiesPath, { + tempRoot: snapshotTempRoot + }) } catch (err) { try { unlinkSync(stagingCookiesPath) diff --git a/src/main/browser/chromium-cookie-snapshot.test.ts b/src/main/browser/chromium-cookie-snapshot.test.ts index 16fbd50a63f..db334e81097 100644 --- a/src/main/browser/chromium-cookie-snapshot.test.ts +++ b/src/main/browser/chromium-cookie-snapshot.test.ts @@ -1,7 +1,10 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import type * as NodeFs from 'node:fs' -const { beforeCopyMock } = vi.hoisted(() => ({ beforeCopyMock: vi.fn() })) +const { beforeCopyMock, rmSyncShouldThrow } = vi.hoisted(() => ({ + beforeCopyMock: vi.fn(), + rmSyncShouldThrow: { value: false } +})) vi.mock('node:fs', async (importOriginal) => { const actual = await importOriginal() @@ -10,6 +13,12 @@ vi.mock('node:fs', async (importOriginal) => { copyFileSync: (...args: Parameters) => { beforeCopyMock(...args) return actual.copyFileSync(...args) + }, + rmSync: (...args: Parameters) => { + if (rmSyncShouldThrow.value) { + throw new Error('cleanup failed') + } + return actual.rmSync(...args) } } }) @@ -21,6 +30,8 @@ import { DatabaseSync } from 'node:sqlite' import { createChromiumCookieTestDatabase } from './browser-cookie-import-test-database' import { createChromiumCookieSnapshot } from './chromium-cookie-snapshot' +const SNAPSHOT_ATTEMPTS = 5 + function sourceFiles(databasePath: string): Map { const files = new Map() for (const suffix of ['', '-wal', '-shm'] as const) { @@ -44,10 +55,12 @@ describe('createChromiumCookieSnapshot', () => { root = mkdtempSync(join(tmpdir(), 'orca-chromium-snapshot-test-')) writer = null beforeCopyMock.mockReset() + rmSyncShouldThrow.value = false }) afterEach(() => { writer?.close() + rmSyncShouldThrow.value = false rmSync(root, { recursive: true, force: true }) }) @@ -103,7 +116,10 @@ describe('createChromiumCookieSnapshot', () => { } }) - const snapshot = createChromiumCookieSnapshot(sourcePath, { tempRoot: root }) + const snapshot = createChromiumCookieSnapshot(sourcePath, { + tempRoot: root, + retryBackoffMs: 0 + }) const database = new DatabaseSync(snapshot.databasePath, { readOnly: true }) const row = database.prepare("SELECT value FROM cookies WHERE name = 'raced-in'").get() database.close() @@ -127,7 +143,10 @@ describe('createChromiumCookieSnapshot', () => { } }) - const snapshot = createChromiumCookieSnapshot(sourcePath, { tempRoot: root }) + const snapshot = createChromiumCookieSnapshot(sourcePath, { + tempRoot: root, + retryBackoffMs: 0 + }) const database = new DatabaseSync(snapshot.databasePath, { readOnly: true }) const row = database.prepare("SELECT value FROM cookies WHERE name = 'checkpointed'").get() database.close() @@ -137,6 +156,89 @@ describe('createChromiumCookieSnapshot', () => { snapshot.cleanup() }) + it('backs off between unstable attempts before a successful snapshot', () => { + const sourcePath = join(root, 'Chrome', 'Default', 'Cookies') + createChromiumCookieTestDatabase(sourcePath, []).close() + const sleep = vi.fn() + let raceCount = 0 + beforeCopyMock.mockImplementation((source: string) => { + if (source === sourcePath && raceCount < 2) { + raceCount += 1 + if (!writer) { + writer = new DatabaseSync(sourcePath) + writer.exec('PRAGMA journal_mode = WAL; PRAGMA wal_autocheckpoint = 0;') + } + writer.exec( + `INSERT INTO cookies (creation_utc, host_key, name, value, path, expires_utc, is_secure, is_httponly, samesite) VALUES (${raceCount}, '.example.com', 'raced-${raceCount}', 'v${raceCount}', '/', 0, 0, 0, 0)` + ) + } + }) + + const snapshot = createChromiumCookieSnapshot(sourcePath, { + tempRoot: root, + retryBackoffMs: 40, + sleep + }) + const database = new DatabaseSync(snapshot.databasePath, { readOnly: true }) + const rows = database.prepare('SELECT name FROM cookies ORDER BY name').all() + database.close() + + expect(rows).toEqual([ + expect.objectContaining({ name: 'raced-1' }), + expect.objectContaining({ name: 'raced-2' }) + ]) + expect(sleep).toHaveBeenCalledTimes(2) + expect(sleep).toHaveBeenNthCalledWith(1, 40) + expect(sleep).toHaveBeenNthCalledWith(2, 40) + expect(beforeCopyMock.mock.calls.filter(([source]) => source === sourcePath)).toHaveLength(3) + snapshot.cleanup() + }) + + it('throws after exhausting unstable snapshot attempts and leaves no temp dirs', () => { + const sourcePath = join(root, 'Chrome', 'Default', 'Cookies') + const snapshotsRoot = join(root, 'snapshots') + mkdirSync(snapshotsRoot) + writer = createChromiumCookieTestDatabase(sourcePath, [{ name: 'session', value: 'value' }], { + journalMode: 'wal' + }) + const sleep = vi.fn() + let mutation = 0 + beforeCopyMock.mockImplementation((source: string) => { + if (source === sourcePath) { + mutation += 1 + writer?.exec( + `INSERT INTO cookies (creation_utc, host_key, name, value, path, expires_utc, is_secure, is_httponly, samesite) VALUES (${mutation}, '.example.com', 'mut-${mutation}', 'v', '/', 0, 0, 0, 0)` + ) + } + }) + + expect(() => + createChromiumCookieSnapshot(sourcePath, { + tempRoot: snapshotsRoot, + retryBackoffMs: 15, + sleep + }) + ).toThrow('Chromium cookies database changed while creating a snapshot') + expect(beforeCopyMock.mock.calls.filter(([source]) => source === sourcePath)).toHaveLength( + SNAPSHOT_ATTEMPTS + ) + expect(sleep).toHaveBeenCalledTimes(SNAPSHOT_ATTEMPTS - 1) + expect(sleep).toHaveBeenCalledWith(15) + expect(readdirSync(snapshotsRoot)).toEqual([]) + }) + + it('preserves the original error when cleanup fails after snapshot failure', () => { + const snapshotsRoot = join(root, 'snapshots') + mkdirSync(snapshotsRoot) + rmSyncShouldThrow.value = true + + expect(() => + createChromiumCookieSnapshot(join(root, 'missing', 'Cookies'), { + tempRoot: snapshotsRoot + }) + ).toThrow('does not exist') + }) + it('removes a partial snapshot when a WAL copy fails', () => { const sourcePath = join(root, 'Chrome', 'Default', 'Cookies') const snapshotsRoot = join(root, 'snapshots') diff --git a/src/main/browser/chromium-cookie-snapshot.ts b/src/main/browser/chromium-cookie-snapshot.ts index 4b54e98be8d..847ea036e57 100644 --- a/src/main/browser/chromium-cookie-snapshot.ts +++ b/src/main/browser/chromium-cookie-snapshot.ts @@ -1,8 +1,11 @@ -import { copyFileSync, mkdtempSync, rmSync, statSync, unlinkSync } from 'node:fs' +import { copyFileSync, mkdirSync, mkdtempSync, rmSync, statSync, unlinkSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' const SNAPSHOT_ATTEMPTS = 5 +// Why: continuous Chromium writers can keep all immediate retries inside one +// write burst; a short pause lets the burst settle without making import laggy. +const SNAPSHOT_RETRY_BACKOFF_MS = 25 type FileState = { device: bigint @@ -18,7 +21,21 @@ export type ChromiumCookieSnapshot = { } type ChromiumCookieSnapshotOptions = { + /** Prefer a private app path (e.g. userData); tests inject an isolated root. */ tempRoot?: string + /** Delay between failed snapshot attempts. Defaults to 25ms. */ + retryBackoffMs?: number + /** Injectable sleep for tests; defaults to portable Atomics.wait sleep. */ + sleep?: (ms: number) => void +} + +function sleepSync(ms: number): void { + if (ms <= 0) { + return + } + // Why: keep createChromiumCookieSnapshot synchronous for the import pipeline; + // Atomics.wait is the portable bounded sleep without child_process/platform tools. + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms) } function removeSnapshotDirectory(path: string): void { @@ -52,6 +69,9 @@ function readFileState(path: string): FileState | null { } function sameFileState(left: FileState | null, right: FileState | null): boolean { + // Why: mtime/ctime equality is coarse FS stability only; concurrent writers that + // finish within the same timestamp quantum can still pass. Retries + backoff + // shrink that window but cannot eliminate it without content hashing. if (!left || !right) { return left === right } @@ -117,12 +137,21 @@ export function createChromiumCookieSnapshot( sourcePath: string, options: ChromiumCookieSnapshotOptions = {} ): ChromiumCookieSnapshot { - const snapshotDir = mkdtempSync(join(options.tempRoot ?? tmpdir(), 'orca-cookie-import-')) + // Why: cookie DB bytes are sensitive; production passes userData via tempRoot. + // Fall back to os.tmpdir() so unit tests can import this module without Electron. + const tempRoot = options.tempRoot ?? tmpdir() + mkdirSync(tempRoot, { recursive: true }) + const snapshotDir = mkdtempSync(join(tempRoot, 'orca-cookie-import-')) const databasePath = join(snapshotDir, 'Cookies') let keepSnapshot = false + const sleep = options.sleep ?? sleepSync + const backoffMs = options.retryBackoffMs ?? SNAPSHOT_RETRY_BACKOFF_MS try { for (let attempt = 0; attempt < SNAPSHOT_ATTEMPTS; attempt += 1) { + if (attempt > 0) { + sleep(backoffMs) + } if (copyStableAttempt(sourcePath, databasePath)) { keepSnapshot = true return { @@ -134,7 +163,11 @@ export function createChromiumCookieSnapshot( throw new Error('Chromium cookies database changed while creating a snapshot') } finally { if (!keepSnapshot) { - removeSnapshotDirectory(snapshotDir) + try { + removeSnapshotDirectory(snapshotDir) + } catch { + // Why: cleanup must not replace the original snapshot failure for the caller. + } } } }