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 f3b4a49ed8d..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 @@ -78,8 +78,8 @@ 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 (STA-4065): an excluded cookie is what forces the per-cookie path; without one the bulk - // fast path clears the jar in a single call and never exercises a partial-failure clear. + // 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') 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 5fa7c0d4fc0..e844341e12f 100644 --- a/src/main/browser/browser-cookie-import-partition.electron.test.ts +++ b/src/main/browser/browser-cookie-import-partition.electron.test.ts @@ -20,7 +20,6 @@ type FixtureResult = { before: Record after: Record electronCookieKeys: string[] - unfilteredNames: string[] importResult: Record remainingNames: string[] } @@ -75,13 +74,26 @@ 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 const before = beforeCookies.find((cookie) => cookie.name === 'partitioned-google') const electronCookie = (await targetSession.cookies.get({ name: 'partitioned-google' }))[0] if (!before || !electronCookie) throw new Error('Partitioned fixture cookie was not created') - const unfilteredNames = (await targetSession.cookies.get({})).map((cookie) => cookie.name).sort() mark('cookies read') const importResult = await importCookiesFromBrowser({ @@ -102,7 +114,6 @@ async function run() { before, after, electronCookieKeys: Object.keys(electronCookie).sort(), - unfilteredNames, importResult, remainingNames: afterCookies.map((cookie) => cookie.name).sort() })) @@ -187,14 +198,16 @@ describe('native Chromium excluded partition cookie under Electron', () => { hasCrossSiteAncestor: true }) expect(result.electronCookieKeys).not.toContain('partitionKey') - // Why: the bulk-clear fast path is gated on an unfiltered get() seeing every excluded cookie, - // so a partitioned one going missing here would silently arm a jar-wide wipe. - expect(result.unfilteredNames).toEqual(['partitioned-google', 'stale']) expect(result.after).toEqual(result.before) expect(result.importResult).toMatchObject({ 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 7b2fad0bfdc..51a1a6107d5 100644 --- a/src/main/browser/browser-cookie-import-policy.test.ts +++ b/src/main/browser/browser-cookie-import-policy.test.ts @@ -303,6 +303,28 @@ describe('removeTransplantableCookies', () => { ]) }) + 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 () => { diff --git a/src/main/browser/browser-cookie-import-policy.ts b/src/main/browser/browser-cookie-import-policy.ts index 315b5eccdd6..6f81407f4a9 100644 --- a/src/main/browser/browser-cookie-import-policy.ts +++ b/src/main/browser/browser-cookie-import-policy.ts @@ -59,6 +59,8 @@ 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 @@ -218,8 +220,8 @@ export async function removeTransplantableCookies( targetSession: CookieClearSession ): Promise { const store = targetSession.cookies - const existingCookies = await store.get({}) - if (existingCookies.length === 0) { + const initialCookies = await store.get({}) + if (initialCookies.length === 0) { return } @@ -234,10 +236,11 @@ export async function removeTransplantableCookies( }) return } catch { - // Why: a rejected bulk clear can still have emptied part of the jar, so fall through to the - // per-cookie path and let it re-remove whatever survived. + // 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 (isNonTransplantableCookieDomain(cookie.domain ?? '')) { 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()