From fab0ca676b1007a7aba072ea8a6f900e7eca776c Mon Sep 17 00:00:00 2001 From: Kainoa Kanter Date: Mon, 29 Dec 2025 12:09:52 -0800 Subject: [PATCH] refactor: use system secrets when possible (#248) --- .github/workflows/ci.yml | 4 + build.js | 22 +- src/agent/client.ts | 9 +- src/auth/setup-ui.tsx | 31 ++- src/cli/App.tsx | 16 +- src/cli/components/WelcomeScreen.tsx | 17 +- src/index.ts | 6 +- src/permissions/loader.ts | 15 +- src/settings-manager.ts | 338 ++++++++++++++++++++++++++- src/tests/secrets.test.ts | 191 +++++++++++++++ src/tests/settings-manager.test.ts | 45 +++- src/utils/secrets.ts | 232 ++++++++++++++++++ 12 files changed, 869 insertions(+), 57 deletions(-) create mode 100644 src/tests/secrets.test.ts create mode 100644 src/utils/secrets.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5574334..b458c7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -66,6 +66,10 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} run: bun install + - name: Unlock GNOME Keyring + if: runner.os == 'Linux' + uses: t1m0thyj/unlock-keyring@v1 + - name: Run tests (extended timeout) run: bun test --timeout 15000 diff --git a/build.js b/build.js index 1593418..fff795d 100644 --- a/build.js +++ b/build.js @@ -29,7 +29,8 @@ await Bun.build({ entry: "letta.js", }, define: { - "process.env.LETTA_VERSION": JSON.stringify(version), + LETTA_VERSION: JSON.stringify(version), + BUILD_TIME: JSON.stringify(new Date().toISOString()), }, // Load text files as strings (for markdown, etc.) loader: { @@ -48,7 +49,20 @@ if (content.startsWith("#!")) { content = content.slice(content.indexOf("\n") + 1); } -const withShebang = `#!/usr/bin/env node\n${content}`; +// Patch secrets requirement back in for node build +content = content.replace( + `(()=>{throw new Error("Cannot require module "+"bun");})().secrets`, + `globalThis.Bun.secrets`, +); + +/** + * Polyglot shebang + * Prefer bun, fallback to node + * ref: https://sambal.org/2014/02/passing-options-node-shebang-line/ + */ +const withShebang = `#!/bin/sh +":" //#; exec /usr/bin/env sh -c 'command -v bun >/dev/null && exec bun "$0" "$@" || exec node "$0" "$@"' "$0" "$@" +${content}`; await Bun.write(outputPath, withShebang); // Make executable @@ -69,6 +83,4 @@ if (existsSync(bundledSkillsSrc)) { console.log("✅ Build complete!"); console.log(` Output: letta.js`); -console.log( - ` Size: ${((await Bun.file(outputPath).size) / 1024).toFixed(0)}KB`, -); +console.log(` Size: ${(Bun.file(outputPath).size / 1024).toFixed(0)}KB`); diff --git a/src/agent/client.ts b/src/agent/client.ts index b2f004e..0092273 100644 --- a/src/agent/client.ts +++ b/src/agent/client.ts @@ -19,7 +19,7 @@ export function getServerUrl(): string { } export async function getClient() { - const settings = settingsManager.getSettings(); + const settings = await settingsManager.getSettingsWithSecureTokens(); let apiKey = process.env.LETTA_API_KEY || settings.env?.LETTA_API_KEY; @@ -45,12 +45,9 @@ export async function getClient() { deviceName, ); - // Update settings with new token - const updatedEnv = { ...settings.env }; - updatedEnv.LETTA_API_KEY = tokens.access_token; - + // Update settings with new token (secrets handles secure storage automatically) settingsManager.updateSettings({ - env: updatedEnv, + env: { ...settings.env, LETTA_API_KEY: tokens.access_token }, refreshToken: tokens.refresh_token || settings.refreshToken, tokenExpiresAt: now + tokens.expires_in * 1000, }); diff --git a/src/auth/setup-ui.tsx b/src/auth/setup-ui.tsx index d3fe1e0..444f733 100644 --- a/src/auth/setup-ui.tsx +++ b/src/auth/setup-ui.tsx @@ -83,21 +83,28 @@ export function SetupUI({ onComplete }: SetupUIProps) { deviceId, deviceName, ) - .then((tokens) => { - // Save tokens + .then(async (tokens) => { + // Save tokens using secrets for secure storage // Note: LETTA_BASE_URL is intentionally NOT saved to settings // It should only come from environment variables const now = Date.now(); - settingsManager.updateSettings({ - env: { - ...settingsManager.getSettings().env, - LETTA_API_KEY: tokens.access_token, - }, - refreshToken: tokens.refresh_token, - tokenExpiresAt: now + tokens.expires_in * 1000, - }); - setMode("done"); - setTimeout(() => onComplete(), 1000); + + try { + // Update settings with non-sensitive data and tokens (secrets handles secure storage) + settingsManager.updateSettings({ + env: { + ...settingsManager.getSettings().env, + LETTA_API_KEY: tokens.access_token, + }, + refreshToken: tokens.refresh_token, + tokenExpiresAt: now + tokens.expires_in * 1000, + }); + + setMode("done"); + setTimeout(() => onComplete(), 1000); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } }) .catch((err) => { setError(err.message); diff --git a/src/cli/App.tsx b/src/cli/App.tsx index 46a5bd7..f0e2b2c 100644 --- a/src/cli/App.tsx +++ b/src/cli/App.tsx @@ -2538,7 +2538,8 @@ export default function App({ try { const { settingsManager } = await import("../settings-manager"); - const currentSettings = settingsManager.getSettings(); + const currentSettings = + await settingsManager.getSettingsWithSecureTokens(); // Revoke refresh token on server if we have one if (currentSettings.refreshToken) { @@ -2546,17 +2547,8 @@ export default function App({ await revokeToken(currentSettings.refreshToken); } - // Clear local credentials - const newEnv = { ...currentSettings.env }; - delete newEnv.LETTA_API_KEY; - // Note: LETTA_BASE_URL is intentionally NOT deleted from settings - // because it should not be stored there in the first place - - settingsManager.updateSettings({ - env: newEnv, - refreshToken: undefined, - tokenExpiresAt: undefined, - }); + // Clear all credentials including secrets + await settingsManager.logout(); buffersRef.current.byId.set(cmdId, { kind: "command", diff --git a/src/cli/components/WelcomeScreen.tsx b/src/cli/components/WelcomeScreen.tsx index 57c9bd3..1d1b11a 100644 --- a/src/cli/components/WelcomeScreen.tsx +++ b/src/cli/components/WelcomeScreen.tsx @@ -1,6 +1,7 @@ import { homedir } from "node:os"; import type { Letta } from "@letta-ai/letta-client"; import { Box, Text } from "ink"; +import { useEffect, useState } from "react"; import type { AgentProvenance } from "../../agent/create"; import { getModelDisplayName } from "../../agent/model"; @@ -24,7 +25,7 @@ function toTildePath(absolutePath: string): string { /** * Determine the auth method used */ -function getAuthMethod(): "url" | "api-key" | "oauth" { +async function getAuthMethod(): Promise<"url" | "api-key" | "oauth"> { // Check if custom URL is being used if (process.env.LETTA_BASE_URL) { return "url"; @@ -33,12 +34,12 @@ function getAuthMethod(): "url" | "api-key" | "oauth" { if (process.env.LETTA_API_KEY) { return "api-key"; } - // Check settings for refresh token (OAuth) - const settings = settingsManager.getSettings(); + // Check settings for refresh token (OAuth) from keychain tokens + const settings = await settingsManager.getSettingsWithSecureTokens(); if (settings.refreshToken) { return "oauth"; } - // Check if API key stored in settings + // Check if API key stored in settings or keychain if (settings.env?.LETTA_API_KEY) { return "api-key"; } @@ -86,7 +87,13 @@ export function WelcomeScreen({ : undefined; // Get auth method - const authMethod = getAuthMethod(); + const [authMethod, setAuthMethod] = useState<"url" | "api-key" | "oauth">( + "oauth", + ); + + useEffect(() => { + getAuthMethod().then(setAuthMethod); + }, []); const authDisplay = authMethod === "url" ? process.env.LETTA_BASE_URL || "Custom URL" diff --git a/src/index.ts b/src/index.ts index b234c8c..538f49d 100755 --- a/src/index.ts +++ b/src/index.ts @@ -60,7 +60,7 @@ BEHAVIOR - Use /profile save to bookmark your current agent Profiles are stored in: - - Global: ~/.letta/settings.json (available everywhere) + - Global: ~/.config/letta/settings.json (available everywhere) - Local: .letta/settings.local.json (pinned to project) If no credentials are configured, you'll be prompted to authenticate via @@ -291,7 +291,7 @@ async function getPinnedAgentNames(): Promise<{ id: string; name: string }[]> { async function main(): Promise { // Initialize settings manager (loads settings once into memory) await settingsManager.initialize(); - const settings = settingsManager.getSettings(); + const settings = await settingsManager.getSettingsWithSecureTokens(); // Initialize telemetry (enabled by default, opt-out via LETTA_CODE_TELEM=0) telemetry.init(); @@ -585,7 +585,7 @@ async function main(): Promise { "Your credentials may be invalid or the server may be unreachable.", ); console.error( - "Delete ~/.letta/settings.json then run 'letta' to re-authenticate", + "Delete ~/.config/letta/settings.json then run 'letta' to re-authenticate", ); process.exit(1); } diff --git a/src/permissions/loader.ts b/src/permissions/loader.ts index d34075e..fbd7fb5 100644 --- a/src/permissions/loader.ts +++ b/src/permissions/loader.ts @@ -17,7 +17,7 @@ type SettingsFile = { * Precedence (highest to lowest): * 1. Local project settings (.letta/settings.local.json) * 2. Project settings (.letta/settings.json) - * 3. User settings (~/.letta/settings.json) + * 3. User settings (~/.config/letta/settings.json) * * Rules are merged by concatenating arrays (more specific settings add to broader ones) */ @@ -33,7 +33,12 @@ export async function loadPermissions( // Load in reverse precedence order (lowest to highest) const sources = [ - join(homedir(), ".letta", "settings.json"), // User + join( + process.env.XDG_CONFIG_HOME || join(homedir(), ".config"), + "letta", + "settings.json", + ), // User + join(homedir(), ".letta", "settings.json"), // User Legacy join(workingDirectory, ".letta", "settings.json"), // Project join(workingDirectory, ".letta", "settings.local.json"), // Local ]; @@ -93,7 +98,11 @@ export async function savePermissionRule( let settingsPath: string; switch (scope) { case "user": - settingsPath = join(homedir(), ".letta", "settings.json"); + settingsPath = join( + process.env.XDG_CONFIG_HOME || join(homedir(), ".config"), + "letta", + "settings.json", + ); break; case "project": settingsPath = join(workingDirectory, ".letta", "settings.json"); diff --git a/src/settings-manager.ts b/src/settings-manager.ts index e3365ea..cef3f40 100644 --- a/src/settings-manager.ts +++ b/src/settings-manager.ts @@ -5,6 +5,13 @@ import { homedir } from "node:os"; import { join } from "node:path"; import type { PermissionRules } from "./permissions/types"; import { exists, mkdir, readFile, writeFile } from "./utils/fs.js"; +import { + deleteSecureTokens, + getSecureTokens, + isKeychainAvailable, + type SecureTokens, + setSecureTokens, +} from "./utils/secrets.js"; export interface Settings { lastAgent: string | null; @@ -17,8 +24,8 @@ export interface Settings { pinnedAgents?: string[]; // Array of agent IDs pinned globally permissions?: PermissionRules; env?: Record; - // Letta Cloud OAuth token management - refreshToken?: string; + // Letta Cloud OAuth token management (stored separately in secrets) + refreshToken?: string; // DEPRECATED: kept for migration, now stored in secrets tokenExpiresAt?: number; // Unix timestamp in milliseconds deviceId?: string; // Tool upsert cache: maps serverUrl -> hash of upserted tools @@ -74,6 +81,7 @@ class SettingsManager { private localProjectSettings: Map = new Map(); private initialized = false; private pendingWrites = new Set>(); + private secretsAvailable: boolean | null = null; /** * Initialize the settings manager (loads from disk) @@ -99,15 +107,104 @@ class SettingsManager { } this.initialized = true; + + // Check secrets availability and warn if not available + await this.checkSecretsSupport(); + + // Migrate tokens to secrets if they exist in settings + await this.migrateTokensToSecrets(); } catch (error) { console.error("Error loading settings, using defaults:", error); this.settings = { ...DEFAULT_SETTINGS }; this.initialized = true; + + // Still check secrets support and try to migrate in case of partial failure + await this.checkSecretsSupport(); + await this.migrateTokensToSecrets(); + } + } + + /** + * Check secrets support and warn user if not available + */ + private async checkSecretsSupport(): Promise { + try { + const available = await this.isKeychainAvailable(); + if (!available) { + console.warn( + "⚠️ System secrets are not available - using fallback storage", + ); + console.warn( + " This may occur when running in Node.js or restricted environments", + ); + } + } catch (error) { + console.warn("⚠️ Could not check secrets availability:", error); + } + } + + /** + * Migrate tokens from old storage location to secrets + */ + private async migrateTokensToSecrets(): Promise { + if (!this.settings) return; + + try { + const tokensToMigrate: SecureTokens = {}; + let needsUpdate = false; + + // Check for refresh token in settings + if (this.settings.refreshToken) { + tokensToMigrate.refreshToken = this.settings.refreshToken; + needsUpdate = true; + } + + // Check for API key in env + if (this.settings.env?.LETTA_API_KEY) { + tokensToMigrate.apiKey = this.settings.env.LETTA_API_KEY; + needsUpdate = true; + } + + // If we have tokens to migrate, store them in secrets + if (needsUpdate && Object.keys(tokensToMigrate).length > 0) { + const available = await this.isKeychainAvailable(); + if (available) { + try { + await setSecureTokens(tokensToMigrate); + + // Remove tokens from settings file + const updatedSettings = { ...this.settings }; + delete updatedSettings.refreshToken; + + if (updatedSettings.env?.LETTA_API_KEY) { + const { LETTA_API_KEY: _, ...otherEnv } = updatedSettings.env; + updatedSettings.env = + Object.keys(otherEnv).length > 0 ? otherEnv : undefined; + } + + this.settings = updatedSettings; + await this.persistSettings(); + + console.log("Successfully migrated tokens to secrets"); + } catch (error) { + console.warn("Failed to migrate tokens to secrets:", error); + console.warn("Tokens will remain in settings file for persistence"); + } + } else { + console.warn( + "Secrets not available - tokens will remain in settings file for persistence", + ); + } + } + } catch (error) { + console.warn("Failed to migrate tokens to secrets:", error); + // Don't throw - app should still work with tokens in settings file } } /** * Get all settings (synchronous, from memory) + * Note: Does not include secure tokens (API key, refresh token) from secrets */ getSettings(): Settings { if (!this.initialized || !this.settings) { @@ -118,6 +215,40 @@ class SettingsManager { return { ...this.settings }; } + /** + * Get all settings including secure tokens from secrets (async) + */ + async getSettingsWithSecureTokens(): Promise { + const baseSettings = this.getSettings(); + let secureTokens: SecureTokens = {}; + + // Try to get tokens from secrets first + const secretsAvailable = await this.isKeychainAvailable(); + if (secretsAvailable) { + secureTokens = await this.getSecureTokens(); + } + + // Fallback to tokens in settings file if secrets are not available + const fallbackRefreshToken = + !secureTokens.refreshToken && baseSettings.refreshToken + ? baseSettings.refreshToken + : secureTokens.refreshToken; + + const fallbackApiKey = + !secureTokens.apiKey && baseSettings.env?.LETTA_API_KEY + ? baseSettings.env.LETTA_API_KEY + : secureTokens.apiKey; + + return { + ...baseSettings, + env: { + ...baseSettings.env, + ...(fallbackApiKey && { LETTA_API_KEY: fallbackApiKey }), + }, + refreshToken: fallbackRefreshToken, + }; + } + /** * Get a specific setting value (synchronous) */ @@ -148,10 +279,37 @@ class SettingsManager { ); } - this.settings = { ...this.settings, ...updates }; + // Extract secure tokens from updates + const { env, refreshToken, ...otherUpdates } = updates; + let apiKey: string | undefined; + let updatedEnv = env; - // Persist asynchronously (track promise for testing) - const writePromise = this.persistSettings() + // Check for API key in env updates + if (env?.LETTA_API_KEY) { + apiKey = env.LETTA_API_KEY; + // Remove from env to prevent storing in settings file + const { LETTA_API_KEY: _, ...otherEnv } = env; + updatedEnv = Object.keys(otherEnv).length > 0 ? otherEnv : undefined; + } + + // Update in-memory settings (without sensitive tokens) + this.settings = { + ...this.settings, + ...otherUpdates, + ...(updatedEnv && { env: { ...this.settings.env, ...updatedEnv } }), + }; + + // Handle secure tokens in keychain + const secureTokens: SecureTokens = {}; + if (apiKey) { + secureTokens.apiKey = apiKey; + } + if (refreshToken) { + secureTokens.refreshToken = refreshToken; + } + + // Persist both regular settings and secure tokens asynchronously + const writePromise = this.persistSettingsAndTokens(secureTokens) .catch((error) => { console.error("Failed to persist settings:", error); }) @@ -161,6 +319,59 @@ class SettingsManager { this.pendingWrites.add(writePromise); } + /** + * Persist settings and tokens, with fallback for secrets unavailability + */ + private async persistSettingsAndTokens( + secureTokens: SecureTokens, + ): Promise { + const secretsAvailable = await this.isKeychainAvailable(); + + if (secretsAvailable && Object.keys(secureTokens).length > 0) { + // Try to store tokens in secrets, fall back to settings file if it fails + try { + await Promise.all([ + this.persistSettings(), + this.setSecureTokens(secureTokens), + ]); + return; + } catch (error) { + console.warn( + "Failed to store tokens in secrets, falling back to settings file:", + error, + ); + // Continue to fallback logic below + } + } + + if (Object.keys(secureTokens).length > 0) { + // Fallback: store tokens in settings file + console.warn( + "Secrets not available, storing tokens in settings file for persistence", + ); + + // biome-ignore lint/style/noNonNullAssertion: at this point will always exist + const fallbackSettings: Settings = { ...this.settings! }; + + if (secureTokens.refreshToken) { + fallbackSettings.refreshToken = secureTokens.refreshToken; + } + + if (secureTokens.apiKey) { + fallbackSettings.env = { + ...fallbackSettings.env, + LETTA_API_KEY: secureTokens.apiKey, + }; + } + + this.settings = fallbackSettings; + await this.persistSettings(); + } else { + // No tokens to store, just persist regular settings + await this.persistSettings(); + } + } + /** * Load project settings for a specific directory */ @@ -250,8 +461,10 @@ class SettingsManager { if (!this.settings) return; const settingsPath = this.getSettingsPath(); - const home = process.env.HOME || homedir(); - const dirPath = join(home, ".letta"); + const dirPath = join( + process.env.XDG_CONFIG_HOME || join(homedir(), ".config"), + "letta", + ); try { if (!exists(dirPath)) { @@ -303,9 +516,11 @@ class SettingsManager { } private getSettingsPath(): string { - // Respect process.env.HOME for testing (homedir() ignores it) - const home = process.env.HOME || homedir(); - return join(home, ".letta", "settings.json"); + return join( + process.env.XDG_CONFIG_HOME || join(homedir(), ".config"), + "letta", + "settings.json", + ); } private getProjectSettingsPath(workingDirectory: string): string { @@ -734,6 +949,73 @@ class SettingsManager { }); } + /** + * Check if secrets are available + */ + async isKeychainAvailable(): Promise { + if (this.secretsAvailable === null) { + this.secretsAvailable = await isKeychainAvailable(); + } + return this.secretsAvailable; + } + + /** + * Get secure tokens from secrets + */ + async getSecureTokens(): Promise { + const available = await this.isKeychainAvailable(); + if (!available) { + return {}; + } + + try { + return await getSecureTokens(); + } catch (error) { + console.warn("Failed to retrieve tokens from secrets:", error); + return {}; + } + } + + /** + * Store secure tokens in secrets + */ + async setSecureTokens(tokens: SecureTokens): Promise { + const available = await this.isKeychainAvailable(); + if (!available) { + console.warn( + "Secrets not available, tokens will use fallback storage (not persistent across restarts)", + ); + return; + } + + try { + await setSecureTokens(tokens); + } catch (error) { + console.warn( + "Failed to store tokens in secrets, falling back to settings file", + ); + // Let the caller handle the fallback by throwing again + throw error; + } + } + + /** + * Delete secure tokens from secrets + */ + async deleteSecureTokens(): Promise { + const available = await this.isKeychainAvailable(); + if (!available) { + return; + } + + try { + await deleteSecureTokens(); + } catch (error) { + console.warn("Failed to delete tokens from secrets:", error); + // Continue anyway as the tokens might not exist + } + } + /** * Wait for all pending writes to complete. * Useful in tests to ensure writes finish before cleanup. @@ -742,6 +1024,41 @@ class SettingsManager { await Promise.all(Array.from(this.pendingWrites)); } + /** + * Logout - clear all tokens and sensitive authentication data + */ + async logout(): Promise { + try { + // Clear tokens from secrets + await this.deleteSecureTokens(); + + // Clear token-related settings from in-memory settings + if (this.settings) { + const updatedSettings = { ...this.settings }; + delete updatedSettings.refreshToken; + delete updatedSettings.tokenExpiresAt; + delete updatedSettings.deviceId; + + // Clear API key from env if present + if (updatedSettings.env?.LETTA_API_KEY) { + const { LETTA_API_KEY: _, ...otherEnv } = updatedSettings.env; + updatedSettings.env = + Object.keys(otherEnv).length > 0 ? otherEnv : undefined; + } + + this.settings = updatedSettings; + await this.persistSettings(); + } + + console.log( + "Successfully logged out and cleared all authentication data", + ); + } catch (error) { + console.error("Error during logout:", error); + throw error; + } + } + /** * Reset the manager (mainly for testing). * Waits for pending writes to complete before resetting. @@ -755,6 +1072,7 @@ class SettingsManager { this.localProjectSettings.clear(); this.initialized = false; this.pendingWrites.clear(); + this.secretsAvailable = null; } } diff --git a/src/tests/secrets.test.ts b/src/tests/secrets.test.ts new file mode 100644 index 0000000..a36a324 --- /dev/null +++ b/src/tests/secrets.test.ts @@ -0,0 +1,191 @@ +// src/tests/keychain.test.ts +// Tests for secrets utility functions + +import { afterEach, beforeEach, describe, expect, test } from "bun:test"; +import { + deleteApiKey, + deleteRefreshToken, + deleteSecureTokens, + getApiKey, + getRefreshToken, + getSecureTokens, + isKeychainAvailable, + keychainAvailablePrecompute, + type SecureTokens, + setApiKey, + setRefreshToken, + setSecureTokens, +} from "../utils/secrets"; + +describe("Secrets utilities", () => { + beforeEach(async () => { + if (keychainAvailablePrecompute) { + await deleteSecureTokens(); + } + }); + + afterEach(async () => { + if (keychainAvailablePrecompute) { + await deleteSecureTokens(); + } + }); + + test("isKeychainAvailable works", async () => { + const available = await isKeychainAvailable(); + expect(typeof available).toBe("boolean"); + }); + + test.skipIf(!keychainAvailablePrecompute)( + "can store and retrieve API key", + async () => { + const testApiKey = "sk-test-api-key-12345"; + + await setApiKey(testApiKey); + const retrievedApiKey = await getApiKey(); + + expect(retrievedApiKey).toBe(testApiKey); + }, + ); + + test.skipIf(!keychainAvailablePrecompute)( + "can store and retrieve refresh token", + async () => { + const testRefreshToken = "rt-test-refresh-token-67890"; + + await setRefreshToken(testRefreshToken); + const retrievedRefreshToken = await getRefreshToken(); + + expect(retrievedRefreshToken).toBe(testRefreshToken); + }, + ); + + test.skipIf(!keychainAvailablePrecompute)( + "can store and retrieve both tokens together", + async () => { + const tokens: SecureTokens = { + apiKey: "sk-test-api-key-combined", + refreshToken: "rt-test-refresh-token-combined", + }; + + await setSecureTokens(tokens); + const retrievedTokens = await getSecureTokens(); + + expect(retrievedTokens.apiKey).toBe(tokens.apiKey); + expect(retrievedTokens.refreshToken).toBe(tokens.refreshToken); + }, + ); + + test.skipIf(!keychainAvailablePrecompute)("can delete API key", async () => { + const testApiKey = "sk-test-api-key-delete"; + + await setApiKey(testApiKey); + let retrievedApiKey = await getApiKey(); + expect(retrievedApiKey).toBe(testApiKey); + + await deleteApiKey(); + retrievedApiKey = await getApiKey(); + expect(retrievedApiKey).toBe(null); + }); + + test.skipIf(!keychainAvailablePrecompute)( + "can delete refresh token", + async () => { + const testRefreshToken = "rt-test-refresh-token-delete"; + + await setRefreshToken(testRefreshToken); + let retrievedRefreshToken = await getRefreshToken(); + expect(retrievedRefreshToken).toBe(testRefreshToken); + + await deleteRefreshToken(); + retrievedRefreshToken = await getRefreshToken(); + expect(retrievedRefreshToken).toBe(null); + }, + ); + + test.skipIf(!keychainAvailablePrecompute)( + "can delete all tokens", + async () => { + const tokens: SecureTokens = { + apiKey: "sk-test-api-key-delete-all", + refreshToken: "rt-test-refresh-token-delete-all", + }; + + await setSecureTokens(tokens); + let retrievedTokens = await getSecureTokens(); + expect(retrievedTokens.apiKey).toBe(tokens.apiKey); + expect(retrievedTokens.refreshToken).toBe(tokens.refreshToken); + + await deleteSecureTokens(); + retrievedTokens = await getSecureTokens(); + expect(retrievedTokens.apiKey).toBeUndefined(); + expect(retrievedTokens.refreshToken).toBeUndefined(); + }, + ); + + test.skipIf(!keychainAvailablePrecompute)( + "returns null for non-existent tokens", + async () => { + // Ensure no tokens exist + await deleteSecureTokens(); + + const apiKey = await getApiKey(); + const refreshToken = await getRefreshToken(); + const tokens = await getSecureTokens(); + + expect(apiKey).toBe(null); + expect(refreshToken).toBe(null); + expect(tokens.apiKey).toBeUndefined(); + expect(tokens.refreshToken).toBeUndefined(); + }, + ); + + test.skipIf(!keychainAvailablePrecompute)( + "handles partial token storage", + async () => { + // Store only API key + await setSecureTokens({ apiKey: "sk-only-api-key" }); + + let tokens = await getSecureTokens(); + expect(tokens.apiKey).toBe("sk-only-api-key"); + expect(tokens.refreshToken).toBeUndefined(); + + // Clean up and store only refresh token + await deleteSecureTokens(); + await setSecureTokens({ refreshToken: "rt-only-refresh-token" }); + + tokens = await getSecureTokens(); + expect(tokens.apiKey).toBeUndefined(); + expect(tokens.refreshToken).toBe("rt-only-refresh-token"); + }, + ); + + test("gracefully handles secrets unavailability", async () => { + // This test should work even if secrets are not available + if (await isKeychainAvailable()) { + // If secrets are available, this is a basic functionality test + const tokens = await getSecureTokens(); + expect(typeof tokens).toBe("object"); + } else { + // If secrets are not available, functions should return null or throw appropriately + const tokens = await getSecureTokens(); + expect(tokens.apiKey).toBeUndefined(); + expect(tokens.refreshToken).toBeUndefined(); + + const apiKey = await getApiKey(); + expect(apiKey).toBe(null); + + const refreshToken = await getRefreshToken(); + expect(refreshToken).toBe(null); + + // Set operations should throw when secrets unavailable (handled by settings manager) + await expect(setSecureTokens({ apiKey: "test" })).rejects.toThrow(); + await expect(setApiKey("test")).rejects.toThrow(); + await expect(setRefreshToken("test")).rejects.toThrow(); + + // Delete operations should not throw (no-op when secrets unavailable) + await expect(deleteSecureTokens()).resolves.toBeUndefined(); + await expect(deleteApiKey()).resolves.toBeUndefined(); + await expect(deleteRefreshToken()).resolves.toBeUndefined(); + } + }); +}); diff --git a/src/tests/settings-manager.test.ts b/src/tests/settings-manager.test.ts index 61af712..5c93e5f 100644 --- a/src/tests/settings-manager.test.ts +++ b/src/tests/settings-manager.test.ts @@ -3,6 +3,11 @@ import { mkdtemp, rm } from "node:fs/promises"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { settingsManager } from "../settings-manager"; +import { + deleteSecureTokens, + isKeychainAvailable, + keychainAvailablePrecompute, +} from "../utils/secrets.js"; // Store original HOME to restore after tests const originalHome = process.env.HOME; @@ -94,8 +99,24 @@ describe("Settings Manager - Initialization", () => { // ============================================================================ describe("Settings Manager - Global Settings", () => { + let keychainSupported: boolean = false; + beforeEach(async () => { await settingsManager.initialize(); + // Check if secrets are available on this system + keychainSupported = await isKeychainAvailable(); + + if (keychainSupported) { + // Clean up any existing test tokens + await deleteSecureTokens(); + } + }); + + afterEach(async () => { + if (keychainSupported) { + // Clean up after each test + await deleteSecureTokens(); + } }); test("Get settings returns a copy", () => { @@ -162,12 +183,34 @@ describe("Settings Manager - Global Settings", () => { }); const settings = settingsManager.getSettings(); + // LETTA_API_KEY should not be in settings file (moved to keychain) expect(settings.env).toEqual({ - LETTA_API_KEY: "sk-test-123", CUSTOM_VAR: "value", }); }); + test.skipIf(!keychainAvailablePrecompute)( + "Get settings with secure tokens (async method)", + async () => { + // This test verifies the async method that includes keychain tokens + settingsManager.updateSettings({ + env: { + LETTA_API_KEY: "sk-test-async-123", + CUSTOM_VAR: "async-value", + }, + refreshToken: "rt-test-refresh", + tokenExpiresAt: Date.now() + 3600000, + }); + + const settingsWithTokens = + await settingsManager.getSettingsWithSecureTokens(); + + // Should include the environment variables and other settings + expect(settingsWithTokens.env?.CUSTOM_VAR).toBe("async-value"); + expect(typeof settingsWithTokens.tokenExpiresAt).toBe("number"); + }, + ); + test("LETTA_BASE_URL should not be cached in settings", () => { // This test verifies that LETTA_BASE_URL is NOT persisted to settings // It should only come from environment variables diff --git a/src/utils/secrets.ts b/src/utils/secrets.ts new file mode 100644 index 0000000..e0c221c --- /dev/null +++ b/src/utils/secrets.ts @@ -0,0 +1,232 @@ +/// +// src/utils/secrets.ts +// Secure storage utilities for tokens using Bun's secrets API with Node.js fallback + +let secrets: typeof Bun.secrets; +let secretsAvailable = false; + +// Try to import Bun's secrets API, fallback if unavailable +try { + secrets = require("bun").secrets; + secretsAvailable = true; +} catch { + // Running in Node.js or Bun secrets unavailable + secretsAvailable = false; +} + +const SERVICE_NAME = "letta-code"; +const API_KEY_NAME = "letta-api-key"; +const REFRESH_TOKEN_NAME = "letta-refresh-token"; + +// Note: When secrets API is unavailable (Node.js), tokens will be managed +// by the settings manager which falls back to storing in the settings file +// This provides persistence across restarts + +export interface SecureTokens { + apiKey?: string; + refreshToken?: string; +} + +/** + * Store API key in system secrets + */ +export async function setApiKey(apiKey: string): Promise { + if (secretsAvailable) { + try { + await secrets.set({ + service: SERVICE_NAME, + name: API_KEY_NAME, + value: apiKey, + }); + return; + } catch (error) { + console.warn( + `Failed to store API key in secrets, using fallback: ${error}`, + ); + } + } + + // When secrets unavailable, let the settings manager handle fallback + throw new Error("Secrets API unavailable"); +} + +/** + * Retrieve API key from system secrets + */ +export async function getApiKey(): Promise { + if (secretsAvailable) { + try { + return await secrets.get({ + service: SERVICE_NAME, + name: API_KEY_NAME, + }); + } catch (error) { + console.warn(`Failed to retrieve API key from secrets: ${error}`); + } + } + + // When secrets unavailable, return null (settings manager will use fallback) + return null; +} + +/** + * Store refresh token in system secrets + */ +export async function setRefreshToken(refreshToken: string): Promise { + if (secretsAvailable) { + try { + await secrets.set({ + service: SERVICE_NAME, + name: REFRESH_TOKEN_NAME, + value: refreshToken, + }); + return; + } catch (error) { + console.warn( + `Failed to store refresh token in secrets, using fallback: ${error}`, + ); + } + } + + // When secrets unavailable, let the settings manager handle fallback + throw new Error("Secrets API unavailable"); +} + +/** + * Retrieve refresh token from system secrets + */ +export async function getRefreshToken(): Promise { + if (secretsAvailable) { + try { + return await secrets.get({ + service: SERVICE_NAME, + name: REFRESH_TOKEN_NAME, + }); + } catch (error) { + console.warn(`Failed to retrieve refresh token from secrets: ${error}`); + } + } + + // When secrets unavailable, return null (settings manager will use fallback) + return null; +} + +/** + * Get both tokens from secrets + */ +export async function getSecureTokens(): Promise { + const [apiKey, refreshToken] = await Promise.allSettled([ + getApiKey(), + getRefreshToken(), + ]); + + return { + apiKey: + apiKey.status === "fulfilled" ? apiKey.value || undefined : undefined, + refreshToken: + refreshToken.status === "fulfilled" + ? refreshToken.value || undefined + : undefined, + }; +} + +/** + * Store both tokens in secrets + */ +export async function setSecureTokens(tokens: SecureTokens): Promise { + const promises: Promise[] = []; + + if (tokens.apiKey) { + promises.push(setApiKey(tokens.apiKey)); + } + + if (tokens.refreshToken) { + promises.push(setRefreshToken(tokens.refreshToken)); + } + + if (promises.length > 0) { + await Promise.all(promises); + } +} + +/** + * Remove API key from system secrets + */ +export async function deleteApiKey(): Promise { + if (secretsAvailable) { + try { + await secrets.delete({ + service: SERVICE_NAME, + name: API_KEY_NAME, + }); + return; + } catch (error) { + console.warn(`Failed to delete API key from secrets: ${error}`); + } + } + + // When secrets unavailable, deletion is handled by settings manager + // No action needed here +} + +/** + * Remove refresh token from system secrets + */ +export async function deleteRefreshToken(): Promise { + if (secretsAvailable) { + try { + await secrets.delete({ + service: SERVICE_NAME, + name: REFRESH_TOKEN_NAME, + }); + return; + } catch (error) { + console.warn(`Failed to delete refresh token from secrets: ${error}`); + } + } + + // When secrets unavailable, deletion is handled by settings manager + // No action needed here +} + +/** + * Remove all tokens from system secrets + */ +export async function deleteSecureTokens(): Promise { + await Promise.allSettled([deleteApiKey(), deleteRefreshToken()]); +} + +/** + * Check if secrets API is available + */ +export async function isKeychainAvailable(): Promise { + if (!secretsAvailable) { + return false; + } + + try { + // Try to set and delete a test value + const testName = "test-availability"; + const testValue = "test"; + + await secrets.set({ + service: SERVICE_NAME, + name: testName, + value: testValue, + }); + + await secrets.delete({ + service: SERVICE_NAME, + name: testName, + }); + + return true; + } catch { + return false; + } +} + +/** Const value of isKeychainAvailable + * Precomputed for tests + */ +export const keychainAvailablePrecompute = await isKeychainAvailable();