Extract the OS keyring into core/keyring_store

config.py sat at exactly the 500-line gate with no headroom. Split the OS
keyring access — the one external secret dependency — into a new
core/keyring_store.py: KEYRING_SERVICE plus set_secret/get_secret/
restore_secret/delete_secret/usable, the single importer of `keyring`.

config.py (500 -> 442) now reads and writes secrets through that wrapper and
no longer imports keyring directly, so the documented "secrets live in the
keyring, not the dotfile" boundary is structural. config.keyring_usable stays
as a thin delegator so the auth-state facade (doctor/login and their tests) is
unchanged. The two clear_* paths collapse onto the shared delete_secret.

Tests that reached into config's keyring internals repointed to keyring_store
(config.keyring -> keyring_store.keyring, config.KEYRING_SERVICE ->
keyring_store.KEYRING_SERVICE). No behavior change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018Bkx51XDcYBdnot1EF8h9U

Author: claude

aai_cli/AGENTS.md

@@ -31,9 +31,9 @@ between layers is enforced — higher may import lower, never the reverse:
 - **`ui/`** — Rich rendering: `output`, `render`, `theme`, `steps`, `follow`,
   `help_text`, `typer_patches`, `update_check`.
 - **`core/`** — the Rich-free library layer: `client`, `config`,
-  `config_builder`, `environments`, `env`, `errors`, `llm`, `telemetry`,
-  `debuglog`, `remotefs`, `sync_stt`, `hotkey`, `ws`, `youtube`, `wer`,
-  `argscan`, `jsonshape`, `timeparse`, `microphone`, `procs`, `stdio`,
+  `config_builder`, `keyring_store`, `environments`, `env`, `errors`, `llm`,
+  `telemetry`, `debuglog`, `remotefs`, `sync_stt`, `hotkey`, `ws`, `youtube`,
+  `wer`, `argscan`, `jsonshape`, `timeparse`, `microphone`, `procs`, `stdio`,
   `choices`. Contract 4 also forbids `rich` here, so "no Rich below the UI
   layer" is structural.
 
@@ -139,7 +139,7 @@ heavily-reworked commands with long bodies; small commands keep the inline
 ### Cross-cutting state (resolution order matters)
 
 - **`app/context.py`** — `AppState` (profile, env) is attached to the Typer context in the root `@app.callback()`. `run_command` is the standard command wrapper.
-- **`core/config.py`** — profiles persisted in `config.toml` (via `platformdirs`); the **API key lives only in the OS keyring** (`KEYRING_SERVICE = "assemblyai-cli"`), never in a dotfile. Key resolution order: `--api-key` flag (validation paths only) → `ASSEMBLYAI_API_KEY` env → keyring. **Run commands deliberately expose no `--api-key` flag** so keys can't leak into `ps`/shell history.
+- **`core/config.py`** — profiles persisted in `config.toml` (via `platformdirs`); the **API key lives only in the OS keyring**, never in a dotfile. The keyring access itself is factored into **`core/keyring_store.py`** (the single importer of `keyring`, holding `KEYRING_SERVICE = "assemblyai-cli"` + `set_secret`/`get_secret`/`restore_secret`/`delete_secret`/`usable`), so the "secrets never touch the dotfile" split is structural; `config` reads/writes secrets through it and only `config.keyring_usable` re-surfaces the probe on the auth facade. Key resolution order: `--api-key` flag (validation paths only) → `ASSEMBLYAI_API_KEY` env → keyring. **Run commands deliberately expose no `--api-key` flag** so keys can't leak into `ps`/shell history.
 - **`core/environments.py`** — a frozen `Environment` (api_base, streaming_host, llm_gateway_base, ams_base, stytch_*). `DEFAULT_ENV` is **`production`**; use `--sandbox` (or `--env sandbox000` / `AAI_ENV`) to target the sandbox. The active environment is a process-global set once at startup; precedence: `--env` → `AAI_ENV` → profile's stored env → default. A credential is only valid against the environment that minted it.
 - **`core/client.py`** — thin wrappers over the `assemblyai` SDK (`transcribe`, `list_transcripts`, `stream_audio`, etc.). It normalizes SDK exceptions: auth failures become a single clean `auth_failure()` `CLIError`; everything else becomes `APIError`. New SDK calls should follow this try/except shape.
 - **`core/errors.py`** — the `CLIError` hierarchy (each with `error_type` + `exit_code`). `ui/output.py` emits errors to **stderr**; stdout stays clean for pipelines. `--json` switches to machine-readable output; it is never auto-enabled — `output.resolve_json()` deliberately keeps human text the default even when piped or agent-run.

aai_cli/core/config.py

@@ -8,16 +8,13 @@
 import uuid
 from pathlib import Path
 
