Skip to content

Commit 35efc8a

Browse files
TheodoreSpeaksTheodoreSpeaks
authored
fix(desktop): keep built-in browser logins across restarts, faster tab reveal (#8337)
* fix(desktop): keep built-in browser logins across app restarts * test(desktop): cover session-cookie persistence with a relaunch e2e * improvement(tab-strip): reveal the active tab in a fixed 200ms * fix(tab-strip): stop an in-flight reveal on every retarget and on pointer scrolling * fix(tab-strip): yield the reveal only to scrolling it did not cause
1 parent caf11b1 commit 35efc8a

6 files changed

Lines changed: 352 additions & 13 deletions

File tree

‎apps/desktop/e2e/session-cookies.spec.ts‎

Lines changed: 178 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,178 @@
1+
import { mkdtempSync, rmSync } from 'node:fs'
2+
import { createServer, type Server } from 'node:http'
3+
import { tmpdir } from 'node:os'
4+
import { join } from 'node:path'
5+
import { fileURLToPath } from 'node:url'
6+
import { type ElectronApplication, _electron as electron, expect, test } from '@playwright/test'
7+
import type { SimDesktopApi } from '@sim/desktop-bridge'
8+
9+
const DESKTOP_DIR = fileURLToPath(new URL('..', import.meta.url))
10+
const SCOPE = 'session-cookies-fixture'
11+
12+
/**
13+
* Electron drops every expiry-less cookie on quit, so a site that keeps its
14+
* login in a session cookie signed the user out of the built-in browser on
15+
* every restart. This drives a real quit and relaunch against one profile.
16+
*/
17+
test.describe('built-in browser session cookies', () => {
18+
let server: Server
19+
let origin: string
20+
let site: string
21+
let userData: string
22+
let app: ElectronApplication | undefined
23+
let serial = 0
24+
let lastCookieHeader: string | undefined
25+
const calls = new Map<
26+
string,
27+
{ chatId: string; toolName: string; args: Record<string, unknown> }
28+
>()
29+
30+
test.beforeAll(async () => {
31+
server = createServer(async (request, response) => {
32+
const path = new URL(request.url ?? '/', 'http://localhost').pathname
33+
if (path === '/api/auth/get-session') {
34+
response.writeHead(200, { 'Content-Type': 'application/json' })
35+
response.end(
36+
JSON.stringify({ user: { id: 'fixture-user' }, session: { id: 'fixture-session' } })
37+
)
38+
return
39+
}
40+
if (path === '/api/desktop/tool/authorize') {
41+
let body = ''
42+
for await (const chunk of request) body += chunk.toString()
43+
const call = calls.get(JSON.parse(body).toolCallId)
44+
response.writeHead(call ? 200 : 403, { 'Content-Type': 'application/json' })
45+
response.end(JSON.stringify(call ?? {}))
46+
return
47+
}
48+
if (path.startsWith('/api/')) {
49+
response.writeHead(200, { 'Content-Type': 'application/json' })
50+
response.end('{}')
51+
return
52+
}
53+
if (path === '/sign-in') {
54+
// A login redirect that sets a short-lived cookie the next hop deletes.
55+
response.writeHead(302, {
56+
'Set-Cookie': 'oauth_state=pending; HttpOnly; Path=/',
57+
Location: '/signed-in',
58+
})
59+
response.end()
60+
return
61+
}
62+
if (path === '/signed-in') {
63+
response.writeHead(200, {
64+
'Content-Type': 'text/html',
65+
'Set-Cookie': [
66+
'oauth_state=; Path=/; Max-Age=0',
67+
'login=fixture; HttpOnly; SameSite=Lax; Path=/',
68+
'remember=1; Path=/; Max-Age=3600',
69+
],
70+
})
71+
response.end('<!doctype html><title>Signed in</title>')
72+
return
73+
}
74+
if (path === '/account') {
75+
lastCookieHeader = request.headers.cookie ?? ''
76+
response.writeHead(200, { 'Content-Type': 'text/html' })
77+
response.end('<!doctype html><title>Account</title>')
78+
return
79+
}
80+
if (request.headers.host?.startsWith('localhost')) {
81+
// Favicon and other stray site requests must not set the app session cookie on the site.
82+
response.writeHead(404)
83+
response.end()
84+
return
85+
}
86+
response.writeHead(200, {
87+
'Content-Type': 'text/html',
88+
'Set-Cookie': 'better-auth.session_token=fixture; HttpOnly; SameSite=Lax; Path=/',
89+
})
90+
response.end('<!doctype html><title>Sim fixture</title><h1>Session cookie fixture</h1>')
91+
})
92+
await new Promise<void>((resolve) => server.listen(0, '127.0.0.1', resolve))
93+
const address = server.address()
94+
if (!address || typeof address === 'string') throw new Error('Missing fixture address')
95+
origin = `http://127.0.0.1:${address.port}`
96+
/** Pages outside the app origin browse in the built-in browser's own partition. */
97+
site = `http://localhost:${address.port}`
98+
})
99+
100+
test.beforeEach(() => {
101+
userData = mkdtempSync(join(tmpdir(), 'sim-session-cookies-e2e-'))
102+
})
103+
104+
test.afterEach(async () => {
105+
await app?.close()
106+
app = undefined
107+
rmSync(userData, { recursive: true, force: true })
108+
calls.clear()
109+
})
110+
111+
test.afterAll(async () => {
112+
await new Promise<void>((resolve, reject) =>
113+
server.close((error) => (error ? reject(error) : resolve()))
114+
)
115+
})
116+
117+
async function launch(): Promise<ElectronApplication> {
118+
const launched = await electron.launch({
119+
args: [process.env.SIM_DESKTOP_E2E_MAIN ?? '.'],
120+
cwd: DESKTOP_DIR,
121+
env: { ...process.env, SIM_DESKTOP_ORIGIN: origin, SIM_DESKTOP_USER_DATA: userData },
122+
})
123+
const host = await launched.firstWindow()
124+
await expect(host.getByRole('heading')).toHaveText('Session cookie fixture')
125+
await host.evaluate(async (scope) => {
126+
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
127+
await api.browserAgent.activateScope(scope)
128+
const updateBounds = () =>
129+
api.browserAgent.setPanelBounds(
130+
{ x: 0, y: 80, width: innerWidth, height: innerHeight - 80 },
131+
null,
132+
scope
133+
)
134+
updateBounds()
135+
setInterval(updateBounds, 200)
136+
}, SCOPE)
137+
return launched
138+
}
139+
140+
async function navigate(target: ElectronApplication, url: string) {
141+
const id = `fixture-${++serial}`
142+
calls.set(id, { chatId: SCOPE, toolName: 'browser_open_url', args: { url } })
143+
const host = await target.firstWindow()
144+
const result = await host.evaluate(
145+
async ({ id, url, scope }) => {
146+
const api = (globalThis as typeof globalThis & { simDesktop: SimDesktopApi }).simDesktop
147+
return api.browserAgent.executeTool(id, 'browser_open_url', { url }, scope)
148+
},
149+
{ id, url, scope: SCOPE }
150+
)
151+
expect(result.ok, result.error).toBe(true)
152+
}
153+
154+
async function cookiesSentToSite(target: ElectronApplication): Promise<string[]> {
155+
lastCookieHeader = undefined
156+
await navigate(target, `${site}/account`)
157+
await expect.poll(() => lastCookieHeader).not.toBeUndefined()
158+
return (lastCookieHeader ?? '')
159+
.split(';')
160+
.map((pair) => pair.trim())
161+
.filter(Boolean)
162+
.sort()
163+
}
164+
165+
test('keeps a session-cookie login across a restart without reviving deleted cookies', async () => {
166+
app = await launch()
167+
await navigate(app, `${site}/sign-in`)
168+
expect(await cookiesSentToSite(app)).toEqual(['login=fixture', 'remember=1'])
169+
170+
await app.evaluate(({ session }) =>
171+
session.fromPartition('persist:sim-browser-agent').cookies.flushStore()
172+
)
173+
await app.close()
174+
175+
app = await launch()
176+
expect(await cookiesSentToSite(app)).toEqual(['login=fixture', 'remember=1'])
177+
})
178+
})

‎apps/desktop/src/main/browser-agent/session-cookies.test.ts‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { describe, expect, it } from 'vitest'
5+
import {
6+
SESSION_COOKIE_LIFETIME_SECONDS,
7+
withSessionCookieLifetime,
8+
withSessionCookieMaxAge,
9+
} from '@/main/browser-agent/session-cookies'
10+
11+
const NOW = 1_800_000_000
12+
const MAX_AGE = `Max-Age=${SESSION_COOKIE_LIFETIME_SECONDS}`
13+
14+
describe('withSessionCookieLifetime', () => {
15+
it('gives an expiry-less cookie the bounded lifetime', () => {
16+
expect(withSessionCookieLifetime({ url: 'https://example.com/', name: 'a' }, NOW)).toEqual({
17+
url: 'https://example.com/',
18+
name: 'a',
19+
expirationDate: NOW + SESSION_COOKIE_LIFETIME_SECONDS,
20+
})
21+
})
22+
23+
it('leaves a cookie that already expires untouched', () => {
24+
const cookie = { url: 'https://example.com/', name: 'a', expirationDate: NOW + 60 }
25+
expect(withSessionCookieLifetime(cookie, NOW)).toBe(cookie)
26+
})
27+
})
28+
29+
describe('withSessionCookieMaxAge', () => {
30+
it('adds Max-Age to a session cookie', () => {
31+
expect(withSessionCookieMaxAge('sid=abc; Path=/; Secure; HttpOnly')).toBe(
32+
`sid=abc; Path=/; Secure; HttpOnly; ${MAX_AGE}`
33+
)
34+
expect(withSessionCookieMaxAge('sid=abc')).toBe(`sid=abc; ${MAX_AGE}`)
35+
})
36+
37+
it('leaves cookies with an expiry or a deletion untouched', () => {
38+
for (const value of [
39+
'sid=abc; Expires=Wed, 21 Oct 2030 07:28:00 GMT',
40+
'sid=abc; max-age=60',
41+
'sid=; Path=/; Max-Age=0',
42+
'sid=abc;EXPIRES=Thu, 01 Jan 1970 00:00:00 GMT',
43+
]) {
44+
expect(withSessionCookieMaxAge(value)).toBe(value)
45+
}
46+
})
47+
48+
it('reads attributes only, never the cookie value', () => {
49+
expect(withSessionCookieMaxAge('max-age=1')).toBe(`max-age=1; ${MAX_AGE}`)
50+
})
51+
})

‎apps/desktop/src/main/browser-agent/session-cookies.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
import type { CookiesSetDetails, Session } from 'electron'
2+
3+
/**
4+
* Session cookies in the browser partition get this lifetime instead of dying
5+
* with the process.
6+
*
7+
* Electron hard-codes Chromium's `persist_session_cookies` and
8+
* `restore_old_session_cookies` to false, so a `persist:` partition still drops
9+
* every expiry-less cookie on quit — which signs the user out of any site that
10+
* keeps its login in one, whether it was imported from Chrome or set here.
11+
* Chrome keeps them across restarts under "continue where you left off"; this
12+
* matches that, bounded. The site re-sending the cookie renews the window, so
13+
* only a login left untouched for this long lapses.
14+
*/
15+
export const SESSION_COOKIE_LIFETIME_SECONDS = 30 * 24 * 60 * 60
16+
17+
/** Gives an expiry-less cookie the bounded lifetime; cookies that already expire are unchanged. */
18+
export function withSessionCookieLifetime(
19+
cookie: CookiesSetDetails,
20+
nowSeconds: number
21+
): CookiesSetDetails {
22+
if (cookie.expirationDate !== undefined) return cookie
23+
return { ...cookie, expirationDate: nowSeconds + SESSION_COOKIE_LIFETIME_SECONDS }
24+
}
25+
26+
/**
27+
* Adds `Max-Age` to a `Set-Cookie` value that has neither `Expires` nor
28+
* `Max-Age`. Deletions carry one of the two, so they pass through untouched.
29+
*/
30+
export function withSessionCookieMaxAge(setCookie: string): string {
31+
const attributes = setCookie.split(';').slice(1)
32+
const expires = attributes.some((attribute) => {
33+
const name = attribute.split('=', 1)[0].trim().toLowerCase()
34+
return name === 'expires' || name === 'max-age'
35+
})
36+
return expires ? setCookie : `${setCookie}; Max-Age=${SESSION_COOKIE_LIFETIME_SECONDS}`
37+
}
38+
39+
/**
40+
* Makes session cookies a site sets over HTTP survive an app restart.
41+
*
42+
* The rewrite happens on the response headers, before Chromium stores the
43+
* cookie, so it is created persistent in the same order as every other cookie
44+
* write — a later deletion from the site still wins. Rewriting stored cookies
45+
* after the fact would race exactly that: login redirects set and clear
46+
* short-lived cookies milliseconds apart. Cookies set from page script
47+
* (`document.cookie`) are not covered; they cannot be `HttpOnly`, so session
48+
* logins rarely live in them.
49+
*/
50+
export function keepSessionCookiesAcrossRestarts(ses: Session): void {
51+
ses.webRequest.onHeadersReceived((details, callback) => {
52+
const headers = details.responseHeaders
53+
const key = headers && Object.keys(headers).find((name) => name.toLowerCase() === 'set-cookie')
54+
if (!headers || !key) {
55+
callback({})
56+
return
57+
}
58+
callback({ responseHeaders: { ...headers, [key]: headers[key].map(withSessionCookieMaxAge) } })
59+
})
60+
}

‎apps/desktop/src/main/browser-agent/session.ts‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,10 @@ import {
7272
registerAgentWebContents,
7373
} from '@/main/browser-agent/registry'
7474
import { handleBrowserRequest } from '@/main/browser-agent/request-policy'
75+
import {
76+
keepSessionCookiesAcrossRestarts,
77+
withSessionCookieLifetime,
78+
} from '@/main/browser-agent/session-cookies'
7579
import { clearHostVerdictCache } from '@/main/browser-agent/url-guard'
7680
import type { BrowserSessionSnapshot } from '@/main/desktop-chat-session-store'
7781
import { suggestedFilename, uniqueDownloadPath } from '@/main/downloads'
@@ -1327,17 +1331,19 @@ export async function listAgentCookieSignals(): Promise<BrowserCookieSignal[]> {
13271331
* here and is counted rather than being quietly relaxed.
13281332
*
13291333
* Failures are per-cookie: one rejected cookie must not cost the user the
1330-
* rest. Nothing about a cookie is logged.
1334+
* rest. Nothing about a cookie is logged. Session cookies get the bounded
1335+
* lifetime from {@link withSessionCookieLifetime} so they survive a restart.
13311336
*/
13321337
export async function importAgentCookies(
13331338
cookies: CookiesSetDetails[]
13341339
): Promise<{ imported: number; failed: number }> {
13351340
const jar = electronSession.fromPartition(AGENT_PARTITION).cookies
1341+
const nowSeconds = Date.now() / 1000
13361342
let imported = 0
13371343
let failed = 0
13381344
for (const cookie of cookies) {
13391345
try {
1340-
await jar.set(cookie)
1346+
await jar.set(withSessionCookieLifetime(cookie, nowSeconds))
13411347
imported += 1
13421348
} catch {
13431349
failed += 1
@@ -1594,6 +1600,7 @@ const browserPermissions: BrowserPermissionHandlers = {
15941600
function configureAgentPartition(ses: Session): void {
15951601
if (configuredPartitions.has(ses)) return
15961602
configuredPartitions.add(ses)
1603+
keepSessionCookiesAcrossRestarts(ses)
15971604
ses.setPermissionRequestHandler(browserPermissions.request)
15981605
ses.setPermissionCheckHandler(browserPermissions.check)
15991606
ses.webRequest.onBeforeRequest((details, callback) => {

‎apps/desktop/src/test/electron-mock.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ function createWebContentsMock() {
241241
setPermissionRequestHandler: vi.fn(),
242242
setPermissionCheckHandler: vi.fn(),
243243
setUserAgent: vi.fn(),
244-
webRequest: { onBeforeRequest: vi.fn() },
244+
webRequest: { onBeforeRequest: vi.fn(), onHeadersReceived: vi.fn() },
245245
on: vi.fn(),
246246
},
247247
}

0 commit comments

Comments
 (0)