From 2563c1b534bacb2afbcd4f4a72491fc44de02d91 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 12:37:44 +0800 Subject: [PATCH 01/22] feat(oauth): add keychain-backed token storage with file fallback Add KeyringTokenStorage (backed by @napi-rs/keyring) and a resolveTokenStorage factory that selects the OS keychain when usable and falls back to the existing plaintext FileTokenStorage otherwise. The factory guards two distinct failure modes: native binary not loadable (require throws at import) and binary loads but has no live OS backend (operations throw at call time, caught by a set/get/delete capability probe under an isolated sentinel service). KIMI_DISABLE_KEYRING=1 forces the file backend. When the keychain is selected but a token is still only on disk (older file-only build), load() migrates it into the keychain then deletes the plaintext file. remove()/list() reconcile against the legacy store so pre-migration plaintext can never linger or go missing. The class depends on an injectable KeyringApi interface, keeping it fully unit-testable without touching the real OS keychain. Adds a `test` script to the package (was missing) so the suite is runnable via pnpm filter. --- packages/oauth/package.json | 2 + packages/oauth/src/keyring-storage.ts | 198 ++++++++++++++++++++ packages/oauth/src/types.ts | 2 +- packages/oauth/test/keyring-storage.test.ts | Bin 0 -> 7925 bytes pnpm-lock.yaml | 135 +++++++++++++ 5 files changed, 336 insertions(+), 1 deletion(-) create mode 100644 packages/oauth/src/keyring-storage.ts create mode 100644 packages/oauth/test/keyring-storage.test.ts diff --git a/packages/oauth/package.json b/packages/oauth/package.json index 9ed7b0487..2481abf8b 100644 --- a/packages/oauth/package.json +++ b/packages/oauth/package.json @@ -40,9 +40,11 @@ "scripts": { "build": "tsdown", "typecheck": "tsc -p tsconfig.json --noEmit", + "test": "vitest run", "clean": "rm -rf dist" }, "dependencies": { + "@napi-rs/keyring": "1.3.0", "proper-lockfile": "^4.1.2" }, "devDependencies": { diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts new file mode 100644 index 000000000..71c7bafa0 --- /dev/null +++ b/packages/oauth/src/keyring-storage.ts @@ -0,0 +1,198 @@ +/** + * Keychain-backed OAuth token storage with plaintext-file fallback. + * + * Backend: the OS keychain (macOS Keychain, Windows Credential Manager, + * Linux Secret Service) via `@napi-rs/keyring`. All tokens live under a single + * keychain *service* (`KEYRING_SERVICE`); each token is one entry whose + * *account* is the token `name` and whose *password* is the snake_case wire + * JSON — the exact same payload `FileTokenStorage` writes to disk. + * + * Selection (`resolveTokenStorage`): the keychain is used only when it is + * actually usable. Two guards are required because the failure modes differ: + * 1. The native binary may fail to load (unsupported platform / missing + * optional binary) — `require` THROWS at import time. Caught → file. + * 2. The binary may load but have no live OS backend at runtime (e.g. + * headless Linux with no Secret Service) — `require` succeeds but entry + * operations throw at CALL time. A set/get/delete capability probe under + * a SEPARATE sentinel service catches this. Failed/​mismatched → file. + * `KIMI_DISABLE_KEYRING=1` forces the file backend outright. + * + * Migration: when the keychain is selected but a token is still only on disk + * (written by an older file-only build), `load` migrates it — copy into the + * keychain, then delete the plaintext file — so secrets stop living in the + * clear. `remove` and `list` also reconcile against the legacy file store so + * pre-migration plaintext can never linger or go missing. + */ + +import { createRequire } from 'node:module'; + +import { FileTokenStorage } from './storage'; +import type { TokenStorage } from './storage'; +import type { TokenInfo, TokenInfoWire } from './types'; +import { tokenFromWire, tokenToWire } from './types'; +import { isRecord } from './utils'; + +/** Keychain service that holds every kimi-code token entry. */ +export const KEYRING_SERVICE = 'kimi-code'; +/** Isolated service for the capability probe; never collides with real data. */ +export const KEYRING_PROBE_SERVICE = 'kimi-code-keyring-probe'; + +/** Minimal keychain entry surface (structurally satisfied by `Entry`). */ +export interface KeyringEntry { + getPassword(): string | null; + setPassword(password: string): void; + /** Returns true if a credential existed. */ + deleteCredential(): boolean; +} + +/** Injectable keychain API so the storage is unit-testable without the OS. */ +export interface KeyringApi { + createEntry(service: string, account: string): KeyringEntry; + /** Accounts present under a service. */ + findAccounts(service: string): string[]; +} + +interface KeyringTokenStorageOptions { + readonly keyring: KeyringApi; + /** File store used both as migration source and reconciliation target. */ + readonly legacy: FileTokenStorage; + readonly service?: string; +} + +export class KeyringTokenStorage implements TokenStorage { + private readonly keyring: KeyringApi; + private readonly legacy: FileTokenStorage; + private readonly service: string; + + constructor(opts: KeyringTokenStorageOptions) { + this.keyring = opts.keyring; + this.legacy = opts.legacy; + this.service = opts.service ?? KEYRING_SERVICE; + } + + private serialize(token: TokenInfo): string { + return JSON.stringify(tokenToWire(token)); + } + + private deserialize(raw: string): TokenInfo | undefined { + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch { + return undefined; + } + if (!isRecord(parsed)) return undefined; + return tokenFromWire(parsed as Partial); + } + + async load(name: string): Promise { + const raw = this.keyring.createEntry(this.service, name).getPassword(); + if (raw !== null) { + return this.deserialize(raw); + } + // Not in the keychain — migrate any plaintext token written by an older + // file-only build, then drop the cleartext copy. + const legacyToken = await this.legacy.load(name); + if (legacyToken === undefined) return undefined; + this.keyring.createEntry(this.service, name).setPassword(this.serialize(legacyToken)); + await this.legacy.remove(name); + return legacyToken; + } + + async save(name: string, token: TokenInfo): Promise { + this.keyring.createEntry(this.service, name).setPassword(this.serialize(token)); + } + + async remove(name: string): Promise { + // Clear both stores so a pre-migration plaintext copy can never linger + // (e.g. logout before the token was ever migrated). Missing credentials + // are a no-op, not an error. + this.keyring.createEntry(this.service, name).deleteCredential(); + await this.legacy.remove(name); + } + + async list(): Promise { + const fromKeyring = this.keyring.findAccounts(this.service); + const fromLegacy = await this.legacy.list(); + return [...new Set([...fromKeyring, ...fromLegacy])]; + } +} + +/** + * Adapter over the real `@napi-rs/keyring` module shape, kept narrow so the + * production load path and the test fakes share one `KeyringApi` contract. + */ +interface NapiKeyringModule { + Entry: new (service: string, account: string) => KeyringEntry; + findCredentials: (service: string) => Array<{ account: string; password: string }>; +} + +function adaptNapiKeyring(mod: NapiKeyringModule): KeyringApi { + return { + createEntry(service, account) { + return new mod.Entry(service, account); + }, + findAccounts(service) { + return mod.findCredentials(service).map((c) => c.account); + }, + }; +} + +/** Real native load: throws (caught here) when the binary can't be required. */ +function loadNativeKeyring(): KeyringApi | undefined { + try { + const require = createRequire(import.meta.url); + const mod = require('@napi-rs/keyring') as NapiKeyringModule; + return adaptNapiKeyring(mod); + } catch { + return undefined; + } +} + +/** + * Round-trip a sentinel under an isolated service to prove the keychain has a + * live backend. Any throw, or a read-back mismatch, means the keychain is not + * usable on this host. + */ +function probeKeyring(keyring: KeyringApi): boolean { + const sentinel = `probe-${Date.now()}-${Math.random().toString(36).slice(2)}`; + try { + const entry = keyring.createEntry(KEYRING_PROBE_SERVICE, 'probe'); + entry.setPassword(sentinel); + const readBack = entry.getPassword(); + entry.deleteCredential(); + return readBack === sentinel; + } catch { + return false; + } +} + +interface ResolveTokenStorageDeps { + /** Returns a usable KeyringApi, or undefined when the native load fails. */ + loadKeyring?: () => KeyringApi | undefined; + /** Force the file backend (defaults to the KIMI_DISABLE_KEYRING env flag). */ + disabled?: boolean; +} + +/** + * Pick the token backend for `credentialsDir`: keychain when usable, otherwise + * the plaintext file store (which also seeds migration). The `deps` seam is for + * tests only; production uses the real native load + env flag. + */ +export function resolveTokenStorage( + credentialsDir: string, + deps: ResolveTokenStorageDeps = {}, +): TokenStorage { + const legacy = new FileTokenStorage(credentialsDir); + + const disabled = deps.disabled ?? process.env['KIMI_DISABLE_KEYRING'] === '1'; + if (disabled) return legacy; + + const loadKeyring = deps.loadKeyring ?? loadNativeKeyring; + const keyring = loadKeyring(); + if (keyring === undefined) return legacy; + + if (!probeKeyring(keyring)) return legacy; + + return new KeyringTokenStorage({ keyring, legacy }); +} diff --git a/packages/oauth/src/types.ts b/packages/oauth/src/types.ts index 9073f8614..19d3cc0d6 100644 --- a/packages/oauth/src/types.ts +++ b/packages/oauth/src/types.ts @@ -8,7 +8,7 @@ * contract; in-process types use camelCase per TS convention. */ -export type OAuthStorageBackend = 'file'; +export type OAuthStorageBackend = 'file' | 'keyring'; /** A persisted OAuth token bundle. */ export interface TokenInfo { diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..4b8b801165111981e888a464d377b4536738ae6e GIT binary patch literal 7925 zcmd5>+in}l5#48fMLihcP)m*EEU*j6D+lsgyHFfoU@5x@97Al*G{s&GXC~cE(p-f= zJ|bV3FUhIyo*PNYlQv9+{{9~A(<`~uD!;xgZe)I83oWiCeM?%J zBAd(3Q!9-%^xuE~hsLukThc`8skADIA%-QrIs5hOJ=tQGOr#+@k&q%XdV4`Pa+yqo z%8%Vs{KY5INF+EjC1ptDr1FEQoECb?qQ>Gz(lDrYTB;#Mf?kV~CCQXVOBx!f=PHr^ z_;hq6lBAgBcG!>U5~q$;h94r!_yCqRGtftPZH7PN+qPLlD&|t-Ydb9|RhmpeQ+2I{ zRYeX@%yRy{P;7)U+(LrCY2~ zaNxxg7c$kH@C>tl0?7Sh3EEqO;=nLgn4>OJOIF(FhS^ zEA^R3CIcGDG4%GIQ)v>dMsfi4N}1RJDO*dLD~@?v%pTFJv%kMTfAh=L#o7B`&wn}_ z@S{x(@#8HZ8the8#POlg$zcG)0lN~=Ys|LGQo?Bs1_ssVv)t-st>fq9+|qi)*SFqv zv_r{xJ}%ZbuxcImTTY(K;x`2(tl3ws-5byH#6fB*Zsg^(e5rKQKL&(cF5JRhNs$}t zAb3I?&Nwb zLiFAD{n%v4nCMUa)sUt2pW#Oh#0aMh;BYOInKo)Jk*sx;paz{Z5kxH%=JAu2bHm~OKAl%}tAZ!Wm9Jl}zPVzxL+y&WRwS7)hJFS> z0_+LJBa>5^qwqGF;9OVEMu;XCwY11ljeRXTpv5uhSK zV<*bQEp2Np9W1M)sf>54-5;z@t+E$ObS>>Wcz99hw3U1Rq8$CqIhFU+yPTYOB20wy z`}Aju{1Zs=PlO5H?bET7;SYr$JZOka>%cOc+;Z={P-*L2FCKeX{D7(r!P1u4RA$o3 zpR`PY4CEObGb)NqqK{U#u9@MtIK9`j1e8PM#wt%wJqFBr=xQ*0_!y4xQ7Vy|>JtLh zhJ^z|6`Oz%R0R)72soh+AH5c6L==U1Q!)18_I`j5Ow{k!2B|ScH?grOE#^10I?iN%Z72QU#8n#>)jsb4fS2?6w4G>+Nnz03UBf{g@uUD;B3pG1@3cqUboW+P7fQ}>Cq{gJA>LQ%hlmb>q18$ z8Z|GgshF4xm5EU%dz+wZV;PFAVfTy0?}q6wJmwCpZ-P7c*u!+iEw3UEyWsD{_+?WrcS}7eC76wMdr78+4@l9(ZRefCWwcufF4q06VJT&_3JM^@h?# zjUp7tM4NH2MAnCOK^`(5*7*@Lfe`+nQ?;;BjN1td3W|jVTRaopGRoMnL3OHU6#Nq+ zYFgUS+jHVOD9M0OHIgnr3DT5}dKx_p6kMy4Nxy6JG*^O*n9IYg5b0qC9)74XY|KFh zNEoKyZLj%y;*INzg)b1gYhlJbhkFMJTB@FiJOOz77JuX zz&~0SRF9cGpjWPGwECt7&amrU$nZdV*}$mfKQ=Nhg*Gzc(;7rVCa zt{ak@;nw3HlWyU>3oG0V4}4A}CILr5yyJP`{t}n^Y83C33C$24xf#h#Ee(d_-%XJt z^}Nd>o?tw7hz<`?=``fk;}e!*Yp#O}4l;I?UE2|104iH{O2h1aB?~u?|v(CA@#N49^ zVpX8cfm(}YxcLI5b{n|8Rxoug*hHywX9t>YA>SI{&Wd|h1JVz@N{yZ~Yojzf%=hbj z;>zn9AL0HiDYTxIw!w)j&QV$*rSighD&5~2nrp}Ls+%gc zBvs_)b3J}72o zxfdo1yRd<);?_WZ)~|eh%)4JzldR8+rV+4`&Pw;H!Cj|es@=`+k1#M1r5LG9*(JLa zH#6?`z&zTxfU+$n@)&*Dz2d2E?7x!D26>z`x4`WJ)I_vuMc!I7!-cv`+1|Tm+-gI( z2-uwOJ;{A+L&PxdNG9Ed%|$az%_kvjZ~Q%&p98{VMF5x zLU$k9Jc_puXHfDEPN_*c#arX@Zqws&x*AO58 literal 0 HcmV?d00001 diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 723961f2d..b804ab2ef 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -528,6 +528,9 @@ importers: packages/oauth: dependencies: + '@napi-rs/keyring': + specifier: 1.3.0 + version: 1.3.0 proper-lockfile: specifier: ^4.1.2 version: 4.1.2 @@ -1681,6 +1684,87 @@ packages: resolution: {integrity: sha512-juG5VWh4qAivzTAeMzvY9xs9HY5rAcr2E4I7tiSSCokRFi7XIZCAu92ZkSTsIj1OPceCifL3cpfteP3pDT9/QQ==} engines: {node: '>=14.0.0'} + '@napi-rs/keyring-darwin-arm64@1.3.0': + resolution: {integrity: sha512-pl76hJvdYUBn6I24bXiOBMA9nbDapo3I5B+f3OorjDU4dUMSypXeKbOVehJe8fhgTiH24flMyTS3aAIy43xegQ==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [darwin] + + '@napi-rs/keyring-darwin-x64@1.3.0': + resolution: {integrity: sha512-YcJtEV5LA3cvA4z3BurgxH5IhTsW1JfIvcAAcqcecwk06Si9F9NqkxbZVIfDwQ8oRHgaBmT3zZJnLAotCrVahw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [darwin] + + '@napi-rs/keyring-freebsd-x64@1.3.0': + resolution: {integrity: sha512-vlLf31TGhfRAaxLDBhg8b89ss0HHD/lyNmL5F3UjSaz5CUXElsJmKYq9fqA/B+cZKUEUcLHHGhF0I/CqcFdaVw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [freebsd] + + '@napi-rs/keyring-linux-arm-gnueabihf@1.3.0': + resolution: {integrity: sha512-KiWdMMu/Inz/bHHIAGrnF7r54FZDYXuHO6UFF/rhIrshUsxbMG1Rl9lEymNtqqsVo927G0VYcb02FzWQ3iBQRQ==} + engines: {node: '>= 10'} + cpu: [arm] + os: [linux] + + '@napi-rs/keyring-linux-arm64-gnu@1.3.0': + resolution: {integrity: sha512-eyKGpY40lm9Jvs1aD294XRH4y7+TlJM0YVAryZeXA6TX0mb4gMkxVXwSQv7MCwgah7raeUd0dKUb4BPAYIgcMg==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-arm64-musl@1.3.0': + resolution: {integrity: sha512-iIK6JWHXAJqDrEyLY3TmswwloVyt2vj+04TZnew+uSJ9gnDO8EwRbp3/iw3LpWaXiDO7VomGO6y8I0Id8uBZSw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-linux-riscv64-gnu@1.3.0': + resolution: {integrity: sha512-/PGqrwn6EwgtK6vccASSXJRfOSP4vN1F4ASsIQ+7MdrK6hNvAJ1FZPrIuD5gGGdxezo3F++To2Wq7DbuGIeuNQ==} + engines: {node: '>= 10'} + cpu: [riscv64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-gnu@1.3.0': + resolution: {integrity: sha512-2PDK1WKWTu9lBGq9VvNEkSlQD3O7YwVpmnyN2M3cy4v7NJ/8gDMd9GXv3G+FVXN13uhp4gnnPBS+ScefmEeD2A==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [glibc] + + '@napi-rs/keyring-linux-x64-musl@1.3.0': + resolution: {integrity: sha512-oJ2HkX8YUo46QBkn0pG+HuIKQNqr523q6vBobCn+P95s4C4K6/kLBqHY/1bg5J4ap31DzsznhnFKcfBNBsjCnw==} + engines: {node: '>= 10'} + cpu: [x64] + os: [linux] + libc: [musl] + + '@napi-rs/keyring-win32-arm64-msvc@1.3.0': + resolution: {integrity: sha512-tOd3c/uAaeoE4ycVlmAdSvygz0Zt3zdca6Y7gokBeIbaRDWpjDIUOpU3MvML59XAaqyuKGsVVu0F/DZb1lHPmw==} + engines: {node: '>= 10'} + cpu: [arm64] + os: [win32] + + '@napi-rs/keyring-win32-ia32-msvc@1.3.0': + resolution: {integrity: sha512-sPSqeAFZMGqP1R++M2JTza7GQJJ/TpCo6JU6Vcd4jnebvOaEDs9b7eipakU1PJdSvhpC2yXMCNRk9gXfrhuwHQ==} + engines: {node: '>= 10'} + cpu: [ia32] + os: [win32] + + '@napi-rs/keyring-win32-x64-msvc@1.3.0': + resolution: {integrity: sha512-4DnCWXwDc0HRKwyRlG5y0VhKZW2tNRQfKKfyj6IX/KWfDNyq9hn4n+GL1auyDcOO/v8PwnhmYo2+rOOqCkvvOg==} + engines: {node: '>= 10'} + cpu: [x64] + os: [win32] + + '@napi-rs/keyring@1.3.0': + resolution: {integrity: sha512-WrOw/bcXm0f9qHkumlT1QlArXSTWqaY9sunsDpOk+yCCorCKMxvWT/a3xko4EYHVdeZoh00yI2TydXn6eyICDA==} + engines: {node: '>= 10'} + '@napi-rs/wasm-runtime@1.1.4': resolution: {integrity: sha512-3NQNNgA1YSlJb/kMH1ildASP9HW7/7kYnRI2szWJaofaS1hWmbGI4H+d3+22aGzXXN9IJ+n+GiFVcGipJP18ow==} peerDependencies: @@ -7505,6 +7589,57 @@ snapshots: '@mozilla/readability@0.6.0': {} + '@napi-rs/keyring-darwin-arm64@1.3.0': + optional: true + + '@napi-rs/keyring-darwin-x64@1.3.0': + optional: true + + '@napi-rs/keyring-freebsd-x64@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm-gnueabihf@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-arm64-musl@1.3.0': + optional: true + + '@napi-rs/keyring-linux-riscv64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-x64-gnu@1.3.0': + optional: true + + '@napi-rs/keyring-linux-x64-musl@1.3.0': + optional: true + + '@napi-rs/keyring-win32-arm64-msvc@1.3.0': + optional: true + + '@napi-rs/keyring-win32-ia32-msvc@1.3.0': + optional: true + + '@napi-rs/keyring-win32-x64-msvc@1.3.0': + optional: true + + '@napi-rs/keyring@1.3.0': + optionalDependencies: + '@napi-rs/keyring-darwin-arm64': 1.3.0 + '@napi-rs/keyring-darwin-x64': 1.3.0 + '@napi-rs/keyring-freebsd-x64': 1.3.0 + '@napi-rs/keyring-linux-arm-gnueabihf': 1.3.0 + '@napi-rs/keyring-linux-arm64-gnu': 1.3.0 + '@napi-rs/keyring-linux-arm64-musl': 1.3.0 + '@napi-rs/keyring-linux-riscv64-gnu': 1.3.0 + '@napi-rs/keyring-linux-x64-gnu': 1.3.0 + '@napi-rs/keyring-linux-x64-musl': 1.3.0 + '@napi-rs/keyring-win32-arm64-msvc': 1.3.0 + '@napi-rs/keyring-win32-ia32-msvc': 1.3.0 + '@napi-rs/keyring-win32-x64-msvc': 1.3.0 + '@napi-rs/wasm-runtime@1.1.4(@emnapi/core@1.10.0)(@emnapi/runtime@1.10.0)': dependencies: '@emnapi/core': 1.10.0 From 789ed25b2ca5d7f8469d6ed1de4f6dfca6c26958 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 13:24:12 +0800 Subject: [PATCH 02/22] fix(oauth): harden keyring migration, remove, and probe against races Three correctness fixes to KeyringTokenStorage/resolveTokenStorage: 1. Migration now re-reads the legacy token immediately before deleting the plaintext file. A concurrent file-backend writer could previously slip a newer token onto disk between our copy-in and our remove; we now let the latest observed value win so the keychain stays authoritative and the newer token is never dropped. 2. remove() now always clears the plaintext file even when the native keyring delete throws (permissions, lock state, ambiguous entries). The keyring error is re-thrown after legacy cleanup rather than leaving cleartext behind. A missing credential is still a no-op. 3. The capability probe now derives a unique account per attempt (probe--) and cleans it up in a finally, so concurrent probes can no longer clobber each other's sentinel and force a spurious file fallback on a healthy keychain. Adds four hermetic tests covering each fix; existing 12 keyring tests intact. --- packages/oauth/src/keyring-storage.ts | 60 +++++++++++++++++--- packages/oauth/test/keyring-storage.test.ts | Bin 7925 -> 14106 bytes 2 files changed, 53 insertions(+), 7 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 71c7bafa0..47af377ca 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -24,6 +24,7 @@ * pre-migration plaintext can never linger or go missing. */ +import { randomBytes } from 'node:crypto'; import { createRequire } from 'node:module'; import { FileTokenStorage } from './storage'; @@ -92,11 +93,32 @@ export class KeyringTokenStorage implements TokenStorage { } // Not in the keychain — migrate any plaintext token written by an older // file-only build, then drop the cleartext copy. - const legacyToken = await this.legacy.load(name); - if (legacyToken === undefined) return undefined; - this.keyring.createEntry(this.service, name).setPassword(this.serialize(legacyToken)); + // + // Re-read-before-delete guards against a concurrent file-backend writer + // (an old build, a KIMI_DISABLE_KEYRING process, or a fallback instance) + // saving a NEWER token between our copy-in and our remove. Without the + // re-read we would delete that newer file and leave a stale token shadowing + // it in the keychain forever. So we re-read immediately before deleting and + // let the latest observed value win. + const first = await this.legacy.load(name); + if (first === undefined) return undefined; + let serialized = this.serialize(first); + this.keyring.createEntry(this.service, name).setPassword(serialized); + + let latest = first; + const second = await this.legacy.load(name); + if (second !== undefined) { + const secondSerialized = this.serialize(second); + if (secondSerialized !== serialized) { + // A newer token landed on disk; make the keychain authoritative with it. + this.keyring.createEntry(this.service, name).setPassword(secondSerialized); + serialized = secondSerialized; + latest = second; + } + } + await this.legacy.remove(name); - return legacyToken; + return latest; } async save(name: string, token: TokenInfo): Promise { @@ -107,7 +129,20 @@ export class KeyringTokenStorage implements TokenStorage { // Clear both stores so a pre-migration plaintext copy can never linger // (e.g. logout before the token was ever migrated). Missing credentials // are a no-op, not an error. - this.keyring.createEntry(this.service, name).deleteCredential(); + // + // The legacy cleanup must ALWAYS run so a failing native keyring delete + // (permissions, lock state, ambiguous entries) can never leave the + // plaintext file behind — the "both stores cleared" guarantee must hold. + // A genuine keyring error is surfaced after the file is cleared, never + // swallowed. (`deleteCredential() === false` for a missing entry is normal, + // not an error.) + try { + this.keyring.createEntry(this.service, name).deleteCredential(); + } catch (error) { + // Keyring delete failed — still clear the plaintext copy, then re-throw. + await this.legacy.remove(name); + throw error; + } await this.legacy.remove(name); } @@ -155,15 +190,26 @@ function loadNativeKeyring(): KeyringApi | undefined { * usable on this host. */ function probeKeyring(keyring: KeyringApi): boolean { + // A UNIQUE account per attempt: two CLI processes probing concurrently must + // not share one sentinel account, or one's delete clobbers the other's + // round-trip → false mismatch → spurious file fallback on a healthy keychain + // (which then splits file/keyring state — the very thing migration avoids). + const account = `probe-${process.pid}-${randomBytes(8).toString('hex')}`; const sentinel = `probe-${Date.now()}-${Math.random().toString(36).slice(2)}`; + const entry = keyring.createEntry(KEYRING_PROBE_SERVICE, account); try { - const entry = keyring.createEntry(KEYRING_PROBE_SERVICE, 'probe'); entry.setPassword(sentinel); const readBack = entry.getPassword(); - entry.deleteCredential(); return readBack === sentinel; } catch { return false; + } finally { + // Always remove our own sentinel, even if the round-trip threw mid-way. + try { + entry.deleteCredential(); + } catch { + // best-effort cleanup; a failed delete must not mask the probe result + } } } diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 4b8b801165111981e888a464d377b4536738ae6e..5f3b061816f24e9a7c74b00e69d728c191456847 100644 GIT binary patch literal 14106 zcmc&*?QR>#742_7#RLezr9-Y{2S@=qN+4Nss`|605~ENBbGaOf8!dO4oh4~T5zt?K zfTB;BC+WHO&g{-EDOqxiHh>+OoVj!7&beP_MmrB5K2Q(UbGQb-cX_RDp{3-rLE2>cxW6iNjjEXXqWc`_*<;9vrO`=P!hE}zMc`{T%q+UjIlBCCK zv{pl-i)9k)e|*^8jp8_8WaY3Ms#B~wNmBe3r70~y(iR5#M54{`8NT(mHB|Xh7x=oI z&DA(5l*!dBxhSGC$un4Dk$ zjS)ZRN!I*+9+gwbsD(t6QWwWjJngBGo9?u&V# z)br!Nzdd>N!}~XHUmqU79~{5^>E!8g&-=pvXcJsCatpe858U>`aK{w!jzvkIYz$1+ z_9)kL4Qtlm=((Sdv$9xM%70AGm1;wwJ@uxg89pc3ByUe3)f(qgl2@JP?-D?ywLdrZ z!DNxejI|j+e>$5VB}LHP1Gd;nu3)Y>&rHeK+*fqc0X|cNSD+&3l0lUkUM91o?~F#r z*8lSAC@OWB<*T53jeoraB*G%f#`!GhhGjnB6@st7>4qkSD+Q0c*F%!leS*I#ga%lp z2ZI+nUKA!-YIt$6z)k#bKSTtBEcW1`vII$A&uRwO{$o_Z4%WfG;|Rwn#T z?W?P6?6>+v(Uz@P`cWzPw>E|c?5R#v_M=hUk!=@x0x(V8cSYI%u>RG42SCH_=AhhD zkKX@zclZ6S{E?X^&ga-l2Qg}_Rt1o5rcGVus zkVRn!0gY0|TX1ey_RPD?ld-q1h{qff-&1aa-Ki&btW#a;r-dE^8SpdO%qY)OjRfoV z+S-OpVZB>g0?PK}CP_9v5Db|1(N$nLJGV>7Db3-U$p;*i4+{o^I~$7-L+GN6j##=sHcg)T}x4%IVTJD}KID`Z$W zeT7&;b*Q8e(bB^k%2<@D&D?SYUub6$=MsnOffU~G)$JMO?A+xPml!`=)?tuy^%lq8Zqq>DZFcIiw-qP3IN$qlR(qe_DWS;g#tEtUzcC;-vM zdTv!IHtBDOs9KqZ-qJ_iP_6dJ<>F@PYNaJ*&?!*fplvIXIW>x00jgyZDHARAPMSyK zofN&>PJsq>i8>J;NG+RhHAz)6b072S>eudC+N&U1MQE_B2|^Mbbhvbn(f4(_RF@y> zAG0x%XQok%@P@JF#xzpPpVkwV@4eFTFALNNtn{sOv3Dd~s3sH6JdMw{YbciTb-Z6{xL$c>vL^8 z`)Q|spK}uk710WwVeyZu2C8#pLmAc5==x6;aE49mLWTh8sD_beKiXtCj|!s$TCJ{D z>mz1NNaJep(Kf1ZBuuQ-goxlD?1*GD?1Fh8;B$(|Ety;lPbAGBF{wnO@J^mk6E6X` z&)m?~7!H3nc?Q=LlLge=3U-v**+J+n(0r4lT3U$OSjh#l6W%kjc|KpH=x>ZWyBS;!lebhOlqA_^VshVn%h8fKcj(eGrrXZsD?o8vDv5- z<%TFqE~cf>TvY2gbf|#9fZCY`uGK}J>8_Y-yT5M>7VSvpd4Emj6}m6FNahnfa{7$u zWI%5esG!nypXdOTMO5G*5D2JIq?_9S`v~Xhhy5!m;F`2Wo{k}5ld_=CEuG>ZQa=8; zIB2(39DVM>A4B-SWl@eM@WrT9Q}95B%X@YDNP*#`6PYm9$wpvg@duRdJ5ai(kL>`) zFnq$u=>}4<>b^KOcEdIz(c2P71Z$Q<8?X77n^&4jmPTEq@k-LMWBbK6;{MEjCh{#MvA@3*ai!3s~7j9{U z6}?%1*>Ve}!+M;%h|ot-xG1MNq5=TDq*6g{12{k+JLDs37_i1NO2Lcz(cFvqkDFHJ!>G_@^Oqar4C$8>e%sQjCVHyQ zh@CE#95o{**0j<1zSYBHN3q!X_k)H@ERBNBR+k`ngl}P#N;Ifz5wSXt2>m3R`0gS|dP7 zRZP8N>&izSl=p1UMv|Klcf>4B@d>TDYsoj6EgHsjZS=u3~7rGKmJUykYc7r^RopUuyg6yGXrS^A!PS}&YYWFfQFtpd+#QtT2*j4Q1mgMDp#F?qxz6$kQ)}bH{$ze~b9aG}&w5K-LrHdn1YgZ6xZNb? z7ERDNa1rV*JI3((yClJ@;k~o_H!JxTMF=rz^@j$>~fJ=8GQ0nY?)^Tcdcw6V} zs$Qo+nG}cPk~Pr)T+mVV3v#zu%*(oIVWuD$=jiXUoZy5g+ueFJ)`4sugG|0t6;ZTQhf#=ud0(KIMj+b5MYICYgy+Ov)Ulo-d{%d z3w@qV34j71zto`!*dh+)Wj<;nY_lfl8^i{tmUj_vTpq36xcji0sqK0b$SBK99OOyNQg?hBWQJa^Q9 zuF^lMD#Udj;Yka{H+r*j#Xq?P?U`fHmecYYyowOD69myG8LmRJSihbG?X6p=<0VRx z@s*KVm#$D{n`l0(xYxBM%UyOPdx1vaf$lP%4XT$ijk;Rz)TJWXtyzFU z)8pH^G&eyw0`C^gT{cM<{HZ7J#NW{d)^dJuh4I*WC_E2Ph@q+^wu?(I+Au$Kw5#+u z>7&*dC+56$vzDcBQwd5PltlI#J` zr7=wB+51=Y&v<^gt=yGkb;t8RI(?ks8+JoHqEqNwzzCOAx@z2|(Az1i^^+>eZ@oxT zYh5y=*&Y|MQ|pZAtsERbB`Xe6l_pCd#0fyw~AZ1`*io*_%Ydc>jXpB!hD3EkK#i(EdcS!imnB@#iw^9oB&ZjKcFU%Eqrewuj^}E;S8Y5H z5HO8AS))&uWxcOb%sn&W^6UqI8?SIrLB6-^&T92rC%91Ziam#bfV%@^^AG41jdLq& z2Emy3jtZZHs+_1ZbikE#$njzY56AQz3@UW9-fyvJ8ZJNp9H23()WVb0xBI(Hg4Q%*_JVBg9yQ*1$sih~J69=pE2xJC-wv*XR$iqNBNA zzc{iW8(IJitCKkF7+WSfVXJ%rP_7a@(>}nL6)u1O*T0biNg1@v^stgXV15^SFtezP zr}7c)lWIV+v%rKRFWMe^vP7TYNt9CoCLl8DQtLS{NGni2iDDaP#lHMr)|FKC1czP#o zwp-CHDg+CvS|OMsgCJMr$w_?SfKx}=>gyU(#O~L2mH`5-5n}}il0|O-fsN(fvyQej zkrTq{t1F()sPFvL1$USF7JgG|O~uzdw)+f? zYxNQtVr<35&*-dC{1MgnmXWS&@j;ZKtD@gWLIm}Yg6ca`gTEZSJwB=qe*`~8g-XF) zLTJDdhC-Nqr(gq2bzgbEs-{}Lju=&abpNKyMPVd)5FOC8(tNy--!qX-;E5CR3nhXd zU!DBz#~SPOEY${DewG?JiP)umB4OpizJk$gwTYW0Vnba*OiKJp7Lpy*pvxxC z%Xr+1{}9n(P`rh}`zCTOG(D3E{#{qDxUzB&U2#Hee)_c*6^(G&gmku=CP)}bC5|1E qNDRo3J;!;9YMOq13lss`&oTYV8cHTQ<0&*Mr|UYuLMzJt^yj~q1=_O! delta 27 jcmbQ0_tkcT1JmY6ruQ71lX+X&H Date: Sun, 14 Jun 2026 13:45:05 +0800 Subject: [PATCH 03/22] feat(oauth): use keychain storage by default in the OAuth toolkit Wire resolveTokenStorage onto the KimiOAuthToolkit default storage path so the keychain backend (added in Task 1) is actually reachable at runtime, and export the new symbols from the package entry. - toolkit: default storage is now resolveTokenStorage(credentialsDir) instead of new FileTokenStorage(...); explicit options.storage injection is untouched. - index: export KeyringTokenStorage, resolveTokenStorage, and the KeyringApi / KeyringEntry types. - apps/kimi-code and packages/node-sdk both bundle the oauth package, so declare @napi-rs/keyring as a runtime dependency in each; otherwise the bundled createRequire("@napi-rs/keyring") fails to resolve in a consumer install and credentials silently fall back to plaintext. - vitest: force KIMI_DISABLE_KEYRING=1 for node-sdk and cli test projects so credential tests stay hermetic and never touch the developer's real OS keychain (the source alias resolves the native binary). - test: add a hermetic integration test proving the keyring store is reachable end-to-end through KimiOAuthToolkit (status/read/refresh/logout all hit a fake keychain, nothing lands in plaintext on disk), plus a default-path assertion that the factory is used when no storage injected. Note: the native/SEA build packaging of @napi-rs/keyring is intentionally out of scope (a separate later task). --- apps/kimi-code/package.json | 1 + apps/kimi-code/vitest.config.ts | 4 ++++ packages/node-sdk/package.json | 1 + packages/node-sdk/vitest.config.ts | 5 +++++ packages/oauth/src/index.ts | 3 +++ packages/oauth/src/toolkit.ts | 5 +++-- .../test/toolkit-keyring-integration.test.ts | Bin 0 -> 11241 bytes pnpm-lock.yaml | 6 ++++++ 8 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 packages/oauth/test/toolkit-keyring-integration.test.ts diff --git a/apps/kimi-code/package.json b/apps/kimi-code/package.json index 0305be0e5..a58acd4a5 100644 --- a/apps/kimi-code/package.json +++ b/apps/kimi-code/package.json @@ -75,6 +75,7 @@ }, "optionalDependencies": { "@mariozechner/clipboard": "^0.3.2", + "@napi-rs/keyring": "1.3.0", "chalk": "^5.4.1", "cli-highlight": "^2.1.11", "commander": "^13.1.0", diff --git a/apps/kimi-code/vitest.config.ts b/apps/kimi-code/vitest.config.ts index 350417bc2..1576ed88c 100644 --- a/apps/kimi-code/vitest.config.ts +++ b/apps/kimi-code/vitest.config.ts @@ -14,6 +14,10 @@ export default defineConfig({ name: 'cli', env: { KIMI_LOG_LEVEL: 'off', + // Keep credential tests hermetic: the OAuth toolkit now defaults to a + // keychain-backed store via resolveTokenStorage. Force the file backend + // so tests never read/write the developer's real OS keychain. + KIMI_DISABLE_KEYRING: '1', }, include: ['test/**/*.test.ts', 'test/**/*.test.tsx'], }, diff --git a/packages/node-sdk/package.json b/packages/node-sdk/package.json index 1d83cf528..5df0b220c 100644 --- a/packages/node-sdk/package.json +++ b/packages/node-sdk/package.json @@ -57,6 +57,7 @@ }, "dependencies": { "@antfu/utils": "^9.3.0", + "@napi-rs/keyring": "1.3.0", "smol-toml": "^1.6.1", "yazl": "^3.3.1", "zod": "^4.3.6" diff --git a/packages/node-sdk/vitest.config.ts b/packages/node-sdk/vitest.config.ts index 807e24f44..972fc2645 100644 --- a/packages/node-sdk/vitest.config.ts +++ b/packages/node-sdk/vitest.config.ts @@ -15,6 +15,11 @@ export default defineConfig({ name: 'kimi-sdk', env: { KIMI_LOG_LEVEL: 'off', + // Keep credential tests hermetic: the OAuth toolkit now defaults to a + // keychain-backed store via resolveTokenStorage, and the oauth source + // alias resolves the native @napi-rs/keyring binary. Force the file + // backend so tests never read/write the developer's real OS keychain. + KIMI_DISABLE_KEYRING: '1', }, include: ['test/**/*.test.ts'], }, diff --git a/packages/oauth/src/index.ts b/packages/oauth/src/index.ts index 2c515393d..597e2372b 100644 --- a/packages/oauth/src/index.ts +++ b/packages/oauth/src/index.ts @@ -20,6 +20,9 @@ export { tokenFromWire, tokenToWire } from './types'; export type { TokenStorage } from './storage'; export { FileTokenStorage } from './storage'; +export { KeyringTokenStorage, resolveTokenStorage } from './keyring-storage'; +export type { KeyringApi, KeyringEntry } from './keyring-storage'; + export type { DevicePollResult, RefreshOptions } from './oauth'; export { pollDeviceToken, refreshAccessToken, requestDeviceAuthorization } from './oauth'; diff --git a/packages/oauth/src/toolkit.ts b/packages/oauth/src/toolkit.ts index e5726b677..a6ce69905 100644 --- a/packages/oauth/src/toolkit.ts +++ b/packages/oauth/src/toolkit.ts @@ -24,8 +24,9 @@ import { type FetchManagedUsageError, type ParsedManagedUsage, } from './managed-usage'; +import { resolveTokenStorage } from './keyring-storage'; import { OAuthManager, type LoginOptions, type OAuthManagerOptions } from './oauth-manager'; -import { FileTokenStorage, type TokenStorage } from './storage'; +import type { TokenStorage } from './storage'; import type { OAuthFlowConfig } from './types'; export interface BearerTokenProvider { @@ -105,7 +106,7 @@ export class KimiOAuthToolkit { options.identity === undefined ? undefined : assertKimiHostIdentity(options.identity); this.homeDir = options.homeDir ?? defaultKimiHome(); const credentialsDir = options.credentialsDir ?? join(this.homeDir, 'credentials'); - this.storage = options.storage ?? new FileTokenStorage(credentialsDir); + this.storage = options.storage ?? resolveTokenStorage(credentialsDir); this.flowConfig = options.flowConfig ?? KIMI_CODE_FLOW_CONFIG; this.configAdapter = options.configAdapter; this.fetchImpl = options.fetchImpl; diff --git a/packages/oauth/test/toolkit-keyring-integration.test.ts b/packages/oauth/test/toolkit-keyring-integration.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..692f59fed70bec0ea02ef566b63346a95bfaae66 GIT binary patch literal 11241 zcmdT~VQ3#=8nvRG53N>Wk;*N6KN z_Y3cr+CwEJT;zFrlT<@h z&Wo{*jQaOK|E0$BG+n8QDW;}MqMkY#{4h9ERX&d(h^me5#6hRwYwIVF49Z*(0m6O43xtW~}FFMQOPjzE0Bi zo|J}0IjdYvM|oPRr729-sVL2Sn&ky%c0+Xm#Xzztt5T(TbfaJo&RQlseIKJaK^=tZ zs9rzHv$87Y5!YTO)kJCPlH+&fJTZ2d7e)nEm2wu!JR2On6KP6q*Q0_VF>?hyb3AvY zN~5P;6&K0ES&%kXYeYbnN|UjPRuND`5R{d!=B3&rm}2?g7)G1muQb2T=ameib`0#b zQc+=I14Ub_QJl=8Q?;5)5mDX?6bPn8@YMLjh# zV_@0-9h)*Ll9B1rgG^NQpwOOLB(<~!@KRPyQ9DXG{a>9A&VD$4Iq31x{oumKTVhLZ z$L}-@_sSyL^Pc9=u|9gWni*)(i9VVoJ^%M0tAO*z%DGjWv1D`Vy`{10Mp)jQ;sAbc zw-QE=vvGcJ0!~A#?TDMGi^2KD)$yyr$;I)-PwG(JLR<6F6i3&<@rNRh5smt)bAxTb zv0}3urAdb>Wk`lM;oW|Se|N<`R4M|lh_07-{qFR~tCy!IuaDoj1u{J~O`#$%*GN9! z^HT^pHt0uIBfky&g%ry!b<5VGG!?Ppi28| z_drxkcagyE-@2dmYxrmak2->-4)(A1_xG)?$|#@FtWHS9eO>@En|=)J zeD$@T9EM?iB#OI}L+?F(s*bbWshKi)tvwL=?8aDn6C>;)NxauHW)CDz{2T(n3+Vmt zpZ520v>fSv7&0bCDFUQ=jmb`yLXb?&6c8$#;;AE>384!e=NZV4joF9DOiQlzjYU$= zA?kSmqj47UDX_u41a21|bUiAad+`t2aHQJB;=?DmZsy%zp4_@QcSAx4I8e;1SYg1} z^Oz;vNoBm?020VExJ3Y^nj~e|*loQ_0ihr34=etlw4Ns=z*h6#E3B#A>;#`Xa+F3O!iFDUg`;f=qr~xuM=gp2UrH zt$56#_#QH+o#0N|5F4A+ROV$(V0f4jNTShML!7ghm1?KRzBhIWC_BrIlPo^67$`SQ zSA*g5W6Mf2cwh1fhHAn>z~F7;5Q0cxl?4HZ>hh!A1#+wkoV_h&hPmyIp@TB$cIyKr zV-?)s9l}ZZBiLtPm&ORcLO7e36ONIfW?zS?$*!wO*D3L?2@5~ZDgfZ6KTA|m@VFuY z`9kEgjLGvr@oa3(RxtA{1`QJ#^ zqDfnph0R?n;fxK29cQz7^?i{B3IFJ$rn*7;%j(UE7$S&l38k3~Q-_C#s$)^v>H7F4 zuLubax8oATBb`N@`2E?t_Q-Go4|s6+nfYZ79i zkaM|>f}D>G(ufc0*hNplb)5K)MS!QNLFXPV`^|FA*UjBdPX%HW8W16d+1=Zj8|EVi z$bJ*M{v90`l}q2}DnZP-eJJT?O{#uwOXkY~d5tL13Zq=@l-?tQ3J}j7Q0lcEj zmQ#3nOh*U`DM|)gF|k1Or9Vs7C5(zamnbTdaT1{#Td`YGmsn2OULNZeDcv~;fIwl$ zE=!TjN~Nz+h9YfB5eEi=%+HEWdMnJ^7~#ddy;D3<2%h$Cg4OKU!?8SBlBcx9pGR<|O>DuF+Z$S|lMhO9ULH>3cLZ zkUK0!@MaFh%tv_fCe26Kg@08@RrvtV?I|Rus7-p^7LqFHY*CFf_0|J0?9w!jwn^pL z^ftlr1qR3&T#G^)A<~$E8AsBlaSajtuCtDBo}k5n*vDOqao`&owURW|oK2*JPeK#o z%@!rwGF<>fpt3nYuv%=`xFi$^F3vLIg*kR{@pK$)5hwa@Xw3F!;P(iabO|8RC8km@ z5b)4Nu{{Xv)}~jqMSx(`LoR1AK&q8z}vLyVW6(Fz`BMmr}IcX#QDL*aJBYg|uU^F2=DBAs%3H;<@nMKmYu;WNjSZ z#d`Xh><(Q-wisRiv&HYMLpboY+`vOrl-we9UAl&~-ep4-A6+zZwts+BH7DB{4gW#i z0e0@oV(}vz*lDT=O)HvHh#qKyPOc}21#Bntwcqbn2YQ?lZ#Rl^yW%cLG`7@1bap5F ze|JYwAzDidga!0u!cDce-1G(T-d4ot+gUR}A{0#2j7=W-G-%e8f9;e^L3&LD#JY1xc zy6)2KTlh$fwD_^C9^DUc17z@V7P!FQCiv*YW=ZvJ-!tK3{Hhr&J>B`+mLfrX4faFf z+6dTT-RHa8)3anxe(nh-hR(FMGN=m)EFX??sPM0?YU00;`@EQC(afu^xC0bRl8MPj1T$B%)wNVtMpEjTDQD=c7~4qX zl7BltJwf_mftid~b++Lv>(*8pa*P9CZ9cbWj%+R580wz8t{O+m8EN#Zq}5!r?>72# z_Mx0cPP*Bq8mf1m2nrSZN9u!9pvQ-8Ge^lRtIwQjO1Rk{kq5c z7{P1ZvT^oc+@)AxkKEA`$DkuPf^XIE#IOcxeZu@xNyxgz`B)laT?|C#glQLD&_HzdokF&z@J7f$vTyPj2FMNYI6*DXgrgJ4-+@59FeZBm>S^K& zT!NOtfmbo27nAlz94vj5MDVJEV(Ye!B}JK`B)W<;?mThBiK|uo19@}+A;k=^CFm5I zZY$O_5^>oLacpRr&rxMgus!LN08Y3e0@=2V)r<3478@xm0MLd@tL;KB5y~(^+BwZ8 z=|(rF*SOB;h{LaYmKKvvbCB3YlfRMxaDo=5FKg_AE_pyLht|~ zv?c3?v1^??Z>?1eyL8QX7iT`!;m2ktA0XN`Zp>~)i_X32K4?5R-{vVska{w^^bd|- z2atNaZ}+=7fPs8i@Lp(i!7wsKJ?$1cawJOndrQC)S30~dul=K2S4|!TN%k|U8|Zxq z4;xAyj!v$@T_?CdQ!Z#3v->~3;Lc06&?#y164ArenU`fg;gS_S1$M%k6+UB9 zi;y-{>{X;If8;mVJz8!T6P=wFmYd#l{x9UVW33b1c<=&Pk5H1#D}%;_FUEK;F3#4t z`rgKoAIWk3QIAkQ1GzC03jELJt!>PQtt{T=Ywiac%S^@lX)xe4ZSoW6CBDi6je98^ zxy7OX2eGPdRIp26zdR7GKE(x+J&$1W59Q|nU)-C)*I6g9>BJjpT?*F~@8$l-Z+QuG z!OF%(8`g9{svu`;KJsdBnHR||nw*a4oHf<)%RF34%OXnUH}>{(qm2HnV@N}+9l3l4 zk<&g{SAI@0DFx+kbc+4Q8?L5reS-q8y!R0l$k4hO^FYrSS22rMw%O|2$?8}1=&@Fk znOH3OJ6ppIk;rc_BdbtcB`hvG+xoN}xo>jC^!5%S+x;!6p!4+>@g`bo#6W> zCv&LmhfeS(kS@=drO@PeV_kJ@OgVn6t#5Nqubq(0Fgt2qmpmpKfA}3uq~;;lBn%SdJ5u*u zp1(Q*iCpqaQubx-^%B~@V#bu BQTG4< literal 0 HcmV?d00001 diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b804ab2ef..62b29c3d1 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -121,6 +121,9 @@ importers: '@mariozechner/clipboard': specifier: ^0.3.2 version: 0.3.2 + '@napi-rs/keyring': + specifier: 1.3.0 + version: 1.3.0 chalk: specifier: ^5.4.1 version: 5.6.2 @@ -500,6 +503,9 @@ importers: '@antfu/utils': specifier: ^9.3.0 version: 9.3.0 + '@napi-rs/keyring': + specifier: 1.3.0 + version: 1.3.0 smol-toml: specifier: ^1.6.1 version: 1.6.1 From 4848f3f9f7b0bc01b9dec01d811dabccf4e2be3e Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 13:56:06 +0800 Subject: [PATCH 04/22] build(kimi-code): collect @napi-rs/keyring in the native SEA build Mirror the clipboard precedent so the native single-executable build keeps @napi-rs/keyring external and collects its host JS plus the per-target .node binary into the SEA asset blob: - tsdown.native.config.ts: add @napi-rs/keyring to optionalNativeDependencies (neverBundle, kept external like cpu-features). - native-deps.mjs: add keyringSubpackageByTarget map and keyring-host (js-only) + keyring-target (native-files) descriptors. - check-bundle.mjs: allow the external @napi-rs/keyring runtime require. The assets are now embedded and resolvable via the native-asset runtime (loadNativePackage). NOTE: the oauth loadNativeKeyring loader still uses a plain createRequire(import.meta.url)('@napi-rs/keyring') that is not routed through the native-asset runtime, so on machines without node_modules the SEA binary falls back to file storage. Wiring the loader to the runtime is a follow-up outside this task's scope. --- .../kimi-code/scripts/native/check-bundle.mjs | 2 +- apps/kimi-code/scripts/native/native-deps.mjs | 21 +++++++++++++++++++ apps/kimi-code/tsdown.native.config.ts | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/apps/kimi-code/scripts/native/check-bundle.mjs b/apps/kimi-code/scripts/native/check-bundle.mjs index 1521d6716..8e9578b72 100644 --- a/apps/kimi-code/scripts/native/check-bundle.mjs +++ b/apps/kimi-code/scripts/native/check-bundle.mjs @@ -23,7 +23,7 @@ const optionalRuntimeRequires = new Set([ 'utf-8-validate', ]); const optionalRelativeRuntimeRequires = new Set(['./crypto/build/Release/sshcrypto.node']); -const handledNativeRuntimeRequires = new Set(['koffi']); +const handledNativeRuntimeRequires = new Set(['koffi', '@napi-rs/keyring']); function isAllowedSpecifier(specifier) { if (builtins.has(specifier) || specifier.startsWith('node:')) return true; diff --git a/apps/kimi-code/scripts/native/native-deps.mjs b/apps/kimi-code/scripts/native/native-deps.mjs index f195a3cb1..bb95f982e 100644 --- a/apps/kimi-code/scripts/native/native-deps.mjs +++ b/apps/kimi-code/scripts/native/native-deps.mjs @@ -27,6 +27,15 @@ const clipboardSubpackageByTarget = Object.freeze({ 'win32-x64': '@mariozechner/clipboard-win32-x64-msvc', }); +const keyringSubpackageByTarget = Object.freeze({ + 'darwin-arm64': '@napi-rs/keyring-darwin-arm64', + 'darwin-x64': '@napi-rs/keyring-darwin-x64', + 'linux-arm64': '@napi-rs/keyring-linux-arm64-gnu', + 'linux-x64': '@napi-rs/keyring-linux-x64-gnu', + 'win32-arm64': '@napi-rs/keyring-win32-arm64-msvc', + 'win32-x64': '@napi-rs/keyring-win32-x64-msvc', +}); + const koffiTripletByTarget = Object.freeze({ 'darwin-arm64': 'darwin_arm64', 'darwin-x64': 'darwin_x64', @@ -68,6 +77,18 @@ export const nativeDeps = Object.freeze([ collect: 'native-files', parent: 'clipboard-host', }, + { + id: 'keyring-host', + name: () => '@napi-rs/keyring', + collect: 'js-only', + parent: null, + }, + { + id: 'keyring-target', + name: (target) => keyringSubpackageByTarget[target], + collect: 'native-files', + parent: 'keyring-host', + }, { id: 'pi-tui', name: () => '@earendil-works/pi-tui', diff --git a/apps/kimi-code/tsdown.native.config.ts b/apps/kimi-code/tsdown.native.config.ts index c1008cb61..f25f27dd6 100644 --- a/apps/kimi-code/tsdown.native.config.ts +++ b/apps/kimi-code/tsdown.native.config.ts @@ -16,7 +16,7 @@ const builtins = new Set([ ...builtinModules, ...builtinModules.map((name) => `node:${name}`), ]); -const optionalNativeDependencies = new Set(['cpu-features']); +const optionalNativeDependencies = new Set(['cpu-features', '@napi-rs/keyring']); function shouldAlwaysBundle(id: string): boolean { if (builtins.has(id) || id.startsWith('node:')) return false; From 1d8335a9dc0f8ab9a78cc3305937d88e413515a3 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 14:02:36 +0800 Subject: [PATCH 05/22] fix(kimi-code): route @napi-rs/keyring through the native-asset module hook The native module hook only intercepted `koffi`, so the oauth code's `createRequire(...)('@napi-rs/keyring')` resolved against on-disk node_modules (absent in an end-user SEA install) and threw, silently falling back to plaintext file storage. Generalize the hook to a small NATIVE_ASSET_PACKAGES set covering `koffi` and `@napi-rs/keyring`, and add keyring to the native asset smoke test. --- apps/kimi-code/src/native/module-hook.ts | 5 +++-- apps/kimi-code/src/native/smoke.ts | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/apps/kimi-code/src/native/module-hook.ts b/apps/kimi-code/src/native/module-hook.ts index bbef5d4cb..15b8c9311 100644 --- a/apps/kimi-code/src/native/module-hook.ts +++ b/apps/kimi-code/src/native/module-hook.ts @@ -9,6 +9,7 @@ interface ModuleWithLoad { } const nodeRequire = createRequire(import.meta.url); +const NATIVE_ASSET_PACKAGES = new Set(['koffi', '@napi-rs/keyring']); let installed = false; let loadingNativePackage = false; @@ -26,10 +27,10 @@ export function installNativeModuleHook(): void { parent: unknown, isMain: boolean, ): unknown { - if (request === 'koffi' && !loadingNativePackage) { + if (NATIVE_ASSET_PACKAGES.has(request) && !loadingNativePackage) { loadingNativePackage = true; try { - const pkg = loadNativePackage('koffi'); + const pkg = loadNativePackage(request); if (pkg !== null) return pkg; } finally { loadingNativePackage = false; diff --git a/apps/kimi-code/src/native/smoke.ts b/apps/kimi-code/src/native/smoke.ts index 56d39253f..fbf09982a 100644 --- a/apps/kimi-code/src/native/smoke.ts +++ b/apps/kimi-code/src/native/smoke.ts @@ -1,6 +1,6 @@ import { getEmbeddedNativeAssetManifest, getNativePackageRoot } from './native-assets'; -const smokePackages = ['@mariozechner/clipboard', 'koffi']; +const smokePackages = ['@mariozechner/clipboard', 'koffi', '@napi-rs/keyring']; export function runNativeAssetSmokeIfRequested(): boolean { if (process.env['KIMI_CODE_NATIVE_ASSET_SMOKE'] !== '1') return false; From 5e2901e3dfb7f980df36f3b6dd2d7a386732299c Mon Sep 17 00:00:00 2001 From: LongYinan Date: Wed, 17 Jun 2026 22:05:12 +0800 Subject: [PATCH 06/22] build(nix): update fetchPnpmDeps hash for @napi-rs/keyring --- flake.nix | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/flake.nix b/flake.nix index 88a79f610..68e5e3378 100644 --- a/flake.nix +++ b/flake.nix @@ -150,7 +150,7 @@ inherit (finalAttrs) pname version src pnpmWorkspaces; inherit pnpm; fetcherVersion = 3; - hash = "sha256-u+u5Vm6UgrMW/SwiBoSz2WhKp8GOehk4p6euwlinwFI="; + hash = "sha256-XbJ8lTNywbZ+saZ33A7qAa9V3JHYgPBM1Rh2ySenvxs="; }; nativeBuildInputs = [ From 7caa1a59ef50453da562eba3eb1da57aef895469 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 16:10:24 +0800 Subject: [PATCH 07/22] fix(oauth): align keychain storage with file backend (per-profile namespace, safe migration delete) The keyring backend now namespaces the keychain service per credentials directory (keyringServiceForCredentialsDir), matching the file backend's per-directory isolation, and migration uses a lock-free compare-and-delete loop that only unlinks a plaintext file whose bytes still match the value made keychain-authoritative. Align the tests with the namespaced service and the converge-loop semantics: - toolkit-keyring-integration.test.ts: storage is built via resolveTokenStorage(dir, ...), so derive the service from the dir via keyringServiceForCredentialsDir instead of the bare KEYRING_SERVICE constant (tmp dirs hash to kimi-code-). keychainTokenNames now takes the service explicitly. (status / refresh / logout assertions.) - keyring-storage.test.ts: the "selects KeyringTokenStorage when probe succeeds" test inspects the resolveTokenStorage-built service, so query keyringServiceForCredentialsDir(dir). Direct new KeyringTokenStorage({...}) tests keep KEYRING_SERVICE (the default service). - keyring-storage.test.ts: rework the compare-and-delete race test so its fake legacy store returns a DIFFERENT token on every read (persistent churn). The file then never stabilises within the bounded converge budget, so the loop exhausts its budget WITHOUT deleting the file (removeCalls === 0) and the keychain holds the newest observed value. The previous harness returned a fixed newer token forever, which the loop legitimately re-migrates and then deletes once keychain == file (no data loss), so its "NOT deleted" expectation contradicted the intended converge behavior. No production logic changed for the race: at the moment legacy.remove() runs, the keychain already holds the exact bytes on disk, so deletion is safe. --- packages/oauth/src/keyring-storage.ts | 110 +++++++++++++----- packages/oauth/src/types.ts | 2 +- packages/oauth/test/keyring-storage.test.ts | Bin 14106 -> 16500 bytes .../test/toolkit-keyring-integration.test.ts | Bin 11241 -> 11547 bytes 4 files changed, 85 insertions(+), 27 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 47af377ca..66b61a2be 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -2,10 +2,12 @@ * Keychain-backed OAuth token storage with plaintext-file fallback. * * Backend: the OS keychain (macOS Keychain, Windows Credential Manager, - * Linux Secret Service) via `@napi-rs/keyring`. All tokens live under a single - * keychain *service* (`KEYRING_SERVICE`); each token is one entry whose - * *account* is the token `name` and whose *password* is the snake_case wire - * JSON — the exact same payload `FileTokenStorage` writes to disk. + * Linux Secret Service) via `@napi-rs/keyring`. Tokens live under a keychain + * *service* derived per credentials directory (`keyringServiceForCredentialsDir`) + * so distinct profiles / SDK callers stay isolated exactly like the file backend + * isolates them by directory; each token is one entry whose *account* is the + * token `name` and whose *password* is the snake_case wire JSON — the exact same + * payload `FileTokenStorage` writes to disk. * * Selection (`resolveTokenStorage`): the keychain is used only when it is * actually usable. Two guards are required because the failure modes differ: @@ -19,13 +21,18 @@ * * Migration: when the keychain is selected but a token is still only on disk * (written by an older file-only build), `load` migrates it — copy into the - * keychain, then delete the plaintext file — so secrets stop living in the - * clear. `remove` and `list` also reconcile against the legacy file store so - * pre-migration plaintext can never linger or go missing. + * keychain, then compare-and-delete the plaintext file (only unlink a file that + * still matches the value we made keychain-authoritative) — so secrets stop + * living in the clear without ever dropping a newer token a concurrent + * file-backend writer may have just landed. Migration is LOCK-FREE, exactly like + * `FileTokenStorage`. `remove` and `list` also reconcile against the legacy file + * store so pre-migration plaintext can never linger or go missing. */ -import { randomBytes } from 'node:crypto'; +import { createHash, randomBytes } from 'node:crypto'; import { createRequire } from 'node:module'; +import { homedir } from 'node:os'; +import { join, resolve } from 'node:path'; import { FileTokenStorage } from './storage'; import type { TokenStorage } from './storage'; @@ -92,32 +99,56 @@ export class KeyringTokenStorage implements TokenStorage { return this.deserialize(raw); } // Not in the keychain — migrate any plaintext token written by an older - // file-only build, then drop the cleartext copy. + // file-only build, then drop the cleartext copy (LOCK-FREE, exactly like + // FileTokenStorage — no proper-lockfile, because reusing the oauth-manager + // refresh lock here would deadlock the manager's in-lock re-read, and a + // separate lock wouldn't coordinate with an old file-backend process). // - // Re-read-before-delete guards against a concurrent file-backend writer - // (an old build, a KIMI_DISABLE_KEYRING process, or a fallback instance) - // saving a NEWER token between our copy-in and our remove. Without the - // re-read we would delete that newer file and leave a stale token shadowing - // it in the keychain forever. So we re-read immediately before deleting and - // let the latest observed value win. + // Compare-and-delete guards against a concurrent file-backend writer (an old + // build, a KIMI_DISABLE_KEYRING process, or a fallback instance) saving a + // NEWER token between our copy-in and our remove. We never unlink a file + // whose serialized value differs from the one we just made authoritative in + // the keychain — only a token we actually migrated is deleted. const first = await this.legacy.load(name); if (first === undefined) return undefined; let serialized = this.serialize(first); + let latest = first; this.keyring.createEntry(this.service, name).setPassword(serialized); - let latest = first; - const second = await this.legacy.load(name); - if (second !== undefined) { - const secondSerialized = this.serialize(second); - if (secondSerialized !== serialized) { - // A newer token landed on disk; make the keychain authoritative with it. - this.keyring.createEntry(this.service, name).setPassword(secondSerialized); - serialized = secondSerialized; - latest = second; + // Bounded converge loop: ensure the keychain holds the latest observed + // serialized value `S`, then re-read the file ONE more time right before + // deleting and only unlink when it still equals `S`. A newer value found on + // the pre-delete re-read is written to keychain and we retry; persistent + // churn after a few iterations leaves the file in place (a later load + // reconciles) rather than risk deleting a token we didn't migrate. + // + // NOTE: the residual sub-microsecond TOCTOU under concurrently-MIXED + // file+keyring backends is a documented best-effort limitation; running the + // file and keyring backends simultaneously against one credentialsDir is + // unsupported. + for (let i = 0; i < 3; i += 1) { + const current = await this.legacy.load(name); + if (current === undefined) { + // A peer already removed/migrated the file — nothing to delete, the + // keychain already holds the latest value we observed. + return latest; } + const currentSerialized = this.serialize(current); + if (currentSerialized === serialized) { + // The file still matches what we migrated → safe to drop the cleartext. + await this.legacy.remove(name); + return latest; + } + // A newer token landed; make the keychain authoritative with it and retry + // the compare-and-delete against this newer value. + this.keyring.createEntry(this.service, name).setPassword(currentSerialized); + serialized = currentSerialized; + latest = current; } - await this.legacy.remove(name); + // Converge budget exhausted under persistent churn: leave the file in place + // (never delete a token we may not have migrated) and return the latest + // value we wrote to the keychain; a subsequent load will reconcile. return latest; } @@ -213,6 +244,27 @@ function probeKeyring(keyring: KeyringApi): boolean { } } +/** + * Derive the keychain *service* name for a credentials directory so that the + * keyring backend isolates profiles by directory exactly like the file backend + * isolates them by `credentialsDir`. Without this, every profile / SDK caller + * collides on one fixed `'kimi-code'` service — a data-loss regression vs the + * file store. + * + * The "default" detection is deliberately keyed off the STANDARD path + * (`~/.kimi-code/credentials`), NOT `defaultKimiHome()` / `KIMI_CODE_HOME`: + * two different `KIMI_CODE_HOME` values would both look "default" and re-collide + * on `'kimi-code'`. Hashing the actual resolved dir guarantees isolation for + * every non-standard dir, while the one standard home per OS user keeps a + * stable, human-readable `'kimi-code'` service for the common case. + */ +export function keyringServiceForCredentialsDir(credentialsDir: string): string { + const resolved = resolve(credentialsDir); + const standard = resolve(join(homedir(), '.kimi-code', 'credentials')); + if (resolved === standard) return KEYRING_SERVICE; + return `kimi-code-${createHash('sha256').update(resolved).digest('hex').slice(0, 16)}`; +} + interface ResolveTokenStorageDeps { /** Returns a usable KeyringApi, or undefined when the native load fails. */ loadKeyring?: () => KeyringApi | undefined; @@ -240,5 +292,11 @@ export function resolveTokenStorage( if (!probeKeyring(keyring)) return legacy; - return new KeyringTokenStorage({ keyring, legacy }); + // Namespace the keychain service by credentialsDir so distinct profiles / + // SDK callers stay isolated, matching the file backend's per-directory + // isolation. The legacy file store and the derived service both come from the + // SAME credentialsDir, so a file at `/.json` migrates + // into the matching namespaced service. + const service = keyringServiceForCredentialsDir(credentialsDir); + return new KeyringTokenStorage({ keyring, legacy, service }); } diff --git a/packages/oauth/src/types.ts b/packages/oauth/src/types.ts index 19d3cc0d6..9073f8614 100644 --- a/packages/oauth/src/types.ts +++ b/packages/oauth/src/types.ts @@ -8,7 +8,7 @@ * contract; in-process types use camelCase per TS convention. */ -export type OAuthStorageBackend = 'file' | 'keyring'; +export type OAuthStorageBackend = 'file'; /** A persisted OAuth token bundle. */ export interface TokenInfo { diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 5f3b061816f24e9a7c74b00e69d728c191456847..6874c4dff9eb530755195b4acf4a104a99fba002 100644 GIT binary patch delta 2490 zcmcguO>Z1U5LGM^P((l`@kb0HB|!3SvL9>`E0UcM<--w{APKQU4k5z5GqpR7XQ$WQ zJzlR+v^jAAaiJq2amx`RAy9q+e<5cs$!~}_Q`J3VFUAr#e6i)NdR_IZ-mCp7d-?Fd zZ@(OPEm1v;rLMW34?H`$fkQ2`?kegn8R2!MuWF1jvr?qyn$%AR6Ps^ciBzvIG}c8H z*JDi4;vxv>$P^*ZV${$_ZKSc32T_vi45Wb@%bzdbh7caR=H%3<*@VlGVpl{%Fji@V zL81)8mPm8tlxr*3uP@(TzOe=>1KdJA1dSqw7Y`pnPZ3l~rGhvN^h02g%A#CrewAyf zw?(vpSquX$Efr-GVPONYZy=@0<%n~kqtPV%siIhZCzfWTL~NrSApJYMtuh4F0hMbi zqt2C>8W=^u^#82RiAC)hCs0qKEmLXG5PAvO6E;eG%82m>2w&yKLQlj<#SNoG$gD1M zn<%PUShF2xul9lI*@5ftnvtHbd56kQ{D<9*2*lhfm|Xp5t*-#`|cL!_Ms+c5YADorQhF z7Iwq#k2jPOjlLObBQ)0#wo-3ZL73exm)x`I)g%49oyv%4>ZvVs$7ViT2&4p`7|&;k zGYQrd7`kJyTX)@h%;%(<3Q;z6y| z8j#oSG%8F97^LDQ716LT9#;q(0|{k(v?QeA?r5P7t-E{X>ydH0kCCnL4zSBuwY-^& zv=RUdNfj|3acq(teFZBYx^E}nb}x^cSBoTOe%%{G)-f09a~%cJ7o z@6+Kb9K1MTbb06?5%`O5PW}K!bddB&XPobVPl-|#+TyltxT{mm^P#z|7i!pBXbSgjVV(lz(}_6vcV%M*M@+RHQKtR})A=o5Va9VkKymx=<-n0)iVe$-7O4PG-u?yu>V| z(J$cQii@sXE25a4D>p7&iC@5lAHbDs@xGV-!P(3_-kEcL=gyx=u&+P94QLwtp!F-6 z>F2q}=k5(;&k8S%28&gTB685dZDSE4ROqGdt$~29(cRcu)LxCNBP+c}4H`#i!J8dE zkEEvGjU2s(;=EB2eSz~AcrU@T%8ir2iyfQ?A3~G%mtdanCYn9oXPQ)0ufsYh?1}Ce zoKGNpUwG&XonVRX4zwD;sJ@S|EiD@+#K6CqadYF*#ul`~1Hqx8LynH1oZQ*vj%h7C zXuB6x;Me38I5~(Fk3skTE6X7Yq3y&F+lPLN(-K_={Rp|ai7l7g9ap7JL6UEa12e=L zv}cDl^rd$xHS?;}wU+wPVs?`z*<18CTg>~ubTwr}bVn%VpxJCf18#zEz#0#ZE|s$M z_2RWKEn=Ln9zWf2j&R`20NIKWy@9SmVQD2-pc9uL(wp3i;69a>S00#-Lp#I>hC;;% zZo%u|4>R47O-Qb;wk_@oUw9K4Tg`~vf?9{vz zO&x{ee6T3Yn9O2@l++@ilTsAYit=+65>hgY5_A+C^YTj|=7B6vN-RoM@OF(1^7M0$ z4|WX-^K^DqNY2kIE=kNQ(F3Z>NiBgn%u1oS1QZC9#RW`sVYb*33K|WdwVPc8cC&E; YT?h?}|6)cQ@PKiU7e|&0SDnNL07|Qc82|tP delta 124 zcmbOo^)h?|7ptmkWRRzydwj5KP?)E)s}7fff_G|VQD$CxNPc!|UT{f%QDS=PW_#A- z>?}p8i8+(Gd2J_0^6Kz=qiWpT!@HbmGOwV~=3>DuY?}=vj5siqWGaZWZSGW?#0CK7 CSSNS@ From 5d589d98bfcc77b9c3838420c8a624465fd08ca1 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 16:37:24 +0800 Subject: [PATCH 08/22] fix(oauth): reconcile newer plaintext token on keychain hit (sequential fallback) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit KeyringTokenStorage.load() early-returned the keychain value on any keychain hit, consulting the legacy file only on a miss. A per-run backend flip-flop (keychain locked / headless / KIMI_DISABLE_KEYRING=1 / probe fail) could leave an OLDER token in the keychain while a fallback run wrote a NEWER token to the plaintext file, so a later keychain-usable run silently ignored the user's valid newer token — and could overwrite the keychain with a revoked tombstone built from the stale value. load() now reconciles against the legacy file on the HIT path too, with a conservative rule: adopt the file token (and make the keychain authoritative, then compare-and-delete the cleartext) ONLY when BOTH sides are valid and the file is strictly newer by expiresAt. A deliberate revoked tombstone is never un-revoked from stale plaintext; a redundant byte-identical plaintext copy is pruned, otherwise the file is left in place. Lock-free, exactly like FileTokenStorage; the keychain-MISS migration loop is untouched. --- packages/oauth/src/keyring-storage.ts | 87 +++++++++++++++++++- packages/oauth/test/keyring-storage.test.ts | Bin 16500 -> 22311 bytes 2 files changed, 86 insertions(+), 1 deletion(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 66b61a2be..065aa6be3 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -27,6 +27,17 @@ * file-backend writer may have just landed. Migration is LOCK-FREE, exactly like * `FileTokenStorage`. `remove` and `list` also reconcile against the legacy file * store so pre-migration plaintext can never linger or go missing. + * + * Reconcile-on-hit (flip-flop repair): `resolveTokenStorage` can pick a + * DIFFERENT backend per run for one credentialsDir (keychain locked, + * headless/SSH, `KIMI_DISABLE_KEYRING=1`, native binary missing, probe fails). + * A sequential flip-flop then splits state — the keychain may hold an OLDER + * token while a fallback run wrote a NEWER one to the plaintext file. So `load` + * reconciles against the legacy file even on a keychain HIT, adopting the file + * token ONLY when BOTH sides are valid (neither a tombstone) AND the file is + * strictly newer (`expiresAt`). It NEVER un-revokes a deliberate tombstone from + * stale plaintext, and only prunes a plaintext copy it made authoritative or one + * byte-identical to the authoritative keychain value. See `reconcileOnHit`. */ import { createHash, randomBytes } from 'node:crypto'; @@ -36,6 +47,7 @@ import { join, resolve } from 'node:path'; import { FileTokenStorage } from './storage'; import type { TokenStorage } from './storage'; +import { classifyToken } from './token-state'; import type { TokenInfo, TokenInfoWire } from './types'; import { tokenFromWire, tokenToWire } from './types'; import { isRecord } from './utils'; @@ -96,7 +108,7 @@ export class KeyringTokenStorage implements TokenStorage { async load(name: string): Promise { const raw = this.keyring.createEntry(this.service, name).getPassword(); if (raw !== null) { - return this.deserialize(raw); + return this.reconcileOnHit(name, raw); } // Not in the keychain — migrate any plaintext token written by an older // file-only build, then drop the cleartext copy (LOCK-FREE, exactly like @@ -152,6 +164,79 @@ export class KeyringTokenStorage implements TokenStorage { return latest; } + /** + * Reconcile a keychain HIT against the legacy plaintext file. + * + * The keychain backend must be a faithful drop-in for the single-store file + * backend, but `resolveTokenStorage` can pick a DIFFERENT backend per run for + * one credentialsDir (keychain locked, headless/SSH, KIMI_DISABLE_KEYRING=1, + * native binary missing, probe fails). A flip-flop then splits state: the + * keychain may hold an OLDER token while a fallback run wrote a NEWER one to + * the plaintext file. Returning the keychain value blindly would silently + * ignore the user's real, newer token — and if the older token's refresh_token + * is now rejected, the manager would re-read it and overwrite the keychain + * with a revoked tombstone while the valid file token sits ignored → forced + * re-login despite a valid token. So we reconcile on the HIT path too. + * + * Invariant — NEVER un-revoke from plaintext: a deliberately-written revoked + * tombstone (refresh_token rejected) must outrank any stale plaintext token. + * We therefore adopt the file token ONLY when BOTH sides are VALID (neither is + * a tombstone) AND the file is STRICTLY newer (`expiresAt` is stamped at + * issuance time, so a larger value means more-recently issued). In every other + * case the keychain stays authoritative. + */ + private async reconcileOnHit(name: string, raw: string): Promise { + const keyringToken = this.deserialize(raw); + const fileToken = await this.legacy.load(name); + + // FAST PATH: steady state has no plaintext file (one cheap readFile that + // ENOENTs), or the keychain bytes were corrupt JSON (must still return + // undefined per the existing contract). Either way, nothing to reconcile. + if (fileToken === undefined || keyringToken === undefined) { + return keyringToken; + } + + // Adopt the file token ONLY when both are valid (not tombstones) and the file + // is strictly newer. This is the flip-flop repair: a fallback run landed a + // newer token on disk; make it keychain-authoritative now. + if ( + classifyToken(keyringToken).kind === 'valid' && + classifyToken(fileToken).kind === 'valid' && + fileToken.expiresAt > keyringToken.expiresAt + ) { + const fileSerialized = this.serialize(fileToken); + this.keyring.createEntry(this.service, name).setPassword(fileSerialized); + // Compare-and-delete: re-read and unlink ONLY if the on-disk bytes still + // equal what we just made authoritative — never delete a token whose bytes + // changed under a concurrent writer. + await this.removeIfBytesMatch(name, fileSerialized); + return fileToken; + } + + // Keychain stays authoritative (keyring newer/equal, or EITHER side is a + // tombstone — the no-un-revoke invariant). As cleanup, prune the plaintext + // ONLY when it is byte-identical to the authoritative keychain value; a file + // whose bytes differ is left in place (conservative — we never delete a + // token we did not make authoritative). + if (this.serialize(fileToken) === raw) { + await this.removeIfBytesMatch(name, raw); + } + return keyringToken; + } + + /** + * Compare-and-delete the plaintext copy: re-read the file and unlink it ONLY + * when its serialized bytes still equal `expected` (the value we made + * keychain-authoritative). A concurrent file-backend writer that landed a + * different token between our decision and this delete is left untouched. + */ + private async removeIfBytesMatch(name: string, expected: string): Promise { + const current = await this.legacy.load(name); + if (current !== undefined && this.serialize(current) === expected) { + await this.legacy.remove(name); + } + } + async save(name: string, token: TokenInfo): Promise { this.keyring.createEntry(this.service, name).setPassword(this.serialize(token)); } diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 6874c4dff9eb530755195b4acf4a104a99fba002..8dd47193afe75112e845e3b6407b1c820b5ef4f5 100644 GIT binary patch delta 3522 zcmcIn&u`pB6jo>@q_j|}khZBx`yiFFZg)57FNCs+&@M^U3P>axLDWk$v1hYxyq>K+ z&Sq6rTkdc&C&V2N2oQ)ncLWlD0fGZJ#DzP2Z|rgG?N;JK4$;b(!8t@YoWi} zI`$>)?8}kc6Mjf{?rveTt3uZg?NDUi(e=kBc zyiyxnH5fI`V)PCvnYOfPm2j^?APp>PkJ2yZPtUZ|A877Md$CUPaNtKWZtE4gw!FMd zkDbGsv~QQIV2bJ z3qS$@q*}g|04*aGcf+o7n!P60{qW5*V=1FLsR;EtI+IUw3VugYCs7!i_{9ZG zwQQ+4j{zndh{7l=1pQGC!8*rwZOncxvmOpx38VFBtU-be*~!#+>OFEzX2~zB>I#8JDnx9ikzo-ZZ6o4k zAF@MYTp0;~C;L!i+8`AZeag+-R1_JQ8Z(65*qWOPGhQT|l8nkyZir)yPx{&X#sA0b zf`t$JynuOS%7)OW8E8$?VMq42aTH;Z>Q_vdgsX^lU#^(A4f9weFHh(iftJ$N2p*^KAF3q;g4&*9zoG-&db)J7rXw~%_ YH`za1t(rN6rmW?&d8*cYds$!j7ky8vo&W#< delta 23 fcmZ3!j`2$a Date: Sun, 14 Jun 2026 16:46:06 +0800 Subject: [PATCH 09/22] docs(oauth): clarify keychain reconcile comments + pin strict-newer tie test Correct the 'byte-identical' wording in keyring-storage.ts to 'equal to the keychain value after canonical re-serialization' (the cleanup compares serialize(fileToken) === raw, so reordered keys / extra fields would not match and are conservatively left). Document that a file left by the conservative keychain-wins branch persists until the next explicit remove()/logout, and note that removeIfBytesMatch's re-read is the compare-and-delete guard, not redundant I/O. Rename the duplicate-cleanup test to match the wording, and add a regression test pinning the strict > (not >=) adoption: equal expiresAt with differing bytes leaves the keychain authoritative and the file intact. --- packages/oauth/src/keyring-storage.ts | 19 +++++++++++++++---- packages/oauth/test/keyring-storage.test.ts | Bin 22311 -> 23708 bytes 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 065aa6be3..641e88cbd 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -37,7 +37,8 @@ * token ONLY when BOTH sides are valid (neither a tombstone) AND the file is * strictly newer (`expiresAt`). It NEVER un-revokes a deliberate tombstone from * stale plaintext, and only prunes a plaintext copy it made authoritative or one - * byte-identical to the authoritative keychain value. See `reconcileOnHit`. + * equal to the keychain value after canonical re-serialization. See + * `reconcileOnHit`. */ import { createHash, randomBytes } from 'node:crypto'; @@ -215,9 +216,16 @@ export class KeyringTokenStorage implements TokenStorage { // Keychain stays authoritative (keyring newer/equal, or EITHER side is a // tombstone — the no-un-revoke invariant). As cleanup, prune the plaintext - // ONLY when it is byte-identical to the authoritative keychain value; a file - // whose bytes differ is left in place (conservative — we never delete a - // token we did not make authoritative). + // ONLY when it is equal to the authoritative keychain value after canonical + // re-serialization (a redundant duplicate); a file whose serialized form + // differs — reordered keys, extra fields, or a genuinely different token — is + // left in place (conservative — we never delete a token we did not make + // authoritative). + // + // A file left here is intentional and persists: no future `load` cleans it + // up (this branch only prunes the redundant-duplicate case), so it lingers + // until the next explicit `remove()` / logout. Deliberate — we never delete a + // token we did not make authoritative. if (this.serialize(fileToken) === raw) { await this.removeIfBytesMatch(name, raw); } @@ -231,6 +239,9 @@ export class KeyringTokenStorage implements TokenStorage { * different token between our decision and this delete is left untouched. */ private async removeIfBytesMatch(name: string, expected: string): Promise { + // The re-read is the compare-and-delete guard, not redundant I/O: it catches + // a concurrent file-backend writer that landed a newer token between our + // decision and this delete, so we only unlink bytes that still match. const current = await this.legacy.load(name); if (current !== undefined && this.serialize(current) === expected) { await this.legacy.remove(name); diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 8dd47193afe75112e845e3b6407b1c820b5ef4f5..4b362f812ebc584ffcbfb4789d190aaac9901d06 100644 GIT binary patch delta 883 zcmah|O=}ZD7$%?wN>L>CQUzZ`F^k)7V*Lmy3ZW)Yv@s+B(_6;fi5WVbS!ZX{u9tcc zPrZ!?4_-WZQ=unMBI3;-A^rgU3%--hZVDpi^k(;+XP&Qp_rCDvQ{kiMZG@QIKDA9GN2C8CNE&Yls?q%d#KE}75HqIuzcE2d{6 zGrX~E!pfQdAAVW+T2eHKML@){yN@1KUl(dA0lFVDMWcoW7D4-AA0n-ouXzH3?os8! z4o&Tr6;+lKVv48ZK&emagcL-%6H4b-%4v;{GmOcd3XAutdInG$}Ac6@> z3}CPT`13rAJB1rDP9fbOaBs8KYId9L{%m_2Iq0$7JAhtes|h*zRp>F{Q~N4IoFbtn zORCrJZf^7+Y-gG*@myD=NrJ*+J(h}TbbUfy;F1IwhK`kpGQ@$K)9eOpG{T(36Ugr2 zbc7xXn>7+C`%++#lPMhheEw?6gCOKAjSMs|ggJftqB*QMMI>f{O<{Q{pTAem7cHWO z#C)k;I$zZqd)%CT+S-0L!8UF35mOE6q5RqLhW7Y!GDYeXx z3#ZJ7YI_UM-p|s1oM>BRWfNzd!vM+{2T3~;M1& delta 133 zcmbQUlX3Yv#tr)VCW#73l_jaVnJKAxC7H>IISNIoDW!QSiFqXo1v!bCc_pb8B?`&; z1(gb!#R@5@IjJS7DU%lpica3I$uoJf{!!jyumQyixrrso8I$7;G}%)t5|c}EDkp2& Y=}*3HFoh4*g3Wb?GZ;6gSXih407u(0egFUf From b6172566e5f0b7efa436bdf42c56b93336abaf93 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 17:00:57 +0800 Subject: [PATCH 10/22] fix(oauth): compare token issuance time, not expiry, in keychain reconcile The keychain HIT reconcile guard used `expiresAt` as a write-order proxy to decide whether to adopt a newer plaintext token. But `expiresAt` is an EXPIRATION time (mint second + `expiresIn`), so a server returning a shorter `expires_in` on a later refresh can mint a NEWER token B whose `expiresAt` is LESS than an OLDER, longer-lived token A. The guard then kept returning stale A and never adopted B, resurrecting the split-state failure (a 401 on A's rotated-out refresh token tombstones the keychain over the valid, ignored B). Compare issuance order instead: `issuedAt = expiresAt - expiresIn` recovers the mint second (the `expiresIn` cancels out), so the comparison is robust to a variable `expires_in` and needs NO new wire field. Strict `>` keeps the keychain authoritative on a same-second tie (1s granularity; practically unreachable). Everything else (both-valid precondition, compare-and-delete adoption, duplicate cleanup, lock-free design, MISS migration converge loop) is unchanged. --- packages/oauth/src/keyring-storage.ts | 40 ++++++++++++++++---- packages/oauth/test/keyring-storage.test.ts | Bin 23708 -> 25415 bytes 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 641e88cbd..de0af5a7a 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -34,8 +34,9 @@ * A sequential flip-flop then splits state — the keychain may hold an OLDER * token while a fallback run wrote a NEWER one to the plaintext file. So `load` * reconciles against the legacy file even on a keychain HIT, adopting the file - * token ONLY when BOTH sides are valid (neither a tombstone) AND the file is - * strictly newer (`expiresAt`). It NEVER un-revokes a deliberate tombstone from + * token ONLY when BOTH sides are valid (neither a tombstone) AND the file was + * issued strictly later (mint second `expiresAt - expiresIn`, not the expiration + * time `expiresAt`). It NEVER un-revokes a deliberate tombstone from * stale plaintext, and only prunes a plaintext copy it made authoritative or one * equal to the keychain value after canonical re-serialization. See * `reconcileOnHit`. @@ -182,9 +183,19 @@ export class KeyringTokenStorage implements TokenStorage { * Invariant — NEVER un-revoke from plaintext: a deliberately-written revoked * tombstone (refresh_token rejected) must outrank any stale plaintext token. * We therefore adopt the file token ONLY when BOTH sides are VALID (neither is - * a tombstone) AND the file is STRICTLY newer (`expiresAt` is stamped at - * issuance time, so a larger value means more-recently issued). In every other - * case the keychain stays authoritative. + * a tombstone) AND the file was ISSUED strictly later. `expiresAt` is an + * EXPIRATION time (mint second + `expiresIn`), NOT a write-order proxy, so we + * recover the mint second via `issuedAt = expiresAt - expiresIn` — the + * `expiresIn` cancels out, making the comparison robust to the server returning + * a different `expires_in` across refreshes (an older, longer-lived token can + * otherwise have a LARGER `expiresAt` than a newer, shorter-lived one). In + * every other case the keychain stays authoritative. + * + * Residual limitation: issuance time has 1-second granularity, so two tokens + * minted in the SAME wall-clock second tie and the keychain stays authoritative + * (strict `>`). That edge is practically unreachable, and we deliberately avoid + * a new wire field / monotonic generation counter to keep ZERO breaking change + * to the on-disk + keychain format. */ private async reconcileOnHit(name: string, raw: string): Promise { const keyringToken = this.deserialize(raw); @@ -198,12 +209,15 @@ export class KeyringTokenStorage implements TokenStorage { } // Adopt the file token ONLY when both are valid (not tombstones) and the file - // is strictly newer. This is the flip-flop repair: a fallback run landed a - // newer token on disk; make it keychain-authoritative now. + // was ISSUED strictly later. This is the flip-flop repair: a fallback run + // landed a newer token on disk; make it keychain-authoritative now. We compare + // mint time (`expiresAt - expiresIn`), NOT `expiresAt` (an expiration time), + // so a variable server `expires_in` can't make an older long-lived token look + // newer than a freshly-rotated short-lived one. if ( classifyToken(keyringToken).kind === 'valid' && classifyToken(fileToken).kind === 'valid' && - fileToken.expiresAt > keyringToken.expiresAt + issuedAt(fileToken) > issuedAt(keyringToken) ) { const fileSerialized = this.serialize(fileToken); this.keyring.createEntry(this.service, name).setPassword(fileSerialized); @@ -280,6 +294,16 @@ export class KeyringTokenStorage implements TokenStorage { } } +/** + * Recover a token's mint second from persisted fields. `expiresAt` is stamped at + * issuance as `floor(mintTime) + expiresIn`, so subtracting `expiresIn` cancels + * the lifetime and yields the issuance instant — robust to a variable server + * `expires_in` across refreshes (1-second granularity; same-second mints tie). + */ +function issuedAt(token: TokenInfo): number { + return token.expiresAt - token.expiresIn; +} + /** * Adapter over the real `@napi-rs/keyring` module shape, kept narrow so the * production load path and the test fakes share one `KeyringApi` contract. diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 4b362f812ebc584ffcbfb4789d190aaac9901d06..b8e8eb27aa19bd1ad29242e5dd0692467de620b5 100644 GIT binary patch delta 943 zcmZ8gO^ee&7-kQO^dPJshed=}S#}$2x?SuFwkoDr)?#g>E$D4Kn@pO)=}eeRw%dbL z!9U=}qagkOLGa|!i~bKU{te&Bhb@~+GH+&{=Xsy^nNL49e*fC|Cav^n`_f^+I0Y?V zQvocF6Y2venkw*V95GFS!05Tt?{|klPa~#i>}aUQp;Ts5QR)dR{jzy|xwi*HI>kLP zlL7*%K!+4gh+^cJQ+rZuKVo760cVjNa2eI1tUG#w8i0rj;Dcb@S1)=)XWVlKpxKPV zLzpq4N7R$ThdV3w8pINqUYgSbcZ{0n3bor(`6x0{@^n!zF`vl#(*ZwnGO-B?+y;}CuadQ3ffr8>?zTVd-gH$&^{lQ0pZ*%KqUet$jrVe6$ulR zFJRDpiC9=hgZ+ZAQ!tf)qo?k0yvCTd)~T<&e*n(fik{%ybSRT)2%RO&NnA{^#LLvy zVT^MizKpaP%rqg&hglM9@Z>Baie`t1@F?V#RaboW%4VwsCD?3+2Av!lt;B83A|JLC z*lo_Ds7{o?b{(iT8;u4$f&=ryNugUYwbISj?TZ;OoE_XKzS5stS8qDIwY**`aC9p> zY|8293tQLTSZx!NJhzz3ePl@A?cCZf&JoYUERIdMTd+&CZN^oi8jfYBT&;5$o#&`~ z2(VvP&(q$$j}85K`>_y`Cj{ MukFj Date: Sun, 14 Jun 2026 17:07:37 +0800 Subject: [PATCH 11/22] docs(oauth): note issuedAt graceful-degradation on rehydrated tokens --- packages/oauth/src/keyring-storage.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index de0af5a7a..ac0992ec3 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -299,6 +299,11 @@ export class KeyringTokenStorage implements TokenStorage { * issuance as `floor(mintTime) + expiresIn`, so subtracting `expiresIn` cancels * the lifetime and yields the issuance instant — robust to a variable server * `expires_in` across refreshes (1-second granularity; same-second mints tie). + * + * Operates on any `TokenInfo`: both fields are always numeric (`tokenFromWire` + * defaults them to 0), so even an externally-edited record compares as a + * consistent integer order — never NaN/throw. Only the caller's both-valid guard + * feeds it real minted tokens; tombstones (`expiresIn: 0`) are excluded there. */ function issuedAt(token: TokenInfo): number { return token.expiresAt - token.expiresIn; From 11bf327da55c2a3f4d1bc5ff449345eff69a5f08 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 19:12:34 +0800 Subject: [PATCH 12/22] docs(oauth): document keychain-by-default storage and KIMI_DISABLE_KEYRING opt-out OAuth credentials are now stored in the OS keychain by default, with any pre-existing plaintext credential file migrated into the keychain and then removed. Document this behavior near the provider `oauth` field and add a `KIMI_DISABLE_KEYRING` row to the runtime-switches table, in both the en and zh locales. --- docs/en/configuration/config-files.md | 2 ++ docs/en/configuration/env-vars.md | 1 + docs/zh/configuration/config-files.md | 2 ++ docs/zh/configuration/env-vars.md | 1 + 4 files changed, 6 insertions(+) diff --git a/docs/en/configuration/config-files.md b/docs/en/configuration/config-files.md index 0d64f5044..2b6886412 100644 --- a/docs/en/configuration/config-files.md +++ b/docs/en/configuration/config-files.md @@ -107,6 +107,8 @@ Each entry in the `providers` table defines an API provider, keyed by a unique n | `env` | `table` | No | Fallback source for provider credentials; see below | | `custom_headers` | `table` | No | Custom HTTP headers attached to each request | +> **OAuth credential storage**: OAuth tokens are stored in the operating system keychain (macOS Keychain, Windows Credential Manager, Linux Secret Service) by default. Any pre-existing plaintext credential file is migrated into the keychain and then removed. The `oauth.storage` field records where the token lives and is injected automatically — it does not select the backend. Set [`KIMI_DISABLE_KEYRING=1`](./env-vars.md#runtime-switches) to force plaintext-file storage; this is also the automatic fallback when no keychain is available. + **`env` sub-table**: You can write provider-conventional key names (such as `KIMI_API_KEY`) inside `[providers..env]` as a fallback source for `api_key` / `base_url`. This sub-table is **read only from the config file** and does not modify the shell environment: ```toml diff --git a/docs/en/configuration/env-vars.md b/docs/en/configuration/env-vars.md index 77b7bdfb2..5d4528c75 100644 --- a/docs/en/configuration/env-vars.md +++ b/docs/en/configuration/env-vars.md @@ -134,6 +134,7 @@ Switches that control the behavior of subsystems such as telemetry, background t | `KIMI_MODEL_THINKING_KEEP` | Moonshot preserved-thinking passthrough (`thinking.keep`); applies to the `kimi` provider only, and only while Thinking is on | A value the API accepts, e.g. `all` | | `KIMI_CODE_NO_AUTO_UPDATE` | Fully disable the update preflight — no check, background install, or prompt. Legacy alias `KIMI_CLI_NO_AUTO_UPDATE` is also honored | Truthy: `1`/`true`/`yes`/`on` | | `KIMI_DISABLE_CRON` | Disable the scheduled-task tool (`CronCreate` rejects new schedules; existing tasks do not fire) | `1` to disable | +| `KIMI_DISABLE_KEYRING` | Force plaintext-file OAuth credential storage instead of the OS keychain (also the automatic fallback when no keychain is available) | `1` to force the file backend | ## Diagnostic logs diff --git a/docs/zh/configuration/config-files.md b/docs/zh/configuration/config-files.md index e0a215f56..fdddc6bae 100644 --- a/docs/zh/configuration/config-files.md +++ b/docs/zh/configuration/config-files.md @@ -107,6 +107,8 @@ timeout = 5 | `env` | `table` | 否 | 供应商凭证的备用来源,详见下文 | | `custom_headers` | `table` | 否 | 每次请求附加的自定义 HTTP 头 | +> **OAuth 凭据存储**:OAuth 令牌默认存放在操作系统密钥链(macOS Keychain、Windows 凭据管理器、Linux Secret Service)中。已有的明文凭据文件会被迁移到密钥链并随后删除。`oauth.storage` 字段记录令牌所在位置、由登录流程自动注入,并不用于选择后端。设置 [`KIMI_DISABLE_KEYRING=1`](./env-vars.md#运行时开关) 可强制使用明文文件存储;当系统没有可用密钥链时也会自动回退到该方式。 + **`env` 子表**:可以把供应商惯用的键名(如 `KIMI_API_KEY`)写在 `[providers..env]` 里,作为 `api_key` / `base_url` 的备用来源。这个子表**只在配置文件里读取**,不会修改 shell 环境: ```toml diff --git a/docs/zh/configuration/env-vars.md b/docs/zh/configuration/env-vars.md index e87eb146a..558d29a74 100644 --- a/docs/zh/configuration/env-vars.md +++ b/docs/zh/configuration/env-vars.md @@ -134,6 +134,7 @@ kimi | `KIMI_MODEL_THINKING_KEEP` | Moonshot 保留思考透传(`thinking.keep`),仅对 `kimi` 供应商生效,且仅在 Thinking 开启时注入 | API 接受的值,如 `all` | | `KIMI_CODE_NO_AUTO_UPDATE` | 完全禁用更新预检——不检查、不后台安装、不提示。同时兼容旧名 `KIMI_CLI_NO_AUTO_UPDATE` | 真值:`1`/`true`/`yes`/`on` | | `KIMI_DISABLE_CRON` | 禁用定时任务工具(`CronCreate` 拒绝新计划,已有任务不触发) | `1` 表示禁用 | +| `KIMI_DISABLE_KEYRING` | 强制 OAuth 凭据使用明文文件存储而非操作系统密钥链(系统无可用密钥链时也会自动回退到该方式) | `1` 表示强制使用文件后端 | ## 诊断日志 From fcc1148ccae49ec55cacb40fd4ae2e5c38a70191 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 22:30:44 +0800 Subject: [PATCH 13/22] test(oauth): use \x00 escape instead of literal NUL in fake keyring keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The FakeKeyring Map-key separator was written as a literal NUL byte in the template literals, which made git/GitHub classify the two test files as binary ("Binary file not shown" — diff unreviewable). Replace each literal NUL with the \x00 escape sequence: it evaluates to the same U+0000 separator at runtime (keys stay NUL-separated, tests unchanged) while keeping the source plain UTF-8 text so the diff renders. --- packages/oauth/test/keyring-storage.test.ts | Bin 25415 -> 25433 bytes .../test/toolkit-keyring-integration.test.ts | Bin 11547 -> 11553 bytes 2 files changed, 0 insertions(+), 0 deletions(-) diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index b8e8eb27aa19bd1ad29242e5dd0692467de620b5..55e684c853308d259b5515c37140404725fa6a7b 100644 GIT binary patch delta 77 zcmX?pjPd3%#trA#SYj#+3^t!<6Jvs~o^kkbLRh9kCQz{~5gSX0*mB=44lrwSmPqX6 N25;WYbHY~h000X;8Q%Z^ delta 58 zcmcb4jPdv}#trA#7#TKSViRKmQg1l?IDwR%kO_!aCSqd=q_+8XaWFDWE)$8JWWc+5 IUD#?K05b0qL;wH) diff --git a/packages/oauth/test/toolkit-keyring-integration.test.ts b/packages/oauth/test/toolkit-keyring-integration.test.ts index 44e12ed6970e30cba2dc9b19b8df80a090ca3efa..72f40dd50068cddacffe1966d4c57fe76bac8820 100644 GIT binary patch delta 27 ecmbOowJ>VKZ%&q&3Il`9e>laMAS_`X4n+W!FbLoP delta 21 bcmZ1&H9Km Date: Sun, 14 Jun 2026 22:49:21 +0800 Subject: [PATCH 14/22] fix(oauth): surface failed keychain deletes on logout (@napi-rs/keyring returns false, never throws) --- packages/oauth/src/keyring-storage.ts | 22 +++++-- packages/oauth/test/keyring-storage.test.ts | 73 +++++++++++++++++++++ 2 files changed, 91 insertions(+), 4 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index ac0992ec3..6e3c04774 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -63,7 +63,12 @@ export const KEYRING_PROBE_SERVICE = 'kimi-code-keyring-probe'; export interface KeyringEntry { getPassword(): string | null; setPassword(password: string): void; - /** Returns true if a credential existed. */ + /** + * Returns true when a credential was deleted. The native binding NEVER throws + * and maps EVERY failure (locked/no-access/ambiguous/platform error) to the + * SAME `false` it returns for "no such entry", so `false` is ambiguous — + * "did not exist" OR "delete failed". Disambiguate by re-reading `getPassword()`. + */ deleteCredential(): boolean; } @@ -275,10 +280,19 @@ export class KeyringTokenStorage implements TokenStorage { // (permissions, lock state, ambiguous entries) can never leave the // plaintext file behind — the "both stores cleared" guarantee must hold. // A genuine keyring error is surfaced after the file is cleared, never - // swallowed. (`deleteCredential() === false` for a missing entry is normal, - // not an error.) + // swallowed. try { - this.keyring.createEntry(this.service, name).deleteCredential(); + const entry = this.keyring.createEntry(this.service, name); + const deleted = entry.deleteCredential(); + // The native binding maps EVERY failure (locked/no-access/ambiguous/platform) + // to `false` — the same value as "no such entry" — and never throws. So a bare + // `false` is ambiguous; disambiguate by re-reading: if the credential is still + // present, the delete genuinely failed and must be surfaced (the file backend's + // unlink would have thrown). `getPassword()` also swallows errors to null, so a + // null re-read is the strongest lock-free signal that the entry is gone. + if (!deleted && entry.getPassword() !== null) { + throw new Error(`failed to delete keyring credential "${name}"`); + } } catch (error) { // Keyring delete failed — still clear the plaintext copy, then re-throw. await this.legacy.remove(name); diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 55e684c85..75fa1ccf4 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -446,6 +446,79 @@ describe('KeyringTokenStorage', () => { expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); }); + it('remove() surfaces a failed keychain delete that returns false (real @napi-rs/keyring never throws)', async () => { + // The REAL @napi-rs/keyring v1.3.0 binding maps EVERY delete failure (locked + // keychain, no-access, ambiguous, platform error) to a plain `false` and + // NEVER throws — the SAME `false` returned for "no such entry". getPassword() + // likewise swallows errors to null. This fake models a delete that FAILS: + // deleteCredential() returns false while getPassword() STILL returns the + // stored value, so the credential definitively persists. remove() must + // disambiguate via the re-read and surface the failure — otherwise a logout + // would clear the plaintext but silently leave the keychain token alive. + const token = sampleToken(); + + class FailingDeleteKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + getPassword: () => base.getPassword(), + setPassword(p: string): void { + base.setPassword(p); + }, + // Genuine failure: returns false but the credential is NOT removed, + // so a follow-up getPassword() still sees it. + deleteCredential(): boolean { + return false; + }, + }; + } + } + + const failingKeyring = new FailingDeleteKeyring(); + const failingStorage = new KeyringTokenStorage({ keyring: failingKeyring, legacy }); + + // Seed the keychain (the credential persists through the failed delete) and a + // lingering plaintext copy that must still be cleared. + await failingStorage.save('kimi-code', token); + await legacy.save('kimi-code', token); + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + + // The genuine keyring failure must be surfaced... + await expect(failingStorage.remove('kimi-code')).rejects.toThrow( + /failed to delete keyring credential/, + ); + // ...but the legacy plaintext cleanup must still have run. + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + // The keychain credential genuinely survived (the bug being guarded against). + expect(failingKeyring.createEntry(KEYRING_SERVICE, 'kimi-code').getPassword()).not.toBeNull(); + }); + + it('remove() treats deleteCredential()=false with a null re-read as a no-op success (absent entry)', async () => { + // A bare `false` from deleteCredential() is overloaded: it means "delete + // failed" OR "no such entry". When the re-read getPassword() returns null the + // entry is genuinely gone (deleted or never existed), so logging out an absent + // entry must RESOLVE without throwing. + class AbsentDeleteKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + // Always reports the entry as absent. + getPassword: () => null, + setPassword(p: string): void { + base.setPassword(p); + }, + // Mirrors the native binding's "did not exist" → false. + deleteCredential(): boolean { + return false; + }, + }; + } + } + + const absentStorage = new KeyringTokenStorage({ keyring: new AbsentDeleteKeyring(), legacy }); + await expect(absentStorage.remove('never-existed')).resolves.toBeUndefined(); + }); + it('list() unions keyring accounts and un-migrated legacy names, deduped', async () => { await storage.save('alpha', sampleToken()); // lands in keyring await legacy.save('beta', sampleToken()); // un-migrated plaintext From 725f64b31a0fa7e13c07e1c22a658c0714ea5581 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 23:23:31 +0800 Subject: [PATCH 15/22] fix(oauth): prune stale plaintext copy after keychain save (prevent fallback resurrection) --- packages/oauth/src/keyring-storage.ts | 13 ++++- packages/oauth/test/keyring-storage.test.ts | 29 ++++++++++ .../test/toolkit-keyring-integration.test.ts | 54 +++++++++++++++++++ 3 files changed, 95 insertions(+), 1 deletion(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 6e3c04774..097ec486f 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -26,7 +26,9 @@ * living in the clear without ever dropping a newer token a concurrent * file-backend writer may have just landed. Migration is LOCK-FREE, exactly like * `FileTokenStorage`. `remove` and `list` also reconcile against the legacy file - * store so pre-migration plaintext can never linger or go missing. + * store so pre-migration plaintext can never linger or go missing. `save` prunes + * any legacy plaintext copy after the keychain write so a later file-backend run + * can't resurrect a superseded token. * * Reconcile-on-hit (flip-flop repair): `resolveTokenStorage` can pick a * DIFFERENT backend per run for one credentialsDir (keychain locked, @@ -269,6 +271,15 @@ export class KeyringTokenStorage implements TokenStorage { async save(name: string, token: TokenInfo): Promise { this.keyring.createEntry(this.service, name).setPassword(this.serialize(token)); + // The keychain is now authoritative for `name`. Drop any plaintext copy so a + // later FILE-backend run (KIMI_DISABLE_KEYRING=1, probe/native-load failure) — + // which is keychain-unaware and cannot reconcile — can't resurrect an obsolete + // token or a stale tombstone, and no secret lingers in cleartext. Lock-free + // unlink; a missing file is a no-op. Safe because save() always writes the + // current authoritative token (refresh/login/tombstone), so any on-disk copy + // is superseded — a concurrent file-backend writer is the documented + // unsupported mixed-backend case. + await this.legacy.remove(name); } async remove(name: string): Promise { diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 75fa1ccf4..f31946d76 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -155,6 +155,35 @@ describe('KeyringTokenStorage', () => { expect(JSON.parse(raw as string)).toEqual(tokenToWire(token)); }); + it('save() prunes a lingering plaintext copy after writing the keychain', async () => { + // A stale plaintext file lingers from a prior file-backend run (or a + // keychain-wins reconcile that left an older file). A later save() must make + // the keychain authoritative AND drop the cleartext, so a subsequent + // KIMI_DISABLE_KEYRING / probe-failure run (keychain-unaware FileTokenStorage) + // can no longer resurrect the obsolete token, and no secret lingers on disk. + const fileTok = sampleToken({ accessToken: 'at-stale-file', refreshToken: 'rt-stale-file' }); + const newTok = sampleToken({ accessToken: 'at-new', refreshToken: 'rt-new' }); + await legacy.save('kimi-code', fileTok); + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + + await storage.save('kimi-code', newTok); + + const raw = keyring.createEntry(KEYRING_SERVICE, 'kimi-code').getPassword(); + expect(raw).toBe(JSON.stringify(tokenToWire(newTok))); + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + }); + + it('save() is a no-op on the file when none exists (ENOENT path)', async () => { + const tok = sampleToken({ accessToken: 'at-fresh', refreshToken: 'rt-fresh' }); + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + + await expect(storage.save('kimi-code', tok)).resolves.toBeUndefined(); + + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + const raw = keyring.createEntry(KEYRING_SERVICE, 'kimi-code').getPassword(); + expect(JSON.parse(raw as string)).toEqual(tokenToWire(tok)); + }); + it('migrates a plaintext token into the keychain, then deletes the file', async () => { const token = sampleToken(); await legacy.save('kimi-code', token); diff --git a/packages/oauth/test/toolkit-keyring-integration.test.ts b/packages/oauth/test/toolkit-keyring-integration.test.ts index 72f40dd50..61acf6941 100644 --- a/packages/oauth/test/toolkit-keyring-integration.test.ts +++ b/packages/oauth/test/toolkit-keyring-integration.test.ts @@ -252,6 +252,60 @@ describe('KimiOAuthToolkit with a keyring-backed store (hermetic)', () => { expect(fetchImpl).toHaveBeenCalledTimes(1); }); + it('a refresh prunes a pre-seeded stale plaintext file so a later file run cannot resurrect it', async () => { + // Pre-seed a stale plaintext token at /kimi-code.json via a + // real FileTokenStorage — exactly what a prior file-backend fallback run (or + // a keychain-wins reconcile) leaves behind. The keychain holds an expired + // token so ensureFresh refreshes and calls save(). After save() the cleartext + // copy must be gone, so a later KIMI_DISABLE_KEYRING run (keychain-unaware) + // can no longer read it back and resurrect the obsolete credential. + await new FileTokenStorage(dir).save( + 'kimi-code', + token({ accessToken: 'stale-plaintext', refreshToken: 'stale-plaintext-refresh' }), + ); + expect(plaintextTokenFiles(dir)).toEqual(['kimi-code.json']); + + // Keychain token is already expired so ensureFresh must refresh it. + await storage.save('kimi-code', token({ accessToken: 'stale-access', expiresAt: 100 })); + + const fetchImpl = vi.fn(async (input: unknown, init?: RequestInit) => { + expect(fetchInputUrl(input)).toBe(`${FLOW_CONFIG.oauthHost}/api/oauth/token`); + if (typeof init?.body !== 'string') throw new TypeError('expected form body'); + expect(new URLSearchParams(init.body).get('grant_type')).toBe('refresh_token'); + return new Response( + JSON.stringify({ + access_token: 'rotated-access', + refresh_token: 'rotated-refresh', + expires_in: 3600, + scope: '', + token_type: 'Bearer', + }), + { status: 200, headers: { 'Content-Type': 'application/json' } }, + ); + }); + vi.stubGlobal('fetch', fetchImpl); + + const toolkit = new KimiOAuthToolkit({ + homeDir: dir, + identity: TEST_IDENTITY, + storage, + now: () => 1_000, + flowConfig: FLOW_CONFIG, + }); + + await expect(toolkit.ensureFresh()).resolves.toBe('rotated-access'); + expect(fetchImpl).toHaveBeenCalledTimes(1); + + // The rotated token is authoritative in the keychain... + const raw = keyring.createEntry(service, 'kimi-code').getPassword(); + expect(raw).not.toBeNull(); + expect((JSON.parse(raw as string) as Record)['access_token']).toBe( + 'rotated-access', + ); + // ...and the pre-seeded stale plaintext copy is gone (no resurrection path). + expect(plaintextTokenFiles(dir)).toEqual([]); + }); + it('logout() removes the token from the keychain', async () => { await storage.save('kimi-code', token()); const toolkit = new KimiOAuthToolkit({ From 843abd7f16e4e9789f8d5769c84e90cd9201d3be Mon Sep 17 00:00:00 2001 From: LongYinan Date: Sun, 14 Jun 2026 23:41:21 +0800 Subject: [PATCH 16/22] docs(oauth): clarify save() prune is a deliberate fail-closed choice for tombstones --- packages/oauth/src/keyring-storage.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 097ec486f..2977c20b3 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -279,6 +279,20 @@ export class KeyringTokenStorage implements TokenStorage { // current authoritative token (refresh/login/tombstone), so any on-disk copy // is superseded — a concurrent file-backend writer is the documented // unsupported mixed-backend case. + // + // This unconditional prune deliberately DIFFERS from reconcileOnHit's + // read-path "leave a differing file in place" rule: there we have NOT + // established write-order authority over the file; here save() just MADE the + // keychain authoritative, so the file is by definition superseded. The only + // case the two could disagree is a TOMBSTONE-save whose `name` still has a + // valid plaintext copy — a flip-flop where the file run minted a token in the + // SAME second the keychain token was issued, so reconcileOnHit's strict `>` + // declined to adopt it. We prune anyway, ON PURPOSE: honoring the revocation + // (no resurrection of a deliberately tombstoned credential by a later + // keychain-unaware file run) outranks preserving that token. The cost is a + // forced re-login in that vanishingly rare edge — fail-CLOSED, and identical + // to what the file backend alone does once tombstoned. A recoverable re-login + // beats a resurrected revoked secret. await this.legacy.remove(name); } From 7aa600fe2d141b9ba14503393817f9ede3db276a Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 15 Jun 2026 09:54:55 +0800 Subject: [PATCH 17/22] fix(oauth): reject keyrings that cannot delete during capability probe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capability probe previously selected the keychain backend whenever it could set and read back a sentinel, discarding the delete result in the finally block. A backend that can store+read but whose delete is denied (the real @napi-rs/keyring binding maps a failed delete to false and never throws) would still pass the probe. Selecting it then migrates plaintext credentials into a store that can never remove them: load()'s migrate-then-delete unlinks the plaintext irreversibly, and a later logout/remove() throws because the keychain entry survives — leaving the token stuck in the keychain forever. Make delete capability part of the keyring usability contract: the probe now deletes its sentinel and confirms removal via a re-read (getPassword() === null), mirroring remove()'s own disambiguation rather than trusting the boolean. A backend that cannot reliably delete is rejected, so we fall back to the plaintext file store instead of migrating into a one-way keychain. --- packages/oauth/src/keyring-storage.ts | 29 ++++++++-- packages/oauth/test/keyring-storage.test.ts | 61 +++++++++++++++++++++ 2 files changed, 84 insertions(+), 6 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 2977c20b3..cfc3b7463 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -381,8 +381,14 @@ function loadNativeKeyring(): KeyringApi | undefined { /** * Round-trip a sentinel under an isolated service to prove the keychain has a - * live backend. Any throw, or a read-back mismatch, means the keychain is not - * usable on this host. + * live backend. Any throw, a read-back mismatch, or an inability to DELETE the + * sentinel means the keychain is not usable on this host. Delete capability is + * part of the usability contract: once selected the keychain is the + * authoritative store, so logout/revocation (`remove`) and `load`'s + * migrate-then-delete both depend on it being able to remove entries. A backend + * that can set/read but not delete would trap migrated tokens it can never + * remove and make logout throw — so we reject it here and fall back to the file + * store instead of migrating plaintext into a one-way keychain. */ function probeKeyring(keyring: KeyringApi): boolean { // A UNIQUE account per attempt: two CLI processes probing concurrently must @@ -394,16 +400,27 @@ function probeKeyring(keyring: KeyringApi): boolean { const entry = keyring.createEntry(KEYRING_PROBE_SERVICE, account); try { entry.setPassword(sentinel); - const readBack = entry.getPassword(); - return readBack === sentinel; + if (entry.getPassword() !== sentinel) return false; + // The keychain is the AUTHORITATIVE store once selected, so it MUST also be + // able to DELETE — logout/revocation and load()'s migrate-then-delete all + // depend on it. The native binding never throws and maps a failed delete to + // `false`, so we do NOT trust the boolean: delete the sentinel and confirm it + // is actually gone via a re-read (mirrors remove()'s own disambiguation). A + // backend that stores+reads but cannot delete would trap migrated tokens it + // can never remove and make logout throw — reject it here so we fall back to + // the plaintext file store instead of migrating into a one-way keychain. + entry.deleteCredential(); + return entry.getPassword() === null; } catch { return false; } finally { - // Always remove our own sentinel, even if the round-trip threw mid-way. + // Safety-net cleanup for the early-return-mismatch and throw paths, and a + // harmless idempotent no-op on the success path: never leave a sentinel + // behind, and never let cleanup mask the probe result. try { entry.deleteCredential(); } catch { - // best-effort cleanup; a failed delete must not mask the probe result + /* best-effort */ } } } diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index f31946d76..3eb40fc79 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -629,6 +629,67 @@ describe('resolveTokenStorage', () => { expect(await storage.list()).toEqual([]); }); + it('falls back to FileTokenStorage when the keyring can set/read but cannot DELETE', () => { + // The keychain is the AUTHORITATIVE store once selected — logout/revocation + // and load()'s migrate-then-delete depend on delete working. A backend that + // stores+reads fine but whose deleteCredential() fails (returns false AND + // leaves the entry present) would trap migrated tokens it can never remove + // and make logout throw. The probe must treat that as unusable and fall back + // to the plaintext file store, NOT migrate into a one-way keychain. + class NoDeleteKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + getPassword: () => base.getPassword(), + setPassword(p: string): void { + base.setPassword(p); + }, + // Genuine delete failure: returns false and the entry SURVIVES, so a + // follow-up getPassword() still sees the sentinel. + deleteCredential: () => false, + }; + } + } + const storage = resolveTokenStorage(dir, { loadKeyring: () => new NoDeleteKeyring() }); + expect(storage).toBeInstanceOf(FileTokenStorage); + expect(storage).not.toBeInstanceOf(KeyringTokenStorage); + }); + + it('rejects a keyring whose delete returns true but the entry SURVIVES (lying boolean)', () => { + // The native binding maps a failed delete to `false`, but the probe does NOT + // trust the boolean — it confirms removal via a re-read (getPassword() === + // null), mirroring remove()'s own disambiguation. A backend that LIES (delete + // reports true while the entry persists) must still be rejected, proving the + // probe relies on the authoritative re-read, not the return value. + class LyingDeleteKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + getPassword: () => base.getPassword(), + setPassword(p: string): void { + base.setPassword(p); + }, + // Lies: claims success while the entry remains present. + deleteCredential: () => true, + }; + } + } + const storage = resolveTokenStorage(dir, { loadKeyring: () => new LyingDeleteKeyring() }); + expect(storage).toBeInstanceOf(FileTokenStorage); + expect(storage).not.toBeInstanceOf(KeyringTokenStorage); + }); + + it('selects KeyringTokenStorage and leaves NO probe sentinel behind on the healthy path', () => { + // The healthy fake deletes properly (Map.delete mutates the backing store), + // so the probe's delete-and-re-read confirms removal → keyring is selected. + // Assert via instanceof AND that the isolated probe service has zero accounts + // afterward, proving the probe's own cleanup ran (no leaked sentinel). + const keyring = new FakeKeyring(); + const storage = resolveTokenStorage(dir, { loadKeyring: () => keyring }); + expect(storage).toBeInstanceOf(KeyringTokenStorage); + expect(keyring.findAccounts(KEYRING_PROBE_SERVICE)).toEqual([]); + }); + it('the probe uses a unique, non-constant account per attempt', () => { const a = new RecordingKeyring(); const b = new RecordingKeyring(); From 4fae69a271e4e1209f4e41c384f145b4b93b23e9 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 15 Jun 2026 11:07:10 +0800 Subject: [PATCH 18/22] docs(oauth): document+test that a file tombstone never force-revokes a valid keychain token Pins the deliberate keychain-authoritative tradeoff for the inverse of the no-un-revoke direction. A file-side tombstone (a fallback run's token was 401-rejected) is timestamp-less (issuedAt === 0), so it is unorderable against a valid keychain token; letting it win would invert the plaintext-is-less-trusted trust model and let a stale leftover tombstone repeatedly force-logout a fresh keychain token. The keychain stays authoritative and self-heals via the next refresh 401 if its own refresh_token is also revoked. Adds a characterization test and a cross-referenced comment; zero behavior change. --- packages/oauth/src/keyring-storage.ts | 15 +++++++++ packages/oauth/test/keyring-storage.test.ts | 35 +++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index cfc3b7463..3bdcb6d90 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -247,6 +247,21 @@ export class KeyringTokenStorage implements TokenStorage { // up (this branch only prunes the redundant-duplicate case), so it lingers // until the next explicit `remove()` / logout. Deliberate — we never delete a // token we did not make authoritative. + // + // Inverse of the no-un-revoke invariant above: a file-side TOMBSTONE (a + // fallback run's token was 401-rejected) does NOT force-revoke a VALID + // keychain token — the keychain stays authoritative and we return its valid + // token here. Correct because that returned token carries its OWN + // refresh_token: if it too is revoked, the next refresh 401 tombstones the + // KEYCHAIN and it self-heals; if it is still valid, honoring it is right (the + // file tombstone revoked a DIFFERENT, older fallback token). We can't do + // better: a tombstone is timestamp-less (`issuedAt === 0`, all fields empty), + // so it is unorderable against the keychain token. Letting the file tombstone + // WIN would (a) invert the plaintext-is-less-trusted trust model — the mirror + // of the no-un-revoke invariant — and (b) let a stale leftover tombstone + // repeatedly force-logout a fresh keychain token. A correct ordering-based fix + // would need a wire-format change the design deliberately avoids (see the + // `expiresIn` / wire-format note above). if (this.serialize(fileToken) === raw) { await this.removeIfBytesMatch(name, raw); } diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 3eb40fc79..d1ed6882e 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -364,6 +364,41 @@ describe('KeyringTokenStorage', () => { expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); }); + // CHARACTERIZATION/guard test: pins the deliberate keychain-authoritative + // tradeoff for the INVERSE of the no-un-revoke direction. It encodes CURRENT + // behavior (expected to pass on current code) — not a fix verification. + it('reconcileOnHit() keeps a valid keychain token authoritative over a file-side tombstone (no force-revoke from plaintext)', async () => { + // Inverse of the no-un-revoke test: keychain holds a VALID token; a file-side + // tombstone (a fallback run's token was 401-rejected) sits in the plaintext + // file. A tombstone is timestamp-less (issuedAt === 0), so it cannot order + // against the keychain token — and the less-trusted plaintext must NOT + // force-revoke the keychain. load() must return the keychain's VALID token, + // leave the keychain entry unchanged, and leave the file tombstone in place. + const valid = sampleToken({ accessToken: 'at-valid', refreshToken: 'rt-valid', expiresAt: 2000 }); + const tombstone = revokedTombstone(valid); + + await storage.save('kimi-code', valid); // keychain holds the VALID token + await legacy.save('kimi-code', tombstone); // file holds a revoked tombstone + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + + const loaded = await storage.load('kimi-code'); + // The keychain's VALID token wins — NOT a revoked/undefined result. + expect(loaded).toEqual(valid); + expect(classifyToken(loaded).kind).toBe('valid'); + expect((loaded as TokenInfo).accessToken).toBe('at-valid'); + + // The keychain entry is UNCHANGED — it was not tombstoned or deleted. + const raw = keyring.createEntry(KEYRING_SERVICE, 'kimi-code').getPassword(); + expect(JSON.parse(raw as string)).toEqual(tokenToWire(valid)); + + // The file tombstone is LEFT IN PLACE (conservative "leave differing file"): + // its bytes differ from the authoritative keychain value, so it is not pruned, + // and the plaintext is never allowed to force-revoke the keychain. + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + const fileRaw = await legacy.load('kimi-code'); + expect(classifyToken(fileRaw).kind).toBe('revoked'); + }); + it('keychain HIT reconcile: prunes a plaintext duplicate equal after canonical re-serialization', async () => { // Keychain and file hold the SAME token (a just-migrated state observed by a // concurrent peer, or a redundant copy). load() returns the keychain value From c9c61963caee61b8c56b025346e49fde0eda2333 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 15 Jun 2026 12:38:58 +0800 Subject: [PATCH 19/22] fix(oauth): validate token name before keychain writes (file-backend fail-before-write parity) keyring save() wrote to the keychain (setPassword) before the legacy name check threw, orphaning a credential under an invalid account while the caller saw a thrown error. The same assertValidTokenName guard (extracted verbatim from FileTokenStorage.pathFor) now runs first in save/load/remove, restoring the file backend's fail-before-write contract and strict drop-in name-rejection parity. --- packages/oauth/src/keyring-storage.ts | 8 ++++- packages/oauth/src/storage.ts | 29 +++++++++++------ packages/oauth/test/keyring-storage.test.ts | 35 +++++++++++++++++++++ 3 files changed, 61 insertions(+), 11 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 3bdcb6d90..35397cb44 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -49,7 +49,7 @@ import { createRequire } from 'node:module'; import { homedir } from 'node:os'; import { join, resolve } from 'node:path'; -import { FileTokenStorage } from './storage'; +import { assertValidTokenName, FileTokenStorage } from './storage'; import type { TokenStorage } from './storage'; import { classifyToken } from './token-state'; import type { TokenInfo, TokenInfoWire } from './types'; @@ -115,6 +115,7 @@ export class KeyringTokenStorage implements TokenStorage { } async load(name: string): Promise { + assertValidTokenName(name); const raw = this.keyring.createEntry(this.service, name).getPassword(); if (raw !== null) { return this.reconcileOnHit(name, raw); @@ -285,6 +286,10 @@ export class KeyringTokenStorage implements TokenStorage { } async save(name: string, token: TokenInfo): Promise { + // Reject invalid names BEFORE the keychain write to preserve the file + // backend's fail-before-write contract — otherwise setPassword would orphan + // a credential under an invalid account before the legacy name check threw. + assertValidTokenName(name); this.keyring.createEntry(this.service, name).setPassword(this.serialize(token)); // The keychain is now authoritative for `name`. Drop any plaintext copy so a // later FILE-backend run (KIMI_DISABLE_KEYRING=1, probe/native-load failure) — @@ -312,6 +317,7 @@ export class KeyringTokenStorage implements TokenStorage { } async remove(name: string): Promise { + assertValidTokenName(name); // Clear both stores so a pre-migration plaintext copy can never linger // (e.g. logout before the token was ever migrated). Missing credentials // are a no-op, not an error. diff --git a/packages/oauth/src/storage.ts b/packages/oauth/src/storage.ts index f7d4e1abb..e5e390e8d 100644 --- a/packages/oauth/src/storage.ts +++ b/packages/oauth/src/storage.ts @@ -38,6 +38,23 @@ export interface TokenStorage { list(): Promise; } +/** + * Guard against path traversal: caller-provided names (from config.toml or + * slash commands) must not escape the credentials dir. `basename` strips any + * `..` or `/` segments; if the sanitized value differs from the input we refuse + * the request entirely rather than silently writing to a different file than the + * caller asked for. Leading-dot names (hidden files) are also rejected. + * + * Shared so non-file backends (e.g. KeyringTokenStorage) enforce the identical + * name-rejection rule, error text, and fail-before-write timing. + */ +export function assertValidTokenName(name: string): void { + const safe = basename(name); + if (safe.length === 0 || safe !== name || safe.startsWith('.')) { + throw new Error(`Invalid token name: "${name}"`); + } +} + export class FileTokenStorage implements TokenStorage { private readonly dir: string; @@ -57,16 +74,8 @@ export class FileTokenStorage implements TokenStorage { } private pathFor(name: string): string { - // Guard against path traversal: caller-provided names (from config.toml - // or slash commands) must not escape the credentials dir. `basename` - // strips any `..` or `/` segments; if the sanitized value differs from - // the input we refuse the request entirely rather than silently - // writing to a different file than the caller asked for. - const safe = basename(name); - if (safe.length === 0 || safe !== name || safe.startsWith('.')) { - throw new Error(`Invalid token name: "${name}"`); - } - return join(this.dir, `${safe}.json`); + assertValidTokenName(name); + return join(this.dir, `${name}.json`); } async load(name: string): Promise { diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index d1ed6882e..166a0d917 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -608,6 +608,41 @@ describe('KeyringTokenStorage', () => { it('remove() does not throw when nothing exists', async () => { await expect(storage.remove('never-existed')).resolves.toBeUndefined(); }); + + // Invalid-name rejection — strict drop-in parity with FileTokenStorage + // (storage.test.ts:146-166): same rule, same /Invalid token name/ error, and + // the guard must run BEFORE any keychain op so save() can't orphan a + // credential under an invalid account (file backend's fail-before-write). + describe('rejects invalid token names (file-backend parity)', () => { + const badNames = ['../../etc/passwd', '../etc/passwd', '.hidden', '']; + + for (const bad of badNames) { + it(`save() rejects ${JSON.stringify(bad)}`, async () => { + await expect(storage.save(bad, sampleToken())).rejects.toThrow(/Invalid token name/); + }); + + it(`load() rejects ${JSON.stringify(bad)}`, async () => { + await expect(storage.load(bad)).rejects.toThrow(/Invalid token name/); + }); + + it(`remove() rejects ${JSON.stringify(bad)}`, async () => { + await expect(storage.remove(bad)).rejects.toThrow(/Invalid token name/); + }); + } + + it('save() with an invalid name writes NOTHING to the keychain (no orphan)', async () => { + // This is the assertion that proves the orphan bug is fixed: the pre-fix + // save() called setPassword BEFORE the legacy name check threw, leaving a + // credential orphaned under the invalid account. With the guard first, the + // FakeKeyring backing store stays empty after the rejection. + await expect(storage.save('../../etc/passwd', sampleToken())).rejects.toThrow( + /Invalid token name/, + ); + expect(keyring.store.size).toBe(0); + expect(keyring.findAccounts(KEYRING_SERVICE)).toEqual([]); + expect(keyring.createEntry(KEYRING_SERVICE, '../../etc/passwd').getPassword()).toBeNull(); + }); + }); }); describe('resolveTokenStorage', () => { From 7118757fc9e7d0562ca7007ef8c9e13c7f22fe77 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 15 Jun 2026 13:17:27 +0800 Subject: [PATCH 20/22] fix(oauth): prove keychain credential absence via service listing on logout (don't treat false deletes as gone) getPassword()===null couldn't prove deletion because the v1.3.0 binding collapses every read error to null; remove() now confirms via findAccounts (throws on unreachable store, lists when reachable), mirroring FileTokenStorage's ENOENT-no-op / EACCES-throw, so logout never silently leaves an OS credential behind. --- packages/oauth/src/keyring-storage.ts | 27 ++-- packages/oauth/test/keyring-storage.test.ts | 141 +++++++++++++++++++- 2 files changed, 152 insertions(+), 16 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index 35397cb44..d4c1e72ab 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -69,7 +69,9 @@ export interface KeyringEntry { * Returns true when a credential was deleted. The native binding NEVER throws * and maps EVERY failure (locked/no-access/ambiguous/platform error) to the * SAME `false` it returns for "no such entry", so `false` is ambiguous — - * "did not exist" OR "delete failed". Disambiguate by re-reading `getPassword()`. + * "did not exist" OR "delete failed". `getPassword()` cannot disambiguate it + * (the binding likewise collapses every read error to `null`); prove absence + * with the service-scoped `findAccounts()` listing instead (see `remove`). */ deleteCredential(): boolean; } @@ -330,14 +332,21 @@ export class KeyringTokenStorage implements TokenStorage { try { const entry = this.keyring.createEntry(this.service, name); const deleted = entry.deleteCredential(); - // The native binding maps EVERY failure (locked/no-access/ambiguous/platform) - // to `false` — the same value as "no such entry" — and never throws. So a bare - // `false` is ambiguous; disambiguate by re-reading: if the credential is still - // present, the delete genuinely failed and must be surfaced (the file backend's - // unlink would have thrown). `getPassword()` also swallows errors to null, so a - // null re-read is the strongest lock-free signal that the entry is gone. - if (!deleted && entry.getPassword() !== null) { - throw new Error(`failed to delete keyring credential "${name}"`); + if (!deleted) { + // deleteCredential() returns false for BOTH a missing entry AND a failed/denied + // delete: the v1.3.0 binding collapses every error to false, and getPassword() + // likewise collapses every error to null, so a null re-read CANNOT prove the + // entry is gone (equally "deleted" or "read denied"). Prove absence the only + // non-error-ambiguous way this binding allows — enumerate the service: + // findAccounts() THROWS when the store is unreachable (locked / no-access) and + // returns the account list (incl. empty) when reachable. So an entry that + // survives a denied delete, or a store we cannot even reach, is surfaced as a + // failure; only a reachable store that does NOT list `name` is a true no-op + // (mirrors FileTokenStorage: ENOENT no-op, EACCES throw). A thrown + // findAccounts propagates to the catch below → plaintext cleared → re-thrown. + if (this.keyring.findAccounts(this.service).includes(name)) { + throw new Error(`failed to delete keyring credential "${name}"`); + } } } catch (error) { // Keyring delete failed — still clear the plaintext copy, then re-throw. diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 166a0d917..0e4b4851d 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -557,32 +557,159 @@ describe('KeyringTokenStorage', () => { expect(failingKeyring.createEntry(KEYRING_SERVICE, 'kimi-code').getPassword()).not.toBeNull(); }); - it('remove() treats deleteCredential()=false with a null re-read as a no-op success (absent entry)', async () => { + it('remove() treats deleteCredential()=false with the name absent from the service listing as a no-op success', async () => { // A bare `false` from deleteCredential() is overloaded: it means "delete - // failed" OR "no such entry". When the re-read getPassword() returns null the - // entry is genuinely gone (deleted or never existed), so logging out an absent - // entry must RESOLVE without throwing. + // failed" OR "no such entry". getPassword() cannot disambiguate (the binding + // collapses every read error to null), so absence is proven via the + // service-scoped findAccounts() listing: when the reachable store does NOT + // list `name`, the entry is genuinely gone, so logging out must RESOLVE + // without throwing (mirrors FileTokenStorage's ENOENT no-op). class AbsentDeleteKeyring extends FakeKeyring { override createEntry(service: string, account: string): KeyringEntry { const base = super.createEntry(service, account); return { - // Always reports the entry as absent. - getPassword: () => null, + getPassword: () => base.getPassword(), setPassword(p: string): void { base.setPassword(p); }, - // Mirrors the native binding's "did not exist" → false. + // Mirrors the native binding's "did not exist" → false; never touches + // the backing store, so findAccounts() does NOT list the name. deleteCredential(): boolean { return false; }, }; } + // The store is reachable and the name is absent from the listing. + override findAccounts(service: string): string[] { + return super.findAccounts(service); + } } const absentStorage = new KeyringTokenStorage({ keyring: new AbsentDeleteKeyring(), legacy }); await expect(absentStorage.remove('never-existed')).resolves.toBeUndefined(); }); + it('remove() surfaces a denied delete when the entry survives AND the read is denied (getPassword null)', async () => { + // FAILS on pre-fix code, PASSES on the fix. The v1.3.0 binding collapses + // every read error to null, so a denied/locked read returns null EVEN THOUGH + // the entry still exists. Here getPassword() returns null (read denied) while + // the entry STILL lives in the backing store (so findAccounts lists it) and + // deleteCredential() returns false without removing it. The OLD code keyed off + // `getPassword() !== null` → saw null → silently no-op'd → keychain credential + // SURVIVES while the plaintext is wiped and logout reports success. The NEW + // code proves absence via the service listing, sees `name` still present, and + // surfaces the failure — after still clearing the plaintext copy. + const token = sampleToken(); + + class DeniedReadSurvivingKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + // Read is DENIED: collapses to null even though the entry exists. + getPassword: () => null, + setPassword(p: string): void { + base.setPassword(p); + }, + // Denied delete: returns false and the entry SURVIVES in the store. + deleteCredential(): boolean { + return false; + }, + }; + } + } + + const deniedKeyring = new DeniedReadSurvivingKeyring(); + const deniedStorage = new KeyringTokenStorage({ keyring: deniedKeyring, legacy }); + + // Seed the keychain (entry persists through the denied delete) and a lingering + // plaintext copy that must still be cleared. + deniedKeyring.store.set(`${KEYRING_SERVICE}\x00kimi-code`, JSON.stringify(tokenToWire(token))); + await legacy.save('kimi-code', token); + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + + // The surviving entry must be surfaced as a failure... + await expect(deniedStorage.remove('kimi-code')).rejects.toThrow( + /failed to delete keyring credential/, + ); + // ...but the legacy plaintext cleanup must still have run. + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + // The keychain credential genuinely survived (the bug being guarded against). + expect(deniedKeyring.findAccounts(KEYRING_SERVICE)).toContain('kimi-code'); + }); + + it('remove() surfaces an unreachable store (deleteCredential false, findAccounts throws)', async () => { + // FAILS on pre-fix code, PASSES on the fix. A locked / no-access store: the + // binding collapses the delete to false and a read to null, but the + // service-scoped enumeration THROWS (findCredentials throws on an unreachable + // store). The OLD code saw `getPassword() === null` → silently no-op'd + // (success), leaving the OS credential behind. The NEW code lets the + // findAccounts throw propagate to the catch → plaintext cleared → re-thrown, + // so logout fails loud (mirrors FileTokenStorage's EACCES throw). + const token = sampleToken(); + + class UnreachableStoreKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + getPassword: () => null, + setPassword(p: string): void { + base.setPassword(p); + }, + deleteCredential: () => false, + }; + } + override findAccounts(): string[] { + throw new Error('keychain store unreachable (locked / no-access)'); + } + } + + const unreachableStorage = new KeyringTokenStorage({ + keyring: new UnreachableStoreKeyring(), + legacy, + }); + await legacy.save('kimi-code', token); // lingering plaintext to clear + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + + // The unreachable store must surface as a failure... + await expect(unreachableStorage.remove('kimi-code')).rejects.toThrow(/unreachable/); + // ...but the legacy plaintext cleanup must still have run. + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + }); + + it('remove() resolves for a genuinely-missing entry (deleteCredential false, findAccounts lacks name)', async () => { + // FAILS-guard: pins missing⇒no-op parity with FileTokenStorage's ENOENT path + // and guards against over-throwing. deleteCredential() returns false (no such + // entry), getPassword() returns null, and the reachable store's findAccounts() + // returns a list WITHOUT `name`, so remove() must RESOLVE (no throw) while + // still clearing any plaintext copy. (On the pre-fix code this also resolved, + // so this is a parity guard; the two throwing tests above are the fix proofs.) + const token = sampleToken(); + + class MissingEntryKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + getPassword: () => null, + setPassword(p: string): void { + base.setPassword(p); + }, + deleteCredential: () => false, + }; + } + // Reachable store that lists OTHER accounts but not the one being removed. + override findAccounts(): string[] { + return ['some-other-account']; + } + } + + const missingStorage = new KeyringTokenStorage({ keyring: new MissingEntryKeyring(), legacy }); + await legacy.save('kimi-code', token); // a plaintext copy that must still clear + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(true); + + await expect(missingStorage.remove('kimi-code')).resolves.toBeUndefined(); + expect(existsSync(join(dir, 'kimi-code.json'))).toBe(false); + }); + it('list() unions keyring accounts and un-migrated legacy names, deduped', async () => { await storage.save('alpha', sampleToken()); // lands in keyring await legacy.save('beta', sampleToken()); // un-migrated plaintext From 8d5d6ca2bca093cbd3a90ff719dd864156a1caa8 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 15 Jun 2026 13:38:15 +0800 Subject: [PATCH 21/22] fix(oauth): confirm probe sentinel deletion via service listing (consistent with remove()) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The capability probe proved sentinel deletion with getPassword()===null, which the v1.3.0 binding can't distinguish from a read error (it collapses every read error to null). It now confirms via findAccounts (throws on an unreachable store, lists accounts when reachable) — the same proof-of-absence as remove() — and it still catches a lying deleteCredential() that reports success while the entry survives. --- packages/oauth/src/keyring-storage.ts | 20 ++++-- packages/oauth/test/keyring-storage.test.ts | 76 +++++++++++++++++++-- 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/packages/oauth/src/keyring-storage.ts b/packages/oauth/src/keyring-storage.ts index d4c1e72ab..a80cf40fc 100644 --- a/packages/oauth/src/keyring-storage.ts +++ b/packages/oauth/src/keyring-storage.ts @@ -434,13 +434,21 @@ function probeKeyring(keyring: KeyringApi): boolean { // The keychain is the AUTHORITATIVE store once selected, so it MUST also be // able to DELETE — logout/revocation and load()'s migrate-then-delete all // depend on it. The native binding never throws and maps a failed delete to - // `false`, so we do NOT trust the boolean: delete the sentinel and confirm it - // is actually gone via a re-read (mirrors remove()'s own disambiguation). A - // backend that stores+reads but cannot delete would trap migrated tokens it - // can never remove and make logout throw — reject it here so we fall back to - // the plaintext file store instead of migrating into a one-way keychain. + // `false`, so we do NOT trust the boolean. A backend that stores+reads but + // cannot delete would trap migrated tokens it can never remove and make + // logout throw — reject it here so we fall back to the plaintext file store + // instead of migrating into a one-way keychain. entry.deleteCredential(); - return entry.getPassword() === null; + // Confirm the delete the same non-error-ambiguous way remove() does. The binding + // collapses a denied delete to `false` AND a denied/errored read to `null`, so + // `getPassword() === null` cannot prove the sentinel is gone (equally "deleted" or + // "read denied"). findAccounts() THROWS on an unreachable store (→ caught below → + // unusable) and lists accounts when reachable; our UNIQUE per-process probe + // account being ABSENT from that listing proves the backend can truly delete (and + // also catches a "lying" deleteCredential() that returns success while the entry + // survives). Checking only our own unique account keeps this correct under + // concurrent probes. + return !keyring.findAccounts(KEYRING_PROBE_SERVICE).includes(account); } catch { return false; } finally { diff --git a/packages/oauth/test/keyring-storage.test.ts b/packages/oauth/test/keyring-storage.test.ts index 0e4b4851d..a2a882ee0 100644 --- a/packages/oauth/test/keyring-storage.test.ts +++ b/packages/oauth/test/keyring-storage.test.ts @@ -854,10 +854,11 @@ describe('resolveTokenStorage', () => { it('rejects a keyring whose delete returns true but the entry SURVIVES (lying boolean)', () => { // The native binding maps a failed delete to `false`, but the probe does NOT - // trust the boolean — it confirms removal via a re-read (getPassword() === - // null), mirroring remove()'s own disambiguation. A backend that LIES (delete - // reports true while the entry persists) must still be rejected, proving the - // probe relies on the authoritative re-read, not the return value. + // trust the boolean — it confirms removal via the service-scoped findAccounts() + // listing (our unique probe account must be ABSENT), mirroring remove()'s own + // disambiguation. A backend that LIES (delete reports true while the entry + // persists) must still be rejected, proving the probe relies on the + // authoritative service listing, not the return value. class LyingDeleteKeyring extends FakeKeyring { override createEntry(service: string, account: string): KeyringEntry { const base = super.createEntry(service, account); @@ -876,6 +877,73 @@ describe('resolveTokenStorage', () => { expect(storage).not.toBeInstanceOf(KeyringTokenStorage); }); + it('rejects a keyring whose post-delete read is DENIED (null) while the sentinel SURVIVES (read-error ambiguity)', () => { + // FAILS on the pre-fix probe, PASSES on the fix. The v1.3.0 binding collapses + // every read error to null, so a denied/locked post-delete read returns null + // EVEN THOUGH the sentinel still exists. Here the probe's set + FIRST getPassword + // round-trip succeed, deleteCredential() returns false (denied — sentinel NOT + // removed), and the post-delete getPassword() collapses to null (read denied) — + // BUT findAccounts(KEYRING_PROBE_SERVICE) STILL lists the probe account because + // the entry physically persists. The OLD probe keyed off `getPassword() === null` + // → saw null → returned true → wrongly selected KeyringTokenStorage (a one-way + // keychain that traps migrated tokens). The NEW probe proves absence via the + // service listing, sees its own account still present → returns false → falls + // back to FileTokenStorage. + class DeniedReadSurvivingProbeKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + let reads = 0; + return { + // First read (the round-trip check) returns the stored sentinel; every + // later read collapses to null (denied), as the binding does on error. + getPassword(): string | null { + reads += 1; + return reads === 1 ? base.getPassword() : null; + }, + setPassword(p: string): void { + base.setPassword(p); + }, + // Denied delete: returns false and the sentinel SURVIVES in the store, + // so the faithful findAccounts() still lists this probe account. + deleteCredential: () => false, + }; + } + } + const storage = resolveTokenStorage(dir, { + loadKeyring: () => new DeniedReadSurvivingProbeKeyring(), + }); + expect(storage).toBeInstanceOf(FileTokenStorage); + expect(storage).not.toBeInstanceOf(KeyringTokenStorage); + }); + + it('rejects a keyring whose post-delete findAccounts THROWS (unreachable store mid-probe)', () => { + // A locked / no-access store discovered only at the post-delete confirmation: + // set + the first read succeed, deleteCredential() returns false, but the + // service-scoped findAccounts() THROWS (findCredentials throws on an + // unreachable store). The probe's findAccounts throw is caught → probe returns + // false → FileTokenStorage, never migrating plaintext into an unusable keychain. + class UnreachableProbeKeyring extends FakeKeyring { + override createEntry(service: string, account: string): KeyringEntry { + const base = super.createEntry(service, account); + return { + getPassword: () => base.getPassword(), + setPassword(p: string): void { + base.setPassword(p); + }, + deleteCredential: () => false, + }; + } + override findAccounts(): string[] { + throw new Error('keychain store unreachable (locked / no-access)'); + } + } + const storage = resolveTokenStorage(dir, { + loadKeyring: () => new UnreachableProbeKeyring(), + }); + expect(storage).toBeInstanceOf(FileTokenStorage); + expect(storage).not.toBeInstanceOf(KeyringTokenStorage); + }); + it('selects KeyringTokenStorage and leaves NO probe sentinel behind on the healthy path', () => { // The healthy fake deletes properly (Map.delete mutates the backing store), // so the probe's delete-and-re-read confirms removal → keyring is selected. From 9a46a9956163278aec86eaacc0060a8c10bf4f09 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 15 Jun 2026 14:48:26 +0800 Subject: [PATCH 22/22] chore: add changeset for keychain credential storage --- .changeset/keychain-credential-storage.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/keychain-credential-storage.md diff --git a/.changeset/keychain-credential-storage.md b/.changeset/keychain-credential-storage.md new file mode 100644 index 000000000..26f6894c2 --- /dev/null +++ b/.changeset/keychain-credential-storage.md @@ -0,0 +1,6 @@ +--- +"@moonshot-ai/kimi-code": minor +"@moonshot-ai/kimi-code-oauth": minor +--- + +Store OAuth credentials in the OS keychain (macOS Keychain, Windows Credential Manager, Linux Secret Service) by default, falling back to the plaintext file store when the keychain is unavailable — unsupported platform, missing/locked backend, native binary not loadable, or `KIMI_DISABLE_KEYRING=1`. Existing plaintext credentials are migrated into the keychain on first read and then deleted, so users stay logged in and the on-disk secret is removed. Set `KIMI_DISABLE_KEYRING=1` to opt out and keep using the plaintext file store.