diff --git a/README.md b/README.md index 2f50dfc..6fe079a 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`, `runOAuthFlow`, `createPkceProvider`, `createSecureStore`, `createKeyringTokenStore`, PKCE helpers, `AuthProvider` / `TokenStore` / `AccountRef` / `SecureStore` / `UserRecordStore` types, `AttachLogoutRevokeContext` | OAuth runtime plus the Commander attachers for ` [auth] login` / `logout` / `status` / `token`. `attachLogoutCommand` accepts an optional `revokeToken` hook for best-effort server-side token revocation. Ships the standard public-client PKCE flow (`createPkceProvider`), 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). `AuthProvider` and `TokenStore` remain the escape hatches for DCR or fully bespoke backends. `logout` / `status` / `token` always attach `--user ` and thread the parsed ref to `store.active(ref)` (and `store.clear(ref)` on `logout`). `commander` (when using the attachers), `open` (browser launch), and `@napi-rs/keyring` (when using `createSecureStore` or the keyring `TokenStore`) 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` | Vitest helpers reusable by consuming CLIs (e.g. parametrised empty-state suite covering `--json` / `--ndjson` / human modes). | +| Module | Key exports | Purpose | +| -------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `auth` (subpath) | `attachLoginCommand`, `attachLogoutCommand`, `attachStatusCommand`, `attachTokenViewCommand`, `runOAuthFlow`, `createPkceProvider`, `createSecureStore`, `createKeyringTokenStore`, `migrateLegacyAuth`, PKCE helpers, `AuthProvider` / `TokenStore` / `AccountRef` / `SecureStore` / `UserRecordStore` types, `AttachLogoutRevokeContext` | OAuth runtime plus the Commander attachers for ` [auth] login` / `logout` / `status` / `token`. `attachLogoutCommand` accepts an optional `revokeToken` hook for best-effort server-side token revocation. Ships the standard public-client PKCE flow (`createPkceProvider`), 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). `AuthProvider` and `TokenStore` remain the escape hatches for DCR or fully bespoke backends. `logout` / `status` / `token` always attach `--user ` and thread the parsed ref to `store.active(ref)` (and `store.clear(ref)` on `logout`). `commander` (when using the attachers), `open` (browser launch), and `@napi-rs/keyring` (when using `createSecureStore` or the keyring `TokenStore`) 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` | Vitest helpers reusable by consuming CLIs (e.g. parametrised empty-state suite covering `--json` / `--ndjson` / human modes). | ## Usage @@ -362,6 +362,42 @@ When a matching record exists but the keyring read fails, `active(ref)` throws ` For sync/lazy-decrypt or fully bespoke backends, implement `TokenStore` directly. +For one-time migration of a v1 single-user token into the v2 multi-user shape, use `migrateLegacyAuth` from a postinstall hook. The helper requires a durable **migration marker** the consumer owns — a boolean persisted in their config — so the migration is genuinely one-way: a later `logout` (which empties `userRecords`) followed by a reinstall won't re-migrate a stale legacy token. + +```ts +import { getConfigPath, readConfig, updateConfig } from '@doist/cli-core' +import { migrateLegacyAuth } from '@doist/cli-core/auth' + +const configPath = getConfigPath('todoist-cli') + +const result = await migrateLegacyAuth({ + serviceName: 'todoist-cli', + legacyAccount: 'api-token', + userRecords, + // Durable one-way gate. Persist `migrated_v2: true` in your config + // after a successful migration; check it on every run. + hasMigrated: async () => + (await readConfig<{ migrated_v2?: boolean }>(configPath)).migrated_v2 === true, + markMigrated: async () => + updateConfig<{ migrated_v2: boolean }>(configPath, { migrated_v2: true }), + loadLegacyPlaintextToken: async () => + (await readConfig<{ api_token?: string }>(configPath)).api_token ?? null, + identifyAccount: async (token) => fetchUser(token), + cleanupLegacyConfig: async () => clearLegacyAuthFields(configPath), + silent: true, + logPrefix: 'todoist-cli', +}) + +if (result.status === 'skipped' && result.reason === 'legacy-keyring-unreachable') { + // Retryable — the next postinstall run with the keyring online will + // pick up where this one left off. +} +``` + +`MigrateAuthResult` is a discriminated union on `status` (`'already-migrated' | 'no-legacy-state' | 'migrated' | 'skipped'`). `migrated` carries the resolved `account`; `skipped` carries a stable `reason` (`'identify-failed' | 'legacy-keyring-unreachable' | 'user-record-write-failed' | 'marker-write-failed'`) plus a free-form `detail`. + +The helper is best-effort throughout: any failure (offline keyring, network error fetching the user, upsert blip) leaves the v1 state untouched so the consumer's runtime fallback can keep serving the legacy token until the next attempt. `markMigrated()` is called **before** the legacy keyring delete + `cleanupLegacyConfig`, so cleanup failures can't cause re-migration on the next run — the marker is the one-way gate, not cleanup success. The legacy delete and `cleanupLegacyConfig` run concurrently via `Promise.allSettled`. stderr output uses fixed phrases keyed off `MigrateSkipReason` and the success log omits the account identifier entirely so consumer-supplied error text (and PII-shaped `account.id` values like emails) can't leak into logs. + #### `--user ` and multi-user wiring The three account-touching attachers (`attachLogoutCommand` / `attachStatusCommand` / `attachTokenViewCommand`) always attach `--user ` on their subcommand. `attachLogoutCommand` threads the parsed ref to both `store.active(ref)` and `store.clear(ref)`; `attachStatusCommand` and `attachTokenViewCommand` only call `store.active(ref)`. When `--user` is supplied but `store.active(ref)` returns `null`, each attacher throws `CliError('ACCOUNT_NOT_FOUND', …)` so the user sees a typed miss rather than `NOT_AUTHENTICATED` or a silent `✓ Logged out`. Single-user stores returning `null` for a non-matching ref is the supported way to feed this guard. diff --git a/src/auth/index.ts b/src/auth/index.ts index 4ab7f9e..90858c9 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -39,11 +39,15 @@ export { SecureStoreUnavailableError, createKeyringTokenStore, createSecureStore, + migrateLegacyAuth, } from './keyring/index.js' export type { CreateKeyringTokenStoreOptions, CreateSecureStoreOptions, KeyringTokenStore, + MigrateAuthResult, + MigrateLegacyAuthOptions, + MigrateSkipReason, SecureStore, TokenStorageLocation, TokenStorageResult, diff --git a/src/auth/keyring/index.ts b/src/auth/keyring/index.ts index f4913a7..ac2f131 100644 --- a/src/auth/keyring/index.ts +++ b/src/auth/keyring/index.ts @@ -4,6 +4,9 @@ export type { CreateSecureStoreOptions, SecureStore } from './secure-store.js' export { createKeyringTokenStore } from './token-store.js' export type { CreateKeyringTokenStoreOptions, KeyringTokenStore } from './token-store.js' +export { migrateLegacyAuth } from './migrate.js' +export type { MigrateAuthResult, MigrateLegacyAuthOptions, MigrateSkipReason } from './migrate.js' + export type { TokenStorageLocation, TokenStorageResult, diff --git a/src/auth/keyring/migrate.test.ts b/src/auth/keyring/migrate.test.ts new file mode 100644 index 0000000..0204295 --- /dev/null +++ b/src/auth/keyring/migrate.test.ts @@ -0,0 +1,373 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { buildKeyringMap, buildUserRecords } from '../../test-support/keyring-mocks.js' +import { migrateLegacyAuth, type MigrateLegacyAuthOptions } from './migrate.js' +import { SecureStoreUnavailableError } from './secure-store.js' +import type { UserRecord } from './types.js' + +vi.mock('./secure-store.js', async () => { + const actual = await vi.importActual('./secure-store.js') + return { + ...actual, + createSecureStore: vi.fn(), + } +}) + +const { createSecureStore } = await import('./secure-store.js') +const mockedCreateSecureStore = vi.mocked(createSecureStore) + +type Account = { + id: string + label?: string + email: string +} + +const SERVICE = 'cli-core-test' +const LEGACY = 'api-token' + +type SlotSeed = { secret?: string | null; getErr?: unknown; setErr?: unknown; delErr?: unknown } + +type MarkerState = { migrated: boolean; markFails?: unknown } + +/** + * One-shot setup: wire `createSecureStore` to a fresh `buildKeyringMap`, + * seed any keyring slots and any pre-existing user records, expose a + * mutable marker that the helper's `hasMigrated` / `markMigrated` callbacks + * read and write, and run `migrateLegacyAuth` with sensible defaults plus + * the supplied overrides. + */ +async function runMigration( + opts: { + slots?: Record + seedRecords?: Record> + seedDefaultId?: string + marker?: MarkerState + options?: Partial> + } = {}, +) { + const km = buildKeyringMap() + for (const [slug, slot] of Object.entries(opts.slots ?? {})) { + km.slots.set(slug, { secret: null, ...slot }) + } + mockedCreateSecureStore.mockImplementation(km.create) + + const harness = buildUserRecords() + for (const [id, rec] of Object.entries(opts.seedRecords ?? {})) { + harness.state.records.set(id, rec) + } + if (opts.seedDefaultId !== undefined) harness.state.defaultId = opts.seedDefaultId + + const marker: MarkerState = opts.marker ?? { migrated: false } + const markMigrated = vi.fn(async () => { + if (marker.markFails) throw marker.markFails + marker.migrated = true + }) + + const result = await migrateLegacyAuth({ + serviceName: SERVICE, + legacyAccount: LEGACY, + userRecords: harness.store, + hasMigrated: async () => marker.migrated, + markMigrated, + loadLegacyPlaintextToken: async () => null, + identifyAccount: async () => ({ id: '1', email: 'a@b' }), + silent: true, + ...opts.options, + }) + + return { km, harness, state: harness.state, marker, markMigrated, result } +} + +describe('migrateLegacyAuth', () => { + beforeEach(() => { + mockedCreateSecureStore.mockReset() + }) + + it('returns already-migrated when the durable marker is set', async () => { + const { result, markMigrated } = await runMigration({ marker: { migrated: true } }) + + expect(result.status).toBe('already-migrated') + // Locks the short-circuit: nothing about the keyring is touched. + expect(mockedCreateSecureStore).not.toHaveBeenCalled() + expect(markMigrated).not.toHaveBeenCalled() + }) + + it('returns no-legacy-state when neither slot has a token', async () => { + const { result } = await runMigration() + expect(result.status).toBe('no-legacy-state') + }) + + it('returns skipped(legacy-keyring-unreachable) when the keyring is offline and there is no plaintext fallback', async () => { + // Token may exist in the keyring but we can't see it — collapsing + // to `no-legacy-state` would tell the caller "nothing to migrate" + // and they'd stop retrying. Surface the retryable failure instead. + const { result } = await runMigration({ + slots: { [LEGACY]: { getErr: new SecureStoreUnavailableError('no dbus') } }, + }) + + expect(result).toMatchObject({ status: 'skipped', reason: 'legacy-keyring-unreachable' }) + }) + + it('migrates a legacy keyring token into a per-user slot and clears the legacy entry', async () => { + const cleanup = vi.fn(async () => undefined) + const { km, state, harness, result, marker, markMigrated } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + options: { + identifyAccount: async (token) => { + expect(token).toBe('legacy_tok') + return { id: '99', email: 'me@x.io', label: 'me@x.io' } + }, + cleanupLegacyConfig: cleanup, + }, + }) + + expect(result.status).toBe('migrated') + if (result.status === 'migrated') expect(result.account.id).toBe('99') + expect(km.slots.get('user-99')?.secret).toBe('legacy_tok') + expect(km.slots.get(LEGACY)?.secret).toBeNull() + expect(state.records.get('99')?.fallbackToken).toBeUndefined() + expect(state.defaultId).toBe('99') + expect(harness.upsertSpy).toHaveBeenCalledTimes(1) + expect(cleanup).toHaveBeenCalledTimes(1) + expect(markMigrated).toHaveBeenCalledTimes(1) + expect(marker.migrated).toBe(true) + }) + + it('falls back to loadLegacyPlaintextToken when the legacy keyring slot is empty', async () => { + const { km, state, result } = await runMigration({ + options: { + loadLegacyPlaintextToken: async () => 'plain_legacy', + identifyAccount: async () => ({ id: '7', email: 'p@l.x' }), + }, + }) + + expect(result.status).toBe('migrated') + expect(km.slots.get('user-7')?.secret).toBe('plain_legacy') + expect(state.records.get('7')?.fallbackToken).toBeUndefined() + }) + + it('migrates against an entirely offline keyring when the plaintext slot has the token (WSL/headless)', async () => { + // The keyring is dead: reading the legacy slot throws and writing + // the per-user slot would too. Migration must still complete by + // sourcing the token from the consumer's plaintext slot and + // parking it on the user record as `fallbackToken`. + const { state, result } = await runMigration({ + slots: { + [LEGACY]: { getErr: new SecureStoreUnavailableError('no dbus') }, + 'user-7': { setErr: new SecureStoreUnavailableError('no dbus') }, + }, + options: { + loadLegacyPlaintextToken: async () => 'plain_legacy', + identifyAccount: async () => ({ id: '7', email: 'p@l.x' }), + }, + }) + + expect(result.status).toBe('migrated') + expect(state.records.get('7')?.fallbackToken).toBe('plain_legacy') + expect(state.defaultId).toBe('7') + }) + + it('returns skipped(identify-failed) when identifyAccount throws', async () => { + const { km, state, result } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + options: { + identifyAccount: async () => { + throw new Error('HTTP 401') + }, + }, + }) + + expect(result).toMatchObject({ status: 'skipped', reason: 'identify-failed' }) + if (result.status === 'skipped') expect(result.detail).toContain('HTTP 401') + expect(state.records.size).toBe(0) + // Legacy entry must remain so a retry can find it. + expect(km.slots.get(LEGACY)?.secret).toBe('legacy_tok') + }) + + it('returns skipped(user-record-write-failed) when userRecords.upsert throws and leaves the legacy entry intact', async () => { + const km = buildKeyringMap() + km.slots.set(LEGACY, { secret: 'legacy_tok' }) + mockedCreateSecureStore.mockImplementation(km.create) + const harness = buildUserRecords() + harness.upsertSpy.mockRejectedValueOnce(new Error('disk full')) + const marker = { migrated: false } + + const result = await migrateLegacyAuth({ + serviceName: SERVICE, + legacyAccount: LEGACY, + userRecords: harness.store, + hasMigrated: async () => marker.migrated, + markMigrated: async () => { + marker.migrated = true + }, + loadLegacyPlaintextToken: async () => null, + identifyAccount: async () => ({ id: '99', email: 'me@x.io' }), + silent: true, + }) + + expect(result).toMatchObject({ status: 'skipped', reason: 'user-record-write-failed' }) + // Legacy keyring entry is untouched so the next attempt can retry. + expect(km.slots.get(LEGACY)?.secret).toBe('legacy_tok') + // Marker is NOT set — otherwise the next run would short-circuit. + expect(marker.migrated).toBe(false) + }) + + it('returns skipped(marker-write-failed) without touching the legacy slot when markMigrated throws', async () => { + // The v2 record IS already written at this point, but the durable + // gate isn't set. Surfacing `migrated` would let a later `logout` + // open the door to re-migrating the still-present legacy token. + const { km, state, result } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + marker: { migrated: false, markFails: new Error('config write blocked') }, + options: { + identifyAccount: async () => ({ id: '42', email: 'a@b' }), + }, + }) + + expect(result).toMatchObject({ status: 'skipped', reason: 'marker-write-failed' }) + // v2 record is on disk; that's fine — the marker gate is what + // keeps the next run from re-migrating. + expect(state.records.has('42')).toBe(true) + // Legacy entry preserved so the retry can find it. + expect(km.slots.get(LEGACY)?.secret).toBe('legacy_tok') + }) + + it('still returns migrated when legacy cleanup fails (marker is the one-way gate, not cleanup)', async () => { + const { result, marker } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok', delErr: new Error('keyring busy') } }, + options: { + identifyAccount: async () => ({ id: '42', email: 'a@b' }), + cleanupLegacyConfig: async () => { + throw new Error('config cleanup blew up') + }, + }, + }) + + expect(result.status).toBe('migrated') + expect(marker.migrated).toBe(true) + }) + + it('still returns migrated when cleanupLegacyConfig throws synchronously', async () => { + // Without the `Promise.resolve().then(...)` wrapper a synchronous + // throw escapes `Promise.allSettled` and makes the whole helper + // reject — *after* the marker is already set. + const { result } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + options: { + identifyAccount: async () => ({ id: '42', email: 'a@b' }), + cleanupLegacyConfig: (() => { + throw new Error('sync explosion') + }) as unknown as () => Promise, + }, + }) + + expect(result.status).toBe('migrated') + }) + + it('preserves an already-pinned defaultId across a successful migration', async () => { + // Retry scenario: a previous run wrote the v2 record + setDefaultId + // but `markMigrated` failed. Between then and now the user logged + // in to a different account and picked it as the default. The + // retry must not blindly promote the legacy account back. + const otherAccount = { id: 'other', label: 'other', email: 'o@x' } + const { state, result } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + seedRecords: { other: { account: otherAccount } }, + seedDefaultId: 'other', + options: { + identifyAccount: async () => ({ id: '42', email: 'a@b' }), + }, + }) + + expect(result.status).toBe('migrated') + expect(state.records.has('42')).toBe(true) + expect(state.defaultId).toBe('other') + }) + + it('writes to a custom keyring slot when accountForUser is overridden', async () => { + const { km, result } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + options: { + accountForUser: (id) => `custom-${id}`, + identifyAccount: async () => ({ id: '99', email: 'me@x.io' }), + }, + }) + + expect(result.status).toBe('migrated') + expect(km.slots.get('custom-99')?.secret).toBe('legacy_tok') + expect(km.slots.get('user-99')?.secret).toBeUndefined() + }) + + it('prefers the legacy keyring token over the plaintext fallback when both are populated', async () => { + // Locks the keyring-first precedence — a refactor that flipped the + // order would silently surface a stale plaintext token even when a + // freshly-rotated keyring credential exists. + const { km, state, result } = await runMigration({ + slots: { [LEGACY]: { secret: 'fresh_keyring_tok' } }, + options: { + loadLegacyPlaintextToken: async () => 'stale_plaintext_tok', + identifyAccount: async (token) => { + expect(token).toBe('fresh_keyring_tok') + return { id: '7', email: 'p@l.x' } + }, + }, + }) + + expect(result.status).toBe('migrated') + expect(km.slots.get('user-7')?.secret).toBe('fresh_keyring_tok') + expect(state.records.get('7')?.fallbackToken).toBeUndefined() + }) +}) + +describe('migrateLegacyAuth — stderr privacy', () => { + let consoleError: ReturnType + beforeEach(() => { + mockedCreateSecureStore.mockReset() + consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined) + }) + afterEach(() => { + consoleError.mockRestore() + }) + + it('the success line carries no account identifier (id/label/email may all be PII)', async () => { + await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + options: { + identifyAccount: async () => ({ + id: 'sensitive-id@email.example', + label: 'sensitive@email.example', + email: 'sensitive@email.example', + }), + silent: false, + logPrefix: 'td', + }, + }) + + const lines = consoleError.mock.calls.flat().join('\n') + expect(lines).toContain('migrated existing token to multi-user store') + expect(lines).not.toContain('sensitive') + }) + + it('the skip line is generic and does not echo the raw exception text', async () => { + const { result } = await runMigration({ + slots: { [LEGACY]: { secret: 'legacy_tok' } }, + options: { + identifyAccount: async () => { + throw new Error('email leak: sensitive@email.example at /Users/me/.config/x') + }, + silent: false, + logPrefix: 'td', + }, + }) + + expect(result.status).toBe('skipped') + const lines = consoleError.mock.calls.flat().join('\n') + expect(lines).toContain('could not identify user') + expect(lines).not.toContain('sensitive@email.example') + expect(lines).not.toContain('/Users/me/.config/x') + // The raw detail is preserved on the result for in-process callers. + if (result.status === 'skipped') { + expect(result.detail).toContain('sensitive@email.example') + } + }) +}) diff --git a/src/auth/keyring/migrate.ts b/src/auth/keyring/migrate.ts new file mode 100644 index 0000000..9b407ba --- /dev/null +++ b/src/auth/keyring/migrate.ts @@ -0,0 +1,258 @@ +import { getErrorMessage } from '../../errors.js' +import type { AuthAccount } from '../types.js' +import { writeRecordWithKeyringFallback } from './record-write.js' +import { + createSecureStore, + DEFAULT_ACCOUNT_FOR_USER, + type SecureStore, + SecureStoreUnavailableError, +} from './secure-store.js' +import type { UserRecordStore } from './types.js' + +export type MigrateLegacyAuthOptions = { + serviceName: string + /** Legacy single-user keyring account slug, e.g. `'api-token'`. */ + legacyAccount: string + /** v2 user-record store the migrated record is written into. */ + userRecords: UserRecordStore + /** Per-user keyring slug for the new entry. Defaults to `user-${id}`. */ + accountForUser?: (id: string) => string + /** + * Reads the durable "migration already ran" marker the consumer owns + * (typically a flag in their config). When this returns `true`, the + * helper short-circuits with `already-migrated` and never touches the + * legacy state. This is the **one-way gate** — without it, a later + * `logout` (which empties `userRecords`) followed by a reinstall would + * cause the helper to re-migrate a stale legacy token. + */ + hasMigrated: () => Promise + /** + * Persists the durable migration marker. Called **after** the v2 record + * write succeeds and **before** legacy cleanup so the gate is set before + * any best-effort follow-up runs. If this throws, the helper returns a + * `skipped` result with reason `marker-write-failed` — the v2 record is + * already on disk, but the marker isn't, so the caller should retry. + */ + markMigrated: () => Promise + /** + * Returns the v1 token from the consumer's *plaintext* config slot, or + * `null` if absent. cli-core handles the legacy keyring slot itself. + */ + loadLegacyPlaintextToken: () => Promise + /** + * Identifies the user behind the v1 token. Implementations typically hit + * the product API with the token to fetch the canonical `id` / `email` + * for the new account record. + */ + identifyAccount: (token: string) => Promise + /** + * Optional best-effort cleanup of v1-only config fields after a + * successful migration (e.g. unset legacy `api_token` / `auth_mode`). + * Runs concurrently with the legacy keyring delete; failures are + * swallowed because the marker (above) is what gates re-migration. + */ + cleanupLegacyConfig?: () => Promise + /** Suppress stderr output (postinstall hooks set this). */ + silent?: boolean + /** Label used in the stderr log line. Defaults to `'cli'`. */ + logPrefix?: string +} + +/** + * Stable skip reasons. `legacy-keyring-unreachable` is retryable (a later + * run with the keyring online would succeed); the others are diagnostic. + */ +export type MigrateSkipReason = + | 'identify-failed' + | 'legacy-keyring-unreachable' + | 'user-record-write-failed' + | 'marker-write-failed' + +const SKIP_REASON_MESSAGES: Record = { + 'identify-failed': 'could not identify user', + 'legacy-keyring-unreachable': 'legacy credential is unreachable (keyring offline)', + 'user-record-write-failed': 'failed to update user records', + 'marker-write-failed': 'failed to persist migration marker', +} + +/** + * Discriminated by `status`. Narrowing on `status === 'skipped'` exposes + * the structured `reason` + free-form `detail`; `migrated` carries the + * resolved account. + */ +export type MigrateAuthResult = + | { status: 'already-migrated' } + | { status: 'no-legacy-state' } + | { status: 'migrated'; account: TAccount } + | { status: 'skipped'; reason: MigrateSkipReason; detail: string } + +type LegacyTokenResult = + | { kind: 'found'; token: string } + | { kind: 'none' } + | { kind: 'keyring-unavailable' } + +/** + * One-time migration of a v1 single-user auth state into a v2 multi-user + * shape. Best-effort: any failure (offline keyring, network error fetching + * the user, …) leaves the v1 state untouched so the consumer's runtime + * fallback can keep serving the legacy token until the next attempt. + * + * Order of operations is deliberate so the migration is genuinely one-way: + * + * 1. `hasMigrated()` short-circuits if the durable marker is already set. + * 2. Read the v1 token (legacy keyring slot first, then plaintext). + * 3. `identifyAccount(token)` resolves the v2 `account` shape. + * 4. `writeRecordWithKeyringFallback` writes the v2 record. + * 5. Best-effort `setDefaultId(account.id)` so the new record is active. + * 6. `markMigrated()` persists the marker. **If this fails, we return + * `skipped` even though the v2 record is on disk** — the marker is + * what prevents re-migration on the next run. + * 7. Best-effort legacy keyring delete + `cleanupLegacyConfig()` run + * concurrently. Failures here are harmless because the marker is set. + */ +export async function migrateLegacyAuth( + options: MigrateLegacyAuthOptions, +): Promise> { + const { + serviceName, + legacyAccount, + userRecords, + hasMigrated, + markMigrated, + loadLegacyPlaintextToken, + identifyAccount, + cleanupLegacyConfig, + silent, + } = options + const accountForUser = options.accountForUser ?? DEFAULT_ACCOUNT_FOR_USER + const logPrefix = options.logPrefix ?? 'cli' + + if (await hasMigrated()) { + return { status: 'already-migrated' } + } + + // One legacy-keyring handle covers both the initial read and the + // post-success cleanup delete. + const legacyStore = createSecureStore({ serviceName, account: legacyAccount }) + + const legacyToken = await readLegacyToken(legacyStore, loadLegacyPlaintextToken) + if (legacyToken.kind === 'none') return { status: 'no-legacy-state' } + if (legacyToken.kind === 'keyring-unavailable') { + return skipped( + silent, + logPrefix, + 'legacy-keyring-unreachable', + 'OS keyring unreachable while reading legacy slot', + ) + } + + let account: TAccount + try { + account = await identifyAccount(legacyToken.token) + } catch (error) { + return skipped(silent, logPrefix, 'identify-failed', getErrorMessage(error)) + } + + // `writeRecordWithKeyringFallback` swallows `SecureStoreUnavailableError` + // internally (writing to `fallbackToken` instead), so any error here is + // a non-keyring failure — typically a `userRecords.upsert` rejection. + try { + await writeRecordWithKeyringFallback({ + secureStore: createSecureStore({ + serviceName, + account: accountForUser(account.id), + }), + userRecords, + account, + token: legacyToken.token, + }) + } catch (error) { + return skipped(silent, logPrefix, 'user-record-write-failed', getErrorMessage(error)) + } + + // Default promotion is best-effort and **only fires when nothing is + // already pinned**. A retry after a previous `markMigrated()` failure + // can land on a store where the user has since logged in to a different + // account and picked it as default — blindly setting the legacy account + // back as default would silently switch the user. + try { + const existingDefault = await userRecords.getDefaultId() + if (!existingDefault) { + await userRecords.setDefaultId(account.id) + } + } catch { + // best-effort + } + + // Persist the one-way marker BEFORE legacy cleanup. If this fails, the + // v2 record is already written but the gate is unset — surface as + // `skipped` so the caller retries. Without this ordering, a later + // `logout` could let the next run re-migrate the stale v1 token. + try { + await markMigrated() + } catch (error) { + return skipped(silent, logPrefix, 'marker-write-failed', getErrorMessage(error)) + } + + // Best-effort legacy cleanup — runs concurrently so we don't pay + // keyring latency followed by config-write latency on every install. + // The marker is already set, so a failure here can't cause + // re-migration on the next run. The `Promise.resolve().then(...)` + // wrappers convert any *synchronous* throw from a consumer-supplied + // `cleanupLegacyConfig` (or an oddly-implemented `SecureStore`) into + // a rejected promise that `Promise.allSettled` can swallow. + await Promise.allSettled([ + Promise.resolve().then(() => legacyStore.deleteSecret()), + Promise.resolve().then(() => cleanupLegacyConfig?.()), + ]) + + if (!silent) { + // No identifier in the log line — `account.id` is typed as `string` + // but consumers can legitimately use an email or other PII there. + // Callers that need richer telemetry can compose it from the + // returned `account`. + console.error(`${logPrefix}: migrated existing token to multi-user store.`) + } + + return { status: 'migrated', account } +} + +async function readLegacyToken( + legacyStore: SecureStore, + loadLegacyPlaintextToken: () => Promise, +): Promise { + let keyringUnavailable = false + try { + const stored = await legacyStore.getSecret() + if (stored?.trim()) return { kind: 'found', token: stored.trim() } + } catch (error) { + if (!(error instanceof SecureStoreUnavailableError)) throw error + keyringUnavailable = true + } + + const plaintext = await loadLegacyPlaintextToken() + if (plaintext?.trim()) return { kind: 'found', token: plaintext.trim() } + + return keyringUnavailable ? { kind: 'keyring-unavailable' } : { kind: 'none' } +} + +/** + * Emit the migration skip line. The stderr text is a fixed phrase keyed off + * `MigrateSkipReason` so consumer-supplied callbacks (`identifyAccount`, + * the `UserRecordStore`, …) can't leak emails, paths, or auth diagnostics + * into logs. The raw error message is still attached to the returned + * `MigrateAuthResult.detail` for in-process callers that need it. + */ +function skipped( + silent: boolean | undefined, + logPrefix: string, + reason: MigrateSkipReason, + detail: string, +): MigrateAuthResult { + if (!silent) { + console.error( + `${logPrefix}: skipped legacy auth migration — ${SKIP_REASON_MESSAGES[reason]}.`, + ) + } + return { status: 'skipped', reason, detail } +}