From 686f5dca1a4fc181b7d77042d1d6089dd57ffb9d Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Tue, 11 Aug 2026 16:39:17 -0700 Subject: [PATCH] fix(browser): exclude Google cookies from imports - direct sign-in is the only Google path (STA-3811) (#13670) * fix(browser): exclude Google cookies from imports (STA-3811) Imports never write and never remove a google.com-family cookie, on any path. Signing in directly inside Orca is the only Google session that survives, so the live jar always beats anything an import could plant. * fix(browser): roll back selective cookie clears --- ...ser-cookie-import-google-exclusion.test.ts | 239 ++++++++++++++++++ .../browser-cookie-import-policy.test.ts | 161 ++++++++++++ .../browser/browser-cookie-import-policy.ts | 80 ++++++ .../browser-cookie-import-replacement.test.ts | 27 +- .../browser/browser-cookie-import.test.ts | 5 +- src/main/browser/browser-cookie-import.ts | 43 +++- ...r-cookie-import-google-disclosure.test.tsx | 138 ++++++++++ .../browser-toolbar-menu-dropdown.tsx | 7 + .../components/settings/BrowserProfileRow.tsx | 7 + src/renderer/src/i18n/locales/en.json | 6 +- 10 files changed, 687 insertions(+), 26 deletions(-) create mode 100644 src/main/browser/browser-cookie-import-google-exclusion.test.ts create mode 100644 src/renderer/src/components/browser-cookie-import-google-disclosure.test.tsx diff --git a/src/main/browser/browser-cookie-import-google-exclusion.test.ts b/src/main/browser/browser-cookie-import-google-exclusion.test.ts new file mode 100644 index 00000000000..24472f8c828 --- /dev/null +++ b/src/main/browser/browser-cookie-import-google-exclusion.test.ts @@ -0,0 +1,239 @@ +/** + * STA-3811: an import must never write and never remove a google.com-family cookie, on any + * path. Removing 'google.com' from NON_TRANSPLANTABLE_DOMAINS flips every test here red. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const { + appGetPathMock, + clearPendingCookieImportMock, + execFileSyncMock, + sessionFromPartitionMock, + setPendingCookieImportMock +} = vi.hoisted(() => ({ + appGetPathMock: vi.fn(), + clearPendingCookieImportMock: vi.fn(), + execFileSyncMock: vi.fn(), + sessionFromPartitionMock: vi.fn(), + setPendingCookieImportMock: vi.fn() +})) + +vi.mock('./browser-session-registry', () => ({ + browserSessionRegistry: { + setPendingCookieImport: setPendingCookieImportMock, + clearPendingCookieImport: clearPendingCookieImportMock + } +})) +vi.mock('node:child_process', () => ({ execFileSync: execFileSyncMock })) +vi.mock('electron', () => ({ + app: { getPath: appGetPathMock }, + dialog: { showOpenDialog: vi.fn() }, + session: { fromPartition: sessionFromPartitionMock } +})) + +import { importCookiesFromBrowser, importCookiesFromFile } from './browser-cookie-import' +import { createChromiumCookieTestDatabase } from './browser-cookie-import-test-database' +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { DatabaseSync } from 'node:sqlite' + +function existingCookie(domain: string, name: string) { + return { domain, name, path: '/', secure: true, sameSite: 'unspecified', value: 'live' } +} + +const INCOMING = [ + { domain: '.google.com', name: 'SAPISID', value: 'transplanted', secure: true }, + { domain: 'accounts.google.com', name: '__Secure-1PSID', value: 'transplanted', secure: true }, + { domain: '.youtube.com', name: 'LOGIN_INFO', value: 'transplanted', secure: true }, + { domain: '.linear.app', name: 'session', value: 'new', secure: true } +] + +describe('file import excludes the Google cookie family', () => { + let cookiesGetMock: ReturnType + let cookiesRemoveMock: ReturnType + let cookiesSetMock: ReturnType + let tmpDir: string + + beforeEach(() => { + vi.clearAllMocks() + tmpDir = mkdtempSync(join(tmpdir(), 'orca-google-exclusion-file-')) + cookiesGetMock = vi.fn().mockResolvedValue([]) + cookiesRemoveMock = vi.fn().mockResolvedValue(undefined) + cookiesSetMock = vi.fn().mockResolvedValue(undefined) + sessionFromPartitionMock.mockReturnValue({ + cookies: { get: cookiesGetMock, remove: cookiesRemoveMock, set: cookiesSetMock } + }) + }) + + afterEach(() => { + rmSync(tmpDir, { recursive: true, force: true }) + }) + + function writeCookies(cookies: unknown[]): string { + const filePath = join(tmpDir, 'cookies.json') + writeFileSync(filePath, JSON.stringify(cookies)) + return filePath + } + + it('writes no Google cookie and removes none, whatever is already in the jar', async () => { + cookiesGetMock.mockResolvedValue([ + existingCookie('.google.com', 'SID'), + existingCookie('accounts.google.com', '__Secure-1PSID'), + existingCookie('.linear.app', 'old-linear') + ]) + + const result = await importCookiesFromFile(writeCookies(INCOMING), 'persist:test') + + expect(result.ok && result.summary).toMatchObject({ + totalCookies: 4, + importedCookies: 2, + skippedCookies: 2, + domains: ['linear.app', 'youtube.com'] + }) + expect(cookiesRemoveMock.mock.calls).toEqual([['https://linear.app/', 'old-linear']]) + expect(cookiesSetMock.mock.calls.map(([details]) => details.domain)).toEqual([ + '.youtube.com', + '.linear.app' + ]) + }) + + it('leaves Google cookies from an older import in place too', async () => { + cookiesGetMock.mockResolvedValue([existingCookie('.google.com', 'SAPISID')]) + + const result = await importCookiesFromFile( + writeCookies([{ domain: '.google.com', name: 'SAPISID', value: 'newer', secure: true }]), + 'persist:test' + ) + + expect(result.ok && result.summary).toMatchObject({ + totalCookies: 1, + importedCookies: 0, + skippedCookies: 1, + domains: [] + }) + expect(cookiesRemoveMock).not.toHaveBeenCalled() + expect(cookiesSetMock).not.toHaveBeenCalled() + }) +}) + +describe('native Chromium import excludes the Google cookie family', () => { + let clearStorageDataMock: ReturnType + let cookiesGetMock: ReturnType + let cookiesRemoveMock: ReturnType + let cookiesSetMock: ReturnType + let tmpDir: string + let platformSpy: ReturnType + + beforeEach(() => { + vi.clearAllMocks() + tmpDir = mkdtempSync(join(tmpdir(), 'orca-google-exclusion-native-')) + appGetPathMock.mockReturnValue(join(tmpDir, 'userData')) + execFileSyncMock.mockImplementation(() => { + throw new Error('OS browser version lookup unavailable') + }) + clearStorageDataMock = vi.fn().mockResolvedValue(undefined) + cookiesGetMock = vi.fn().mockResolvedValue([]) + cookiesRemoveMock = vi.fn().mockResolvedValue(undefined) + cookiesSetMock = vi.fn().mockResolvedValue(undefined) + sessionFromPartitionMock.mockReturnValue({ + cookies: { + flushStore: vi.fn().mockResolvedValue(undefined), + get: cookiesGetMock, + remove: cookiesRemoveMock, + set: cookiesSetMock + }, + clearStorageData: clearStorageDataMock + }) + platformSpy = vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin') + }) + + afterEach(() => { + platformSpy.mockRestore() + rmSync(tmpDir, { recursive: true, force: true }) + }) + + function seedSource(rows: { domain: string; name: string; value: string }[]): string { + const sourceCookiesPath = join(tmpDir, 'Chrome', 'Default', 'Network', 'Cookies') + createChromiumCookieTestDatabase(sourceCookiesPath, rows).close() + return sourceCookiesPath + } + + function seedTarget(rows: { domain: string; name: string; value: string }[]): void { + const targetCookiesPath = join(tmpDir, 'userData', 'Partitions', 'test', 'Network', 'Cookies') + createChromiumCookieTestDatabase(targetCookiesPath, rows).close() + } + + it('never wipes the jar wholesale and keeps the live Google cookies', async () => { + const sourceCookiesPath = seedSource([ + { domain: '.google.com', name: 'SID', value: 'transplanted-sid' }, + { domain: '.example.com', name: 'session', value: 'new' } + ]) + seedTarget([{ domain: '.google.com', name: 'SID', value: 'live-sid' }]) + cookiesGetMock.mockResolvedValue([ + existingCookie('.google.com', 'SID'), + existingCookie('.example.com', 'stale') + ]) + + const result = await importCookiesFromBrowser(chromeBrowser(sourceCookiesPath), 'persist:test') + + expect(result.ok && result.summary).toMatchObject({ + totalCookies: 2, + importedCookies: 1, + skippedCookies: 1, + domains: ['example.com'] + }) + expect(clearStorageDataMock).not.toHaveBeenCalled() + expect(cookiesRemoveMock.mock.calls).toEqual([['https://example.com/', 'stale']]) + expect(cookiesSetMock.mock.calls.map(([details]) => details.domain)).toEqual(['.example.com']) + }) + + it('keeps the live Google rows in the staged restart-fallback database', async () => { + const sourceCookiesPath = seedSource([ + { domain: '.google.com', name: 'SID', value: 'transplanted-sid' }, + { domain: '.example.com', name: 'session', value: 'new' } + ]) + seedTarget([ + { domain: '.google.com', name: 'SID', value: 'live-sid' }, + { domain: '.example.com', name: 'stale', value: 'stale' } + ]) + // Why: a rejected set() forces the staged DB to be kept for cold-restart replay. + cookiesSetMock.mockRejectedValue(new Error('cookie rejected')) + + const result = await importCookiesFromBrowser(chromeBrowser(sourceCookiesPath), 'persist:test') + + expect(result.ok).toBe(true) + expect(setPendingCookieImportMock).toHaveBeenCalledTimes(1) + const [partition, stagedPath] = setPendingCookieImportMock.mock.calls[0] + expect(partition).toBe('persist:test') + expect(readStagedRows(stagedPath)).toEqual([ + { host_key: '.example.com', name: 'session', value: 'new' }, + { host_key: '.google.com', name: 'SID', value: 'live-sid' } + ]) + }) +}) + +// Why: freshly imported rows store the decrypted value as a BLOB; read both forms as text. +function readStagedRows(stagedPath: string): { host_key: string; name: string; value: string }[] { + const stagedDb = new DatabaseSync(stagedPath, { readOnly: true }) + const rows = stagedDb + .prepare('SELECT host_key, name, value FROM cookies ORDER BY host_key, name') + .all() as { host_key: string; name: string; value: string | Uint8Array }[] + stagedDb.close() + return rows.map((row) => ({ + ...row, + value: typeof row.value === 'string' ? row.value : Buffer.from(row.value).toString('latin1') + })) +} + +function chromeBrowser(cookiesPath: string) { + return { + family: 'chrome' as const, + label: 'Google Chrome', + cookiesPath, + keychainService: 'Chrome Safe Storage', + keychainAccount: 'Chrome', + profiles: [{ name: 'Default', directory: 'Default' }], + selectedProfile: 'Default' + } +} diff --git a/src/main/browser/browser-cookie-import-policy.test.ts b/src/main/browser/browser-cookie-import-policy.test.ts index 31ca329bf8f..0969921724c 100644 --- a/src/main/browser/browser-cookie-import-policy.test.ts +++ b/src/main/browser/browser-cookie-import-policy.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it, vi } from 'vitest' +import { DatabaseSync } from 'node:sqlite' import type { Cookie } from 'electron' import { isGoogleSourceBoundCookie, + isNonTransplantableCookieDomain, + NON_TRANSPLANTABLE_HOST_KEY_SQL, normalizeCookieDomain, + removeAllCookiesExcept, replaceCookiesForImportedDomains } from './browser-cookie-import-policy' @@ -147,3 +151,160 @@ describe('replaceCookiesForImportedDomains', () => { }) }) }) + +describe('isNonTransplantableCookieDomain', () => { + it('covers the whole google.com registrable family', () => { + expect(isNonTransplantableCookieDomain('google.com')).toBe(true) + expect(isNonTransplantableCookieDomain('.google.com')).toBe(true) + expect(isNonTransplantableCookieDomain('accounts.google.com')).toBe(true) + expect(isNonTransplantableCookieDomain('MAIL.Google.Com')).toBe(true) + }) + + it('does not match lookalikes or unrelated sites', () => { + expect(isNonTransplantableCookieDomain('withgoogle.com')).toBe(false) + expect(isNonTransplantableCookieDomain('google.com.evil.example')).toBe(false) + expect(isNonTransplantableCookieDomain('notgoogle.com')).toBe(false) + expect(isNonTransplantableCookieDomain('linear.app')).toBe(false) + expect(isNonTransplantableCookieDomain('')).toBe(false) + }) + + // Why: youtube.com re-issues its cookies from a transplanted session, so excluding it would + // drop imports users asked for. Locking it in keeps a future "just add it too" edit honest. + it('deliberately leaves youtube.com transplantable', () => { + expect(isNonTransplantableCookieDomain('.youtube.com')).toBe(false) + expect(isNonTransplantableCookieDomain('accounts.youtube.com')).toBe(false) + }) +}) + +describe('NON_TRANSPLANTABLE_HOST_KEY_SQL', () => { + it('selects the google.com family and nothing that merely looks like it', () => { + const db = new DatabaseSync(':memory:') + db.exec('CREATE TABLE cookies (host_key TEXT)') + for (const hostKey of [ + 'google.com', + '.google.com', + 'accounts.google.com', + 'withgoogle.com', + 'google.com.evil.example', + '.youtube.com', + '.linear.app' + ]) { + db.prepare('INSERT INTO cookies (host_key) VALUES (?)').run(hostKey) + } + + const matched = db + .prepare( + `SELECT host_key FROM cookies WHERE ${NON_TRANSPLANTABLE_HOST_KEY_SQL} ORDER BY host_key` + ) + .all() as { host_key: string }[] + db.close() + + expect(matched.map((row) => row.host_key)).toEqual([ + '.google.com', + 'accounts.google.com', + 'google.com' + ]) + }) +}) + +describe('removeAllCookiesExcept', () => { + it('removes only the cookies the predicate does not exclude', async () => { + const get = vi + .fn() + .mockResolvedValue([ + cookie('.google.com', 'SID'), + cookie('.example.com', 'session'), + cookie('other.test', 'tracker', '/scoped') + ]) + const remove = vi.fn().mockResolvedValue(undefined) + const set = vi.fn().mockResolvedValue(undefined) + + await removeAllCookiesExcept({ get, remove, set }, (c) => c.domain === '.google.com') + + expect(remove.mock.calls).toEqual([ + ['https://example.com/', 'session'], + ['https://other.test/scoped', 'tracker'] + ]) + expect(set).not.toHaveBeenCalled() + }) + + it('restores every successfully removed cookie when another removal fails', async () => { + const get = vi + .fn() + .mockResolvedValue([ + 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') + } + }) + const set = vi.fn().mockResolvedValue(undefined) + + await expect(removeAllCookiesExcept({ get, remove, set }, () => false)).rejects.toThrow( + 'Could not clear existing cookies' + ) + expect(remove).toHaveBeenCalledTimes(3) + expect(set.mock.calls.map(([details]) => details.name)).toEqual(['first', 'third']) + }) + + 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 clearing = removeAllCookiesExcept({ get, remove, set }, () => false) + await vi.waitFor(() => expect(remove).toHaveBeenCalledTimes(8)) + expect(maxActive).toBe(8) + releaseRemovals?.() + await clearing + + expect(remove).toHaveBeenCalledTimes(12) + expect(set).not.toHaveBeenCalled() + }) + + 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 set = vi.fn().mockResolvedValue(undefined) + + const clearing = removeAllCookiesExcept({ get, remove, set }, () => false) + await vi.waitFor(() => expect(remove).toHaveBeenCalledOnce()) + releaseFirst?.() + await clearing + + expect(remove.mock.calls).toEqual([ + ['https://example.com/', 'session'], + ['https://example.com/', 'session'] + ]) + }) +}) diff --git a/src/main/browser/browser-cookie-import-policy.ts b/src/main/browser/browser-cookie-import-policy.ts index 216542f08ea..8f6008ac7a4 100644 --- a/src/main/browser/browser-cookie-import-policy.ts +++ b/src/main/browser/browser-cookie-import-policy.ts @@ -1,5 +1,6 @@ import type { Cookie, Cookies } from 'electron' import { parse as parseDomain } from 'psl' +import { mapSettledWithConcurrency } from '../../shared/map-with-concurrency' const GOOGLE_SOURCE_BOUND_COOKIE_NAMES = new Set([ 'SIDCC', @@ -53,6 +54,36 @@ export function normalizeCookieImportDomain(domain: string): string | null { return normalized } +// Why (STA-3811): registrable families whose sessions are device-bound server-side, so a +// transplanted cookie is rejected (or flagged and expired within ~1h) no matter how faithfully +// 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. +// 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 COOKIE_CLEAR_CONCURRENCY = 8 + +export function isNonTransplantableCookieDomain(domain: string): boolean { + const normalized = normalizeCookieDomain(domain) + if (!normalized) { + return false + } + return NON_TRANSPLANTABLE_DOMAINS.some( + (root) => normalized === root || normalized.endsWith(`.${root}`) + ) +} + +// Why: Chromium stores host_key lowercase as 'google.com', '.google.com' or 'sub.google.com'; +// the LIKE pattern covers the leading-dot row and cannot match lookalikes ('withgoogle.com'). +export const NON_TRANSPLANTABLE_HOST_KEY_SQL = NON_TRANSPLANTABLE_DOMAINS.map( + (root) => `host_key = '${root}' OR host_key LIKE '%.${root}'` +).join(' OR ') + +// Why: subsumed by the domain exclusion above for google.com — kept because it is the general +// rule for rotation-only cookies and applies to any family added without a full exclusion. export function isGoogleSourceBoundCookie(name: string, domain: string): boolean { if (!GOOGLE_SOURCE_BOUND_COOKIE_NAMES.has(name)) { return false @@ -165,6 +196,55 @@ export async function restoreImportedDomainCookies( } } +// Why: clearStorageData wipes the whole jar, including the non-transplantable families an +// import is never allowed to remove; this is the clear step that can leave them in place. +export async function removeAllCookiesExcept( + store: Pick, + isExcluded: (cookie: Cookie) => boolean +): Promise { + const existingCookies = await store.get({}) + const removableGroups = new Map() + for (const cookie of existingCookies) { + if (isExcluded(cookie)) { + continue + } + const domain = cookie.domain ? normalizeCookieDomain(cookie.domain) : null + const url = domain ? cookieRemovalUrl(cookie, domain) : null + if (!url) { + continue + } + const key = JSON.stringify([url, cookie.name]) + const group = removableGroups.get(key) ?? [] + group.push({ cookie, url }) + removableGroups.set(key, group) + } + + const removedCookies: Cookie[] = [] + const results = await mapSettledWithConcurrency( + [...removableGroups.values()], + COOKIE_CLEAR_CONCURRENCY, + async (group) => { + // Why: identical remove keys must stay ordered so duplicate scoped cookies are not raced. + for (const { cookie, url } of group) { + await store.remove(url, cookie.name) + removedCookies.push(cookie) + } + } + ) + const failures = results.flatMap((result) => + result.status === 'rejected' ? [result.reason] : [] + ) + if (failures.length === 0) { + return + } + try { + await restoreImportedDomainCookies(store, removedCookies) + } catch (restoreError) { + throw new AggregateError([...failures, restoreError], 'Cookie clearing and rollback failed') + } + throw new AggregateError(failures, 'Could not clear existing cookies') +} + export async function replaceCookiesForImportedDomains( store: Pick, importedDomains: readonly string[] diff --git a/src/main/browser/browser-cookie-import-replacement.test.ts b/src/main/browser/browser-cookie-import-replacement.test.ts index 74c2ec432f1..79553d3a651 100644 --- a/src/main/browser/browser-cookie-import-replacement.test.ts +++ b/src/main/browser/browser-cookie-import-replacement.test.ts @@ -59,10 +59,11 @@ describe('validated cookie replacement', () => { return filePath } - it('filters Google source-bound cookies before replacing imported domain scopes', async () => { + it('keeps Google out of the replace scope so its cookies are never removed', async () => { cookiesGetMock.mockResolvedValue([ cookie('.google.com', 'old-google'), cookie('.accounts.google.com', 'old-accounts', '/signin'), + cookie('.example.com', 'old-example'), cookie('.unrelated.com', 'keep'), cookie('.google.com.evil.example', 'keep-suffix-confusion') ]) @@ -76,15 +77,12 @@ describe('validated cookie replacement', () => { expect(result.ok && result.summary).toMatchObject({ totalCookies: 3, - importedCookies: 2, - skippedCookies: 1, - domains: ['example.com', 'google.com'] + importedCookies: 1, + skippedCookies: 2, + domains: ['example.com'] }) - expect(cookiesRemoveMock.mock.calls).toEqual([ - ['https://google.com/', 'old-google'], - ['https://accounts.google.com/signin', 'old-accounts'] - ]) - expect(cookiesSetMock.mock.calls.map(([details]) => details.name)).toEqual(['SAPISID', 'SIDCC']) + expect(cookiesRemoveMock.mock.calls).toEqual([['https://example.com/', 'old-example']]) + expect(cookiesSetMock.mock.calls.map(([details]) => details.name)).toEqual(['SIDCC']) expect(Math.max(...cookiesRemoveMock.mock.invocationCallOrder)).toBeLessThan( Math.min(...cookiesSetMock.mock.invocationCallOrder) ) @@ -185,6 +183,7 @@ describe('native Chromium integrity-cookie accounting', () => { sessionFromPartitionMock.mockReset().mockReturnValue({ cookies: { flushStore: vi.fn().mockResolvedValue(undefined), + get: vi.fn().mockResolvedValue([]), remove: vi.fn().mockResolvedValue(undefined), set: cookiesSetMock }, @@ -196,7 +195,7 @@ describe('native Chromium integrity-cookie accounting', () => { rmSync(tmpDir, { recursive: true, force: true }) }) - it('includes domain-scoped integrity cookies in skippedCookies', async () => { + it('counts every excluded Google cookie in skippedCookies', async () => { const sourceCookiesPath = join(tmpDir, 'Chrome', 'Default', 'Network', 'Cookies') const targetCookiesPath = join(tmpDir, 'userData', 'Partitions', 'test', 'Network', 'Cookies') createChromiumCookieTestDatabase(sourceCookiesPath, [ @@ -214,11 +213,11 @@ describe('native Chromium integrity-cookie accounting', () => { ) expect(result.ok && result.summary).toMatchObject({ totalCookies: 3, - importedCookies: 2, - skippedCookies: 1, - domains: ['example.com', 'google.com'] + importedCookies: 1, + skippedCookies: 2, + domains: ['example.com'] }) - expect(cookiesSetMock.mock.calls.map(([details]) => details.name)).toEqual(['SAPISID', 'AEC']) + expect(cookiesSetMock.mock.calls.map(([details]) => details.name)).toEqual(['AEC']) } finally { platformSpy.mockRestore() } diff --git a/src/main/browser/browser-cookie-import.test.ts b/src/main/browser/browser-cookie-import.test.ts index aefdd27fbf5..50e232c0b7e 100644 --- a/src/main/browser/browser-cookie-import.test.ts +++ b/src/main/browser/browser-cookie-import.test.ts @@ -445,6 +445,7 @@ describe('importCookiesFromBrowser Chromium', () => { sessionFromPartitionMock.mockReset() sessionFromPartitionMock.mockReturnValue({ cookies: { + get: vi.fn().mockResolvedValue([]), set: cookiesSetMock, remove: cookiesRemoveMock, flushStore: cookiesFlushStoreMock @@ -510,7 +511,9 @@ describe('importCookiesFromBrowser Chromium', () => { ['', '-wal', '-shm'].map((suffix) => readFileSync(sourceCookiesPath + suffix)) ).toEqual(sourceFilesBefore) expect(cookiesRemoveMock).not.toHaveBeenCalled() - expect(clearStorageDataMock).toHaveBeenCalledWith({ storages: ['cookies'] }) + // Why: STA-3811 — the pre-import clear is selective now, so a wholesale wipe would + // take the non-transplantable families with it. + expect(clearStorageDataMock).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 66a3afdb3cf..bffb12cfa02 100644 --- a/src/main/browser/browser-cookie-import.ts +++ b/src/main/browser/browser-cookie-import.ts @@ -76,8 +76,11 @@ import type { import { browserSessionRegistry } from './browser-session-registry' import { isGoogleSourceBoundCookie, + isNonTransplantableCookieDomain, + NON_TRANSPLANTABLE_HOST_KEY_SQL, normalizeCookieDomain, normalizeCookieImportDomain, + removeAllCookiesExcept, replaceCookiesForImportedDomains, restoreImportedDomainCookies, type CookieImportMode @@ -571,13 +574,19 @@ async function importValidatedCookies( } return valid }) - const importableCookies = validDomainCookies.filter( + const sourceBoundFiltered = validDomainCookies.filter( (cookie) => !isGoogleSourceBoundCookie(cookie.name, cookie.domain) ) - const integritySkipped = validDomainCookies.length - importableCookies.length + // Why: dropping these before the replace scope is computed is what keeps the existing + // Google session intact — replaceCookiesForImportedDomains only clears domains we import. + const importableCookies = sourceBoundFiltered.filter( + (cookie) => !isNonTransplantableCookieDomain(cookie.domain) + ) + const integritySkipped = validDomainCookies.length - sourceBoundFiltered.length + const nonTransplantableSkipped = sourceBoundFiltered.length - importableCookies.length const invalidDomainSkipped = cookies.length - validDomainCookies.length diag( - `importValidatedCookies: ${cookies.length} validated, ${invalidDomainSkipped} unsafe-domain skipped, ${integritySkipped} source-bound skipped of ${totalInput} total, partition="${targetPartition}"` + `importValidatedCookies: ${cookies.length} validated, ${invalidDomainSkipped} unsafe-domain skipped, ${integritySkipped} source-bound skipped, ${nonTransplantableSkipped} non-transplantable skipped of ${totalInput} total, partition="${targetPartition}"` ) const targetSession = session.fromPartition(targetPartition) let importedCount = 0 @@ -1548,7 +1557,10 @@ export async function importCookiesFromBrowser( const targetCols: string[] = targetColumnInfo.map((r) => r.name) colList = targetCols.join(', ') placeholders = targetCols.map(() => '?').join(', ') - stagingDb.exec('DELETE FROM cookies') + // Why: the staged DB replaces the whole live DB at cold start, so it is a clear step + // like any other — keep the live non-transplantable rows in it rather than replaying + // a wipe the in-memory path was not allowed to perform. + stagingDb.exec(`DELETE FROM cookies WHERE NOT (${NON_TRANSPLANTABLE_HOST_KEY_SQL})`) } catch (err) { diag(` staging database unusable, restart fallback disabled: ${String(err)}`) stagingAvailable = false @@ -1596,6 +1608,7 @@ export async function importCookiesFromBrowser( let imported = 0 let skipped = 0 let integritySkipped = 0 + let nonTransplantableSkipped = 0 let memoryLoaded = 0 let memoryFailed = 0 const domainSet = new Set() @@ -1669,6 +1682,12 @@ export async function importCookiesFromBrowser( continue } + // Why: transplanting these replaces a working sign-in with a session the site rejects. + if (isNonTransplantableCookieDomain(domain)) { + nonTransplantableSkipped++ + continue + } + let validDomain = sourceDomainValidity.get(domain) if (validDomain === undefined) { validDomain = normalizeCookieImportDomain(domain) !== null @@ -1718,7 +1737,9 @@ export async function importCookiesFromBrowser( // the optional staging DB is unavailable. imported++ } - diag(` skipped ${integritySkipped} Google integrity cookies (SIDCC/STRP/AEC)`) + diag( + ` skipped ${integritySkipped} Google integrity cookies (SIDCC/STRP/AEC) and ${nonTransplantableSkipped} non-transplantable-domain cookies` + ) if (decryptedCookies.length === 0) { closeStagingDb() @@ -1729,7 +1750,7 @@ export async function importCookiesFromBrowser( summary: { totalCookies: sourceRows.length, importedCookies: 0, - skippedCookies: skipped + integritySkipped, + skippedCookies: skipped + integritySkipped + nonTransplantableSkipped, domains: [] } } @@ -1747,8 +1768,12 @@ export async function importCookiesFromBrowser( diag(` staging skipped: ${imported} cookies will load in-memory only`) } - // Why: clear stale cookies first; mixing them with the imported set makes sites like Google reject the session. - await targetSession.clearStorageData({ storages: ['cookies'] }) + // 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 ?? '') + ) diag( ` cleared existing session cookies before loading ${decryptedCookies.length} imported cookies` ) @@ -1816,7 +1841,7 @@ export async function importCookiesFromBrowser( const summary: BrowserCookieImportSummary = { totalCookies: sourceRows.length, importedCookies: imported, - skippedCookies: skipped + integritySkipped, + skippedCookies: skipped + integritySkipped + nonTransplantableSkipped, domains: [...domainSet].sort(), ...(warning ? { warning } : {}) } diff --git a/src/renderer/src/components/browser-cookie-import-google-disclosure.test.tsx b/src/renderer/src/components/browser-cookie-import-google-disclosure.test.tsx new file mode 100644 index 00000000000..6ef0481edab --- /dev/null +++ b/src/renderer/src/components/browser-cookie-import-google-disclosure.test.tsx @@ -0,0 +1,138 @@ +/** + * @vitest-environment happy-dom + * + * STA-3811: imports never touch the Google cookie family, so both import menus must say so at + * the moment of decision. Covers the browser toolbar menu and the Settings profile row. + */ +import { act, type ReactNode } from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import en from '@/i18n/locales/en.json' + +const DISCLOSURE = 'Google requires signing in directly - imports skip it.' + +vi.mock('@/components/ui/dropdown-menu', () => dropdownMenuStubs()) +vi.mock('../ui/dropdown-menu', () => dropdownMenuStubs()) +vi.mock('@/store', () => ({ useAppStore: appStoreStub() })) +vi.mock('../../store', () => ({ useAppStore: appStoreStub() })) +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })) + +import { BrowserToolbarMenuDropdown } from './browser-pane/browser-toolbar-menu-dropdown' +import { BrowserProfileRow } from './settings/BrowserProfileRow' + +const DETECTED_BROWSERS = [ + { + family: 'chrome', + label: 'Google Chrome', + profiles: [{ name: 'Default', directory: 'Default' }], + selectedProfile: 'Default' + } +] + +describe('cookie-import Google disclosure caption', () => { + let container: HTMLDivElement + let root: Root + + beforeEach(() => { + container = document.createElement('div') + document.body.append(container) + root = createRoot(container) + }) + + afterEach(() => { + act(() => root.unmount()) + container.remove() + }) + + it('is shown in the browser toolbar import menu', () => { + act(() => { + root.render( + + ) + }) + + expect(container.textContent).toContain(DISCLOSURE) + }) + + it('is shown in the Settings browser-profile import menu', () => { + act(() => { + root.render( + + ) + }) + + expect(container.textContent).toContain(DISCLOSURE) + }) + + // Why: the rendered text comes from the catalog, not the translate() fallback, so a copy + // drift in en.json alone would otherwise slip through both render assertions. + it('reads the same copy from the catalog on both surfaces', () => { + expect(catalogEntry('auto.components.browser.pane.BrowserToolbarMenu.c186b4d890')).toBe( + DISCLOSURE + ) + expect(catalogEntry('auto.components.settings.BrowserProfileRow.654a0c2073')).toBe(DISCLOSURE) + }) +}) + +function catalogEntry(key: string): unknown { + return key + .split('.') + .reduce( + (node, part) => + typeof node === 'object' && node !== null + ? (node as Record)[part] + : undefined, + en + ) +} + +function dropdownMenuStubs(): Record { + const passthrough = ({ children }: { children?: ReactNode }): ReactNode => children + const block = ({ children }: { children?: ReactNode }): ReactNode =>
{children}
+ return { + DropdownMenu: passthrough, + DropdownMenuContent: block, + DropdownMenuItem: block, + DropdownMenuLabel: block, + DropdownMenuPortal: passthrough, + DropdownMenuRadioGroup: passthrough, + DropdownMenuRadioItem: block, + DropdownMenuSeparator: () => null, + DropdownMenuSub: passthrough, + DropdownMenuSubContent: block, + DropdownMenuSubTrigger: block, + DropdownMenuTrigger: passthrough + } +} + +function appStoreStub(): unknown { + const state = { + fetchDetectedBrowsers: vi.fn(), + openSettingsTarget: vi.fn(), + openSettingsPage: vi.fn() + } + const useAppStore = (selector?: (s: typeof state) => unknown): unknown => + selector ? selector(state) : state + useAppStore.getState = (): typeof state => state + return useAppStore +} diff --git a/src/renderer/src/components/browser-pane/browser-toolbar-menu-dropdown.tsx b/src/renderer/src/components/browser-pane/browser-toolbar-menu-dropdown.tsx index b34453ba3e3..0491f55db05 100644 --- a/src/renderer/src/components/browser-pane/browser-toolbar-menu-dropdown.tsx +++ b/src/renderer/src/components/browser-pane/browser-toolbar-menu-dropdown.tsx @@ -4,6 +4,7 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, + DropdownMenuLabel, DropdownMenuPortal, DropdownMenuRadioGroup, DropdownMenuRadioItem, @@ -127,6 +128,12 @@ export function BrowserToolbarMenuDropdown({ + + {translate( + 'auto.components.browser.pane.BrowserToolbarMenu.c186b4d890', + 'Google requires signing in directly - imports skip it.' + )} + {detectedBrowsers.map((browser) => browser.profiles.length > 1 ? ( diff --git a/src/renderer/src/components/settings/BrowserProfileRow.tsx b/src/renderer/src/components/settings/BrowserProfileRow.tsx index 891164d5bd5..7ab502698ee 100644 --- a/src/renderer/src/components/settings/BrowserProfileRow.tsx +++ b/src/renderer/src/components/settings/BrowserProfileRow.tsx @@ -7,6 +7,7 @@ import { DropdownMenu, DropdownMenuContent, DropdownMenuItem, + DropdownMenuLabel, DropdownMenuPortal, DropdownMenuSeparator, DropdownMenuSub, @@ -171,6 +172,12 @@ export function BrowserProfileRow({ + + {translate( + 'auto.components.settings.BrowserProfileRow.654a0c2073', + 'Google requires signing in directly - imports skip it.' + )} + {detectedBrowsers.map((browser) => browser.profiles.length > 1 ? ( diff --git a/src/renderer/src/i18n/locales/en.json b/src/renderer/src/i18n/locales/en.json index d3ef9da104e..e8718740205 100644 --- a/src/renderer/src/i18n/locales/en.json +++ b/src/renderer/src/i18n/locales/en.json @@ -5847,7 +5847,8 @@ "a3f8c2d1e0b4": "Imported {{value0}} cookies from {{value1}} ({{value2}}) into {{value3}}.", "b4e9d3f2a1c5": "Imported {{value0}} cookies from {{value1}} into {{value2}}.", "c5a273a809": "From {{value0}}", - "b5c0479e21": "Unmodified user agent" + "b5c0479e21": "Unmodified user agent", + "654a0c2073": "Google requires signing in directly - imports skip it." }, "BrowserUseComputerUseNotice": { "15b5e680ba": "Open Computer Use", @@ -14195,7 +14196,8 @@ "bf648471c5": "Creating…", "53bbe3dab4": "Imported {{value0}} cookies from file.", "c5f0e4d3b2a1": "Imported {{value0}} cookies from {{value1}} ({{value2}}).", - "d6a1f5e4c3b2": "Imported {{value0}} cookies from {{value1}}." + "d6a1f5e4c3b2": "Imported {{value0}} cookies from {{value1}}.", + "c186b4d890": "Google requires signing in directly - imports skip it." }, "GrabConfirmationSheet": { "314a0aaa5b": "Attach to AI",