feat(auth): store API tokens in the OS keyring via @doist/cli-core by scottlovegrove · Pull Request #73 · Doist/outline-cli

scottlovegrove · 2026-05-17T15:15:34Z

Summary

Replaces outline-cli's bespoke plaintext-config token storage with cli-core's createKeyringTokenStore + createSecureStore (@napi-rs/keyring). Tokens now live in the OS credential manager (macOS Keychain, Windows Credential Manager, libsecret/D-Bus on Linux) instead of ~/.config/outline-cli/config.json. Mirrors the twist-cli architecture so the auth surface stays consistent across Doist CLIs.

Single-account scope. Multi-account commands (ol account list/use/current/remove + global --user <ref>) are a deliberate follow-up.

Storage cascade

createOutlineTokenStore.active() resolves in this order:

OUTLINE_API_TOKEN env var (only when no explicit ref is supplied)
Lazy migration runs once per process, memoised
v2 record from users[] → keyring getSecret (or fallbackToken on the record when keyring was unavailable at write time)
v1 legacy snapshot (plaintext api_token in config) when migration is inconclusive — keeps the CLI working through offline migrations

WSL / no-keyring handling

cli-core handles this natively — no extra code on our side:

createSecureStore lazy-imports @napi-rs/keyring. On failure it surfaces SecureStoreUnavailableError.
createKeyringTokenStore.set() catches that, writes fallbackToken on the user record instead, and returns a TokenStorageResult with storage: 'config-file' and a warning string.
Login still succeeds; the user sees a stderr warning (Warning: system credential manager unavailable; token saved as plaintext in …) via logTokenStorageResult.
migrateLegacyAuth returns legacy-keyring-unreachable as an inconclusive skip — the runtime legacy-snapshot fallback covers the gap.

Migration

One-time, lazy. Triggered on first active()/set()/clear()/list()/setDefault(), memoised per-process via ensureMigrated(). Idempotent via the new config_version: 2 marker.

