From 24be00ad092bc1f58d6bb0004a3133fd7a85030d Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Thu, 20 Aug 2026 20:16:21 -0400 Subject: [PATCH] fix(browser): quarantine unsettled cookie mutations --- ...owser-cookie-clear-store-lifecycle.test.ts | 28 ++- .../browser/browser-cookie-clear-store.ts | 82 +++------ .../browser-cookie-debugger-command.ts | 16 +- ...ookie-debugger-retirement.electron.test.ts | 139 ++++++++------ .../browser-cookie-debugger-session.ts | 51 ++++++ .../browser-cookie-import-cdp-timeout.test.ts | 171 +++++++++++++++--- .../browser/browser-cookie-import-clear.ts | 8 + src/main/browser/browser-cookie-import.ts | 21 ++- .../browser-cookie-mutation-quarantine.ts | 36 ++++ 9 files changed, 390 insertions(+), 162 deletions(-) create mode 100644 src/main/browser/browser-cookie-debugger-session.ts create mode 100644 src/main/browser/browser-cookie-mutation-quarantine.ts diff --git a/src/main/browser/browser-cookie-clear-store-lifecycle.test.ts b/src/main/browser/browser-cookie-clear-store-lifecycle.test.ts index 9485d7bfb13..afb65555135 100644 --- a/src/main/browser/browser-cookie-clear-store-lifecycle.test.ts +++ b/src/main/browser/browser-cookie-clear-store-lifecycle.test.ts @@ -3,7 +3,6 @@ import type { Session } from 'electron' const electron = vi.hoisted(() => { const windows: BrowserWindow[] = [] - const getAllWebContents = vi.fn(() => []) class BrowserWindow { destroy = vi.fn() @@ -30,14 +29,13 @@ const electron = vi.hoisted(() => { } } - return { BrowserWindow, getAllWebContents, windows } + return { BrowserWindow, windows } }) const lease = vi.hoisted(() => ({ release: vi.fn() })) vi.mock('electron', () => ({ - BrowserWindow: electron.BrowserWindow, - webContents: { getAllWebContents: electron.getAllWebContents } + BrowserWindow: electron.BrowserWindow })) vi.mock('./electron-debugger-lease', () => ({ acquireElectronDebugger: vi.fn(() => lease) @@ -52,7 +50,6 @@ function targetSession(): Session { describe('cookie clear debugger lifecycle', () => { beforeEach(() => { electron.windows.length = 0 - electron.getAllWebContents.mockReturnValue([]) lease.release.mockClear() }) @@ -103,7 +100,7 @@ describe('cookie clear debugger lifecycle', () => { expect(sendCommand.mock.calls.map(([, params]) => params?.name)).toEqual(['first', 'second']) }) - it('classifies a retired command as timed out only after its late result settles', async () => { + it('classifies a retired command after a bounded settlement grace', async () => { vi.useFakeTimers() let resolveCommand!: (value: unknown) => void const pendingCommand = new Promise((resolve) => { @@ -119,19 +116,18 @@ describe('cookie clear debugger lifecycle', () => { const window = electron.windows[0] window.webContents.debugger.sendCommand.mockReturnValueOnce(pendingCommand) window.resolveLoad() - await vi.advanceTimersByTimeAsync(10_000) - - expect(lease.release).toHaveBeenCalledOnce() - expect(window.destroy).toHaveBeenCalledOnce() - await expect( - Promise.race([write.then(() => 'settled'), Promise.resolve('pending')]) - ).resolves.toBe('pending') - - resolveCommand({ success: true }) - await expect(write).rejects.toMatchObject({ + const timeoutExpectation = expect(write).rejects.toMatchObject({ name: 'CookieDebuggerCommandTimeoutError', message: 'Cookie debugger command Network.setCookie timed out after 10000ms' }) + await vi.advanceTimersByTimeAsync(11_000) + + expect(lease.release).toHaveBeenCalledOnce() + expect(window.destroy).toHaveBeenCalledOnce() + await timeoutExpectation + + resolveCommand({ success: true }) + await Promise.resolve() store.dispose() expect(lease.release).toHaveBeenCalledOnce() }) diff --git a/src/main/browser/browser-cookie-clear-store.ts b/src/main/browser/browser-cookie-clear-store.ts index 6a759c0948f..cf4b458d5a1 100644 --- a/src/main/browser/browser-cookie-clear-store.ts +++ b/src/main/browser/browser-cookie-clear-store.ts @@ -1,6 +1,13 @@ -import { BrowserWindow, type Cookie, type Session } from 'electron' +import type { Cookie, Session } from 'electron' import { sendCookieDebuggerCommand } from './browser-cookie-debugger-command' -import { acquireElectronDebugger } from './electron-debugger-lease' +import { + leaseCookieDebuggerSession, + type CookieDebuggerSession +} from './browser-cookie-debugger-session' +import { + assertCookieMutationsAvailable, + quarantineCookieMutations +} from './browser-cookie-mutation-quarantine' import { normalizeCookieDomain } from './browser-cookie-import-policy' import type { CookieClearIdentity, @@ -30,15 +37,6 @@ type CdpCookie = { partitionKeyOpaque?: boolean } -type CookieClearDebugger = { - sendCommand: (method: string, params?: Record) => Promise -} - -type CookieClearSession = { - debugger: CookieClearDebugger - dispose: () => void -} - function cdpSameSite(sameSite: Cookie['sameSite']): 'Strict' | 'Lax' | 'None' | undefined { if (sameSite === 'strict') { return 'Strict' @@ -122,50 +120,6 @@ function identityFromCdpCookie(url: string, cdpCookie: CdpCookie): CookieClearId } } -function openHiddenCookieWindow(targetSession: Session): BrowserWindow { - return new BrowserWindow({ - show: false, - webPreferences: { - session: targetSession, - sandbox: true, - contextIsolation: true, - nodeIntegration: false - } - }) -} - -async function leaseHiddenCookieDebugger(targetSession: Session): Promise { - const window = openHiddenCookieWindow(targetSession) - try { - await window.loadURL('data:text/html,cookie-clear') - const contents = window.webContents - if (contents.isDestroyed()) { - throw new Error('Could not attach to the cookie session for an atomic clear') - } - const lease = acquireElectronDebugger(contents) - let disposed = false - return { - debugger: contents.debugger, - dispose: () => { - if (disposed) { - return - } - disposed = true - try { - lease.release() - } finally { - if (!contents.isDestroyed()) { - window.destroy() - } - } - } - } - } catch (error) { - window.destroy() - throw error - } -} - export function cookieClearIdentitiesFromCdp( cookies: readonly { cookie: Cookie; url: string }[], cdpCookies: readonly CdpCookie[] @@ -259,10 +213,11 @@ async function writeIdentityWithCdp( export function openCookieClearStore( targetSession: Session ): CookieClearStore & CookieImportWriteStore & { dispose: () => void } { - let attached: CookieClearSession | null = null - let pendingAttach: Promise | null = null + let attached: CookieDebuggerSession | null = null + let pendingAttach: Promise | null = null let disposed = false - const retire = (session: CookieClearSession) => { + const retire = (session: CookieDebuggerSession, settled: Promise) => { + quarantineCookieMutations(targetSession, settled) if (attached === session) { attached = null } @@ -278,7 +233,7 @@ export function openCookieClearStore( if (pendingAttach) { return pendingAttach } - const pending = leaseHiddenCookieDebugger(targetSession).then((session) => { + const pending = leaseCookieDebuggerSession(targetSession).then((session) => { if (disposed) { session.dispose() throw new Error('Cookie clear store was disposed during debugger attachment') @@ -296,15 +251,20 @@ export function openCookieClearStore( } } const sendCommand = async (method: string, params?: Record) => { + assertCookieMutationsAvailable(targetSession) const session = await attach() - return sendCookieDebuggerCommand(session, method, params, () => retire(session)) + return sendCookieDebuggerCommand(session, method, params, (settled) => retire(session, settled)) } return { get: (filter) => targetSession.cookies.get(filter), - remove: (url, name) => targetSession.cookies.remove(url, name), + remove: (url, name) => { + assertCookieMutationsAvailable(targetSession) + return targetSession.cookies.remove(url, name) + }, snapshotClearIdentities: async (cookies) => snapshotClearIdentitiesFromCdp(sendCommand, cookies), restoreClearIdentities: async (identities) => { + assertCookieMutationsAvailable(targetSession) await attach() await restoreEveryCookieIdentity(identities, (identity) => writeIdentityWithCdp(sendCommand, identity, 'restore') diff --git a/src/main/browser/browser-cookie-debugger-command.ts b/src/main/browser/browser-cookie-debugger-command.ts index 4a141e22214..8114865b52c 100644 --- a/src/main/browser/browser-cookie-debugger-command.ts +++ b/src/main/browser/browser-cookie-debugger-command.ts @@ -5,6 +5,7 @@ type CookieDebuggerCommandSession = { } const COOKIE_DEBUGGER_COMMAND_TIMEOUT_MS = 10_000 +const COOKIE_DEBUGGER_RETIREMENT_TIMEOUT_MS = 1_000 export class CookieDebuggerCommandTimeoutError extends Error { constructor(method: string, options?: ErrorOptions) { @@ -20,11 +21,12 @@ export async function sendCookieDebuggerCommand( session: CookieDebuggerCommandSession, method: string, params: Record | undefined, - retire: () => void + retire: (settled: Promise) => void ): Promise { let timedOut = false let retirementError: unknown = null let timer: ReturnType | null = null + let retirementTimer: ReturnType | null = null const command = Promise.resolve().then(() => session.debugger.sendCommand(method, params)) const settled = command.then( () => undefined, @@ -34,7 +36,7 @@ export async function sendCookieDebuggerCommand( timer = setTimeout(() => { timedOut = true try { - retire() + retire(settled) } catch (error) { retirementError = error } @@ -47,11 +49,19 @@ export async function sendCookieDebuggerCommand( if (!timedOut) { return await command } - await settled + await Promise.race([ + settled, + new Promise((resolve) => { + retirementTimer = setTimeout(resolve, COOKIE_DEBUGGER_RETIREMENT_TIMEOUT_MS) + }) + ]) throw new CookieDebuggerCommandTimeoutError(method, { cause: retirementError }) } finally { if (timer) { clearTimeout(timer) } + if (retirementTimer) { + clearTimeout(retirementTimer) + } } } diff --git a/src/main/browser/browser-cookie-debugger-retirement.electron.test.ts b/src/main/browser/browser-cookie-debugger-retirement.electron.test.ts index d1e1569739f..cbb6d95dc3c 100644 --- a/src/main/browser/browser-cookie-debugger-retirement.electron.test.ts +++ b/src/main/browser/browser-cookie-debugger-retirement.electron.test.ts @@ -18,6 +18,7 @@ afterAll(() => { type FixtureResult = { step: string firstError: { name: string; message: string } | null + secondError: { name: string; message: string } | null events: string[] recoveredCookieValues: string[] } @@ -26,11 +27,7 @@ function fixtureMain(bundlePath: string, resultPath: string): string { return ` const { app, BrowserWindow, session } = require('electron') const { writeFileSync } = require('node:fs') -const { - openCookieClearStore, - sendCookieDebuggerCommand, - withCookieMutationLock -} = require(${JSON.stringify(bundlePath)}) +const { openCookieClearStore, withCookieMutationLock } = require(${JSON.stringify(bundlePath)}) const resultPath = ${JSON.stringify(resultPath)} let step = 'starting' const events = [] @@ -49,72 +46,92 @@ async function run() { const targetSession = session.fromPartition(partition) const keeper = new BrowserWindow({ show: false }) await keeper.loadURL('data:text/html,fixture keeper') - const firstWindow = new BrowserWindow({ - show: false, - webPreferences: { session: targetSession, sandbox: true, contextIsolation: true } + let targetCount = 0 + let resolveOrphan = null + app.on('web-contents-created', (_event, contents) => { + targetCount += 1 + const label = String(targetCount) + events.push('target:' + label + ':created') + contents.once('destroyed', () => events.push('target:' + label + ':destroyed')) + const sendCommand = contents.debugger.sendCommand.bind(contents.debugger) + contents.debugger.sendCommand = (method, params) => { + events.push('target:' + label + ':' + method) + if (label === '1' && method === 'Network.setCookie') { + return new Promise((resolve) => { + resolveOrphan = () => { + events.push('first:late-completion') + resolve({ success: true }) + } + }) + } + return sendCommand(method, params) + } }) - await firstWindow.loadURL('data:text/html,wedged cookie debugger') - const firstDebugger = firstWindow.webContents.debugger - firstDebugger.attach('1.3') let firstError = null const first = withCookieMutationLock(targetSession, async () => { events.push('first:start') - try { - await sendCookieDebuggerCommand( - { debugger: firstDebugger }, - 'Runtime.evaluate', - { expression: 'new Promise(() => {})', awaitPromise: true }, - () => { - events.push('first:retire') - if (firstDebugger.isAttached()) firstDebugger.detach() - firstWindow.destroy() - } - ) - } catch (error) { - firstError = { name: error?.name || '', message: String(error?.message || error) } - events.push('first:error') - } - }) - - const second = withCookieMutationLock(targetSession, async () => { - events.push('second:start') const store = openCookieClearStore(targetSession) try { await store.writeCookieIdentity({ - url: 'https://recovered.example/', - name: 'recovered', - value: 'written-after-retirement', + url: 'https://wedged.example/', + name: 'wedged', + value: 'never-committed', sameSite: 'unspecified', secure: true }) - events.push('second:done') + } catch (error) { + firstError = { name: error?.name || '', message: String(error?.message || error) } + events.push('first:error') } finally { store.dispose() } }) + let secondError = null + const second = withCookieMutationLock(targetSession, async () => { + events.push('second:start') + }).catch((error) => { + secondError = { name: error?.name || '', message: String(error?.message || error) } + events.push('second:error') + }) + mark('commands started') await Promise.all([first, second]) - mark('imports settled') + mark('quarantine proven') - const viewer = new BrowserWindow({ - show: false, - webPreferences: { session: targetSession, sandbox: true, contextIsolation: true } + if (!resolveOrphan) throw new Error('the injected cookie command never started') + resolveOrphan() + await new Promise((resolve) => setImmediate(resolve)) + + await withCookieMutationLock(targetSession, async () => { + events.push('third:start') + const store = openCookieClearStore(targetSession) + try { + await store.writeCookieIdentity({ + url: 'https://recovered.example/', + name: 'recovered', + value: 'written-after-quarantine', + sameSite: 'unspecified', + secure: true + }) + events.push('third:done') + } finally { + store.dispose() + } }) - await viewer.loadURL('data:text/html,cookie debugger verifier') - const viewerDebugger = viewer.webContents.debugger - viewerDebugger.attach('1.3') - const cookies = (await viewerDebugger.sendCommand('Network.getAllCookies')).cookies + + const cookies = await targetSession.cookies.get({ name: 'recovered' }) const recoveredCookieValues = cookies .filter((cookie) => cookie.name === 'recovered') .map((cookie) => cookie.value) - viewerDebugger.detach() - viewer.destroy() keeper.destroy() clearTimeout(fixtureTimeout) - writeFileSync(resultPath, JSON.stringify({ step, firstError, events, recoveredCookieValues })) + writeFileSync( + resultPath, + JSON.stringify({ step, firstError, secondError, events, recoveredCookieValues }) + ) app.exit(0) } @@ -136,7 +153,6 @@ async function runFixture(): Promise { bundleEntryPath, [ `export { openCookieClearStore } from ${JSON.stringify(join(process.cwd(), 'src/main/browser/browser-cookie-clear-store.ts'))}`, - `export { sendCookieDebuggerCommand } from ${JSON.stringify(join(process.cwd(), 'src/main/browser/browser-cookie-debugger-command.ts'))}`, `export { withCookieMutationLock } from ${JSON.stringify(join(process.cwd(), 'src/main/browser/browser-cookie-import-clear.ts'))}` ].join('\n') ) @@ -171,21 +187,30 @@ async function runFixture(): Promise { } describe('cookie debugger retirement in Electron', () => { - it('settles the wedged command before a queued cookie mutation starts', async () => { + it('quarantines an unsettled cookie command and recovers after its late completion', async () => { const result = await runFixture() - expect(result.step).toBe('imports settled') + expect(result.step).toBe('quarantine proven') expect(result.firstError).toEqual({ name: 'CookieDebuggerCommandTimeoutError', - message: 'Cookie debugger command Runtime.evaluate timed out after 10000ms' + message: 'Cookie debugger command Network.setCookie timed out after 10000ms' }) - expect(result.events).toEqual([ - 'first:start', - 'first:retire', - 'first:error', - 'second:start', - 'second:done' - ]) - expect(result.recoveredCookieValues).toEqual(['written-after-retirement']) + expect(result.secondError).toEqual({ + name: 'CookieMutationQuarantinedError', + message: + 'A previous cookie import is still finishing in Chromium. Wait a moment and try again; if it continues, restart Orca.' + }) + expect(result.events).not.toContain('second:start') + expect(result.events.indexOf('first:error')).toBeLessThan( + result.events.indexOf('first:late-completion') + ) + expect(result.events.indexOf('first:late-completion')).toBeLessThan( + result.events.indexOf('third:start') + ) + expect(result.events).toContain('target:1:Network.setCookie') + expect(result.events).toContain('target:1:destroyed') + expect(result.events).toContain('target:2:Network.setCookie') + expect(result.events).toContain('third:done') + expect(result.recoveredCookieValues).toEqual(['written-after-quarantine']) }, 60_000) }) diff --git a/src/main/browser/browser-cookie-debugger-session.ts b/src/main/browser/browser-cookie-debugger-session.ts new file mode 100644 index 00000000000..5d06c3be48e --- /dev/null +++ b/src/main/browser/browser-cookie-debugger-session.ts @@ -0,0 +1,51 @@ +import { BrowserWindow, type Session } from 'electron' +import { acquireElectronDebugger } from './electron-debugger-lease' + +export type CookieDebuggerSession = { + debugger: { + sendCommand: (method: string, params?: Record) => Promise + } + dispose: () => void +} + +export async function leaseCookieDebuggerSession( + targetSession: Session +): Promise { + const window = new BrowserWindow({ + show: false, + webPreferences: { + session: targetSession, + sandbox: true, + contextIsolation: true, + nodeIntegration: false + } + }) + try { + await window.loadURL('data:text/html,cookie-clear') + const contents = window.webContents + if (contents.isDestroyed()) { + throw new Error('Could not attach to the cookie session for an atomic clear') + } + const lease = acquireElectronDebugger(contents) + let disposed = false + return { + debugger: contents.debugger, + dispose: () => { + if (disposed) { + return + } + disposed = true + try { + lease.release() + } finally { + if (!contents.isDestroyed()) { + window.destroy() + } + } + } + } + } catch (error) { + window.destroy() + throw error + } +} diff --git a/src/main/browser/browser-cookie-import-cdp-timeout.test.ts b/src/main/browser/browser-cookie-import-cdp-timeout.test.ts index 61428b044ab..db8fd63f239 100644 --- a/src/main/browser/browser-cookie-import-cdp-timeout.test.ts +++ b/src/main/browser/browser-cookie-import-cdp-timeout.test.ts @@ -2,6 +2,11 @@ import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { sendCookieDebuggerCommand } from './browser-cookie-debugger-command' + +type CookieDebuggerCommandModule = { + sendCookieDebuggerCommand: typeof sendCookieDebuggerCommand +} const electron = vi.hoisted(() => { type PendingCommand = { @@ -11,24 +16,54 @@ const electron = vi.hoisted(() => { const events: string[] = [] const windows: BrowserWindow[] = [] + const sessions = new Map>() let firstCommandStarted: () => void = () => undefined let firstCommand: PendingCommand let userDataPath = '' + let commandTimeoutEnabled = true + + function createSession(partition: string) { + return { + partition, + cookies: { + get: vi.fn(async () => []), + remove: vi.fn(async (_url: string, name: string) => { + events.push(`remove:${partition}:${name}`) + }) + } + } + } + + function fromPartition(partition: string) { + let target = sessions.get(partition) + if (!target) { + target = createSession(partition) + sessions.set(partition, target) + } + return target + } function reset(): void { events.length = 0 windows.length = 0 + sessions.clear() + firstCommandStarted = () => undefined + commandTimeoutEnabled = true let resolveCommand!: (value: unknown) => void firstCommand = { promise: new Promise((resolve) => { resolveCommand = resolve }), - resolve: resolveCommand + resolve: (value) => { + events.push('A:late-completion') + resolveCommand(value) + } } } class BrowserWindow { - readonly label = windows.length === 0 ? 'A' : 'B' + readonly label = String.fromCharCode(65 + windows.length) + private closed = false readonly webContents = { label: this.label, debugger: { @@ -41,7 +76,7 @@ const electron = vi.hoisted(() => { return Promise.resolve({ success: true }) }) }, - isDestroyed: vi.fn(() => false) + isDestroyed: vi.fn(() => this.closed) } constructor() { @@ -51,44 +86,40 @@ const electron = vi.hoisted(() => { async loadURL(): Promise {} destroy(): void { + this.closed = true events.push(`${this.label}:destroy`) } } - const cookies = { - get: vi.fn(async () => []), - remove: vi.fn(async (_url: string, name: string) => { - events.push(`rollback:${name}`) - }) - } - const stableSession = { cookies } - reset() return { BrowserWindow, - cookies, events, firstCommand: () => firstCommand, + fromPartition, + isCommandTimeoutEnabled: () => commandTimeoutEnabled, onFirstCommand: () => new Promise((resolve) => { firstCommandStarted = resolve }), reset, - stableSession, + sessions, windows, getUserDataPath: () => userDataPath, setUserDataPath: (value: string) => { userDataPath = value + }, + setCommandTimeoutEnabled: (value: boolean) => { + commandTimeoutEnabled = value } } }) vi.mock('electron', () => ({ app: { getPath: electron.getUserDataPath }, - BrowserWindow: electron.BrowserWindow, dialog: { showOpenDialog: vi.fn() }, - session: { fromPartition: vi.fn(() => electron.stableSession) }, - webContents: { getAllWebContents: vi.fn(() => []) } + session: { fromPartition: vi.fn(electron.fromPartition) }, + BrowserWindow: electron.BrowserWindow })) vi.mock('./electron-debugger-lease', () => ({ @@ -99,6 +130,19 @@ vi.mock('./electron-debugger-lease', () => ({ }) })) +vi.mock('./browser-cookie-debugger-command', async (importOriginal) => { + const actual = await importOriginal() + return { + ...actual, + sendCookieDebuggerCommand: ( + ...args: Parameters + ): ReturnType => + electron.isCommandTimeoutEnabled() + ? actual.sendCookieDebuggerCommand(...args) + : args[0].debugger.sendCommand(args[1], args[2]) + } +}) + vi.mock('./browser-session-registry', () => ({ browserSessionRegistry: { setPendingCookieImport: vi.fn(), @@ -114,8 +158,6 @@ describe('cookie import debugger timeout', () => { beforeEach(() => { vi.useFakeTimers() electron.reset() - electron.cookies.get.mockClear() - electron.cookies.remove.mockClear() fixtureDir = mkdtempSync(join(tmpdir(), 'orca-cookie-cdp-timeout-')) electron.setUserDataPath(fixtureDir) }) @@ -136,20 +178,21 @@ describe('cookie import debugger timeout', () => { return filePath } - it('retires a timed-out debugger without releasing serialization before its late write settles', async () => { + it('recovers after retirement proves the late command settled during its grace period', async () => { const firstStarted = electron.onFirstCommand() const first = importCookiesFromFile(writeCookieSource('first'), 'persist:timeout-oracle') await firstStarted const second = importCookiesFromFile(writeCookieSource('second'), 'persist:timeout-oracle') - await vi.advanceTimersByTimeAsync(60_000) + await vi.advanceTimersByTimeAsync(10_000) const whileFirstCanCompleteLate = [...electron.events] - electron.events.push('A:late-completion') + const firstState = await Promise.race([first.then(() => 'settled'), Promise.resolve('pending')]) electron.firstCommand().resolve({ success: true }) const [firstResult, secondResult] = await Promise.all([first, second]) expect(whileFirstCanCompleteLate).toEqual(['A:Network.setCookie', 'A:detach', 'A:destroy']) + expect(firstState).toBe('pending') expect(firstResult).toEqual({ ok: false, reason: expect.stringContaining('Could not safely replace cookies for the imported sites.') @@ -160,7 +203,7 @@ describe('cookie import debugger timeout', () => { 'A:detach', 'A:destroy', 'A:late-completion', - 'rollback:first', + 'remove:persist:timeout-oracle:first', 'B:Network.setCookie', 'B:detach', 'B:destroy' @@ -172,4 +215,88 @@ describe('cookie import debugger timeout', () => { ) ).toBe(true) }) + + it('fails fast while an orphan is unsettled and reopens only after its late completion', async () => { + const firstStarted = electron.onFirstCommand() + const first = importCookiesFromFile(writeCookieSource('first'), 'persist:timeout-oracle') + await firstStarted + const second = importCookiesFromFile(writeCookieSource('second'), 'persist:timeout-oracle') + + await vi.advanceTimersByTimeAsync(11_000) + const [firstResult, secondResult] = await Promise.all([first, second]) + + expect(firstResult).toEqual({ + ok: false, + reason: expect.stringContaining('Could not safely replace cookies for the imported sites.') + }) + expect(secondResult).toEqual({ + ok: false, + reason: + 'A previous cookie import is still finishing in Chromium. Wait a moment and try again; if it continues, restart Orca.' + }) + expect(electron.events).toEqual(['A:Network.setCookie', 'A:detach', 'A:destroy']) + expect(electron.windows).toHaveLength(1) + + const otherResult = await importCookiesFromFile( + writeCookieSource('other'), + 'persist:other-partition' + ) + expect(otherResult.ok).toBe(true) + + electron.firstCommand().resolve({ success: true }) + await electron.firstCommand().promise + await Promise.resolve() + const recoveredResult = await importCookiesFromFile( + writeCookieSource('recovered'), + 'persist:timeout-oracle' + ) + + expect(recoveredResult.ok).toBe(true) + expect(electron.events).toEqual([ + 'A:Network.setCookie', + 'A:detach', + 'A:destroy', + 'B:Network.setCookie', + 'B:detach', + 'B:destroy', + 'A:late-completion', + 'C:Network.setCookie', + 'C:detach', + 'C:destroy' + ]) + expect(electron.windows).toHaveLength(3) + }) + + it('exposes the permanent-blockage signal when retirement is disabled', async () => { + electron.setCommandTimeoutEnabled(false) + const firstStarted = electron.onFirstCommand() + const first = importCookiesFromFile(writeCookieSource('first'), 'persist:timeout-oracle') + await firstStarted + const second = importCookiesFromFile(writeCookieSource('second'), 'persist:timeout-oracle') + + await vi.advanceTimersByTimeAsync(60_000) + const firstState = await Promise.race([first.then(() => 'settled'), Promise.resolve('pending')]) + const secondState = await Promise.race([ + second.then(() => 'settled'), + Promise.resolve('pending') + ]) + + expect(firstState).toBe('pending') + expect(secondState).toBe('pending') + expect(electron.events).toEqual(['A:Network.setCookie']) + + electron.firstCommand().resolve({ success: true }) + const [firstResult, secondResult] = await Promise.all([first, second]) + expect(firstResult.ok).toBe(true) + expect(secondResult.ok).toBe(true) + expect(electron.events).toEqual([ + 'A:Network.setCookie', + 'A:late-completion', + 'A:detach', + 'A:destroy', + 'B:Network.setCookie', + 'B:detach', + 'B:destroy' + ]) + }) }) diff --git a/src/main/browser/browser-cookie-import-clear.ts b/src/main/browser/browser-cookie-import-clear.ts index af79c4249ed..f5791383c20 100644 --- a/src/main/browser/browser-cookie-import-clear.ts +++ b/src/main/browser/browser-cookie-import-clear.ts @@ -1,5 +1,6 @@ import type { Cookie, Cookies } from 'electron' import { mapSettledWithConcurrency } from '../../shared/map-with-concurrency' +import { assertCookieMutationsAvailable } from './browser-cookie-mutation-quarantine' import type { ImportedDomainScope } from './browser-cookie-import-policy' import { cookieRemovalUrl, @@ -86,6 +87,7 @@ export function identitiesFromClearCookies( * try/finally take it directly; callers with a single callback use the wrapper below. */ export async function acquireCookieMutationLock(owner: object): Promise<() => void> { + assertCookieMutationsAvailable(owner) const previous = mutationLocks.get(owner) ?? Promise.resolve() let release!: () => void const current = new Promise((resolve) => { @@ -96,6 +98,12 @@ export async function acquireCookieMutationLock(owner: object): Promise<() => vo previous.then(() => current) ) await previous + try { + assertCookieMutationsAvailable(owner) + } catch (error) { + release() + throw error + } return release } diff --git a/src/main/browser/browser-cookie-import.ts b/src/main/browser/browser-cookie-import.ts index 22d4dc5e270..8473382fc31 100644 --- a/src/main/browser/browser-cookie-import.ts +++ b/src/main/browser/browser-cookie-import.ts @@ -92,6 +92,10 @@ import { type CookieImportWriteStore } from './browser-cookie-import-clear' import { openCookieClearStore } from './browser-cookie-clear-store' +import { + COOKIE_MUTATION_QUARANTINED_REASON, + isCookieMutationQuarantinedError +} from './browser-cookie-mutation-quarantine' import { readChromiumRowPartition, readFirefoxRowPartition, @@ -677,9 +681,10 @@ async function importValidatedCookies( // remove cookies the newer import already wrote and reported as imported. Taken AFTER the // store is opened on purpose — openWriteStore only builds the adapter, it attaches no // debugger, so holding it while queued cannot deadlock against the holder. - const releaseMutationLock = await acquireCookieMutationLock(target.mutationLockOwner) + let releaseMutationLock: (() => void) | null = null let replaced: ReplacedImportedDomainCookies | null = null try { + releaseMutationLock = await acquireCookieMutationLock(target.mutationLockOwner) if (mode === 'replace-imported-domains') { try { // Why (STA-4300 I2 / §2b): the removal scope is the write set. Filtering per exact @@ -737,11 +742,16 @@ async function importValidatedCookies( reason: reasonWithDiagLog('Could not safely replace cookies for the imported sites.') } } + } catch (error) { + if (isCookieMutationQuarantinedError(error)) { + return { ok: false, reason: COOKIE_MUTATION_QUARANTINED_REASON } + } + throw error } finally { try { cookieClearStore.dispose() } finally { - releaseMutationLock() + releaseMutationLock?.() } } } @@ -1595,7 +1605,7 @@ export async function importCookiesFromBrowser( // clear/write lock was reached. Hold the per-partition lock from the first flush through staging, // live replacement, pending-image bookkeeping, and cleanup so an older image cannot race a newer // import on the same partition. - return withCookieMutationLock(targetSession, async () => { + return withCookieMutationLock(targetSession, async () => { await targetSession.cookies.flushStore() // Why (STA-4300): ask the Session where its own storage lives instead of rebuilding the path from @@ -2189,5 +2199,10 @@ export async function importCookiesFromBrowser( diag(` Chromium snapshot cleanup failed: ${String(err)}`) } } + }).catch((error: unknown) => { + if (isCookieMutationQuarantinedError(error)) { + return { ok: false as const, reason: COOKIE_MUTATION_QUARANTINED_REASON } + } + throw error }) } diff --git a/src/main/browser/browser-cookie-mutation-quarantine.ts b/src/main/browser/browser-cookie-mutation-quarantine.ts new file mode 100644 index 00000000000..997d3edc212 --- /dev/null +++ b/src/main/browser/browser-cookie-mutation-quarantine.ts @@ -0,0 +1,36 @@ +const mutationQuarantines = new WeakMap>() + +export const COOKIE_MUTATION_QUARANTINED_REASON = + 'A previous cookie import is still finishing in Chromium. Wait a moment and try again; if it continues, restart Orca.' + +export class CookieMutationQuarantinedError extends Error { + constructor() { + super(COOKIE_MUTATION_QUARANTINED_REASON) + this.name = 'CookieMutationQuarantinedError' + } +} + +export function quarantineCookieMutations(owner: object, until: Promise): void { + const quarantine = until.then( + () => undefined, + () => undefined + ) + mutationQuarantines.set(owner, quarantine) + void quarantine.then(() => { + if (mutationQuarantines.get(owner) === quarantine) { + mutationQuarantines.delete(owner) + } + }) +} + +export function assertCookieMutationsAvailable(owner: object): void { + if (mutationQuarantines.has(owner)) { + throw new CookieMutationQuarantinedError() + } +} + +export function isCookieMutationQuarantinedError( + error: unknown +): error is CookieMutationQuarantinedError { + return error instanceof CookieMutationQuarantinedError +}