diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 085448f8b63..da8b80ab598 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -12,6 +12,7 @@ first time it reads one. This module has no dependencies on proxy code and can be safely imported at the SDK level. """ +import math import time from dataclasses import dataclass from pathlib import Path @@ -144,14 +145,29 @@ 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. """ - staged: Final = _stage_token_file(_without_secret(record)) + stamped: Final = _stamped_past_the_login_on_disk(record) + staged: Final = _stage_token_file(_without_secret(stamped)) if isinstance(staged, CredentialNotSaved): return staged - outcome: Final = SecretStored() if record.key is None else vault.write(_encode_secret(record, record.key)) + outcome: Final = SecretStored() if stamped.key is None else vault.write(_encode_secret(stamped, stamped.key)) if isinstance(outcome, SecretStored): return outcome if _commit_token_file(staged) else CredentialNotRecorded() discard_staged_json(staged) - return _keep_the_secret_in_the_file(record, outcome) + return _keep_the_secret_in_the_file(stamped, outcome) + + +def _stamped_past_the_login_on_disk(record: CliTokenRecord) -> CliTokenRecord: + """Keep a sign-in's stamp ahead of the one it replaces, whatever the clock did in between. + + The stamp is what decides a keychain secret against one still on disk, so a clock that stepped + backwards between two logins would hand the older of them the win and put a superseded + credential back in use. The file already names the login being replaced, and pinning the new + stamp just past it costs one read that changes nothing on a clock that only moves forwards. + """ + previous: Final = _read_token_file() + if previous is None or previous.timestamp < record.timestamp: + return record + return record.model_copy(update=MappingProxyType({"timestamp": math.nextafter(previous.timestamp, math.inf)})) def _keep_the_secret_in_the_file(record: CliTokenRecord, outcome: SecretWrite) -> SecretSave: @@ -331,7 +347,9 @@ def _apply_vault_secret(record: CliTokenRecord, blob: str, vault: SecretVault) - The sign-in each secret came from decides it, because either store can be the stale one. A secret is usually left on disk by a keychain that would not take it, which makes the file the fresher of the two. It is the older one when a login the keychain did take could not replace - the file afterwards, and serving that one would put a superseded credential back in use. + the file afterwards, and serving that one would put a superseded credential back in use. Equal + stamps are one login sitting in both stores, left by a migration whose scrub was refused, so + that branch retries the migration rather than trading one credential for another. A scrub the file refuses leaves that superseded secret where it lies, which is the state the login already named when it could not replace the file, and which `lite logout` reports rather diff --git a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py index 4fc835d2db1..d9fc17f908f 100644 --- a/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py +++ b/tests/test_litellm/litellm_core_utils/test_cli_token_utils.py @@ -488,6 +488,46 @@ class TestSaveCliToken: assert path.read_text() == before assert list(path.parent.glob(".tmp-*")) == [] + def test_a_login_is_stamped_past_the_one_it_replaces_even_on_a_clock_that_went_back( + self, isolated_home, secret_vault_factory + ): + """The stamp is what decides the keychain secret against the one on disk, so a login that + carries an earlier wall clock than the login before it must not be filed as the older of + the two.""" + _write_legacy_file(isolated_home, key="sk-old", timestamp=2000.0) + vault = secret_vault_factory() + + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-new", timestamp=1000.0), vault=vault) + + assert json.loads(vault.blob)["timestamp"] > 2000.0 + + def test_a_clock_that_went_back_does_not_hand_the_win_to_the_superseded_login( + self, isolated_home, secret_vault_factory + ): + """The disk state a login reports as CredentialNotRecorded: the keychain took the new + secret and the file still holds the previous one. Reading it back has to produce the login + that was just made, and an earlier wall clock is no reason to serve the one it replaced.""" + _write_legacy_file(isolated_home, key="sk-superseded", timestamp=2000.0) + vault = secret_vault_factory() + + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-fresh", timestamp=1000.0), vault=vault) + _write_legacy_file(isolated_home, key="sk-superseded", timestamp=2000.0) + + assert load_cli_token(vault=vault).key == "sk-fresh" + + def test_a_login_on_a_clock_that_moved_forwards_keeps_its_own_time( + self, isolated_home, secret_vault_factory + ): + """Pinning the stamp above the previous login is only ever a floor. The ordinary case has + to record when the user actually signed in, because that is what decides expiry.""" + _write_legacy_file(isolated_home, key="sk-old", timestamp=1000.0) + vault = secret_vault_factory() + + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-new", timestamp=2000.0), vault=vault) + + assert json.loads(vault.blob)["timestamp"] == 2000.0 + assert json.loads(_token_file(isolated_home).read_text())["timestamp"] == 2000.0 + class TestScrubFailure: """A keychain that took the secret while the file kept it is the worst of both stores: the