v2.17.0 (#822)

Co-authored-by: Clément Sirieix <clement.sirieix@mistral.ai>
Co-authored-by: Guillaume LE GOFF <guillaume.lgf@gmail.com>
Co-authored-by: Hdandria <henri.dandria@mistral.ai>
Co-authored-by: Ivana Dunisijevic <ivana.dunisijevic@mistral.ai>
Co-authored-by: Jean Burellier <sheplu@users.noreply.github.com>
Co-authored-by: Mathias Gesbert <mathias.gesbert@mistral.ai>
Co-authored-by: Mert Unsal <mert.unsal@mistral.ai>
Co-authored-by: Michel Thomazo <51709227+michelTho@users.noreply.github.com>
Co-authored-by: Paul VEZIA <166131032+le-codeur-rapide@users.noreply.github.com>
Co-authored-by: Pierre Rossinès <pierre.rossines@mistral.ai>
Co-authored-by: Val <102326092+vdeva@users.noreply.github.com>
Co-authored-by: Vincent G <10739306+VinceOPS@users.noreply.github.com>
Co-authored-by: renovate-mistral[bot] <253709520+renovate-mistral[bot]@users.noreply.github.com>
Co-authored-by: Mistral Vibe <vibe@mistral.ai>

tests/setup/auth/test_api_key_persistence.py

@@ -0,0 +1,148 @@
+from __future__ import annotations
+
+import os
+
+from dotenv import dotenv_values, set_key
+import keyring
+from keyring.errors import KeyringError, NoKeyringError, PasswordDeleteError
+import pytest
+
+from vibe.core.config import ProviderConfig
+from vibe.core.paths import GLOBAL_ENV_FILE
+from vibe.core.types import Backend
+from vibe.setup.auth.api_key_persistence import persist_api_key, remove_api_key
+
+
+def _provider(*, api_key_env_var: str = "CUSTOM_API_KEY") -> ProviderConfig:
+    # Backend.GENERIC keeps onboarding telemetry out of these unit tests.
+    return ProviderConfig(
+        name="custom",
+        api_base="https://custom.example/v1",
+        api_key_env_var=api_key_env_var,
+        backend=Backend.GENERIC,
+    )
+
+
+def test_persist_stores_in_keyring_and_clears_stale_env(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    stored: dict[str, str] = {}
+    monkeypatch.delenv("CUSTOM_API_KEY", raising=False)
+    monkeypatch.setattr(
+        keyring,
+        "set_password",
+        lambda service, username, password: stored.__setitem__(username, password),
+    )
+    # A stale plaintext copy that should be dropped after the keyring write.
+    GLOBAL_ENV_FILE.path.parent.mkdir(parents=True, exist_ok=True)
+    set_key(GLOBAL_ENV_FILE.path, "CUSTOM_API_KEY", "old-key")
+
+    result = persist_api_key(_provider(), "new-key")
+
+    assert result == "completed"
+    assert stored == {"CUSTOM_API_KEY": "new-key"}
+    assert os.environ["CUSTOM_API_KEY"] == "new-key"
+    assert "CUSTOM_API_KEY" not in dotenv_values(GLOBAL_ENV_FILE.path)
+
+
+def test_persist_falls_back_to_env_when_keyring_unavailable(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.delenv("CUSTOM_API_KEY", raising=False)
+
+    def _unavailable(service: str, username: str, password: str) -> None:
+        raise KeyringError("no keyring")
+
+    monkeypatch.setattr(keyring, "set_password", _unavailable)
+
+    result = persist_api_key(_provider(), "new-key")
+
+    assert result == "completed"
+    assert os.environ["CUSTOM_API_KEY"] == "new-key"
+    assert dotenv_values(GLOBAL_ENV_FILE.path)["CUSTOM_API_KEY"] == "new-key"
+
+
+def test_persist_returns_env_var_error_for_empty_env_var(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    def _fail(service: str, username: str, password: str) -> None:
+        raise AssertionError("keyring should not be used for an empty env var")
+
+    monkeypatch.setattr(keyring, "set_password", _fail)
+
+    result = persist_api_key(_provider(api_key_env_var=""), "new-key")
+
+    assert result == "env_var_error:<empty>"
+
+
+def test_remove_deletes_keyring_env_and_process_env(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    deleted: list[str] = []
+    monkeypatch.setenv("CUSTOM_API_KEY", "live-key")
+    monkeypatch.setattr(
+        keyring, "delete_password", lambda service, username: deleted.append(username)
+    )
+    GLOBAL_ENV_FILE.path.parent.mkdir(parents=True, exist_ok=True)
+    set_key(GLOBAL_ENV_FILE.path, "CUSTOM_API_KEY", "file-key")
+
+    remove_api_key(_provider())
+
+    assert deleted == ["CUSTOM_API_KEY"]
+    assert "CUSTOM_API_KEY" not in dotenv_values(GLOBAL_ENV_FILE.path)
+    assert "CUSTOM_API_KEY" not in os.environ
+
+
+def test_remove_ignores_keyring_unavailable_and_still_clears_env(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("CUSTOM_API_KEY", "live-key")
+
+    def _unavailable(service: str, username: str) -> None:
+        raise NoKeyringError("no keyring")
+
+    monkeypatch.setattr(keyring, "delete_password", _unavailable)
+    GLOBAL_ENV_FILE.path.parent.mkdir(parents=True, exist_ok=True)
+    set_key(GLOBAL_ENV_FILE.path, "CUSTOM_API_KEY", "file-key")
+
+    remove_api_key(_provider())
+
+    assert "CUSTOM_API_KEY" not in dotenv_values(GLOBAL_ENV_FILE.path)
+    assert "CUSTOM_API_KEY" not in os.environ
+
+
+def test_remove_ignores_missing_keyring_entry_and_still_clears_env(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("CUSTOM_API_KEY", "live-key")
+
+    def _missing(service: str, username: str) -> None:
+        raise PasswordDeleteError("not found")
+
+    monkeypatch.setattr(keyring, "delete_password", _missing)
+    GLOBAL_ENV_FILE.path.parent.mkdir(parents=True, exist_ok=True)
+    set_key(GLOBAL_ENV_FILE.path, "CUSTOM_API_KEY", "file-key")
+
+    remove_api_key(_provider())
+
+    assert "CUSTOM_API_KEY" not in dotenv_values(GLOBAL_ENV_FILE.path)
+    assert "CUSTOM_API_KEY" not in os.environ
+
+
+def test_remove_surfaces_keyring_operation_error_but_still_clears_env(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    monkeypatch.setenv("CUSTOM_API_KEY", "live-key")
+
+    def _failed(service: str, username: str) -> None:
+        raise KeyringError("delete failed")
+
+    monkeypatch.setattr(keyring, "delete_password", _failed)
+    GLOBAL_ENV_FILE.path.parent.mkdir(parents=True, exist_ok=True)
+    set_key(GLOBAL_ENV_FILE.path, "CUSTOM_API_KEY", "file-key")
+
+    with pytest.raises(KeyringError, match="delete failed"):
+        remove_api_key(_provider())
+
+    assert "CUSTOM_API_KEY" not in dotenv_values(GLOBAL_ENV_FILE.path)
+    assert "CUSTOM_API_KEY" not in os.environ

tests/setup/auth/test_auth_state.py

@@ -2,11 +2,19 @@ from __future__ import annotations
 
 from pathlib import Path
 
+import keyring
+import pytest
+
 from vibe.core.config import DEFAULT_MISTRAL_API_ENV_KEY, ProviderConfig
 from vibe.core.types import Backend
 from vibe.setup.auth import AuthState, AuthStateKind, assess_auth_state
 
 
+@pytest.fixture(autouse=True)
+def disable_keyring(monkeypatch: pytest.MonkeyPatch) -> None:
+    monkeypatch.setattr(keyring, "get_password", lambda service, username: None)
+
+
 def _mistral_provider(
     *, api_key_env_var: str = DEFAULT_MISTRAL_API_ENV_KEY
 ) -> ProviderConfig:
@@ -100,7 +108,7 @@ def test_assess_process_env_when_default_key_is_only_in_process_env(
     )
 
 
-def test_assess_vibe_home_env_file_overrides_process_env_when_both_sources_exist(
+def test_assess_process_env_when_process_env_and_dotenv_both_exist(
     tmp_path: Path,
 ) -> None:
     env_path = tmp_path / ".env"
@@ -114,9 +122,9 @@ def test_assess_vibe_home_env_file_overrides_process_env_when_both_sources_exist
     )
 
     assert state == AuthState(
-        kind=AuthStateKind.VIBE_HOME_ENV_FILE_OVERRIDES_PROCESS_ENV,
+        kind=AuthStateKind.PROCESS_ENV,
         can_use_active_provider=True,
-        sign_out_available=True,
+        sign_out_available=False,
         env_key=DEFAULT_MISTRAL_API_ENV_KEY,
     )
 
@@ -214,3 +222,62 @@ def test_assess_empty_dotenv_value_as_signed_out(tmp_path: Path) -> None:
         sign_out_available=False,
         env_key=DEFAULT_MISTRAL_API_ENV_KEY,
     )
+
+
+def test_assess_os_keyring_when_default_key_is_in_keyring(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(
+        keyring, "get_password", lambda service, username: "keyring-key"
+    )
+
+    state = assess_auth_state(
+        _mistral_provider(), env_path=tmp_path / ".env", environ={}
+    )
+
+    assert state == AuthState(
+        kind=AuthStateKind.OS_KEYRING,
+        can_use_active_provider=True,
+        sign_out_available=True,
+        env_key=DEFAULT_MISTRAL_API_ENV_KEY,
+    )
+
+
+def test_assess_vibe_home_env_file_when_dotenv_and_keyring_both_have_value(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    # resolve_api_key reads the .env-injected os.environ value before the keyring,
+    # so an overlap must be reported as the .env file, not OS keyring.
+    monkeypatch.setattr(
+        keyring, "get_password", lambda service, username: "keyring-key"
+    )
+    env_path = tmp_path / ".env"
+    _write_env_file(env_path, f"{DEFAULT_MISTRAL_API_ENV_KEY}=file-key\n")
+
+    state = assess_auth_state(_mistral_provider(), env_path=env_path, environ={})
+
+    assert state == AuthState(
+        kind=AuthStateKind.VIBE_HOME_ENV_FILE,
+        can_use_active_provider=True,
+        sign_out_available=True,
+        env_key=DEFAULT_MISTRAL_API_ENV_KEY,
+    )
+
+
+def test_assess_unsupported_provider_when_custom_key_is_in_keyring(
+    tmp_path: Path, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    monkeypatch.setattr(
+        keyring, "get_password", lambda service, username: "keyring-key"
+    )
+
+    state = assess_auth_state(
+        _generic_provider(), env_path=tmp_path / ".env", environ={}
+    )
+
+    assert state == AuthState(
+        kind=AuthStateKind.UNSUPPORTED_PROVIDER,
+        can_use_active_provider=True,
+        sign_out_available=False,
+        env_key="CUSTOM_API_KEY",
+    )