From fb69fcf765997610e7f32fc041200cd5b5217e3a Mon Sep 17 00:00:00 2001 From: mateo-berri <277851410+mateo-berri@users.noreply.github.com> Date: Thu, 20 Aug 2026 05:07:21 -0700 Subject: [PATCH] fix(cli): stamp each sign-in past the keychain as well as the file A login the keychain took but the token file could not record leaves the keychain naming a later sign-in than the file does. Reading only the file then stamps the next login below that keychain entry, and a clock that went back far enough puts the superseded credential back in use. --- litellm/litellm_core_utils/cli_token_utils.py | 40 +++++++++++++++---- .../test_cli_token_utils.py | 13 ++++++ 2 files changed, 45 insertions(+), 8 deletions(-) diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index da8b80ab598..c0e57a737e4 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -145,7 +145,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. """ - stamped: Final = _stamped_past_the_login_on_disk(record) + stamped: Final = _stamped_past_every_stored_login(record, vault) staged: Final = _stage_token_file(_without_secret(stamped)) if isinstance(staged, CredentialNotSaved): return staged @@ -156,18 +156,42 @@ def save_cli_token(record: CliTokenRecord, *, vault: SecretVault = SYSTEM_KEYRIN 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. +def _stamped_past_every_stored_login(record: CliTokenRecord, vault: SecretVault) -> CliTokenRecord: + """Keep a sign-in's stamp ahead of every login already stored, 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. + credential back in use. Pinning the new stamp just past the highest one either store holds costs + one read each and changes nothing on a clock that only moves forwards. + """ + highest: Final = _highest_stamp_already_stored(record.base_url, vault) + if highest < record.timestamp: + return record + return record.model_copy(update=MappingProxyType({"timestamp": math.nextafter(highest, math.inf)})) + + +def _highest_stamp_already_stored(base_url: str, vault: SecretVault) -> float: + """When the latest login either store still holds was made, or minus infinity when neither has one. + + Both are asked because the file names the login being replaced only while the two agree. A login + the keychain took but the file could not record afterwards leaves the keychain holding the later + of the two, and reading only the file would stamp the next sign-in below it. """ 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)})) + secret: Final = _stored_secret(base_url, vault) + return max( + -math.inf if previous is None else previous.timestamp, + -math.inf if secret is None else secret.timestamp, + ) + + +def _stored_secret(base_url: str, vault: SecretVault) -> CliTokenSecret | None: + """The keychain's secret for this server, when it holds one this login may be compared against""" + match vault.read(): + case SecretFound(blob=blob): + return _decode_secret(blob, base_url) + case SecretMissing() | KeyringNotInstalled() | KeyringDisabled() | KeyringUnreachable(): + return None def _keep_the_secret_in_the_file(record: CliTokenRecord, outcome: SecretWrite) -> SecretSave: 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 d9fc17f908f..e78add61dd5 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 @@ -528,6 +528,19 @@ class TestSaveCliToken: assert json.loads(vault.blob)["timestamp"] == 2000.0 assert json.loads(_token_file(isolated_home).read_text())["timestamp"] == 2000.0 + def test_a_login_is_stamped_past_the_keychain_the_file_could_not_keep_up_with( + self, isolated_home, secret_vault_factory + ): + """A login reported as CredentialNotRecorded leaves the keychain holding a later sign-in + than the file names, so the file alone is no longer the floor. A later login on a clock + that went back past that keychain entry still has to be the one served.""" + _write_legacy_file(isolated_home, key="sk-superseded", timestamp=1000.0) + vault = secret_vault_factory(blob=_blob(key="sk-recorded", timestamp=2000.0), writable=False) + + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-fresh", timestamp=1500.0), vault=vault) + + assert load_cli_token(vault=vault).key == "sk-fresh" + class TestScrubFailure: """A keychain that took the secret while the file kept it is the worst of both stores: the