diff --git a/litellm/litellm_core_utils/cli_token_utils.py b/litellm/litellm_core_utils/cli_token_utils.py index 825967e6866..15b1390f337 100644 --- a/litellm/litellm_core_utils/cli_token_utils.py +++ b/litellm/litellm_core_utils/cli_token_utils.py @@ -45,14 +45,25 @@ from litellm.litellm_core_utils.private_json import ( @dataclass(frozen=True, slots=True) class CredentialNotSaved: - """The credential was minted but no store would keep it, so this machine has none.""" + """The credential was minted but no store would keep it, so this machine has none. + + Nothing was touched on the way to this, so a login that already worked still does. + """ detail: str -SecretSave: TypeAlias = SecretWrite | CredentialNotSaved +@dataclass(frozen=True, slots=True) +class CredentialNotRecorded: + """The keychain took the credential, but the file that names it could not be replaced. -_UNREPLACEABLE_FILE: Final = "the staged file could not replace the one already there" + The keychain holds one entry, so the secret that was there is already gone and no rollback + brings it back. Removing the new one as well would only turn a login this machine may still + be able to use into no login at all, so it stays, and the user is told what is where. + """ + + +SecretSave: TypeAlias = SecretWrite | CredentialNotSaved | CredentialNotRecorded class CliTokenRecord(BaseModel): @@ -111,6 +122,10 @@ def save_cli_token(record: CliTokenRecord, *, vault: SecretVault = SYSTEM_KEYRIN half that a read-only or full directory refuses, so it is staged before the keychain is handed anything. A save that cannot land then leaves both stores exactly as it found them, which matters most when the login it failed to replace is still perfectly good. + + Staging can still succeed and the replacement fail afterwards. That is the one case where the + 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)) if isinstance(staged, CredentialNotSaved): @@ -121,7 +136,7 @@ def save_cli_token(record: CliTokenRecord, *, vault: SecretVault = SYSTEM_KEYRIN else vault.write(_encode_secret(record.base_url, record.key, record.jwt_token)) ) if isinstance(outcome, SecretStored): - return outcome if _commit_token_file(staged) else CredentialNotSaved(_UNREPLACEABLE_FILE) + return outcome if _commit_token_file(staged) else CredentialNotRecorded() discard_staged_json(staged) return _keep_the_secret_in_the_file(record, outcome) diff --git a/litellm/proxy/client/cli/commands/auth.py b/litellm/proxy/client/cli/commands/auth.py index d89641d2366..eb956cd536c 100644 --- a/litellm/proxy/client/cli/commands/auth.py +++ b/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, + CredentialNotRecorded, CredentialNotSaved, SecretSave, clear_cli_token, @@ -126,6 +127,12 @@ def storage_notice(outcome: SecretSave) -> str: "Any login you already had is untouched. Run 'lite login' again once that path is " "writable, or 'lite logout' to clear whatever is stored now." ) + case CredentialNotRecorded(): + return ( + f"Signed in, and the credential is in your OS keychain, but {path} could not be " + "replaced, so this machine may still be using your previous login. Run 'lite login' " + "again once that path is writable, or 'lite logout' to clear both." + ) def keychain_unreadable_notice(vault: SecretVault) -> str: @@ -754,7 +761,7 @@ def login(ctx: click.Context, config_claude: bool): click.echo("\nLogin successful!") click.echo(f"JWT Token: {api_key[:20]}...") click.echo(storage_notice(stored)) - if isinstance(stored, CredentialNotSaved): + if isinstance(stored, (CredentialNotSaved, CredentialNotRecorded)): return click.echo("You can now use the CLI without specifying --api-key") 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 69ce47b25b3..0cf1e55d363 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 @@ -26,6 +26,7 @@ from litellm.litellm_core_utils.cli_keyring import ( ) from litellm.litellm_core_utils.cli_token_utils import ( CliTokenRecord, + CredentialNotRecorded, CredentialNotSaved, clear_cli_token, get_cli_token_file_path, @@ -368,6 +369,37 @@ class TestSaveCliToken: assert json.loads(vault.blob)["key"] == "sk-in-use" assert load_cli_token(vault=vault).key == "sk-in-use" + def test_a_keychain_write_the_file_cannot_be_pointed_at_is_reported_as_that( + self, isolated_home, secret_vault_factory + ): + """Staging the file can succeed and the replacement still fail, and that is the one path + where the keychain already took the new secret. Reporting it as a save that kept nothing + would send the user looking for a credential that is sitting in their keychain.""" + vault = secret_vault_factory() + path = _token_file(isolated_home) + path.parent.mkdir(parents=True, exist_ok=True) + path.mkdir() + + outcome = save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-new"), vault=vault) + + assert isinstance(outcome, CredentialNotRecorded) + assert json.loads(vault.blob)["key"] == "sk-new" + + def test_the_credential_the_file_cannot_name_is_left_in_the_keychain( + self, isolated_home, secret_vault_factory + ): + """The keychain holds one entry, so the secret that was there went the moment this one + landed. Taking the new one back out would turn a login this machine may still be able to + use into no login at all, and it cannot restore the old one either way.""" + vault = secret_vault_factory(blob=_blob(key="sk-in-use")) + path = _token_file(isolated_home) + path.parent.mkdir(parents=True, exist_ok=True) + path.mkdir() + + save_cli_token(CliTokenRecord(base_url=SERVER, key="sk-new"), vault=vault) + + assert vault.blob is not None + def test_a_failed_write_leaves_the_previous_credential_intact(self, isolated_home, secret_vault_factory, monkeypatch): path = _write_legacy_file(isolated_home) before = path.read_text()