feat(cli): store the lite login credential in the OS keychain - #37566
Merged
mateo-berri merged 26 commits intoAug 20, 2026
Conversation
lite login used to write the minted cli-session key in cleartext to ~/.litellm/token.json. The secret material (key plus any JWT) now goes to the OS keychain through the optional keyring package, with the 0600 file kept for non-secret metadata and as the fallback on headless boxes. Legacy plaintext files keep authenticating and are migrated into the keychain, then scrubbed, on first read. A secret still on disk always outranks the keychain entry, so a failed keychain write can never resurrect a stale key. LITELLM_PROXY_API_KEY and --api-key precedence is unchanged, lite logout clears both stores and warns when the keychain will not release the entry, and ~/.litellm is created 0700 (tightened from 0755 where an older CLI left it broader). LITELLM_CLI_DISABLE_KEYRING=1 forces the file fallback.
Drop the inline notes on keychain erasure and disk-vs-vault precedence in favour of docstrings on the two functions that own those rules, and remove a stale section header and a field note that the code already says plainly.
Contributor
Greptile SummaryThis PR moves Lite CLI credentials into the operating-system keychain while retaining private file fallback and legacy-token migration.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains.
|
| Filename | Overview |
|---|---|
| litellm/litellm_core_utils/cli_keyring.py | Adds the optional OS-keychain abstraction, bounded write preflight, read-back verification, and explicit result types. |
| litellm/litellm_core_utils/cli_token_utils.py | Splits secret and metadata storage, migrates legacy credentials, orders competing store entries, and tracks incomplete logout cleanup. |
| litellm/litellm_core_utils/private_json.py | Adds owner-only atomic JSON staging, replacement, and in-place overwrite helpers. |
| litellm/proxy/client/cli/commands/auth.py | Integrates the new storage outcomes into login, logout, identity, and token-printing behavior. |
| pyproject.toml | Adds keyring support to the CLI dependency extra. |
| tests/test_litellm/litellm_core_utils/test_cli_token_utils.py | Exercises migration, competing-store ordering, clock movement, rollback, fallback, and repeated-logout behavior. |
| tests/test_litellm/proxy/client/cli/test_auth_commands.py | Verifies user-facing authentication behavior for successful and degraded credential-storage outcomes. |
Reviews (22): Last reviewed commit: "test(cli): pin the shared stamp's effect..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
lite ships with every install of litellm, but the keyring package it needs for keychain storage only ships with the cli extra. Such a user on a Mac was told 'No OS keychain available' about a machine that plainly has one, with nothing pointing at the missing package. The vault now reports which of the three unusable states it is in, so login can point at the install, name the kill switch, or report a genuinely absent keychain.
…itellm_cli_refresh_tokens # Conflicts: # basedpyright-code-budget.json
Contributor
Author
|
bugbot run |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Logout reports success without keychain
- Removed the early returns in KeyringVault.erase so a missing or disabled keyring package now maps through read() to False, causing lite logout to warn instead of silently claiming success when a credential from another environment may still live in the OS keychain.
Or push these changes by commenting:
@cursor push 4225a53d9e
Preview (4225a53d9e)
diff --git a/litellm/litellm_core_utils/cli_keyring.py b/litellm/litellm_core_utils/cli_keyring.py
--- a/litellm/litellm_core_utils/cli_keyring.py
+++ b/litellm/litellm_core_utils/cli_keyring.py
@@ -120,13 +120,10 @@
def erase(self) -> bool:
"""Whether the keychain is guaranteed to hold no credential afterwards.
- An uninstalled `keyring` package can never have stored one. A kill switch set after
- a credential was stored leaves that entry out of reach, so erasure cannot be promised.
+ An uninstalled or disabled `keyring` package leaves any credential a different
+ environment (e.g. an install with the `cli` extra) stored under this service out
+ of reach, so erasure cannot be promised. A locked keychain is the same story.
"""
- if _import_keyring() is None:
- return True
- if _keyring_disabled():
- return False
match self.read():
case KeyringNotInstalled() | KeyringDisabled() | KeyringUnreachable():
return False
diff --git a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
--- a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
+++ b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
@@ -402,14 +402,16 @@
def test_an_uninstalled_keyring_library_degrades_to_the_file(self, monkeypatch):
"""keyring is an optional extra, so the SDK must survive its absence rather than raise on
- the hot path."""
+ the hot path. Erase still fails: a credential stored from another environment (e.g. an
+ install with the `cli` extra) may be in the keychain, and without keyring `lite logout`
+ cannot verify it is gone, so it must warn instead."""
monkeypatch.delenv(DISABLE_KEYRING_ENV_VAR, raising=False)
monkeypatch.setitem(sys.modules, "keyring", None)
vault = KeyringVault()
assert vault.read() == KeyringNotInstalled()
assert vault.write("blob-1") == KeyringNotInstalled()
- assert vault.erase() is True
+ assert vault.erase() is False
def test_a_locked_keychain_is_reported_not_raised(self, install_fake_keyring):
install_fake_keyring(_FakeKeyringModule(get_error=RuntimeError("keyring is locked")))You can send follow-ups to the cloud agent here.
Migration moved the secret into the keychain and then suppressed any OSError from rewriting token.json, so a file that could not be rewritten kept the credential in cleartext while every command reported success. That file is now removed instead: signing in again costs one command, a stranded live credential costs the credential `lite logout` also reported a clean logout whenever the keyring package was missing, on the reasoning that an install without it could never have stored anything. The entry belongs to the OS, so a keychain-backed login survives a logout run from a venv without the cli extra. erase() now reports which keychain state applies, and logout warns with the advice that fixes each one, staying quiet for file-backed logins whose token file still carries its own secret Also pins the migration path's tightening of a world-readable legacy token.json, and moves the logout tests off patch() onto the injected vault
… be removed Removing the file when it could not be rewritten covered a full disk, but not a ~/.litellm that permits neither the rewrite nor the delete, which is what a `sudo lite login` leaves behind. There the secret was copied into the keychain and kept in cleartext on disk, so migration widened exposure instead of narrowing it Migration now only keeps the vault copy if the file's copy is gone. When it is not, the write is rolled back and the user is left exactly as they were, logged in with one copy of the credential
Contributor
Author
|
bugbot run |
Contributor
Author
![veria-ai[bot]](https://avatars.githubusercontent.com/in/1723776?s=60&v=4){kind=small}
Contributor
PR overviewThis pull request updates the CLI’s lite login flow to store credentials in the operating system keychain, including changes to CLI token persistence and logout handling. One security issue has already been addressed, but logout can still leave an active refresh token in token.json when revocation fails and the keychain is unavailable. A process able to read that file could redeem the token for new access keys, so logout should always scrub it even if other metadata must remain. Open issues (1)
Fixed/addressed: 1 · PR risk: 6/10 |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Logout success with leftover keychain
- Restricted the
_secret_lives_in_keychainheuristic toKeyringNotInstalled, soKeyringDisabledandKeyringUnreachablenow bubble up tolite logoutasSecretStranded-style warnings instead of being masked asSecretErasedwhen the file still holds its own secret.
- Restricted the
Or push these changes by commenting:
@cursor push 0b10c8c111
Preview (0b10c8c111)
diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py
--- a/litellm/litellm_core_utils/cli_token_utils.py
+++ b/litellm/litellm_core_utils/cli_token_utils.py
@@ -105,13 +105,20 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretErase:
def _nothing_left_behind(outcome: SecretErase) -> bool:
- """Whether the keychain can be trusted to hold no credential of ours once the file is gone"""
+ """Whether the keychain can be trusted to hold no credential of ours once the file is gone.
+
+ Only `KeyringNotInstalled` can lean on the token file: a machine with no keyring package cannot
+ have put a credential in one from this install. `KeyringDisabled` (kill switch flipped after an
+ earlier login) and `KeyringUnreachable` (backend locked or broken) both leave the door open to
+ a live entry the current process cannot see, so a file that has since fallen back to holding its
+ own secret is not proof the keychain is clean.
+ """
match outcome:
case SecretErased():
return True
- case SecretStranded():
+ case SecretStranded() | KeyringDisabled() | KeyringUnreachable():
return False
- case KeyringNotInstalled() | KeyringDisabled() | KeyringUnreachable():
+ case KeyringNotInstalled():
return not _secret_lives_in_keychain()
@@ -105,13 +105,20 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretErase:
def _nothing_left_behind(outcome: SecretErase) -> bool:
- """Whether the keychain can be trusted to hold no credential of ours once the file is gone"""
+ """Whether the keychain can be trusted to hold no credential of ours once the file is gone.
+
+ Only `KeyringNotInstalled` can lean on the token file: a machine with no keyring package cannot
+ have put a credential in one from this install. `KeyringDisabled` (kill switch flipped after an
+ earlier login) and `KeyringUnreachable` (backend locked or broken) both leave the door open to
+ a live entry the current process cannot see, so a file that has since fallen back to holding its
+ own secret is not proof the keychain is clean.
+ """
match outcome:
case SecretErased():
return True
- case SecretStranded():
+ case SecretStranded() | KeyringDisabled() | KeyringUnreachable():
return False
- case KeyringNotInstalled() | KeyringDisabled() | KeyringUnreachable():
+ case KeyringNotInstalled():
return not _secret_lives_in_keychain()
diff --git a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
--- a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
+++ b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
@@ -400,6 +400,19 @@ def test_a_file_backed_login_logs_out_quietly_without_keyring(self, isolated_hom
def test_is_safe_when_nothing_was_ever_stored(self, isolated_home, secret_vault_factory):
assert clear_cli_token(vault=secret_vault_factory()) == SecretErased()
+ @pytest.mark.parametrize("failure", [KeyringDisabled(), KeyringUnreachable()])
+ def test_a_file_that_fell_back_from_a_now_unreachable_keychain_still_warns(
+ self, isolated_home, secret_vault_factory, failure
+ ):
+ """An earlier keychain-backed login could have left an entry a subsequent fallback-to-file
+ login never cleared. When the current process cannot reach the keychain to check, a file
+ that has since regained its own secret is not evidence the keychain is clean."""
+ _write_legacy_file(isolated_home)
+ vault = secret_vault_factory(available=False, failure=failure)
+
+ assert clear_cli_token(vault=vault) == failure
+ assert not _token_file(isolated_home).exists()
+
class TestIsCliTokenFresh:
def test_a_just_issued_token_is_fresh(self):
@@ -400,6 +400,19 @@ def test_a_file_backed_login_logs_out_quietly_without_keyring(self, isolated_hom
def test_is_safe_when_nothing_was_ever_stored(self, isolated_home, secret_vault_factory):
assert clear_cli_token(vault=secret_vault_factory()) == SecretErased()
+ @pytest.mark.parametrize("failure", [KeyringDisabled(), KeyringUnreachable()])
+ def test_a_file_that_fell_back_from_a_now_unreachable_keychain_still_warns(
+ self, isolated_home, secret_vault_factory, failure
+ ):
+ """An earlier keychain-backed login could have left an entry a subsequent fallback-to-file
+ login never cleared. When the current process cannot reach the keychain to check, a file
+ that has since regained its own secret is not evidence the keychain is clean."""
+ _write_legacy_file(isolated_home)
+ vault = secret_vault_factory(available=False, failure=failure)
+
+ assert clear_cli_token(vault=vault) == failure
+ assert not _token_file(isolated_home).exists()
+
class TestIsCliTokenFresh:
def test_a_just_issued_token_is_fresh(self):You can send follow-ups to the cloud agent here.
…t done A keyring backend can accept a write and keep nothing. That is exactly what `keyring --disable` and PYTHON_KEYRING_BACKEND=keyring.backends.null.Keyring select, and it raises nothing to distinguish itself, so `lite login` was handing the credential to a black hole, scrubbing its own copy from token.json, and printing a success message over a login that no longer worked. Reading the value back is the only way to tell that backend apart from a keychain that really stored the secret. The same rule closes the rest of the gaps. A credential the token file will not record is taken back out of the keychain instead of being left live on a machine with no record of it, and is reported rather than raised. The migration stages its scrubbed file before the keychain is handed anything, so a directory that will not accept the rewrite stops the move rather than leaving the secret in two places. Logout no longer reads a key in the file as proof that the keychain is clear, which was never sound across two separate runs, and only draws that conclusion when the `keyring` package is missing outright, where nothing could have reached a keychain at all.
Contributor
Author
|
bugbot run |
Contributor
Author
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Failed save wipes a findable login
- save_cli_token now consults the surviving token.json's base_url and, when it still pairs with the freshly stored vault entry, keeps the secret instead of erasing it and reporting the credential lost.
Or push these changes by commenting:
@cursor push 1e60366e2c
Preview (1e60366e2c)
diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py
--- a/litellm/litellm_core_utils/cli_token_utils.py
+++ b/litellm/litellm_core_utils/cli_token_utils.py
@@ -108,7 +108,10 @@
The token file is what makes a keychain-backed credential findable again, so a file that will
not be written takes the keychain copy down with it rather than leaving a live credential
- stored under a machine that has no record of it.
+ stored under a machine that has no record of it. A same-server re-login is the exception: an
+ earlier successful save's file survives an atomic rewrite that never lands, and still pairs
+ with the vault slot the new secret just replaced, so the login is findable and the vault
+ entry stays.
"""
outcome: Final = (
SecretStored()
@@ -119,11 +122,26 @@
_write_token_file(_without_secret(record) if isinstance(outcome, SecretStored) else record)
except OSError as error:
if record.key is not None and isinstance(outcome, SecretStored):
+ if _existing_file_pairs_with(record.base_url):
+ return outcome
vault.erase()
return CredentialNotSaved(str(error))
return outcome
+def _existing_file_pairs_with(base_url: str) -> bool:
+ """Whether a surviving token.json still points at this same-server login.
+
+ `write_private_json` is atomic: it fails on the staged temp file, so an aborted metadata
+ rewrite leaves the previous file untouched. `_apply_vault_secret` pairs that file with the
+ vault entry we just refreshed whenever the base_url still matches, so the credential is
+ findable on the next call even though the rewrite that would have refreshed the metadata
+ did not land.
+ """
+ existing: Final = _read_token_file()
+ return existing is not None and existing.base_url == base_url
+
+
def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretErase:
"""Remove the credential from both stores. Reports whether the keychain is now free of it"""
outcome: Final = vault.erase()
diff --git a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
--- a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
+++ b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py
@@ -346,6 +346,47 @@
assert vault.blob is None
+ def test_a_same_server_relogin_keeps_the_new_secret_when_the_prior_file_still_pairs(
+ self, isolated_home, secret_vault_factory, monkeypatch
+ ):
+ """`write_private_json` fails on the staged temp file, so a prior successful save's
+ token.json survives an aborted rewrite. Its base_url still pairs with the vault slot the
+ new secret just replaced, so the login is findable and the fresh secret must not be
+ erased on top of the one it just overwrote."""
+ _write_metadata_only_file(isolated_home)
+ vault = secret_vault_factory(blob=_blob(key="sk-old"))
+
+ def _explode(*args, **kwargs):
+ raise OSError("read-only file system")
+
+ monkeypatch.setattr("litellm.litellm_core_utils.private_json.json.dump", _explode)
+
+ outcome = save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-new"), vault=vault)
+
+ assert outcome == SecretStored()
+ assert json.loads(vault.blob)["key"] == "sk-new"
+ assert vault.erases == 0
+ assert load_cli_token(vault=vault).key == "sk-new"
+
+ def test_a_different_server_relogin_still_erases_when_the_prior_file_cannot_pair(
+ self, isolated_home, secret_vault_factory, monkeypatch
+ ):
+ """A prior file pointing at another server does not make the new secret findable: the
+ vault entry would be stranded under metadata that names the wrong base_url, so it has to
+ come back out."""
+ _write_metadata_only_file(isolated_home)
+ vault = secret_vault_factory(blob=_blob(key="sk-old"))
+
+ def _explode(*args, **kwargs):
+ raise OSError("read-only file system")
+
+ monkeypatch.setattr("litellm.litellm_core_utils.private_json.json.dump", _explode)
+
+ outcome = save_cli_token(CliTokenRecord(base_url=OTHER_SERVER, key="sk-new"), vault=vault)
+
+ assert isinstance(outcome, CredentialNotSaved)
+ assert vault.blob is None
+
def test_a_failed_write_leaves_the_previous_credential_intact(self, isolated_home, secret_vault_factory, monkeypatch):
path = _write_legacy_file(isolated_home)
before = path.read_text()You can send follow-ups to the cloud agent here.
`make lint` hands every path in the diff against the base branch to `ruff format --check`, including the ones the branch deleted, so any branch that moves or removes a file under `litellm/` fails the gate with "No such file or directory" instead of a formatting complaint. test-linting.yml already filters those out with `--diff-filter=ACMR`, so the Makefile was the half that drifted. Match it.
…t answer Three ways the credential commands could mislead or hang. `lite logout` on a machine that never logged in warned that a credential may be stranded in a keychain it could not check, and told the user to install keyring to go clear it. There was nothing there. A missing token file is now read as the evidence it is, because logout keeps a secret-free file behind whenever the keychain is left unconfirmed, so a later run can tell a machine with a credential it cannot reach apart from one that never had a login. That holds on the LITELLM_CLI_DISABLE_KEYRING path too. `KeyringDiscardsWrites` was handled on the read and erase paths, which cannot produce it: the null backend returns None from `get_password` rather than raising, so only a write ever detects it. It now lives on `SecretWrite` alone and the unreachable arms are gone. `keyring.set_password` blocks forever under a HOME with no usable login keychain, which is what containers, CI images, `sudo -H`, and service accounts run with, and reads answer normally there so nothing cheaper tells them apart. `lite login` never touched a keychain before this, so a sign-in that simply never returns would be a new way for it to fail. Writes are pre-flighted with a throwaway value on a bounded wait, and a keychain that stays silent falls back to the token file. The real credential is never the thing handed to a call that might land long after we stopped waiting. Saving also stages the token file before the keychain is given anything, since the file is the half a read-only or full directory refuses. A save that cannot land now leaves both stores as it found them, which matters most when the login it failed to replace still works.
…name Staging the token file can succeed and the replacement still fail afterwards, and that is the one save path where the keychain has already taken the new secret. It was reported as a save that kept nothing, which sends the user looking for a credential that is sitting in their keychain, and it claimed the previous login was untouched when the one keychain slot had just been written over. Give that path its own outcome and its own notice. The new secret stays where it is: the entry it replaced went the moment it landed, so no rollback brings that back, and removing the new one too would turn a login this machine may still be able to use into no login at all. The remaining `CredentialNotSaved` paths all leave both stores untouched, so the reassurance they carry is now true wherever it is printed.
Contributor
Author
A logout that could not reach the keychain deleted the token file whenever it still held its own secret, and the next logout read that missing file as proof the keychain was clean. It answered the warning the first run had just issued with "Logged out successfully" while the entry an earlier login left behind was still live. The file is the only record that something may still be in there, which is what `_nothing_left_behind` already says it relies on, so keep it and take only the secret out. A keychain that did answer is a different case. `SecretStranded` means the entry is confirmed there and would not delete, and that needs no note in the file, while keeping one lets every later command read the credential straight back out of the keychain, which makes "Logged out locally" untrue. That one drops the file, as it did before. The secret still goes first either way: a copy that cannot be replaced with a secret-free one is removed rather than kept.
The two stores hold different credentials on that path, so the rollback a migration does would hand the superseded one back out. The login that could not replace the file already named the state, and logout reports it too.
Contributor
Author
added 2 commits
August 20, 2026 04:53
The stamp in the keychain entry is what decides that secret against one still sitting in the token file, and it came straight off the wall clock. A clock that stepped backwards between two logins therefore handed the win to the older of them: a login the keychain took but the token file could not be pointed at was resolved back to the credential it replaced, and the fresh one was erased from the keychain on the way past. save_cli_token now reads the stamp already on disk and pins the new sign-in just above it, so the ordering never depends on the clock having moved forwards. On a clock that did, this changes nothing.
The comment above the extra named cryptography as one of the heavy imports a thin install leaves out. That stopped being true when keyring joined the extra: on Linux it reaches the Secret Service through secretstorage, which depends on cryptography.
Contributor
Author
A login the keychain took but the token file could not record leaves the keychain naming a later sign-in than the file does. Reading only the file then stamps the next login below that keychain entry, and a clock that went back far enough puts the superseded credential back in use.
Contributor
Author
Contributor
Author
FakeSecretVault could only stand in for a discarding backend by passing KeyringDiscardsWrites as its `failure`, which also made read() and erase() hand it back. Neither SecretRead nor SecretErase admits that outcome and the real KeyringVault never produces it there, so the login path's match was falling through on a value it can never see. Give the double a `discards` flag that reports it from write() alone, which is what the null backend does. Also widen lint-format-check-changed's pathspec. Git wildmatch runs without FNM_PATHNAME here, so 'litellm/**/*.py' still requires an intermediate directory and silently skipped all 21 top-level modules, litellm/__init__.py and litellm/main.py among them. All 21 already pass ruff format.
Contributor
Author
Contributor
Author
|
bugbot run |
Contributor
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
Autofix Details
Bugbot Autofix prepared a fix for the issue found in the latest run.
- ✅ Fixed: Format check skips nested Python files
- Restored the recursive
litellm/**/*.pypathspec inlint-format-check-changedso it matches CI'stest-linting.ymland covers nested modules.
- Restored the recursive
Or push these changes by commenting:
@cursor push 4769a37798
Preview (4769a37798)
diff --git a/Makefile b/Makefile
--- a/Makefile
+++ b/Makefile
@@ -146,7 +146,7 @@
# only the litellm Python files changed vs the base are checked, so a pre-existing
# format issue elsewhere doesn't block an unrelated commit.
lint-format-check-changed: $(LINT_DEP_INSTALL) $(LINT_DEP_BASE)
- @files=$$(git diff --name-only --diff-filter=ACMR origin/litellm_internal_staging...HEAD -- 'litellm/*.py' | grep -v '^litellm/enterprise/' || true); \
+ @files=$$(git diff --name-only --diff-filter=ACMR origin/litellm_internal_staging...HEAD -- 'litellm/**/*.py' | grep -v '^litellm/enterprise/' || true); \
if [ -z "$$files" ]; then \
echo "No changed litellm Python files to format-check."; \
else \You can send follow-ups to the cloud agent here.
The stamp both orders the two stores and drives is_cli_token_fresh, and nothing tied the two together, so a login that inherits a stamp from the future could stop being a deliberate trade without anything failing. Also corrects the lint-format-check-changed comment: git pathspecs match recursively, so the target checks a superset of the CI step rather than an identical set.
Contributor
Author
Contributor
Author
|
You withdrew this at line 422, then scored 5/5 on code identical to this tip. A failed rollback retries on the next read |
Contributor
|
That matches what the code does: after a failed rollback, |
Contributor
Author
Contributor
Author
|
bugbot run |
Contributor
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 1fe06a1. Configure here.
mateo-berri
enabled auto-merge
August 20, 2026 13:39
yucheng-berri
approved these changes
Aug 20, 2026
Base landed the native CLI OAuth + PKCE login, which added its own token storage and a silent refresh that wrote the key straight to token.json. This branch had already moved that secret into the OS keychain, so the two had to be joined rather than picked between. auth.py now keeps one pair of record helpers, load_token and save_token, that read and write through the vault and hand the PKCE layer the plain mapping it works with. fresh_api_key and revoke_stored_credential get vault-bound save and reload callables, so a renewed key is stored in the keychain like any other and a sibling process's rotation is still seen. login goes through _replace_stored_token on both paths, so the credential it replaces is revoked on the proxy and the user is still told where the new one landed. logout revokes first, then reports what the clear actually managed to do.
|
|
||
| def _scrub_file_secret(record: CliTokenRecord) -> bool: | ||
| """Leave no secret material in the token file once the vault holds it""" | ||
| if record.key is None and not record.jwt_token: |
Contributor
There was a problem hiding this comment.
Medium: Refresh token remains after logout
A keychain-backed PKCE record has key=None and an empty jwt_token, so this returns without removing its refresh_token. If revocation returns PkceFailure and the keychain is unavailable, logout continues through this path and leaves the active refresh token in token.json; a process that later reads the file can redeem it for new access keys. Treat the refresh token as secret material—preferably storing it in the vault—and ensure the logout scrub removes it even when the metadata note must remain.
tin-berri
approved these changes
Aug 20, 2026
mateo-berri
added a commit
to BerriAI/litellm-docs
that referenced
this pull request
Aug 20, 2026
* docs(cli): move the `lite login` credential to the OS keychain `lite login` now stores the credential in the OS keychain and keeps only non-secret metadata in ~/.litellm/token.json, falling back to that owner-only file when no keychain is available. Update the CLI, SSO, and identity provisioning pages, and document LITELLM_CLI_DISABLE_KEYRING. See BerriAI/litellm#37566 * docs(cli): say the proxy extra leaves out keyring The quick start offered litellm[proxy] as an equivalent way to get the lite command, so a reader who took that path never reached a keychain and nothing told them why. Also names the SDK getter, which needs keyring for the same reason and returns None without it even when lite login stored a credential.
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TLDR
Problem this solves:
lite loginwrites the proxy credential to a cleartext file~/.litellmwas created traversable by other accountsHow it solves it:
litekeep working--pkcerefresh token is not moved yet, see CaveatsUser Flow
Before: a developer signs in to their company gateway, and the credential that grants their whole role sits in a cleartext file, so any program running as them can lift it and spend against their account
lite --base-url https://litellm-domain login, finishes SSO in the browser, and seesLogin successful!lite models listand get their model list backls -ld ~/.litellmand seedrwxr-xr-x, traversable by other accounts on the boxcat ~/.litellm/token.jsonand read the credential itself in the clear, in akeyfield next to their gateway URL and user idAfter: the same login puts the credential in the OS keychain, so the file no longer carries it and the OS gates who may read it
lite --base-url https://litellm-domain login, finishes SSO in the browser, and seesLogin successful!followed byCredential stored in your OS keychain.lite models listand get their model list backls -ld ~/.litellmand seedrwx------, reachable only by themcat ~/.litellm/token.jsonand see only their gateway URL, user id, role, and sign-in time; there is no credential in the file~/.litellm/token.jsonfinds no credential, and the request it sends without one comes back 401security find-generic-password -s litellm-cli -a credential -won macOS, or open Credential Manager on Windows, and the credential is there, held by the OS keychainliteis not asked to sign in again: their existing credential still authenticates on the next command, moves into the keychain, and is wiped from the filelite logoutclears the keychain entry as well as the file, and warns rather than reporting success if the keychain refuses to release it~/.litellmwill accept no new file,lite logoutstill printsLogged out successfully, andcat ~/.litellm/token.jsonshows the credential gone, instead of handing the developer instructions to delete the file themselveslite whoamiopens withSigned in, but the credential cannot be readrather thanAuthenticated, so the developer knows why their next request failsLITELLM_PROXY_API_KEYor passing--api-keystill wins over the stored credential, unchanged~/.litellm/token.jsoncannot be replaced, a freshlite loginstill takes effect: the CLI says the file could not be replaced, and the next command authenticates with the credential just minted rather than the superseded one the file still names, and that still holds when the machine's clock has moved backwards since the previous sign-inlite login --pkceinstead gets the same result:Credential stored in your OS keychain., and no key in~/.litellm/token.jsonlitecommand renews it without asking; the renewed key goes into the keychain too, socat ~/.litellm/token.jsonstill shows no keylite logouton that credential sends POST https://litellm-domain/revoke first, so the refresh token stops working on the server as well as disappearing from the machine--pkcesign-in does still leave in~/.litellm/token.jsonis the refresh token, so a build script reading that file can trade it at POST https://litellm-domain/token for a key that works; closing that is the follow-up named under CaveatsBefore, a second program running as that developer could reach every route their role allows, on any machine the file was copied to. After, a
lite logincredential leaves it nothing to lift and the OS gates the keychain entry; a--pkcecredential leaves it the refresh token, which is narrower but not yet nothingRelevant issues
Linear ticket
Resolves LIT-5855
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@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
Every leg below is the shipped
liteCLI driven end to end against a live proxy, real Postgres, and real Anthropic calls onanthropic/claude-haiku-4-5. No mocks, no pytest, no DB queries, no LiteLLM internals imported as evidence. The browser half of SSO is stood in for by curl against the proxy's own/sso/key/generateand/sso/cli/complete/<login_id>endpoints, and the browser half of PKCE by curl against/authorizeand/authorize/complete, with a local OIDC stub as the identity provider. Everything else is what an end user types.lite loginprints only the first 20 characters of the credential it minted, and every credential in these logs is truncated further to 12.b0911585d7d8fbb571b59431ecd1b2b21f1b781c, which is this merge's second parent, so it is this branch with the PR taken away. Proxy on port 97155f7c0e1e49c70b030aad11d38dcf82b1f0149a0c. Proxy on port 9714pwdand thelitellm.__file__it loaded, so neither side can be the other tree by accidentBefore, at
b0911585d7d8fbb571b59431ecd1b2b21f1b781cA sign-in, then a second process running as the same user reading the credential straight out of
~/.litellm/token.jsonand spending it on a real provider call.~/.litellmisdrwxr-xr-xhere, and the keychain is never touched.The full before leg
After, at
5f7c0e1e49c70b030aad11d38dcf82b1f0149a0cThe same sign-in.
token.jsonkeeps the gateway URL, user id, role, header name, and timestamp, and no credential.~/.litellmis nowdrwx------. The same scavenger finds nothing to lift and its request comes back 401, while the CLI's own request goes through.The full after leg
The PKCE flow that arrived in the merge, at the same tip
The base branch grew
lite login --pkcewhile this PR was in review, and it renews the key behind the user's back. That renewal is a second write path into credential storage, so it gets its own leg: sign in with--pkce, expire the stored credential by hand, run one ordinary command, and see where the renewed key lands. It lands in the keychain, not intoken.json. Logout then revokes the refresh token on the proxy for real, and replaying it afterwards is refused.The full PKCE leg, including the silent renewal and the revocation
What this PR does not fix: the refresh token is still in the file
A
--pkcelogin writesrefresh_tokeninto~/.litellm/token.jsonin the clear, and this PR does not move it. The key is in the keychain, so the scavenger above cannot read a bearer token out of the file, but it can POST that refresh token to the proxy's/tokenendpoint and be handed a working one. The file alone still buys access.The gap, demonstrated
This is the remaining half of LIT-5855 and it is tracked as a follow-up. It is scoped small now that the merge landed native refresh and revocation: the refresh token needs to travel in the keychain blob next to the key, the same way the key already does.
The edge cases, re-run at the tip
The merge rewrote
auth.py's save, load, and logout paths into vault-backed adapters, so the failure modes were driven again against the merged code rather than carried over.The six edge-case legs
Reading those in order:
LITELLM_CLI_DISABLE_KEYRING=1keeps the credential in the owner-only file and never touches the machine keychain, and login says so by name. Logout blanks the secret out of the file but leaves the metadata behind and warns, because with the keychain switched off it cannot confirm that an earlier login left nothing therewhoamiand a real request both worktoken.jsoncarrying a key in the clear, the shape an olderlitewrote, is picked up by the next ordinary command: the key moves to the keychain and comes out of the file, with nothing printed about itwhoamiopens withSigned in, but the credential cannot be readinstead ofAuthenticated, andprint-tokensays the same rather than printing nothing$HOMEdoes not inherit the first one's session, because the metadata it needs is in the first one'stoken.json. What it does share is the slot itself, so alite logoutfrom the second$HOMEclears the credential the first one was using, and the first one then reads as not authenticatedWhat surprised me
security find-generic-password -whangs on an approval dialog. PR leaves alone.$HOMEshares. PR causes it.liteREPL never re-runs its group callback per command. PR leaves alone.--pkcerefresh token sits in the token file in the clear. PR leaves alone.Type
🆕 New Feature
Caveats (if any)
keyringinstalls with thecliextracryptography$HOMEcliextra to reach a keychain credential--pkcerefresh token stays intoken.jsonin the clearThe keychain entry is one per operating-system user, so two
$HOMEs on the same account pointed at the same gateway now share one slot where they used to keep a token file each. A second$HOMEdoes not silently inherit the first one's session, because the metadata naming the gateway and the user is still in the first one'stoken.json, so it reads as not authenticated. What they do share is the slot: the last login wins it, and alite logoutfrom either$HOMEclears the credential the other was using. Keying the entry per$HOMEwould restore the split, and it would also mint entries that a logout from any other$HOMEcan never find or clear, which is the stranded-credential failure the logout logic exists to prevent.$HOMEwas never a trust boundary inside one OS account either, since that user could already read every other$HOME's token file, so the split was not buying isolation. It stays one entry per userA sign-in is stamped past the latest stamp either store already holds, which is what keeps the ordering of the two independent of the clock, and it costs
lite loginone keychain read before the write it was going to make anyway. A store left carrying a stamp in the future therefore hands that stamp to the next login as well, where before this PR a fresh login would have reset it to the current time. The one thing that reads it is the local expiry shortcut behindlite auth print-tokenandlite up, so the cost is that they stop failing fast and let the gateway reject the call instead; the gateway is what enforces expiry either way. Giving the arbitration a counter of its own would separate the two, and it would add a second stored field and a migration for it to close a gap that costs a round trip, so the stamp stays sharedA machine whose keychain answers reads but refuses writes pays the five-second write pre-flight on every command that still finds a secret in
token.json, because the migration that would take the secret out of the file is the thing that cannot finish. The file keeps its copy, so the next command tries again and pays it again. Containers, CI images,sudo -H, and service accounts are where this lands, andlite auth print-tokenis what Claude Code calls as itsapiKeyHelper, so the cost is per request rather than per session. Measured on one such box,print-tokenwent from 1.30s and 1.13s before this PR to 6.15s and 6.27s after. SettingLITELLM_CLI_DISABLE_KEYRING=1skips the keychain and returns those boxes to the file-only path they were already on. Remembering the refusal across processes would fix it properly, and it needs somewhere to write that verdict down, which is its own changelitellm.get_litellm_gateway_api_key()reads the keychain now, andkeyringships only with thecliextra, so an install without that extra cannot see a credentiallite loginput in the keychain and returnsNonewithout saying why. An install that never hadkeyringis unaffected, because its own login fell back to the token file and the getter still reads it there. The gap is the mixed case, one environment signing in with the extra and another reading without it, and installinglitellm[cli]on the reading side closes itA
lite login --pkcecredential comes in two pieces, and this PR only moves one of them. The key goes to the keychain, and every silent renewal puts the new key there too, but the refresh token that mints those keys stays in~/.litellm/token.jsonin the clear. A second process running as the user cannot read a bearer token out of that file any more, and it does not need one: it can post the refresh token to the proxy's/tokenendpoint and be handed a key that works, which is the leg above. Sotoken.jsonis still worth protecting on a--pkcelogin, and the exposure this PR closes is narrower there than it is forlite login. Moving the refresh token into the keychain blob alongside the key is the rest of LIT-5855 and is tracked as a follow-up; it was not folded in here because the PKCE flow arrived from the base branch mid-review and widening the scope during a merge resolution is how merges go wrongA keyring backend that accepts writes and keeps nothing reads back as
SecretMissing, which is the same answer a machine that never signed in gives, solite whoamisays not authenticated rather than naming the backend. The write path catches this by reading the value back and says so plainly; the read path has nothing to compare against, since telling the two apart needs a write.lite loginnames it at the moment it happens, which is the point where the user can act on itFinal Attestation
5f7c0e1e49c70b030aad11d38dcf82b1f0149a0cpasses /live-pr-risk, with its findings recorded under Caveats