fix(cli): keep the --pkce refresh token in the OS keychain, not in token.json - #37665
Conversation
`lite login --pkce` mints a refresh token that buys a fresh key from the proxy on demand, so it is the credential just as much as the key is. Moving the key into the keychain left it behind in ~/.litellm/token.json, where any process running as the user can read it and renew the login for itself. It now travels with the key: `save_cli_token` writes both into the keychain entry, the token file keeps only metadata, and `lite logout` takes it out of the file whether or not the keychain answers. Upgrading finds one sign-in split across the two stores, the key already in the keychain and the refresh token still on disk. That case rejoins the two halves into a single entry before scrubbing the file, so the write never replaces a live key with nothing, and a machine that refuses the scrub keeps what it has rather than having the key rolled back out from under it.
Greptile SummaryThis PR hardens CLI credential storage by keeping PKCE refresh tokens alongside access keys in the OS keychain while retaining the owner-only file fallback.
Confidence Score: 5/5The PR appears safe to merge, with no concrete blocking or non-blocking defect identified in the changed behavior. The new refresh-token handling preserves coherent credentials across successful keychain storage, legacy migration, renewal, unavailable-keychain fallback, and documented file-scrubbing failures.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/cli_token_utils.py | Moves refresh tokens through the existing keychain lifecycle, including save, load, migration, arbitration, scrubbing, fallback, and upgrade rejoin behavior; no actionable defect was established. |
| tests/test_litellm/litellm_core_utils/test_cli_token_utils.py | Adds focused coverage for refresh-token round trips, legacy split-store upgrades, fallback storage, and keychain/file failure branches without weakening existing assertions. |
| litellm/proxy/client/README.md | Updates the in-tree CLI authentication documentation to describe keychain storage for PKCE refresh tokens and the owner-only fallback. |
Reviews (1): Last reviewed commit: "fix(cli): keep the refresh token in the ..." | Re-trigger Greptile
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
bugbot run |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 0f1e09b. Configure here.
TLDR
Problem this solves:
lite login --pkceleaves its refresh token in a cleartext fileHow it solves it:
User Flow
Before: a developer signs in with
lite login --pkce, and although their key is out of reach in the OS keychain, the refresh token that mints keys is in a cleartext file, so any program running as them can trade it for a working keylite --base-url https://litellm-domain login --pkce, approves the sign-in in the browser, and seesLogin successful!followed byCredential stored in your OS keychain.cat ~/.litellm/token.jsonand see no key, but arefresh_tokenin the clear next to their gateway URL and atoken_endpointsaying where to send itgrant_type=refresh_tokenand that value, getting a 200 and a key of its ownlitecommand renews it silently, andcat ~/.litellm/token.jsonshows the rotated refresh token sitting in the file exactly as beforeAfter: the same sign-in puts both halves in the keychain, so the file has nothing left for that program to trade
lite --base-url https://litellm-domain login --pkce, approves the sign-in in the browser, and seesLogin successful!followed byCredential stored in your OS keychain.cat ~/.litellm/token.jsonand see their gateway URL, user id, role, sign-in time, and endpoints, with no key and no refresh token in itlitecommand renews it silently, andcat ~/.litellm/token.jsonstill shows neither half;security find-generic-password -s litellm-cli -a credential -won macOS, or Credential Manager on Windows, holds the renewed pairlite logoutstill sends POST https://litellm-domain/revoke first, so replaying that refresh token at POST https://litellm-domain/token afterwards comes back 400invalid_grant~/.litellm/token.json, readable only by the developer, and everything keeps workingBefore, a second program running as that developer could mint keys for their whole role for as long as the login lived, on any machine the token file was copied to. After, it finds nothing in the file to mint with, and the OS gates the keychain entry that holds the pair
Relevant issues
Docs for this change: BerriAI/litellm-docs#967
Linear ticket
Resolves LIT-5855
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/test_litellm/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or more@greptileaito re-request a review after pushing changes)Delays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Shared setup for every case below: a proxy built from the commit under test, on a random free port, with a real Postgres and a real Anthropic key, and a stub SSO provider standing in for the company IdP
Each case runs in a throwaway
$HOMEwith thelitellm-clikeychain entry deleted first, so it starts from a machine that has never signed in. Two commands do the work in every case, the sign-in and one real provider call:The browser half of the sign-in, opening the authorize URL and approving the consent page, is driven with
curlso the run is scriptable. The provider-call steps show thecontentfield taken out of the 200 the proxy answered with, so the sentence in it names the case and the commit that call ran against. Two small helpers print what the token file and the keychain entry hold, truncating any secret to...REDACTED..., and a third prints sha256 fingerprints (first 12 characters) so the same secret can be recognized across the two stores without printing it. The second process is this, a program with the developer's file access and nothing else:Before (edbb342)
lite login --pkce, and what another program running as the user can liftlitedoesSilent renewal, then
lite logout$HOMEand fingerprint both storesA machine with no usable keychain
A machine upgrading from the previous release
After (0f1e09b)
lite login --pkce, and what another program running as the user can liftlitedoesSilent renewal, then
lite logoutA machine with no usable keychain
A machine upgrading from the previous release
lite, so the machine starts in the state that release leaves behindType
🐛 Bug Fix
Caveats (if any)
LITELLM_CLI_DISABLE_KEYRING=1boxes keep today's exposureThe upgrade path rejoins the two halves and then writes them back as one entry, so a machine whose keychain answers reads but refuses writes keeps its refresh token in
token.jsonand tries again on the next command. That is the same pre-flight cost #37566 named for the key, andLITELLM_CLI_DISABLE_KEYRING=1is still the way off it. Nothing is lost when the write fails: the file keeps what it had, and the key already in the keychain is left alone rather than rolled backThe rejoin only merges a file half and a keychain half that carry the same sign-in stamp, which is one login split across the two stores by the upgrade. Halves with different stamps are two different sign-ins, and the newer one wins whole, exactly as it did before this PR
A
--pkcecredential renews itself, and the refresh token that does it now lives where the key does, so an install that cannot reach the keychain cannot renew either. That is the same boundary the key was already behind: an install without thecliextra could not read the key it needed, and now it cannot read the refresh token eitherA machine that takes this release and then goes back to the previous one has its refresh token in a store that release does not read, so silent renewal stops and
lite logoutno longer has the token it revokes with, which leaves that refresh token live on the gateway until it expires. Nothing is lost or corrupted: the previous release still reads the key out of the keychain and keeps serving requests on it, both stores stay exactly as they were,lite login --pkceputs the machine back on a renewing credential, and coming back to this release picks the pair up again and renews on it. Writing the refresh token to both stores through the transition would make the downgrade seamless, and it would also keep the cleartext copy this PR exists to remove, so the downgrade pays the cost insteadOn a machine whose keychain cannot be read, two strings get vaguer.
lite upsays to runlite loginwhere it used to saylite login --pkce, andlite whoamidrops the ", renewed on next use" it appends to the expiry. Both read the same presence test, whether the record in hand carries a refresh token, and a keychain that cannot be read makes the answer no. The line directly above each of them already names the real problem, that the credential is in the keychain and something is blocking it, so the user is told what to fix either wayFinal Attestation
0f1e09b5558160ad18db40387d63ebb90a1daf4apasses /live-pr-risk, with its findings recorded under CaveatsNote
High Risk
Changes how CLI auth secrets (including refresh tokens that mint new keys) are stored, migrated, and scrubbed. Incorrect rejoin or rollback logic could leak credentials or drop a live login.
Overview
Stops
lite login --pkcefrom leaving the refresh token in~/.litellm/token.json. The refresh token is now treated as secret material and stored in the OS keychain with the bearer key (or in the same 0600 file when there is no keychain).On load, a split upgrade state (key already in the keychain, refresh token still on disk) is rejoined only when both halves share the same sign-in stamp, then migrated and scrubbed. Migration will not roll back a pre-existing keychain entry if the file cannot be rewritten. Different stamps still pick the newer login whole, so an old refresh token is never paired with a newer key.
CliTokenSecret.keyis optional so a file that holds only a refresh token can move into the keychain without inventing a key. Logout still strips refresh tokens from disk even when the keychain cannot be cleared.Reviewed by Cursor Bugbot for commit 0f1e09b. Bugbot is set up for automated code reviews on this repo. Configure here.