diff --git a/aai_cli/AGENTS.md b/aai_cli/AGENTS.md index 50303b8..2476d5d 100644 --- a/aai_cli/AGENTS.md +++ b/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. diff --git a/aai_cli/core/config.py b/aai_cli/core/config.py index cf5312a..b5a1c1c 100644 --- a/aai_cli/core/config.py +++ b/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,43 +200,9 @@ 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: @@ -247,36 +210,19 @@ def set_api_key(profile: str, api_key: str) -> None: _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) diff --git a/aai_cli/core/keyring_store.py b/aai_cli/core/keyring_store.py new file mode 100644 index 0000000..015e028 --- /dev/null +++ b/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 diff --git a/tests/test_config.py b/tests/test_config.py index 388ba76..e6ceb88 100644 --- a/tests/test_config.py +++ b/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): diff --git a/tests/test_config_session.py b/tests/test_config_session.py index 004ab4c..5bdb85b 100644 --- a/tests/test_config_session.py +++ b/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