From 0acfe593874babfe106c4fd547d677896005b2e5 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 29 Oct 2026 02:38:11 -0700 Subject: [PATCH 01/10] fix(cli): end the local login when logout cannot check the OS keychain With no usable keyring, lite logout kept a secret-free token.json as a note that the keychain went unchecked. Every reader took that note for a live keychain-backed login, so lite whoami, lite auth print-token, and the bare lite shell all reported a login the user had just ended, and a keychain that answered again could pair the note with an earlier login's entry and sign the machine back in. Logout now removes token.json whatever the keychain answers. When the keychain could not be checked and there was a login to end, it records that in a separate owner-only keychain_unchecked file beside it, written after the removal so a full disk still has room for it. A repeat logout still warns while that file is there, and a logout that confirms the keychain is clean removes it. --- litellm/litellm_core_utils/cli_token_utils.py | 102 ++++++++---------- .../test_cli_token_utils.py | 101 ++++++++++++----- .../proxy/client/cli/test_auth_commands.py | 20 ++++ .../proxy/client/cli/test_global_options.py | 1 + 4 files changed, 139 insertions(+), 85 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 37ed4efa0ca..6d44ed933b1 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -13,8 +13,10 @@ This module has no dependencies on proxy code and can be safely imported at the """ import math +import os import time from collections.abc import Mapping +from contextlib import suppress from dataclasses import dataclass from pathlib import Path from types import MappingProxyType @@ -42,6 +44,7 @@ from litellm.litellm_core_utils.private_json import ( ensure_private_dir, overwrite_private_json, stage_private_json, + write_private_bytes, write_private_json, ) from litellm.types.llms.base_model import LiteLLMBaseModel @@ -220,27 +223,30 @@ def _keep_the_secret_in_the_file(record: CliTokenRecord, outcome: SecretWrite) - def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear: """Remove the credential from both stores. Reports whether the keychain is now free of it. - A logout the keychain never answered keeps the token file, with its secret taken out, because - that file is the only remaining record that something may still be in there to remove. It is - what lets a later run tell a machine with a credential it cannot reach apart from one that never - had a login at all, and taking it away would leave the next logout answering the warning this - one just issued with a false all-clear. The secret goes either way, and a file that will give up - neither its copy nor itself is removed rather than kept, with the note written again afterwards - so the warning still outlives this run. + The token file goes whatever the keychain answers, so nothing left on disk reads as a login. A + keychain that could not be checked may still hold an entry an earlier login put there, so a + logout that had something to end leaves a secret-free note beside the file, and the next logout + warns again instead of reading the missing file as a clean keychain. Only a keychain that + confirms the erase takes the note away. A file that will not go has its secret taken out where + it lies instead. """ outcome: Final = vault.erase() record: Final = _read_token_file() settled: Final = _nothing_left_behind(outcome, record) - if not settled and _keep_the_unchecked_keychain_on_record(outcome, record): - return outcome - removal: Final = _remove_token_file() - if removal is not None and record is not None and not _scrub_file_secret(record): - return removal - if removal is None and record is not None and _the_keychain_went_unchecked(outcome): - _write_the_note_the_removal_took_with_it(record) + left_on_disk: Final = _end_the_local_login(record) + _note_what_the_keychain_answered(outcome, settled) + if left_on_disk is not None: + return left_on_disk return SecretErased() if settled else outcome +def _end_the_local_login(record: CliTokenRecord | None) -> CredentialNotCleared | None: + removal: Final = _remove_token_file() + if removal is None or record is None or _scrub_file_secret(record): + return None + return removal + + def _remove_token_file() -> CredentialNotCleared | None: try: Path(get_cli_token_file_path()).unlink(missing_ok=True) @@ -249,54 +255,38 @@ def _remove_token_file() -> CredentialNotCleared | None: return None -def _write_the_note_the_removal_took_with_it(record: CliTokenRecord) -> None: - """Put the secret-free note back after the file carrying it had to go to get the secret off disk. +def _unchecked_keychain_note_path() -> Path: + return Path(get_cli_token_file_path()).with_name("keychain_unchecked") - Reaching here means neither rewrite would take, so the file went instead, and its absence is - what the next logout would read as a keychain already known to be clean. Removing it is also - what frees the room the rewrite was refused for, so the note usually lands on this second try. - When it does not, the warning this logout printed is the only one the user gets. + +def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool) -> None: + """Keep the note that a logout left the keychain unchecked for exactly as long as that is true. + + The secret is off disk by the time this runs, so a note that will not be written or removed is + no reason to fail the logout. The warning printed now is then the only one the user gets. """ - staged: Final = _stage_scrubbed_file(record) - if staged is not None: - _commit_token_file(staged) - - -def _keep_the_unchecked_keychain_on_record(outcome: SecretErase, record: CliTokenRecord | None) -> bool: - """Whether the token file, stripped of its secret, is worth keeping as the note that says so. - - Only a keychain that could not be reached leaves the question open. One that answered for itself - is remembered without any help from the file, and a file it can still pair a live entry with - would leave the machine signed in to the login that was just ended. A copy that will give up - its secret neither to a staged replacement nor to an overwrite is not kept either, because the - secret goes first. - """ - if record is None or not _the_keychain_went_unchecked(outcome): - return False - return _scrub_file_secret(record) - - -def _the_keychain_went_unchecked(outcome: SecretErase) -> bool: - """Whether the keychain neither confirmed the erase nor answered that it still holds the secret""" - match outcome: - case SecretErased() | SecretStranded(): - return False - case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - return True + note: Final = _unchecked_keychain_note_path() + with suppress(OSError): + match outcome: + case SecretErased(): + note.unlink(missing_ok=True) + case SecretStranded(): + pass + case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): + if not settled: + ensure_private_dir(note.parent) + write_private_bytes(str(note), b"") def _nothing_left_behind(outcome: SecretErase, record: CliTokenRecord | None) -> bool: """Whether the keychain can be trusted to hold no credential of ours once the file is gone. - A machine with no token file has no stored login to end, and `clear_cli_token` keeps one behind - whenever the keychain is left unconfirmed, taking the secret out in place when it cannot stage a - replacement and writing the note again when the file holding it had to go, so a missing file is - real evidence rather than the absence of it. Past that, a - keychain that could not be reached is never trusted, whatever the file looks like. Even a file - holding its own secret says only that the login which wrote it had no keychain to write to, and - the login before it may well have had one: the entry that login left outlives both the - uninstalled package and the file that replaced it. `SecretStranded` is - the keychain answering for itself and outranks the file. + A keychain that could not be reached is trusted only on a machine with nothing to end: no token + file, and no note from an earlier logout that left the keychain unchecked. Even a file holding + its own secret says only that the login which wrote it had no keychain to write to, and the + login before it may well have had one: the entry that login left outlives both the uninstalled + package and the file that replaced it. `SecretStranded` is the keychain answering for itself and + outranks the file. """ match outcome: case SecretErased(): @@ -304,7 +294,7 @@ def _nothing_left_behind(outcome: SecretErase, record: CliTokenRecord | None) -> case SecretStranded(): return False case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - return record is None + return record is None and not os.path.exists(_unchecked_keychain_note_path()) def get_litellm_gateway_api_key( diff --git a/tests/unit/litellm_core_utils/test_cli_token_utils.py b/tests/unit/litellm_core_utils/test_cli_token_utils.py index 7e7eee5373f..70dbf03d7f1 100644 --- a/tests/unit/litellm_core_utils/test_cli_token_utils.py +++ b/tests/unit/litellm_core_utils/test_cli_token_utils.py @@ -54,6 +54,11 @@ def _token_file(home): return home / ".litellm" / "token.json" +def _everything_on_disk(home): + """Every file left under ~/.litellm, read together, so a secret cannot hide in one a test did not name.""" + return "".join(path.read_text() for path in (home / ".litellm").rglob("*") if path.is_file()) + + def _write_legacy_file(home, **overrides): payload = { "base_url": SERVER, @@ -794,7 +799,8 @@ class TestClearCliToken: vault = secret_vault_factory(available=False, failure=failure) assert clear_cli_token(vault=vault) == failure - assert json.loads(_token_file(isolated_home).read_text()).get("key") is None + assert not _token_file(isolated_home).exists() + assert "sk-legacy" not in _everything_on_disk(isolated_home) def test_a_second_logout_still_reports_the_keychain_it_could_not_clear( self, isolated_home, secret_vault_factory @@ -817,20 +823,24 @@ class TestClearCliToken: vault = secret_vault_factory(available=False, failure=KeyringNotInstalled()) assert clear_cli_token(vault=vault) == KeyringNotInstalled() - assert json.loads(_token_file(isolated_home).read_text()).get("key") is None + assert load_cli_token(vault=vault) is None - def test_a_logout_that_cannot_clear_the_keychain_keeps_the_record_that_it_has_to( - self, isolated_home, secret_vault_factory + @pytest.mark.parametrize("failure", [KeyringDisabled(), KeyringUnreachable(), KeyringNotInstalled()]) + def test_a_logout_that_cannot_check_the_keychain_still_ends_the_local_login( + self, isolated_home, secret_vault_factory, failure ): - """The file left behind holds no secret. It is what a later run reads to tell a machine with - a credential it cannot reach apart from one that never had a login, which is the difference - between warning the user and inventing a credential for them to worry about.""" - _write_metadata_only_file(isolated_home) - vault = secret_vault_factory(available=False, failure=KeyringUnreachable()) + """Sign in on a machine with no usable keychain, so the credential lands in the token file, + and log out. The warning about the keychain is about an earlier login, not this one. Leaving + a file behind that every reader takes for a keychain-backed login keeps `lite whoami` saying + the machine is signed in, and keeps the SDK looking for a key, after the user logged out.""" + vault = secret_vault_factory(available=False, failure=failure) + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-in-file", user_id="u-1"), vault=vault) - clear_cli_token(vault=vault) + assert clear_cli_token(vault=vault) == failure - assert json.loads(_token_file(isolated_home).read_text()).get("key") is None + assert load_cli_token(vault=vault) is None + assert get_litellm_gateway_api_key(vault=vault) is None + assert not _token_file(isolated_home).exists() def test_a_logout_that_cannot_clear_the_keychain_still_takes_the_file_secret_away( self, isolated_home, secret_vault_factory @@ -842,7 +852,7 @@ class TestClearCliToken: clear_cli_token(vault=vault) - left_on_disk = _token_file(isolated_home).read_text() + left_on_disk = _everything_on_disk(isolated_home) assert "sk-legacy" not in left_on_disk assert "rt-legacy" not in left_on_disk @@ -862,7 +872,44 @@ class TestClearCliToken: assert clear_cli_token(vault=vault) == KeyringUnreachable() assert clear_cli_token(vault=vault) == KeyringUnreachable() assert vault.blob is not None - assert "sk-second" not in _token_file(isolated_home).read_text() + assert load_cli_token(vault=vault) is None + assert "sk-second" not in _everything_on_disk(isolated_home) + + def test_a_keychain_that_answers_again_never_signs_an_ended_login_back_in( + self, isolated_home, secret_vault_factory + ): + """Sign in while the keychain works, sign in again once it is out of reach, and log out. The + first login's entry is still in the keychain. Once the keychain answers again, nothing on + disk may pair with that entry, or the machine is signed back in to a login the user ended. + The next logout is the one that can finally erase the entry, and then it can say so.""" + vault = secret_vault_factory() + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-first"), vault=vault) + vault.available = False + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-second"), vault=vault) + assert clear_cli_token(vault=vault) == KeyringUnreachable() + + vault.available = True + + assert load_cli_token(vault=vault) is None + assert get_litellm_gateway_api_key(vault=vault) is None + assert clear_cli_token(vault=vault) == SecretErased() + assert vault.blob is None + + def test_a_keychain_confirmed_clean_is_not_warned_about_again(self, isolated_home, secret_vault_factory): + """A logout that left the keychain unchecked, then one that erased it, answer the first + warning in full. A keychain that goes out of reach after that holds nothing of ours, so a + logout then has nothing to warn about.""" + vault = secret_vault_factory() + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-first"), vault=vault) + vault.available = False + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-second"), vault=vault) + assert clear_cli_token(vault=vault) == KeyringUnreachable() + vault.available = True + assert clear_cli_token(vault=vault) == SecretErased() + + vault.available = False + + assert clear_cli_token(vault=vault) == SecretErased() @pytest.mark.parametrize("failure", [KeyringNotInstalled(), KeyringDisabled(), KeyringUnreachable()]) def test_logging_out_of_a_machine_that_never_logged_in_invents_nothing_to_warn_about( @@ -874,6 +921,7 @@ class TestClearCliToken: vault = secret_vault_factory(available=False, failure=failure) assert clear_cli_token(vault=vault) == SecretErased() + assert clear_cli_token(vault=vault) == SecretErased() def test_a_file_backed_login_cannot_vouch_for_a_keychain_no_package_can_reach( self, isolated_home, secret_vault_factory @@ -890,7 +938,8 @@ class TestClearCliToken: assert clear_cli_token(vault=vault) == KeyringNotInstalled() assert vault.blob is not None - assert "sk-in-file" not in _token_file(isolated_home).read_text() + assert not _token_file(isolated_home).exists() + assert "sk-in-file" not in _everything_on_disk(isolated_home) @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores file permissions") def test_a_file_that_gives_up_neither_its_secret_nor_itself_is_reported_not_raised( @@ -917,10 +966,9 @@ class TestClearCliToken: def test_a_directory_that_takes_no_new_file_still_gives_up_the_secret_in_the_old_one( self, isolated_home, secret_vault_factory ): - """A read-only `~/.litellm` accepts no replacement token file and no removal of the one it - has, and still lets that one be shortened. The secret goes, the file stays as the note that - the keychain went unchecked, and the logout after it warns again instead of reading the gap - the removal would have left as a clean keychain. + """A read-only `~/.litellm` accepts no removal of the token file and no new file beside it, + and still lets the one it has be shortened. The secret goes, the file stays because nothing + else can, and the logout after it warns again rather than calling the keychain clean. The key is a realistic length so the file genuinely shrinks: a rewrite in place that leaves the tail of the old contents behind hands the next run a file it cannot parse.""" @@ -935,19 +983,13 @@ class TestClearCliToken: assert json.loads(path.read_text()).get("key") is None - @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores file permissions") - def test_a_note_the_logout_had_to_remove_is_written_again_for_the_next_one( + def test_a_full_disk_still_keeps_the_unchecked_keychain_on_record( self, isolated_home, secret_vault_factory, monkeypatch ): - """A full disk refuses the replacement file and a read-only token file refuses the rewrite - in place, so the only way left to get the secret off disk is to remove the file carrying it. - That file was also the note saying the keychain went unchecked, and its absence is what the - next logout would read as a keychain already known to be clean. - - Removing it is what frees the room the replacement was refused for, so the note is written - again on the way out and the logout after this one still warns.""" + """A disk with no room for a new file still lets the token file go, and the room that frees + is what the note about the unchecked keychain needs. Without the note, the next logout reads + the missing file as a keychain already known to be clean.""" path = _write_legacy_file(isolated_home) - path.chmod(0o400) monkeypatch.setattr( "litellm.litellm_core_utils.private_json.tempfile.mkstemp", _MkstempThatNeedsTheOldFileGone(path), @@ -957,7 +999,8 @@ class TestClearCliToken: assert clear_cli_token(vault=vault) == KeyringUnreachable() assert clear_cli_token(vault=vault) == KeyringUnreachable() - assert json.loads(path.read_text()).get("key") is None + assert not path.exists() + assert "sk-legacy" not in _everything_on_disk(isolated_home) @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions") def test_a_metadata_file_that_will_not_go_is_not_worth_alarming_the_user_over( diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index 5d5ebcf5dfd..259edc47d61 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -16,6 +16,7 @@ from litellm.litellm_core_utils.cli_keyring import ( DISABLE_KEYRING_ENV_VAR, KeyringDisabled, KeyringNotInstalled, + KeyringUnreachable, SecretErased, SecretStored, ) @@ -1344,6 +1345,25 @@ class TestKeychainBackedCommands: assert "could not be removed" in result.output assert not (isolated_home / ".litellm" / "token.json").exists() + @pytest.mark.parametrize("failure", [KeyringDisabled(), KeyringUnreachable(), KeyringNotInstalled()]) + def test_logout_without_a_usable_keychain_leaves_every_command_signed_out( + self, isolated_home, secret_vault_factory, failure + ): + """With no usable keychain, `lite login` keeps the credential in the token file. After `lite + logout`, `lite whoami` and `lite auth print-token` have to agree the machine is signed out, + rather than describing a login whose credential is stuck in a keychain that never held it.""" + vault = secret_vault_factory(available=False, failure=failure) + self._login(vault) + obj = {"base_url": "https://test.example.com", "secret_vault": vault} + self.runner.invoke(logout, obj=obj) + + whoami_result = self.runner.invoke(whoami, obj=obj) + print_result = self.runner.invoke(print_token, obj=obj) + + assert whoami_result.output == "Not authenticated. Run 'lite login' to authenticate.\n" + assert print_result.exit_code == 1 + assert print_result.output == "Not authenticated. Run 'lite login'.\n" + def test_print_token_explains_a_locked_keychain_instead_of_printing_nothing( self, isolated_home, secret_vault_factory ): diff --git a/tests/unit/proxy/client/cli/test_global_options.py b/tests/unit/proxy/client/cli/test_global_options.py index 8a1e9a3d141..2acffb704ca 100644 --- a/tests/unit/proxy/client/cli/test_global_options.py +++ b/tests/unit/proxy/client/cli/test_global_options.py @@ -253,6 +253,7 @@ def test_lite_goes_back_to_localhost_after_logout_without_a_keychain(cli_runner, result = cli_runner.invoke(cli, [], obj={"secret_vault": no_keychain}, input="quit\n") assert "Connected to LiteLLM server: http://localhost:4000" in result.output + assert "keychain" not in result.output def test_corrupt_config_file_falls_back_to_default(cli_runner, isolated_home): From 6d9a4e43f34fe977619fe897a22047c2449d4689 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 29 Oct 2026 03:16:56 -0700 Subject: [PATCH 02/10] fix(cli): keep warning after a logout the keychain refused to clear A keychain that will not give up its entry gets the same note as one that could not be checked, so a later logout made while the keychain is out of reach does not report a clean logout while the credential is still there. --- litellm/litellm_core_utils/cli_token_utils.py | 26 +++++++++---------- .../test_cli_token_utils.py | 15 +++++++++++ 2 files changed, 27 insertions(+), 14 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 6d44ed933b1..881305706e1 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -224,11 +224,11 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear: """Remove the credential from both stores. Reports whether the keychain is now free of it. The token file goes whatever the keychain answers, so nothing left on disk reads as a login. A - keychain that could not be checked may still hold an entry an earlier login put there, so a - logout that had something to end leaves a secret-free note beside the file, and the next logout - warns again instead of reading the missing file as a clean keychain. Only a keychain that - confirms the erase takes the note away. A file that will not go has its secret taken out where - it lies instead. + keychain that could not be checked, or that would not give up its entry, may still hold a + credential of ours, so a logout that had something to end leaves a secret-free note beside the + file, and the next logout warns again instead of reading the missing file as a clean keychain. + Only a keychain that confirms the erase takes the note away. A file that will not go has its + secret taken out where it lies instead. """ outcome: Final = vault.erase() record: Final = _read_token_file() @@ -255,24 +255,22 @@ def _remove_token_file() -> CredentialNotCleared | None: return None -def _unchecked_keychain_note_path() -> Path: - return Path(get_cli_token_file_path()).with_name("keychain_unchecked") +def _keychain_not_cleared_note_path() -> Path: + return Path(get_cli_token_file_path()).with_name("keychain_not_cleared") def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool) -> None: - """Keep the note that a logout left the keychain unchecked for exactly as long as that is true. + """Keep the note that a logout left the keychain uncleared for exactly as long as that is true. The secret is off disk by the time this runs, so a note that will not be written or removed is no reason to fail the logout. The warning printed now is then the only one the user gets. """ - note: Final = _unchecked_keychain_note_path() + note: Final = _keychain_not_cleared_note_path() with suppress(OSError): match outcome: case SecretErased(): note.unlink(missing_ok=True) - case SecretStranded(): - pass - case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): + case SecretStranded() | KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): if not settled: ensure_private_dir(note.parent) write_private_bytes(str(note), b"") @@ -282,7 +280,7 @@ def _nothing_left_behind(outcome: SecretErase, record: CliTokenRecord | None) -> """Whether the keychain can be trusted to hold no credential of ours once the file is gone. A keychain that could not be reached is trusted only on a machine with nothing to end: no token - file, and no note from an earlier logout that left the keychain unchecked. Even a file holding + file, and no note from an earlier logout that left the keychain uncleared. Even a file holding its own secret says only that the login which wrote it had no keychain to write to, and the login before it may well have had one: the entry that login left outlives both the uninstalled package and the file that replaced it. `SecretStranded` is the keychain answering for itself and @@ -294,7 +292,7 @@ def _nothing_left_behind(outcome: SecretErase, record: CliTokenRecord | None) -> case SecretStranded(): return False case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - return record is None and not os.path.exists(_unchecked_keychain_note_path()) + return record is None and not os.path.exists(_keychain_not_cleared_note_path()) def get_litellm_gateway_api_key( diff --git a/tests/unit/litellm_core_utils/test_cli_token_utils.py b/tests/unit/litellm_core_utils/test_cli_token_utils.py index 70dbf03d7f1..7725f1eb28e 100644 --- a/tests/unit/litellm_core_utils/test_cli_token_utils.py +++ b/tests/unit/litellm_core_utils/test_cli_token_utils.py @@ -782,6 +782,21 @@ class TestClearCliToken: assert clear_cli_token(vault=vault) == SecretStranded() assert load_cli_token(vault=vault) is None + def test_a_keychain_that_kept_the_secret_is_still_warned_about_once_it_is_out_of_reach( + self, isolated_home, secret_vault_factory + ): + """The keychain refused to give up the entry, so the first logout said so. A second logout + made after the keychain went out of reach cannot see that entry either, and reading the token + file the first logout removed as proof of a clean keychain retracts that warning while the + credential is still there.""" + _write_metadata_only_file(isolated_home) + vault = secret_vault_factory(blob=_blob(), erasable=False) + assert clear_cli_token(vault=vault) == SecretStranded() + + vault.available = False + + assert clear_cli_token(vault=vault) == KeyringUnreachable() + @pytest.mark.parametrize( "failure", [KeyringDisabled(), KeyringUnreachable(), KeyringNotInstalled()] ) From ba33eb7db229ae405db427a011a0fdb57497c134 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 13:58:22 -0800 Subject: [PATCH 03/10] fix(cli): revoke the login a logout left in an unreachable keychain once it answers A logout made while the keychain was locked ended the local login and dropped the metadata that pairs with the refresh token still in the keychain, so the next logout had nothing to revoke. The not-cleared note now keeps that record without its secret, and a later logout pairs it with the keychain entry and revokes it before erasing. --- litellm/litellm_core_utils/cli_token_utils.py | 54 ++++++++++++++++--- litellm/proxy/client/cli/commands/auth.py | 12 ++++- .../test_cli_token_utils.py | 2 + .../proxy/client/cli/test_auth_commands.py | 22 ++++++++ 4 files changed, 81 insertions(+), 9 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 881305706e1..1e2d1b55830 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -225,16 +225,17 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear: The token file goes whatever the keychain answers, so nothing left on disk reads as a login. A keychain that could not be checked, or that would not give up its entry, may still hold a - credential of ours, so a logout that had something to end leaves a secret-free note beside the - file, and the next logout warns again instead of reading the missing file as a clean keychain. - Only a keychain that confirms the erase takes the note away. A file that will not go has its - secret taken out where it lies instead. + credential of ours, so a logout that had something to end leaves a note beside the file holding + that login's record without its secret. The next logout warns again instead of reading the + missing file as a clean keychain, and once the keychain answers it pairs the note with the entry + to revoke that login at the proxy. Only a keychain that confirms the erase takes the note away. A + file that will not go has its secret taken out where it lies instead. """ outcome: Final = vault.erase() record: Final = _read_token_file() settled: Final = _nothing_left_behind(outcome, record) left_on_disk: Final = _end_the_local_login(record) - _note_what_the_keychain_answered(outcome, settled) + _note_what_the_keychain_answered(outcome, settled, record) if left_on_disk is not None: return left_on_disk return SecretErased() if settled else outcome @@ -259,7 +260,34 @@ def _keychain_not_cleared_note_path() -> Path: return Path(get_cli_token_file_path()).with_name("keychain_not_cleared") -def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool) -> None: +def load_cli_token_left_in_keychain(*, vault: SecretVault = SYSTEM_KEYRING) -> CliTokenRecord | None: + """The login an earlier logout could not erase from the keychain, once the keychain answers again. + + The token file went with that logout, so its note is the only record of the login, and the + refresh token that revokes it is only in the keychain. A keychain entry from any other sign-in + is never paired with the note, since the endpoints it names belong to that one login. + """ + note: Final = _read_keychain_not_cleared_note() + if note is None: + return None + secret: Final = _stored_secret(note.base_url, vault) + if secret is None or secret.timestamp != note.timestamp: + return None + return note.model_copy( + update=MappingProxyType( + {"key": secret.key, "jwt_token": secret.jwt_token, "refresh_token": secret.refresh_token} + ) + ) + + +def _read_keychain_not_cleared_note() -> CliTokenRecord | None: + try: + return CliTokenRecord.model_validate_json(_keychain_not_cleared_note_path().read_bytes()) + except (OSError, ValidationError): + return None + + +def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool, record: CliTokenRecord | None) -> None: """Keep the note that a logout left the keychain uncleared for exactly as long as that is true. The secret is off disk by the time this runs, so a note that will not be written or removed is @@ -272,8 +300,18 @@ def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool) -> Non note.unlink(missing_ok=True) case SecretStranded() | KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): if not settled: - ensure_private_dir(note.parent) - write_private_bytes(str(note), b"") + _write_keychain_not_cleared_note(note, record) + + +def _write_keychain_not_cleared_note(note: Path, record: CliTokenRecord | None) -> None: + """Record the login whose entry may still be in the keychain, or keep the note that already does""" + if record is None and note.exists(): + return + ensure_private_dir(note.parent) + if record is None: + write_private_bytes(str(note), b"") + return + write_private_json(str(note), _without_secret(record).model_dump(exclude_none=True)) def _nothing_left_behind(outcome: SecretErase, record: CliTokenRecord | None) -> bool: diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index e38f54adc4f..f0776ceafe9 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/litellm/proxy/client/cli/commands/auth.py @@ -37,6 +37,7 @@ from litellm.litellm_core_utils.cli_token_utils import ( get_cli_token_file_path, is_cli_token_fresh, load_cli_token, + load_cli_token_left_in_keychain, save_cli_token, ) @@ -234,6 +235,15 @@ def load_token(*, vault: SecretVault = SYSTEM_KEYRING) -> Mapping[str, object] | return None if record is None else record.model_dump(exclude_none=True) +def _credential_to_revoke(vault: SecretVault) -> Mapping[str, object] | None: + """The login this logout ends, or the one an earlier logout left in a keychain it could not reach""" + current: Final = load_token(vault=vault) + if current is not None: + return current + left: Final = load_cli_token_left_in_keychain(vault=vault) + return None if left is None else left.model_dump(exclude_none=True) + + def save_token(record: CliTokenData, *, vault: SecretVault = SYSTEM_KEYRING) -> SecretSave: """Store a credential the PKCE layer produced, secret in the vault and the rest on disk""" return save_cli_token(CliTokenRecord(**record), vault=vault) @@ -992,7 +1002,7 @@ def login(ctx: click.Context, config_claude: bool, pkce: bool | None) -> None: def logout(ctx: click.Context): """Logout and clear stored authentication""" vault: Final = context_secret_vault(ctx) - token_data: Final = load_token(vault=vault) + token_data: Final = _credential_to_revoke(vault) revocation: Final = revoke_stored_credential(token_data, requests.Session()) if token_data is not None else None match revocation: case RevocationUnavailable(reason=reason): diff --git a/tests/unit/litellm_core_utils/test_cli_token_utils.py b/tests/unit/litellm_core_utils/test_cli_token_utils.py index 7725f1eb28e..55d6ff454f8 100644 --- a/tests/unit/litellm_core_utils/test_cli_token_utils.py +++ b/tests/unit/litellm_core_utils/test_cli_token_utils.py @@ -36,6 +36,7 @@ from litellm.litellm_core_utils.cli_token_utils import ( get_litellm_gateway_api_key, is_cli_token_fresh, load_cli_token, + load_cli_token_left_in_keychain, save_cli_token, ) @@ -907,6 +908,7 @@ class TestClearCliToken: assert load_cli_token(vault=vault) is None assert get_litellm_gateway_api_key(vault=vault) is None + assert load_cli_token_left_in_keychain(vault=vault) is None assert clear_cli_token(vault=vault) == SecretErased() assert vault.blob is None diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index 259edc47d61..d8dc0d9f82f 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -1953,6 +1953,28 @@ class TestPkceLogoutCommand: assert "Logged out successfully" not in result.output clear.assert_not_called() + def test_a_logout_that_reaches_the_keychain_late_still_revokes_the_login( + self, isolated_home, secret_vault_factory + ): + """A logout made while the keychain is locked cannot read the refresh token, so the proxy + hears nothing. The next logout, once the keychain answers, has to revoke it before erasing it.""" + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession): + self.runner.invoke(logout, obj={"secret_vault": locked}) + result = self.runner.invoke(logout, obj={"secret_vault": unlocked}) + + assert result.output == "Logged out successfully. Authentication token cleared.\n" + assert unlocked.blob is None + assert _FakeSession.instances[-1].posts == [ + ( + f"{PKCE_BASE_URL}/revoke", + {"token": "llm_srefresh_old", "token_type_hint": "refresh_token", "client_id": "llm_dcrc_abc"}, + ) + ] + def test_logout_of_a_classic_token_makes_no_request(self): with ( patch("litellm.proxy.client.cli.commands.auth.load_token", return_value={"key": "sk-classic"}), From 1734c238b1863998c7c74486576d20a6cc576867 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 14:41:10 -0800 Subject: [PATCH 04/10] fix(cli): revoke a locked-keychain login on the next login, and only once after a refused erase A login made after a logout that could not reach the keychain now revokes the login it replaces, read from the keychain_not_cleared note, as the merge base did through the kept token.json. A keychain that refused the erase had already handed that logout the refresh token, so its note no longer pairs with the entry and the next logout does not revoke the same token again. --- litellm/litellm_core_utils/cli_token_utils.py | 29 +++++++----- litellm/proxy/client/README.md | 2 +- litellm/proxy/client/cli/commands/auth.py | 4 +- .../proxy/client/cli/test_auth_commands.py | 46 +++++++++++++++++++ 4 files changed, 66 insertions(+), 15 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 1e2d1b55830..5cd5033a7a7 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -225,17 +225,16 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear: The token file goes whatever the keychain answers, so nothing left on disk reads as a login. A keychain that could not be checked, or that would not give up its entry, may still hold a - credential of ours, so a logout that had something to end leaves a note beside the file holding - that login's record without its secret. The next logout warns again instead of reading the - missing file as a clean keychain, and once the keychain answers it pairs the note with the entry - to revoke that login at the proxy. Only a keychain that confirms the erase takes the note away. A - file that will not go has its secret taken out where it lies instead. + credential of ours, so a logout that had something to end leaves a note beside the file, and the + next logout warns again instead of reading the missing file as a clean keychain. Only a keychain + that confirms the erase takes the note away. A file that will not go has its secret taken out + where it lies instead. """ outcome: Final = vault.erase() record: Final = _read_token_file() settled: Final = _nothing_left_behind(outcome, record) left_on_disk: Final = _end_the_local_login(record) - _note_what_the_keychain_answered(outcome, settled, record) + _note_what_the_keychain_answered(outcome, record) if left_on_disk is not None: return left_on_disk return SecretErased() if settled else outcome @@ -287,9 +286,15 @@ def _read_keychain_not_cleared_note() -> CliTokenRecord | None: return None -def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool, record: CliTokenRecord | None) -> None: +def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecord | None) -> None: """Keep the note that a logout left the keychain uncleared for exactly as long as that is true. + A keychain that could not be read kept this logout from revoking the login, so the note holds + that login's record without its secret, and the next logout or login that finds the keychain + answering pairs it with the entry to revoke it. One that would not give up its entry had already + handed this logout the refresh token, so its note holds nothing to revoke again. A logout with no + file to end keeps whatever note an earlier one left. + The secret is off disk by the time this runs, so a note that will not be written or removed is no reason to fail the logout. The warning printed now is then the only one the user gets. """ @@ -298,15 +303,15 @@ def _note_what_the_keychain_answered(outcome: SecretErase, settled: bool, record match outcome: case SecretErased(): note.unlink(missing_ok=True) - case SecretStranded() | KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - if not settled: + case SecretStranded(): + _write_keychain_not_cleared_note(note, None) + case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): + if record is not None: _write_keychain_not_cleared_note(note, record) def _write_keychain_not_cleared_note(note: Path, record: CliTokenRecord | None) -> None: - """Record the login whose entry may still be in the keychain, or keep the note that already does""" - if record is None and note.exists(): - return + """Record the login a later command can still revoke, or only that the keychain was left uncleared""" ensure_private_dir(note.parent) if record is None: write_private_bytes(str(note), b"") diff --git a/litellm/proxy/client/README.md b/litellm/proxy/client/README.md index ceec636567f..bc723397818 100644 --- a/litellm/proxy/client/README.md +++ b/litellm/proxy/client/README.md @@ -380,7 +380,7 @@ The key itself, together with the refresh token that renews a `--pkce` credentia Keychain storage needs the `keyring` package, which ships with `pip install 'litellm[cli]'`. Headless boxes and CI runners usually have no keychain either. In all of those cases the key and the refresh token stay in the same `0600` file alongside the metadata, exactly as they did before, and `lite login` names which one applies: the package is missing, the machine has no keychain, or you set `LITELLM_CLI_DISABLE_KEYRING=1` to force the file even where a keychain exists. A `token.json` written by an older `lite` keeps working and is moved into the keychain, and scrubbed from the file, the first time a keychain-capable `lite` reads it. That includes a refresh token left behind by the release that moved only the key. -`lite logout` clears both stores. If the keychain is locked at that moment it says so, and re-running it once the keychain is unlocked finishes the job. +`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. The stored credential is a short-lived, per-session agent token, not a managed virtual key. It is scoped to the user and team you logged in as and inherits their models and budgets; spend is tracked against the shared team and user budgets rather than a separate per-session cap, so multiple logins or several concurrent agents all draw down the same allowance. It is short-lived by design (default 24h, configurable via `LITELLM_CLI_JWT_EXPIRATION_HOURS`); re-run `lite login` to refresh it and pick up your latest team and user settings. `lite auth print-token` (usable as Claude Code's `apiKeyHelper`) prints it, and once it is past 24 hours (or the client's `LITELLM_CLI_JWT_EXPIRATION_HOURS`) asks the proxy first and fails if the proxy rejects it. There is no silent renewal. It is accepted on a default deployment without `EXPERIMENTAL_UI_LOGIN`, does not appear in the Keys UI, and cannot be rotated or revoked mid-session. A credential from `lite login --pkce` is the exception: it carries a refresh token, so the CLI renews the key shortly before it expires and `lite logout` revokes the refresh token on the proxy (see [Browser sign-in with PKCE](https://docs.litellm.ai/docs/proxy/cli_sso#browser-sign-in-with-pkce)). Only the holder can end a `--pkce` session early, with `lite logout`; an admin has no button for it, but every renewal re-reads the user on the proxy, so deactivating the user or removing them from the team makes the next renewal fail and the key runs out within `LITELLM_CLI_JWT_EXPIRATION_HOURS`. On a proxy with more than one worker or replica, configure Redis (`litellm_settings.cache` with Redis `cache_params`, or `general_settings.coordination_redis`) so a refresh token stays single-use and `lite logout` holds on every worker; without Redis each worker keeps its own record. For a long-lived, rotatable, Keys-UI-visible credential, create a dedicated virtual key in the dashboard and pass it via `--api-key` or `LITELLM_PROXY_API_KEY`. diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index f0776ceafe9..e171e18ab86 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/litellm/proxy/client/cli/commands/auth.py @@ -236,7 +236,7 @@ def load_token(*, vault: SecretVault = SYSTEM_KEYRING) -> Mapping[str, object] | def _credential_to_revoke(vault: SecretVault) -> Mapping[str, object] | None: - """The login this logout ends, or the one an earlier logout left in a keychain it could not reach""" + """The stored login, or the one an earlier logout left in a keychain it could not reach""" current: Final = load_token(vault=vault) if current is not None: return current @@ -863,7 +863,7 @@ def _finish_login(base_url: str, api_key: str, config_claude: bool, stored: Secr def _replace_stored_token(record: CliTokenData, http: Http, vault: SecretVault) -> SecretSave: - previous: Final = load_token(vault=vault) + previous: Final = _credential_to_revoke(vault) stored: Final = save_token(record, vault=vault) if previous is None or isinstance(stored, CredentialNotSaved): return stored diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index d8dc0d9f82f..bb91bc7e9af 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -1975,6 +1975,52 @@ class TestPkceLogoutCommand: ) ] + def test_a_login_after_a_logout_that_missed_the_keychain_revokes_the_login_it_replaces( + self, isolated_home, secret_vault_factory + ): + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=_pkce_credential()), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + self.runner.invoke(logout, obj={"secret_vault": locked}) + result = self.runner.invoke( + login, ["--pkce"], obj={"base_url": PKCE_BASE_URL, "secret_vault": unlocked} + ) + + assert result.exit_code == 0, result.output + assert "Could not revoke" not in result.output + assert [post for session in _FakeSession.instances for post in session.posts] == [ + ( + f"{PKCE_BASE_URL}/revoke", + {"token": "llm_srefresh_old", "token_type_hint": "refresh_token", "client_id": "llm_dcrc_abc"}, + ) + ] + + def test_a_logout_after_the_keychain_refused_the_erase_does_not_revoke_the_login_again( + self, isolated_home, secret_vault_factory + ): + vault = secret_vault_factory(erasable=False) + save_cli_token(CliTokenRecord(**_pkce_record()), vault=vault) + + with patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession): + first = self.runner.invoke(logout, obj={"secret_vault": vault}) + second = self.runner.invoke(logout, obj={"secret_vault": vault}) + + assert first.exit_code == 0, first.output + assert second.exit_code == 0, second.output + assert "still in the OS keychain" in second.output + assert [post for session in _FakeSession.instances for post in session.posts] == [ + ( + f"{PKCE_BASE_URL}/revoke", + {"token": "llm_srefresh_old", "token_type_hint": "refresh_token", "client_id": "llm_dcrc_abc"}, + ) + ] + def test_logout_of_a_classic_token_makes_no_request(self): with ( patch("litellm.proxy.client.cli.commands.auth.load_token", return_value={"key": "sk-classic"}), From 88adf9e581ee8ecca22d93bd242733007be3385f Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 16:39:22 -0800 Subject: [PATCH 05/10] fix(cli): keep the first login revocable after a second locked-keychain logout A logout whose login kept its secret in token.json never reached the keychain, so it now leaves the note an earlier logout wrote in place, or writes an empty one, instead of overwriting it. The first login's refresh token stays revocable once the keychain answers, and a keychain-less machine no longer keeps user and team ids in the note. Also documents the limits that remain in the CLI README and drops the docstrings from the tests this PR added. --- litellm/litellm_core_utils/cli_token_utils.py | 18 +++++++-- litellm/proxy/client/README.md | 2 +- .../test_cli_token_utils.py | 39 ++++++++++--------- .../proxy/client/cli/test_auth_commands.py | 29 +++++++++++--- 4 files changed, 59 insertions(+), 29 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 5cd5033a7a7..cbf8b7fc80b 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -292,8 +292,9 @@ def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecor A keychain that could not be read kept this logout from revoking the login, so the note holds that login's record without its secret, and the next logout or login that finds the keychain answering pairs it with the entry to revoke it. One that would not give up its entry had already - handed this logout the refresh token, so its note holds nothing to revoke again. A logout with no - file to end keeps whatever note an earlier one left. + handed this logout the refresh token, so its note holds nothing to revoke again. A login that + kept its secret in the file never reached the keychain, so the note an earlier logout left still + names the login the keychain holds and stays, and a logout with no file to end keeps it too. The secret is off disk by the time this runs, so a note that will not be written or removed is no reason to fail the logout. The warning printed now is then the only one the user gets. @@ -306,8 +307,17 @@ def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecor case SecretStranded(): _write_keychain_not_cleared_note(note, None) case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - if record is not None: - _write_keychain_not_cleared_note(note, record) + _note_a_keychain_left_unchecked(note, record) + + +def _note_a_keychain_left_unchecked(note: Path, record: CliTokenRecord | None) -> None: + if record is None: + return + if not _holds_a_secret(record): + _write_keychain_not_cleared_note(note, record) + return + if not note.exists(): + _write_keychain_not_cleared_note(note, None) def _write_keychain_not_cleared_note(note: Path, record: CliTokenRecord | None) -> None: diff --git a/litellm/proxy/client/README.md b/litellm/proxy/client/README.md index bc723397818..bce3721775d 100644 --- a/litellm/proxy/client/README.md +++ b/litellm/proxy/client/README.md @@ -380,7 +380,7 @@ The key itself, together with the refresh token that renews a `--pkce` credentia Keychain storage needs the `keyring` package, which ships with `pip install 'litellm[cli]'`. Headless boxes and CI runners usually have no keychain either. In all of those cases the key and the refresh token stay in the same `0600` file alongside the metadata, exactly as they did before, and `lite login` names which one applies: the package is missing, the machine has no keychain, or you set `LITELLM_CLI_DISABLE_KEYRING=1` to force the file even where a keychain exists. A `token.json` written by an older `lite` keeps working and is moved into the keychain, and scrubbed from the file, the first time a keychain-capable `lite` reads it. That includes a refresh token left behind by the release that moved only the key. -`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. +`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. A login made while the keychain is still locked keeps its secret in `token.json`, and a command run after the keychain unlocks moves it into the keychain over the entry the note names, so unlock the keychain and run `lite logout` before signing in again. On a machine with no keychain at all, every later `lite logout` repeats the warning until you delete `~/.litellm/keychain_not_cleared`. A machine that an older `lite` logged out while its keychain was locked still has its `token.json`, and still reads as signed in, until you run `lite logout` once more. The stored credential is a short-lived, per-session agent token, not a managed virtual key. It is scoped to the user and team you logged in as and inherits their models and budgets; spend is tracked against the shared team and user budgets rather than a separate per-session cap, so multiple logins or several concurrent agents all draw down the same allowance. It is short-lived by design (default 24h, configurable via `LITELLM_CLI_JWT_EXPIRATION_HOURS`); re-run `lite login` to refresh it and pick up your latest team and user settings. `lite auth print-token` (usable as Claude Code's `apiKeyHelper`) prints it, and once it is past 24 hours (or the client's `LITELLM_CLI_JWT_EXPIRATION_HOURS`) asks the proxy first and fails if the proxy rejects it. There is no silent renewal. It is accepted on a default deployment without `EXPERIMENTAL_UI_LOGIN`, does not appear in the Keys UI, and cannot be rotated or revoked mid-session. A credential from `lite login --pkce` is the exception: it carries a refresh token, so the CLI renews the key shortly before it expires and `lite logout` revokes the refresh token on the proxy (see [Browser sign-in with PKCE](https://docs.litellm.ai/docs/proxy/cli_sso#browser-sign-in-with-pkce)). Only the holder can end a `--pkce` session early, with `lite logout`; an admin has no button for it, but every renewal re-reads the user on the proxy, so deactivating the user or removing them from the team makes the next renewal fail and the key runs out within `LITELLM_CLI_JWT_EXPIRATION_HOURS`. On a proxy with more than one worker or replica, configure Redis (`litellm_settings.cache` with Redis `cache_params`, or `general_settings.coordination_redis`) so a refresh token stays single-use and `lite logout` holds on every worker; without Redis each worker keeps its own record. For a long-lived, rotatable, Keys-UI-visible credential, create a dedicated virtual key in the dashboard and pass it via `--api-key` or `LITELLM_PROXY_API_KEY`. diff --git a/tests/unit/litellm_core_utils/test_cli_token_utils.py b/tests/unit/litellm_core_utils/test_cli_token_utils.py index 55d6ff454f8..602a831f544 100644 --- a/tests/unit/litellm_core_utils/test_cli_token_utils.py +++ b/tests/unit/litellm_core_utils/test_cli_token_utils.py @@ -56,7 +56,6 @@ def _token_file(home): def _everything_on_disk(home): - """Every file left under ~/.litellm, read together, so a secret cannot hide in one a test did not name.""" return "".join(path.read_text() for path in (home / ".litellm").rglob("*") if path.is_file()) @@ -786,10 +785,6 @@ class TestClearCliToken: def test_a_keychain_that_kept_the_secret_is_still_warned_about_once_it_is_out_of_reach( self, isolated_home, secret_vault_factory ): - """The keychain refused to give up the entry, so the first logout said so. A second logout - made after the keychain went out of reach cannot see that entry either, and reading the token - file the first logout removed as proof of a clean keychain retracts that warning while the - credential is still there.""" _write_metadata_only_file(isolated_home) vault = secret_vault_factory(blob=_blob(), erasable=False) assert clear_cli_token(vault=vault) == SecretStranded() @@ -845,10 +840,6 @@ class TestClearCliToken: def test_a_logout_that_cannot_check_the_keychain_still_ends_the_local_login( self, isolated_home, secret_vault_factory, failure ): - """Sign in on a machine with no usable keychain, so the credential lands in the token file, - and log out. The warning about the keychain is about an earlier login, not this one. Leaving - a file behind that every reader takes for a keychain-backed login keeps `lite whoami` saying - the machine is signed in, and keeps the SDK looking for a key, after the user logged out.""" vault = secret_vault_factory(available=False, failure=failure) save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-in-file", user_id="u-1"), vault=vault) @@ -894,10 +885,6 @@ class TestClearCliToken: def test_a_keychain_that_answers_again_never_signs_an_ended_login_back_in( self, isolated_home, secret_vault_factory ): - """Sign in while the keychain works, sign in again once it is out of reach, and log out. The - first login's entry is still in the keychain. Once the keychain answers again, nothing on - disk may pair with that entry, or the machine is signed back in to a login the user ended. - The next logout is the one that can finally erase the entry, and then it can say so.""" vault = secret_vault_factory() save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-first"), vault=vault) vault.available = False @@ -913,9 +900,6 @@ class TestClearCliToken: assert vault.blob is None def test_a_keychain_confirmed_clean_is_not_warned_about_again(self, isolated_home, secret_vault_factory): - """A logout that left the keychain unchecked, then one that erased it, answer the first - warning in full. A keychain that goes out of reach after that holds nothing of ours, so a - logout then has nothing to warn about.""" vault = secret_vault_factory() save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-first"), vault=vault) vault.available = False @@ -928,6 +912,26 @@ class TestClearCliToken: assert clear_cli_token(vault=vault) == SecretErased() + def test_a_second_logout_that_misses_the_keychain_keeps_the_first_login_revocable( + self, isolated_home, secret_vault_factory + ): + vault = secret_vault_factory() + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-first", refresh_token="rt-first", timestamp=1000.0), vault=vault + ) + vault.available = False + assert clear_cli_token(vault=vault) == KeyringUnreachable() + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-second", refresh_token="rt-second", timestamp=2000.0), vault=vault + ) + assert clear_cli_token(vault=vault) == KeyringUnreachable() + + vault.available = True + left = load_cli_token_left_in_keychain(vault=vault) + + assert left is not None + assert (left.key, left.refresh_token) == ("sk-first", "rt-first") + @pytest.mark.parametrize("failure", [KeyringNotInstalled(), KeyringDisabled(), KeyringUnreachable()]) def test_logging_out_of_a_machine_that_never_logged_in_invents_nothing_to_warn_about( self, isolated_home, secret_vault_factory, failure @@ -1003,9 +1007,6 @@ class TestClearCliToken: def test_a_full_disk_still_keeps_the_unchecked_keychain_on_record( self, isolated_home, secret_vault_factory, monkeypatch ): - """A disk with no room for a new file still lets the token file go, and the room that frees - is what the note about the unchecked keychain needs. Without the note, the next logout reads - the missing file as a keychain already known to be clean.""" path = _write_legacy_file(isolated_home) monkeypatch.setattr( "litellm.litellm_core_utils.private_json.tempfile.mkstemp", diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index bb91bc7e9af..42fe11790f4 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -1349,9 +1349,6 @@ class TestKeychainBackedCommands: def test_logout_without_a_usable_keychain_leaves_every_command_signed_out( self, isolated_home, secret_vault_factory, failure ): - """With no usable keychain, `lite login` keeps the credential in the token file. After `lite - logout`, `lite whoami` and `lite auth print-token` have to agree the machine is signed out, - rather than describing a login whose credential is stuck in a keychain that never held it.""" vault = secret_vault_factory(available=False, failure=failure) self._login(vault) obj = {"base_url": "https://test.example.com", "secret_vault": vault} @@ -1956,8 +1953,6 @@ class TestPkceLogoutCommand: def test_a_logout_that_reaches_the_keychain_late_still_revokes_the_login( self, isolated_home, secret_vault_factory ): - """A logout made while the keychain is locked cannot read the refresh token, so the proxy - hears nothing. The next logout, once the keychain answers, has to revoke it before erasing it.""" unlocked = secret_vault_factory() save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) locked = secret_vault_factory(blob=unlocked.blob, available=False) @@ -2001,6 +1996,30 @@ class TestPkceLogoutCommand: ) ] + def test_a_second_logout_that_misses_the_keychain_still_lets_the_first_login_be_revoked( + self, isolated_home, secret_vault_factory + ): + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=_pkce_credential()), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + self.runner.invoke(logout, obj={"secret_vault": locked}) + self.runner.invoke(login, ["--pkce"], obj={"base_url": PKCE_BASE_URL, "secret_vault": locked}) + self.runner.invoke(logout, obj={"secret_vault": locked}) + result = self.runner.invoke(logout, obj={"secret_vault": unlocked}) + + assert result.output == "Logged out successfully. Authentication token cleared.\n" + assert unlocked.blob is None + assert [post["token"] for session in _FakeSession.instances for _, post in session.posts] == [ + "llm_srefresh_fresh", + "llm_srefresh_old", + ] + def test_a_logout_after_the_keychain_refused_the_erase_does_not_revoke_the_login_again( self, isolated_home, secret_vault_factory ): From 6658373ccfd5efe69c9b921e045f5e0a08eae1bd Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 17:37:37 -0800 Subject: [PATCH 06/10] fix(cli): revoke the keychain's login after a login made while the keychain was locked --- litellm/litellm_core_utils/cli_token_utils.py | 18 +++++- litellm/proxy/client/README.md | 2 +- litellm/proxy/client/cli/commands/auth.py | 57 +++++++++++++------ .../test_cli_token_utils.py | 35 +++++++++++- .../proxy/client/cli/test_auth_commands.py | 29 ++++++++++ 5 files changed, 122 insertions(+), 19 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index cbf8b7fc80b..1f1e032eda3 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -162,6 +162,7 @@ def save_cli_token(record: CliTokenRecord, *, vault: SecretVault = SYSTEM_KEYRIN keychain has already taken the new secret, and it reports itself as such rather than claiming the previous login survived. """ + replaced: Final = _read_token_file() stamped: Final = _stamped_past_every_stored_login(record, vault) staged: Final = _stage_token_file(_without_secret(stamped)) if isinstance(staged, CredentialNotSaved): @@ -170,7 +171,22 @@ def save_cli_token(record: CliTokenRecord, *, vault: SecretVault = SYSTEM_KEYRIN if isinstance(outcome, SecretStored): return outcome if _commit_token_file(staged) else CredentialNotRecorded() discard_staged_json(staged) - return _keep_the_secret_in_the_file(stamped, outcome) + kept: Final = _keep_the_secret_in_the_file(stamped, outcome) + if not isinstance(kept, CredentialNotSaved): + _note_the_login_left_in_the_keychain(replaced) + return kept + + +def _note_the_login_left_in_the_keychain(replaced: CliTokenRecord | None) -> None: + """Keep the login a save could not take out of the keychain revocable once the keychain answers. + + Its token file record is about to be overwritten by one that holds its own secret, so the note + is the only record left of the login whose refresh token the keychain still holds. + """ + if replaced is None or _holds_a_secret(replaced): + return + with suppress(OSError): + _write_keychain_not_cleared_note(_keychain_not_cleared_note_path(), replaced) def _stamped_past_every_stored_login(record: CliTokenRecord, vault: SecretVault) -> CliTokenRecord: diff --git a/litellm/proxy/client/README.md b/litellm/proxy/client/README.md index bce3721775d..783086e577d 100644 --- a/litellm/proxy/client/README.md +++ b/litellm/proxy/client/README.md @@ -380,7 +380,7 @@ The key itself, together with the refresh token that renews a `--pkce` credentia Keychain storage needs the `keyring` package, which ships with `pip install 'litellm[cli]'`. Headless boxes and CI runners usually have no keychain either. In all of those cases the key and the refresh token stay in the same `0600` file alongside the metadata, exactly as they did before, and `lite login` names which one applies: the package is missing, the machine has no keychain, or you set `LITELLM_CLI_DISABLE_KEYRING=1` to force the file even where a keychain exists. A `token.json` written by an older `lite` keeps working and is moved into the keychain, and scrubbed from the file, the first time a keychain-capable `lite` reads it. That includes a refresh token left behind by the release that moved only the key. -`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. A login made while the keychain is still locked keeps its secret in `token.json`, and a command run after the keychain unlocks moves it into the keychain over the entry the note names, so unlock the keychain and run `lite logout` before signing in again. On a machine with no keychain at all, every later `lite logout` repeats the warning until you delete `~/.litellm/keychain_not_cleared`. A machine that an older `lite` logged out while its keychain was locked still has its `token.json`, and still reads as signed in, until you run `lite logout` once more. +`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. A login made while the keychain is still locked keeps its secret in `token.json` and the note names the keychain's login, so the first `lite logout` or `lite login` after the unlock revokes both. Any other command run first moves the newer secret into the keychain over the noted entry, so run `lite logout` right after unlocking. On a machine with no keychain at all, every later `lite logout` repeats the warning until you delete `~/.litellm/keychain_not_cleared`. A machine that an older `lite` logged out while its keychain was locked still has its `token.json`, and still reads as signed in, until you run `lite logout` once more. The stored credential is a short-lived, per-session agent token, not a managed virtual key. It is scoped to the user and team you logged in as and inherits their models and budgets; spend is tracked against the shared team and user budgets rather than a separate per-session cap, so multiple logins or several concurrent agents all draw down the same allowance. It is short-lived by design (default 24h, configurable via `LITELLM_CLI_JWT_EXPIRATION_HOURS`); re-run `lite login` to refresh it and pick up your latest team and user settings. `lite auth print-token` (usable as Claude Code's `apiKeyHelper`) prints it, and once it is past 24 hours (or the client's `LITELLM_CLI_JWT_EXPIRATION_HOURS`) asks the proxy first and fails if the proxy rejects it. There is no silent renewal. It is accepted on a default deployment without `EXPERIMENTAL_UI_LOGIN`, does not appear in the Keys UI, and cannot be rotated or revoked mid-session. A credential from `lite login --pkce` is the exception: it carries a refresh token, so the CLI renews the key shortly before it expires and `lite logout` revokes the refresh token on the proxy (see [Browser sign-in with PKCE](https://docs.litellm.ai/docs/proxy/cli_sso#browser-sign-in-with-pkce)). Only the holder can end a `--pkce` session early, with `lite logout`; an admin has no button for it, but every renewal re-reads the user on the proxy, so deactivating the user or removing them from the team makes the next renewal fail and the key runs out within `LITELLM_CLI_JWT_EXPIRATION_HOURS`. On a proxy with more than one worker or replica, configure Redis (`litellm_settings.cache` with Redis `cache_params`, or `general_settings.coordination_redis`) so a refresh token stays single-use and `lite logout` holds on every worker; without Redis each worker keeps its own record. For a long-lived, rotatable, Keys-UI-visible credential, create a dedicated virtual key in the dashboard and pass it via `--api-key` or `LITELLM_PROXY_API_KEY`. diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index e171e18ab86..fce6e3e871a 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/litellm/proxy/client/cli/commands/auth.py @@ -235,15 +235,28 @@ def load_token(*, vault: SecretVault = SYSTEM_KEYRING) -> Mapping[str, object] | return None if record is None else record.model_dump(exclude_none=True) -def _credential_to_revoke(vault: SecretVault) -> Mapping[str, object] | None: - """The stored login, or the one an earlier logout left in a keychain it could not reach""" - current: Final = load_token(vault=vault) - if current is not None: - return current +def _login_left_in_keychain(vault: SecretVault) -> Mapping[str, object] | None: + """The login an earlier logout left in a keychain it could not reach. + + Read it before `load_token`, which moves a secret a locked-keychain login kept on disk over + that keychain entry. + """ left: Final = load_cli_token_left_in_keychain(vault=vault) return None if left is None else left.model_dump(exclude_none=True) +def _same_refresh_token(left: Mapping[str, object] | None, current: Mapping[str, object] | None) -> bool: + return left is not None and current is not None and left.get("refresh_token") == current.get("refresh_token") + + +def _credentials_to_revoke(vault: SecretVault) -> tuple[Mapping[str, object], ...]: + left: Final = _login_left_in_keychain(vault) + current: Final = load_token(vault=vault) + if current is None or _same_refresh_token(left, current): + return () if left is None else (left,) + return (current,) if left is None else (left, current) + + def save_token(record: CliTokenData, *, vault: SecretVault = SYSTEM_KEYRING) -> SecretSave: """Store a credential the PKCE layer produced, secret in the vault and the rest on disk""" return save_cli_token(CliTokenRecord(**record), vault=vault) @@ -863,17 +876,22 @@ def _finish_login(base_url: str, api_key: str, config_claude: bool, stored: Secr def _replace_stored_token(record: CliTokenData, http: Http, vault: SecretVault) -> SecretSave: - previous: Final = _credential_to_revoke(vault) + previous: Final = _credentials_to_revoke(vault) stored: Final = save_token(record, vault=vault) - if previous is None or isinstance(stored, CredentialNotSaved): + if isinstance(stored, CredentialNotSaved): return stored - revocation: Final = revoke_stored_credential(previous, http) + for login in previous: + _revoke_the_replaced_login(login, http) + return stored + + +def _revoke_the_replaced_login(login: Mapping[str, object], http: Http) -> None: + revocation: Final = revoke_stored_credential(login, http) if revocation is not None: click.echo( f"Could not revoke the previous login's refresh token on the proxy ({revocation.reason}); " "it expires on its own." ) - return stored def _pkce_login(base_url: str, config_claude: bool, vault: SecretVault) -> None: @@ -997,13 +1015,8 @@ def login(ctx: click.Context, config_claude: bool, pkce: bool | None) -> None: return -@click.command(name="logout") -@click.pass_context -def logout(ctx: click.Context): - """Logout and clear stored authentication""" - vault: Final = context_secret_vault(ctx) - token_data: Final = _credential_to_revoke(vault) - revocation: Final = revoke_stored_credential(token_data, requests.Session()) if token_data is not None else None +def _revoke_before_logout(token_data: Mapping[str, object] | None, http: Http) -> None: + revocation: Final = None if token_data is None else revoke_stored_credential(token_data, http) match revocation: case RevocationUnavailable(reason=reason): raise click.ClickException( @@ -1017,6 +1030,18 @@ def logout(ctx: click.Context): case _: assert_never(revocation) + +@click.command(name="logout") +@click.pass_context +def logout(ctx: click.Context): + """Logout and clear stored authentication""" + vault: Final = context_secret_vault(ctx) + http: Final = requests.Session() + left: Final = _login_left_in_keychain(vault) + _revoke_before_logout(left, http) + current: Final = load_token(vault=vault) + _revoke_before_logout(None if _same_refresh_token(left, current) else current, http) + path: Final = get_cli_token_file_path() match clear_cli_token(vault=vault): case SecretErased(): diff --git a/tests/unit/litellm_core_utils/test_cli_token_utils.py b/tests/unit/litellm_core_utils/test_cli_token_utils.py index 602a831f544..ea19852ed56 100644 --- a/tests/unit/litellm_core_utils/test_cli_token_utils.py +++ b/tests/unit/litellm_core_utils/test_cli_token_utils.py @@ -895,7 +895,6 @@ class TestClearCliToken: assert load_cli_token(vault=vault) is None assert get_litellm_gateway_api_key(vault=vault) is None - assert load_cli_token_left_in_keychain(vault=vault) is None assert clear_cli_token(vault=vault) == SecretErased() assert vault.blob is None @@ -932,6 +931,40 @@ class TestClearCliToken: assert left is not None assert (left.key, left.refresh_token) == ("sk-first", "rt-first") + def test_a_locked_login_over_a_keychain_login_keeps_the_keychain_login_revocable( + self, isolated_home, secret_vault_factory + ): + vault = secret_vault_factory() + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-first", refresh_token="rt-first", timestamp=1000.0), vault=vault + ) + vault.available = False + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-second", refresh_token="rt-second", timestamp=2000.0), vault=vault + ) + + vault.available = True + left = load_cli_token_left_in_keychain(vault=vault) + + assert left is not None + assert (left.key, left.refresh_token) == ("sk-first", "rt-first") + + def test_the_note_never_pairs_with_a_keychain_entry_a_later_login_wrote( + self, isolated_home, secret_vault_factory + ): + vault = secret_vault_factory() + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-first", refresh_token="rt-first", timestamp=1000.0), vault=vault + ) + vault.available = False + assert clear_cli_token(vault=vault) == KeyringUnreachable() + vault.available = True + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-second", refresh_token="rt-second", timestamp=2000.0), vault=vault + ) + + assert load_cli_token_left_in_keychain(vault=vault) is None + @pytest.mark.parametrize("failure", [KeyringNotInstalled(), KeyringDisabled(), KeyringUnreachable()]) def test_logging_out_of_a_machine_that_never_logged_in_invents_nothing_to_warn_about( self, isolated_home, secret_vault_factory, failure diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index 42fe11790f4..ee9a3a012a2 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -2020,6 +2020,35 @@ class TestPkceLogoutCommand: "llm_srefresh_old", ] + @pytest.mark.parametrize("logout_while_locked", [True, False]) + def test_a_logout_after_a_locked_keychain_login_revokes_the_login_the_keychain_kept( + self, isolated_home, secret_vault_factory, logout_while_locked + ): + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=_pkce_credential()), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + if logout_while_locked: + self.runner.invoke(logout, obj={"secret_vault": locked}) + self.runner.invoke(login, ["--pkce"], obj={"base_url": PKCE_BASE_URL, "secret_vault": locked}) + result = self.runner.invoke(logout, obj={"secret_vault": unlocked}) + + assert result.output == "Logged out successfully. Authentication token cleared.\n" + assert unlocked.blob is None + assert list((isolated_home / ".litellm").iterdir()) == [] + assert [post for session in _FakeSession.instances for post in session.posts] == [ + ( + f"{PKCE_BASE_URL}/revoke", + {"token": token, "token_type_hint": "refresh_token", "client_id": "llm_dcrc_abc"}, + ) + for token in ("llm_srefresh_old", "llm_srefresh_fresh") + ] + def test_a_logout_after_the_keychain_refused_the_erase_does_not_revoke_the_login_again( self, isolated_home, secret_vault_factory ): From bfbf45b837f506ace8ef4031aed033756d752d5d Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 18:15:10 -0800 Subject: [PATCH 07/10] fix(cli): stop lite from reading the stored login before login and logout The cli group resolved the stored API key before every command, and that read moved a locked-keychain login's secret over the keychain entry the logout note names, so logout and login never saw the first login to revoke it. --- litellm/proxy/client/cli/main.py | 3 +- .../proxy/client/cli/test_auth_commands.py | 30 +++++++++++++++++-- 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/litellm/proxy/client/cli/main.py b/litellm/proxy/client/cli/main.py index a1964d8a1bb..e0bcaaf5a66 100644 --- a/litellm/proxy/client/cli/main.py +++ b/litellm/proxy/client/cli/main.py @@ -199,7 +199,8 @@ def cli(ctx: click.Context, show_version: bool, base_url: str | None, api_key: s # If no API key provided via flag or environment variable, try to load from saved token. # Pass base_url so we only use the stored key when it was issued for this server. setup_command: Final = ctx.invoked_subcommand in ("configure", "reconfigure", "unconfigure") - api_key_from_token_file: Final = api_key is None and not setup_command + reads_the_stored_login_itself: Final = ctx.invoked_subcommand in ("login", "logout") + api_key_from_token_file: Final = api_key is None and not setup_command and not reads_the_stored_login_itself resolved_api_key: Final = ( get_stored_api_key(expected_base_url=base_url, vault=context_secret_vault(ctx)) if api_key_from_token_file diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index ee9a3a012a2..743520fdc01 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -2034,9 +2034,9 @@ class TestPkceLogoutCommand: patch("litellm.proxy.client.cli.interface.show_commands"), ): if logout_while_locked: - self.runner.invoke(logout, obj={"secret_vault": locked}) - self.runner.invoke(login, ["--pkce"], obj={"base_url": PKCE_BASE_URL, "secret_vault": locked}) - result = self.runner.invoke(logout, obj={"secret_vault": unlocked}) + self.runner.invoke(cli, ["logout"], obj={"secret_vault": locked}) + self.runner.invoke(cli, ["--base-url", PKCE_BASE_URL, "login", "--pkce"], obj={"secret_vault": locked}) + result = self.runner.invoke(cli, ["logout"], obj={"secret_vault": unlocked}) assert result.output == "Logged out successfully. Authentication token cleared.\n" assert unlocked.blob is None @@ -2049,6 +2049,30 @@ class TestPkceLogoutCommand: for token in ("llm_srefresh_old", "llm_srefresh_fresh") ] + def test_a_login_after_a_locked_keychain_login_revokes_the_login_the_keychain_kept( + self, isolated_home, secret_vault_factory + ): + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=_pkce_credential()), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + self.runner.invoke(cli, ["--base-url", PKCE_BASE_URL, "login", "--pkce"], obj={"secret_vault": locked}) + result = self.runner.invoke( + cli, ["--base-url", PKCE_BASE_URL, "login", "--pkce"], obj={"secret_vault": unlocked} + ) + + assert result.exit_code == 0, result.output + assert "Could not revoke" not in result.output + assert [post["token"] for session in _FakeSession.instances for _, post in session.posts] == [ + "llm_srefresh_old", + "llm_srefresh_fresh", + ] + def test_a_logout_after_the_keychain_refused_the_erase_does_not_revoke_the_login_again( self, isolated_home, secret_vault_factory ): From f1f9a4163f5f3744e0a8ed58874eceff77eb2830 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 19:05:25 -0800 Subject: [PATCH 08/10] fix(cli): keep the login a locked logout left in the keychain revocable While the keychain still holds the login an earlier logout could not end, reading the stored login now serves the secret token.json kept and leaves that keychain entry alone, whatever the login times say. A clock that stepped back no longer serves the ended login, and commands other than logout and login no longer move the newer secret over it. A silent renewal that writes over the entry revokes that login on the proxy. --- litellm/litellm_core_utils/cli_token_utils.py | 16 +++++ litellm/proxy/client/README.md | 2 +- litellm/proxy/client/cli/commands/auth.py | 31 +++++----- .../proxy/client/cli/test_auth_commands.py | 59 +++++++++++++++++++ 4 files changed, 93 insertions(+), 15 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 1f1e032eda3..a55eb4522d2 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -295,6 +295,15 @@ def load_cli_token_left_in_keychain(*, vault: SecretVault = SYSTEM_KEYRING) -> C ) +def _holds_the_noted_login(blob: str) -> bool: + """Whether the keychain entry is the login an earlier logout could not end, which only it can revoke""" + note: Final = _read_keychain_not_cleared_note() + if note is None: + return False + secret: Final = _decode_secret(blob, note.base_url) + return secret is not None and secret.timestamp == note.timestamp + + def _read_keychain_not_cleared_note() -> CliTokenRecord | None: try: return CliTokenRecord.model_validate_json(_keychain_not_cleared_note_path().read_bytes()) @@ -464,7 +473,14 @@ def _apply_vault_secret(record: CliTokenRecord, blob: str, vault: SecretVault) - than counting as a clean sweep. Rolling the vault back the way a migration does is not the answer here, because the two stores hold different credentials and the rollback would hand the superseded one back out. + + A keychain entry an earlier logout could not end is left alone whatever the stamps say. Its note + pairs only with that entry, so the next login or logout can still revoke it, while serving it + would sign an ended login back in and moving the file's secret over it would leave its refresh + token live with nothing on this machine able to revoke it. """ + if _holds_a_secret(record) and _holds_the_noted_login(blob): + return record secret: Final = _decode_secret(blob, record.base_url) if secret is None or (_holds_a_secret(record) and secret.timestamp <= record.timestamp): return _migrate_file_secret(_rejoined(record, secret), vault, replacing=secret) diff --git a/litellm/proxy/client/README.md b/litellm/proxy/client/README.md index 783086e577d..92c638cf1de 100644 --- a/litellm/proxy/client/README.md +++ b/litellm/proxy/client/README.md @@ -380,7 +380,7 @@ The key itself, together with the refresh token that renews a `--pkce` credentia Keychain storage needs the `keyring` package, which ships with `pip install 'litellm[cli]'`. Headless boxes and CI runners usually have no keychain either. In all of those cases the key and the refresh token stay in the same `0600` file alongside the metadata, exactly as they did before, and `lite login` names which one applies: the package is missing, the machine has no keychain, or you set `LITELLM_CLI_DISABLE_KEYRING=1` to force the file even where a keychain exists. A `token.json` written by an older `lite` keeps working and is moved into the keychain, and scrubbed from the file, the first time a keychain-capable `lite` reads it. That includes a refresh token left behind by the release that moved only the key. -`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. A login made while the keychain is still locked keeps its secret in `token.json` and the note names the keychain's login, so the first `lite logout` or `lite login` after the unlock revokes both. Any other command run first moves the newer secret into the keychain over the noted entry, so run `lite logout` right after unlocking. On a machine with no keychain at all, every later `lite logout` repeats the warning until you delete `~/.litellm/keychain_not_cleared`. A machine that an older `lite` logged out while its keychain was locked still has its `token.json`, and still reads as signed in, until you run `lite logout` once more. +`lite logout` clears both stores. If the keychain is locked at that moment it says so, removes `token.json`, and leaves `~/.litellm/keychain_not_cleared` holding that login's metadata without its secret. Re-running it once the keychain is unlocked revokes a `--pkce` refresh token on the proxy and finishes the job. A `lite login` made once the keychain answers revokes that login too, since it replaces the entry. A login made while the keychain is still locked keeps its secret in `token.json` and the note names the keychain's login, so the first `lite logout` or `lite login` after the unlock revokes both. Other commands leave the noted entry where it is, and a silent renewal that takes over the keychain entry revokes that login too. On a machine with no keychain at all, every later `lite logout` repeats the warning until you delete `~/.litellm/keychain_not_cleared`. A machine that an older `lite` logged out while its keychain was locked still has its `token.json`, and still reads as signed in, until you run `lite logout` once more. The stored credential is a short-lived, per-session agent token, not a managed virtual key. It is scoped to the user and team you logged in as and inherits their models and budgets; spend is tracked against the shared team and user budgets rather than a separate per-session cap, so multiple logins or several concurrent agents all draw down the same allowance. It is short-lived by design (default 24h, configurable via `LITELLM_CLI_JWT_EXPIRATION_HOURS`); re-run `lite login` to refresh it and pick up your latest team and user settings. `lite auth print-token` (usable as Claude Code's `apiKeyHelper`) prints it, and once it is past 24 hours (or the client's `LITELLM_CLI_JWT_EXPIRATION_HOURS`) asks the proxy first and fails if the proxy rejects it. There is no silent renewal. It is accepted on a default deployment without `EXPERIMENTAL_UI_LOGIN`, does not appear in the Keys UI, and cannot be rotated or revoked mid-session. A credential from `lite login --pkce` is the exception: it carries a refresh token, so the CLI renews the key shortly before it expires and `lite logout` revokes the refresh token on the proxy (see [Browser sign-in with PKCE](https://docs.litellm.ai/docs/proxy/cli_sso#browser-sign-in-with-pkce)). Only the holder can end a `--pkce` session early, with `lite logout`; an admin has no button for it, but every renewal re-reads the user on the proxy, so deactivating the user or removing them from the team makes the next renewal fail and the key runs out within `LITELLM_CLI_JWT_EXPIRATION_HOURS`. On a proxy with more than one worker or replica, configure Redis (`litellm_settings.cache` with Redis `cache_params`, or `general_settings.coordination_redis`) so a refresh token stays single-use and `lite logout` holds on every worker; without Redis each worker keeps its own record. For a long-lived, rotatable, Keys-UI-visible credential, create a dedicated virtual key in the dashboard and pass it via `--api-key` or `LITELLM_PROXY_API_KEY`. diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index fce6e3e871a..eb010808238 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/litellm/proxy/client/cli/commands/auth.py @@ -236,11 +236,7 @@ def load_token(*, vault: SecretVault = SYSTEM_KEYRING) -> Mapping[str, object] | def _login_left_in_keychain(vault: SecretVault) -> Mapping[str, object] | None: - """The login an earlier logout left in a keychain it could not reach. - - Read it before `load_token`, which moves a secret a locked-keychain login kept on disk over - that keychain entry. - """ + """The login an earlier logout left in a keychain it could not reach""" left: Final = load_cli_token_left_in_keychain(vault=vault) return None if left is None else left.model_dump(exclude_none=True) @@ -262,17 +258,22 @@ def save_token(record: CliTokenData, *, vault: SecretVault = SYSTEM_KEYRING) -> return save_cli_token(CliTokenRecord(**record), vault=vault) -def _renewal_saver(vault: SecretVault) -> Callable[[CliTokenData], None]: +def _renewal_saver(vault: SecretVault, http: Http) -> Callable[[CliTokenData], None]: """Persist a silently renewed credential, and say on stderr when no store would keep it. A renewal rotates the refresh token, so a rotation that is never stored logs this - machine out on the next command; the user hears about it rather than guessing. + machine out on the next command; the user hears about it rather than guessing. The save + can take the keychain entry of a login an earlier logout could not end, which leaves no + other record of that login's refresh token, so it is revoked the way a login revokes it. """ def save(record: CliTokenData) -> None: + left: Final = _login_left_in_keychain(vault) outcome: Final = save_token(record, vault=vault) if isinstance(outcome, (CredentialNotSaved, CredentialNotRecorded)): _warn(storage_notice(outcome)) + if left is not None: + _revoke_the_replaced_login(left, http, _warn) return save @@ -303,10 +304,11 @@ def get_stored_api_key( return None if expected_base_url is not None and token_data.get("base_url") != expected_base_url.rstrip("/"): return None + http: Final = requests.Session() return fresh_api_key( token_data, - _renewal_saver(vault), - requests.Session(), + _renewal_saver(vault, http), + http, reload=_renewal_reader(vault), warn=_warn, ) @@ -881,14 +883,14 @@ def _replace_stored_token(record: CliTokenData, http: Http, vault: SecretVault) if isinstance(stored, CredentialNotSaved): return stored for login in previous: - _revoke_the_replaced_login(login, http) + _revoke_the_replaced_login(login, http, click.echo) return stored -def _revoke_the_replaced_login(login: Mapping[str, object], http: Http) -> None: +def _revoke_the_replaced_login(login: Mapping[str, object], http: Http, report: Callable[[str], None]) -> None: revocation: Final = revoke_stored_credential(login, http) if revocation is not None: - click.echo( + report( f"Could not revoke the previous login's refresh token on the proxy ({revocation.reason}); " "it expires on its own." ) @@ -1102,13 +1104,14 @@ def print_token(ctx: click.Context): click.echo(keychain_unreadable_notice(vault), err=True) sys.exit(1) + http: Final = requests.Session() api_key: Final = ( ctx_obj.get("api_key") if issued_for_this_server and ctx_obj.get("api_key_from_token_file") else fresh_api_key( token_data, - _renewal_saver(vault), - requests.Session(), + _renewal_saver(vault, http), + http, reload=_renewal_reader(vault), warn=_warn, ) diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index 743520fdc01..156999f37ce 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -1,3 +1,4 @@ +import dataclasses import json import os import stat @@ -2073,6 +2074,64 @@ class TestPkceLogoutCommand: "llm_srefresh_fresh", ] + @pytest.mark.parametrize("seconds_the_clock_stepped_back", [0, 600]) + def test_a_command_between_the_unlock_and_the_logout_keeps_the_login_the_keychain_kept_revocable( + self, isolated_home, secret_vault_factory, seconds_the_clock_stepped_back + ): + unlocked = secret_vault_factory() + first_login = _pkce_record(timestamp=time.time() + seconds_the_clock_stepped_back) + save_cli_token(CliTokenRecord(**first_login), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=_pkce_credential()), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + self.runner.invoke(cli, ["logout"], obj={"secret_vault": locked}) + self.runner.invoke(cli, ["--base-url", PKCE_BASE_URL, "login", "--pkce"], obj={"secret_vault": locked}) + printed = self.runner.invoke(cli, ["auth", "print-token"], obj={"secret_vault": unlocked}) + result = self.runner.invoke(cli, ["logout"], obj={"secret_vault": unlocked}) + + assert printed.stdout == "sk-cli-fresh\n" + assert result.output == "Logged out successfully. Authentication token cleared.\n" + assert [post["token"] for session in _FakeSession.instances for _, post in session.posts] == [ + "llm_srefresh_old", + "llm_srefresh_fresh", + ] + + @pytest.mark.parametrize("revocation_status", [200, 503]) + def test_a_renewal_that_takes_the_keychain_entry_revokes_the_login_left_in_it( + self, isolated_home, secret_vault_factory, revocation_status + ): + class _RenewingSession(_FakeSession): + def post(self, url, *, data=None, json=None, timeout, allow_redirects): + self.posts.append((url, data)) + if url.endswith("/token"): + return _FakeHttpResponse(200, PKCE_TOKEN_RESPONSE) + return _FakeHttpResponse(revocation_status, {}) + + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + expiring_login = dataclasses.replace(_pkce_credential(), expires_at=time.time() + 30) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _RenewingSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=expiring_login), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + self.runner.invoke(cli, ["logout"], obj={"secret_vault": locked}) + self.runner.invoke(cli, ["--base-url", PKCE_BASE_URL, "login", "--pkce"], obj={"secret_vault": locked}) + printed = self.runner.invoke(cli, ["auth", "print-token"], obj={"secret_vault": unlocked}) + self.runner.invoke(cli, ["logout"], obj={"secret_vault": unlocked}) + + assert printed.stdout == "sk-cli-rotated\n" + assert ("Could not revoke" in printed.stderr) is (revocation_status != 200) + assert [ + post["token"] for session in _FakeSession.instances for url, post in session.posts if url.endswith("/revoke") + ] == ["llm_srefresh_old", "llm_srefresh_rotated"] + def test_a_logout_after_the_keychain_refused_the_erase_does_not_revoke_the_login_again( self, isolated_home, secret_vault_factory ): From 88acbdca59dc9235d616a93afed7f97e660ad458 Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:02:29 -0800 Subject: [PATCH 09/10] fix(cli): keep a login revocable when its note cannot be written, and never block logout on an earlier login's proxy A locked logout whose note cannot take the login's record now puts token.json back, so the next logout that reaches the keychain still revokes that login, as the base did. A logout that ends both a current login and an earlier one left in the keychain warns when the earlier login's proxy cannot record the revocation, as lite login does, instead of refusing to log out of the current proxy. --- litellm/litellm_core_utils/cli_token_utils.py | 49 ++++++++---- litellm/proxy/client/cli/commands/auth.py | 19 ++++- .../proxy/client/cli/test_auth_commands.py | 78 +++++++++++++++++++ 3 files changed, 127 insertions(+), 19 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index a55eb4522d2..3ea369767cf 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -250,12 +250,19 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear: record: Final = _read_token_file() settled: Final = _nothing_left_behind(outcome, record) left_on_disk: Final = _end_the_local_login(record) - _note_what_the_keychain_answered(outcome, record) + unnoted: Final = _note_what_the_keychain_answered(outcome, record) + if unnoted is not None: + _put_the_token_file_back(unnoted) if left_on_disk is not None: return left_on_disk return SecretErased() if settled else outcome +def _put_the_token_file_back(record: CliTokenRecord) -> None: + with suppress(OSError): + _write_token_file(record) + + def _end_the_local_login(record: CliTokenRecord | None) -> CredentialNotCleared | None: removal: Final = _remove_token_file() if removal is None or record is None or _scrub_file_secret(record): @@ -311,7 +318,7 @@ def _read_keychain_not_cleared_note() -> CliTokenRecord | None: return None -def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecord | None) -> None: +def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecord | None) -> CliTokenRecord | None: """Keep the note that a logout left the keychain uncleared for exactly as long as that is true. A keychain that could not be read kept this logout from revoking the login, so the note holds @@ -321,28 +328,38 @@ def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecor kept its secret in the file never reached the keychain, so the note an earlier logout left still names the login the keychain holds and stays, and a logout with no file to end keeps it too. - The secret is off disk by the time this runs, so a note that will not be written or removed is - no reason to fail the logout. The warning printed now is then the only one the user gets. + Only a note that cannot take an unrevoked login's record keeps that login from being revoked + later, so that record comes back for the token file to keep, as it did before the note existed. + Any other note that will not be written or removed is no reason to fail the logout, and the + warning printed now is then the only one the user gets. """ note: Final = _keychain_not_cleared_note_path() - with suppress(OSError): - match outcome: - case SecretErased(): + match outcome: + case SecretErased(): + with suppress(OSError): note.unlink(missing_ok=True) - case SecretStranded(): + return None + case SecretStranded(): + with suppress(OSError): _write_keychain_not_cleared_note(note, None) - case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - _note_a_keychain_left_unchecked(note, record) + return None + case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): + return _note_a_keychain_left_unchecked(note, record) -def _note_a_keychain_left_unchecked(note: Path, record: CliTokenRecord | None) -> None: +def _note_a_keychain_left_unchecked(note: Path, record: CliTokenRecord | None) -> CliTokenRecord | None: if record is None: - return + return None if not _holds_a_secret(record): - _write_keychain_not_cleared_note(note, record) - return - if not note.exists(): - _write_keychain_not_cleared_note(note, None) + try: + _write_keychain_not_cleared_note(note, record) + except OSError: + return record + return None + with suppress(OSError): + if not note.exists(): + _write_keychain_not_cleared_note(note, None) + return None def _write_keychain_not_cleared_note(note: Path, record: CliTokenRecord | None) -> None: diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index eb010808238..ccead6c8366 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/litellm/proxy/client/cli/commands/auth.py @@ -1017,7 +1017,21 @@ def login(ctx: click.Context, config_claude: bool, pkce: bool | None) -> None: return -def _revoke_before_logout(token_data: Mapping[str, object] | None, http: Http) -> None: +def _revoke_before_logout(left: Mapping[str, object] | None, current: Mapping[str, object] | None, http: Http) -> None: + """Revoke every login this logout ends, holding the logout back only for the one it signs out. + + An earlier login left in the keychain is the previous login once another one is current, so a + proxy that cannot revoke it only earns the warning `lite login` gives, never a logout that the + current proxy is ready to finish. + """ + if left is None or current is None or _same_refresh_token(left, current): + _revoke_the_login_or_keep_it(current if left is None else left, http) + return + _revoke_the_replaced_login(left, http, click.echo) + _revoke_the_login_or_keep_it(current, http) + + +def _revoke_the_login_or_keep_it(token_data: Mapping[str, object] | None, http: Http) -> None: revocation: Final = None if token_data is None else revoke_stored_credential(token_data, http) match revocation: case RevocationUnavailable(reason=reason): @@ -1040,9 +1054,8 @@ def logout(ctx: click.Context): vault: Final = context_secret_vault(ctx) http: Final = requests.Session() left: Final = _login_left_in_keychain(vault) - _revoke_before_logout(left, http) current: Final = load_token(vault=vault) - _revoke_before_logout(None if _same_refresh_token(left, current) else current, http) + _revoke_before_logout(left, current, http) path: Final = get_cli_token_file_path() match clear_cli_token(vault=vault): diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index 156999f37ce..8737bd2e69d 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -1971,6 +1971,28 @@ class TestPkceLogoutCommand: ) ] + def test_a_logout_that_cannot_note_the_keychain_login_leaves_it_for_the_next_logout_to_revoke( + self, isolated_home, secret_vault_factory + ): + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + (isolated_home / ".litellm" / "keychain_not_cleared").mkdir() + + with patch("litellm.proxy.client.cli.commands.auth.requests.Session", _FakeSession): + locked_logout = self.runner.invoke(logout, obj={"secret_vault": locked}) + result = self.runner.invoke(logout, obj={"secret_vault": unlocked}) + + assert locked_logout.exit_code == 0, locked_logout.output + assert result.output == "Logged out successfully. Authentication token cleared.\n" + assert unlocked.blob is None + assert [post for session in _FakeSession.instances for post in session.posts] == [ + ( + f"{PKCE_BASE_URL}/revoke", + {"token": "llm_srefresh_old", "token_type_hint": "refresh_token", "client_id": "llm_dcrc_abc"}, + ) + ] + def test_a_login_after_a_logout_that_missed_the_keychain_revokes_the_login_it_replaces( self, isolated_home, secret_vault_factory ): @@ -2050,6 +2072,62 @@ class TestPkceLogoutCommand: for token in ("llm_srefresh_old", "llm_srefresh_fresh") ] + def test_a_proxy_that_cannot_revoke_the_earlier_login_never_holds_back_the_current_logout( + self, isolated_home, secret_vault_factory + ): + class _EarlierProxyUnavailableSession(_FakeSession): + def post(self, url, *, data=None, json=None, timeout, allow_redirects): + self.posts.append((url, data)) + if data["token"] == "llm_srefresh_old": + return _FakeHttpResponse(503, {"error": "temporarily_unavailable"}) + return _FakeHttpResponse(200, {}) + + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with ( + patch("litellm.proxy.client.cli.commands.auth.requests.Session", _EarlierProxyUnavailableSession), + patch("litellm.proxy.client.cli.commands.auth.run_pkce_login", return_value=_pkce_credential()), + patch("litellm.proxy.client.cli.interface.show_commands"), + ): + self.runner.invoke(cli, ["logout"], obj={"secret_vault": locked}) + self.runner.invoke(cli, ["--base-url", PKCE_BASE_URL, "login", "--pkce"], obj={"secret_vault": locked}) + result = self.runner.invoke(cli, ["logout"], obj={"secret_vault": unlocked}) + + assert result.exit_code == 0, result.output + assert ( + "Could not revoke the previous login's refresh token on the proxy (revocation failed with 503" + in result.output + ) + assert result.output.endswith("Logged out successfully. Authentication token cleared.\n") + assert unlocked.blob is None + assert list((isolated_home / ".litellm").iterdir()) == [] + assert [post["token"] for session in _FakeSession.instances for _, post in session.posts] == [ + "llm_srefresh_old", + "llm_srefresh_fresh", + ] + + def test_a_logout_whose_only_login_is_the_one_left_in_the_keychain_waits_for_its_revocation( + self, isolated_home, secret_vault_factory + ): + class _UnavailableSession(_FakeSession): + def __init__(self): + super().__init__() + self.response = _FakeHttpResponse(503, {"error": "temporarily_unavailable"}) + + unlocked = secret_vault_factory() + save_cli_token(CliTokenRecord(**_pkce_record()), vault=unlocked) + locked = secret_vault_factory(blob=unlocked.blob, available=False) + + with patch("litellm.proxy.client.cli.commands.auth.requests.Session", _UnavailableSession): + self.runner.invoke(cli, ["logout"], obj={"secret_vault": locked}) + result = self.runner.invoke(cli, ["logout"], obj={"secret_vault": unlocked}) + + assert result.exit_code == 1 + assert "Nothing was cleared; run `lite logout` again shortly." in result.output + assert unlocked.blob is not None + def test_a_login_after_a_locked_keychain_login_revokes_the_login_the_keychain_kept( self, isolated_home, secret_vault_factory ): From 693d4217ec78e5d3e71db25f4f5fb970137d596b Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Wed, 4 Nov 2026 21:29:38 -0800 Subject: [PATCH 10/10] fix(cli): move a locked keychain login's token file into its note instead of putting it back --- litellm/litellm_core_utils/cli_token_utils.py | 90 ++++++++++--------- .../test_cli_token_utils.py | 27 +++++- .../proxy/client/cli/test_auth_commands.py | 1 + 3 files changed, 72 insertions(+), 46 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 3ea369767cf..b4e6d04cdf4 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -249,21 +249,44 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear: outcome: Final = vault.erase() record: Final = _read_token_file() settled: Final = _nothing_left_behind(outcome, record) - left_on_disk: Final = _end_the_local_login(record) - unnoted: Final = _note_what_the_keychain_answered(outcome, record) - if unnoted is not None: - _put_the_token_file_back(unnoted) + left_on_disk: Final = _end_the_local_login(outcome, record) if left_on_disk is not None: return left_on_disk return SecretErased() if settled else outcome -def _put_the_token_file_back(record: CliTokenRecord) -> None: - with suppress(OSError): - _write_token_file(record) +def _end_the_local_login(outcome: SecretErase, record: CliTokenRecord | None) -> CredentialNotCleared | None: + if _names_a_login_the_keychain_kept(outcome, record): + return _move_token_file_to_note() + left_on_disk: Final = _remove_token_file_or_its_secret(record) + _note_what_the_keychain_answered(outcome, record) + return left_on_disk -def _end_the_local_login(record: CliTokenRecord | None) -> CredentialNotCleared | None: +def _names_a_login_the_keychain_kept(outcome: SecretErase, record: CliTokenRecord | None) -> bool: + match outcome: + case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): + return record is not None and not _holds_a_secret(record) + case SecretErased() | SecretStranded(): + return False + + +def _move_token_file_to_note() -> CredentialNotCleared | None: + """Turn the token file of a login the keychain could not be asked to end into the note that ends it later. + + The file holds no secret, so it is already the record a later logout or login pairs with the + keychain entry to revoke the login, and renaming it needs no free space on a full disk. The file + never stays behind as a login that signs the user back in once the keychain unlocks: a note path + that will not take it leaves the file where it is, and the logout says so. + """ + try: + os.replace(get_cli_token_file_path(), _keychain_not_cleared_note_path()) + except OSError as error: + return CredentialNotCleared(str(error)) + return None + + +def _remove_token_file_or_its_secret(record: CliTokenRecord | None) -> CredentialNotCleared | None: removal: Final = _remove_token_file() if removal is None or record is None or _scrub_file_secret(record): return None @@ -318,48 +341,27 @@ def _read_keychain_not_cleared_note() -> CliTokenRecord | None: return None -def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecord | None) -> CliTokenRecord | None: +def _note_what_the_keychain_answered(outcome: SecretErase, record: CliTokenRecord | None) -> None: """Keep the note that a logout left the keychain uncleared for exactly as long as that is true. - A keychain that could not be read kept this logout from revoking the login, so the note holds - that login's record without its secret, and the next logout or login that finds the keychain - answering pairs it with the entry to revoke it. One that would not give up its entry had already - handed this logout the refresh token, so its note holds nothing to revoke again. A login that - kept its secret in the file never reached the keychain, so the note an earlier logout left still - names the login the keychain holds and stays, and a logout with no file to end keeps it too. + A keychain that would not give up its entry had already handed this logout the refresh token, + so its note holds nothing to revoke again. A login that kept its secret in the file never + reached the keychain, so the note an earlier logout left still names the login the keychain + holds and stays, and a logout with no file to end keeps it too. - Only a note that cannot take an unrevoked login's record keeps that login from being revoked - later, so that record comes back for the token file to keep, as it did before the note existed. - Any other note that will not be written or removed is no reason to fail the logout, and the - warning printed now is then the only one the user gets. + The secret is off disk by the time this runs, so a note that will not be written or removed is + no reason to fail the logout. The warning printed now is then the only one the user gets. """ note: Final = _keychain_not_cleared_note_path() - match outcome: - case SecretErased(): - with suppress(OSError): - note.unlink(missing_ok=True) - return None - case SecretStranded(): - with suppress(OSError): - _write_keychain_not_cleared_note(note, None) - return None - case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): - return _note_a_keychain_left_unchecked(note, record) - - -def _note_a_keychain_left_unchecked(note: Path, record: CliTokenRecord | None) -> CliTokenRecord | None: - if record is None: - return None - if not _holds_a_secret(record): - try: - _write_keychain_not_cleared_note(note, record) - except OSError: - return record - return None with suppress(OSError): - if not note.exists(): - _write_keychain_not_cleared_note(note, None) - return None + match outcome: + case SecretErased(): + note.unlink(missing_ok=True) + case SecretStranded(): + _write_keychain_not_cleared_note(note, None) + case KeyringDisabled() | KeyringNotInstalled() | KeyringUnreachable(): + if record is not None and not note.exists(): + _write_keychain_not_cleared_note(note, None) def _write_keychain_not_cleared_note(note: Path, record: CliTokenRecord | None) -> None: diff --git a/tests/unit/litellm_core_utils/test_cli_token_utils.py b/tests/unit/litellm_core_utils/test_cli_token_utils.py index ea19852ed56..19f9f263ee4 100644 --- a/tests/unit/litellm_core_utils/test_cli_token_utils.py +++ b/tests/unit/litellm_core_utils/test_cli_token_utils.py @@ -128,6 +128,10 @@ class _MkstempThatNeedsTheOldFileGone: return _REAL_MKSTEMP(*args, **kwargs) +def _full_disk_mkstemp(*args, **kwargs): + raise OSError(errno.ENOSPC, "No space left on device") + + _REAL_REPLACE = os.replace @@ -1022,7 +1026,8 @@ class TestClearCliToken: ): """A read-only `~/.litellm` accepts no removal of the token file and no new file beside it, and still lets the one it has be shortened. The secret goes, the file stays because nothing - else can, and the logout after it warns again rather than calling the keychain clean. + else can, and the logout after it says the file is still there, since a keychain that answers + again would sign it back in, rather than calling the machine logged out. The key is a realistic length so the file genuinely shrinks: a rewrite in place that leaves the tail of the old contents behind hands the next run a file it cannot parse.""" @@ -1031,7 +1036,7 @@ class TestClearCliToken: path.parent.chmod(0o500) try: assert clear_cli_token(vault=vault) == KeyringUnreachable() - assert clear_cli_token(vault=vault) == KeyringUnreachable() + assert isinstance(clear_cli_token(vault=vault), CredentialNotCleared) finally: path.parent.chmod(0o700) @@ -1053,6 +1058,24 @@ class TestClearCliToken: assert not path.exists() assert "sk-legacy" not in _everything_on_disk(isolated_home) + def test_a_full_disk_keeps_a_locked_keychain_login_revocable_without_signing_it_back_in( + self, isolated_home, secret_vault_factory, monkeypatch + ): + vault = secret_vault_factory() + save_cli_token( + CliTokenRecord(base_url=SERVER, key="sk-first", refresh_token="rt-first", timestamp=1000.0), vault=vault + ) + monkeypatch.setattr("litellm.litellm_core_utils.private_json.tempfile.mkstemp", _full_disk_mkstemp) + vault.available = False + + assert clear_cli_token(vault=vault) == KeyringUnreachable() + + vault.available = True + left = load_cli_token_left_in_keychain(vault=vault) + assert left is not None + assert left.refresh_token == "rt-first" + assert load_cli_token(vault=vault) is None + @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions") def test_a_metadata_file_that_will_not_go_is_not_worth_alarming_the_user_over( self, isolated_home, secret_vault_factory diff --git a/tests/unit/proxy/client/cli/test_auth_commands.py b/tests/unit/proxy/client/cli/test_auth_commands.py index 8737bd2e69d..babf44c1cc8 100644 --- a/tests/unit/proxy/client/cli/test_auth_commands.py +++ b/tests/unit/proxy/client/cli/test_auth_commands.py @@ -1984,6 +1984,7 @@ class TestPkceLogoutCommand: result = self.runner.invoke(logout, obj={"secret_vault": unlocked}) assert locked_logout.exit_code == 0, locked_logout.output + assert "could not be removed" in locked_logout.output assert result.output == "Logged out successfully. Authentication token cleared.\n" assert unlocked.blob is None assert [post for session in _FakeSession.instances for post in session.posts] == [