[scottlovegrove]

Summary

Fixes #155. --credential-store plaintext only lasted until the next token refresh.

The credential-store policy is resolved per invocation from the CLI flag and is never persisted. refreshAccessToken writes the rotated bundle through persistBundle → setBundle, and that write ran with the consumer's default policy ('fallback' in comms-cli). If the keyring was reachable at that moment, the rotated tokens moved into the keyring and the record was upserted without fallbackToken, so the plaintext copy the user picked at login was gone. The next read from a context where the keyring is unreachable (a scheduled job during a macOS dark wake) then failed.

Changes

• New SetBundleOptions type (promoteDefault?, preserveStorage?), exported from @doist/cli-core/auth, used by TokenStore.setBundle and KeyringTokenStore.setBundle.
• persistBundle forwards preserveStorage, and still leaves out the options argument when nothing is set.
• refreshAccessToken passes preserveStorage: true.
• createKeyringTokenStore().setBundle with preserveStorage: if the existing record holds fallbackToken or fallbackRefreshToken, the rotated bundle is written as plaintext, whatever the configured policy. Records held in the credential manager keep the configured policy.
• README updated.

Login and auth token writes still use the configured policy, so an explicit choice at login still wins and a user can move a credential back into the keyring by logging in again.

The storage is inferred from the existing record, not persisted as a marker, so no consumer UserRecordStore or config schema has to change. As a result, a record that fell back to plaintext under 'fallback' (keyring unavailable at login) also stays in plaintext until the next login, where before it moved into the keyring on refresh. That matches what the dark-wake case needs.

Related

Doist/todoist-cli#577 (Linux without Secret Service lands in the per-session kernel keyring) will be fixed in a stacked PR. Its remedy is to point users at --credential-store=plaintext, and for CLIs that refresh tokens that only works with this fix in place.

End-to-end testing

Tested end to end against comms-cli (tdc) main, with a throwaway XDG_CONFIG_HOME and a real OAuth login:

1. tdc auth login --credential-store plaintext, then set accessTokenExpiresAt to the past and run tdc auth status to force a silent refresh.
2. On cli-core 1.6.0 this reproduced the bug: both plaintext fields were dropped from the record, the rotated tokens went to the keychain, and tdc doctor reported "via secure-store" with no warning.
3. With this branch installed, the refresh rotated both tokens and wrote them back to the config, and tdc doctor still reported "token is stored in plaintext config fallback".

Test plan

• npm test, npm run type-check, npm run check
• New token-store tests: plaintext record + preserveStorage stays plaintext with the keyring online (fails without the fix); keyring record keeps the configured policy; without preserveStorage the configured policy applies
• persist / refresh tests cover forwarding preserveStorage: true

[doistbot]

This PR keeps credentials stored as plaintext via --credential-store plaintext in plaintext across silent token refreshes by having the keyring store infer the existing record's storage backend and reuse it on rotated writes. Well-targeted fix, and the tests and README cover the behavior change.

Few things worth tightening:

• storedCredentialStore treats the mere presence of fallbackToken/fallbackRefreshToken as proof of plaintext storage, but the read side only trusts non-empty trimmed values. A keyring-backed record carrying an empty/null fallbackToken would read from the keyring today, then get silently downgraded to plaintext on the next refresh — suggest matching the read semantics (e.g. check existing?.fallbackToken?.trim() || existing?.fallbackRefreshToken?.trim()).

I also included a few optional follow-up notes in the details below.

Optional follow-up notes (5)

• src/auth/types.ts:118: This JSDoc says preserveStorage keeps the credential in the backend that already holds it, but createKeyringTokenStore.setBundle only pins plaintext records: a keyring-held record still follows resolveCredentialStore() (so configured 'plaintext', or 'fallback' with the keyring offline, moves it to plaintext). That asymmetry is intended per the PR description, but the exported contract (here and in the TokenStore.setBundle doc below) should say something like "keep a plaintext credential in plaintext; records in the credential manager keep the configured policy" so implementers don't rely on the broader promise.
• src/auth/persist.ts:17: PersistBundleOptions redeclares the forwarded setBundle option fields instead of reusing the new SetBundleOptions type. Change it to SetBundleOptions & { store: TokenStore<TAccount>; account: TAccount; bundle: TokenBundle } so promoteDefault and preserveStorage can't fall out of sync with TokenStore.setBundle.
• src/auth/keyring/token-store.ts:270: This plaintext-record predicate repeats the exact expression already used in clear()'s fallback detection. Extract one helper (for example hasPlaintextFallback(record)) and call it from both places so the definition of a plaintext-held record has a single source.
• src/auth/keyring/token-store.test.ts:613: This test only covers a record with both fallback fields. storedCredentialStore treats each field independently (fallbackToken !== undefined || fallbackRefreshToken !== undefined), so the fallbackRefreshToken-only branch is unprotected: a record where the access slot wrote to the keyring but the refresh slot fell back would move back into the keyring on refresh if that check regressed, and the suite would still pass. Add a fixture with only fallbackRefreshToken set.
• src/auth/keyring/token-store.test.ts:635: This test runs under the default 'fallback' policy, so it does not actually exercise the configured-policy branch for a keyring-held record. A regression that hardcodes the keyring path for preserveStorage would pass, and a user who switched to --credential-store=plaintext would keep their secret in the keyring on refresh. Add a variant with a non-default credentialStore (e.g. 'plaintext') and assert the rotated bundle lands in fallbackToken/fallbackRefreshToken.

_{Share Feedback • Review Logs}

[doist-release-bot]

🎉 This PR is included in version 1.7.2 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