From 42af5942c20438fce485ffc5955e674ec194cde8 Mon Sep 17 00:00:00 2001 From: Mark Bennett Date: Wed, 11 Feb 2026 06:20:49 -0700 Subject: [PATCH] Move session metadata from keychain to plain file to prevent credential loss MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The current-session-metadata keychain entry was getting wiped by transient errors (e.g. network failure on wake) in the resumeSession() catch block, forcing users to re-authenticate unnecessarily. Changes: - Store session metadata in ~/.config/tangled/session.json (not keychain) so it survives sleep/wake cycles and keychain lock events - Don't clear metadata on transient agent.resumeSession() failures — only clear when loadSession() definitively returns null (no stored credentials) - Update session.test.ts to mock node:fs/promises with in-memory storage - Update api-client.test.ts to reflect new non-clearing behavior Co-Authored-By: Claude Sonnet 4.5 --- src/lib/api-client.ts | 5 +++-- src/lib/session.ts | 36 +++++++++++++++++++++--------------- tests/lib/api-client.test.ts | 4 ++-- tests/lib/session.test.ts | 31 +++++++++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 19 deletions(-) diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index 7843797..a53ea67 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -111,8 +111,9 @@ export class TangledApiClient { // Don't clear credentials — keychain may just be temporarily locked throw error; } - // Session data invalid or agent resume failed — clear stale state - await clearCurrentSessionMetadata(); + // Session resume failed (network error, expired refresh token, etc.) + // Don't clear credentials — the error may be transient. The user can + // run "auth login" explicitly if they need to re-authenticate. return false; } } diff --git a/src/lib/session.ts b/src/lib/session.ts index f1b799b..9c0ed16 100644 --- a/src/lib/session.ts +++ b/src/lib/session.ts @@ -1,7 +1,11 @@ +import { mkdir, readFile, unlink, writeFile } from 'node:fs/promises'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; import type { AtpSessionData } from '@atproto/api'; import { AsyncEntry } from '@napi-rs/keyring'; const SERVICE_NAME = 'tangled-cli'; +const SESSION_METADATA_PATH = join(homedir(), '.config', 'tangled', 'session.json'); export class KeychainAccessError extends Error { constructor(message: string) { @@ -73,13 +77,13 @@ export async function deleteSession(accountId: string): Promise { } /** - * Store metadata about current session for CLI to track active user - * Uses a special "current" account in keychain + * Store metadata about current session for CLI to track active user. + * Written to a plain file — metadata is not secret and must be readable + * even when the keychain is locked (e.g. after sleep/wake). */ export async function saveCurrentSessionMetadata(metadata: SessionMetadata): Promise { - const serialized = JSON.stringify(metadata); - const entry = new AsyncEntry(SERVICE_NAME, 'current-session-metadata'); - await entry.setPassword(serialized); + await mkdir(join(homedir(), '.config', 'tangled'), { recursive: true }); + await writeFile(SESSION_METADATA_PATH, JSON.stringify(metadata, null, 2), 'utf-8'); } /** @@ -87,16 +91,13 @@ export async function saveCurrentSessionMetadata(metadata: SessionMetadata): Pro */ export async function getCurrentSessionMetadata(): Promise { try { - const entry = new AsyncEntry(SERVICE_NAME, 'current-session-metadata'); - const serialized = await entry.getPassword(); - if (!serialized) { + const content = await readFile(SESSION_METADATA_PATH, 'utf-8'); + return JSON.parse(content) as SessionMetadata; + } catch (error) { + if ((error as NodeJS.ErrnoException).code === 'ENOENT') { return null; } - return JSON.parse(serialized) as SessionMetadata; - } catch (error) { - throw new KeychainAccessError( - `Cannot access keychain: ${error instanceof Error ? error.message : 'Unknown error'}` - ); + throw error; } } @@ -104,6 +105,11 @@ export async function getCurrentSessionMetadata(): Promise { - const entry = new AsyncEntry(SERVICE_NAME, 'current-session-metadata'); - await entry.deleteCredential(); + try { + await unlink(SESSION_METADATA_PATH); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'ENOENT') { + throw error; + } + } } diff --git a/tests/lib/api-client.test.ts b/tests/lib/api-client.test.ts index 877d463..aece469 100644 --- a/tests/lib/api-client.test.ts +++ b/tests/lib/api-client.test.ts @@ -143,7 +143,7 @@ describe('TangledApiClient', () => { expect(vi.mocked(sessionModule.clearCurrentSessionMetadata)).toHaveBeenCalled(); }); - it('should return false and cleanup on resume error', async () => { + it('should return false without clearing metadata on transient resume error', async () => { vi.mocked(sessionModule.getCurrentSessionMetadata).mockResolvedValue(mockSessionMetadata); vi.mocked(sessionModule.loadSession).mockResolvedValue(mockSessionData); @@ -153,7 +153,7 @@ describe('TangledApiClient', () => { const resumed = await client.resumeSession(); expect(resumed).toBe(false); - expect(vi.mocked(sessionModule.clearCurrentSessionMetadata)).toHaveBeenCalled(); + expect(vi.mocked(sessionModule.clearCurrentSessionMetadata)).not.toHaveBeenCalled(); }); it('should rethrow KeychainAccessError without clearing metadata', async () => { diff --git a/tests/lib/session.test.ts b/tests/lib/session.test.ts index da387f9..f1c3b65 100644 --- a/tests/lib/session.test.ts +++ b/tests/lib/session.test.ts @@ -33,16 +33,47 @@ vi.mock('@napi-rs/keyring', () => { }; }); +// Mock node:fs/promises for metadata file storage +const mockFileStorage = new Map(); + +vi.mock('node:fs/promises', () => ({ + mkdir: vi.fn().mockResolvedValue(undefined), + writeFile: vi.fn().mockImplementation(async (path: string, content: string) => { + mockFileStorage.set(path as string, content); + }), + readFile: vi.fn().mockImplementation(async (path: string) => { + const content = mockFileStorage.get(path as string); + if (content === undefined) { + const err = Object.assign(new Error(`ENOENT: no such file or directory, open '${path}'`), { + code: 'ENOENT', + }); + throw err; + } + return content; + }), + unlink: vi.fn().mockImplementation(async (path: string) => { + if (!mockFileStorage.has(path as string)) { + const err = Object.assign(new Error(`ENOENT: no such file or directory, unlink '${path}'`), { + code: 'ENOENT', + }); + throw err; + } + mockFileStorage.delete(path as string); + }), +})); + describe('Session Management', () => { beforeEach(() => { // Clear mock storage before each test mockKeyringStorage.clear(); + mockFileStorage.clear(); vi.clearAllMocks(); }); afterEach(() => { // Clean up after each test mockKeyringStorage.clear(); + mockFileStorage.clear(); }); describe('saveSession', () => { -- 2.51.2