fix(cli): report a token file logout cannot remove instead of crashing

A ~/.litellm that has gone read-only, or one left root-owned by a sudo login,
refuses both the scrubbed rewrite and the removal. The removal was unguarded,
so 'lite logout' ended in a PermissionError traceback with the credential still
readable in the file. It now comes back as an outcome the command reports,
naming the file and what to do about it, and a file that holds no secret is
still not worth alarming anyone over.

litellm/litellm_core_utils/cli_token_utils.py

@@ -63,8 +63,22 @@ class CredentialNotRecorded:
     """
 
 
+@dataclass(frozen=True, slots=True)
+class CredentialNotCleared:
+    """The token file still holds the secret, because it could not be removed or rewritten.
+
+    Logging out of the keychain is only half of it. A `~/.litellm` that refuses both the scrubbed
+    rewrite and the removal leaves the credential readable on disk, which is the one thing a logout
+    is for, so it is reported instead of being counted as a clean sweep.
+    """
+
+    detail: str
+
+
 SecretSave: TypeAlias = SecretWrite | CredentialNotSaved | CredentialNotRecorded
 
+SecretClear: TypeAlias = SecretErase | CredentialNotCleared
+
 
 class CliTokenRecord(BaseModel):
     """A stored CLI credential.
@@ -150,23 +164,35 @@ def _keep_the_secret_in_the_file(record: CliTokenRecord, outcome: SecretWrite) -
     return outcome
 
 
