From da5cd8e101b68e741e46c112666f55d825e19078 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Wed, 12 Aug 2026 20:40:47 -0700 Subject: [PATCH] perf(browser): clear cookies in one bulk call that excludes the Google family (STA-4065) (#14131) * perf(browser): bulk clear cookies when an import excludes nothing (STA-4065) PR #14086 replaced the bulk clear with one Electron cookies.remove() per removable cookie so partitioned excluded cookies survive an import. That is required whenever something must be preserved, but it also runs when nothing is excluded, where a single clearStorageData is equivalent and far cheaper: 421ms -> 7.3ms at 1k cookies and 1764ms -> 20.1ms at 3k on Electron 43. The fast path is gated on the same unfiltered cookies.get() the exclusion predicate already reads, which an Electron fixture now proves returns partitioned cookies, so "no exclusions in the snapshot" means "nothing in the jar to preserve". * fix(browser): protect cookie clear fast-path race (STA-4065) * test(browser): keep the no-bulk-clear assertion honest after the clearData switch The mocked session still exposed clearStorageData, so asserting it was never called could no longer fail once the fast path moved to clearData. * docs(browser): stop the fallback comment from asserting a rollback The per-cookie path's rollback is being removed by STA-4061; the comment only needs to say the fallback re-removes what survived. * test(browser): keep the STA-4061 rollback fixture on the per-cookie path The bulk fast path clears a jar with no excluded cookie in one call, which would have made every assertion in the partition-rollback fixture vacuous. Give it a live Google cookie so the partial-failure clear still runs, and assert the bulk call count is zero so a future widening fails loudly. * perf(browser): always bulk clear cookies, not only when nothing is excluded The fast path only fired when the jar held no Google cookie, but this import exists for Google, so a Google cookie is normally present and the common case still paid one native remove per cookie. Measured on Electron 43, clearData excludeOrigins preserves the whole google.com registrable family -- host, leading-dot, subdomain and partitioned cookies -- so the bulk call is safe with cookies to keep, and the gate was never load-bearing for preservation. Drop the caller-supplied exclusion predicate so the preserved set has exactly one source of truth: a predicate that disagreed with NON_TRANSPLANTABLE_DOMAINS would have deleted cookies the bulk call is meant to keep. The per-cookie loop stays as the fallback for a rejected bulk clear, carrying the same exclusion. * fix(browser): re-read the cookie jar after a rejected bulk clear clearData can empty part of the jar before rejecting, so the per-cookie fallback was working from a snapshot taken before the attempt: it retried removals for cookies that were already gone and could not see cookies that arrived during the clear. Re-read on rejection only. Also repairs three fixtures that mocked the session with clearStorageData after the call moved to clearData. The missing method threw, the catch swallowed it, and every one of those tests silently asserted per-cookie behaviour while believing it covered exclusion. Records the registrable-domain invariant at NON_TRANSPLANTABLE_DOMAINS, since excludeOrigins derives one origin per entry, and extends the real-Electron fixture to prove http-only and domain-scoped Google cookies survive alongside the partitioned one. (cherry picked from commit e5906837e42c941432e0fb5c84012c7a2288e438) --- ...ser-cookie-import-google-exclusion.test.ts | 20 +- ...import-partition-rollback.electron.test.ts | 28 ++- ...r-cookie-import-partition.electron.test.ts | 21 +- .../browser-cookie-import-policy.test.ts | 229 +++++++++++++----- .../browser/browser-cookie-import-policy.ts | 44 +++- .../browser-cookie-import-replacement.test.ts | 8 +- .../browser/browser-cookie-import.test.ts | 8 +- src/main/browser/browser-cookie-import.ts | 6 +- 8 files changed, 279 insertions(+), 85 deletions(-) diff --git a/src/main/browser/browser-cookie-import-google-exclusion.test.ts b/src/main/browser/browser-cookie-import-google-exclusion.test.ts index 3cc329a14d7..431ca758f2f 100644 --- a/src/main/browser/browser-cookie-import-google-exclusion.test.ts +++ b/src/main/browser/browser-cookie-import-google-exclusion.test.ts @@ -120,7 +120,7 @@ describe('file import excludes the Google cookie family', () => { }) describe('native Chromium import excludes the Google cookie family', () => { - let clearStorageDataMock: ReturnType + let clearDataMock: ReturnType let cookiesGetMock: ReturnType let cookiesRemoveMock: ReturnType let cookiesSetMock: ReturnType @@ -134,7 +134,7 @@ describe('native Chromium import excludes the Google cookie family', () => { execFileSyncMock.mockImplementation(() => { throw new Error('OS browser version lookup unavailable') }) - clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + clearDataMock = vi.fn().mockResolvedValue(undefined) cookiesGetMock = vi.fn().mockResolvedValue([]) cookiesRemoveMock = vi.fn().mockResolvedValue(undefined) cookiesSetMock = vi.fn().mockResolvedValue(undefined) @@ -145,7 +145,7 @@ describe('native Chromium import excludes the Google cookie family', () => { remove: cookiesRemoveMock, set: cookiesSetMock }, - clearStorageData: clearStorageDataMock + clearData: clearDataMock }) platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') }) @@ -166,7 +166,7 @@ describe('native Chromium import excludes the Google cookie family', () => { createChromiumCookieTestDatabase(targetCookiesPath, rows).close() } - it('never clears or reconstructs live Google cookies before importing', async () => { + it('bulk clears while excluding live Google cookies before importing', async () => { const sourceCookiesPath = seedSource([ { domain: '.google.com', name: 'SID', value: 'transplanted-sid' }, { domain: '.example.com', name: 'session', value: 'new' } @@ -186,9 +186,10 @@ describe('native Chromium import excludes the Google cookie family', () => { googleCookiesSkipped: 1, domains: ['example.com'] }) - expect(clearStorageDataMock).not.toHaveBeenCalled() - expect(cookiesRemoveMock).toHaveBeenCalledOnce() - expect(cookiesRemoveMock).toHaveBeenCalledWith('https://example.com/', 'stale') + expect(clearDataMock.mock.calls).toEqual([ + [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }] + ]) + expect(cookiesRemoveMock).not.toHaveBeenCalled() expect(cookiesSetMock.mock.calls.map(([details]) => details.domain)).toEqual(['.example.com']) }) @@ -239,7 +240,7 @@ describe('native Chromium import excludes the Google cookie family', () => { domains: [] }) expect(execFileSyncMock).not.toHaveBeenCalled() - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).not.toHaveBeenCalled() expect(cookiesSetMock).not.toHaveBeenCalled() }) @@ -284,12 +285,13 @@ describe('native Chromium import excludes the Google cookie family', () => { throw new Error('cookie store unavailable') } }) + clearDataMock.mockRejectedValue(new Error('storage busy')) const result = await importCookiesFromBrowser(chromeBrowser(sourceCookiesPath), 'persist:test') expect(result).toMatchObject({ ok: false }) expect(result.ok || result.reason).toContain('Could not clear existing cookies') - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).toHaveBeenCalledOnce() expect(cookiesRemoveMock.mock.calls.map(([, name]) => name)).toEqual(['removed-first', 'stale']) expect(cookiesSetMock).not.toHaveBeenCalled() expect(setPendingCookieImportMock).not.toHaveBeenCalled() diff --git a/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts b/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts index 5ee11981251..bbd0d9a8a25 100644 --- a/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts +++ b/src/main/browser/browser-cookie-import-partition-rollback.electron.test.ts @@ -20,8 +20,10 @@ type CdpCookie = { name: string; value: string; partitionKey?: Record | undefined clearError: string | null + bulkClearCalls: number remainingChips: CdpCookie[] remainingPlain: CdpCookie[] + remainingExcluded: CdpCookie[] } const EXPECTED_PARTITION_KEY = { @@ -33,7 +35,7 @@ function buildFixtureMain(policyPath: string, resultPath: string): string { return ` const { app, BrowserWindow, session } = require('electron') const { writeFileSync } = require('node:fs') -const { removeAllCookiesExcept, isNonTransplantableCookieDomain } = require(${JSON.stringify(policyPath)}) +const { removeTransplantableCookies } = require(${JSON.stringify(policyPath)}) const resultPath = ${JSON.stringify(resultPath)} let currentStep = 'starting' const mark = (step) => { @@ -76,6 +78,9 @@ async function run() { await targetSession.cookies.set({ url: 'https://plain.example/', name: 'plain', value: 'stale', secure: true }) await targetSession.cookies.set({ url: 'https://victim.example/', name: 'victim', value: 'stale', secure: true }) + // Why: keep a live excluded cookie in the partial-failure fixture so the fallback proves it + // preserves that cookie while deleting the removable ones it can reach. + await targetSession.cookies.set({ url: 'https://accounts.google.com/', name: 'SID', value: 'live', secure: true }) mark('removable cookies set') const store = { @@ -88,9 +93,20 @@ async function run() { } } + // Why: the bulk clear is the ordinary path now, so rejecting it is what routes this fixture onto + // the per-cookie fallback where a partial failure — and the old rollback — could happen at all. + let bulkClearCalls = 0 + const clearSession = { + cookies: store, + clearData: async () => { + bulkClearCalls++ + throw new Error('forced bulk clear failure') + } + } + let clearError = null try { - await removeAllCookiesExcept(store, (cookie) => isNonTransplantableCookieDomain(cookie.domain ?? '')) + await removeTransplantableCookies(clearSession) } catch (error) { clearError = String(error?.message || error) } @@ -104,8 +120,10 @@ async function run() { writeFileSync(resultPath, JSON.stringify({ beforePartitionKey: beforeChips.partitionKey, clearError, + bulkClearCalls, remainingChips: project('chips-auth'), - remainingPlain: project('plain') + remainingPlain: project('plain'), + remainingExcluded: project('SID') })) debug.detach() window.destroy() @@ -164,6 +182,10 @@ describe('non-Google partitioned cookie under a failed Electron cookie clear', ( const result = await runFixture() expect(result.beforePartitionKey).toEqual(EXPECTED_PARTITION_KEY) + // Why (STA-4065): the fallback is only reachable once the bulk clear has been tried and failed; + // if it ever succeeded here the partial-failure assertions below would be vacuous. + expect(result.bulkClearCalls).toBe(1) + expect(result.remainingExcluded.map(({ name }) => name)).toEqual(['SID']) expect(result.clearError).toContain('Could not clear existing cookies') // STA-4061: rollback rebuilt this cookie through cookies.set, which drops partitionKey. expect( diff --git a/src/main/browser/browser-cookie-import-partition.electron.test.ts b/src/main/browser/browser-cookie-import-partition.electron.test.ts index 494ebbd9e2b..e844341e12f 100644 --- a/src/main/browser/browser-cookie-import-partition.electron.test.ts +++ b/src/main/browser/browser-cookie-import-partition.electron.test.ts @@ -74,6 +74,20 @@ async function run() { value: 'remove-me', secure: true }) + // Why: the excluded origin is HTTPS, so prove registrable-domain matching also keeps HTTP. + await targetSession.cookies.set({ + url: 'http://mail.google.com/', + name: 'http-google', + value: 'live-http-value', + secure: false + }) + await targetSession.cookies.set({ + url: 'https://google.com/', + domain: '.google.com', + name: 'domain-google', + value: 'live-domain-value', + secure: true + }) mark('ordinary cookie set') const beforeCookies = (await debug.sendCommand('Network.getAllCookies')).cookies @@ -189,6 +203,11 @@ describe('native Chromium excluded partition cookie under Electron', () => { ok: true, summary: { importedCookies: 1, domains: ['example.com'] } }) - expect(result.remainingNames).toEqual(['imported', 'partitioned-google']) + expect(result.remainingNames).toEqual([ + 'domain-google', + 'http-google', + 'imported', + 'partitioned-google' + ]) }, 90_000) }) diff --git a/src/main/browser/browser-cookie-import-policy.test.ts b/src/main/browser/browser-cookie-import-policy.test.ts index f007c07d8ff..51a1a6107d5 100644 --- a/src/main/browser/browser-cookie-import-policy.test.ts +++ b/src/main/browser/browser-cookie-import-policy.test.ts @@ -1,4 +1,4 @@ -import { describe, expect, it, vi } from 'vitest' +import { describe, expect, it, vi, type Mock } from 'vitest' import { DatabaseSync } from 'node:sqlite' import type { Cookie } from 'electron' import { @@ -6,7 +6,7 @@ import { isNonTransplantableCookieDomain, NON_TRANSPLANTABLE_HOST_KEY_SQL, normalizeCookieDomain, - removeAllCookiesExcept, + removeTransplantableCookies, replaceCookiesForImportedDomains } from './browser-cookie-import-policy' @@ -207,78 +207,193 @@ describe('NON_TRANSPLANTABLE_HOST_KEY_SQL', () => { }) }) -describe('removeAllCookiesExcept', () => { - it('never removes or reconstructs excluded Google cookies', async () => { - const get = vi - .fn() - .mockResolvedValue([ - cookie('.google.com', 'SID'), - cookie('accounts.google.com', 'ACCOUNT'), - cookie('.example.com', 'session'), - cookie('other.test', 'tracker', '/scoped') - ]) - const remove = vi.fn().mockResolvedValue(undefined) - const set = vi.fn().mockResolvedValue(undefined) +type CookieClearMocks = { + get: Mock + remove: Mock + set: Mock + clearData: Mock +} - await removeAllCookiesExcept({ get, remove }, (existingCookie) => - isNonTransplantableCookieDomain(existingCookie.domain ?? '') +describe('removeTransplantableCookies', () => { + const rejectingBulkClear = () => vi.fn().mockRejectedValue(new Error('storage busy')) + + function clearSession(cookies: Cookie[], overrides: Partial = {}) { + const store = { + get: vi.fn().mockResolvedValue(cookies), + remove: vi.fn().mockResolvedValue(undefined), + set: vi.fn().mockResolvedValue(undefined), + ...overrides + } + const clearData = overrides.clearData ?? vi.fn().mockResolvedValue(undefined) + return { + session: { cookies: store, clearData }, + get: store.get, + remove: store.remove, + set: store.set, + clearData + } + } + + // Why (STA-4065): the bulk call is the ordinary path even when the jar holds cookies to keep — + // excludeOrigins preserves the whole google.com family, verified against real Electron. + it('clears a jar holding Google cookies in one call that excludes them', async () => { + const { session, remove, set, clearData } = clearSession([ + cookie('.google.com', 'SID'), + cookie('accounts.google.com', 'ACCOUNT'), + cookie('.example.com', 'session'), + cookie('other.test', 'tracker', '/scoped') + ]) + + await removeTransplantableCookies(session) + + expect(clearData.mock.calls).toEqual([ + [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }] + ]) + expect(remove).not.toHaveBeenCalled() + expect(set).not.toHaveBeenCalled() + }) + + it('bulk clears in one call when the jar holds nothing to preserve', async () => { + const { session, remove, set, clearData } = clearSession([ + cookie('.example.com', 'session'), + cookie('other.test', 'tracker', '/scoped'), + cookie('notgoogle.com', 'lookalike') + ]) + + await removeTransplantableCookies(session) + + expect(clearData.mock.calls).toEqual([ + [{ dataTypes: ['cookies'], excludeOrigins: ['https://google.com'] }] + ]) + expect(remove).not.toHaveBeenCalled() + expect(set).not.toHaveBeenCalled() + }) + + it('touches nothing when the jar is already empty', async () => { + const { session, remove, clearData } = clearSession([]) + + await removeTransplantableCookies(session) + + expect(clearData).not.toHaveBeenCalled() + expect(remove).not.toHaveBeenCalled() + }) + + it('bulk clears cookies the per-cookie path could never address', async () => { + const { session, clearData } = clearSession([ + { ...cookie('.example.com', 'session'), domain: '' } + ]) + + await removeTransplantableCookies(session) + + expect(clearData).toHaveBeenCalledOnce() + }) + + it('falls back to per-cookie removal when the bulk clear rejects', async () => { + const { session, remove, clearData } = clearSession( + [cookie('.example.com', 'session'), cookie('other.test', 'tracker')], + { clearData: rejectingBulkClear() } ) + await removeTransplantableCookies(session) + + expect(clearData).toHaveBeenCalledOnce() expect(remove.mock.calls).toEqual([ ['https://example.com/', 'session'], - ['https://other.test/scoped', 'tracker'] + ['https://other.test/', 'tracker'] ]) + }) + + it('re-reads the jar after a rejected bulk clear', async () => { + const beforeAttempt = [cookie('.removed.test', 'gone-before-fallback')] + const afterAttempt = [ + cookie('.google.com', 'SID'), + cookie('.survivor.test', 'survived'), + cookie('.arrived.test', 'arrived-during-clear') + ] + const get = vi.fn().mockResolvedValueOnce(beforeAttempt).mockResolvedValueOnce(afterAttempt) + const { session, remove } = clearSession(beforeAttempt, { + get, + clearData: rejectingBulkClear() + }) + + await removeTransplantableCookies(session) + + expect(get).toHaveBeenCalledTimes(2) + expect(remove.mock.calls).toEqual([ + ['https://survivor.test/', 'survived'], + ['https://arrived.test/', 'arrived-during-clear'] + ]) + }) + + // Why: the fallback carries the same exclusion as the bulk call, so a rejected clearData must + // not become the path that finally deletes a live Google session. + it('still preserves Google cookies on the per-cookie fallback', async () => { + const { session, remove, set } = clearSession( + [ + cookie('.google.com', 'SID'), + cookie('accounts.google.com', 'ACCOUNT'), + cookie('.example.com', 'session') + ], + { clearData: rejectingBulkClear() } + ) + + await removeTransplantableCookies(session) + + expect(remove.mock.calls).toEqual([['https://example.com/', 'session']]) expect(set).not.toHaveBeenCalled() }) // Why (STA-4061): reconstructing a removed cookie loses its partition key, and the snapshot // cannot say which cookies had one, so a failed clear must stay failed. it('never reconstructs removed cookies when another removal fails', async () => { - const get = vi - .fn() - .mockResolvedValue([ + const { session, remove, set } = clearSession( + [ cookie('.google.com', 'SID'), cookie('.example.com', 'first', '/one'), cookie('.example.com', 'second', '/two'), cookie('.example.com', 'third', '/three') - ]) - const remove = vi.fn().mockImplementation(async (_url: string, name: string) => { - if (name === 'second') { - throw new Error('store unavailable') + ], + { + clearData: rejectingBulkClear(), + remove: vi.fn().mockImplementation(async (_url: string, name: string) => { + if (name === 'second') { + throw new Error('store unavailable') + } + }) } - }) - const set = vi.fn().mockResolvedValue(undefined) + ) - await expect( - removeAllCookiesExcept({ get, remove }, (existingCookie) => - isNonTransplantableCookieDomain(existingCookie.domain ?? '') - ) - ).rejects.toThrow('the session was left partially cleared') + await expect(removeTransplantableCookies(session)).rejects.toThrow( + 'the session was left partially cleared' + ) expect(remove).toHaveBeenCalledTimes(3) expect(set).not.toHaveBeenCalled() }) it('bounds parallel removals so large cookie jars do not clear serially or fan out', async () => { - const get = vi - .fn() - .mockResolvedValue( - Array.from({ length: 12 }, (_, index) => cookie('.example.com', `${index}`)) - ) let releaseRemovals: (() => void) | undefined const removalsReleased = new Promise((resolve) => { releaseRemovals = resolve }) let active = 0 let maxActive = 0 - const remove = vi.fn().mockImplementation(async () => { - active++ - maxActive = Math.max(maxActive, active) - await removalsReleased - active-- - }) - const set = vi.fn().mockResolvedValue(undefined) + const { session, remove, set } = clearSession( + [ + cookie('.google.com', 'SID'), + ...Array.from({ length: 12 }, (_, index) => cookie('.example.com', `${index}`)) + ], + { + clearData: rejectingBulkClear(), + remove: vi.fn().mockImplementation(async () => { + active++ + maxActive = Math.max(maxActive, active) + await removalsReleased + active-- + }) + } + ) - const clearing = removeAllCookiesExcept({ get, remove }, () => false) + const clearing = removeTransplantableCookies(session) await vi.waitFor(() => expect(remove).toHaveBeenCalledTimes(8)) expect(maxActive).toBe(8) releaseRemovals?.() @@ -289,22 +404,26 @@ describe('removeAllCookiesExcept', () => { }) it('serializes cookies that share Electron removal coordinates', async () => { - const get = vi - .fn() - .mockResolvedValue([ - cookie('.example.com', 'session'), - { ...cookie('example.com', 'session'), hostOnly: true } - ]) let releaseFirst: (() => void) | undefined const firstReleased = new Promise((resolve) => { releaseFirst = resolve }) - const remove = vi - .fn() - .mockImplementationOnce(() => firstReleased) - .mockResolvedValueOnce(undefined) + const { session, remove } = clearSession( + [ + cookie('.google.com', 'SID'), + cookie('.example.com', 'session'), + { ...cookie('example.com', 'session'), hostOnly: true } + ], + { + clearData: rejectingBulkClear(), + remove: vi + .fn() + .mockImplementationOnce(() => firstReleased) + .mockResolvedValueOnce(undefined) + } + ) - const clearing = removeAllCookiesExcept({ get, remove }, () => false) + const clearing = removeTransplantableCookies(session) await vi.waitFor(() => expect(remove).toHaveBeenCalledOnce()) releaseFirst?.() await clearing diff --git a/src/main/browser/browser-cookie-import-policy.ts b/src/main/browser/browser-cookie-import-policy.ts index 27be9512218..6f81407f4a9 100644 --- a/src/main/browser/browser-cookie-import-policy.ts +++ b/src/main/browser/browser-cookie-import-policy.ts @@ -1,4 +1,4 @@ -import type { Cookie, Cookies } from 'electron' +import type { Cookie, Cookies, Session } from 'electron' import { parse as parseDomain } from 'psl' import { mapSettledWithConcurrency } from '../../shared/map-with-concurrency' @@ -59,11 +59,16 @@ export function normalizeCookieImportDomain(domain: string): string | null { // it is copied. Signing in directly inside Orca is the only path that produces a working // session, so an import must never write these cookies and never remove them either — the // live session is always more valuable than anything an import could put in its place. +// Entries must be canonical lowercase ASCII (punycode) registrable domains, never subdomains or +// public suffixes, because clearData derives one excluded origin and matches at that boundary. // Adding a site is one entry here. // youtube.com is deliberately NOT listed: YouTube accepts a transplanted session and re-issues // its cookies via the accounts.youtube.com relay, so excluding it would silently drop imports // users actually asked for. const NON_TRANSPLANTABLE_DOMAINS = ['google.com'] as const +const NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS = NON_TRANSPLANTABLE_DOMAINS.map( + (root) => `https://${root}` +) const COOKIE_CLEAR_CONCURRENCY = 8 export function isNonTransplantableCookieDomain(domain: string): boolean { @@ -196,20 +201,49 @@ export async function restoreImportedDomainCookies( } } +// Why (STA-4061): 'set' stays out so the lossy partition-dropping reconstruction cannot return. +export type CookieClearSession = { + cookies: Pick + clearData: Session['clearData'] +} + // Why: Electron cannot round-trip partition identity, so excluded cookies must never be removed. // Why (STA-4061): the same gap forbids rolling a partial clear back. cookies.get() omits // partitionKey and cookies.set() silently drops it, so every reconstruction is a coin flip that // can downgrade a partitioned (CHIPS) cookie into an unpartitioned one — and nothing in the // snapshot says which cookies are at risk. A partially cleared jar is a retryable import failure; // a downgraded cookie is unrecoverable auth-state corruption that survives restart. -export async function removeAllCookiesExcept( - store: Pick, - isExcluded: (cookie: Cookie) => boolean +// Why (STA-4065): the exclusion is module state rather than a parameter so the predicate and the +// origins the bulk clear preserves cannot drift apart — a caller-supplied predicate that disagreed +// with NON_TRANSPLANTABLE_DOMAINS would silently delete a cookie the bulk call is meant to keep. +export async function removeTransplantableCookies( + targetSession: CookieClearSession ): Promise { + const store = targetSession.cookies + const initialCookies = await store.get({}) + if (initialCookies.length === 0) { + return + } + + // Why (STA-4065): measured on Electron 43, excludeOrigins preserves the whole registrable + // family — host, leading-dot, subdomain, and partitioned Google cookies — so one call replaces a + // remove() per cookie even when the jar holds cookies to keep. That is the ordinary case here: + // this import exists for Google, so a Google cookie is usually present. + try { + await targetSession.clearData({ + dataTypes: ['cookies'], + excludeOrigins: NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS + }) + return + } catch { + // Why: a rejected bulk clear can still have changed the jar, so the fallback must act on the + // survivors rather than stale removal coordinates from before the attempt. + } + const existingCookies = await store.get({}) const removableGroups = new Map() for (const cookie of existingCookies) { - if (isExcluded(cookie)) { + if (isNonTransplantableCookieDomain(cookie.domain ?? '')) { continue } const domain = cookie.domain ? normalizeCookieDomain(cookie.domain) : null diff --git a/src/main/browser/browser-cookie-import-replacement.test.ts b/src/main/browser/browser-cookie-import-replacement.test.ts index 8b5c07bb057..db70922ccb5 100644 --- a/src/main/browser/browser-cookie-import-replacement.test.ts +++ b/src/main/browser/browser-cookie-import-replacement.test.ts @@ -168,7 +168,7 @@ describe('validated cookie replacement', () => { }) describe('native Chromium integrity-cookie accounting', () => { - let clearStorageDataMock: ReturnType + let clearDataMock: ReturnType let cookiesSetMock: ReturnType let tmpDir: string @@ -180,7 +180,7 @@ describe('native Chromium integrity-cookie accounting', () => { }) clearPendingCookieImportMock.mockClear() setPendingCookieImportMock.mockClear() - clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + clearDataMock = vi.fn().mockResolvedValue(undefined) cookiesSetMock = vi.fn().mockResolvedValue(undefined) sessionFromPartitionMock.mockReset().mockReturnValue({ cookies: { @@ -189,7 +189,7 @@ describe('native Chromium integrity-cookie accounting', () => { remove: vi.fn().mockResolvedValue(undefined), set: cookiesSetMock }, - clearStorageData: clearStorageDataMock + clearData: clearDataMock }) }) @@ -252,7 +252,7 @@ describe('native Chromium integrity-cookie accounting', () => { googleCookiesSkipped: 2, domains: [] }) - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).not.toHaveBeenCalled() expect(cookiesSetMock).not.toHaveBeenCalled() expect(setPendingCookieImportMock).not.toHaveBeenCalled() expect(clearPendingCookieImportMock).not.toHaveBeenCalled() diff --git a/src/main/browser/browser-cookie-import.test.ts b/src/main/browser/browser-cookie-import.test.ts index d2d4ffd4836..90514321d54 100644 --- a/src/main/browser/browser-cookie-import.test.ts +++ b/src/main/browser/browser-cookie-import.test.ts @@ -423,7 +423,7 @@ describe('importCookiesFromBrowser Chromium', () => { let cookiesSetMock: ReturnType let cookiesRemoveMock: ReturnType let cookiesFlushStoreMock: ReturnType - let clearStorageDataMock: ReturnType + let clearDataMock: ReturnType let setUserAgentMock: ReturnType beforeEach(() => { @@ -431,7 +431,7 @@ describe('importCookiesFromBrowser Chromium', () => { cookiesSetMock = vi.fn().mockResolvedValue(undefined) cookiesRemoveMock = vi.fn().mockResolvedValue(undefined) cookiesFlushStoreMock = vi.fn().mockResolvedValue(undefined) - clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + clearDataMock = vi.fn().mockResolvedValue(undefined) setUserAgentMock = vi.fn() appGetPathMock.mockReset() appGetPathMock.mockReturnValue(join(tmpDir, 'userData')) @@ -450,7 +450,7 @@ describe('importCookiesFromBrowser Chromium', () => { remove: cookiesRemoveMock, flushStore: cookiesFlushStoreMock }, - clearStorageData: clearStorageDataMock, + clearData: clearDataMock, setUserAgent: setUserAgentMock }) }) @@ -511,7 +511,7 @@ describe('importCookiesFromBrowser Chromium', () => { ['', '-wal', '-shm'].map((suffix) => readFileSync(sourceCookiesPath + suffix)) ).toEqual(sourceFilesBefore) expect(cookiesRemoveMock).not.toHaveBeenCalled() - expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(clearDataMock).not.toHaveBeenCalled() // Why: STA-3514 — imports must never impersonate the source browser; the // session keeps the engine UA the registry set at startup. expect(setUserAgentMock).not.toHaveBeenCalled() diff --git a/src/main/browser/browser-cookie-import.ts b/src/main/browser/browser-cookie-import.ts index 2a7bed19950..3c3a701f953 100644 --- a/src/main/browser/browser-cookie-import.ts +++ b/src/main/browser/browser-cookie-import.ts @@ -80,7 +80,7 @@ import { NON_TRANSPLANTABLE_HOST_KEY_SQL, normalizeCookieDomain, normalizeCookieImportDomain, - removeAllCookiesExcept, + removeTransplantableCookies, replaceCookiesForImportedDomains, restoreImportedDomainCookies, type CookieImportMode @@ -1780,9 +1780,7 @@ export async function importCookiesFromBrowser( // Why: clear stale cookies first; mixing them with the imported set makes sites reject the // session. Non-transplantable families are exempt — nothing was imported for them, and their // live session is the only one that works. - await removeAllCookiesExcept(targetSession.cookies, (cookie) => - isNonTransplantableCookieDomain(cookie.domain ?? '') - ) + await removeTransplantableCookies(targetSession) diag( ` cleared existing session cookies before loading ${decryptedCookies.length} imported cookies` )