diff --git a/apps/desktop/electron/main.ts b/apps/desktop/electron/main.ts index 35169f18672..9df9c4be798 100644 --- a/apps/desktop/electron/main.ts +++ b/apps/desktop/electron/main.ts @@ -145,12 +145,12 @@ import { import { nativeRefreshUrl, type NativeTokenSet, - parseStoredTokenSet, parseTokenResponse, resolveLoginStrategy, tokenNeedsRefresh } from './native-oauth' import { runNativeLogin } from './native-oauth-login' +import { loadNativeTokenSet, type NativeTokenStoreIo, persistNativeTokenSet } from './native-token-store' import { serializeJsonBody, setJsonRequestHeaders } from './oauth-net-request' import { createKeepAwake } from './power-save' import { FirstRunSetupResetError, runPrimaryBackendStartup } from './primary-backend-startup' @@ -6225,35 +6225,24 @@ function _nativeTokenStorePath() { return path.join(app.getPath('userData'), 'native-oauth-tokens.json') } -function _readNativeTokenStore(): Record { - try { - const raw = fs.readFileSync(_nativeTokenStorePath(), 'utf8') - const parsed = JSON.parse(raw) - - return parsed && typeof parsed === 'object' ? parsed : {} - } catch { - return {} +// The electron-coupled half of the token store: safeStorage encryption plus the +// userData file. native-token-store.ts owns the serialization/parse round trip +// so it can be tested without an Electron runtime. +function _nativeTokenStoreIo(): NativeTokenStoreIo { + return { + encrypt: encryptDesktopSecret, + decrypt: decryptDesktopSecret, + readStoreText: () => fs.readFileSync(_nativeTokenStorePath(), 'utf8'), + writeStoreText: (text: string) => { + fs.mkdirSync(path.dirname(_nativeTokenStorePath()), { recursive: true }) + fs.writeFileSync(_nativeTokenStorePath(), text, { mode: 0o600 }) + }, + rememberLog } } function _persistNativeTokens(baseUrl: string, tokens: NativeTokenSet | null) { - const store = _readNativeTokenStore() - - if (tokens) { - // Encrypt the whole token set as one blob so the refresh token never - // lands in plaintext on disk. Reuse the hardened encrypt helper. - const secret = encryptDesktopSecret(JSON.stringify(tokens)) - store[baseUrl] = secret - } else { - delete store[baseUrl] - } - - try { - fs.mkdirSync(path.dirname(_nativeTokenStorePath()), { recursive: true }) - fs.writeFileSync(_nativeTokenStorePath(), JSON.stringify(store), { mode: 0o600 }) - } catch (error) { - rememberLog(`[native-oauth] failed to persist tokens: ${(error as Error).message}`) - } + persistNativeTokenSet(baseUrl, tokens, _nativeTokenStoreIo()) } function _loadNativeTokens(baseUrl: string): NativeTokenSet | null { @@ -6263,37 +6252,13 @@ function _loadNativeTokens(baseUrl: string): NativeTokenSet | null { return cached } - const store = _readNativeTokenStore() - const secret = store[baseUrl] + const tokens = loadNativeTokenSet(baseUrl, _nativeTokenStoreIo()) - if (!secret) { - return null - } - - try { - const plaintext = decryptDesktopSecret(secret) - - if (!plaintext) { - rememberLog( - `[native-oauth] failed to decrypt stored tokens for ${baseUrl}; keeping stored entry for retry` - ) - - return null - } - - const tokens = parseStoredTokenSet(JSON.parse(plaintext)) + if (tokens) { _nativeTokens.set(baseUrl, tokens) - - return tokens - } catch (error) { - const detail = error instanceof Error ? error.message : String(error) - - rememberLog( - `[native-oauth] failed to load stored tokens for ${baseUrl}: ${detail}` - ) - - return null } + + return tokens } function _storeNativeTokens(baseUrl: string, tokens: NativeTokenSet) { diff --git a/apps/desktop/electron/native-token-store.test.ts b/apps/desktop/electron/native-token-store.test.ts new file mode 100644 index 00000000000..81e504306aa --- /dev/null +++ b/apps/desktop/electron/native-token-store.test.ts @@ -0,0 +1,292 @@ +/** + * Tests for electron/native-token-store.ts — the encrypted-at-rest persistence + * seam main.ts uses for RFC 8252 native OAuth tokens. + * + * The regression this file exists for (#73271): tokens are persisted as a + * normalized camelCase NativeTokenSet, but the reload path fed the freshly + * decrypted object to parseTokenResponse(), which only understands the + * gateway's snake_case response. It threw on every launch, so a signed-in user + * came back signed out. The parser boundary now lives inside + * loadNativeTokenSet(), so these tests fail if it is ever crossed again. + * + * "Fresh load" here means what it means after a restart: nothing survives but + * the bytes of the store file, so every assertion below is served by + * deserializing and decrypting that text — never by an in-memory object. + * + * (Wired into the vitest `electron` project via electron/**\/*.test.ts.) + */ + +import assert from 'node:assert/strict' + +import { test } from 'vitest' + +import { type NativeTokenSet, parseStoredTokenSet, parseTokenResponse } from './native-oauth' +import { loadNativeTokenSet, type NativeTokenStoreIo, persistNativeTokenSet } from './native-token-store' + +const GATEWAY = 'https://gw.example.com' + +const TOKENS: NativeTokenSet = { + accessToken: 'AT-live-abc123', + refreshToken: 'RT-live-xyz789', + expiresAt: 1_893_456_000, + provider: 'nous', + userId: 'u-42' +} + +interface FakeDisk { + io: NativeTokenStoreIo + logs: string[] + /** The store-file text as it would sit on disk; null when the file is absent. */ + fileText: () => string | null +} + +/** + * A stand-in for the userData store file plus safeStorage. Encryption is + * base64 rather than the OS keychain — opaque-blob-in, same-plaintext-out is + * the only property this seam depends on, and it keeps the round trip + * observable. `initialText` models a process restart: the new "process" starts + * with nothing but the bytes the previous one wrote. + */ +function createFakeDisk(initialText: string | null = null, overrides: Partial = {}): FakeDisk { + let text = initialText + const logs: string[] = [] + + const io: NativeTokenStoreIo = { + encrypt: plaintext => ({ encoding: 'safeStorage', value: Buffer.from(plaintext, 'utf8').toString('base64') }), + decrypt: secret => + secret?.encoding === 'safeStorage' ? Buffer.from(String(secret.value), 'base64').toString('utf8') : '', + readStoreText: () => { + if (text === null) { + // Matches fs.readFileSync on a missing file: throws, not empty string. + throw Object.assign(new Error('ENOENT: no such file or directory'), { code: 'ENOENT' }) + } + + return text + }, + writeStoreText: next => { + text = next + }, + rememberLog: message => logs.push(message), + ...overrides + } + + return { io, logs, fileText: () => text } +} + +// --- the restart round trip --- + +test('a camelCase token set survives store then a fresh load', () => { + const first = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, first.io) + + const onDisk = first.fileText() + + assert.ok(onDisk, 'persisting must write the store file') + + // Nothing may survive the "restart" except those bytes. + const restarted = createFakeDisk(onDisk) + const loaded = loadNativeTokenSet(GATEWAY, restarted.io) + + assert.ok(loaded, 'a stored token set must reload after a restart') + // Reconstructed from the payload, not handed back the object we stored. + assert.notEqual(loaded, TOKENS) + assert.deepEqual(loaded, TOKENS) + assert.deepEqual(restarted.logs, []) +}) + +test('a fresh load restores both tokens and preserves expiry, provider and user', () => { + const first = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, first.io) + + const loaded = loadNativeTokenSet(GATEWAY, createFakeDisk(first.fileText()).io)! + + assert.equal(loaded.accessToken, 'AT-live-abc123') + assert.equal(loaded.refreshToken, 'RT-live-xyz789') + // Still a number after the JSON round trip, not "1893456000". + assert.equal(loaded.expiresAt, 1_893_456_000) + assert.equal(typeof loaded.expiresAt, 'number') + assert.equal(loaded.provider, 'nous') + assert.equal(loaded.userId, 'u-42') +}) + +test('the loaded set is accepted by the stored-token parsing boundary', () => { + const first = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, first.io) + + const loaded = loadNativeTokenSet(GATEWAY, createFakeDisk(first.fileText()).io)! + + // What comes back out of the store is itself a valid stored set — re-parsing + // it is a no-op, so callers can hand it straight to the refresh path. + assert.deepEqual(parseStoredTokenSet(loaded), loaded) +}) + +test('the persisted payload is what broke the old reload path (#73271)', () => { + const first = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, first.io) + + const restarted = createFakeDisk(first.fileText()) + const secret = JSON.parse(restarted.fileText()!)[GATEWAY] + const decrypted = JSON.parse(restarted.io.decrypt(secret)) + + // The old code passed exactly this object to parseTokenResponse(). A + // normalized set has no snake_case access_token, so every launch threw and + // the user was shown as signed out... + assert.throws(() => parseTokenResponse(decrypted), /missing access_token/i) + // ...while the real load path reads the same bytes successfully. + assert.deepEqual(loadNativeTokenSet(GATEWAY, restarted.io), TOKENS) +}) + +test('the full login-to-restart sequence keeps the two parser boundaries apart', () => { + // Login: the gateway answers /auth/native/token in snake_case, and only + // parseTokenResponse() understands that shape. + const fromGateway = parseTokenResponse({ + access_token: 'AT-fresh', + refresh_token: 'RT-fresh', + expires_at: 1_893_456_789, + provider: 'nous', + user_id: 'u-77' + }) + + const first = createFakeDisk() + + persistNativeTokenSet(GATEWAY, fromGateway, first.io) + + // Restart: what was stored is normalized, so the store's own boundary reads + // it back unchanged. + assert.deepEqual(loadNativeTokenSet(GATEWAY, createFakeDisk(first.fileText()).io), fromGateway) +}) + +// --- storage hygiene --- + +test('tokens are encrypted at rest, never plaintext in the store file', () => { + const disk = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, disk.io) + + const onDisk = disk.fileText()! + + assert.doesNotMatch(onDisk, /AT-live-abc123/) + assert.doesNotMatch(onDisk, /RT-live-xyz789/) + assert.equal(JSON.parse(onDisk)[GATEWAY].encoding, 'safeStorage') +}) + +test('persisting one gateway leaves other gateways intact', () => { + const other = 'https://other.example.com' + const disk = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, disk.io) + persistNativeTokenSet(other, { ...TOKENS, accessToken: 'AT-other', userId: 'u-99' }, disk.io) + + const restarted = createFakeDisk(disk.fileText()) + + assert.equal(loadNativeTokenSet(GATEWAY, restarted.io)!.accessToken, 'AT-live-abc123') + assert.equal(loadNativeTokenSet(other, restarted.io)!.accessToken, 'AT-other') +}) + +test('clearing removes only that gateway and reloads as signed out', () => { + const other = 'https://other.example.com' + const disk = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, disk.io) + persistNativeTokenSet(other, TOKENS, disk.io) + persistNativeTokenSet(GATEWAY, null, disk.io) + + const restarted = createFakeDisk(disk.fileText()) + + assert.equal(loadNativeTokenSet(GATEWAY, restarted.io), null) + assert.ok(loadNativeTokenSet(other, restarted.io)) +}) + +test('an absent store file loads as signed out without logging a failure', () => { + const disk = createFakeDisk() + + assert.equal(loadNativeTokenSet(GATEWAY, disk.io), null) + assert.deepEqual(disk.logs, []) +}) + +// --- failure paths (unchanged by the extraction) --- + +test('a locked keychain keeps the stored entry for a later retry', () => { + const first = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, first.io) + + // safeStorage unavailable at load time ⇒ decryptDesktopSecret returns ''. + const locked = createFakeDisk(first.fileText(), { decrypt: () => '' }) + + assert.equal(loadNativeTokenSet(GATEWAY, locked.io), null) + assert.match(locked.logs[0], /failed to decrypt stored tokens for https:\/\/gw\.example\.com/) + assert.match(locked.logs[0], /keeping stored entry for retry/) + // The refresh token must NOT be dropped just because the keychain was locked. + assert.deepEqual(locked.fileText(), first.fileText()) +}) + +test('a corrupt store file loads as signed out instead of throwing', () => { + const disk = createFakeDisk('{not json') + + assert.equal(loadNativeTokenSet(GATEWAY, disk.io), null) + assert.deepEqual(disk.logs, []) +}) + +test('a corrupt decrypted blob is reported and loads as signed out', () => { + const disk = createFakeDisk(JSON.stringify({ [GATEWAY]: { encoding: 'safeStorage', value: 'bm90LWpzb24=' } })) + + assert.equal(loadNativeTokenSet(GATEWAY, disk.io), null) + assert.match(disk.logs[0], /failed to load stored tokens for https:\/\/gw\.example\.com/) +}) + +test('a decrypted blob missing accessToken is rejected, not half-restored', () => { + const plaintext = JSON.stringify({ refreshToken: 'RT-only', provider: 'nous' }) + + const disk = createFakeDisk( + JSON.stringify({ [GATEWAY]: { encoding: 'safeStorage', value: Buffer.from(plaintext).toString('base64') } }) + ) + + assert.equal(loadNativeTokenSet(GATEWAY, disk.io), null) + assert.match(disk.logs[0], /missing accessToken/i) +}) + +test('a non-Error decryption failure keeps its detail in the log', () => { + const disk = createFakeDisk(JSON.stringify({ [GATEWAY]: { encoding: 'safeStorage', value: 'AAAA' } }), { + decrypt: () => { + throw 'keychain exploded' + } + }) + + assert.equal(loadNativeTokenSet(GATEWAY, disk.io), null) + assert.match(disk.logs[0], /keychain exploded/) +}) + +test('an unwritable store file is logged rather than thrown', () => { + const disk = createFakeDisk(null, { + writeStoreText: () => { + throw new Error('EACCES: permission denied') + } + }) + + assert.doesNotThrow(() => persistNativeTokenSet(GATEWAY, TOKENS, disk.io)) + assert.match(disk.logs[0], /failed to persist tokens: EACCES/) +}) + +test('an unusable keychain fails the write loudly and writes nothing', () => { + const existing = createFakeDisk() + + persistNativeTokenSet(GATEWAY, TOKENS, existing.io) + + const before = existing.fileText() + + const broken = createFakeDisk(before, { + encrypt: () => { + throw new Error('Secure token storage is unavailable') + } + }) + + // Storing must not pretend to succeed when the token cannot be encrypted... + assert.throws(() => persistNativeTokenSet(GATEWAY, { ...TOKENS, accessToken: 'AT-new' }, broken.io), /unavailable/) + // ...and must not clobber the tokens already on disk. + assert.equal(broken.fileText(), before) +}) diff --git a/apps/desktop/electron/native-token-store.ts b/apps/desktop/electron/native-token-store.ts new file mode 100644 index 00000000000..889f02cfb9e --- /dev/null +++ b/apps/desktop/electron/native-token-store.ts @@ -0,0 +1,118 @@ +/** + * native-token-store.ts + * + * The encrypted-at-rest persistence seam for RFC 8252 native OAuth tokens: + * NativeTokenSet → JSON → safeStorage blob → store file, and back again on the + * next launch. + * + * Kept standalone (no `import 'electron'`) so the whole restart path unit-tests + * with the `electron` vitest project — the same pattern as native-oauth.ts. + * main.ts owns the electron-coupled halves and injects them: the safeStorage + * encrypt/decrypt pair and the userData store-file read/write. + * + * The parser direction is the load-bearing detail. What lands on disk is the + * *normalized* camelCase NativeTokenSet, so the reload boundary is + * parseStoredTokenSet(). Gateway `/auth/native/token` responses are snake_case + * and stay with parseTokenResponse(); crossing the two made the decrypted set + * throw on every launch, which surfaced as "signed out after restart" (#73271). + */ + +import { type NativeTokenSet, parseStoredTokenSet } from './native-oauth' + +/** One encrypted blob as written per gateway base URL. */ +export interface StoredTokenSecret { + encoding?: string + value?: string +} + +/** + * The narrow set of side effects main.ts owns. Everything here is injected so + * the store/load round trip can be exercised without an Electron runtime, and + * so production keeps using safeStorage unchanged. + */ +export interface NativeTokenStoreIo { + /** + * Encrypt one plaintext blob. main.ts passes the strict safeStorage helper, + * which THROWS when the OS keychain is unavailable — that must stay loud. + */ + encrypt: (plaintext: string) => StoredTokenSecret | null + /** Decrypt a stored payload; returns '' when it cannot be read. */ + decrypt: (secret: any) => string + /** Raw store-file text. Throws when the file is absent — treated as empty. */ + readStoreText: () => string + /** Persist the store-file text (main.ts writes mode 0600 under userData). */ + writeStoreText: (text: string) => void + rememberLog?: (message: string) => void +} + +/** + * baseUrl → encrypted payload. A missing, unreadable, or hand-mangled store + * reads as empty rather than throwing: a failed *read* falls to the next rung. + */ +function readStore(io: NativeTokenStoreIo): Record { + try { + const parsed = JSON.parse(io.readStoreText()) + + return parsed && typeof parsed === 'object' ? parsed : {} + } catch { + return {} + } +} + +/** + * Write (or, with `tokens === null`, drop) one gateway's token set, merging + * into whatever other gateways are already stored. + */ +export function persistNativeTokenSet(baseUrl: string, tokens: NativeTokenSet | null, io: NativeTokenStoreIo): void { + const store = readStore(io) + + if (tokens) { + // Encrypt the whole set as one blob so the refresh token never lands in + // plaintext on disk. Deliberately outside the try below: an unusable + // keychain is an authoritative write failure and must surface to the + // caller, not be logged away as if the tokens were saved. + store[baseUrl] = io.encrypt(JSON.stringify(tokens)) + } else { + delete store[baseUrl] + } + + try { + io.writeStoreText(JSON.stringify(store)) + } catch (error) { + io.rememberLog?.(`[native-oauth] failed to persist tokens: ${(error as Error).message}`) + } +} + +/** + * Reconstruct a gateway's token set from the stored encrypted payload. Returns + * null when nothing is stored, when the blob cannot be decrypted, or when it + * does not parse — never a partially-populated set. + */ +export function loadNativeTokenSet(baseUrl: string, io: NativeTokenStoreIo): NativeTokenSet | null { + const secret = readStore(io)[baseUrl] + + if (!secret) { + return null + } + + try { + const plaintext = io.decrypt(secret) + + if (!plaintext) { + // A keychain that is merely locked/unavailable right now must not cost + // the user their refresh token — leave the entry for the next attempt. + io.rememberLog?.(`[native-oauth] failed to decrypt stored tokens for ${baseUrl}; keeping stored entry for retry`) + + return null + } + + // Stored blobs are normalized camelCase sets, never raw gateway responses. + return parseStoredTokenSet(JSON.parse(plaintext)) + } catch (error) { + const detail = error instanceof Error ? error.message : String(error) + + io.rememberLog?.(`[native-oauth] failed to load stored tokens for ${baseUrl}: ${detail}`) + + return null + } +}