diff --git a/aai_cli/AGENTS.md b/aai_cli/AGENTS.md index e58e8a18..e5314fbe 100644 --- a/aai_cli/AGENTS.md +++ b/aai_cli/AGENTS.md @@ -160,3 +160,12 @@ heavily-reworked commands with long bodies; small commands keep the inline - **`init/`** — scaffolds a self-contained FastAPI + HTML starter (`audio-transcription`/`live-captions`/`voice-agent` templates), optionally installs deps and opens the browser; writes the key to a git-ignored `.env`. - **`core/telemetry.py`** — anonymous, opt-out usage telemetry (Supabase-CLI model): `context.run_command` wraps each command body in `telemetry.track(ctx.command_path)`, which dispatches one allow-listed event (command path, outcome/exit code, duration, version/OS, and on failure the error message capped at 500 chars — never args or account data) to the Datadog logs intake via a **detached flusher subprocess** (the hidden `assembly telemetry flush`), so commands never wait on telemetry. `SHIPPED_CLIENT_TOKEN` is a committed write-only Datadog *client* token (`pub…`, embeddable by design — never an API key; `AAI_TELEMETRY_CLIENT_TOKEN` overrides). The test suite blanks it via an autouse conftest fixture so no test ever spawns a real flusher. Opt-out: `AAI_TELEMETRY_DISABLED=1` / `DO_NOT_TRACK=1` / `assembly telemetry disable` (persisted as `telemetry_enabled` in config.toml, alongside the random `device_id`). Send-side failures are swallowed (`OSError`/`CLIError`) — telemetry must never break a command. - **`commands/setup.py`** + **`app/setup_exec.py`** — `assembly setup install/status/remove` wires a coding agent up to AssemblyAI by installing three artifacts: the `assemblyai-docs` docs MCP (via `claude mcp add`), the AssemblyAI skill (via `npx skills add`), and the bundled `aai-cli` skill (copied out of the wheel, no network). Missing `claude`/`npx` is reported and skipped, not an error. The step implementations live in `aai_cli/app/setup_exec.py` and the presence probes (docs MCP registered, skills on disk) in `aai_cli/app/coding_agent.py`, so `assembly doctor` (via `app/doctor_checks.py`) and the onboarding wizard share them without command modules importing each other. + +### Long keyring values + +Windows limits each generic credential to 2560 UTF-16 bytes. `keyring_store` +keeps small values in the existing entry and stores large values as base64 chunks +in a separate keyring service. A versioned manifest is published only after all +chunks are written; replacement and deletion clean up the old generation. +Missing or corrupt chunks read as no stored secret. Keep the same wrapper for +rollback so restoring a large session also respects the credential limit. diff --git a/aai_cli/core/keyring_store.py b/aai_cli/core/keyring_store.py index 015e0284..1c790969 100644 --- a/aai_cli/core/keyring_store.py +++ b/aai_cli/core/keyring_store.py @@ -13,7 +13,10 @@ from __future__ import annotations +import base64 import contextlib +import re +import uuid import keyring import keyring.errors # keyring.errors is not re-exported by keyring/__init__ @@ -31,7 +34,7 @@ def set_secret(username: str, secret: str) -> None: it as a CLIError so the command prints a fixable message instead of a traceback. """ try: - keyring.set_password(KEYRING_SERVICE, username, secret) + _write_secret(username, secret) except keyring.errors.KeyringError as exc: raise CLIError( f"Your OS keyring rejected the write ({exc}).", @@ -52,7 +55,7 @@ def get_secret(username: str) -> str | None: signed in" — ASSEMBLYAI_API_KEY still works there — never as a crash. """ try: - return keyring.get_password(KEYRING_SERVICE, username) + return _read_secret(username) except keyring.errors.KeyringError: return None @@ -65,9 +68,9 @@ def restore_secret(username: str, prior: str | None) -> None: """ with contextlib.suppress(keyring.errors.KeyringError): if prior is None: - keyring.delete_password(KEYRING_SERVICE, username) + _delete_secret(username) else: - keyring.set_password(KEYRING_SERVICE, username, prior) + _write_secret(username, prior) def delete_secret(username: str) -> None: @@ -77,7 +80,7 @@ def delete_secret(username: str) -> None: boxes) delete raises NoKeyringError, and "nothing stored" is already the goal. """ with contextlib.suppress(keyring.errors.KeyringError): - keyring.delete_password(KEYRING_SERVICE, username) + _delete_secret(username) def usable() -> bool: @@ -93,3 +96,72 @@ def usable() -> bool: except keyring.errors.KeyringError: return False return True + + +# Windows generic credentials permit 2560 UTF-16 bytes. ASCII chunks stay below +# that limit, including when the original secret contains non-BMP characters. +_CHUNK_SIZE = 1000 +_MAX_CREDENTIAL_BYTES = 2560 +_MANIFEST = re.compile(r"assemblyai-chunks-v1:([0-9a-f]{32}):([1-9][0-9]{0,5})") +_CHUNK_SERVICE = KEYRING_SERVICE + "-chunks-v1" + + +def _chunk_names(raw: str | None) -> list[str]: + match = _MANIFEST.fullmatch(raw or "") + if not match: + return [] + generation, count = match.groups() + return [f"{generation}:{index}" for index in range(int(count))] + + +def _remove_chunks(names: list[str]) -> None: + for name in names: + with contextlib.suppress(keyring.errors.KeyringError): + keyring.delete_password(_CHUNK_SERVICE, name) + + +def _write_secret(username: str, secret: str) -> None: + old = keyring.get_password(KEYRING_SERVICE, username) + names: list[str] = [] + try: + if len(secret.encode("utf-16-le")) <= _MAX_CREDENTIAL_BYTES and not _MANIFEST.fullmatch( + secret + ): + raw = secret + else: + encoded = base64.b64encode(secret.encode()).decode("ascii") + generation = uuid.uuid4().hex + for offset in range(0, len(encoded), _CHUNK_SIZE): + name = f"{generation}:{len(names)}" + names.append(name) + keyring.set_password(_CHUNK_SERVICE, name, encoded[offset : offset + _CHUNK_SIZE]) + raw = f"assemblyai-chunks-v1:{generation}:{len(names)}" + # Publish only after all chunks exist; a failed write leaves the old + # generation readable. Each generation has independent chunk names. + keyring.set_password(KEYRING_SERVICE, username, raw) + except Exception: + _remove_chunks(names) + raise + _remove_chunks(_chunk_names(old)) + + +def _read_secret(username: str) -> str | None: + raw = keyring.get_password(KEYRING_SERVICE, username) + names = _chunk_names(raw) + if not names: + return raw + chunks = [keyring.get_password(_CHUNK_SERVICE, name) for name in names] + if any(chunk is None for chunk in chunks): + return None + try: + return base64.b64decode( + "".join(chunk for chunk in chunks if chunk is not None), validate=True + ).decode() + except (ValueError, UnicodeError): + return None + + +def _delete_secret(username: str) -> None: + raw = keyring.get_password(KEYRING_SERVICE, username) + keyring.delete_password(KEYRING_SERVICE, username) + _remove_chunks(_chunk_names(raw)) diff --git a/tests/test_keyring_large.py b/tests/test_keyring_large.py new file mode 100644 index 00000000..951b660f --- /dev/null +++ b/tests/test_keyring_large.py @@ -0,0 +1,81 @@ +from __future__ import annotations + +import keyring +import keyring.errors +import pytest + +from aai_cli.core import keyring_store + + +def test_large_session_fits_windows_credential_limit(monkeypatch): + original = keyring.set_password + + def limited(service: str, username: str, password: str) -> None: + if len(password.encode("utf-16-le")) > 2560: + raise ValueError("CredWrite: credential exceeds Windows limit") + original(service, username, password) + + monkeypatch.setattr(keyring, "set_password", limited) + secret = "fake-session-😀" * 400 + keyring_store.set_secret("session:default", secret) + assert keyring_store.get_secret("session:default") == secret + keyring_store.delete_secret("session:default") + assert keyring_store.get_secret("session:default") is None + + +def test_failed_large_write_preserves_old_secret(monkeypatch): + keyring_store.set_secret("session:default", "old") + original = keyring.set_password + calls = 0 + + def fail_second(service: str, username: str, password: str) -> None: + nonlocal calls + calls += 1 + if calls == 2: + raise keyring.errors.PasswordSetError("locked") + original(service, username, password) + + monkeypatch.setattr(keyring, "set_password", fail_second) + with pytest.raises(keyring_store.CLIError): + keyring_store.set_secret("session:default", "x" * 4000) + assert keyring_store.get_secret("session:default") == "old" + + +@pytest.mark.parametrize("replacement", ["small", "y" * 5000]) +def test_overwrite_removes_old_chunks(replacement): + keyring_store.set_secret("session:default", "x" * 4000) + raw = keyring.get_password(keyring_store.KEYRING_SERVICE, "session:default") + old_names = keyring_store._chunk_names(raw) + assert old_names + keyring_store.set_secret("session:default", replacement) + assert keyring_store.get_secret("session:default") == replacement + assert all(keyring.get_password(keyring_store._CHUNK_SERVICE, n) is None for n in old_names) + + +@pytest.mark.parametrize("corrupt", [None, "!invalid-base64!", "/w=="]) +def test_missing_or_corrupt_chunk_is_not_a_session(corrupt): + keyring_store.set_secret("session:default", "x" * 4000) + raw = keyring.get_password(keyring_store.KEYRING_SERVICE, "session:default") + name = keyring_store._chunk_names(raw)[0] + if corrupt is None: + keyring.delete_password(keyring_store._CHUNK_SERVICE, name) + else: + keyring.set_password(keyring_store._CHUNK_SERVICE, name, corrupt) + assert keyring_store.get_secret("session:default") is None + + +def test_restore_and_delete_large_secret(): + secret = "z" * 4000 + keyring_store.restore_secret("session:default", secret) + assert keyring_store.get_secret("session:default") == secret + raw = keyring.get_password(keyring_store.KEYRING_SERVICE, "session:default") + names = keyring_store._chunk_names(raw) + keyring_store.restore_secret("session:default", None) + assert keyring_store.get_secret("session:default") is None + assert all(keyring.get_password(keyring_store._CHUNK_SERVICE, n) is None for n in names) + + +def test_manifest_shaped_secret_round_trips(): + secret = "assemblyai-chunks-v1:" + "a" * 32 + ":1" + keyring_store.set_secret("default", secret) + assert keyring_store.get_secret("default") == secret