-import keyring
-import keyring.errors  # keyring.errors is not re-exported by keyring/__init__
 import platformdirs
 import tomli_w
 from pydantic import BaseModel, ConfigDict, Field, ValidationError
 
-from aai_cli.core import debuglog, env
+from aai_cli.core import debuglog, env, keyring_store
 from aai_cli.core.errors import CLIError, NotAuthenticated
 
-KEYRING_SERVICE = "assemblyai-cli"
 ENV_API_KEY = "ASSEMBLYAI_API_KEY"
 DEFAULT_PROFILE = "default"
 
@@ -203,80 +200,29 @@ def set_active_profile(name: str) -> None:
     _dump(cfg)
 
 
-def _keyring_set(username: str, secret: str) -> None:
-    """Write a secret to the OS keyring, turning backend failures into a clean error.
-
-    A locked keychain, or an existing entry whose ACL is bound to another app, makes
-    keyring raise a KeyringError (e.g. macOS errSecInvalidOwnerEdit, -25244). Surface
-    it as a CLIError so the command prints a fixable message instead of a traceback.
-    """
-    try:
-        keyring.set_password(KEYRING_SERVICE, username, secret)
-    except keyring.errors.KeyringError as exc:
-        raise CLIError(
-            f"Your OS keyring rejected the write ({exc}).",
-            error_type="keyring_error",
-            suggestion=(
-                "Unlock your keyring, or remove the stale 'assemblyai-cli' entry and "
-                "retry (macOS: security delete-generic-password -s assemblyai-cli). "
-                "On a headless machine without a keyring, set ASSEMBLYAI_API_KEY instead."
-            ),
-        ) from exc
-
-
-def _keyring_restore(username: str, prior: str | None) -> None:
-    """Best-effort restore of a keyring entry to a snapshot value, for login rollback.
-
-    Suppresses keyring errors (including a delete of an absent entry) so a failed
-    rollback never masks the original write error that triggered it.
-    """
-    with contextlib.suppress(keyring.errors.KeyringError):
-        if prior is None:
-            keyring.delete_password(KEYRING_SERVICE, username)
-        else:
-            keyring.set_password(KEYRING_SERVICE, username, prior)
-
-
 def set_api_key(profile: str, api_key: str) -> None:
     validate_profile(profile)
-    _keyring_set(profile, api_key)
+    keyring_store.set_secret(profile, api_key)
     cfg = _load()
     cfg.profiles.setdefault(profile, Profile())
     if cfg.active_profile is None:
         cfg.active_profile = profile
     _dump(cfg)
 
 
-def _keyring_get(username: str) -> str | None:
-    """Read a secret, treating an unusable keyring backend as "nothing stored".
-
-    Headless machines (containers, CI, servers) routinely have no keyring backend at
-    all, so keyring raises NoKeyringError on every read. That state must read as "not
-    signed in" — ASSEMBLYAI_API_KEY still works there — never as a crash.
-    """
-    try:
-        return keyring.get_password(KEYRING_SERVICE, username)
-    except keyring.errors.KeyringError:
-        return None
-
-
 def get_api_key(profile: str) -> str | None:
-    return _keyring_get(profile)
+    return keyring_store.get_secret(profile)
 
 
 def keyring_usable() -> bool:
-    """True when the OS keyring backend can be read.
+    """True when the OS keyring backend can be read (delegates to ``keyring_store``).
 
-    Headless boxes (containers, CI, bare SSH) often have no keyring backend, so
-    ``keyring`` raises on every access. ``assembly doctor`` uses this to tell a user with
-    no key that the *backend* is the problem — and to recommend ASSEMBLYAI_API_KEY —
-    rather than pointing at `assembly login`, whose browser flow also can't persist there.
+    Kept on ``config`` as part of the auth-state facade: ``assembly doctor``/`login`
+    call it to tell a user with no key that the *backend* is the problem — and to
+    recommend ASSEMBLYAI_API_KEY — rather than pointing at the browser flow, which
+    also can't persist on a keyring-less box.
     """
-    try:
-        keyring.get_password(KEYRING_SERVICE, "__probe__")
-    except keyring.errors.KeyringError:
-        return False
-    return True
+    return keyring_store.usable()
 
 
 def get_profile_env(profile: str) -> str | None:
@@ -308,10 +254,7 @@ def set_profile_email(profile: str, email: str) -> None:
 
 
 def clear_api_key(profile: str) -> None:
-    # KeyringError, not just PasswordDeleteError: with no backend at all (headless
-    # boxes) delete raises NoKeyringError, and "nothing stored" is already the goal.
-    with contextlib.suppress(keyring.errors.KeyringError):
-        keyring.delete_password(KEYRING_SERVICE, profile)
+    keyring_store.delete_secret(profile)
 
 
 SESSION_KEYRING_PREFIX = "session"  # keyring username: f"{prefix}:{profile}"
@@ -328,7 +271,7 @@ def set_session(profile: str, *, session_jwt: str, session_token: str, account_i
     key. The JWT is short-lived; an expired session surfaces as NotAuthenticated.
     """
     validate_profile(profile)