identifyAccount calls POST /api/auth.info via raw fetchWithRetry (bypasses apiRequest so the silent migration doesn't render a spinner and so this module stays out of the runtime auth graph). On failure (offline, 401, no base_url reachable) migration returns skipped/identify-failed, the legacy snapshot keeps serving the token, and the next online run retries.

Test plan

npm run type-check
npm run lint:check — 0 warnings, 0 errors
npm run format:check
npm run check:skill-sync
npm run test — 138/138 across 18 files
Manual happy path (macOS Keychain): seed ~/.config/outline-cli/config.json with a v1 api_token, run ol auth status. Expect: legacy fallback served first call; migration completes; api_token removed from config.json; record visible at users[0] with config_version: 2; token retrievable via security find-generic-password -s outline-cli -a <user-id> -w
Manual: ol auth logout then ol auth login — fresh keyring-backed write, no token field on the record, no stderr warning
Manual WSL / no-keyring path (mocked by moving node_modules/@napi-rs/keyring aside): ol auth login succeeds, stderr shows the keyring-unavailable warning, users[0].token populated
Manual env-token short-circuit: OUTLINE_API_TOKEN=… ol auth status — keyring never touched, reports source: 'env'

🤖 Generated with Claude Code

Replaces outline-cli's bespoke plaintext-config token storage with cli-core's `createKeyringTokenStore` + `createSecureStore` (keytar via `@napi-rs/keyring`). Mirrors the twist-cli architecture so the auth surface stays consistent across Doist CLIs. WSL / headless Linux is handled by cli-core natively: when the OS keyring is unreachable, the token is parked on the user record as a `fallbackToken` plaintext field, login still succeeds, and a stderr warning surfaces via `logTokenStorageResult`. A one-time, lazy migration moves the v1 `api_token` plaintext slot into the v2 `users[]` record on first store access. Migration is memoised per-process and gated by a `config_version` marker so it can never re-run. Failures (offline `identifyAccount`, no keyring) fall back to the legacy snapshot until the next attempt — users are never locked out mid-flight. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

doistbot

Integrating @doist/cli-core for OS keyring token storage is a great architectural update that securely aligns this project's auth with other Doist CLIs. The approach is well-reasoned and cleanly handles fallback environments, ensuring a smooth transition for existing users. A few adjustments are needed to ensure legacy state cleanup is atomic, improve test coverage for the new user record store, address prototype inheritance safety, and optimize token store initialization on the request hot path.

_{Share Feedback • Review Logs}

P1 — correctness bugs - `active()` now tries the v2 store first and only falls back to the legacy snapshot when the v2 store is empty. A stale `api_token` can no longer shadow a successful v2 write even if the best-effort legacy cleanup hasn't run. - `set()` / `clear()` now discharge legacy state AFTER the v2 op succeeds. Previously a failing keyring write would erase the user's only working credentials before the v2 attempt. - Add direct unit tests for `createOutlineUserRecordStore` mutation methods (`upsert` / `remove` / `setDefaultId`) and `getDefaultUserRecord`. Previously zero coverage because tests mocked cli-core's keyring layer. P2 — should-fix - Drop `Object.create(inner) as OutlineTokenStore` prototype-swap + forced cast. Return an explicit delegating object that forwards `getLastStorageResult` / `getLastClearResult` by call. - Fix `getActiveTokenSource` to mirror `active()`s resolution order — v2 record check now runs before the v1 plaintext slot check, so a stale `api_token` surviving a `cleanupLegacyConfig` failure no longer makes `auth status` lie about the source. - Dedupe `TOKEN_ENV_VAR` (now in auth-constants.ts, imported by both auth.ts and auth-provider.ts). - Dedupe `DEFAULT_BASE_URL` (single source in outline-account.ts). - Dedupe legacy-clear payload — shared `LEGACY_CLEAR_PAYLOAD` constant in auth-constants.ts, used by both `migrate-auth.cleanupLegacyConfig` and `auth-provider.dischargeLegacyState`. - Hoist `createOutlineTokenStore()` to a module-level singleton in auth.ts so the request hot path doesn't rebuild keyring + user-record adapters per API POST. - Add 10s timeout to migration `auth.info` call so a stalled connection can't hang the CLI indefinitely. - Restore the `auth logout --ndjson` silent-stdout test. The storage-result confirmation now flowing through `logTokenStorageResult` makes this invariant more important, not less. P3 — nits - Use `${TOKEN_ENV_VAR}` template in NoTokenError hint. - Drop forced `as AuthInfoResponse` cast in migrate-auth. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

scottlovegrove · 2026-05-17T15:42:36Z

@doistbot /review

doistbot

This PR successfully integrates @doist/cli-core to securely transition API tokens from plaintext configuration into the OS keyring, aligning the auth architecture with other Doist tools. The implementation provides a smooth fallback experience for environments without keyring access and sets up a solid foundation for future multi-account support. However, there are a couple of critical sequence issues with the lazy migration and logout flows that could inadvertently overwrite tokens or leave users authenticated after a clear operation. Additionally, a few adjustments are needed to resolve an ESM circular dependency, consolidate token precedence logic, and address some minor test coverage and performance details.

_{Share Feedback • Review Logs}

P1 — correctness - `auth-provider.ts`: `ensureMigrated()` now runs BEFORE every mutating v2 op (`set` / `clear`), and legacy discharge runs after the v2 op succeeds. Eliminates the race where a stale `api_token` survived `inner.set/clear` long enough for the post-op migration to grab it (which would either duplicate the freshly-logged-in account or revive auth right after logout). - `auth-provider.ts`: `clear()` now propagates the legacy-discharge failure instead of swallowing it. With v2 empty after a successful `inner.clear`, a silently-failed cleanup would let `active()` fall back to the surviving `api_token` — `ol auth logout` reporting success while leaving the user authenticated. Logout now fails loudly. `set()` retains best-effort swallow (v2 wins in `active()`). P2 — should-fix - `migrate-auth.hasMigrated` now also accepts `users.length > 0` as a "v2" signal, so fresh installs that login directly (no v1 marker ever written) don't get re-migrated on every launch. - `auth.getApiToken` short-circuits `OUTLINE_API_TOKEN` directly, skipping the token store's `active()` (which would call `getBaseUrl()` to synthesise an account just for the snapshot — redundant since `apiRequest` already resolves the base URL). - `_fixtures/auth.okResponse` / `errResponse` use the native `Response.json()` static helper instead of casting an object literal. - `__tests__/auth.test.ts` mocks `runMigrateLegacyAuth` directly instead of stubbing transitive `fetchWithRetry` failures. - Added regression tests: `clear()` atomicity (no discharge on v2 failure), `clear()` failure propagation, `set()` migration ordering, and `args.options.timeout === 10_000` in the migrate-auth test. - `logTokenStorageResult` exported from `commands/auth.ts` and tested directly — covers the secure-store stdout confirmation, the machine-mode suppression, and the warning-routes-to-stderr branch. Deferred (with rationale on PR thread) - P2.3 `getActiveTokenSource` precedence dedupe: real dedupe requires either extra hot-path disk I/O or augmenting cli-core's `TokenStore` contract. The drift is guarded by the existing regression test. - P2.5 import cycle: `auth.ts` ↔ `auth-provider.ts` is function-level safe today; extracting a new shared module just to break the cycle costs more LOC than the theoretical ESM risk warrants. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

## [1.7.0](v1.6.0...v1.7.0) (2026-05-17) ### Features * **auth:** store API tokens in the OS keyring via @doist/cli-core ([#73](#73)) ([b353aea](b353aea))

doist-release-bot · 2026-05-17T16:04:33Z

🎉 This PR is included in version 1.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

frankieyan · 2026-05-20T05:56:14Z

I was able to verify all the test steps manually:

But it will take me a bit longer to read through everything. Will try to wrap it up when I'm back on Thursday.

doistbot reviewed May 17, 2026

View reviewed changes

scottlovegrove self-assigned this May 17, 2026

scottlovegrove added the 👀 Show PR PR must be reviewed before or after merging label May 17, 2026

doistbot reviewed May 17, 2026

View reviewed changes

scottlovegrove requested review from a team and frankieyan and removed request for a team May 17, 2026 16:03

scottlovegrove merged commit b353aea into main May 17, 2026
5 checks passed

scottlovegrove deleted the scottl/keyring branch May 17, 2026 16:03

doist-release-bot Bot added a commit that referenced this pull request May 17, 2026

chore(release): 1.7.0 [skip ci]

48814df

## [1.7.0](v1.6.0...v1.7.0) (2026-05-17) ### Features * **auth:** store API tokens in the OS keyring via @doist/cli-core ([#73](#73)) ([b353aea](b353aea))

doist-release-bot Bot added the released label May 17, 2026

Conversation

scottlovegrove commented May 17, 2026 • edited by frankieyan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Storage cascade

WSL / no-keyring handling

Migration

Test plan

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

scottlovegrove commented May 17, 2026

Uh oh!

doistbot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

doist-release-bot Bot commented May 17, 2026

Uh oh!

frankieyan commented May 20, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

scottlovegrove commented May 17, 2026 •

edited by frankieyan

Loading