fix(cli): write the logout note again when the file holding it had to go

When a full disk refuses the replacement file and a read-only token file
refuses the rewrite in place, 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, so its absence made the next logout read a
keychain that was never confirmed as one 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.

litellm/litellm_core_utils/cli_token_utils.py

@@ -173,7 +173,8 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear:
     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 outranks whatever the keychain had to say.
+    neither its copy nor itself is removed rather than kept, with the note written again afterwards
+    so the warning still outlives this run.
     """
     outcome: Final = vault.erase()
     record: Final = _read_token_file()
@@ -183,6 +184,8 @@ def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretClear:
     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)
     return SecretErased() if settled else outcome
 
 
@@ -194,6 +197,19 @@ 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.
+
+    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.
+    """
+    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.
 
@@ -203,17 +219,27 @@ def _keep_the_unchecked_keychain_on_record(outcome: SecretErase, record: CliToke
     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 isinstance(outcome, SecretStranded):
+    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
+
+
 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, so a missing file is real evidence rather than the absence of it. Past that, 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

tests/test_litellm/litellm_core_utils/test_cli_token_utils.py

@@ -1,7 +1,9 @@
+import errno
 import json
 import os
 import stat
 import sys
+import tempfile
 import threading
 import time
 
@@ -82,6 +84,25 @@ def _blob(base_url=SERVER, key="sk-vault", jwt_token=""):
     return json.dumps({"base_url": base_url, "key": key, "jwt_token": jwt_token})
 
 
+_REAL_MKSTEMP = tempfile.mkstemp
+
+
+class _MkstempThatNeedsTheOldFileGone:
+    """A disk with exactly one token file's worth of room left on it.
+
+    Staging a replacement needs room for a second file, which is what a full disk refuses. Removing
+    the file already there is what gives that room back.
+    """
+
+    def __init__(self, path):
+        self.path = path
+
+    def __call__(self, *args, **kwargs):
+        if self.path.exists():
+            raise OSError(errno.ENOSPC, "No space left on device")
+        return _REAL_MKSTEMP(*args, **kwargs)
+
+
 _REAL_REPLACE = os.replace
 
 
@@ -553,7 +574,7 @@ class TestClearCliToken:
         assert load_cli_token(vault=vault) is None
 
     @pytest.mark.parametrize(
-        "failure", [KeyringDisabled(), KeyringUnreachable(), KeyringDiscardsWrites()]
+        "failure", [KeyringDisabled(), KeyringUnreachable(), KeyringNotInstalled()]
     )
     def test_a_secret_in_the_file_is_no_evidence_about_a_keychain_that_exists(
         self, isolated_home, secret_vault_factory, failure
@@ -561,7 +582,10 @@ class TestClearCliToken:
         """Store a secret in the keychain, sign in again while the keychain is unusable so the new
         secret lands in the file, then log out while it is still unusable. The file now carries its
         own secret and the first login's entry is still there, so reading the file as proof of a
-        clean keychain reports a logout that did not happen."""
+        clean keychain reports a logout that did not happen.
+
+        The three unusable states are the whole of what an erase can answer besides erased and
+        stranded; a backend that keeps nothing it is given is something only a write finds out."""
         _write_legacy_file(isolated_home)
         vault = secret_vault_factory(available=False, failure=failure)
 
@@ -705,6 +729,30 @@ 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(
+        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."""
+        path = _write_legacy_file(isolated_home)
+        path.chmod(0o400)
+        monkeypatch.setattr(
+            "litellm.litellm_core_utils.private_json.tempfile.mkstemp",
+            _MkstempThatNeedsTheOldFileGone(path),
+        )
+        vault = secret_vault_factory(available=False, failure=KeyringUnreachable())
+
+        assert clear_cli_token(vault=vault) == KeyringUnreachable()
+        assert clear_cli_token(vault=vault) == KeyringUnreachable()
+
+        assert json.loads(path.read_text()).get("key") 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