-    _keyring_set(
+    keyring_store.set_secret(
         _session_username(profile),
         StoredSession(jwt=session_jwt, token=session_token).model_dump_json(),
     )
@@ -339,7 +282,7 @@ def set_session(profile: str, *, session_jwt: str, session_token: str, account_i
 
 def get_session(profile: str) -> dict[str, str] | None:
     """The stored {'jwt', 'token'} for a profile, or None if absent/corrupt."""
-    raw = _keyring_get(_session_username(profile))
+    raw = keyring_store.get_secret(_session_username(profile))
     if not raw:
         return None
     try:
@@ -356,8 +299,7 @@ def get_account_id(profile: str) -> int | None:
 
 
 def clear_session(profile: str) -> None:
-    with contextlib.suppress(keyring.errors.KeyringError):
-        keyring.delete_password(KEYRING_SERVICE, _session_username(profile))
+    keyring_store.delete_secret(_session_username(profile))
     cfg = _load()
     prof = cfg.profiles.get(profile)
     if prof and prof.account_id is not None:
@@ -385,8 +327,8 @@ def persist_login(
     restored best-effort.
     """
     validate_profile(profile)
-    prior_api_key = _keyring_get(profile)
-    prior_session = _keyring_get(_session_username(profile))
+    prior_api_key = keyring_store.get_secret(profile)
+    prior_session = keyring_store.get_secret(_session_username(profile))
     prior_cfg = _load()
     done = False
     try:
@@ -404,8 +346,8 @@ def persist_login(
         done = True
     finally:
         if not done:
-            _keyring_restore(profile, prior_api_key)
-            _keyring_restore(_session_username(profile), prior_session)
+            keyring_store.restore_secret(profile, prior_api_key)
+            keyring_store.restore_secret(_session_username(profile), prior_session)
             _dump(prior_cfg)
 
 

aai_cli/core/keyring_store.py

@@ -0,0 +1,95 @@
+"""The OS keyring as the CLI's secret store.
+
+config.toml holds only non-secret profile settings; every secret — the API key and
+the browser-login session blob — lives in the OS keyring instead, keyed under
+``KEYRING_SERVICE``. This module is the single place that imports ``keyring``, so the
+"secrets never touch the dotfile" boundary is structural: ``config`` reads and writes
+them through this typed wrapper and never opens the keyring directly.
+
+Every backend call is wrapped because a keyring is routinely unusable — a locked
+keychain, an ACL-bound entry, or (on headless boxes) no backend at all — and those
+must surface as a clean ``CLIError`` or read as "nothing stored", never a traceback.
+"""
+
+from __future__ import annotations
+
+import contextlib
+
+import keyring
+import keyring.errors  # keyring.errors is not re-exported by keyring/__init__
+
+from aai_cli.core.errors import CLIError
+
+KEYRING_SERVICE = "assemblyai-cli"
+
+
+def set_secret(username: str, secret: str) -> None:
+    """Write a secret to the OS keyring, turning backend failures into a clean error.
+
+    A locked keychain, or an existing entry whose ACL is bound to another app, makes
+    keyring raise a KeyringError (e.g. macOS errSecInvalidOwnerEdit, -25244). Surface
+    it as a CLIError so the command prints a fixable message instead of a traceback.
+    """
+    try:
+        keyring.set_password(KEYRING_SERVICE, username, secret)
+    except keyring.errors.KeyringError as exc:
+        raise CLIError(
+            f"Your OS keyring rejected the write ({exc}).",
+            error_type="keyring_error",
+            suggestion=(
+                "Unlock your keyring, or remove the stale 'assemblyai-cli' entry and "
+                "retry (macOS: security delete-generic-password -s assemblyai-cli). "
+                "On a headless machine without a keyring, set ASSEMBLYAI_API_KEY instead."
+            ),
+        ) from exc
+
+
+def get_secret(username: str) -> str | None:
+    """Read a secret, treating an unusable keyring backend as "nothing stored".
+
+    Headless machines (containers, CI, servers) routinely have no keyring backend at
+    all, so keyring raises NoKeyringError on every read. That state must read as "not
+    signed in" — ASSEMBLYAI_API_KEY still works there — never as a crash.
+    """
+    try:
+        return keyring.get_password(KEYRING_SERVICE, username)
+    except keyring.errors.KeyringError:
+        return None
+
+
+def restore_secret(username: str, prior: str | None) -> None:
+    """Best-effort restore of a keyring entry to a snapshot value, for login rollback.
+
+    Suppresses keyring errors (including a delete of an absent entry) so a failed
+    rollback never masks the original write error that triggered it.
+    """
+    with contextlib.suppress(keyring.errors.KeyringError):
+        if prior is None:
+            keyring.delete_password(KEYRING_SERVICE, username)
+        else:
+            keyring.set_password(KEYRING_SERVICE, username, prior)
+
+
+def delete_secret(username: str) -> None:
+    """Delete a keyring entry, treating an absent entry or missing backend as success.
+
+    KeyringError, not just PasswordDeleteError: with no backend at all (headless
+    boxes) delete raises NoKeyringError, and "nothing stored" is already the goal.
+    """
+    with contextlib.suppress(keyring.errors.KeyringError):
+        keyring.delete_password(KEYRING_SERVICE, username)
+
+
+def usable() -> bool:
+    """True when the OS keyring backend can be read.
+
+    Headless boxes (containers, CI, bare SSH) often have no keyring backend, so
+    ``keyring`` raises on every access. ``assembly doctor`` uses this to tell a user with
+    no key that the *backend* is the problem — and to recommend ASSEMBLYAI_API_KEY —
+    rather than pointing at `assembly login`, whose browser flow also can't persist there.
+    """
+    try:
+        keyring.get_password(KEYRING_SERVICE, "__probe__")
+    except keyring.errors.KeyringError:
+        return False
+    return True

tests/test_config.py

@@ -2,7 +2,7 @@
 
 import pytest
 
-from aai_cli.core import config
+from aai_cli.core import config, keyring_store
 from aai_cli.core.errors import CLIError, NotAuthenticated
 
 
@@ -29,8 +29,8 @@ def test_resolve_falls_back_to_keyring():
 
 
 def test_get_session_rejects_non_string_jwt():
-    config.keyring.set_password(
-        config.KEYRING_SERVICE,
+    keyring_store.keyring.set_password(
+        keyring_store.KEYRING_SERVICE,
         config._session_username("default"),
         json.dumps({"jwt": 123, "token": "tok"}),
     )
@@ -66,7 +66,7 @@ def test_set_api_key_keyring_failure_raises_clean_error(monkeypatch):
     def boom(*_a, **_k):
         raise keyring.errors.PasswordSetError("denied by keychain")
 
-    monkeypatch.setattr(config.keyring, "set_password", boom)
+    monkeypatch.setattr(keyring_store.keyring, "set_password", boom)
     with pytest.raises(CLIError) as exc:
         config.set_api_key("default", "sk_abc")
     assert "keyring" in exc.value.message.lower()
@@ -84,7 +84,7 @@ def test_keyring_usable_false_when_backend_raises(monkeypatch):
     def boom(*_a, **_k):
         raise keyring.errors.NoKeyringError("no backend on this headless box")
 
-    monkeypatch.setattr(config.keyring, "get_password", boom)
+    monkeypatch.setattr(keyring_store.keyring, "get_password", boom)
     assert config.keyring_usable() is False
 
 
@@ -94,7 +94,7 @@ def test_set_session_keyring_failure_raises_clean_error(monkeypatch):
     def boom(*_a, **_k):
         raise keyring.errors.PasswordSetError("denied by keychain")
 
-    monkeypatch.setattr(config.keyring, "set_password", boom)
+    monkeypatch.setattr(keyring_store.keyring, "set_password", boom)
     with pytest.raises(CLIError) as exc:
         config.set_session("default", session_jwt="j", session_token="t", account_id=1)
     assert "keyring" in exc.value.message.lower()
@@ -123,14 +123,14 @@ def _fail_on_session_write(monkeypatch):
     """
     import keyring.errors
 
-    real_set = config.keyring.set_password
+    real_set = keyring_store.keyring.set_password
 
     def selective(service, username, secret):
         if username.startswith(config.SESSION_KEYRING_PREFIX + ":"):
             raise keyring.errors.PasswordSetError("denied by keychain")
         return real_set(service, username, secret)
 
-    monkeypatch.setattr(config.keyring, "set_password", selective)
+    monkeypatch.setattr(keyring_store.keyring, "set_password", selective)
 
 
 def test_persist_login_rolls_back_to_empty_when_session_write_fails(monkeypatch):

tests/test_config_session.py

@@ -1,4 +1,4 @@
-from aai_cli.core import config
+from aai_cli.core import config, keyring_store
 
 
 def test_set_and_get_session_roundtrips():
@@ -26,5 +26,5 @@ def test_clear_session_is_safe_when_absent():
 def test_get_session_returns_none_for_corrupt_blob():
     import keyring
 
-    keyring.set_password(config.KEYRING_SERVICE, "session:default", "not-json")
+    keyring.set_password(keyring_store.KEYRING_SERVICE, "session:default", "not-json")
     assert config.get_session("default") is None