diff --git a/README.md b/README.md index de0d83f..1e49760 100644 --- a/README.md +++ b/README.md @@ -12,20 +12,20 @@ npm install @doist/cli-core ## What's in it -| Module | Key exports | Purpose | -| -------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `auth` (subpath) | `attachLoginCommand`, `attachLogoutCommand`, `attachStatusCommand`, `attachTokenViewCommand`, `attachRefreshTokenViewCommand`, `attachAccountListCommand`, `attachAccountUseCommand`, `attachAccountCurrentCommand`, `attachAccountRemoveCommand`, `runOAuthFlow`, `refreshAccessToken`, `createPkceProvider`, `createDcrProvider`, `createSecureStore`, `createKeyringTokenStore`, `migrateLegacyAuth`, `persistBundle`, `bundleFromExchange`, PKCE helpers, `AuthProvider` / `TokenStore` / `TokenBundle` / `ActiveBundleSnapshot` / `RefreshInput` / `AccountRef` / `ClearedAccount` / `SecureStore` / `UserRecordStore` types, `AttachLogoutRevokeContext` / `AttachAccountListContext` / `AttachAccountCurrentContext` / `AttachAccountRemoveContext` | OAuth runtime plus the Commander attachers for ` [auth] login` / `logout` / `status` / `token` / `refresh-token view` and ` account list` / `use` / `current` / `remove`. `attachLogoutCommand` accepts an optional `revokeToken` hook for best-effort server-side token revocation. Ships the standard public-client PKCE flow (`createPkceProvider`), the RFC 7591 Dynamic Client Registration flow (`createDcrProvider`, with optional RFC 8707 resource indicators, refresh-token support, and `loadClient`/`saveClient` client caching), a thin cross-platform OS-keyring wrapper (`createSecureStore`), and a multi-account keyring-backed `TokenStore` (`createKeyringTokenStore`) that stores secrets in the OS credential manager and degrades to plaintext in the consumer's config when the keyring is unavailable (WSL/headless Linux/containers). The store contract supports an optional `setBundle(account, bundle)` write method (required on `KeyringTokenStore`) so consumers that need refresh-token persistence can opt in via `TokenBundle`; `active()` stays narrow (access token + account only) so callers that don't need refresh state don't pay extra keyring IPC. `AuthProvider` and `TokenStore` remain the escape hatches for fully bespoke backends (device code, magic-link, …). `logout` / `status` / `token` / `refresh-token view` always attach `--user ` and thread the parsed ref to the matching store read (`store.active(ref)`, `store.activeBundle(ref)`, or `store.clear(ref)`). `commander` (when using the attachers), `open` (browser launch), `@napi-rs/keyring` (when using `createSecureStore` or the keyring `TokenStore`), and `oauth4webapi` (when a consumer opts into silent refresh or uses `createDcrProvider`) are optional peer/optional deps. | -| `commands` (subpath) | `registerChangelogCommand`, `registerUpdateCommand` (+ semver helpers) | Commander wiring for cli-core's standard commands (e.g. ` changelog`, ` update`, ` update switch`). **Requires** `commander` as an optional peer-dep. | -| `config` | `getConfigPath`, `readConfig`, `readConfigStrict`, `writeConfig`, `updateConfig`, `CoreConfig`, `UpdateChannel` | Read / write a per-CLI JSON config file with typed error codes; `CoreConfig` is the shape of fields cli-core itself owns (extend it for per-CLI fields). | -| `empty` | `printEmpty` | Print an empty-state message gated on `--json` / `--ndjson` so machine consumers never see human strings on stdout. | -| `errors` | `CliError` | Typed CLI error class with `code` and exit-code mapping. | -| `global-args` | `parseGlobalArgs`, `stripUserFlag`, `createGlobalArgsStore`, `createAccessibleGate`, `createSpinnerGate`, `getProgressJsonlPath`, `isProgressJsonlEnabled` | Parse well-known global flags (`--json`, `--ndjson`, `--quiet`, `--verbose`, `--accessible`, `--no-spinner`, `--progress-jsonl`, `--user `) and derive predicates from them. `stripUserFlag` removes `--user` tokens from argv so the cleaned array can be forwarded to Commander when the flag has no root-program attachment. | -| `json` | `formatJson`, `formatNdjson` | Stable JSON / newline-delimited JSON formatting for stdout. | -| `markdown` (subpath) | `preloadMarkdown`, `renderMarkdown`, `TerminalRendererOptions` | Lazy-init terminal markdown renderer. **Requires** `marked` and `marked-terminal-renderer` as peer-deps — install only if your CLI uses this subpath. | -| `options` | `ViewOptions` | Type contract for `{ json?, ndjson? }` per-command options that machine-output gates derive from. | -| `spinner` | `createSpinner` | Loading spinner factory wrapping `yocto-spinner` with disable gates. | -| `terminal` | `isCI`, `isStderrTTY`, `isStdinTTY`, `isStdoutTTY` | TTY / CI detection helpers. | -| `testing` (subpath) | `describeEmptyMachineOutput`, `createTestProgram`, `captureConsole`, `captureStream`, `buildTokenStore`, `buildSingleEntryStore`, `ingenEntries`, `alanGrant` / `ellieSattler` / `ianMalcolm`, `TestAccount` / `StoreEntry` / `TokenStoreHarness` / `MatchAccount` types | Vitest helpers + fixtures reusable by consuming CLIs: a parametrised empty-state suite (`--json` / `--ndjson` / human modes); a Commander test-program builder (`createTestProgram`; the whole subpath **requires** `commander` since the barrel re-exports it); console / stdout-stderr spies that silence + auto-restore (`captureConsole` / `captureStream`, call inside a test or `beforeEach`); and a canonical stateful in-memory `TokenStore` mock plus shared account fixtures (`buildTokenStore` / `buildSingleEntryStore`) modelling `createKeyringTokenStore`'s default-selection contract — pass `matchAccount` to mirror a consumer's own ref-matching (numeric-id / case-insensitive label). | +| Module | Key exports | Purpose | +| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `auth` (subpath) | `attachLoginCommand`, `attachLogoutCommand`, `attachStatusCommand`, `attachTokenViewCommand`, `attachRefreshTokenViewCommand`, `attachAccountListCommand`, `attachAccountUseCommand`, `attachAccountCurrentCommand`, `attachAccountRemoveCommand`, `runOAuthFlow`, `refreshAccessToken`, `createPkceProvider`, `createDcrProvider`, `createSecureStore`, `createKeyringTokenStore`, `migrateLegacyAuth`, `persistBundle`, `bundleFromExchange`, PKCE helpers, `AuthProvider` / `TokenStore` / `TokenBundle` / `ActiveBundleSnapshot` / `RefreshInput` / `AccountRef` / `ClearedAccount` / `SecureStore` / `UserRecordStore` / `CredentialStore` types, `AttachLogoutRevokeContext` / `AttachAccountListContext` / `AttachAccountCurrentContext` / `AttachAccountRemoveContext` | OAuth runtime plus the Commander attachers for ` [auth] login` / `logout` / `status` / `token` / `refresh-token view` and ` account list` / `use` / `current` / `remove`. `attachLogoutCommand` accepts an optional `revokeToken` hook for best-effort server-side token revocation. Ships the standard public-client PKCE flow (`createPkceProvider`), the RFC 7591 Dynamic Client Registration flow (`createDcrProvider`, with optional RFC 8707 resource indicators, refresh-token support, and `loadClient`/`saveClient` client caching), a thin cross-platform OS-keyring wrapper (`createSecureStore`), and a multi-account keyring-backed `TokenStore` (`createKeyringTokenStore`) with strict system, explicit plaintext, and fallback storage policies. The store contract supports an optional `setBundle(account, bundle)` write method (required on `KeyringTokenStore`) so consumers that need refresh-token persistence can opt in via `TokenBundle`; `active()` stays narrow (access token + account only) so callers that don't need refresh state don't pay extra keyring IPC. `AuthProvider` and `TokenStore` remain the escape hatches for fully bespoke backends (device code, magic-link, …). `logout` / `status` / `token` / `refresh-token view` always attach `--user ` and thread the parsed ref to the matching store read (`store.active(ref)`, `store.activeBundle(ref)`, or `store.clear(ref)`). `commander` (when using the attachers), `open` (browser launch), `@napi-rs/keyring` (when using `createSecureStore` or the keyring `TokenStore`), and `oauth4webapi` (when a consumer opts into silent refresh or uses `createDcrProvider`) are optional peer/optional deps. | +| `commands` (subpath) | `registerChangelogCommand`, `registerUpdateCommand` (+ semver helpers) | Commander wiring for cli-core's standard commands (e.g. ` changelog`, ` update`, ` update switch`). **Requires** `commander` as an optional peer-dep. | +| `config` | `getConfigPath`, `readConfig`, `readConfigStrict`, `writeConfig`, `updateConfig`, `CoreConfig`, `UpdateChannel` | Read / write a per-CLI JSON config file with typed error codes; `CoreConfig` is the shape of fields cli-core itself owns (extend it for per-CLI fields). | +| `empty` | `printEmpty` | Print an empty-state message gated on `--json` / `--ndjson` so machine consumers never see human strings on stdout. | +| `errors` | `CliError` | Typed CLI error class with `code` and exit-code mapping. | +| `global-args` | `parseGlobalArgs`, `stripUserFlag`, `createGlobalArgsStore`, `createAccessibleGate`, `createSpinnerGate`, `getProgressJsonlPath`, `isProgressJsonlEnabled` | Parse well-known global flags (`--json`, `--ndjson`, `--quiet`, `--verbose`, `--accessible`, `--no-spinner`, `--progress-jsonl`, `--user `) and derive predicates from them. `stripUserFlag` removes `--user` tokens from argv so the cleaned array can be forwarded to Commander when the flag has no root-program attachment. | +| `json` | `formatJson`, `formatNdjson` | Stable JSON / newline-delimited JSON formatting for stdout. | +| `markdown` (subpath) | `preloadMarkdown`, `renderMarkdown`, `TerminalRendererOptions` | Lazy-init terminal markdown renderer. **Requires** `marked` and `marked-terminal-renderer` as peer-deps — install only if your CLI uses this subpath. | +| `options` | `ViewOptions` | Type contract for `{ json?, ndjson? }` per-command options that machine-output gates derive from. | +| `spinner` | `createSpinner` | Loading spinner factory wrapping `yocto-spinner` with disable gates. | +| `terminal` | `isCI`, `isStderrTTY`, `isStdinTTY`, `isStdoutTTY` | TTY / CI detection helpers. | +| `testing` (subpath) | `describeEmptyMachineOutput`, `createTestProgram`, `captureConsole`, `captureStream`, `buildTokenStore`, `buildSingleEntryStore`, `ingenEntries`, `alanGrant` / `ellieSattler` / `ianMalcolm`, `TestAccount` / `StoreEntry` / `TokenStoreHarness` / `MatchAccount` types | Vitest helpers + fixtures reusable by consuming CLIs: a parametrised empty-state suite (`--json` / `--ndjson` / human modes); a Commander test-program builder (`createTestProgram`; the whole subpath **requires** `commander` since the barrel re-exports it); console / stdout-stderr spies that silence + auto-restore (`captureConsole` / `captureStream`, call inside a test or `beforeEach`); and a canonical stateful in-memory `TokenStore` mock plus shared account fixtures (`buildTokenStore` / `buildSingleEntryStore`) modelling `createKeyringTokenStore`'s default-selection contract — pass `matchAccount` to mirror a consumer's own ref-matching (numeric-id / case-insensitive label). | ## Usage @@ -483,10 +483,14 @@ Every failure mode — `@napi-rs/keyring` failing to load on an arch without a p #### Multi-account keyring-backed `TokenStore` -`createKeyringTokenStore` wires `createSecureStore` into the `TokenStore` contract for multi-account CLIs. Secrets live in the OS credential manager; per-user metadata stays in the consumer's config via a small `UserRecordStore` port the consumer implements. When the keyring is unreachable the store transparently falls back to a `fallbackToken` field on the user record and exposes a warning on `getLastStorageResult()` for the login command to surface. +`createKeyringTokenStore` wires `createSecureStore` into the `TokenStore` contract for multi-account CLIs. Secrets live in the OS credential manager; per-user metadata stays in the consumer's config via a small `UserRecordStore` port the consumer implements. Its default `fallback` policy preserves the historic behavior: when the keyring is unreachable it writes a `fallbackToken` field on the user record and exposes a warning on `getLastStorageResult()`. ```ts -import { createKeyringTokenStore, type UserRecordStore } from '@doist/cli-core/auth' +import { + createKeyringTokenStore, + type CredentialStore, + type UserRecordStore, +} from '@doist/cli-core/auth' type Account = { id: string; label?: string; email: string } @@ -515,6 +519,7 @@ const userRecords: UserRecordStore = { export const tokenStore = createKeyringTokenStore({ serviceName: 'todoist-cli', userRecords, + recordsLocation: '~/.config/todoist-cli/config.json', }) // In your login command's onSuccess: @@ -522,6 +527,25 @@ const storage = tokenStore.getLastStorageResult() if (storage?.warning) console.error('Warning:', storage.warning) ``` +Set `credentialStore` to control credential writes: + +- `'fallback'` (the default) tries the system credential manager and falls back to plaintext only when it is unavailable. +- `'system'` is strict: an unavailable credential manager fails the write and never writes a plaintext credential. +- `'plaintext'` deliberately writes to the user record without calling the credential manager. Every successful write returns a plaintext warning through `getLastStorageResult()`. + +The option may be a value or a resolver, which lets a consuming CLI select the policy from a command option parsed after the store is constructed: + +```ts +let credentialStore: CredentialStore = 'system' + +const tokenStore = createKeyringTokenStore({ + serviceName: 'todoist-cli', + userRecords, + recordsLocation: '~/.config/todoist-cli/config.json', + credentialStore: () => credentialStore, +}) +``` + The returned store satisfies the full `TokenStore` contract — including `list()` / `setDefault(ref)` / `ref`-aware `active` / `clear` — so it plugs straight into the `logout` / `status` / `token` attachers. Default ref matching is `account.id === ref || account.label === ref`; override `matchAccount` to broaden it (e.g. case-insensitive email). When a matching record exists but the keyring read fails, `active(ref)` throws `CliError('AUTH_STORE_READ_FAILED', …)`. `attachLogoutCommand` catches it specifically so `logout --user ` still clears the local record even with the keyring offline; status / token-view propagate it because they can't render without the token. diff --git a/src/auth/index.ts b/src/auth/index.ts index f83650d..e6b53ac 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -83,6 +83,7 @@ export type { SecureStore, TokenStorageLocation, TokenStorageResult, + CredentialStore, UserRecord, UserRecordStore, } from './keyring/index.js' diff --git a/src/auth/keyring/index.ts b/src/auth/keyring/index.ts index ac2f131..35250e1 100644 --- a/src/auth/keyring/index.ts +++ b/src/auth/keyring/index.ts @@ -10,6 +10,7 @@ export type { MigrateAuthResult, MigrateLegacyAuthOptions, MigrateSkipReason } f export type { TokenStorageLocation, TokenStorageResult, + CredentialStore, UserRecord, UserRecordStore, } from './types.js' diff --git a/src/auth/keyring/record-write.test.ts b/src/auth/keyring/record-write.test.ts index 476f73c..2f8e223 100644 --- a/src/auth/keyring/record-write.test.ts +++ b/src/auth/keyring/record-write.test.ts @@ -43,6 +43,42 @@ describe('writeRecordWithKeyringFallback', () => { expect(state.records.get('42')?.fallbackToken).toBe('tok_plain') }) + it('does not write a fallbackToken in strict system mode', async () => { + const secureStore = buildSingleSlot() + secureStore.setSpy.mockRejectedValueOnce(new SecureStoreUnavailableError('no dbus')) + const { store: userRecords, state } = buildUserRecords() + + await expect( + writeRecordWithKeyringFallback({ + secureStore, + userRecords, + account, + token: 'tok_plain', + credentialStore: 'system', + }), + ).rejects.toMatchObject({ code: 'AUTH_STORE_WRITE_FAILED' }) + + expect(state.records.size).toBe(0) + }) + + it('writes directly to fallbackToken in explicit plaintext mode', async () => { + const secureStore = buildSingleSlot() + const { store: userRecords, state } = buildUserRecords() + + const result = await writeRecordWithKeyringFallback({ + secureStore, + userRecords, + account, + token: 'tok_plain', + credentialStore: 'plaintext', + }) + + expect(result.storedSecurely).toBe(false) + expect(secureStore.setSpy).not.toHaveBeenCalled() + expect(secureStore.deleteSpy).not.toHaveBeenCalled() + expect(state.records.get('42')?.fallbackToken).toBe('tok_plain') + }) + it('rethrows non-keyring errors from setSecret without writing the record', async () => { const secureStore = buildSingleSlot() const cause = new Error('unexpected backend explosion') @@ -189,6 +225,25 @@ describe('writeBundleWithKeyringFallback', () => { expect(state.records.size).toBe(0) }) + it('rolls back the access slot when strict system storage cannot write the refresh token', async () => { + const { accessStore, refreshStore, store: userRecords, state } = harness() + refreshStore.setSpy.mockRejectedValueOnce(new SecureStoreUnavailableError('no dbus')) + + await expect( + writeBundleWithKeyringFallback({ + accessStore, + refreshStore, + userRecords, + account, + bundle, + credentialStore: 'system', + }), + ).rejects.toMatchObject({ code: 'AUTH_STORE_WRITE_FAILED' }) + + expect(accessStore.deleteSpy).toHaveBeenCalledTimes(1) + expect(state.records.size).toBe(0) + }) + it('rolls back BOTH keyring slots when upsert fails after both writes succeeded', async () => { const { accessStore, refreshStore, store: userRecords, upsertSpy } = harness() upsertSpy.mockRejectedValueOnce(new Error('disk full')) diff --git a/src/auth/keyring/record-write.ts b/src/auth/keyring/record-write.ts index 4c82d39..9adbd5f 100644 --- a/src/auth/keyring/record-write.ts +++ b/src/auth/keyring/record-write.ts @@ -1,7 +1,7 @@ import { CliError } from '../../errors.js' import type { AuthAccount, TokenBundle } from '../types.js' import { type SecureStore, SecureStoreUnavailableError } from './secure-store.js' -import type { UserRecord, UserRecordStore } from './types.js' +import type { CredentialStore, UserRecord, UserRecordStore } from './types.js' type WriteRecordOptions = { /** Per-account keyring slot, already configured by the caller (e.g. via `createSecureStore`). */ @@ -16,10 +16,11 @@ type WriteRecordOptions = { userRecords: UserRecordStore account: TAccount token: string + credentialStore?: CredentialStore } type WriteRecordResult = { - /** `true` when the secret landed in the OS keyring; `false` when the keyring was unavailable and the token was written to `fallbackToken` on the user record. */ + /** `true` when the secret landed in the OS keyring; `false` when it was written to `fallbackToken` on the user record. */ storedSecurely: boolean } @@ -31,14 +32,15 @@ type WriteBundleOptions = { userRecords: UserRecordStore account: TAccount bundle: TokenBundle + credentialStore?: CredentialStore } type WriteBundleResult = { - /** `true` when the access token landed in the OS keyring; `false` when it fell back to `fallbackToken`. */ + /** `true` when the access token landed in the OS keyring; `false` when it was written to `fallbackToken`. */ accessStoredSecurely: boolean /** * `true` when a refresh token landed in the OS keyring. `false` when it - * fell back to `fallbackRefreshToken`. `undefined` when the bundle + * was written to `fallbackRefreshToken`. `undefined` when the bundle * carried no refresh token (nothing to store). */ refreshStoredSecurely: boolean | undefined @@ -46,7 +48,7 @@ type WriteBundleResult = { /** * Single-token write. Thin wrapper over `writeBundleWithKeyringFallback` - * passing a refresh-less bundle, so trim/validate, access-slot fallback, + * passing a refresh-less bundle, so trim/validate, access-slot storage, * upsert rollback, and the deferred refresh-slot wipe all share one * implementation. * @@ -57,7 +59,7 @@ type WriteBundleResult = { export async function writeRecordWithKeyringFallback( options: WriteRecordOptions, ): Promise { - const { secureStore, refreshStore, userRecords, account, token } = options + const { secureStore, refreshStore, userRecords, account, token, credentialStore } = options const { accessStoredSecurely } = await writeBundleWithKeyringFallback({ accessStore: secureStore, @@ -68,6 +70,7 @@ export async function writeRecordWithKeyringFallback, ): Promise { const { accessStore, refreshStore, userRecords, account, bundle } = options + const credentialStore = options.credentialStore ?? 'fallback' const accessToken = bundle.accessToken.trim() if (!accessToken) { throw new CliError( @@ -109,20 +116,23 @@ export async function writeBundleWithKeyringFallback = { @@ -168,7 +183,7 @@ export async function writeBundleWithKeyringFallback { expect(keyring.getSpy).not.toHaveBeenCalled() }) + it('rejects an unavailable keyring in strict system mode without writing plaintext', async () => { + const keyring = buildSingleSlot() + keyring.setSpy.mockRejectedValueOnce(new SecureStoreUnavailableError('no dbus')) + const { store, state } = fixture({ + keyring, + factoryOpts: { credentialStore: 'system' }, + }) + + await expect(store.set(account, 'tok_secret')).rejects.toMatchObject({ + code: 'AUTH_STORE_WRITE_FAILED', + message: 'The system credential manager could not store the credential.', + }) + + expect(state.records.size).toBe(0) + expect(store.getLastStorageResult()).toBeUndefined() + }) + + it('uses an explicit plaintext resolver without calling the keyring', async () => { + const keyring = buildSingleSlot() + let credentialStore: 'system' | 'plaintext' = 'system' + const { store, state } = fixture({ + keyring, + factoryOpts: { credentialStore: () => credentialStore }, + }) + credentialStore = 'plaintext' + + await store.setBundle( + account, + { + accessToken: 'tok_a', + refreshToken: 'tok_r', + accessTokenExpiresAt: 1_700_000_000_000, + refreshTokenExpiresAt: 1_701_000_000_000, + }, + { promoteDefault: true }, + ) + + expect(keyring.setSpy).not.toHaveBeenCalled() + expect(keyring.deleteSpy).not.toHaveBeenCalled() + expect(state.records.get('42')).toEqual({ + account, + fallbackToken: 'tok_a', + fallbackRefreshToken: 'tok_r', + accessTokenExpiresAt: 1_700_000_000_000, + refreshTokenExpiresAt: 1_701_000_000_000, + hasRefreshToken: true, + }) + expect(store.getLastStorageResult()).toEqual({ + storage: 'config-file', + warning: 'credential stored as plaintext in /tmp/fake/config.json', + }) + }) + it('set() still succeeds when the best-effort default promotion fails', async () => { const { store, state, setDefaultSpy } = fixture() setDefaultSpy.mockRejectedValueOnce(new Error('default-write blew up')) diff --git a/src/auth/keyring/token-store.ts b/src/auth/keyring/token-store.ts index 600207c..5cc2cac 100644 --- a/src/auth/keyring/token-store.ts +++ b/src/auth/keyring/token-store.ts @@ -20,7 +20,7 @@ import { type SecureStore, } from './secure-store.js' import { refreshAccountSlot } from './slot-naming.js' -import type { TokenStorageResult, UserRecord, UserRecordStore } from './types.js' +import type { CredentialStore, TokenStorageResult, UserRecord, UserRecordStore } from './types.js' export type CreateKeyringTokenStoreOptions = { /** Application identifier used for every keyring entry (e.g. `'todoist-cli'`). */ @@ -44,6 +44,12 @@ export type CreateKeyringTokenStoreOptions = { * (e.g. case-insensitive email, alias map). */ matchAccount?: (account: TAccount, ref: AccountRef) => boolean + /** + * Credential-write policy. Defaults to `'fallback'` for backwards + * compatibility. A resolver is useful when a consumer's CLI option is + * parsed after the store is constructed. + */ + credentialStore?: CredentialStore | (() => CredentialStore) } export type KeyringTokenStore = TokenStore & { @@ -92,10 +98,11 @@ function accessReadError(outcome: Extract /** * Multi-account `TokenStore` that keeps secrets in the OS credential manager - * and per-user metadata in the consumer's `UserRecordStore`. Falls back to a - * plaintext token on the user record when the keyring is unreachable (WSL - * without D-Bus, missing native binary, locked Keychain, …) so the CLI keeps - * working at the cost of a visible warning. + * and per-user metadata in the consumer's `UserRecordStore`. Its default + * `'fallback'` policy writes plaintext to the user record when the keyring is + * unreachable (WSL without D-Bus, missing native binary, locked Keychain, + * …), with a visible warning. Consumers can instead select strict `'system'` + * storage or explicit `'plaintext'` storage. * * Read order in `active()` is `fallbackToken` first, then the keyring. That * matches the write semantics in `writeRecordWithKeyringFallback`: when the @@ -122,6 +129,11 @@ export function createKeyringTokenStore( const accountForUser = options.accountForUser ?? DEFAULT_ACCOUNT_FOR_USER const matchAccount = options.matchAccount ?? DEFAULT_MATCH_ACCOUNT + function resolveCredentialStore(): CredentialStore { + const configured = options.credentialStore + return typeof configured === 'function' ? configured() : (configured ?? 'fallback') + } + let lastStorageResult: TokenStorageResult | undefined let lastClearResult: TokenStorageResult | undefined @@ -223,7 +235,14 @@ export function createKeyringTokenStore( function bundleStorageResult( accessStored: boolean, refreshStored: boolean | undefined, + credentialStore: CredentialStore, ): TokenStorageResult { + if (credentialStore === 'plaintext') { + return { + storage: 'config-file', + warning: `credential stored as plaintext in ${recordsLocation}`, + } + } const accessFallback = !accessStored const refreshFallback = refreshStored === false if (!accessFallback && !refreshFallback) return { storage: 'secure-store' } @@ -325,22 +344,25 @@ export function createKeyringTokenStore( // warning leaking through `getLastStorageResult`. lastStorageResult = undefined + const credentialStore = resolveCredentialStore() const { storedSecurely } = await writeRecordWithKeyringFallback({ secureStore: secureStoreFor(account), refreshStore: refreshSecureStoreFor(account), userRecords, account, token, + credentialStore, }) await promoteDefaultIfNeeded(account.id) - lastStorageResult = bundleStorageResult(storedSecurely, undefined) + lastStorageResult = bundleStorageResult(storedSecurely, undefined, credentialStore) }, async setBundle(account, bundle, options) { lastStorageResult = undefined + const credentialStore = resolveCredentialStore() const { accessStoredSecurely, refreshStoredSecurely } = await writeBundleWithKeyringFallback({ accessStore: secureStoreFor(account), @@ -348,6 +370,7 @@ export function createKeyringTokenStore( userRecords, account, bundle, + credentialStore, }) // Opt-in: silent refresh omits `promoteDefault` so it can't @@ -356,7 +379,11 @@ export function createKeyringTokenStore( await promoteDefaultIfNeeded(account.id) } - lastStorageResult = bundleStorageResult(accessStoredSecurely, refreshStoredSecurely) + lastStorageResult = bundleStorageResult( + accessStoredSecurely, + refreshStoredSecurely, + credentialStore, + ) }, async clear(ref) { diff --git a/src/auth/keyring/types.ts b/src/auth/keyring/types.ts index 53ce80a..b87c5cf 100644 --- a/src/auth/keyring/types.ts +++ b/src/auth/keyring/types.ts @@ -3,12 +3,20 @@ import type { AuthAccount } from '../types.js' /** Where a token was (or wasn't) persisted on the most recent write/clear. */ export type TokenStorageLocation = 'secure-store' | 'config-file' +/** + * Policy for writing credentials through `createKeyringTokenStore`. + * + * `fallback` preserves the original behavior: prefer the system credential + * manager and use the consumer's config record only when it is unavailable. + */ +export type CredentialStore = 'system' | 'plaintext' | 'fallback' + export type TokenStorageResult = { storage: TokenStorageLocation /** - * Present when the OS keyring was unavailable and the operation fell back - * to (or left state in) the consumer's config file. Suitable for surfacing - * to the user as a `Warning:` line on stderr. + * Present when a write used (or a clear left state in) the consumer's + * config file. Suitable for surfacing to the user as a `Warning:` line on + * stderr. */ warning?: string } @@ -16,11 +24,12 @@ export type TokenStorageResult = { export type UserRecord = { account: TAccount /** - * Plaintext token, present only when the keyring was unavailable at write - * time. The runtime reads it in preference to the keyring slot, so a - * stale fallback would mask a fresh keyring-backed write — consumers + * Plaintext token, present when the keyring was unavailable under the + * `'fallback'` policy or when the consumer explicitly selected + * `'plaintext'`. The runtime reads it in preference to the keyring slot, + * so a stale fallback would mask a fresh keyring-backed write — consumers * implementing `upsert` as replace-not-merge (per the contract below) - * guarantees the field is cleared on every successful keyring write. + * guarantee the field is cleared on every successful keyring write. * Surface its presence as security-relevant: it is the same material * that would otherwise live in the OS credential manager. */