From 6cb459af9e8564e329e1865c1ac57429d9ebf237 Mon Sep 17 00:00:00 2001 From: Doud-FR <59610009+Doud-FR@users.noreply.github.com> Date: Fri, 31 Jul 2026 01:01:56 +0200 Subject: [PATCH] fix(desktop): harden native token store handling --- .../electron/native-token-store.test.ts | 36 +++++++++++++++++++ apps/desktop/electron/native-token-store.ts | 11 ++++-- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/apps/desktop/electron/native-token-store.test.ts b/apps/desktop/electron/native-token-store.test.ts index 81e504306aa..83478b42e60 100644 --- a/apps/desktop/electron/native-token-store.test.ts +++ b/apps/desktop/electron/native-token-store.test.ts @@ -232,6 +232,29 @@ test('a corrupt store file loads as signed out instead of throwing', () => { assert.deepEqual(disk.logs, []) }) +test('an array store file loads as signed out instead of throwing', () => { + const disk = createFakeDisk('[]') + + assert.equal(loadNativeTokenSet(GATEWAY, disk.io), null) + assert.deepEqual(disk.logs, []) +}) + +test('an array store file is replaced by a real map rather than swallowing the write', () => { + const disk = createFakeDisk('[]') + + persistNativeTokenSet(GATEWAY, TOKENS, disk.io) + + const written = JSON.parse(disk.fileText()!) + + // Assigning store[baseUrl] on an array sets a non-index property, which + // JSON.stringify drops — the write would report success and the tokens would + // be gone on the next launch. + assert.equal(Array.isArray(written), false) + assert.ok(written[GATEWAY], 'the gateway entry must survive serialization') + // And it really does come back after a restart. + assert.deepEqual(loadNativeTokenSet(GATEWAY, createFakeDisk(disk.fileText()).io), TOKENS) +}) + test('a corrupt decrypted blob is reported and loads as signed out', () => { const disk = createFakeDisk(JSON.stringify({ [GATEWAY]: { encoding: 'safeStorage', value: 'bm90LWpzb24=' } })) @@ -272,6 +295,19 @@ test('an unwritable store file is logged rather than thrown', () => { assert.match(disk.logs[0], /failed to persist tokens: EACCES/) }) +test('a non-Error write failure keeps its detail in the log', () => { + const disk = createFakeDisk(null, { + writeStoreText: () => { + throw 'disk went away' + } + }) + + // `(error as Error).message` on a thrown string reads as undefined and loses + // the only diagnostic there was. + assert.doesNotThrow(() => persistNativeTokenSet(GATEWAY, TOKENS, disk.io)) + assert.equal(disk.logs[0], '[native-oauth] failed to persist tokens: disk went away') +}) + test('an unusable keychain fails the write loudly and writes nothing', () => { const existing = createFakeDisk() diff --git a/apps/desktop/electron/native-token-store.ts b/apps/desktop/electron/native-token-store.ts index 889f02cfb9e..1adb00a5a20 100644 --- a/apps/desktop/electron/native-token-store.ts +++ b/apps/desktop/electron/native-token-store.ts @@ -48,12 +48,17 @@ export interface NativeTokenStoreIo { /** * baseUrl → encrypted payload. A missing, unreadable, or hand-mangled store * reads as empty rather than throwing: a failed *read* falls to the next rung. + * + * Arrays are rejected alongside every other non-object shape: assigning + * store[baseUrl] on an array would set a non-index property, which + * JSON.stringify drops on the way back out — the write would look like it + * succeeded and the tokens would be gone on the next launch. */ function readStore(io: NativeTokenStoreIo): Record { try { const parsed = JSON.parse(io.readStoreText()) - return parsed && typeof parsed === 'object' ? parsed : {} + return parsed && typeof parsed === 'object' && !Array.isArray(parsed) ? parsed : {} } catch { return {} } @@ -79,7 +84,9 @@ export function persistNativeTokenSet(baseUrl: string, tokens: NativeTokenSet | try { io.writeStoreText(JSON.stringify(store)) } catch (error) { - io.rememberLog?.(`[native-oauth] failed to persist tokens: ${(error as Error).message}`) + const detail = error instanceof Error ? error.message : String(error) + + io.rememberLog?.(`[native-oauth] failed to persist tokens: ${detail}`) } }