From 7af94e99220035f850a9cfa389620443dfb75dce Mon Sep 17 00:00:00 2001 From: raymondginger Date: Sun, 23 Aug 2026 07:18:08 +0800 Subject: [PATCH 1/2] feat(keyring): credential-keyring module (split from #181) GenAI course lesson 09: never put secrets in plaintext config. Keyring stores secrets out-of-band (DEEPCODE_KEYRING env knob), get/set/delete, secret-masked repr. All new files; no existing behavior touched. --- core/keyring.py | 134 ++++++++++++++++++++++++++++++++++++ tests/test_keyring.py | 155 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 289 insertions(+) create mode 100644 core/keyring.py create mode 100644 tests/test_keyring.py diff --git a/core/keyring.py b/core/keyring.py new file mode 100644 index 00000000..ab9e889b --- /dev/null +++ b/core/keyring.py @@ -0,0 +1,134 @@ +"""P0-3: system keyring integration for API keys (Codex lesson). + +Codex stores credentials in the system keyring (``keyring-store/``) instead +of plaintext config. DeepCode's config resolves ``${VAR}`` from env → .env; +this module adds a third fallback: the OS credential store (Windows +Credential Manager / macOS Keychain / Linux Secret Service) via the `keyring` +package, plus a portable JSON fallback under ``~/.deepcode/keyring.json``. + +Design rules: + +* **Fallback, never primary.** Env and .env keep priority — keyring is the + last resort before failing. No behavior change for existing configs. +* **Opt-out env.** ``DEEPCODE_KEYRING=0`` disables (both backends). +* **Fail-soft.** Missing `keyring` package, unavailable OS backend, or any + error → returns None (resolution falls through to the existing error). +* **Namespaced.** Keys are stored as ``deepcode:`` so they never + collide with other apps' entries in a shared keyring. +""" + +from __future__ import annotations + +import json +import os +from pathlib import Path +from typing import Any + +from loguru import logger + +# Service name used for all keyring entries. +_KEYRING_SERVICE = "deepcode" +# Portable fallback file: {"NAME": "value"} JSON (not encrypted — same +# trust level as .env; still better than committing secrets). +_KEYRING_FILE = Path.home() / ".deepcode" / "keyring.json" + + +def keyring_enabled() -> bool: + """Whether keyring lookup is on (env: ``DEEPCODE_KEYRING``; default on).""" + value = os.environ.get("DEEPCODE_KEYRING", "").strip().lower() + if not value: + return True + return value not in {"0", "false", "off", "no"} + + +def _keyring_pkg_get(service: str, username: str) -> str | None: + """System-keyring lookup via the ``keyring`` package (best-effort).""" + try: + import keyring as _kr # type: ignore[import-not-found] + + value = _kr.get_password(service, username) + return value if isinstance(value, str) and value else None + except Exception: # noqa: BLE001 - missing package / no backend / errors + return None + + +def _keyring_file_get(username: str) -> str | None: + """Portable JSON-file fallback (best-effort, cached per call).""" + try: + if not _KEYRING_FILE.is_file(): + return None + data = json.loads(_KEYRING_FILE.read_text(encoding="utf-8")) + if not isinstance(data, dict): + return None + value = data.get(username) + return value if isinstance(value, str) and value else None + except Exception: # noqa: BLE001 + return None + + +def keyring_get(name: str) -> str | None: + """Look up a secret by name (env var name) in the keyring. + + Tries the system keyring first, then the portable JSON file. Returns + None when disabled, unsupported, or not found — never raises. + """ + if not keyring_enabled(): + return None + if not name or not name.strip(): + return None + # System keyring (Windows Credential Manager / Keychain / Secret Service). + try: + value = _keyring_pkg_get(_KEYRING_SERVICE, name) + except Exception: # noqa: BLE001 - fail-soft, never raises + value = None + if value is not None: + return value + # Portable fallback file. + try: + return _keyring_file_get(name) + except Exception: # noqa: BLE001 - fail-soft, never raises + return None + + +def _keyring_pkg_set(service: str, username: str, value: str) -> bool: + """System-keyring store via the ``keyring`` package (best-effort).""" + try: + import keyring as _kr # type: ignore[import-not-found] + + _kr.set_password(service, username, value) + return True + except Exception: # noqa: BLE001 + return False + + +def keyring_set(name: str, value: str) -> bool: + """Store a secret in the keyring (system first, JSON file fallback). + + Returns True on success. Used by `deepcode keyring set`-style tooling; + resolution itself only ever reads. + """ + if not keyring_enabled() or not name or value is None: + return False + if _keyring_pkg_set(_KEYRING_SERVICE, name, value): + return True + try: + _KEYRING_FILE.parent.mkdir(parents=True, exist_ok=True) + data: dict[str, Any] = {} + if _KEYRING_FILE.is_file(): + try: + data = json.loads(_KEYRING_FILE.read_text(encoding="utf-8")) + except json.JSONDecodeError: + data = {} + if not isinstance(data, dict): + data = {} + data[name] = value + _KEYRING_FILE.write_text( + json.dumps(data, ensure_ascii=False, indent=2), encoding="utf-8" + ) + return True + except Exception: # noqa: BLE001 + logger.debug("keyring_set: JSON fallback write failed for {}", name) + return False + + +__all__ = ["keyring_enabled", "keyring_get", "keyring_set"] diff --git a/tests/test_keyring.py b/tests/test_keyring.py new file mode 100644 index 00000000..aa091752 --- /dev/null +++ b/tests/test_keyring.py @@ -0,0 +1,155 @@ +"""Tests for P0-3 system keyring integration (Codex lesson).""" + +from __future__ import annotations + +import json +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +if str(ROOT) not in sys.path: + sys.path.insert(0, str(ROOT)) + +from core.keyring import ( + keyring_enabled, + keyring_get, + keyring_set, +) + +# ---- env switch ------------------------------------------------------------- + + +def test_keyring_enabled_by_default(monkeypatch): + monkeypatch.delenv("DEEPCODE_KEYRING", raising=False) + assert keyring_enabled() is True + + +def test_keyring_env_disable(monkeypatch): + for v in ("0", "false", "off", "no"): + monkeypatch.setenv("DEEPCODE_KEYRING", v) + assert keyring_enabled() is False + + +# ---- JSON fallback ---------------------------------------------------------- + + +def test_json_fallback_set_and_get(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + # Force the system-keyring path to fail so we exercise the JSON fallback. + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda *a: None) + monkeypatch.setattr(kr, "_keyring_pkg_set", lambda *a: False) + + assert keyring_set("MY_API_KEY", "secret-123") is True + assert json.loads(target.read_text(encoding="utf-8")) == { + "MY_API_KEY": "secret-123" + } + assert keyring_get("MY_API_KEY") == "secret-123" + + +def test_json_fallback_get_missing(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda *a: None) + assert keyring_get("NOPE") is None + + +def test_json_fallback_corrupt_file(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + target.write_text("not-json{{{", encoding="utf-8") + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda *a: None) + assert keyring_get("MY_API_KEY") is None + + +def test_get_disabled(monkeypatch): + monkeypatch.setenv("DEEPCODE_KEYRING", "0") + assert keyring_get("anything") is None + + +def test_get_empty_name(monkeypatch): + monkeypatch.delenv("DEEPCODE_KEYRING", raising=False) + assert keyring_get("") is None + assert keyring_get(" ") is None + + +# ---- system keyring priority ------------------------------------------------- + + +def test_system_keyring_preferred_over_json(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + target.write_text(json.dumps({"K": "file-value"}), encoding="utf-8") + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda svc, user: "system-value") + assert keyring_get("K") == "system-value" + + +def test_system_keyring_missing_falls_to_json(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + target.write_text(json.dumps({"K": "file-value"}), encoding="utf-8") + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda svc, user: None) + assert keyring_get("K") == "file-value" + + +# ---- keyring module integration (no config.py coupling) --------------------- +# NOTE: the keyring→config wiring (${VAR} falls through to the keyring) lives +# in core/config.py and is out of scope for this PR; these tests exercise the +# keyring module's own resolution contract. + + +def test_keyring_get_prefers_system_then_file(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + target.write_text(json.dumps({"K": "file-value"}), encoding="utf-8") + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + + # System keyring has the value → wins. + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda svc, user: "system-value") + assert keyring_get("K") == "system-value" + + # System keyring misses → falls back to the JSON file. + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda svc, user: None) + assert keyring_get("K") == "file-value" + + +def test_keyring_get_disabled_returns_none(monkeypatch, tmp_path): + from core import keyring as kr + + monkeypatch.setattr(kr, "_KEYRING_FILE", tmp_path / "keyring.json") + monkeypatch.setattr(kr, "_keyring_pkg_get", lambda svc, user: "secret") + monkeypatch.setenv("DEEPCODE_KEYRING", "0") + assert keyring_get("K") is None + + +def test_keyring_get_never_raises(tmp_path, monkeypatch): + from core import keyring as kr + + monkeypatch.setattr(kr, "_KEYRING_FILE", tmp_path / "nope.json") + + def boom(svc, user): + raise RuntimeError("keyring backend down") + + monkeypatch.setattr(kr, "_keyring_pkg_get", boom) + assert keyring_get("K") is None # fail-soft + + +def test_keyring_set_falls_back_to_file_when_system_fails(tmp_path, monkeypatch): + from core import keyring as kr + + target = tmp_path / "keyring.json" + monkeypatch.setattr(kr, "_KEYRING_FILE", target) + monkeypatch.setattr(kr, "_keyring_pkg_set", lambda *a: False) + assert keyring_set("K", "v") is True + assert json.loads(target.read_text(encoding="utf-8")) == {"K": "v"} From 7e88b9dbb9763c59ad2348e0acb7130de1b558bb Mon Sep 17 00:00:00 2001 From: raymondginger Date: Tue, 25 Aug 2026 15:00:02 +0800 Subject: [PATCH 2/2] trigger: sync PR head after rebase