-def clear_cli_token(*, vault: SecretVault = SYSTEM_KEYRING) -> SecretErase:
+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.
+    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.
     """
     outcome: Final = vault.erase()
     record: Final = _read_token_file()
     settled: Final = _nothing_left_behind(outcome, record)
-    if settled or not _keep_the_unchecked_keychain_on_record(outcome, record):
-        Path(get_cli_token_file_path()).unlink(missing_ok=True)
+    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 record.key is not None:
+        return removal
     return SecretErased() if settled else outcome
 
 
+def _remove_token_file() -> CredentialNotCleared | None:
+    try:
+        Path(get_cli_token_file_path()).unlink(missing_ok=True)
+    except OSError as error:
+        return CredentialNotCleared(str(error))
+    return None
+
+
 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.
 

litellm/proxy/client/cli/commands/auth.py

@@ -27,6 +27,7 @@ from litellm.litellm_core_utils.cli_keyring import (
 )
 from litellm.litellm_core_utils.cli_token_utils import (
     CliTokenRecord,
+    CredentialNotCleared,
     CredentialNotRecorded,
     CredentialNotSaved,
     SecretSave,
@@ -796,9 +797,13 @@ def login(ctx: click.Context, config_claude: bool):
 @click.pass_context
 def logout(ctx: click.Context):
     """Logout and clear stored authentication"""
+    path: Final = get_cli_token_file_path()
     match clear_cli_token(vault=context_secret_vault(ctx)):
         case SecretErased():
             click.echo("Logged out successfully. Authentication token cleared.")
+        case CredentialNotCleared(detail=detail):
+            click.echo(f"Your credential is still in {path}, which could not be removed: {detail}.")
+            click.echo("Delete that file, or make the directory writable and run 'lite logout' again.")
         case SecretStranded():
             click.echo(STRANDED_CREDENTIAL_MESSAGE)
             click.echo("Unlock your keychain and run 'lite logout' again to clear it.")

tests/test_litellm/litellm_core_utils/test_cli_token_utils.py

@@ -27,6 +27,7 @@ from litellm.litellm_core_utils.cli_keyring import (
 from litellm.litellm_core_utils.cli_token_utils import (
     CliTokenRecord,
     CredentialNotRecorded,
+    CredentialNotCleared,
     CredentialNotSaved,
     clear_cli_token,
     get_cli_token_file_path,
@@ -81,6 +82,26 @@ def _blob(base_url=SERVER, key="sk-vault", jwt_token=""):
     return json.dumps({"base_url": base_url, "key": key, "jwt_token": jwt_token})
 
 
+_REAL_REPLACE = os.replace
+
+
+def _refuse_replace(*args, **kwargs):
+    raise OSError("device or resource busy")
+
+
+class _ReplaceThatStartsRefusing:
+    """`os.replace` standing in for a path that cannot be replaced yet: a file another process holds
+    open on Windows, a directory that went read-only between staging and the rewrite."""
+
+    def __init__(self):
+        self.allowed = False
+
+    def __call__(self, src, dst):
+        if not self.allowed:
+            raise OSError("device or resource busy")
+        _REAL_REPLACE(src, dst)
+
+
 class TestGetCliTokenFilePath:
     def test_points_at_the_home_config_file(self, isolated_home):
         assert get_cli_token_file_path() == str(isolated_home / ".litellm" / "token.json")
@@ -459,6 +480,43 @@ class TestScrubFailure:
         assert json.loads(path.read_text())["key"] == "sk-legacy"
         assert list(path.parent.glob(".tmp-*")) == []
 
+    def test_a_rewrite_that_fails_after_the_keychain_took_the_secret_hands_it_back(
+        self, isolated_home, secret_vault_factory, monkeypatch
+    ):
+        """Staging can succeed and the rewrite still fail afterwards, which is the one window where
+        both stores hold the credential. The keychain copy goes back, so the file is left exactly as
+        it was found and the move can be tried again."""
+        path = _write_legacy_file(isolated_home)
+        vault = secret_vault_factory()
+        monkeypatch.setattr("litellm.litellm_core_utils.private_json.os.replace", _refuse_replace)
+
+        record = load_cli_token(vault=vault)
+
+        assert record.key == "sk-legacy"
+        assert vault.blob is None
+        assert json.loads(path.read_text())["key"] == "sk-legacy"
+        assert list(path.parent.glob(".tmp-*")) == []
+
+    def test_a_rollback_the_keychain_refuses_is_finished_by_the_next_read(
+        self, isolated_home, secret_vault_factory, monkeypatch
+    ):
+        """A keychain that will not give back what it just took leaves the credential in both stores.
+        Nothing is lost by that, and nothing is abandoned either: the next read carries the move the
+        rest of the way, so the duplicate outlives only the condition that caused it."""
+        path = _write_legacy_file(isolated_home)
+        vault = secret_vault_factory(erasable=False)
+        replace = _ReplaceThatStartsRefusing()
+        monkeypatch.setattr("litellm.litellm_core_utils.private_json.os.replace", replace)
+
+        assert load_cli_token(vault=vault).key == "sk-legacy"
+        assert vault.blob is not None
+        assert json.loads(path.read_text())["key"] == "sk-legacy"
+
+        replace.allowed = True
+
+        assert load_cli_token(vault=vault).key == "sk-legacy"
+        assert json.loads(path.read_text()).get("key") is None
+
 
 class TestClearCliToken:
     def test_removes_the_credential_from_both_stores(self, isolated_home, secret_vault_factory):
@@ -599,6 +657,39 @@ class TestClearCliToken:
         assert vault.blob is not None
         assert "sk-in-file" not in _token_file(isolated_home).read_text()
 
+    @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions")
+    def test_a_file_that_can_be_neither_scrubbed_nor_removed_is_reported_not_raised(
+        self, isolated_home, secret_vault_factory
+    ):
+        """A `~/.litellm` gone read-only, or one left root-owned by a `sudo lite login`, refuses the
+        scrubbed rewrite and the removal alike. The credential is still readable on disk, which is
+        the one thing logging out is for, so it has to come back as an answer rather than as a
+        traceback the user has to read the code to understand."""
+        path = _write_legacy_file(isolated_home)
+        path.parent.chmod(0o500)
+        try:
+            outcome = clear_cli_token(vault=secret_vault_factory())
+        finally:
+            path.parent.chmod(0o700)
+
+        assert isinstance(outcome, CredentialNotCleared)
+        assert json.loads(path.read_text())["key"] == "sk-legacy"
+
+    @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
+    ):
+        """The secret was in the keychain and the keychain gave it up. What is stuck on disk names a
+        credential that no longer exists, so the logout it describes really did happen."""
+        path = _write_metadata_only_file(isolated_home)
+        path.parent.chmod(0o500)
+        try:
+            outcome = clear_cli_token(vault=secret_vault_factory(blob=_blob()))
+        finally:
+            path.parent.chmod(0o700)
+
+        assert outcome == SecretErased()
+
     def test_is_safe_when_nothing_was_ever_stored(self, isolated_home, secret_vault_factory):
         assert clear_cli_token(vault=secret_vault_factory()) == SecretErased()
 

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

@@ -492,6 +492,23 @@ class TestLogoutCommand:
         assert "still in the OS keychain" in result.output
         assert "Unlock your keychain" in result.output
 
+    @pytest.mark.skipif(os.geteuid() == 0, reason="root ignores directory permissions")
+    def test_logout_reports_a_token_file_it_cannot_remove(self, isolated_home, secret_vault_factory):
+        """`lite logout` on a read-only ~/.litellm used to end in a PermissionError traceback with
+        the credential still sitting in the file. The user has to be told what is left and where."""
+        _write_token_file(isolated_home, key="sk-in-file")
+        config_dir = isolated_home / ".litellm"
+        config_dir.chmod(0o500)
+        try:
+            result = self.runner.invoke(logout, obj={"secret_vault": secret_vault_factory()})
+        finally:
+            config_dir.chmod(0o700)
+
+        assert result.exit_code == 0
+        assert "Logged out successfully" not in result.output
+        assert "still in" in result.output
+        assert str(config_dir / "token.json") in result.output
+
     def test_logout_without_the_keyring_package_still_warns_about_a_file_held_secret(
         self, isolated_home, secret_vault_factory
     ):