fix(cli): stop a repeat logout from retracting its own keychain warning

A logout that could not reach the keychain deleted the token file whenever it
still held its own secret, and the next logout read that missing file as proof
the keychain was clean. It answered the warning the first run had just issued
with "Logged out successfully" while the entry an earlier login left behind was
still live. The file is the only record that something may still be in there,
which is what `_nothing_left_behind` already says it relies on, so keep it and
take only the secret out.

A keychain that did answer is a different case. `SecretStranded` means the entry
is confirmed there and would not delete, and that needs no note in the file,
while keeping one lets every later command read the credential straight back out
of the keychain, which makes "Logged out locally" untrue. That one drops the
file, as it did before.

The secret still goes first either way: a copy that cannot be replaced with a
secret-free one is removed rather than kept.

litellm/litellm_core_utils/cli_token_utils.py

@@ -153,19 +153,33 @@ def _keep_the_secret_in_the_file(record: CliTokenRecord, outcome: SecretWrite) -
 def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretErase:
     """Remove the credential from both stores. Reports whether the keychain is now free of it.
 
-    A file that holds no secret of its own is kept when the keychain will not confirm the entry is
-    gone, because it is the only remaining record that something is still in there to remove. That
-    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. Anything still holding a secret is removed either way.
+    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.
     """
     outcome: Final = vault.erase()
     record: Final = _read_token_file()
     settled: Final = _nothing_left_behind(outcome, record)
-    if settled or record is None or record.key is not None:
+    if settled or not _keep_the_unchecked_keychain_on_record(outcome, record):
         Path(get_cli_token_file_path()).unlink(missing_ok=True)
     return SecretErased() if settled else outcome
 
 
+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 cannot be
+    replaced with a secret-free one is not kept either, because the secret goes first.
+    """
+    if record is None or isinstance(outcome, SecretStranded):
+        return False
+    return _scrub_file_secret(record)
+
+
 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.
 

tests/test_litellm/litellm_core_utils/test_cli_token_utils.py

@@ -477,6 +477,18 @@ class TestClearCliToken:
         assert clear_cli_token(vault=vault) == SecretStranded()
         assert not _token_file(isolated_home).exists()
 
+    def test_a_keychain_that_will_not_release_the_secret_still_ends_the_local_login(
+        self, isolated_home, secret_vault_factory
+    ):
+        """The warning this returns says the machine is logged out locally and the keychain entry is
+        what is left over. Keeping the file that names that entry makes the first half untrue: every
+        later command reads the credential straight back out of the keychain and keeps working."""
+        _write_metadata_only_file(isolated_home)
+        vault = secret_vault_factory(blob=_blob(), erasable=False)
+
+        assert clear_cli_token(vault=vault) == SecretStranded()
+        assert load_cli_token(vault=vault) is None
+
     @pytest.mark.parametrize(
         "failure", [KeyringDisabled(), KeyringUnreachable(), KeyringDiscardsWrites()]
     )
@@ -491,7 +503,7 @@ class TestClearCliToken:
         vault = secret_vault_factory(available=False, failure=failure)
 
         assert clear_cli_token(vault=vault) == failure
-        assert not _token_file(isolated_home).exists()
+        assert json.loads(_token_file(isolated_home).read_text()).get("key") is None
 
     def test_a_second_logout_still_reports_the_keychain_it_could_not_clear(
         self, isolated_home, secret_vault_factory
@@ -539,7 +551,25 @@ class TestClearCliToken:
 
         clear_cli_token(vault=vault)
 
-        assert not _token_file(isolated_home).exists()
+        assert "sk-legacy" not in _token_file(isolated_home).read_text()
+
+    def test_a_repeat_logout_never_answers_its_own_warning_with_an_all_clear(
+        self, isolated_home, secret_vault_factory
+    ):
+        """Sign in while the keychain works, sign in again once it has gone out of reach so the
+        second secret lands in the file, then log out twice. The first logout cannot say the first
+        login's entry is gone, and says so. If the second one reads the file the first one took
+        away as proof of a clean keychain, it retracts that warning while the credential behind it
+        is still live."""
+        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()
+        assert clear_cli_token(vault=vault) == KeyringUnreachable()
+        assert vault.blob is not None
+        assert "sk-second" not in _token_file(isolated_home).read_text()
 
     @pytest.mark.parametrize("failure", [KeyringNotInstalled(), KeyringDisabled(), KeyringUnreachable()])
     def test_logging_out_of_a_machine_that_never_logged_in_invents_nothing_to_warn_about(

tests/test_litellm/proxy/client/cli/test_auth_commands.py

@@ -1074,7 +1074,7 @@ class TestKeychainBackedCommands:
 
         assert result.exit_code == 0
         assert "could not be removed" in result.output
-        assert json.loads((isolated_home / ".litellm" / "token.json").read_text()).get("key") is None
+        assert not (isolated_home / ".litellm" / "token.json").exists()
 
     def test_print_token_explains_a_locked_keychain_instead_of_printing_nothing(
         self, isolated_home, secret_vault_factory