mirror of
https://github.com/stablyai/orca.git
synced 2026-10-22 00:03:47 +00:00
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 e5906837e4)
This commit is contained in:
@@ -120,7 +120,7 @@ describe('file import excludes the Google cookie family', () => {
|
||||
})
|
||||
|
||||
describe('native Chromium import excludes the Google cookie family', () => {
|
||||
let clearStorageDataMock: ReturnType<typeof vi.fn>
|
||||
let clearDataMock: ReturnType<typeof vi.fn>
|
||||
let cookiesGetMock: ReturnType<typeof vi.fn>
|
||||
let cookiesRemoveMock: ReturnType<typeof vi.fn>
|
||||
let cookiesSetMock: ReturnType<typeof vi.fn>
|
||||
@@ -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()
|
||||
|
||||
@@ -20,8 +20,10 @@ type CdpCookie = { name: string; value: string; partitionKey?: Record<string, un
|
||||
type FixtureResult = {
|
||||
beforePartitionKey: Record<string, unknown> | 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(
|
||||
|
||||
@@ -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)
|
||||
})
|
||||
|
||||
@@ -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<CookieClearMocks> = {}) {
|
||||
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<void>((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<void>((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
|
||||
|
||||
@@ -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<Cookies, 'get' | 'remove'>
|
||||
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<Cookies, 'get' | 'remove'>,
|
||||
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<void> {
|
||||
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<string, { cookie: Cookie; url: string }[]>()
|
||||
for (const cookie of existingCookies) {
|
||||
if (isExcluded(cookie)) {
|
||||
if (isNonTransplantableCookieDomain(cookie.domain ?? '')) {
|
||||
continue
|
||||
}
|
||||
const domain = cookie.domain ? normalizeCookieDomain(cookie.domain) : null
|
||||
|
||||
@@ -168,7 +168,7 @@ describe('validated cookie replacement', () => {
|
||||
})
|
||||
|
||||
describe('native Chromium integrity-cookie accounting', () => {
|
||||
let clearStorageDataMock: ReturnType<typeof vi.fn>
|
||||
let clearDataMock: ReturnType<typeof vi.fn>
|
||||
let cookiesSetMock: ReturnType<typeof vi.fn>
|
||||
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()
|
||||
|
||||
@@ -423,7 +423,7 @@ describe('importCookiesFromBrowser Chromium', () => {
|
||||
let cookiesSetMock: ReturnType<typeof vi.fn>
|
||||
let cookiesRemoveMock: ReturnType<typeof vi.fn>
|
||||
let cookiesFlushStoreMock: ReturnType<typeof vi.fn>
|
||||
let clearStorageDataMock: ReturnType<typeof vi.fn>
|
||||
let clearDataMock: ReturnType<typeof vi.fn>
|
||||
let setUserAgentMock: ReturnType<typeof vi.fn>
|
||||
|
||||
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()
|
||||
|
||||
@@ -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`
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user