From ae728ba828ced2e5efc9625b7ec822f22888985c Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 14 Sep 2026 18:01:57 +0000 Subject: [PATCH] fix: persist CLI credentials on headless Linux Pin the Linux keyring to Secret Service so @napi-rs/keyring cannot silently use the in-memory kernel keyring. When Secret Service is missing, and when keyring get returns null, fall back to the documented 0600 file store. Closes #5 Co-authored-by: Kent C. Dodds --- README.md | 2 +- package.json | 2 +- skills/kody/SKILL.md | 6 +- src/store.ts | 64 ++++++++++++++----- test/store.test.ts | 149 ++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 201 insertions(+), 22 deletions(-) diff --git a/README.md b/README.md index 33881ba..c5adf64 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ CLI credentials are stored in the OS secret store: - macOS: Keychain - Windows: Credential Manager -- Linux: Secret Service / libsecret +- Linux: Secret Service / libsecret (not the in-memory kernel keyring) If the keychain is unavailable (common on headless Linux), the CLI writes a `0600` file under `$XDG_CONFIG_HOME/kody` (or `%APPDATA%\kody` on Windows, diff --git a/package.json b/package.json index 04c6933..51c5eb6 100644 --- a/package.json +++ b/package.json @@ -49,7 +49,7 @@ "dependencies": { "@inquirer/checkbox": "^5.2.2", "@modelcontextprotocol/client": "2.0.0", - "@napi-rs/keyring": "^1.3.0", + "@napi-rs/keyring": "^2.1.0", "add-mcp": "^2.4.0", "ps-list": "^9.0.0" }, diff --git a/skills/kody/SKILL.md b/skills/kody/SKILL.md index d4f8224..5b942b0 100644 --- a/skills/kody/SKILL.md +++ b/skills/kody/SKILL.md @@ -60,8 +60,10 @@ kody logout The CLI opens a browser for Kody OAuth (PKCE + Client ID Metadata Documents). If a browser cannot open, it prints the URL. Tokens (access + refresh) are -stored in the OS keychain on macOS, Windows, and Linux. Linux without Secret -Service falls back to a `0600` file under `$XDG_CONFIG_HOME/kody`. +stored in the OS keychain on macOS, Windows, and Linux (Secret Service). +Linux without Secret Service — including headless machines that only have an +in-memory kernel keyring — falls back to a `0600` file under +`$XDG_CONFIG_HOME/kody`. Never ask the user to paste tokens into chat. diff --git a/src/store.ts b/src/store.ts index 82d864b..dd390b5 100644 --- a/src/store.ts +++ b/src/store.ts @@ -32,6 +32,16 @@ export type SecretBackend = { delete(): boolean } +/** Linux-only: require Secret Service. Ignored on macOS and Windows. */ +export const secretServiceKeyringOptions = { + linux: { store: 'secret-service' }, +} as const + +export type StoreResolution = { + createKeyring?: (mcpUrl: string) => SecretBackend + fileStorePath?: (mcpUrl: string) => string +} + export function accountForMcpUrl(mcpUrl: string): string { return `cli:${new URL(mcpUrl).origin}` } @@ -82,7 +92,11 @@ export function createFileBackend(path: string): SecretBackend { } export function createKeyringBackend(mcpUrl: string): SecretBackend { - const entry = new Entry(keyringService, accountForMcpUrl(mcpUrl)) + const entry = new Entry( + keyringService, + accountForMcpUrl(mcpUrl), + secretServiceKeyringOptions, + ) return { kind: 'keyring', get() { @@ -105,15 +119,24 @@ export function createKeyringBackend(mcpUrl: string): SecretBackend { } } +function fileBackendFor( + mcpUrl: string, + resolution?: StoreResolution, +): SecretBackend { + const path = resolution?.fileStorePath?.(mcpUrl) ?? fileStorePath(mcpUrl) + return createFileBackend(path) +} + export function resolveBackend( mcpUrl: string, preferred?: SecretBackend, + resolution?: StoreResolution, ): SecretBackend { if (preferred) return preferred try { - return createKeyringBackend(mcpUrl) + return (resolution?.createKeyring ?? createKeyringBackend)(mcpUrl) } catch { - return createFileBackend(fileStorePath(mcpUrl)) + return fileBackendFor(mcpUrl, resolution) } } @@ -125,36 +148,42 @@ export function parseCredentials(raw: string): StoredCredentials { return parsed } +function readParsed(store: SecretBackend): StoredCredentials | null { + const raw = store.get() + if (!raw) return null + return parseCredentials(raw) +} + export function loadCredentials( mcpUrl: string = defaultMcpUrl, backend?: SecretBackend, + resolution?: StoreResolution, ): StoredCredentials | null { - const store = resolveBackend(mcpUrl, backend) + const store = resolveBackend(mcpUrl, backend, resolution) try { - const raw = store.get() - if (!raw) return null - return parseCredentials(raw) + const loaded = readParsed(store) + if (loaded) return loaded } catch (error) { - if (store.kind === 'keyring' && !backend) { - const fallback = createFileBackend(fileStorePath(mcpUrl)) - const raw = fallback.get() - return raw ? parseCredentials(raw) : null - } - throw error + if (!(store.kind === 'keyring' && !backend)) throw error + } + if (store.kind === 'keyring' && !backend) { + return readParsed(fileBackendFor(mcpUrl, resolution)) } + return null } export function saveCredentials( credentials: StoredCredentials, backend?: SecretBackend, + resolution?: StoreResolution, ): { backend: SecretBackend } { - const store = resolveBackend(credentials.mcpUrl, backend) + const store = resolveBackend(credentials.mcpUrl, backend, resolution) try { store.set(JSON.stringify(credentials)) return { backend: store } } catch (error) { if (store.kind === 'keyring' && !backend) { - const fallback = createFileBackend(fileStorePath(credentials.mcpUrl)) + const fallback = fileBackendFor(credentials.mcpUrl, resolution) fallback.set(JSON.stringify(credentials)) return { backend: fallback } } @@ -165,8 +194,9 @@ export function saveCredentials( export function deleteCredentials( mcpUrl: string = defaultMcpUrl, backend?: SecretBackend, + resolution?: StoreResolution, ): { deleted: boolean; backend: SecretBackend } { - const store = resolveBackend(mcpUrl, backend) + const store = resolveBackend(mcpUrl, backend, resolution) let deleted = false try { deleted = store.delete() @@ -174,7 +204,7 @@ export function deleteCredentials( deleted = false } if (store.kind === 'keyring' && !backend) { - const file = createFileBackend(fileStorePath(mcpUrl)) + const file = fileBackendFor(mcpUrl, resolution) deleted = file.delete() || deleted } return { deleted, backend: store } diff --git a/test/store.test.ts b/test/store.test.ts index 7430729..f1e1621 100644 --- a/test/store.test.ts +++ b/test/store.test.ts @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { chmodSync, mkdtempSync, statSync } from 'node:fs' +import { chmodSync, existsSync, mkdtempSync, readFileSync, statSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { test } from 'node:test' @@ -11,6 +11,9 @@ import { saveCredentials, loadCredentials, deleteCredentials, + resolveBackend, + secretServiceKeyringOptions, + type SecretBackend, type StoredCredentials, } from '../src/store.js' @@ -27,6 +30,31 @@ const sample: StoredCredentials = { scope: 'profile email', } +function memoryKeyring(initial: string | null = null): SecretBackend & { + value: string | null +} { + const store: SecretBackend & { value: string | null } = { + kind: 'keyring', + value: initial, + get() { + return store.value + }, + set(value: string) { + store.value = value + }, + delete() { + const had = store.value !== null + store.value = null + return had + }, + } + return store +} + +function tempFilePath(): string { + return join(mkdtempSync(join(tmpdir(), 'kody-cli-')), 'credentials.json') +} + test('accountForMcpUrl is origin-scoped', () => { assert.equal(accountForMcpUrl('https://kody.codes/mcp'), 'cli:https://kody.codes') assert.equal( @@ -40,6 +68,10 @@ test('fileStorePath uses XDG on linux-like homes', () => { assert.match(path, /credentials-kody\.codes\.json$/) }) +test('Linux keyring is pinned to Secret Service so keyutils cannot silently win', () => { + assert.equal(secretServiceKeyringOptions.linux.store, 'secret-service') +}) + test('file backend stores, loads, and deletes credentials', () => { const dir = mkdtempSync(join(tmpdir(), 'kody-cli-')) const backend = createFileBackend(join(dir, 'credentials.json')) @@ -58,3 +90,118 @@ test('parseCredentials rejects invalid payloads', () => { assert.throws(() => parseCredentials('{}'), /invalid/i) assert.throws(() => parseCredentials('{"version":2}'), /invalid/i) }) + +test('resolveBackend uses the file store when the keyring constructor throws', () => { + const path = tempFilePath() + const backend = resolveBackend(sample.mcpUrl, undefined, { + createKeyring() { + throw new Error('Secret Service is unavailable') + }, + fileStorePath: () => path, + }) + assert.equal(backend.kind, 'file') + assert.equal(backend.path, path) +}) + +test('saveCredentials writes the file when Secret Service is unavailable', () => { + const path = tempFilePath() + const saved = saveCredentials(sample, undefined, { + createKeyring() { + throw new Error('Secret Service is unavailable') + }, + fileStorePath: () => path, + }) + assert.equal(saved.backend.kind, 'file') + assert.equal(saved.backend.path, path) + assert.equal(existsSync(path), true) + if (process.platform !== 'win32') { + assert.equal(statSync(path).mode & 0o777, 0o600) + } + assert.deepEqual(loadCredentials(sample.mcpUrl, saved.backend), sample) +}) + +test('saveCredentials falls back to the file when keyring set throws', () => { + const path = tempFilePath() + const keyring: SecretBackend = { + kind: 'keyring', + get: () => null, + set() { + throw new Error('setPassword failed') + }, + delete: () => false, + } + const saved = saveCredentials(sample, undefined, { + createKeyring: () => keyring, + fileStorePath: () => path, + }) + assert.equal(saved.backend.kind, 'file') + assert.equal(JSON.parse(readFileSync(path, 'utf8')).accessToken, 'access-1') +}) + +test('loadCredentials falls back to the file when keyring credentials are invalid', () => { + const path = tempFilePath() + createFileBackend(path).set(JSON.stringify(sample)) + const loaded = loadCredentials(sample.mcpUrl, undefined, { + createKeyring: () => memoryKeyring('{"version":1}'), + fileStorePath: () => path, + }) + assert.deepEqual(loaded, sample) +}) + +test('loadCredentials surfaces invalid file credentials after a keyring miss', () => { + const path = tempFilePath() + createFileBackend(path).set('{"version":1}') + assert.throws( + () => + loadCredentials(sample.mcpUrl, undefined, { + createKeyring: () => memoryKeyring(null), + fileStorePath: () => path, + }), + /invalid/i, + ) +}) + +test('loadCredentials falls back to the file when the keyring returns null', () => { + const path = tempFilePath() + createFileBackend(path).set(JSON.stringify(sample)) + const loaded = loadCredentials(sample.mcpUrl, undefined, { + createKeyring: () => memoryKeyring(null), + fileStorePath: () => path, + }) + assert.deepEqual(loaded, sample) +}) + +test('loadCredentials prefers keyring credentials over a leftover file', () => { + const path = tempFilePath() + createFileBackend(path).set( + JSON.stringify({ ...sample, accessToken: 'file-access' }), + ) + const loaded = loadCredentials(sample.mcpUrl, undefined, { + createKeyring: () => + memoryKeyring(JSON.stringify({ ...sample, accessToken: 'keyring-access' })), + fileStorePath: () => path, + }) + assert.equal(loaded?.accessToken, 'keyring-access') +}) + +test('loadCredentials does not use the file when a preferred backend returns null', () => { + const path = tempFilePath() + createFileBackend(path).set(JSON.stringify(sample)) + const loaded = loadCredentials(sample.mcpUrl, memoryKeyring(null), { + fileStorePath: () => path, + }) + assert.equal(loaded, null) +}) + +test('deleteCredentials removes both keyring and file copies', () => { + const path = tempFilePath() + const keyring = memoryKeyring(JSON.stringify(sample)) + createFileBackend(path).set(JSON.stringify(sample)) + const result = deleteCredentials(sample.mcpUrl, undefined, { + createKeyring: () => keyring, + fileStorePath: () => path, + }) + assert.equal(result.deleted, true) + assert.equal(keyring.value, null) + assert.equal(existsSync(path), false) +})