test(desktop): cover native OAuth persistence path

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<string, any> {
-  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) {

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<NativeTokenStoreIo> = {}): 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)
+})

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<string, any> {
+  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
+  }
+}