fix(browser): protect cookie clear fast-path race (STA-4065)

This commit is contained in:
Brennan Benson
2026-08-12 18:57:46 -07:00
parent 5ee9cb2ff0
commit 54163cd3aa
2 changed files with 26 additions and 17 deletions
@@ -211,7 +211,7 @@ type CookieClearMocks = {
get: Mock
remove: Mock
set: Mock
clearStorageData: Mock
clearData: Mock
}
describe('removeAllCookiesExcept', () => {
@@ -222,13 +222,13 @@ describe('removeAllCookiesExcept', () => {
set: vi.fn().mockResolvedValue(undefined),
...overrides
}
const clearStorageData = overrides.clearStorageData ?? vi.fn().mockResolvedValue(undefined)
const clearData = overrides.clearData ?? vi.fn().mockResolvedValue(undefined)
return {
session: { cookies: store, clearStorageData },
session: { cookies: store, clearData },
get: store.get,
remove: store.remove,
set: store.set,
clearStorageData
clearData
}
}
@@ -236,7 +236,7 @@ describe('removeAllCookiesExcept', () => {
isNonTransplantableCookieDomain(existingCookie.domain ?? '')
it('never removes or reconstructs excluded Google cookies', async () => {
const { session, remove, set, clearStorageData } = clearSession([
const { session, remove, set, clearData } = clearSession([
cookie('.google.com', 'SID'),
cookie('accounts.google.com', 'ACCOUNT'),
cookie('.example.com', 'session'),
@@ -245,7 +245,7 @@ describe('removeAllCookiesExcept', () => {
await removeAllCookiesExcept(session, excludesNonTransplantable)
expect(clearStorageData).not.toHaveBeenCalled()
expect(clearData).not.toHaveBeenCalled()
expect(remove.mock.calls).toEqual([
['https://example.com/', 'session'],
['https://other.test/scoped', 'tracker']
@@ -254,7 +254,7 @@ describe('removeAllCookiesExcept', () => {
})
it('bulk clears in one call when the jar holds nothing to preserve', async () => {
const { session, remove, set, clearStorageData } = clearSession([
const { session, remove, set, clearData } = clearSession([
cookie('.example.com', 'session'),
cookie('other.test', 'tracker', '/scoped'),
cookie('notgoogle.com', 'lookalike')
@@ -262,39 +262,41 @@ describe('removeAllCookiesExcept', () => {
await removeAllCookiesExcept(session, excludesNonTransplantable)
expect(clearStorageData.mock.calls).toEqual([[{ storages: ['cookies'] }]])
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, clearStorageData } = clearSession([])
const { session, remove, clearData } = clearSession([])
await removeAllCookiesExcept(session, excludesNonTransplantable)
expect(clearStorageData).not.toHaveBeenCalled()
expect(clearData).not.toHaveBeenCalled()
expect(remove).not.toHaveBeenCalled()
})
it('bulk clears cookies the per-cookie path could never address', async () => {
const { session, clearStorageData } = clearSession([
const { session, clearData } = clearSession([
{ ...cookie('.example.com', 'session'), domain: '' }
])
await removeAllCookiesExcept(session, excludesNonTransplantable)
expect(clearStorageData).toHaveBeenCalledOnce()
expect(clearData).toHaveBeenCalledOnce()
})
it('falls back to per-cookie removal when the bulk clear rejects', async () => {
const { session, remove, clearStorageData } = clearSession(
const { session, remove, clearData } = clearSession(
[cookie('.example.com', 'session'), cookie('other.test', 'tracker')],
{ clearStorageData: vi.fn().mockRejectedValue(new Error('storage busy')) }
{ clearData: vi.fn().mockRejectedValue(new Error('storage busy')) }
)
await removeAllCookiesExcept(session, excludesNonTransplantable)
expect(clearStorageData).toHaveBeenCalledOnce()
expect(clearData).toHaveBeenCalledOnce()
expect(remove.mock.calls).toEqual([
['https://example.com/', 'session'],
['https://other.test/', 'tracker']
@@ -64,6 +64,9 @@ export function normalizeCookieImportDomain(domain: string): string | null {
// 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 {
@@ -199,7 +202,7 @@ 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'>
clearStorageData: Session['clearStorageData']
clearData: Session['clearData']
}
// Why: Electron cannot round-trip partition identity, so excluded cookies must never be removed.
@@ -219,7 +222,11 @@ export async function removeAllCookiesExcept(
// one bulk clear can replace a remove() per cookie. An empty jar needs neither.
if (existingCookies.length > 0 && !existingCookies.some(isExcluded)) {
try {
await targetSession.clearStorageData({ storages: ['cookies'] })
// Why: exclusions protect a non-transplantable cookie arriving between snapshot and clear.
await targetSession.clearData({
dataTypes: ['cookies'],
excludeOrigins: NON_TRANSPLANTABLE_CLEAR_EXCLUDED_ORIGINS
})
return
} catch {
// Why: a rejected bulk clear can still have emptied part of the jar, so fall through to the