From adf8074383477b34fb608bcf43464f00d3c3dab7 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Fri, 3 Jul 2026 11:03:25 -0400
Subject: [PATCH 01/16] feat(secret-store): add generic OS keyring secret store
 to OSS core

Introduce code_puppy/secret_store.py, a generic secrets manager that
reads/writes via the OS keyring with a permission-hardened JSON file
fallback for headless and CI environments.

Public API: get_secret / set_secret / delete_secret / keyring_available.

- keyring is a hard dependency (plain import, no ImportError guard); the
  packages we ship always include it. Runtime backend absence (headless/
  CI) is detected via keyring_available() and degrades to the fallback.
- Fallback writes 0o600 JSON atomically (temp + fchmod + os.replace),
  repairs loose permissions on read, and warns once per process that
  fallback storage is active.
- Ships no secret values and holds no product-specific key names or
  migration logic. Migrating specific secrets (puppy_token, API keys)
  onto this store is deferred to follow-up work, one category per PR.

Adds keyring>=24.0.0 to pyproject. 20 unit tests cover the three paths:
keyring available, keyring missing, and fallback file behavior.
---
 code_puppy/secret_store.py | 216 +++++++++++++++++++++++++++++++++++++
 pyproject.toml             |   1 +
 tests/test_secret_store.py | 216 +++++++++++++++++++++++++++++++++++++
 uv.lock                    | 105 ++++++++++++++++++
 4 files changed, 538 insertions(+)
 create mode 100644 code_puppy/secret_store.py
 create mode 100644 tests/test_secret_store.py

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
new file mode 100644
index 000000000..ff16af185
--- /dev/null
+++ b/code_puppy/secret_store.py
@@ -0,0 +1,216 @@
+"""Generic OS keyring secret store for Code Puppy.
+
+Reads and writes secrets through the operating system keyring, with a
+permission-hardened JSON file fallback for headless and CI environments
+where no keyring backend is available.
+
+Public API
+----------
+``keyring_available()``
+    Report whether a usable keyring backend is configured.
+``get_secret(name)`` / ``set_secret(name, value)`` / ``delete_secret(name)``
+    Keyring-first secret operations that transparently fall back to the
+    ``0o600`` JSON file when the keyring backend is unavailable.
+"""
+
+from __future__ import annotations
+
+import json
+import os
+import tempfile
+import warnings
+
+import keyring
+
+from code_puppy.config import CONFIG_DIR
+
+# Namespace under which every secret is stored in the OS keyring. Downstream
+# distributions should use a distinct service name so secrets never bleed
+# across builds.
+_SERVICE_NAME = "code-puppy"
+
+# Permission-hardened JSON fallback used only when the keyring backend is
+# unavailable (headless boxes, minimal CI containers, etc.).
+_FALLBACK_FILE = os.path.join(CONFIG_DIR, "secrets.json")
+_FALLBACK_MODE = 0o600
+
+# Emit the "fallback storage is active" warning at most once per process so
+# we do not spam the console on every read/write.
+_warned_fallback = False
+
+
+# ---------------------------------------------------------------------------
+# Keyring availability + low-level access
+# ---------------------------------------------------------------------------
+
+
+def keyring_available() -> bool:
+    """Return True when a usable keyring backend is configured.
+
+    A backend with ``priority <= 0`` (for example the fail/null backend on
+    a headless Linux box) is treated as unavailable, so callers degrade to
+    the file fallback instead of writing into a black hole.
+    """
+    try:
+        backend = keyring.get_keyring()
+    except Exception:
+        return False
+    priority = getattr(backend, "priority", None)
+    if priority is None:
+        return True
+    try:
+        return float(priority) > 0
+    except Exception:
+        return True
+
+
+def _keyring_get(name: str) -> str | None:
+    try:
+        value = keyring.get_password(_SERVICE_NAME, name)
+    except Exception:
+        return None
+    if value is None:
+        return None
+    normalized = str(value).strip()
+    return normalized or None
+
+
+def _keyring_set(name: str, value: str) -> bool:
+    normalized = str(value).strip()
+    if not normalized:
+        return False
+    try:
+        keyring.set_password(_SERVICE_NAME, name, normalized)
+    except Exception:
+        return False
+    return True
+
+
+def _keyring_delete(name: str) -> bool:
+    try:
+        keyring.delete_password(_SERVICE_NAME, name)
+    except Exception:
+        return False
+    return True
+
+
+# ---------------------------------------------------------------------------
+# Permission-hardened JSON file fallback (secure I/O helper)
+# ---------------------------------------------------------------------------
+
+
+def _warn_fallback_active() -> None:
+    global _warned_fallback
+    if _warned_fallback:
+        return
+    _warned_fallback = True
+    warnings.warn(
+        "No OS keyring backend is available; secrets are stored in the "
+        f"permission-hardened fallback file at {_FALLBACK_FILE} "
+        "(mode 0o600). This is intended for headless/CI use only.",
+        stacklevel=2,
+    )
+
+
+def _read_fallback() -> dict[str, str]:
+    """Read the fallback secrets file, repairing its permissions on read."""
+    try:
+        # Repair permissions on read: if the file leaked to a broader mode
+        # (bad umask, restored backup), tighten it back to 0o600.
+        try:
+            current = os.stat(_FALLBACK_FILE).st_mode & 0o777
+            if current != _FALLBACK_MODE:
+                os.chmod(_FALLBACK_FILE, _FALLBACK_MODE)
+        except OSError:
+            pass
+        with open(_FALLBACK_FILE, encoding="utf-8") as f:
+            data = json.load(f)
+    except (OSError, json.JSONDecodeError):
+        return {}
+    return data if isinstance(data, dict) else {}
+
+
+def _write_fallback(data: dict[str, str]) -> bool:
+    """Atomically write the fallback file with ``0o600`` permissions.
+
+    Writes to a temp file in the same directory, ``chmod`` s it before it
+    ever holds content the target will keep, then ``os.replace`` s it into
+    place so a crash mid-write can never leave a truncated secrets file.
+    """
+    try:
+        os.makedirs(CONFIG_DIR, exist_ok=True)
+    except OSError:
+        return False
+
+    fd, tmp = tempfile.mkstemp(dir=CONFIG_DIR, suffix=".tmp")
+    try:
+        os.fchmod(fd, _FALLBACK_MODE)
+        with os.fdopen(fd, "w", encoding="utf-8") as f:
+            json.dump(data, f, indent=2)
+            f.flush()
+            os.fsync(f.fileno())
+        os.replace(tmp, _FALLBACK_FILE)
+    except OSError:
+        try:
+            os.unlink(tmp)
+        except OSError:
+            pass
+        return False
+    return True
+
+
+# ---------------------------------------------------------------------------
+# Public high-level API
+# ---------------------------------------------------------------------------
+
+
+def get_secret(name: str) -> str | None:
+    """Return a secret by name, or ``None`` when it is not stored.
+
+    Reads the keyring first, then the file fallback. The fallback is only
+    consulted when the keyring backend is unavailable.
+    """
+    value = _keyring_get(name)
+    if value:
+        return value
+
+    if keyring_available():
+        return None
+
+    _warn_fallback_active()
+    stored = _read_fallback().get(name)
+    if stored is None:
+        return None
+    normalized = str(stored).strip()
+    return normalized or None
+
+
+def set_secret(name: str, value: str) -> None:
+    """Persist a secret by name.
+
+    Writes to the keyring when a backend is available, otherwise to the
+    permission-hardened JSON fallback.
+    """
+    if _keyring_set(name, value):
+        return
+
+    _warn_fallback_active()
+    normalized = str(value).strip()
+    if not normalized:
+        return
+    data = _read_fallback()
+    data[name] = normalized
+    _write_fallback(data)
+
+
+def delete_secret(name: str) -> None:
+    """Best-effort removal of a secret from both the keyring and fallback."""
+    _keyring_delete(name)
+
+    if keyring_available():
+        return
+
+    data = _read_fallback()
+    if name in data:
+        del data[name]
+        _write_fallback(data)
diff --git a/pyproject.toml b/pyproject.toml
index 6a6db639c..9ba4c0bd2 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -29,6 +29,7 @@ dependencies = [
     "azure-identity>=1.15.0",
     "anthropic==0.79.0",
     "boto3>=1.43.9",
+    "keyring>=24.0.0",
 ]
 dev-dependencies = [
     "pytest>=8.3.4",
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
new file mode 100644
index 000000000..9b49abf2e
--- /dev/null
+++ b/tests/test_secret_store.py
@@ -0,0 +1,216 @@
+"""Tests for code_puppy.secret_store -- generic OS keyring wrapper.
+
+Covers the three paths called out in the subtask:
+    1. keyring available   -- reads/writes route through the OS keyring
+    2. keyring missing      -- operations degrade to the file fallback
+    3. fallback file        -- 0o600 perms, atomic write, read-repair
+"""
+
+import json
+import os
+import stat
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from code_puppy import secret_store
+
+
+# ---------------------------------------------------------------------------
+# Fixtures
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture(autouse=True)
+def _reset_warn_flag():
+    """The one-shot fallback warning is process global; reset per test."""
+    secret_store._warned_fallback = False
+    yield
+    secret_store._warned_fallback = False
+
+
+@pytest.fixture
+def tmp_fallback(tmp_path, monkeypatch):
+    """Point the fallback file + config dir at a temp location."""
+    cfg_dir = tmp_path / "cfg"
+    cfg_dir.mkdir()
+    fallback = cfg_dir / "secrets.json"
+    monkeypatch.setattr(secret_store, "CONFIG_DIR", str(cfg_dir))
+    monkeypatch.setattr(secret_store, "_FALLBACK_FILE", str(fallback))
+    return fallback
+
+
+@pytest.fixture
+def working_keyring():
+    """A keyring with a healthy backend and an in-memory store."""
+    store: dict[tuple[str, str], str] = {}
+    fake = MagicMock()
+
+    fake.get_password = MagicMock(
+        side_effect=lambda service, name: store.get((service, name))
+    )
+
+    def _set(service, name, value):
+        store[(service, name)] = value
+
+    def _delete(service, name):
+        key = (service, name)
+        if key not in store:
+            raise Exception("not found")
+        del store[key]
+
+    fake.set_password = MagicMock(side_effect=_set)
+    fake.delete_password = MagicMock(side_effect=_delete)
+
+    backend = MagicMock()
+    backend.priority = 10
+    fake.get_keyring = MagicMock(return_value=backend)
+
+    with patch.object(secret_store, "keyring", fake):
+        yield fake, store
+
+
+@pytest.fixture
+def missing_keyring():
+    """A keyring whose backend is unavailable (priority 0, ops raise)."""
+    fake = MagicMock()
+    fake.get_password = MagicMock(side_effect=Exception("no backend"))
+    fake.set_password = MagicMock(side_effect=Exception("no backend"))
+    fake.delete_password = MagicMock(side_effect=Exception("no backend"))
+
+    backend = MagicMock()
+    backend.priority = 0
+    fake.get_keyring = MagicMock(return_value=backend)
+
+    with patch.object(secret_store, "keyring", fake):
+        yield fake
+
+
+# ---------------------------------------------------------------------------
+# 1. keyring_available
+# ---------------------------------------------------------------------------
+
+
+class TestKeyringAvailable:
+    def test_true_for_healthy_backend(self, working_keyring):
+        assert secret_store.keyring_available() is True
+
+    def test_false_for_priority_zero(self, missing_keyring):
+        assert secret_store.keyring_available() is False
+
+    def test_true_when_priority_missing(self):
+        fake = MagicMock()
+        backend = MagicMock(spec=[])  # no .priority attribute
+        fake.get_keyring = MagicMock(return_value=backend)
+        with patch.object(secret_store, "keyring", fake):
+            assert secret_store.keyring_available() is True
+
+    def test_false_when_get_keyring_raises(self):
+        fake = MagicMock()
+        fake.get_keyring = MagicMock(side_effect=RuntimeError("boom"))
+        with patch.object(secret_store, "keyring", fake):
+            assert secret_store.keyring_available() is False
+
+
+# ---------------------------------------------------------------------------
+# 2. keyring-available path
+# ---------------------------------------------------------------------------
+
+
+class TestKeyringPath:
+    def test_set_then_get_roundtrip(self, working_keyring, tmp_fallback):
+        _, store = working_keyring
+        secret_store.set_secret("my_key", "hunter2")
+        assert store[(secret_store._SERVICE_NAME, "my_key")] == "hunter2"
+        assert secret_store.get_secret("my_key") == "hunter2"
+
+    def test_get_missing_returns_none(self, working_keyring):
+        assert secret_store.get_secret("nope") is None
+
+    def test_get_strips_whitespace(self, working_keyring):
+        _, store = working_keyring
+        store[(secret_store._SERVICE_NAME, "k")] = "  spaced  "
+        assert secret_store.get_secret("k") == "spaced"
+
+    def test_delete_removes_from_keyring(self, working_keyring):
+        _, store = working_keyring
+        store[(secret_store._SERVICE_NAME, "k")] = "v"
+        secret_store.delete_secret("k")
+        assert (secret_store._SERVICE_NAME, "k") not in store
+
+    def test_set_does_not_write_fallback_file(self, working_keyring, tmp_fallback):
+        secret_store.set_secret("k", "v")
+        assert not os.path.exists(tmp_fallback)
+
+    def test_get_ignores_fallback_when_keyring_healthy(
+        self, working_keyring, tmp_fallback
+    ):
+        # A stale fallback file must not shadow a healthy (empty) keyring.
+        tmp_fallback.write_text(json.dumps({"k": "stale"}))
+        assert secret_store.get_secret("k") is None
+
+
+# ---------------------------------------------------------------------------
+# 3. keyring-missing / file fallback path
+# ---------------------------------------------------------------------------
+
+
+class TestFallbackPath:
+    def test_set_writes_fallback_file(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("k", "v")
+        assert tmp_fallback.exists()
+        assert json.loads(tmp_fallback.read_text())["k"] == "v"
+
+    def test_set_then_get_roundtrip(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("k", "v")
+        assert secret_store.get_secret("k") == "v"
+
+    def test_get_missing_returns_none(self, missing_keyring, tmp_fallback):
+        assert secret_store.get_secret("nope") is None
+
+    def test_fallback_file_is_0600(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("k", "v")
+        mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
+        assert mode == 0o600
+
+    def test_read_repairs_loose_permissions(self, missing_keyring, tmp_fallback):
+        tmp_fallback.write_text(json.dumps({"k": "v"}))
+        os.chmod(tmp_fallback, 0o644)
+        assert secret_store.get_secret("k") == "v"
+        mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
+        assert mode == 0o600
+
+    def test_set_blank_is_ignored(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("k", "   ")
+        assert not tmp_fallback.exists()
+
+    def test_delete_removes_from_fallback(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("k", "v")
+        secret_store.set_secret("keep", "me")
+        secret_store.delete_secret("k")
+        data = json.loads(tmp_fallback.read_text())
+        assert "k" not in data
+        assert data["keep"] == "me"
+
+    def test_set_preserves_existing_keys(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("a", "1")
+        secret_store.set_secret("b", "2")
+        data = json.loads(tmp_fallback.read_text())
+        assert data == {"a": "1", "b": "2"}
+
+    def test_corrupt_fallback_file_is_tolerated(self, missing_keyring, tmp_fallback):
+        tmp_fallback.write_text("{ not valid json")
+        assert secret_store.get_secret("k") is None
+        # A subsequent write recovers the file.
+        secret_store.set_secret("k", "v")
+        assert secret_store.get_secret("k") == "v"
+
+    def test_fallback_emits_warning_once(self, missing_keyring, tmp_fallback):
+        with pytest.warns(UserWarning, match="fallback"):
+            secret_store.set_secret("k", "v")
+        # Second op must not warn again (one-shot guard).
+        import warnings as _w
+
+        with _w.catch_warnings():
+            _w.simplefilter("error")
+            secret_store.set_secret("k2", "v2")  # would raise if it warned
diff --git a/uv.lock b/uv.lock
index 1f7ef4a7a..8d9b5510f 100644
--- a/uv.lock
+++ b/uv.lock
@@ -90,6 +90,15 @@ wheels = [
     { url = "https://files.pythonhosted.org/packages/49/9a/417b3a533e01953a7c618884df2cb05a71e7b68bdbce4fbdb62349d2a2e8/azure_identity-1.25.3-py3-none-any.whl", hash = "sha256:f4d0b956a8146f30333e071374171f3cfa7bdb8073adb8c3814b65567aa7447c", size = 192138, upload-time = "2026-03-13T01:12:22.951Z" },
 ]
 
+[[package]]
+name = "backports-tarfile"
+version = "1.2.0"
+source = { registry = "https://pypi.org/simple" }
+sdist = { url = "https://files.pythonhosted.org/packages/86/72/cd9b395f25e290e633655a100af28cb253e4393396264a98bd5f5951d50f/backports_tarfile-1.2.0.tar.gz", hash = "sha256:d75e02c268746e1b8144c278978b6e98e85de6ad16f8e4b0844a154557eca991", size = 86406, upload-time = "2024-05-28T17:01:54.731Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/b9/fa/123043af240e49752f1c4bd24da5053b6bd00cad78c2be53c0d1e8b975bc/backports.tarfile-1.2.0-py3-none-any.whl", hash = "sha256:77e284d754527b01fb1e6fa8a1afe577858ebe4e9dad8919e34c862cb399bc34", size = 30181, upload-time = "2024-05-28T17:01:53.112Z" },
+]
+
 [[package]]
 name = "boto3"
 version = "1.43.9"
@@ -308,6 +317,7 @@ dependencies = [
     { name = "boto3" },
     { name = "httpx", extra = ["http2"] },
     { name = "json-repair" },
+    { name = "keyring" },
     { name = "mcp" },
     { name = "openai" },
     { name = "pillow" },
@@ -351,6 +361,7 @@ requires-dist = [
     { name = "dbos", marker = "extra == 'durable'", specifier = ">=2.11.0" },
     { name = "httpx", extras = ["http2"], specifier = ">=0.24.1" },
     { name = "json-repair", specifier = ">=0.46.2" },
+    { name = "keyring", specifier = ">=24.0.0" },
     { name = "mcp", specifier = ">=1.9.4" },
     { name = "openai", specifier = ">=1.99.1" },
     { name = "pillow", specifier = ">=10.0.0" },
@@ -802,6 +813,51 @@ wheels = [
     { url = "https://files.pythonhosted.org/packages/cb/b1/3846dd7f199d53cb17f49cba7e651e9ce294d8497c8c150530ed11865bb8/iniconfig-2.3.0-py3-none-any.whl", hash = "sha256:f631c04d2c48c52b84d0d0549c99ff3859c98df65b3101406327ecc7d53fbf12", size = 7484, upload-time = "2025-10-18T21:55:41.639Z" },
 ]
 
+[[package]]
+name = "jaraco-classes"
+version = "3.4.0"
+source = { registry = "https://pypi.org/simple" }
+dependencies = [
+    { name = "more-itertools" },
+]
+sdist = { url = "https://files.pythonhosted.org/packages/06/c0/ed4a27bc5571b99e3cff68f8a9fa5b56ff7df1c2251cc715a652ddd26402/jaraco.classes-3.4.0.tar.gz", hash = "sha256:47a024b51d0239c0dd8c8540c6c7f484be3b8fcf0b2d85c13825780d3b3f3acd", size = 11780, upload-time = "2024-03-31T07:27:36.643Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/7f/66/b15ce62552d84bbfcec9a4873ab79d993a1dd4edb922cbfccae192bd5b5f/jaraco.classes-3.4.0-py3-none-any.whl", hash = "sha256:f662826b6bed8cace05e7ff873ce0f9283b5c924470fe664fff1c2f00f581790", size = 6777, upload-time = "2024-03-31T07:27:34.792Z" },
+]
+
+[[package]]
+name = "jaraco-context"
+version = "6.1.2"
+source = { registry = "https://pypi.org/simple" }
+dependencies = [
+    { name = "backports-tarfile", marker = "python_full_version < '3.12'" },
+]
+sdist = { url = "https://files.pythonhosted.org/packages/af/50/4763cd07e722bb6285316d390a164bc7e479db9d90daa769f22578f698b4/jaraco_context-6.1.2.tar.gz", hash = "sha256:f1a6c9d391e661cc5b8d39861ff077a7dc24dc23833ccee564b234b81c82dfe3", size = 16801, upload-time = "2026-03-20T22:13:33.922Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/f2/58/bc8954bda5fcda97bd7c19be11b85f91973d67a706ed4a3aec33e7de22db/jaraco_context-6.1.2-py3-none-any.whl", hash = "sha256:bf8150b79a2d5d91ae48629d8b427a8f7ba0e1097dd6202a9059f29a36379535", size = 7871, upload-time = "2026-03-20T22:13:32.808Z" },
+]
+
+[[package]]
+name = "jaraco-functools"
+version = "4.5.0"
+source = { registry = "https://pypi.org/simple" }
+dependencies = [
+    { name = "more-itertools" },
+]
+sdist = { url = "https://files.pythonhosted.org/packages/36/cf/ea4ef2920830dea3f5ab2ea4da6fb67724e6dca80ee2553788c3607243d0/jaraco_functools-4.5.0.tar.gz", hash = "sha256:3bb5665ea4a020cf78a7040e89154c77edadb3ca74f366479669c5999aa70b03", size = 20272, upload-time = "2026-05-15T21:34:10.025Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/96/9a/982e48afcffcd727a9144506720ffd4224b6b7e355c98641866f38b7c043/jaraco_functools-4.5.0-py3-none-any.whl", hash = "sha256:79ce39246eddbde4b3a03b77ea5f0f7878dc669b166a66cf3fa8e266aa3fa2f4", size = 10594, upload-time = "2026-05-15T21:34:08.595Z" },
+]
+
+[[package]]
+name = "jeepney"
+version = "0.9.0"
+source = { registry = "https://pypi.org/simple" }
+sdist = { url = "https://files.pythonhosted.org/packages/7b/6f/357efd7602486741aa73ffc0617fb310a29b588ed0fd69c2399acbb85b0c/jeepney-0.9.0.tar.gz", hash = "sha256:cf0e9e845622b81e4a28df94c40345400256ec608d0e55bb8a3feaa9163f5732", size = 106758, upload-time = "2025-02-27T18:51:01.684Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/b2/a3/e137168c9c44d18eff0376253da9f1e9234d0239e0ee230d2fee6cea8e55/jeepney-0.9.0-py3-none-any.whl", hash = "sha256:97e5714520c16fc0a45695e5365a2e11b81ea79bba796e26f9f1d178cb182683", size = 49010, upload-time = "2025-02-27T18:51:00.104Z" },
+]
+
 [[package]]
 name = "jiter"
 version = "0.14.0"
@@ -937,6 +993,24 @@ wheels = [
     { url = "https://files.pythonhosted.org/packages/41/45/1a4ed80516f02155c51f51e8cedb3c1902296743db0bbc66608a0db2814f/jsonschema_specifications-2025.9.1-py3-none-any.whl", hash = "sha256:98802fee3a11ee76ecaca44429fda8a41bff98b00a0f2838151b113f210cc6fe", size = 18437, upload-time = "2025-09-08T01:34:57.871Z" },
 ]
 
+[[package]]
+name = "keyring"
+version = "25.7.0"
+source = { registry = "https://pypi.org/simple" }
+dependencies = [
+    { name = "importlib-metadata", marker = "python_full_version < '3.12'" },
+    { name = "jaraco-classes" },
+    { name = "jaraco-context" },
+    { name = "jaraco-functools" },
+    { name = "jeepney", marker = "sys_platform == 'linux'" },
+    { name = "pywin32-ctypes", marker = "sys_platform == 'win32'" },
+    { name = "secretstorage", marker = "sys_platform == 'linux'" },
+]
+sdist = { url = "https://files.pythonhosted.org/packages/43/4b/674af6ef2f97d56f0ab5153bf0bfa28ccb6c3ed4d1babf4305449668807b/keyring-25.7.0.tar.gz", hash = "sha256:fe01bd85eb3f8fb3dd0405defdeac9a5b4f6f0439edbb3149577f244a2e8245b", size = 63516, upload-time = "2025-11-16T16:26:09.482Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/81/db/e655086b7f3a705df045bf0933bdd9c2f79bb3c97bfef1384598bb79a217/keyring-25.7.0-py3-none-any.whl", hash = "sha256:be4a0b195f149690c166e850609a477c532ddbfbaed96a404d4e43f8d5e2689f", size = 39160, upload-time = "2025-11-16T16:26:08.402Z" },
+]
+
 [[package]]
 name = "logfire-api"
 version = "4.33.0"
@@ -992,6 +1066,15 @@ wheels = [
     { url = "https://files.pythonhosted.org/packages/b3/38/89ba8ad64ae25be8de66a6d463314cf1eb366222074cfda9ee839c56a4b4/mdurl-0.1.2-py3-none-any.whl", hash = "sha256:84008a41e51615a49fc9966191ff91509e3c40b939176e643fd50a5c2196b8f8", size = 9979, upload-time = "2022-08-14T12:40:09.779Z" },
 ]
 
+[[package]]
+name = "more-itertools"
+version = "11.1.0"
+source = { registry = "https://pypi.org/simple" }
+sdist = { url = "https://files.pythonhosted.org/packages/de/1d/f4da6f02cdffe04d6362210b807146a26044c88d839208aec273bb0d9184/more_itertools-11.1.0.tar.gz", hash = "sha256:48e8f4d9e7e5878571ecf6f2b4e57634f93cd474cc8cfbd2376f2d11b396e30d", size = 145772, upload-time = "2026-05-22T14:14:29.909Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/e8/3d/1087453384dbde46a8c7f9356eead2c58be8a7bf156bca40243377c85715/more_itertools-11.1.0-py3-none-any.whl", hash = "sha256:4b65538ae22f6fed0ce4874efd317463a7489796a0939fa66824dd542125a192", size = 72226, upload-time = "2026-05-22T14:14:28.824Z" },
+]
+
 [[package]]
 name = "msal"
 version = "1.36.0"
@@ -1597,6 +1680,15 @@ wheels = [
     { url = "https://files.pythonhosted.org/packages/c0/d2/21af5c535501a7233e734b8af901574572da66fcc254cb35d0609c9080dd/pywin32-311-cp314-cp314-win_arm64.whl", hash = "sha256:a508e2d9025764a8270f93111a970e1d0fbfc33f4153b388bb649b7eec4f9b42", size = 8932540, upload-time = "2025-07-14T20:13:36.379Z" },
 ]
 
+[[package]]
+name = "pywin32-ctypes"
+version = "0.2.3"
+source = { registry = "https://pypi.org/simple" }
+sdist = { url = "https://files.pythonhosted.org/packages/85/9f/01a1a99704853cb63f253eea009390c88e7131c67e66a0a02099a8c917cb/pywin32-ctypes-0.2.3.tar.gz", hash = "sha256:d162dc04946d704503b2edc4d55f3dba5c1d539ead017afa00142c38b9885755", size = 29471, upload-time = "2024-08-14T10:15:34.626Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/de/3d/8161f7711c017e01ac9f008dfddd9410dff3674334c233bde66e7ba65bbf/pywin32_ctypes-0.2.3-py3-none-any.whl", hash = "sha256:8a1513379d709975552d202d942d9837758905c8d01eb82b8bcc30918929e7b8", size = 30756, upload-time = "2024-08-14T10:15:33.187Z" },
+]
+
 [[package]]
 name = "pyyaml"
 version = "6.0.3"
@@ -2044,6 +2136,19 @@ wheels = [
     { url = "https://files.pythonhosted.org/packages/87/72/c6c32d2b657fa3dad1de340254e14390b1e334ce38268b7ad51abda3c8c2/s3transfer-0.17.0-py3-none-any.whl", hash = "sha256:ce3801712acf4ad3e89fb9990df97b4972e93f4b3b0004d214be5bce12814c20", size = 86811, upload-time = "2026-04-29T22:07:34.966Z" },
 ]
 
+[[package]]
+name = "secretstorage"
+version = "3.5.0"
+source = { registry = "https://pypi.org/simple" }
+dependencies = [
+    { name = "cryptography" },
+    { name = "jeepney" },
+]
+sdist = { url = "https://files.pythonhosted.org/packages/1c/03/e834bcd866f2f8a49a85eaff47340affa3bfa391ee9912a952a1faa68c7b/secretstorage-3.5.0.tar.gz", hash = "sha256:f04b8e4689cbce351744d5537bf6b1329c6fc68f91fa666f60a380edddcd11be", size = 19884, upload-time = "2025-11-23T19:02:53.191Z" }
+wheels = [
+    { url = "https://files.pythonhosted.org/packages/b7/46/f5af3402b579fd5e11573ce652019a67074317e18c1935cc0b4ba9b35552/secretstorage-3.5.0-py3-none-any.whl", hash = "sha256:0ce65888c0725fcb2c5bc0fdb8e5438eece02c523557ea40ce0703c266248137", size = 15554, upload-time = "2025-11-23T19:02:51.545Z" },
+]
+
 [[package]]
 name = "shellingham"
 version = "1.5.4"

From 9be6abc75992ce000613a4e5bfc7a460f1773a44 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Fri, 3 Jul 2026 13:44:56 -0400
Subject: [PATCH 02/16] feat(secret-store): add consolidated macOS keyring
 backend

Store all secrets in a single macOS Keychain item so the user sees at
most one Keychain access prompt regardless of secret count.

Why
- Each Keychain item's ACL is anchored to the calling binary's identity.
  uv's managed CPython is ad-hoc signed (no stable Team ID), so its
  identity is its cdhash, which changes on every Python version bump.
  That invalidates the ACL and prompts the user once per keychain item.
  With ~12 secrets landing across upcoming subtasks, that is a dozen
  "Always Allow" dialogs after a routine upgrade. One item means one ACL
  and therefore one prompt.

Design
- New code_puppy/secret_store_backends.py: ConsolidatedKeychainBackend, a
  custom keyring backend that stores a service's secrets as one JSON blob
  under a single (service, __blob__) keychain item, delegating actual
  keychain I/O to the stock macOS backend.
- Darwin-gated: should_use_consolidated_backend() returns True only on
  macOS, only when the active backend is the stock macOS one, and only
  when the user has not pinned a backend (PYTHON_KEYRING_BACKEND). Off
  macOS the native keyring backend is used unchanged.
- Selection is explicit via keyring.set_keyring. priority is set BELOW
  the stock backend (0.5 < 5) so keyring's automatic priority-based
  discovery never picks this backend on any platform; the Darwin gate is
  the single source of truth. The macOS delegate is constructed lazily so
  merely importing/instantiating the class during cross-platform backend
  discovery never touches macOS-only code.
- secret_store installs the backend lazily and once, on first secret op,
  via _ensure_backend(). Best-effort: any failure leaves the native
  backend in place.

Concurrency + corruption safety
- set/delete hold an in-process lock plus an advisory flock on a lock
  file beside CONFIG_DIR, covering the full read-modify-write so parallel
  sessions / MCP subprocesses cannot clobber each other's writes.
- A corrupt or non-dict blob raises rather than being treated as empty,
  so a bad read can never silently overwrite every stored secret.

Public API unchanged: get_secret / set_secret / delete_secret /
keyring_available are identical on every platform. Consolidation is a
backend concern callers never observe.

Tests: gating (darwin/stock/user-pinned), blob storage (N secrets share
one item, distinct services get distinct items), corruption safety, and
best-effort install. Verified end-to-end against the real macOS Keychain
(3 secrets -> one item, surgical delete). 35 passed, ruff clean.
---
 code_puppy/secret_store.py          |  25 ++++
 code_puppy/secret_store_backends.py | 197 ++++++++++++++++++++++++++++
 tests/test_secret_store.py          |   2 +
 tests/test_secret_store_backends.py | 161 +++++++++++++++++++++++
 4 files changed, 385 insertions(+)
 create mode 100644 code_puppy/secret_store_backends.py
 create mode 100644 tests/test_secret_store_backends.py

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index ff16af185..f78d2a9c4 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -38,6 +38,27 @@
 # we do not spam the console on every read/write.
 _warned_fallback = False
 
+# The consolidated macOS backend is installed lazily, once, on first use.
+_backend_installed = False
+
+
+def _ensure_backend() -> None:
+    """Install the consolidated macOS backend once, before any secret op.
+
+    Off macOS (or when the user pinned a backend) this is a no-op and the
+    native ``keyring`` backend is used unchanged. Best-effort: a failure
+    leaves the native backend in place.
+    """
+    global _backend_installed
+    if _backend_installed:
+        return
+    _backend_installed = True
+    from code_puppy.secret_store_backends import (
+        install_consolidated_backend_if_appropriate,
+    )
+
+    install_consolidated_backend_if_appropriate()
+
 
 # ---------------------------------------------------------------------------
 # Keyring availability + low-level access
@@ -51,6 +72,7 @@ def keyring_available() -> bool:
     a headless Linux box) is treated as unavailable, so callers degrade to
     the file fallback instead of writing into a black hole.
     """
+    _ensure_backend()
     try:
         backend = keyring.get_keyring()
     except Exception:
@@ -170,6 +192,7 @@ def get_secret(name: str) -> str | None:
     Reads the keyring first, then the file fallback. The fallback is only
     consulted when the keyring backend is unavailable.
     """
+    _ensure_backend()
     value = _keyring_get(name)
     if value:
         return value
@@ -191,6 +214,7 @@ def set_secret(name: str, value: str) -> None:
     Writes to the keyring when a backend is available, otherwise to the
     permission-hardened JSON fallback.
     """
+    _ensure_backend()
     if _keyring_set(name, value):
         return
 
@@ -205,6 +229,7 @@ def set_secret(name: str, value: str) -> None:
 
 def delete_secret(name: str) -> None:
     """Best-effort removal of a secret from both the keyring and fallback."""
+    _ensure_backend()
     _keyring_delete(name)
 
     if keyring_available():
diff --git a/code_puppy/secret_store_backends.py b/code_puppy/secret_store_backends.py
new file mode 100644
index 000000000..1ac5fea95
--- /dev/null
+++ b/code_puppy/secret_store_backends.py
@@ -0,0 +1,197 @@
+"""macOS consolidated keyring backend for Code Puppy.
+
+Stores every secret in a single macOS Keychain item (one JSON blob) so
+the user sees at most one Keychain access prompt regardless of how many
+secrets Code Puppy stores.
+
+Why this exists
+---------------
+A macOS Keychain item carries an ACL naming which binaries may read it
+without prompting. The ACL is anchored to the calling binary's identity.
+The interpreter that talks to Keychain is uv's managed CPython, which is
+ad-hoc signed (no stable Team ID), so its identity is its cdhash. Every
+Python version bump produces a new cdhash, misses the stored ACL, and
+prompts the user once per keychain item. With a dozen secrets that is a
+dozen "Always Allow" dialogs after a routine upgrade. One item means one
+ACL and therefore one prompt.
+
+Scope
+-----
+This backend is registered only on macOS, only when the active backend is
+the stock macOS one, and only when the user has not pinned their own
+backend. Off macOS the native ``keyring`` backend is used unchanged. The
+``secret_store`` public API does not change; consolidation is a backend
+concern that callers never observe.
+"""
+
+from __future__ import annotations
+
+import contextlib
+import fcntl
+import json
+import os
+import sys
+import threading
+
+import keyring
+from keyring.backend import KeyringBackend
+
+from code_puppy.config import CONFIG_DIR
+
+# All secrets for a given service live under this single, fixed account.
+# The service name is supplied per call by secret_store, so the public and
+# enterprise builds (distinct service names) get distinct blob items.
+_BLOB_ACCOUNT = "__code_puppy_secret_blob__"
+
+# Cross-process lock file guarding the read-modify-write of the blob.
+_LOCK_FILE = os.path.join(CONFIG_DIR, ".secrets.lock")
+
+# In-process guard. flock covers other processes; this covers threads in
+# this process cheaply and predictably.
+_thread_lock = threading.RLock()
+
+
+@contextlib.contextmanager
+def _blob_lock():
+    """Serialize the blob read-modify-write across threads and processes.
+
+    Keychain item writes are atomic at the item level, so readers always
+    see a whole blob (old or new), never a partial one. Writers, however,
+    must not interleave their read-modify-write cycles or one would clobber
+    the other's addition. This takes an in-process lock plus an advisory
+    ``flock`` on a lock file beside ``CONFIG_DIR``.
+    """
+    with _thread_lock:
+        try:
+            os.makedirs(CONFIG_DIR, exist_ok=True)
+        except OSError:
+            # If we cannot create the lock dir, fall through with only the
+            # thread lock. Better to proceed than to wedge secret writes.
+            yield
+            return
+        fd = os.open(_LOCK_FILE, os.O_CREAT | os.O_RDWR, 0o600)
+        try:
+            fcntl.flock(fd, fcntl.LOCK_EX)
+            yield
+        finally:
+            try:
+                fcntl.flock(fd, fcntl.LOCK_UN)
+            finally:
+                os.close(fd)
+
+
+class ConsolidatedKeychainBackend(KeyringBackend):
+    """Store all of a service's secrets in one keychain item (JSON blob).
+
+    This backend is selected explicitly via ``keyring.set_keyring`` from
+    ``install_consolidated_backend_if_appropriate``; it is never chosen by
+    ``keyring``'s automatic priority-based discovery. Its ``priority`` is
+    deliberately below the stock macOS backend so auto-discovery prefers
+    the native backend on every platform, and the Darwin gating in
+    ``should_use_consolidated_backend`` stays the single source of truth
+    for when consolidation is active.
+    """
+
+    # Below the stock macOS backend (5) so keyring's auto-discovery never
+    # selects this backend on any platform. Selection is explicit via
+    # set_keyring. Still positive so secret_store.keyring_available()
+    # treats it as usable once installed.
+    priority = 0.5
+
+    def __init__(self) -> None:
+        super().__init__()
+        # Delegate is created lazily (see _get_delegate) rather than here,
+        # so merely constructing this class during keyring's cross-platform
+        # backend discovery never touches the macOS-only backend.
+        self._delegate_cache = None
+
+    def _get_delegate(self):
+        """Lazily construct the stock macOS backend we delegate I/O to.
+
+        This backend only reshapes the key space (one item instead of one
+        per secret); it does not reimplement keychain I/O.
+        """
+        if self._delegate_cache is None:
+            from keyring.backends import macOS as macos_backend
+
+            self._delegate_cache = macos_backend.Keyring()
+        return self._delegate_cache
+
+    # -- blob helpers -------------------------------------------------------
+
+    def _read_blob(self, service: str) -> dict[str, str]:
+        """Read and parse the blob. Raise on corruption; never assume empty.
+
+        Treating a corrupt blob as empty would let the next write clobber
+        every stored secret. Raising instead surfaces the problem and,
+        because secret_store wraps keyring calls, degrades safely rather
+        than destroying data.
+        """
+        raw = self._get_delegate().get_password(service, _BLOB_ACCOUNT)
+        if not raw:
+            return {}
+        data = json.loads(raw)
+        if not isinstance(data, dict):
+            raise ValueError("secret blob is not a JSON object")
+        return data
+
+    def _write_blob(self, service: str, blob: dict[str, str]) -> None:
+        self._get_delegate().set_password(service, _BLOB_ACCOUNT, json.dumps(blob))
+
+    # -- KeyringBackend interface ------------------------------------------
+
+    def get_password(self, service: str, username: str) -> str | None:
+        # Item-level atomicity makes a lock unnecessary for a single read.
+        blob = self._read_blob(service)
+        value = blob.get(username)
+        return value if value else None
+
+    def set_password(self, service: str, username: str, password: str) -> None:
+        with _blob_lock():
+            blob = self._read_blob(service)  # raises on corruption, no clobber
+            blob[username] = password
+            self._write_blob(service, blob)
+
+    def delete_password(self, service: str, username: str) -> None:
+        with _blob_lock():
+            blob = self._read_blob(service)
+            if username in blob:
+                del blob[username]
+                self._write_blob(service, blob)
+
+
+def should_use_consolidated_backend() -> bool:
+    """Report whether the consolidated backend should be installed.
+
+    True only on macOS, only when the active backend is the stock macOS
+    one, and only when the user has not pinned a backend via the
+    ``PYTHON_KEYRING_BACKEND`` env var. A backend configured through
+    ``keyringrc.cfg`` becomes the active backend, so the stock-backend
+    check below also defers to it.
+    """
+    if sys.platform != "darwin":
+        return False
+    if os.environ.get("PYTHON_KEYRING_BACKEND"):
+        return False
+    try:
+        from keyring.backends import macOS as macos_backend
+
+        current = keyring.get_keyring()
+    except Exception:
+        return False
+    return isinstance(current, macos_backend.Keyring)
+
+
+def install_consolidated_backend_if_appropriate() -> bool:
+    """Install the consolidated backend when appropriate. Best-effort.
+
+    Returns True when the backend was installed. Never raises: a failure
+    here must leave the native backend in place rather than break startup.
+    """
+    try:
+        if not should_use_consolidated_backend():
+            return False
+        keyring.set_keyring(ConsolidatedKeychainBackend())
+        return True
+    except Exception:
+        return False
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 9b49abf2e..ffb7d747b 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -25,8 +25,10 @@
 def _reset_warn_flag():
     """The one-shot fallback warning is process global; reset per test."""
     secret_store._warned_fallback = False
+    secret_store._backend_installed = True  # skip lazy install in these tests
     yield
     secret_store._warned_fallback = False
+    secret_store._backend_installed = False
 
 
 @pytest.fixture
diff --git a/tests/test_secret_store_backends.py b/tests/test_secret_store_backends.py
new file mode 100644
index 000000000..a8d62b61d
--- /dev/null
+++ b/tests/test_secret_store_backends.py
@@ -0,0 +1,161 @@
+"""Tests for code_puppy.secret_store_backends -- consolidated macOS backend.
+
+Covers:
+    1. gating -- should_use_consolidated_backend only on darwin + default
+    2. blob storage -- N secrets share one keychain item
+    3. concurrency-safety invariants -- corrupt blob never silently clobbered
+    4. install helper -- best-effort, never raises
+"""
+
+import json
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from code_puppy import secret_store_backends as ssb
+
+
+# ---------------------------------------------------------------------------
+# Gating: should_use_consolidated_backend
+# ---------------------------------------------------------------------------
+
+
+class TestGating:
+    def test_false_off_darwin(self, monkeypatch):
+        monkeypatch.setattr(ssb.sys, "platform", "linux")
+        assert ssb.should_use_consolidated_backend() is False
+
+    def test_false_when_user_pinned_backend(self, monkeypatch):
+        monkeypatch.setattr(ssb.sys, "platform", "darwin")
+        monkeypatch.setenv("PYTHON_KEYRING_BACKEND", "keyring.backends.fail.Keyring")
+        assert ssb.should_use_consolidated_backend() is False
+
+    def test_true_on_darwin_with_stock_backend(self, monkeypatch):
+        monkeypatch.setattr(ssb.sys, "platform", "darwin")
+        monkeypatch.delenv("PYTHON_KEYRING_BACKEND", raising=False)
+        from keyring.backends import macOS as macos_backend
+
+        with patch.object(
+            ssb.keyring, "get_keyring", return_value=macos_backend.Keyring()
+        ):
+            assert ssb.should_use_consolidated_backend() is True
+
+    def test_false_when_active_backend_not_stock_macos(self, monkeypatch):
+        monkeypatch.setattr(ssb.sys, "platform", "darwin")
+        monkeypatch.delenv("PYTHON_KEYRING_BACKEND", raising=False)
+        with patch.object(ssb.keyring, "get_keyring", return_value=MagicMock()):
+            assert ssb.should_use_consolidated_backend() is False
+
+    def test_false_when_get_keyring_raises(self, monkeypatch):
+        monkeypatch.setattr(ssb.sys, "platform", "darwin")
+        monkeypatch.delenv("PYTHON_KEYRING_BACKEND", raising=False)
+        with patch.object(ssb.keyring, "get_keyring", side_effect=RuntimeError("boom")):
+            assert ssb.should_use_consolidated_backend() is False
+
+
+# ---------------------------------------------------------------------------
+# Blob storage behavior
+# ---------------------------------------------------------------------------
+
+
+@pytest.fixture
+def backend(tmp_path, monkeypatch):
+    """A ConsolidatedKeychainBackend whose delegate is an in-memory store."""
+    monkeypatch.setattr(ssb, "CONFIG_DIR", str(tmp_path))
+    monkeypatch.setattr(ssb, "_LOCK_FILE", str(tmp_path / ".secrets.lock"))
+
+    store: dict[tuple[str, str], str] = {}
+    delegate = MagicMock()
+    delegate.get_password = MagicMock(side_effect=lambda s, a: store.get((s, a)))
+    delegate.set_password = MagicMock(
+        side_effect=lambda s, a, v: store.__setitem__((s, a), v)
+    )
+
+    with patch.object(ssb.ConsolidatedKeychainBackend, "__init__", lambda self: None):
+        be = ssb.ConsolidatedKeychainBackend()
+    be._delegate_cache = delegate
+    return be, store
+
+
+class TestBlobStorage:
+    def test_multiple_secrets_share_one_item(self, backend):
+        be, store = backend
+        be.set_password("code-puppy", "a", "1")
+        be.set_password("code-puppy", "b", "2")
+        be.set_password("code-puppy", "c", "3")
+        # Exactly one keychain item exists, holding all three.
+        assert len(store) == 1
+        blob = json.loads(next(iter(store.values())))
+        assert blob == {"a": "1", "b": "2", "c": "3"}
+
+    def test_roundtrip(self, backend):
+        be, _ = backend
+        be.set_password("code-puppy", "tok", "hunter2")
+        assert be.get_password("code-puppy", "tok") == "hunter2"
+
+    def test_get_missing_returns_none(self, backend):
+        be, _ = backend
+        assert be.get_password("code-puppy", "nope") is None
+
+    def test_delete_removes_only_that_key(self, backend):
+        be, store = backend
+        be.set_password("code-puppy", "a", "1")
+        be.set_password("code-puppy", "b", "2")
+        be.delete_password("code-puppy", "a")
+        blob = json.loads(next(iter(store.values())))
+        assert blob == {"b": "2"}
+
+    def test_distinct_services_get_distinct_items(self, backend):
+        be, store = backend
+        be.set_password("code-puppy", "tok", "oss")
+        be.set_password("code-puppy-enterprise", "tok", "ent")
+        assert len(store) == 2
+        assert be.get_password("code-puppy", "tok") == "oss"
+        assert be.get_password("code-puppy-enterprise", "tok") == "ent"
+
+
+class TestCorruptionSafety:
+    def test_corrupt_blob_raises_not_clobbers(self, backend):
+        be, store = backend
+        # Simulate an unparseable existing blob.
+        store[("code-puppy", ssb._BLOB_ACCOUNT)] = "{ not json"
+        with pytest.raises(json.JSONDecodeError):
+            be.set_password("code-puppy", "a", "1")
+        # The bad blob is left intact; no silent overwrite.
+        assert store[("code-puppy", ssb._BLOB_ACCOUNT)] == "{ not json"
+
+    def test_non_object_blob_rejected(self, backend):
+        be, store = backend
+        store[("code-puppy", ssb._BLOB_ACCOUNT)] = json.dumps(["not", "a", "dict"])
+        with pytest.raises(ValueError, match="not a JSON object"):
+            be.get_password("code-puppy", "a")
+
+
+# ---------------------------------------------------------------------------
+# install_consolidated_backend_if_appropriate
+# ---------------------------------------------------------------------------
+
+
+class TestInstall:
+    def test_installs_when_appropriate(self):
+        with (
+            patch.object(ssb, "should_use_consolidated_backend", return_value=True),
+            patch.object(ssb.keyring, "set_keyring") as set_kr,
+        ):
+            assert ssb.install_consolidated_backend_if_appropriate() is True
+            set_kr.assert_called_once()
+
+    def test_skips_when_not_appropriate(self):
+        with (
+            patch.object(ssb, "should_use_consolidated_backend", return_value=False),
+            patch.object(ssb.keyring, "set_keyring") as set_kr,
+        ):
+            assert ssb.install_consolidated_backend_if_appropriate() is False
+            set_kr.assert_not_called()
+
+    def test_never_raises(self):
+        with patch.object(
+            ssb, "should_use_consolidated_backend", side_effect=RuntimeError("boom")
+        ):
+            # Must swallow and report False, never propagate.
+            assert ssb.install_consolidated_backend_if_appropriate() is False

From 9af3a4509cda7fa33a1f1282c9b2d24bcb13fe29 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Tue, 7 Jul 2026 12:13:03 -0400
Subject: [PATCH 03/16] fix(secret-store): cross-platform guards +
 stranded-secret warning

Three minimalist bug fixes for the secret store introduced in PR #2:

1. _ensure_backend: catch ImportError when secret_store_backends is
   unimportable (Windows has no fcntl module). The consolidated macOS
   backend is irrelevant off macOS anyway.

2. _write_fallback: use hasattr(os, 'fchmod') guard and fall back to
   os.chmod on the path. os.fchmod is POSIX-only and raises
   AttributeError on Windows, which the existing except OSError would
   not catch.

3. set_secret: when the keyring backend is healthy but the write fails
   (transient error, dismissed Keychain prompt, etc.), warn instead of
   silently writing to the fallback file. The old behavior stranded the
   secret because get_secret skips the fallback when keyring_available()
   is True.

Tests added for all three cases.
---
 code_puppy/secret_store.py | 29 +++++++++++++++++++++----
 tests/test_secret_store.py | 43 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 68 insertions(+), 4 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index f78d2a9c4..f94f92ad7 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -53,9 +53,14 @@ def _ensure_backend() -> None:
     if _backend_installed:
         return
     _backend_installed = True
-    from code_puppy.secret_store_backends import (
-        install_consolidated_backend_if_appropriate,
-    )
+    try:
+        from code_puppy.secret_store_backends import (
+            install_consolidated_backend_if_appropriate,
+        )
+    except ImportError:
+        # secret_store_backends imports fcntl (POSIX-only); on Windows
+        # the consolidated macOS backend is irrelevant anyway.
+        return
 
     install_consolidated_backend_if_appropriate()
 
@@ -166,7 +171,11 @@ def _write_fallback(data: dict[str, str]) -> bool:
 
     fd, tmp = tempfile.mkstemp(dir=CONFIG_DIR, suffix=".tmp")
     try:
-        os.fchmod(fd, _FALLBACK_MODE)
+        # os.fchmod is POSIX-only; fall back to os.chmod on the path.
+        if hasattr(os, "fchmod"):
+            os.fchmod(fd, _FALLBACK_MODE)
+        else:
+            os.chmod(tmp, _FALLBACK_MODE)
         with os.fdopen(fd, "w", encoding="utf-8") as f:
             json.dump(data, f, indent=2)
             f.flush()
@@ -218,6 +227,18 @@ def set_secret(name: str, value: str) -> None:
     if _keyring_set(name, value):
         return
 
+    if keyring_available():
+        # The backend is healthy but the write still failed (transient
+        # error, permission prompt dismissed, etc.).  Writing to the
+        # fallback here would strand the secret: get_secret() skips the
+        # fallback when the keyring is available.  Warn instead.
+        warnings.warn(
+            f"Keyring write failed for {name!r} despite a healthy "
+            "backend; the secret was not persisted.",
+            stacklevel=2,
+        )
+        return
+
     _warn_fallback_active()
     normalized = str(value).strip()
     if not normalized:
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index ffb7d747b..9c702e0c7 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -216,3 +216,46 @@ def test_fallback_emits_warning_once(self, missing_keyring, tmp_fallback):
         with _w.catch_warnings():
             _w.simplefilter("error")
             secret_store.set_secret("k2", "v2")  # would raise if it warned
+
+
+# ---------------------------------------------------------------------------
+# 4. Cross-platform fixes
+# ---------------------------------------------------------------------------
+
+
+class TestCrossPlatform:
+    def test_ensure_backend_survives_import_error(self):
+        """_ensure_backend must not crash when secret_store_backends is
+        unimportable (e.g. Windows where fcntl doesn't exist)."""
+        secret_store._backend_installed = False
+        with patch.dict(
+            "sys.modules", {"code_puppy.secret_store_backends": None}
+        ):
+            # Should complete without raising.
+            secret_store._ensure_backend()
+        assert secret_store._backend_installed is True
+
+    def test_write_fallback_without_fchmod(self, tmp_fallback, monkeypatch):
+        """_write_fallback must succeed when os.fchmod is absent (Windows)."""
+        monkeypatch.delattr(os, "fchmod", raising=False)
+        assert secret_store._write_fallback({"k": "v"}) is True
+        assert json.loads(tmp_fallback.read_text()) == {"k": "v"}
+        mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
+        assert mode == 0o600
+
+    def test_set_warns_on_transient_keyring_failure(self, tmp_fallback):
+        """When keyring is healthy but the write fails, set_secret warns
+        instead of silently stranding the secret in the fallback file."""
+        fake = MagicMock()
+        fake.set_password = MagicMock(side_effect=Exception("transient"))
+        fake.get_password = MagicMock(return_value=None)
+
+        backend = MagicMock()
+        backend.priority = 10  # healthy
+        fake.get_keyring = MagicMock(return_value=backend)
+
+        with patch.object(secret_store, "keyring", fake):
+            with pytest.warns(UserWarning, match="Keyring write failed"):
+                secret_store.set_secret("k", "v")
+        # Must NOT have written to the fallback file.
+        assert not tmp_fallback.exists()

From d154a5e9b577a28e495c8f6b5f456c9893f66c82 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Tue, 7 Jul 2026 12:20:36 -0400
Subject: [PATCH 04/16] refactor: replace os.fchmod with os.chmod for
 OS-agnostic fallback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The fallback path exists for environments without a keyring (headless
Linux, CI, Windows). Using os.chmod instead of os.fchmod removes the
hasattr guard from the prior commit — one cross-platform call instead
of a branching shim. Simpler test, same behavior.
---
 code_puppy/secret_store.py | 6 +-----
 tests/test_secret_store.py | 5 ++---
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index f94f92ad7..961ce7449 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -171,11 +171,7 @@ def _write_fallback(data: dict[str, str]) -> bool:
 
     fd, tmp = tempfile.mkstemp(dir=CONFIG_DIR, suffix=".tmp")
     try:
-        # os.fchmod is POSIX-only; fall back to os.chmod on the path.
-        if hasattr(os, "fchmod"):
-            os.fchmod(fd, _FALLBACK_MODE)
-        else:
-            os.chmod(tmp, _FALLBACK_MODE)
+        os.chmod(tmp, _FALLBACK_MODE)
         with os.fdopen(fd, "w", encoding="utf-8") as f:
             json.dump(data, f, indent=2)
             f.flush()
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 9c702e0c7..114e409db 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -235,9 +235,8 @@ def test_ensure_backend_survives_import_error(self):
             secret_store._ensure_backend()
         assert secret_store._backend_installed is True
 
-    def test_write_fallback_without_fchmod(self, tmp_fallback, monkeypatch):
-        """_write_fallback must succeed when os.fchmod is absent (Windows)."""
-        monkeypatch.delattr(os, "fchmod", raising=False)
+    def test_write_fallback_uses_chmod(self, tmp_fallback, monkeypatch):
+        """_write_fallback uses os.chmod (cross-platform), not os.fchmod."""
         assert secret_store._write_fallback({"k": "v"}) is True
         assert json.loads(tmp_fallback.read_text()) == {"k": "v"}
         mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)

From bba4bca83e20267ff648f63e95a3960ea29f6c39 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 13:03:09 -0400
Subject: [PATCH 05/16] refactor: transparent chunking + three-tier fallback +
 configurable service name

Windows Credential Manager encodes blobs as UTF-16-LE and caps them at
~2,560 bytes.  Long tokens (e.g. JWTs) routinely exceed this, causing
silent write failures (error 1783).

Changes:
- Split oversized secrets into <=1,200-char chunks, stored as numbered
  keyring entries with an atomic count marker.  Reads reassemble
  transparently; pre-chunking entries remain readable.
- Three-tier write strategy: direct keyring -> chunked keyring -> 0o600
  file fallback.  get_secret always checks the fallback as a last resort
  so secrets written there by a prior session are still recoverable.
- Add configure_service_name() so downstream distributions can namespace
  their secrets without bleeding across builds.
- Add sys.platform == 'win32' skipif decorators on POSIX permission
  tests and an fcntl collection guard on the backends test module.
- 52 tests pass (9 new chunking + 4 new service-name tests).
---
 code_puppy/secret_store.py          | 252 ++++++++++++++++++++++----
 tests/test_secret_store.py          | 272 +++++++++++++++++++++-------
 tests/test_secret_store_backends.py |  11 +-
 3 files changed, 435 insertions(+), 100 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index 961ce7449..a8923a08c 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -1,16 +1,38 @@
 """Generic OS keyring secret store for Code Puppy.
 
 Reads and writes secrets through the operating system keyring, with a
-permission-hardened JSON file fallback for headless and CI environments
-where no keyring backend is available.
+permission-hardened JSON file fallback as a final safety net.
+
+Write strategy (three tiers, in order):
+  1. **Direct keyring write** -- used when the value fits in one entry.
+  2. **Chunked keyring write** -- the value is split into ≤``_CHUNK_SIZE``
+     pieces when the OS imposes a per-entry size cap.  The primary real-world
+     case is Windows Credential Manager, which rejects blobs larger than
+     ~2 560 bytes UTF-16-LE (error 1783); long tokens routinely exceed
+     this.  Chunking keeps the keyring as the source of truth.
+  3. **Permission-hardened file fallback** -- used only when both keyring
+     strategies fail (genuinely broken backend, backend crash, headless CI).
+     The file is ``0o600`` and written atomically.
+
+Read strategy:
+  The keyring is queried first (with transparent chunk reassembly).  The
+  fallback file is always consulted as a last resort so secrets written there
+  by a previous session (after exhausting both keyring options) are still
+  recoverable even when the keyring subsequently becomes healthy.
+
+The keyring service name is configurable via ``configure_service_name`` so
+each distribution can namespace its secrets and never read, copy, or alias
+secrets across builds.  The default is ``"code-puppy"``; downstream
+distributions override it at startup.
 
 Public API
 ----------
 ``keyring_available()``
     Report whether a usable keyring backend is configured.
+``configure_service_name(name)``
+    Override the keyring service name used for all secret operations.
 ``get_secret(name)`` / ``set_secret(name, value)`` / ``delete_secret(name)``
-    Keyring-first secret operations that transparently fall back to the
-    ``0o600`` JSON file when the keyring backend is unavailable.
+    Three-tier secret operations (keyring direct → keyring chunked → file).
 """
 
 from __future__ import annotations
@@ -25,9 +47,9 @@
 from code_puppy.config import CONFIG_DIR
 
 # Namespace under which every secret is stored in the OS keyring. Downstream
-# distributions should use a distinct service name so secrets never bleed
-# across builds.
-_SERVICE_NAME = "code-puppy"
+# distributions call ``configure_service_name`` to use a distinct name so
+# secrets never bleed across builds.
+_service_name = "code-puppy"
 
 # Permission-hardened JSON fallback used only when the keyring backend is
 # unavailable (headless boxes, minimal CI containers, etc.).
@@ -65,6 +87,49 @@ def _ensure_backend() -> None:
     install_consolidated_backend_if_appropriate()
 
 
+def get_service_name() -> str:
+    """Return the current keyring service name."""
+    return _service_name
+
+
+def configure_service_name(name: str) -> None:
+    """Override the keyring service name used for all secret operations.
+
+    Call this early at startup -- before any get/set/delete calls -- so
+    secrets are namespaced per distribution.  Downstream distributions
+    call this from their ``startup`` callback.  The default is
+    ``"code-puppy"``.
+    """
+    global _service_name
+    name = str(name).strip()
+    if not name:
+        raise ValueError("service name must be non-empty")
+    _service_name = name
+
+
+# Maximum characters per keyring entry.  Windows Credential Manager encodes
+# credential blobs as UTF-16-LE (2 bytes per char) and caps them at ~2 560
+# bytes, giving a ~1 280-char ceiling.  We stay conservatively below that so
+# typical ASCII padding in the JWT header/signature doesn't push us over.
+_CHUNK_SIZE = 1200
+
+# Suffix tokens used to build chunk-related keyring entry names.  The ``cp``
+# prefix scopes them to Code Puppy and makes accidental collisions with real
+# secret names essentially impossible.
+_CHUNK_NS = ":cp:"
+_COUNT_SUFFIX = ":cp:n"
+
+
+def _chunk_count_key(name: str) -> str:
+    """Keyring entry name for the chunk-count (commit) marker."""
+    return f"{name}{_COUNT_SUFFIX}"
+
+
+def _chunk_key(name: str, i: int) -> str:
+    """Keyring entry name for chunk *i* of *name*."""
+    return f"{name}{_CHUNK_NS}{i}"
+
+
 # ---------------------------------------------------------------------------
 # Keyring availability + low-level access
 # ---------------------------------------------------------------------------
@@ -91,9 +156,10 @@ def keyring_available() -> bool:
         return True
 
 
-def _keyring_get(name: str) -> str | None:
+def _kr_get_raw(name: str) -> str | None:
+    """Read one keyring entry; ``None`` on any error or absence."""
     try:
-        value = keyring.get_password(_SERVICE_NAME, name)
+        value = keyring.get_password(_service_name, name)
     except Exception:
         return None
     if value is None:
@@ -102,25 +168,132 @@ def _keyring_get(name: str) -> str | None:
     return normalized or None
 
 
-def _keyring_set(name: str, value: str) -> bool:
-    normalized = str(value).strip()
-    if not normalized:
-        return False
+def _kr_set_raw(name: str, value: str) -> bool:
+    """Write one keyring entry; return ``False`` on any error."""
     try:
-        keyring.set_password(_SERVICE_NAME, name, normalized)
+        keyring.set_password(_service_name, name, value)
     except Exception:
         return False
     return True
 
 
-def _keyring_delete(name: str) -> bool:
+def _kr_del_raw(name: str) -> bool:
+    """Delete one keyring entry; return ``False`` on any error."""
     try:
-        keyring.delete_password(_SERVICE_NAME, name)
+        keyring.delete_password(_service_name, name)
     except Exception:
         return False
     return True
 
 
+# ---------------------------------------------------------------------------
+# Chunk-aware keyring helpers (transparent to callers)
+# ---------------------------------------------------------------------------
+
+
+def _keyring_get(name: str) -> str | None:
+    """Read a logical secret, transparently reassembling chunks if present.
+
+    Checks for a chunk-count key first.  When found, reads and concatenates
+    all chunks in order.  Falls back to a direct single-entry read for values
+    written by older builds or that never needed chunking.
+    """
+    count_raw = _kr_get_raw(_chunk_count_key(name))
+    if count_raw is not None:
+        try:
+            n = int(count_raw)
+        except ValueError:
+            return None  # corrupt count -- treat as absent
+        parts: list[str] = []
+        for i in range(n):
+            chunk = _kr_get_raw(_chunk_key(name, i))
+            if chunk is None:
+                return None  # partial write -- treat as absent
+            parts.append(chunk)
+        assembled = "".join(parts)
+        return assembled or None
+
+    # No chunk metadata -- try a plain single-entry read.
+    return _kr_get_raw(name)
+
+
+def _delete_chunks(name: str) -> None:
+    """Best-effort removal of all chunk entries for *name*."""
+    count_raw = _kr_get_raw(_chunk_count_key(name))
+    if count_raw is None:
+        return
+    try:
+        n = int(count_raw)
+    except ValueError:
+        n = 0
+    for i in range(n):
+        _kr_del_raw(_chunk_key(name, i))
+    _kr_del_raw(_chunk_count_key(name))
+
+
+def _keyring_set(name: str, value: str) -> bool:
+    """Write a logical secret, chunking automatically when it exceeds _CHUNK_SIZE.
+
+    **Small values** (len <= _CHUNK_SIZE):
+      - Written as a single keyring entry under *name*.
+      - Any stale chunk entries from a prior oversized write are removed.
+
+    **Large values** (len > _CHUNK_SIZE):
+      - Split into ceil(len / _CHUNK_SIZE) pieces.
+      - Chunk data entries are written first, then the count key last so a
+        mid-write crash never leaves a partially-readable value behind.
+      - Excess chunks from a prior write with more pieces are pruned.
+      - The direct *name* entry is removed to avoid ambiguity on read.
+
+    Returns ``True`` on success, ``False`` if any keyring write fails.
+    """
+    normalized = str(value).strip()
+    if not normalized:
+        return False
+
+    if len(normalized) <= _CHUNK_SIZE:
+        if not _kr_set_raw(name, normalized):
+            return False
+        _delete_chunks(name)  # clean up any old chunked write
+        return True
+
+    # --- Chunked write path ---
+    chunks = [
+        normalized[i: i + _CHUNK_SIZE]
+        for i in range(0, len(normalized), _CHUNK_SIZE)
+    ]
+
+    # Write data entries first.
+    for idx, chunk in enumerate(chunks):
+        if not _kr_set_raw(_chunk_key(name, idx), chunk):
+            # Partial write -- roll back what we managed.
+            for j in range(idx):
+                _kr_del_raw(_chunk_key(name, j))
+            return False
+
+    # Prune stale trailing chunks from a prior larger write.
+    stale = len(chunks)
+    while _kr_get_raw(_chunk_key(name, stale)) is not None:
+        _kr_del_raw(_chunk_key(name, stale))
+        stale += 1
+
+    # Commit: write the count key last as the atomic marker.
+    if not _kr_set_raw(_chunk_count_key(name), str(len(chunks))):
+        for idx in range(len(chunks)):
+            _kr_del_raw(_chunk_key(name, idx))
+        return False
+
+    _kr_del_raw(name)  # remove any stale single-entry write
+    return True
+
+
+def _keyring_delete(name: str) -> bool:
+    """Delete a logical secret, removing chunks if present."""
+    direct = _kr_del_raw(name)
+    _delete_chunks(name)
+    return direct
+
+
 # ---------------------------------------------------------------------------
 # Permission-hardened JSON file fallback (secure I/O helper)
 # ---------------------------------------------------------------------------
@@ -194,18 +367,22 @@ def _write_fallback(data: dict[str, str]) -> bool:
 def get_secret(name: str) -> str | None:
     """Return a secret by name, or ``None`` when it is not stored.
 
-    Reads the keyring first, then the file fallback. The fallback is only
-    consulted when the keyring backend is unavailable.
+    Resolution order:
+      1. OS keyring -- direct read or transparent chunk reassembly.
+      2. Permission-hardened fallback file -- always consulted as a last
+         resort so secrets written there by a previous session (after
+         exhausting both keyring options) are still recoverable.
     """
     _ensure_backend()
     value = _keyring_get(name)
     if value:
         return value
 
-    if keyring_available():
-        return None
-
-    _warn_fallback_active()
+    # Always check the fallback file: a prior set_secret may have ended up
+    # there after both keyring paths failed.  Only emit the headless warning
+    # when the keyring backend itself is unavailable.
+    if not keyring_available():
+        _warn_fallback_active()
     stored = _read_fallback().get(name)
     if stored is None:
         return None
@@ -216,26 +393,30 @@ def get_secret(name: str) -> str | None:
 def set_secret(name: str, value: str) -> None:
     """Persist a secret by name.
 
-    Writes to the keyring when a backend is available, otherwise to the
-    permission-hardened JSON fallback.
+    Attempts three strategies in order:
+      1. Direct keyring write (small values).
+      2. Chunked keyring write (oversized values, e.g. Windows CM cap).
+      3. Permission-hardened JSON file fallback (only when both keyring
+         strategies fail -- unexpected backend error or no keyring at all).
     """
     _ensure_backend()
     if _keyring_set(name, value):
         return
 
     if keyring_available():
-        # The backend is healthy but the write still failed (transient
-        # error, permission prompt dismissed, etc.).  Writing to the
-        # fallback here would strand the secret: get_secret() skips the
-        # fallback when the keyring is available.  Warn instead.
+        # Both direct and chunked writes failed despite a healthy backend.
+        # Unexpected (transient error, backend crash, prompt dismissed).
+        # Warn so it's diagnosable, then persist to the file so the secret
+        # is not lost.
         warnings.warn(
-            f"Keyring write failed for {name!r} despite a healthy "
-            "backend; the secret was not persisted.",
+            f"Keyring write failed for {name!r} despite a healthy backend "
+            "(transient error or backend crash). Storing in the secure file "
+            f"fallback at {_FALLBACK_FILE}.",
             stacklevel=2,
         )
-        return
+    else:
+        _warn_fallback_active()
 
-    _warn_fallback_active()
     normalized = str(value).strip()
     if not normalized:
         return
@@ -245,13 +426,12 @@ def set_secret(name: str, value: str) -> None:
 
 
 def delete_secret(name: str) -> None:
-    """Best-effort removal of a secret from both the keyring and fallback."""
+    """Best-effort removal of a secret from keyring (and chunks) and fallback."""
     _ensure_backend()
     _keyring_delete(name)
 
-    if keyring_available():
-        return
-
+    # Always scrub the fallback file: a prior write may have ended up there
+    # even on a system where the keyring is now healthy.
     data = _read_fallback()
     if name in data:
         del data[name]
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 114e409db..da7028719 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -4,11 +4,14 @@
     1. keyring available   -- reads/writes route through the OS keyring
     2. keyring missing      -- operations degrade to the file fallback
     3. fallback file        -- 0o600 perms, atomic write, read-repair
+
+Plus the configurable service name for downstream distributions.
 """
 
 import json
 import os
 import stat
+import sys
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -16,24 +19,20 @@
 from code_puppy import secret_store
 
 
-# ---------------------------------------------------------------------------
-# Fixtures
-# ---------------------------------------------------------------------------
-
-
 @pytest.fixture(autouse=True)
-def _reset_warn_flag():
-    """The one-shot fallback warning is process global; reset per test."""
+def _reset_state():
+    """Reset process-global state between tests."""
     secret_store._warned_fallback = False
+    secret_store._service_name = "code-puppy"
     secret_store._backend_installed = True  # skip lazy install in these tests
     yield
     secret_store._warned_fallback = False
+    secret_store._service_name = "code-puppy"
     secret_store._backend_installed = False
 
 
 @pytest.fixture
 def tmp_fallback(tmp_path, monkeypatch):
-    """Point the fallback file + config dir at a temp location."""
     cfg_dir = tmp_path / "cfg"
     cfg_dir.mkdir()
     fallback = cfg_dir / "secrets.json"
@@ -44,10 +43,8 @@ def tmp_fallback(tmp_path, monkeypatch):
 
 @pytest.fixture
 def working_keyring():
-    """A keyring with a healthy backend and an in-memory store."""
     store: dict[tuple[str, str], str] = {}
     fake = MagicMock()
-
     fake.get_password = MagicMock(
         side_effect=lambda service, name: store.get((service, name))
     )
@@ -63,33 +60,53 @@ def _delete(service, name):
 
     fake.set_password = MagicMock(side_effect=_set)
     fake.delete_password = MagicMock(side_effect=_delete)
-
     backend = MagicMock()
     backend.priority = 10
     fake.get_keyring = MagicMock(return_value=backend)
-
     with patch.object(secret_store, "keyring", fake):
         yield fake, store
 
 
 @pytest.fixture
 def missing_keyring():
-    """A keyring whose backend is unavailable (priority 0, ops raise)."""
     fake = MagicMock()
     fake.get_password = MagicMock(side_effect=Exception("no backend"))
     fake.set_password = MagicMock(side_effect=Exception("no backend"))
     fake.delete_password = MagicMock(side_effect=Exception("no backend"))
-
     backend = MagicMock()
     backend.priority = 0
     fake.get_keyring = MagicMock(return_value=backend)
-
     with patch.object(secret_store, "keyring", fake):
         yield fake
 
 
 # ---------------------------------------------------------------------------
-# 1. keyring_available
+# configure_service_name
+# ---------------------------------------------------------------------------
+
+
+class TestServiceName:
+    def test_default(self):
+        assert secret_store.get_service_name() == "code-puppy"
+
+    def test_override(self):
+        secret_store.configure_service_name("my-custom-distribution")
+        assert secret_store.get_service_name() == "my-custom-distribution"
+
+    def test_rejects_empty(self):
+        with pytest.raises(ValueError, match="non-empty"):
+            secret_store.configure_service_name("  ")
+
+    def test_secrets_land_under_configured_name(self, working_keyring):
+        _, store = working_keyring
+        secret_store.configure_service_name("my-custom-distribution")
+        secret_store.set_secret("tok", "v")
+        assert ("my-custom-distribution", "tok") in store
+        assert ("code-puppy", "tok") not in store
+
+
+# ---------------------------------------------------------------------------
+# keyring_available
 # ---------------------------------------------------------------------------
 
 
@@ -102,7 +119,7 @@ def test_false_for_priority_zero(self, missing_keyring):
 
     def test_true_when_priority_missing(self):
         fake = MagicMock()
-        backend = MagicMock(spec=[])  # no .priority attribute
+        backend = MagicMock(spec=[])
         fake.get_keyring = MagicMock(return_value=backend)
         with patch.object(secret_store, "keyring", fake):
             assert secret_store.keyring_available() is True
@@ -115,7 +132,7 @@ def test_false_when_get_keyring_raises(self):
 
 
 # ---------------------------------------------------------------------------
-# 2. keyring-available path
+# keyring-available path
 # ---------------------------------------------------------------------------
 
 
@@ -123,7 +140,7 @@ class TestKeyringPath:
     def test_set_then_get_roundtrip(self, working_keyring, tmp_fallback):
         _, store = working_keyring
         secret_store.set_secret("my_key", "hunter2")
-        assert store[(secret_store._SERVICE_NAME, "my_key")] == "hunter2"
+        assert store[(secret_store._service_name, "my_key")] == "hunter2"
         assert secret_store.get_secret("my_key") == "hunter2"
 
     def test_get_missing_returns_none(self, working_keyring):
@@ -131,29 +148,173 @@ def test_get_missing_returns_none(self, working_keyring):
 
     def test_get_strips_whitespace(self, working_keyring):
         _, store = working_keyring
-        store[(secret_store._SERVICE_NAME, "k")] = "  spaced  "
+        store[(secret_store._service_name, "k")] = "  spaced  "
         assert secret_store.get_secret("k") == "spaced"
 
     def test_delete_removes_from_keyring(self, working_keyring):
         _, store = working_keyring
-        store[(secret_store._SERVICE_NAME, "k")] = "v"
+        store[(secret_store._service_name, "k")] = "v"
         secret_store.delete_secret("k")
-        assert (secret_store._SERVICE_NAME, "k") not in store
+        assert (secret_store._service_name, "k") not in store
 
     def test_set_does_not_write_fallback_file(self, working_keyring, tmp_fallback):
+        """A successful keyring write must never touch the fallback file."""
         secret_store.set_secret("k", "v")
         assert not os.path.exists(tmp_fallback)
 
-    def test_get_ignores_fallback_when_keyring_healthy(
+    def test_get_falls_through_to_fallback_as_last_resort(
         self, working_keyring, tmp_fallback
     ):
-        # A stale fallback file must not shadow a healthy (empty) keyring.
-        tmp_fallback.write_text(json.dumps({"k": "stale"}))
-        assert secret_store.get_secret("k") is None
+        """When keyring has no entry, the fallback file is consulted.
+
+        This covers recovery from a prior session where both keyring strategies
+        failed and set_secret wrote to the file as a last resort.
+        """
+        tmp_fallback.write_text(json.dumps({"k": "rescued"}))
+        assert secret_store.get_secret("k") == "rescued"
+
+    def test_keyring_value_takes_precedence_over_fallback(
+        self, working_keyring, tmp_fallback
+    ):
+        """Keyring entry wins when both stores have a value for the same key."""
+        _, store = working_keyring
+        store[(secret_store._service_name, "k")] = "from-keyring"
+        tmp_fallback.write_text(json.dumps({"k": "from-file"}))
+        assert secret_store.get_secret("k") == "from-keyring"
+
+    def test_set_warns_and_writes_file_when_keyring_fails(
+        self, working_keyring, tmp_fallback
+    ):
+        """When all keyring writes fail despite a healthy backend, set_secret
+        emits a warning then persists to the fallback file so the secret is
+        not lost."""
+        fake, _ = working_keyring
+        fake.set_password.side_effect = Exception("backend crash")
+
+        with pytest.warns(UserWarning, match="despite a healthy backend"):
+            secret_store.set_secret("k", "v")
+
+        assert tmp_fallback.exists()
+        assert json.loads(tmp_fallback.read_text())["k"] == "v"
+
+    def test_delete_also_cleans_fallback(
+        self, working_keyring, tmp_fallback
+    ):
+        """delete_secret always scrubs the fallback file, in case a prior write
+        landed there as a last resort."""
+        tmp_fallback.write_text(json.dumps({"k": "leftover", "other": "keep"}))
+        secret_store.delete_secret("k")
+        data = json.loads(tmp_fallback.read_text())
+        assert "k" not in data
+        assert data["other"] == "keep"
+
+
+# ---------------------------------------------------------------------------
+# Transparent chunking (Windows Credential Manager size-limit)
+# ---------------------------------------------------------------------------
+
+
+class TestChunking:
+    """Verify that oversized secrets are split into <=_CHUNK_SIZE pieces and
+    reassembled transparently.  Chunking keeps the keyring as the primary store;
+    the file fallback is only reached if chunking itself also fails.
+    """
+
+    def test_large_value_stored_as_chunks(self, working_keyring):
+        """A value that exceeds _CHUNK_SIZE is split into chunk keys."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        big = "A" * (secret_store._CHUNK_SIZE * 2 + 100)  # 3 chunks
+        secret_store.set_secret("tok", big)
+
+        count_key = secret_store._chunk_count_key("tok")
+        assert store.get((svc, count_key)) == "3"
+        for i in range(3):
+            assert (svc, secret_store._chunk_key("tok", i)) in store
+        # Direct entry must NOT exist (unambiguous read path)
+        assert (svc, "tok") not in store
+
+    def test_large_value_roundtrip(self, working_keyring):
+        """get_secret reassembles chunks to return the original value."""
+        big = "Z" * (secret_store._CHUNK_SIZE * 3 + 50)
+        secret_store.set_secret("tok", big)
+        assert secret_store.get_secret("tok") == big
+
+    def test_small_value_uses_direct_entry(self, working_keyring):
+        """Values under the chunk threshold go to the direct key, not chunks."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        secret_store.set_secret("small", "tiny")
+        assert store.get((svc, "small")) == "tiny"
+        assert (svc, secret_store._chunk_count_key("small")) not in store
+
+    def test_stale_chunks_pruned_on_smaller_write(self, working_keyring):
+        """If a secret shrinks from 3 chunks to 2, the 3rd chunk is removed."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        big3 = "B" * (secret_store._CHUNK_SIZE * 2 + 100)  # 3 chunks
+        secret_store.set_secret("tok", big3)
+        assert store.get((svc, secret_store._chunk_count_key("tok"))) == "3"
+
+        big2 = "C" * (secret_store._CHUNK_SIZE + 100)  # 2 chunks
+        secret_store.set_secret("tok", big2)
+        assert store.get((svc, secret_store._chunk_count_key("tok"))) == "2"
+        assert (svc, secret_store._chunk_key("tok", 2)) not in store
+        assert secret_store.get_secret("tok") == big2
+
+    def test_delete_removes_all_chunk_keys(self, working_keyring):
+        """delete_secret wipes the count key and every chunk entry."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        big = "D" * (secret_store._CHUNK_SIZE * 2 + 1)  # 3 chunks
+        secret_store.set_secret("tok", big)
+        secret_store.delete_secret("tok")
+
+        assert (svc, secret_store._chunk_count_key("tok")) not in store
+        for i in range(3):
+            assert (svc, secret_store._chunk_key("tok", i)) not in store
+
+    def test_old_single_entry_still_readable(self, working_keyring):
+        """Pre-chunking entries written without a count key are still readable."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        store[(svc, "legacy")] = "old-value"
+        assert secret_store.get_secret("legacy") == "old-value"
+
+    def test_missing_chunk_returns_none(self, working_keyring):
+        """A partially-written chunk sequence (count present, chunk missing)
+        is treated as absent rather than returning corrupt data."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        store[(svc, secret_store._chunk_count_key("tok"))] = "3"
+        store[(svc, secret_store._chunk_key("tok", 0))] = "part0"
+        # chunk 1 and 2 missing
+        assert secret_store.get_secret("tok") is None
+
+    def test_direct_entry_removed_after_chunked_write(self, working_keyring):
+        """Writing a small value then a large one leaves no direct entry."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        secret_store.set_secret("tok", "small")
+        assert (svc, "tok") in store
+
+        secret_store.set_secret("tok", "X" * (secret_store._CHUNK_SIZE * 2))
+        assert (svc, "tok") not in store
+
+    def test_chunk_keys_cleaned_on_revert_to_small(self, working_keyring):
+        """Writing a large value then a small one removes all chunk keys."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        secret_store.set_secret("tok", "X" * (secret_store._CHUNK_SIZE * 2))
+        assert (svc, secret_store._chunk_count_key("tok")) in store
+
+        secret_store.set_secret("tok", "small")
+        assert (svc, secret_store._chunk_count_key("tok")) not in store
+        assert store.get((svc, "tok")) == "small"
 
 
 # ---------------------------------------------------------------------------
-# 3. keyring-missing / file fallback path
+# keyring-missing / file fallback path
 # ---------------------------------------------------------------------------
 
 
@@ -170,17 +331,23 @@ def test_set_then_get_roundtrip(self, missing_keyring, tmp_fallback):
     def test_get_missing_returns_none(self, missing_keyring, tmp_fallback):
         assert secret_store.get_secret("nope") is None
 
+    @pytest.mark.skipif(
+        sys.platform == "win32",
+        reason="NTFS does not enforce POSIX permission bits via os.chmod",
+    )
     def test_fallback_file_is_0600(self, missing_keyring, tmp_fallback):
         secret_store.set_secret("k", "v")
-        mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
-        assert mode == 0o600
+        assert stat.S_IMODE(os.stat(tmp_fallback).st_mode) == 0o600
 
+    @pytest.mark.skipif(
+        sys.platform == "win32",
+        reason="NTFS does not enforce POSIX permission bits via os.chmod",
+    )
     def test_read_repairs_loose_permissions(self, missing_keyring, tmp_fallback):
         tmp_fallback.write_text(json.dumps({"k": "v"}))
         os.chmod(tmp_fallback, 0o644)
         assert secret_store.get_secret("k") == "v"
-        mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
-        assert mode == 0o600
+        assert stat.S_IMODE(os.stat(tmp_fallback).st_mode) == 0o600
 
     def test_set_blank_is_ignored(self, missing_keyring, tmp_fallback):
         secret_store.set_secret("k", "   ")
@@ -194,32 +361,24 @@ def test_delete_removes_from_fallback(self, missing_keyring, tmp_fallback):
         assert "k" not in data
         assert data["keep"] == "me"
 
-    def test_set_preserves_existing_keys(self, missing_keyring, tmp_fallback):
-        secret_store.set_secret("a", "1")
-        secret_store.set_secret("b", "2")
-        data = json.loads(tmp_fallback.read_text())
-        assert data == {"a": "1", "b": "2"}
-
-    def test_corrupt_fallback_file_is_tolerated(self, missing_keyring, tmp_fallback):
+    def test_corrupt_fallback_tolerated(self, missing_keyring, tmp_fallback):
         tmp_fallback.write_text("{ not valid json")
         assert secret_store.get_secret("k") is None
-        # A subsequent write recovers the file.
         secret_store.set_secret("k", "v")
         assert secret_store.get_secret("k") == "v"
 
-    def test_fallback_emits_warning_once(self, missing_keyring, tmp_fallback):
-        with pytest.warns(UserWarning, match="fallback"):
-            secret_store.set_secret("k", "v")
-        # Second op must not warn again (one-shot guard).
+    def test_fallback_warns_once(self, missing_keyring, tmp_fallback):
         import warnings as _w
 
+        with pytest.warns(UserWarning, match="fallback"):
+            secret_store.set_secret("k", "v")
         with _w.catch_warnings():
             _w.simplefilter("error")
-            secret_store.set_secret("k2", "v2")  # would raise if it warned
+            secret_store.set_secret("k2", "v2")
 
 
 # ---------------------------------------------------------------------------
-# 4. Cross-platform fixes
+# Cross-platform fixes
 # ---------------------------------------------------------------------------
 
 
@@ -235,26 +394,13 @@ def test_ensure_backend_survives_import_error(self):
             secret_store._ensure_backend()
         assert secret_store._backend_installed is True
 
-    def test_write_fallback_uses_chmod(self, tmp_fallback, monkeypatch):
+    @pytest.mark.skipif(
+        sys.platform == "win32",
+        reason="NTFS does not enforce POSIX permission bits via os.chmod",
+    )
+    def test_write_fallback_uses_chmod(self, tmp_fallback):
         """_write_fallback uses os.chmod (cross-platform), not os.fchmod."""
         assert secret_store._write_fallback({"k": "v"}) is True
         assert json.loads(tmp_fallback.read_text()) == {"k": "v"}
         mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
         assert mode == 0o600
-
-    def test_set_warns_on_transient_keyring_failure(self, tmp_fallback):
-        """When keyring is healthy but the write fails, set_secret warns
-        instead of silently stranding the secret in the fallback file."""
-        fake = MagicMock()
-        fake.set_password = MagicMock(side_effect=Exception("transient"))
-        fake.get_password = MagicMock(return_value=None)
-
-        backend = MagicMock()
-        backend.priority = 10  # healthy
-        fake.get_keyring = MagicMock(return_value=backend)
-
-        with patch.object(secret_store, "keyring", fake):
-            with pytest.warns(UserWarning, match="Keyring write failed"):
-                secret_store.set_secret("k", "v")
-        # Must NOT have written to the fallback file.
-        assert not tmp_fallback.exists()
diff --git a/tests/test_secret_store_backends.py b/tests/test_secret_store_backends.py
index a8d62b61d..6f09933a1 100644
--- a/tests/test_secret_store_backends.py
+++ b/tests/test_secret_store_backends.py
@@ -8,11 +8,20 @@
 """
 
 import json
+import sys
 from unittest.mock import MagicMock, patch
 
 import pytest
 
-from code_puppy import secret_store_backends as ssb
+# secret_store_backends imports fcntl which is POSIX-only. Skip the entire
+# module on Windows before attempting the import so collection doesn't crash.
+if sys.platform == "win32":
+    pytest.skip(
+        "secret_store_backends uses fcntl (POSIX-only); not applicable on Windows",
+        allow_module_level=True,
+    )
+
+from code_puppy import secret_store_backends as ssb  # noqa: E402
 
 
 # ---------------------------------------------------------------------------

From a78a30e626047927983a71458fa804b34f53c3c9 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 16:48:05 -0400
Subject: [PATCH 06/16] style: apply ruff format to secret_store and tests

---
 code_puppy/secret_store.py | 3 +--
 tests/test_secret_store.py | 8 ++------
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index a8923a08c..21cfaba95 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -259,8 +259,7 @@ def _keyring_set(name: str, value: str) -> bool:
 
     # --- Chunked write path ---
     chunks = [
-        normalized[i: i + _CHUNK_SIZE]
-        for i in range(0, len(normalized), _CHUNK_SIZE)
+        normalized[i : i + _CHUNK_SIZE] for i in range(0, len(normalized), _CHUNK_SIZE)
     ]
 
     # Write data entries first.
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index da7028719..909a4d815 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -197,9 +197,7 @@ def test_set_warns_and_writes_file_when_keyring_fails(
         assert tmp_fallback.exists()
         assert json.loads(tmp_fallback.read_text())["k"] == "v"
 
-    def test_delete_also_cleans_fallback(
-        self, working_keyring, tmp_fallback
-    ):
+    def test_delete_also_cleans_fallback(self, working_keyring, tmp_fallback):
         """delete_secret always scrubs the fallback file, in case a prior write
         landed there as a last resort."""
         tmp_fallback.write_text(json.dumps({"k": "leftover", "other": "keep"}))
@@ -387,9 +385,7 @@ def test_ensure_backend_survives_import_error(self):
         """_ensure_backend must not crash when secret_store_backends is
         unimportable (e.g. Windows where fcntl doesn't exist)."""
         secret_store._backend_installed = False
-        with patch.dict(
-            "sys.modules", {"code_puppy.secret_store_backends": None}
-        ):
+        with patch.dict("sys.modules", {"code_puppy.secret_store_backends": None}):
             # Should complete without raising.
             secret_store._ensure_backend()
         assert secret_store._backend_installed is True

From e1e2dfcd574ba18e8f6afb6a9363656b9d892095 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 17:41:32 -0400
Subject: [PATCH 07/16] fix(secret-store): enforce reserved ':cp:' namespace on
 secret names (F9)

Caller-supplied names containing ':cp:' could shadow a real secret's chunk
metadata or, via delete_secret, destroy an unrelated entry. Add _validate_name
(also rejecting empty names) and call it at the top of get/set/delete_secret.
Introduces SecretStoreError for later failure-contract work.

Addresses review finding: discussion_r3554814765
---
 code_puppy/secret_store.py | 33 +++++++++++++++++++++++++++++++++
 tests/test_secret_store.py | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index 21cfaba95..e9a59d5d6 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -120,6 +120,36 @@ def configure_service_name(name: str) -> None:
 _COUNT_SUFFIX = ":cp:n"
 
 
+class SecretStoreError(RuntimeError):
+    """Raised when a secret cannot be persisted or removed as requested.
+
+    Distinct from a *missing* secret (``get_secret`` returns ``None``): this
+    signals an active failure -- e.g. a fallback write to a read-only or full
+    filesystem -- so callers never mistake a lost credential for success.
+    """
+
+
+def _validate_name(name: str) -> str:
+    """Validate a caller-supplied secret name.
+
+    The chunk machinery reserves the ``:cp:`` token to build internal entry
+    names (``<name>:cp:<i>`` for chunks, ``<name>:cp:n`` for the commit
+    marker).  A caller-supplied name containing ``:cp:`` could therefore
+    shadow a real secret's chunk metadata or, via ``delete_secret``, destroy
+    an unrelated entry.  Because this module is a generic store whose names
+    may be built from user- or config-derived strings, we reject the reserved
+    token outright rather than trust that "nobody would name a secret that."
+    """
+    if not isinstance(name, str) or not name:
+        raise ValueError("secret name must be a non-empty string")
+    if _CHUNK_NS in name:
+        raise ValueError(
+            f"secret name {name!r} contains the reserved substring "
+            f"{_CHUNK_NS!r}; it is used internally for chunk metadata"
+        )
+    return name
+
+
 def _chunk_count_key(name: str) -> str:
     """Keyring entry name for the chunk-count (commit) marker."""
     return f"{name}{_COUNT_SUFFIX}"
@@ -372,6 +402,7 @@ def get_secret(name: str) -> str | None:
          resort so secrets written there by a previous session (after
          exhausting both keyring options) are still recoverable.
     """
+    _validate_name(name)
     _ensure_backend()
     value = _keyring_get(name)
     if value:
@@ -398,6 +429,7 @@ def set_secret(name: str, value: str) -> None:
       3. Permission-hardened JSON file fallback (only when both keyring
          strategies fail -- unexpected backend error or no keyring at all).
     """
+    _validate_name(name)
     _ensure_backend()
     if _keyring_set(name, value):
         return
@@ -426,6 +458,7 @@ def set_secret(name: str, value: str) -> None:
 
 def delete_secret(name: str) -> None:
     """Best-effort removal of a secret from keyring (and chunks) and fallback."""
+    _validate_name(name)
     _ensure_backend()
     _keyring_delete(name)
 
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 909a4d815..5eae3b9b8 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -400,3 +400,37 @@ def test_write_fallback_uses_chmod(self, tmp_fallback):
         assert json.loads(tmp_fallback.read_text()) == {"k": "v"}
         mode = stat.S_IMODE(os.stat(tmp_fallback).st_mode)
         assert mode == 0o600
+
+
+# ---------------------------------------------------------------------------
+# F9 -- reserved ':cp:' namespace is enforced on caller-supplied names
+# ---------------------------------------------------------------------------
+
+
+class TestReservedNamespace:
+    """A caller name containing ':cp:' must be rejected before it can shadow
+    or destroy chunk metadata (PR #531 review finding F9)."""
+
+    def test_set_rejects_reserved_substring(self, working_keyring):
+        with pytest.raises(ValueError, match="reserved substring"):
+            secret_store.set_secret("foo:cp:n", "3")
+
+    def test_get_rejects_reserved_substring(self, working_keyring):
+        with pytest.raises(ValueError, match="reserved substring"):
+            secret_store.get_secret("foo:cp:0")
+
+    def test_delete_rejects_reserved_substring(self, working_keyring):
+        with pytest.raises(ValueError, match="reserved substring"):
+            secret_store.delete_secret("foo:cp:n")
+
+    def test_empty_name_rejected(self, working_keyring):
+        with pytest.raises(ValueError, match="non-empty string"):
+            secret_store.set_secret("", "v")
+
+    def test_shadow_attack_cannot_poison_count_marker(self, working_keyring):
+        """Rejecting 'foo:cp:n' means a real 'foo' entry can't be shadowed by
+        a bogus chunk-count marker."""
+        secret_store.set_secret("foo", "legit")
+        with pytest.raises(ValueError):
+            secret_store.set_secret("foo:cp:n", "3")
+        assert secret_store.get_secret("foo") == "legit"

From 9de46228379f27c030be198dec4e14ec61d5a48e Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 17:46:07 -0400
Subject: [PATCH 08/16] fix(secret-store): stop stripping secrets; reject empty
 values loudly (F8)

Two related normalization bugs:
- set_secret('token', '  ') hit an early return, so _keyring_set reported
  False -- indistinguishable from a real backend failure -- and set_secret
  then warned misleadingly about a 'healthy backend' write failure.
- Universal .strip() on write and read silently mutated secrets that carry
  legitimate leading/trailing whitespace.

Now: _validate_value rejects empty/whitespace-only input with ValueError at
the public boundary, and all secret payloads are stored/returned verbatim.
Stripping is confined to config values (service name) and int() parsing of
the chunk-count marker, which tolerates whitespace anyway.

Addresses review finding: discussion_r3554814761
---
 code_puppy/secret_store.py | 56 ++++++++++++++++++++++++--------------
 tests/test_secret_store.py | 48 +++++++++++++++++++++++++++++---
 2 files changed, 80 insertions(+), 24 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index e9a59d5d6..f07e51cce 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -150,6 +150,23 @@ def _validate_name(name: str) -> str:
     return name
 
 
+def _validate_value(value: str) -> str:
+    """Validate a caller-supplied secret value.
+
+    Empty or whitespace-only values are rejected with a ``ValueError`` so an
+    empty write can never be confused with a backend failure (the old code
+    silently no-oped and then emitted a misleading "keyring write failed"
+    warning).  Values with *content* plus surrounding whitespace are allowed
+    and stored verbatim -- secrets with significant leading/trailing
+    whitespace exist and must not be silently mutated.
+    """
+    if not isinstance(value, str):
+        raise ValueError("secret value must be a string")
+    if not value.strip():
+        raise ValueError("secret value must be non-empty")
+    return value
+
+
 def _chunk_count_key(name: str) -> str:
     """Keyring entry name for the chunk-count (commit) marker."""
     return f"{name}{_COUNT_SUFFIX}"
@@ -187,15 +204,19 @@ def keyring_available() -> bool:
 
 
 def _kr_get_raw(name: str) -> str | None:
-    """Read one keyring entry; ``None`` on any error or absence."""
+    """Read one keyring entry verbatim; ``None`` on error or absence.
+
+    The stored value is returned byte-for-byte (no ``.strip()``): a secret
+    with legitimate leading/trailing whitespace must round-trip unchanged.
+    A truly empty string is treated as absence.
+    """
     try:
         value = keyring.get_password(_service_name, name)
     except Exception:
         return None
-    if value is None:
+    if not value:
         return None
-    normalized = str(value).strip()
-    return normalized or None
+    return str(value)
 
 
 def _kr_set_raw(name: str, value: str) -> bool:
@@ -276,21 +297,19 @@ def _keyring_set(name: str, value: str) -> bool:
       - The direct *name* entry is removed to avoid ambiguity on read.
 
     Returns ``True`` on success, ``False`` if any keyring write fails.
-    """
-    normalized = str(value).strip()
-    if not normalized:
-        return False
 
-    if len(normalized) <= _CHUNK_SIZE:
-        if not _kr_set_raw(name, normalized):
+    The value is stored verbatim; callers reject empty/whitespace-only input
+    at the public boundary (``set_secret``), so a ``False`` here always means
+    a genuine backend failure -- never "you passed an empty string."
+    """
+    if len(value) <= _CHUNK_SIZE:
+        if not _kr_set_raw(name, value):
             return False
         _delete_chunks(name)  # clean up any old chunked write
         return True
 
     # --- Chunked write path ---
-    chunks = [
-        normalized[i : i + _CHUNK_SIZE] for i in range(0, len(normalized), _CHUNK_SIZE)
-    ]
+    chunks = [value[i : i + _CHUNK_SIZE] for i in range(0, len(value), _CHUNK_SIZE)]
 
     # Write data entries first.
     for idx, chunk in enumerate(chunks):
@@ -414,10 +433,9 @@ def get_secret(name: str) -> str | None:
     if not keyring_available():
         _warn_fallback_active()
     stored = _read_fallback().get(name)
-    if stored is None:
+    if not stored:
         return None
-    normalized = str(stored).strip()
-    return normalized or None
+    return str(stored)
 
 
 def set_secret(name: str, value: str) -> None:
@@ -430,6 +448,7 @@ def set_secret(name: str, value: str) -> None:
          strategies fail -- unexpected backend error or no keyring at all).
     """
     _validate_name(name)
+    _validate_value(value)
     _ensure_backend()
     if _keyring_set(name, value):
         return
@@ -448,11 +467,8 @@ def set_secret(name: str, value: str) -> None:
     else:
         _warn_fallback_active()
 
-    normalized = str(value).strip()
-    if not normalized:
-        return
     data = _read_fallback()
-    data[name] = normalized
+    data[name] = value
     _write_fallback(data)
 
 
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 5eae3b9b8..43cf60013 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -146,10 +146,12 @@ def test_set_then_get_roundtrip(self, working_keyring, tmp_fallback):
     def test_get_missing_returns_none(self, working_keyring):
         assert secret_store.get_secret("nope") is None
 
-    def test_get_strips_whitespace(self, working_keyring):
+    def test_get_preserves_whitespace(self, working_keyring):
+        """Secrets are stored/returned verbatim -- leading/trailing whitespace
+        that is part of the value must survive the round-trip (F8)."""
         _, store = working_keyring
         store[(secret_store._service_name, "k")] = "  spaced  "
-        assert secret_store.get_secret("k") == "spaced"
+        assert secret_store.get_secret("k") == "  spaced  "
 
     def test_delete_removes_from_keyring(self, working_keyring):
         _, store = working_keyring
@@ -347,8 +349,11 @@ def test_read_repairs_loose_permissions(self, missing_keyring, tmp_fallback):
         assert secret_store.get_secret("k") == "v"
         assert stat.S_IMODE(os.stat(tmp_fallback).st_mode) == 0o600
 
-    def test_set_blank_is_ignored(self, missing_keyring, tmp_fallback):
-        secret_store.set_secret("k", "   ")
+    def test_set_blank_raises(self, missing_keyring, tmp_fallback):
+        """Empty/whitespace-only values raise ValueError instead of silently
+        no-oping and emitting a misleading backend-failure warning (F8)."""
+        with pytest.raises(ValueError, match="non-empty"):
+            secret_store.set_secret("k", "   ")
         assert not tmp_fallback.exists()
 
     def test_delete_removes_from_fallback(self, missing_keyring, tmp_fallback):
@@ -434,3 +439,38 @@ def test_shadow_attack_cannot_poison_count_marker(self, working_keyring):
         with pytest.raises(ValueError):
             secret_store.set_secret("foo:cp:n", "3")
         assert secret_store.get_secret("foo") == "legit"
+
+
+# ---------------------------------------------------------------------------
+# F8 -- empty value raises; whitespace-bearing values are preserved verbatim
+# ---------------------------------------------------------------------------
+
+
+class TestValueNormalization:
+    def test_empty_string_raises(self, working_keyring):
+        with pytest.raises(ValueError, match="non-empty"):
+            secret_store.set_secret("k", "")
+
+    def test_whitespace_only_raises(self, working_keyring):
+        with pytest.raises(ValueError, match="non-empty"):
+            secret_store.set_secret("k", "\t \n")
+
+    def test_surrounding_whitespace_preserved_keyring(self, working_keyring):
+        secret_store.set_secret("k", "  tok-with-spaces  ")
+        assert secret_store.get_secret("k") == "  tok-with-spaces  "
+
+    def test_surrounding_whitespace_preserved_fallback(
+        self, missing_keyring, tmp_fallback
+    ):
+        with pytest.warns(UserWarning):
+            secret_store.set_secret("k", "  tok  ")
+        assert secret_store.get_secret("k") == "  tok  "
+
+    def test_no_false_alarm_warning_on_empty(self, working_keyring, tmp_fallback):
+        """An empty value must not reach the 'keyring write failed' path."""
+        import warnings as _w
+
+        with _w.catch_warnings():
+            _w.simplefilter("error")
+            with pytest.raises(ValueError):
+                secret_store.set_secret("k", "  ")

From 9e91e9ad210487ae2c1087e112d11810a41226ee Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 17:49:53 -0400
Subject: [PATCH 09/16] fix(secret-store): uniform fallback failure contract,
 surfaced to callers (F3/F4/F10)

F3: tempfile.mkstemp() sat outside the try, so its failure raised a raw
OSError while every other failure returned False. Moved it inside the try
so _write_fallback has one uniform False-on-any-failure contract.

F4: set_secret() ignored the _write_fallback() result and returned normally
after losing the credential. It now raises SecretStoreError when the keyring
is unavailable AND the fallback write fails, so a read-only/full filesystem
can no longer masquerade as success.

F10: delete_secret() had the same swallowed-return bug -- worse for a delete,
since 'delete' would report success while the plaintext secret survived. It
now raises SecretStoreError when a required scrub write fails (and stays
silent when there was nothing to scrub).

Addresses review findings: discussion_r3554790360, discussion_r3554790363,
and the delete_secret note in pullrequestreview-4666753894
---
 code_puppy/secret_store.py | 37 +++++++++++++++++++++++++++++--------
 tests/test_secret_store.py | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 65 insertions(+), 8 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index f07e51cce..e3baabf83 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -384,14 +384,19 @@ def _write_fallback(data: dict[str, str]) -> bool:
     Writes to a temp file in the same directory, ``chmod`` s it before it
     ever holds content the target will keep, then ``os.replace`` s it into
     place so a crash mid-write can never leave a truncated secrets file.
+
+    Returns ``True`` on success and ``False`` on *any* failure (including a
+    ``mkstemp`` that fails on a read-only or full filesystem).  The contract
+    is uniform so callers can act on it -- see ``set_secret``/``delete_secret``.
     """
     try:
         os.makedirs(CONFIG_DIR, exist_ok=True)
     except OSError:
         return False
 
-    fd, tmp = tempfile.mkstemp(dir=CONFIG_DIR, suffix=".tmp")
+    tmp = None
     try:
+        fd, tmp = tempfile.mkstemp(dir=CONFIG_DIR, suffix=".tmp")
         os.chmod(tmp, _FALLBACK_MODE)
         with os.fdopen(fd, "w", encoding="utf-8") as f:
             json.dump(data, f, indent=2)
@@ -399,10 +404,11 @@ def _write_fallback(data: dict[str, str]) -> bool:
             os.fsync(f.fileno())
         os.replace(tmp, _FALLBACK_FILE)
     except OSError:
-        try:
-            os.unlink(tmp)
-        except OSError:
-            pass
+        if tmp is not None:
+            try:
+                os.unlink(tmp)
+            except OSError:
+                pass
         return False
     return True
 
@@ -469,11 +475,21 @@ def set_secret(name: str, value: str) -> None:
 
     data = _read_fallback()
     data[name] = value
-    _write_fallback(data)
+    if not _write_fallback(data):
+        raise SecretStoreError(
+            f"Failed to persist secret {name!r}: the OS keyring is unavailable "
+            f"and the fallback file at {_FALLBACK_FILE} could not be written "
+            "(read-only or full filesystem?). The secret was NOT saved."
+        )
 
 
 def delete_secret(name: str) -> None:
-    """Best-effort removal of a secret from keyring (and chunks) and fallback."""
+    """Best-effort removal of a secret from keyring (and chunks) and fallback.
+
+    Raises ``SecretStoreError`` if the fallback file holds the secret but
+    cannot be rewritten to scrub it -- otherwise "delete" would report success
+    while the plaintext secret survives on disk.
+    """
     _validate_name(name)
     _ensure_backend()
     _keyring_delete(name)
@@ -483,4 +499,9 @@ def delete_secret(name: str) -> None:
     data = _read_fallback()
     if name in data:
         del data[name]
-        _write_fallback(data)
+        if not _write_fallback(data):
+            raise SecretStoreError(
+                f"Failed to remove secret {name!r} from the fallback file at "
+                f"{_FALLBACK_FILE} (read-only or full filesystem?). The "
+                "plaintext secret may still be present on disk."
+            )
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 43cf60013..370dd6e23 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -474,3 +474,39 @@ def test_no_false_alarm_warning_on_empty(self, working_keyring, tmp_fallback):
             _w.simplefilter("error")
             with pytest.raises(ValueError):
                 secret_store.set_secret("k", "  ")
+
+
+# ---------------------------------------------------------------------------
+# F3/F4/F10 -- uniform fallback failure contract; callers surface failures
+# ---------------------------------------------------------------------------
+
+
+class TestFallbackFailureContract:
+    def test_write_fallback_returns_false_on_mkstemp_error(self, tmp_fallback):
+        """F3: a mkstemp failure returns False instead of raising raw OSError."""
+        with patch("tempfile.mkstemp", side_effect=OSError("read-only fs")):
+            assert secret_store._write_fallback({"k": "v"}) is False
+
+    def test_write_fallback_returns_false_on_replace_error(self, tmp_fallback):
+        with patch("os.replace", side_effect=OSError("disk full")):
+            assert secret_store._write_fallback({"k": "v"}) is False
+
+    def test_set_raises_when_fallback_write_fails(self, missing_keyring, tmp_fallback):
+        """F4: a lost credential must not report success."""
+        with patch.object(secret_store, "_write_fallback", return_value=False):
+            with pytest.warns(UserWarning):
+                with pytest.raises(secret_store.SecretStoreError, match="NOT saved"):
+                    secret_store.set_secret("k", "v")
+
+    def test_delete_raises_when_scrub_write_fails(self, missing_keyring, tmp_fallback):
+        """F10: a failed scrub must not report a successful delete."""
+        tmp_fallback.write_text(json.dumps({"k": "leftover"}))
+        with patch.object(secret_store, "_write_fallback", return_value=False):
+            with pytest.raises(secret_store.SecretStoreError, match="still be present"):
+                secret_store.delete_secret("k")
+
+    def test_delete_absent_key_does_not_raise(self, missing_keyring, tmp_fallback):
+        """No fallback entry -> nothing to scrub -> no error even if write would fail."""
+        tmp_fallback.write_text(json.dumps({"other": "keep"}))
+        with patch.object(secret_store, "_write_fallback", return_value=False):
+            secret_store.delete_secret("k")  # must not raise

From 202b31f772648563124fd185b1162bd5496e6b2b Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 17:56:30 -0400
Subject: [PATCH 10/16] fix(secret-store): lock fallback RMW + scrub stale
 plaintext on healthy set (F5/F6)

F5: the fallback read-modify-write was unlocked, so two processes (main app
+ MCP subprocess) could each read the document, add a different key, and the
second writer would clobber the first (lost update). Added a cross-platform
advisory lock (fcntl on POSIX, msvcrt on Windows -- the platform where the
chunked path actually activates) plus an in-process thread lock, wrapping the
entire read->mutate->write cycle via new _fallback_set/_fallback_delete
helpers.

F6: a successful keyring write never scrubbed the fallback, so a rotated-out
secret lingered in plaintext forever and could be silently resurrected if the
keyring entry later vanished. set_secret now calls _fallback_scrub(name) after
a healthy keyring write (best-effort under the same lock; warns but does not
fail the set, since the keyring already holds the source of truth).

Addresses review findings: discussion_r3554790366, discussion_r3554814753
---
 code_puppy/secret_store.py | 128 +++++++++++++++++++++++++++++++++----
 tests/test_secret_store.py |  61 ++++++++++++++++++
 2 files changed, 177 insertions(+), 12 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index e3baabf83..0262cfa02 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -37,9 +37,11 @@
 
 from __future__ import annotations
 
+import contextlib
 import json
 import os
 import tempfile
+import threading
 import warnings
 
 import keyring
@@ -347,6 +349,64 @@ def _keyring_delete(name: str) -> bool:
 # ---------------------------------------------------------------------------
 
 
+# Cross-process advisory lock guarding the fallback read-modify-write. Atomic
+# os.replace() prevents truncated reads but not *lost updates*: two processes
+# (e.g. the main app and an MCP subprocess) can each read the document, add a
+# different key, and the second writer clobbers the first. The lock serializes
+# the whole read->mutate->write cycle. The chunked keyring path activates on
+# Windows, so this lock is cross-platform (fcntl on POSIX, msvcrt on Windows).
+_FALLBACK_LOCK_FILE = os.path.join(CONFIG_DIR, ".secrets.lock")
+_fallback_thread_lock = threading.RLock()
+
+try:
+    import fcntl
+
+    def _lock_fd(fd: int) -> None:
+        fcntl.flock(fd, fcntl.LOCK_EX)
+
+    def _unlock_fd(fd: int) -> None:
+        fcntl.flock(fd, fcntl.LOCK_UN)
+except ImportError:  # pragma: no cover - platform-specific (Windows)
+    import msvcrt
+
+    def _lock_fd(fd: int) -> None:
+        # Byte-range lock on the first byte; blocks until acquired.
+        msvcrt.locking(fd, msvcrt.LK_LOCK, 1)
+
+    def _unlock_fd(fd: int) -> None:
+        try:
+            os.lseek(fd, 0, os.SEEK_SET)
+            msvcrt.locking(fd, msvcrt.LK_UNLCK, 1)
+        except OSError:
+            pass
+
+
+@contextlib.contextmanager
+def _fallback_lock():
+    """Serialize the fallback read-modify-write across threads and processes.
+
+    Best-effort: if the lock file cannot be created (e.g. read-only config
+    dir), we degrade to the in-process thread lock alone rather than wedge
+    secret operations. The subsequent _write_fallback will surface any real
+    persistence failure.
+    """
+    with _fallback_thread_lock:
+        try:
+            os.makedirs(CONFIG_DIR, exist_ok=True)
+            fd = os.open(_FALLBACK_LOCK_FILE, os.O_CREAT | os.O_RDWR, _FALLBACK_MODE)
+        except OSError:
+            yield
+            return
+        try:
+            _lock_fd(fd)
+            yield
+        finally:
+            try:
+                _unlock_fd(fd)
+            finally:
+                os.close(fd)
+
+
 def _warn_fallback_active() -> None:
     global _warned_fallback
     if _warned_fallback:
@@ -413,6 +473,52 @@ def _write_fallback(data: dict[str, str]) -> bool:
     return True
 
 
+def _fallback_set(name: str, value: str) -> bool:
+    """Add/update a fallback entry under the cross-process lock (F5)."""
+    with _fallback_lock():
+        data = _read_fallback()
+        data[name] = value
+        return _write_fallback(data)
+
+
+def _fallback_delete(name: str) -> bool | None:
+    """Remove a fallback entry under the lock.
+
+    Returns ``True`` on a successful scrub, ``False`` if the rewrite failed,
+    and ``None`` when there was nothing to remove (so callers don't treat a
+    no-op as a failure).
+    """
+    with _fallback_lock():
+        data = _read_fallback()
+        if name not in data:
+            return None
+        del data[name]
+        return _write_fallback(data)
+
+
+def _fallback_scrub(name: str) -> None:
+    """Best-effort removal of a stale fallback entry after a healthy keyring
+    write (F6).
+
+    A successful keyring write must not leave a rotated-out secret sitting in
+    plaintext on disk, where it could later be resurrected if the keyring
+    entry vanishes. The keyring already holds the source of truth here, so a
+    failure to rewrite the file is not fatal -- but it is worth surfacing.
+    """
+    with _fallback_lock():
+        data = _read_fallback()
+        if name not in data:
+            return
+        del data[name]
+        if not _write_fallback(data):
+            warnings.warn(
+                f"Secret {name!r} was written to the keyring but its stale "
+                f"plaintext copy in {_FALLBACK_FILE} could not be removed "
+                "(read-only or full filesystem?).",
+                stacklevel=2,
+            )
+
+
 # ---------------------------------------------------------------------------
 # Public high-level API
 # ---------------------------------------------------------------------------
@@ -457,6 +563,9 @@ def set_secret(name: str, value: str) -> None:
     _validate_value(value)
     _ensure_backend()
     if _keyring_set(name, value):
+        # F6: scrub any stale plaintext copy so a rotated secret can't linger
+        # on disk and be resurrected later if the keyring entry vanishes.
+        _fallback_scrub(name)
         return
 
     if keyring_available():
@@ -473,9 +582,7 @@ def set_secret(name: str, value: str) -> None:
     else:
         _warn_fallback_active()
 
-    data = _read_fallback()
-    data[name] = value
-    if not _write_fallback(data):
+    if not _fallback_set(name, value):
         raise SecretStoreError(
             f"Failed to persist secret {name!r}: the OS keyring is unavailable "
             f"and the fallback file at {_FALLBACK_FILE} could not be written "
@@ -496,12 +603,9 @@ def delete_secret(name: str) -> None:
 
     # Always scrub the fallback file: a prior write may have ended up there
     # even on a system where the keyring is now healthy.
-    data = _read_fallback()
-    if name in data:
-        del data[name]
-        if not _write_fallback(data):
-            raise SecretStoreError(
-                f"Failed to remove secret {name!r} from the fallback file at "
-                f"{_FALLBACK_FILE} (read-only or full filesystem?). The "
-                "plaintext secret may still be present on disk."
-            )
+    if _fallback_delete(name) is False:
+        raise SecretStoreError(
+            f"Failed to remove secret {name!r} from the fallback file at "
+            f"{_FALLBACK_FILE} (read-only or full filesystem?). The "
+            "plaintext secret may still be present on disk."
+        )
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 370dd6e23..25ea66fce 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -510,3 +510,64 @@ def test_delete_absent_key_does_not_raise(self, missing_keyring, tmp_fallback):
         tmp_fallback.write_text(json.dumps({"other": "keep"}))
         with patch.object(secret_store, "_write_fallback", return_value=False):
             secret_store.delete_secret("k")  # must not raise
+
+
+# ---------------------------------------------------------------------------
+# F5 -- locked read-modify-write; F6 -- scrub stale fallback on healthy set
+# ---------------------------------------------------------------------------
+
+
+class TestFallbackLockingAndScrub:
+    def test_concurrent_writers_no_lost_update(self, missing_keyring, tmp_fallback):
+        """F5: many threads each add a distinct key; the lock must prevent
+        lost updates so every key survives the read-modify-write races."""
+        import threading
+
+        import warnings as _w
+
+        def writer(i):
+            with _w.catch_warnings():
+                _w.simplefilter("ignore")
+                secret_store.set_secret(f"key{i}", f"val{i}")
+
+        threads = [threading.Thread(target=writer, args=(i,)) for i in range(25)]
+        for t in threads:
+            t.start()
+        for t in threads:
+            t.join()
+
+        data = json.loads(tmp_fallback.read_text())
+        for i in range(25):
+            assert data[f"key{i}"] == f"val{i}"
+
+    def test_healthy_set_scrubs_fallback(self, working_keyring, tmp_fallback):
+        """F6: a successful keyring write removes any stale plaintext copy."""
+        tmp_fallback.write_text(json.dumps({"tok": "OLD", "other": "keep"}))
+        secret_store.set_secret("tok", "NEW")
+
+        # keyring now holds the new value...
+        assert secret_store.get_secret("tok") == "NEW"
+        # ...and the stale plaintext copy is gone, unrelated entries preserved.
+        data = json.loads(tmp_fallback.read_text())
+        assert "tok" not in data
+        assert data["other"] == "keep"
+
+    def test_healthy_set_no_resurrection(self, working_keyring, tmp_fallback):
+        """F6: after a healthy set scrubs the file, a vanished keyring entry
+        must not resurrect the old plaintext value."""
+        _, store = working_keyring
+        tmp_fallback.write_text(json.dumps({"tok": "OLD"}))
+        secret_store.set_secret("tok", "NEW")
+        # Simulate the keyring entry vanishing (reset keychain / new profile).
+        store.clear()
+        assert secret_store.get_secret("tok") is None
+
+    def test_scrub_failure_warns_but_does_not_raise(
+        self, working_keyring, tmp_fallback
+    ):
+        """F6: if the stale-copy scrub can't be written, warn -- but the set
+        still succeeded (keyring holds the truth), so don't raise."""
+        tmp_fallback.write_text(json.dumps({"tok": "OLD"}))
+        with patch.object(secret_store, "_write_fallback", return_value=False):
+            with pytest.warns(UserWarning, match="stale"):
+                secret_store.set_secret("tok", "NEW")  # must not raise

From 13a18b03e758ba99af7b56f4346fde667069f7d3 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 18:01:07 -0400
Subject: [PATCH 11/16] fix(secret-store): namespace the fallback file by
 service name (F2)

The keyring tier scoped entries by _service_name, but the fallback was a flat
{name: value} dict indexed only by secret name. In fallback mode, distribution
A could read, overwrite, or delete distribution B's secrets -- the isolation
promised by configure_service_name() did not hold for the file tier.

The on-disk shape is now {service_name: {name: value}}. _read_fallback_doc()
reads the full nested document (and transparently migrates a legacy flat file
under the default 'code-puppy' service so historical secrets stay readable
without leaking into another namespace). _read_fallback() returns just the
current service's slice, and the locked _fallback_set/_delete/_scrub helpers
mutate only that slice, pruning empty service buckets.

Addresses review finding: discussion_r3554790358
---
 code_puppy/secret_store.py | 70 ++++++++++++++++++++++++++++----------
 tests/test_secret_store.py | 63 ++++++++++++++++++++++++++++++----
 2 files changed, 109 insertions(+), 24 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index 0262cfa02..81207d1ae 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -53,6 +53,11 @@
 # secrets never bleed across builds.
 _service_name = "code-puppy"
 
+# The default service namespace.  A legacy flat fallback file (pre per-service
+# scoping) is migrated under this name so it stays readable on the default
+# build without leaking into another distribution's namespace.
+_DEFAULT_SERVICE = "code-puppy"
+
 # Permission-hardened JSON fallback used only when the keyring backend is
 # unavailable (headless boxes, minimal CI containers, etc.).
 _FALLBACK_FILE = os.path.join(CONFIG_DIR, "secrets.json")
@@ -420,8 +425,17 @@ def _warn_fallback_active() -> None:
     )
 
 
-def _read_fallback() -> dict[str, str]:
-    """Read the fallback secrets file, repairing its permissions on read."""
+def _read_fallback_doc() -> dict[str, dict[str, str]]:
+    """Read the raw fallback file as a service-namespaced document.
+
+    The on-disk shape is ``{service_name: {secret_name: value}}`` so each
+    distribution's fallback secrets are isolated the same way keyring entries
+    are (F2).  Permissions are repaired on read.  A legacy *flat*
+    ``{secret_name: value}`` file (written before per-service scoping existed)
+    is migrated under the default service namespace so historical secrets stay
+    readable under the default build without leaking into another
+    distribution's namespace.
+    """
     try:
         # Repair permissions on read: if the file leaked to a broader mode
         # (bad umask, restored backup), tighten it back to 0o600.
@@ -435,10 +449,23 @@ def _read_fallback() -> dict[str, str]:
             data = json.load(f)
     except (OSError, json.JSONDecodeError):
         return {}
-    return data if isinstance(data, dict) else {}
+    if not isinstance(data, dict):
+        return {}
+    # Current (nested) format: service -> {name: value}.
+    nested = {k: v for k, v in data.items() if isinstance(v, dict)}
+    if nested:
+        return nested
+    # Legacy flat format: migrate under the default service namespace.
+    flat = {k: v for k, v in data.items() if isinstance(v, str)}
+    return {_DEFAULT_SERVICE: flat} if flat else {}
+
+
+def _read_fallback() -> dict[str, str]:
+    """Return the current service's fallback slice (used by ``get_secret``)."""
+    return dict(_read_fallback_doc().get(_service_name, {}))
 
 
-def _write_fallback(data: dict[str, str]) -> bool:
+def _write_fallback(data: dict[str, dict[str, str]]) -> bool:
     """Atomically write the fallback file with ``0o600`` permissions.
 
     Writes to a temp file in the same directory, ``chmod`` s it before it
@@ -474,31 +501,35 @@ def _write_fallback(data: dict[str, str]) -> bool:
 
 
 def _fallback_set(name: str, value: str) -> bool:
-    """Add/update a fallback entry under the cross-process lock (F5)."""
+    """Add/update a fallback entry under the cross-process lock (F5), scoped to
+    the current service namespace (F2)."""
     with _fallback_lock():
-        data = _read_fallback()
-        data[name] = value
-        return _write_fallback(data)
+        doc = _read_fallback_doc()
+        doc.setdefault(_service_name, {})[name] = value
+        return _write_fallback(doc)
 
 
 def _fallback_delete(name: str) -> bool | None:
-    """Remove a fallback entry under the lock.
+    """Remove a fallback entry (current service only) under the lock.
 
     Returns ``True`` on a successful scrub, ``False`` if the rewrite failed,
     and ``None`` when there was nothing to remove (so callers don't treat a
     no-op as a failure).
     """
     with _fallback_lock():
-        data = _read_fallback()
-        if name not in data:
+        doc = _read_fallback_doc()
+        slice_ = doc.get(_service_name, {})
+        if name not in slice_:
             return None
-        del data[name]
-        return _write_fallback(data)
+        del slice_[name]
+        if not slice_:
+            doc.pop(_service_name, None)
+        return _write_fallback(doc)
 
 
 def _fallback_scrub(name: str) -> None:
     """Best-effort removal of a stale fallback entry after a healthy keyring
-    write (F6).
+    write (F6), scoped to the current service namespace (F2).
 
     A successful keyring write must not leave a rotated-out secret sitting in
     plaintext on disk, where it could later be resurrected if the keyring
@@ -506,11 +537,14 @@ def _fallback_scrub(name: str) -> None:
     failure to rewrite the file is not fatal -- but it is worth surfacing.
     """
     with _fallback_lock():
-        data = _read_fallback()
-        if name not in data:
+        doc = _read_fallback_doc()
+        slice_ = doc.get(_service_name, {})
+        if name not in slice_:
             return
-        del data[name]
-        if not _write_fallback(data):
+        del slice_[name]
+        if not slice_:
+            doc.pop(_service_name, None)
+        if not _write_fallback(doc):
             warnings.warn(
                 f"Secret {name!r} was written to the keyring but its stale "
                 f"plaintext copy in {_FALLBACK_FILE} could not be removed "
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 25ea66fce..d3b5591c9 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -19,6 +19,12 @@
 from code_puppy import secret_store
 
 
+def _slice(fb, service=None):
+    """Return the current service's slice of the nested fallback file (F2)."""
+    doc = json.loads(fb.read_text())
+    return doc.get(service or secret_store._service_name, {})
+
+
 @pytest.fixture(autouse=True)
 def _reset_state():
     """Reset process-global state between tests."""
@@ -197,14 +203,14 @@ def test_set_warns_and_writes_file_when_keyring_fails(
             secret_store.set_secret("k", "v")
 
         assert tmp_fallback.exists()
-        assert json.loads(tmp_fallback.read_text())["k"] == "v"
+        assert _slice(tmp_fallback)["k"] == "v"
 
     def test_delete_also_cleans_fallback(self, working_keyring, tmp_fallback):
         """delete_secret always scrubs the fallback file, in case a prior write
         landed there as a last resort."""
         tmp_fallback.write_text(json.dumps({"k": "leftover", "other": "keep"}))
         secret_store.delete_secret("k")
-        data = json.loads(tmp_fallback.read_text())
+        data = _slice(tmp_fallback)
         assert "k" not in data
         assert data["other"] == "keep"
 
@@ -322,7 +328,7 @@ class TestFallbackPath:
     def test_set_writes_fallback_file(self, missing_keyring, tmp_fallback):
         secret_store.set_secret("k", "v")
         assert tmp_fallback.exists()
-        assert json.loads(tmp_fallback.read_text())["k"] == "v"
+        assert _slice(tmp_fallback)["k"] == "v"
 
     def test_set_then_get_roundtrip(self, missing_keyring, tmp_fallback):
         secret_store.set_secret("k", "v")
@@ -360,7 +366,7 @@ def test_delete_removes_from_fallback(self, missing_keyring, tmp_fallback):
         secret_store.set_secret("k", "v")
         secret_store.set_secret("keep", "me")
         secret_store.delete_secret("k")
-        data = json.loads(tmp_fallback.read_text())
+        data = _slice(tmp_fallback)
         assert "k" not in data
         assert data["keep"] == "me"
 
@@ -536,7 +542,7 @@ def writer(i):
         for t in threads:
             t.join()
 
-        data = json.loads(tmp_fallback.read_text())
+        data = _slice(tmp_fallback)
         for i in range(25):
             assert data[f"key{i}"] == f"val{i}"
 
@@ -548,7 +554,7 @@ def test_healthy_set_scrubs_fallback(self, working_keyring, tmp_fallback):
         # keyring now holds the new value...
         assert secret_store.get_secret("tok") == "NEW"
         # ...and the stale plaintext copy is gone, unrelated entries preserved.
-        data = json.loads(tmp_fallback.read_text())
+        data = _slice(tmp_fallback)
         assert "tok" not in data
         assert data["other"] == "keep"
 
@@ -571,3 +577,48 @@ def test_scrub_failure_warns_but_does_not_raise(
         with patch.object(secret_store, "_write_fallback", return_value=False):
             with pytest.warns(UserWarning, match="stale"):
                 secret_store.set_secret("tok", "NEW")  # must not raise
+
+
+# ---------------------------------------------------------------------------
+# F2 -- fallback file is namespaced by service; distributions stay isolated
+# ---------------------------------------------------------------------------
+
+
+class TestFallbackServiceIsolation:
+    def test_distributions_cannot_see_each_others_fallback(
+        self, missing_keyring, tmp_fallback
+    ):
+        """Distribution A's fallback secret must be invisible to distribution
+        B, and B must not overwrite or delete it (F2)."""
+        import warnings as _w
+
+        with _w.catch_warnings():
+            _w.simplefilter("ignore")
+            secret_store.configure_service_name("dist-a")
+            secret_store.set_secret("token", "A-secret")
+
+            secret_store.configure_service_name("dist-b")
+            assert secret_store.get_secret("token") is None  # cannot read A's
+            secret_store.set_secret("token", "B-secret")
+            secret_store.delete_secret("token")  # only removes B's
+
+            secret_store.configure_service_name("dist-a")
+            assert secret_store.get_secret("token") == "A-secret"  # intact
+
+    def test_fallback_file_is_service_nested(self, missing_keyring, tmp_fallback):
+        import warnings as _w
+
+        with _w.catch_warnings():
+            _w.simplefilter("ignore")
+            secret_store.configure_service_name("dist-x")
+            secret_store.set_secret("k", "v")
+        doc = json.loads(tmp_fallback.read_text())
+        assert doc == {"dist-x": {"k": "v"}}
+
+    def test_legacy_flat_file_migrated_under_default(
+        self, missing_keyring, tmp_fallback
+    ):
+        """A pre-scoping flat file stays readable under the default service."""
+        tmp_fallback.write_text(json.dumps({"legacy": "old"}))
+        assert secret_store.get_service_name() == "code-puppy"
+        assert secret_store.get_secret("legacy") == "old"

From c4c7bfd54bd7c9027fd33420a27d7d818e137521 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 18:03:56 -0400
Subject: [PATCH 12/16] fix(secret-store): honest owner-only hardening on
 Windows, not false chmod (F1)

The fallback warning claimed 'permission-hardened ... (mode 0o600)', but
chmod(0o600) does not establish an owner-only NTFS DACL on Windows -- it only
toggles the read-only attribute. On the older/stripped-down Windows machines
where this fallback actually activates, the file was effectively plaintext
with default inheritance while the warning implied it was protected. The tests
also skipped permission verification on Windows.

Now _harden_permissions branches by platform: chmod 0o600 on POSIX, and an
owner-only NTFS DACL via icacls (/inheritance:r + /grant:r <user>:F, no extra
dependency) on Windows. The outcome is recorded so _warn_fallback_active tells
the truth: it states the NTFS ACL when applied, and warns the file is PLAINTEXT
when icacls is unavailable. Module docstring updated to match. The Windows path
is now unit-tested (mocked icacls) instead of skipped.

Addresses review finding: discussion_r3554790356
---
 code_puppy/secret_store.py | 102 ++++++++++++++++++++++++++++++++-----
 tests/test_secret_store.py |  65 +++++++++++++++++++++++
 2 files changed, 155 insertions(+), 12 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index 81207d1ae..6e9e26134 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -12,7 +12,10 @@
      this.  Chunking keeps the keyring as the source of truth.
   3. **Permission-hardened file fallback** -- used only when both keyring
      strategies fail (genuinely broken backend, backend crash, headless CI).
-     The file is ``0o600`` and written atomically.
+     Written atomically and restricted to the owner: ``0o600`` on POSIX, and
+     an owner-only NTFS DACL (via ``icacls``) on Windows.  If the Windows ACL
+     cannot be applied, the fallback warning says so plainly -- the file is
+     then plaintext with default inheritance, not owner-only protected.
 
 Read strategy:
   The keyring is queried first (with transparent chunk reassembly).  The
@@ -40,6 +43,8 @@
 import contextlib
 import json
 import os
+import subprocess
+import sys
 import tempfile
 import threading
 import warnings
@@ -417,14 +422,84 @@ def _warn_fallback_active() -> None:
     if _warned_fallback:
         return
     _warned_fallback = True
+    if sys.platform == "win32" and _windows_acl_hardened is False:
+        detail = (
+            "secrets are stored in PLAINTEXT at "
+            f"{_FALLBACK_FILE}; an owner-only NTFS ACL could not be applied "
+            "(icacls unavailable on this system). Treat this file as sensitive."
+        )
+    elif sys.platform == "win32":
+        detail = (
+            "secrets are stored in the fallback file at "
+            f"{_FALLBACK_FILE}, restricted to your account by an owner-only "
+            "NTFS ACL."
+        )
+    else:
+        detail = (
+            "secrets are stored in the permission-hardened fallback file at "
+            f"{_FALLBACK_FILE} (mode 0o600)."
+        )
     warnings.warn(
-        "No OS keyring backend is available; secrets are stored in the "
-        f"permission-hardened fallback file at {_FALLBACK_FILE} "
-        "(mode 0o600). This is intended for headless/CI use only.",
+        "No OS keyring backend is available; "
+        + detail
+        + " This is intended for headless/CI use only.",
         stacklevel=2,
     )
 
 
+# Tracks whether the most recent Windows ACL hardening attempt succeeded, so
+# the fallback warning can be honest about the actual on-disk protection.
+# None until the first Windows hardening attempt.
+_windows_acl_hardened: bool | None = None
+
+
+def _harden_windows(path: str) -> bool:
+    """Apply an owner-only NTFS DACL to *path* via ``icacls`` (no extra deps).
+
+    Removes inherited ACEs and grants Full control to the current user only.
+    Best-effort: minimal or older Windows images -- exactly where this
+    fallback activates -- may lack a usable ``icacls``.
+    """
+    user = os.environ.get("USERNAME")
+    if not user:
+        return False
+    try:
+        subprocess.run(
+            ["icacls", path, "/inheritance:r"],
+            check=True,
+            capture_output=True,
+            timeout=10,
+        )
+        subprocess.run(
+            ["icacls", path, "/grant:r", f"{user}:F"],
+            check=True,
+            capture_output=True,
+            timeout=10,
+        )
+        return True
+    except (OSError, subprocess.SubprocessError):
+        return False
+
+
+def _harden_permissions(path: str) -> bool:
+    """Restrict *path* to the owner, honestly per platform.
+
+    POSIX: ``chmod 0o600``.  Windows: an owner-only NTFS DACL via ``icacls``,
+    because ``chmod(0o600)`` does **not** establish owner-only protection on
+    NTFS -- it only toggles the read-only attribute.  The Windows outcome is
+    recorded so ``_warn_fallback_active`` can tell the user the truth.
+    """
+    global _windows_acl_hardened
+    if sys.platform == "win32":
+        _windows_acl_hardened = _harden_windows(path)
+        return _windows_acl_hardened
+    try:
+        os.chmod(path, _FALLBACK_MODE)
+        return True
+    except OSError:
+        return False
+
+
 def _read_fallback_doc() -> dict[str, dict[str, str]]:
     """Read the raw fallback file as a service-namespaced document.
 
@@ -438,13 +513,16 @@ def _read_fallback_doc() -> dict[str, dict[str, str]]:
     """
     try:
         # Repair permissions on read: if the file leaked to a broader mode
-        # (bad umask, restored backup), tighten it back to 0o600.
-        try:
-            current = os.stat(_FALLBACK_FILE).st_mode & 0o777
-            if current != _FALLBACK_MODE:
-                os.chmod(_FALLBACK_FILE, _FALLBACK_MODE)
-        except OSError:
-            pass
+        # (bad umask, restored backup), tighten it back to owner-only.
+        if sys.platform == "win32":
+            _harden_permissions(_FALLBACK_FILE)
+        else:
+            try:
+                current = os.stat(_FALLBACK_FILE).st_mode & 0o777
+                if current != _FALLBACK_MODE:
+                    os.chmod(_FALLBACK_FILE, _FALLBACK_MODE)
+            except OSError:
+                pass
         with open(_FALLBACK_FILE, encoding="utf-8") as f:
             data = json.load(f)
     except (OSError, json.JSONDecodeError):
@@ -484,7 +562,7 @@ def _write_fallback(data: dict[str, dict[str, str]]) -> bool:
     tmp = None
     try:
         fd, tmp = tempfile.mkstemp(dir=CONFIG_DIR, suffix=".tmp")
-        os.chmod(tmp, _FALLBACK_MODE)
+        _harden_permissions(tmp)
         with os.fdopen(fd, "w", encoding="utf-8") as f:
             json.dump(data, f, indent=2)
             f.flush()
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index d3b5591c9..58f8cd57f 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -31,10 +31,12 @@ def _reset_state():
     secret_store._warned_fallback = False
     secret_store._service_name = "code-puppy"
     secret_store._backend_installed = True  # skip lazy install in these tests
+    secret_store._windows_acl_hardened = None
     yield
     secret_store._warned_fallback = False
     secret_store._service_name = "code-puppy"
     secret_store._backend_installed = False
+    secret_store._windows_acl_hardened = None
 
 
 @pytest.fixture
@@ -622,3 +624,66 @@ def test_legacy_flat_file_migrated_under_default(
         tmp_fallback.write_text(json.dumps({"legacy": "old"}))
         assert secret_store.get_service_name() == "code-puppy"
         assert secret_store.get_secret("legacy") == "old"
+
+
+# ---------------------------------------------------------------------------
+# F1 -- Windows fallback hardening is honest (owner-only DACL, not chmod)
+# ---------------------------------------------------------------------------
+
+
+class TestWindowsHardening:
+    def test_harden_windows_invokes_icacls(self, monkeypatch, tmp_path):
+        """On Windows, _harden_permissions applies an owner-only DACL via
+        icacls rather than relying on chmod (which does nothing useful on
+        NTFS)."""
+        monkeypatch.setattr(secret_store.sys, "platform", "win32")
+        monkeypatch.setenv("USERNAME", "alice")
+        calls = []
+
+        def fake_run(cmd, **kw):
+            calls.append(cmd)
+            return MagicMock()
+
+        monkeypatch.setattr(secret_store.subprocess, "run", fake_run)
+        p = str(tmp_path / "secrets.json")
+        assert secret_store._harden_permissions(p) is True
+        assert secret_store._windows_acl_hardened is True
+        assert ["icacls", p, "/inheritance:r"] in calls
+        assert ["icacls", p, "/grant:r", "alice:F"] in calls
+
+    def test_harden_windows_false_when_icacls_missing(self, monkeypatch, tmp_path):
+        monkeypatch.setattr(secret_store.sys, "platform", "win32")
+        monkeypatch.setenv("USERNAME", "alice")
+        monkeypatch.setattr(
+            secret_store.subprocess,
+            "run",
+            MagicMock(side_effect=FileNotFoundError("no icacls")),
+        )
+        assert secret_store._harden_permissions(str(tmp_path / "s.json")) is False
+        assert secret_store._windows_acl_hardened is False
+
+    def test_warning_is_plaintext_honest_when_acl_fails(self, monkeypatch):
+        """When the Windows ACL can't be applied, the warning must NOT claim
+        owner-only protection -- it must say the file is plaintext."""
+        monkeypatch.setattr(secret_store.sys, "platform", "win32")
+        secret_store._windows_acl_hardened = False
+        with pytest.warns(UserWarning, match="PLAINTEXT"):
+            secret_store._warn_fallback_active()
+
+    def test_warning_states_acl_when_hardened(self, monkeypatch):
+        monkeypatch.setattr(secret_store.sys, "platform", "win32")
+        secret_store._windows_acl_hardened = True
+        with pytest.warns(UserWarning, match="NTFS ACL"):
+            secret_store._warn_fallback_active()
+
+    def test_posix_uses_chmod_not_icacls(self, monkeypatch, tmp_path):
+        """Sanity: on POSIX the code path stays chmod-based."""
+        if sys.platform == "win32":
+            pytest.skip("POSIX-only assertion")
+        called = MagicMock()
+        monkeypatch.setattr(secret_store.subprocess, "run", called)
+        p = tmp_path / "s.json"
+        p.write_text("{}")
+        assert secret_store._harden_permissions(str(p)) is True
+        called.assert_not_called()
+        assert stat.S_IMODE(os.stat(p).st_mode) == 0o600

From e6df57d53bbf1e2a951b8fd8be164f31df6dfc57 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 18:09:27 -0400
Subject: [PATCH 13/16] fix(secret-store): route user-facing notices through
 the messaging bus (F11)

warnings.warn is effectively invisible inside the TUI and is deduplicated by
the default warnings filter, so the headless-fallback and write-failure
notices never reached a human. Added _notify(message) which emits through
code_puppy.messaging.emit_warning (imported lazily to avoid an import cycle)
and falls back to warnings.warn only when the bus is unavailable, so a notice
is never silently dropped. The three user-facing call sites now use _notify.

Tests capture bus notices via an autouse fixture instead of pytest.warns.

Addresses review finding: the warnings.warn note in pullrequestreview-4666753894
---
 code_puppy/secret_store.py |  34 +++++++++---
 tests/test_secret_store.py | 111 ++++++++++++++++++++++++-------------
 2 files changed, 97 insertions(+), 48 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index 6e9e26134..c69e9aaf2 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -417,6 +417,24 @@ def _fallback_lock():
                 os.close(fd)
 
 
+def _notify(message: str) -> None:
+    """Surface a user-facing notice through the messaging bus.
+
+    ``warnings.warn`` is effectively invisible inside the TUI and is
+    deduplicated by the default warnings filter, so the headless-fallback and
+    write-failure notices never reached a human.  Routing through the bus makes
+    them render.  Falls back to ``warnings.warn`` when the bus is unavailable
+    (early startup, non-TUI callers, or an import failure) so a notice is never
+    silently dropped.
+    """
+    try:
+        from code_puppy.messaging import emit_warning
+
+        emit_warning(message)
+    except Exception:
+        warnings.warn(message, stacklevel=2)
+
+
 def _warn_fallback_active() -> None:
     global _warned_fallback
     if _warned_fallback:
@@ -439,12 +457,12 @@ def _warn_fallback_active() -> None:
             "secrets are stored in the permission-hardened fallback file at "
             f"{_FALLBACK_FILE} (mode 0o600)."
         )
-    warnings.warn(
+    warnings_msg = (
         "No OS keyring backend is available; "
         + detail
-        + " This is intended for headless/CI use only.",
-        stacklevel=2,
+        + " This is intended for headless/CI use only."
     )
+    _notify(warnings_msg)
 
 
 # Tracks whether the most recent Windows ACL hardening attempt succeeded, so
@@ -623,11 +641,10 @@ def _fallback_scrub(name: str) -> None:
         if not slice_:
             doc.pop(_service_name, None)
         if not _write_fallback(doc):
-            warnings.warn(
+            _notify(
                 f"Secret {name!r} was written to the keyring but its stale "
                 f"plaintext copy in {_FALLBACK_FILE} could not be removed "
-                "(read-only or full filesystem?).",
-                stacklevel=2,
+                "(read-only or full filesystem?)."
             )
 
 
@@ -685,11 +702,10 @@ def set_secret(name: str, value: str) -> None:
         # Unexpected (transient error, backend crash, prompt dismissed).
         # Warn so it's diagnosable, then persist to the file so the secret
         # is not lost.
-        warnings.warn(
+        _notify(
             f"Keyring write failed for {name!r} despite a healthy backend "
             "(transient error or backend crash). Storing in the secure file "
-            f"fallback at {_FALLBACK_FILE}.",
-            stacklevel=2,
+            f"fallback at {_FALLBACK_FILE}."
         )
     else:
         _warn_fallback_active()
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 58f8cd57f..f62a60237 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -39,6 +39,22 @@ def _reset_state():
     secret_store._windows_acl_hardened = None
 
 
+@pytest.fixture(autouse=True)
+def notices(monkeypatch):
+    """Capture user-facing notices routed through the messaging bus (F11).
+
+    Autouse so no test hits the real bus; request it by name to assert on the
+    captured messages.
+    """
+    captured: list[str] = []
+    import code_puppy.messaging as _msg
+
+    monkeypatch.setattr(
+        _msg, "emit_warning", lambda m, *a, **k: captured.append(str(m))
+    )
+    return captured
+
+
 @pytest.fixture
 def tmp_fallback(tmp_path, monkeypatch):
     cfg_dir = tmp_path / "cfg"
@@ -193,16 +209,16 @@ def test_keyring_value_takes_precedence_over_fallback(
         assert secret_store.get_secret("k") == "from-keyring"
 
     def test_set_warns_and_writes_file_when_keyring_fails(
-        self, working_keyring, tmp_fallback
+        self, working_keyring, tmp_fallback, notices
     ):
         """When all keyring writes fail despite a healthy backend, set_secret
-        emits a warning then persists to the fallback file so the secret is
+        emits a bus notice then persists to the fallback file so the secret is
         not lost."""
         fake, _ = working_keyring
         fake.set_password.side_effect = Exception("backend crash")
 
-        with pytest.warns(UserWarning, match="despite a healthy backend"):
-            secret_store.set_secret("k", "v")
+        secret_store.set_secret("k", "v")
+        assert any("despite a healthy backend" in m for m in notices)
 
         assert tmp_fallback.exists()
         assert _slice(tmp_fallback)["k"] == "v"
@@ -378,14 +394,12 @@ def test_corrupt_fallback_tolerated(self, missing_keyring, tmp_fallback):
         secret_store.set_secret("k", "v")
         assert secret_store.get_secret("k") == "v"
 
-    def test_fallback_warns_once(self, missing_keyring, tmp_fallback):
-        import warnings as _w
-
-        with pytest.warns(UserWarning, match="fallback"):
-            secret_store.set_secret("k", "v")
-        with _w.catch_warnings():
-            _w.simplefilter("error")
-            secret_store.set_secret("k2", "v2")
+    def test_fallback_warns_once(self, missing_keyring, tmp_fallback, notices):
+        secret_store.set_secret("k", "v")
+        assert sum("fallback" in m for m in notices) == 1
+        # A second fallback write must not re-emit the notice (dedup).
+        secret_store.set_secret("k2", "v2")
+        assert sum("fallback" in m for m in notices) == 1
 
 
 # ---------------------------------------------------------------------------
@@ -470,18 +484,16 @@ def test_surrounding_whitespace_preserved_keyring(self, working_keyring):
     def test_surrounding_whitespace_preserved_fallback(
         self, missing_keyring, tmp_fallback
     ):
-        with pytest.warns(UserWarning):
-            secret_store.set_secret("k", "  tok  ")
+        secret_store.set_secret("k", "  tok  ")
         assert secret_store.get_secret("k") == "  tok  "
 
-    def test_no_false_alarm_warning_on_empty(self, working_keyring, tmp_fallback):
+    def test_no_false_alarm_warning_on_empty(
+        self, working_keyring, tmp_fallback, notices
+    ):
         """An empty value must not reach the 'keyring write failed' path."""
-        import warnings as _w
-
-        with _w.catch_warnings():
-            _w.simplefilter("error")
-            with pytest.raises(ValueError):
-                secret_store.set_secret("k", "  ")
+        with pytest.raises(ValueError):
+            secret_store.set_secret("k", "  ")
+        assert notices == []
 
 
 # ---------------------------------------------------------------------------
@@ -502,9 +514,8 @@ def test_write_fallback_returns_false_on_replace_error(self, tmp_fallback):
     def test_set_raises_when_fallback_write_fails(self, missing_keyring, tmp_fallback):
         """F4: a lost credential must not report success."""
         with patch.object(secret_store, "_write_fallback", return_value=False):
-            with pytest.warns(UserWarning):
-                with pytest.raises(secret_store.SecretStoreError, match="NOT saved"):
-                    secret_store.set_secret("k", "v")
+            with pytest.raises(secret_store.SecretStoreError, match="NOT saved"):
+                secret_store.set_secret("k", "v")
 
     def test_delete_raises_when_scrub_write_fails(self, missing_keyring, tmp_fallback):
         """F10: a failed scrub must not report a successful delete."""
@@ -531,12 +542,8 @@ def test_concurrent_writers_no_lost_update(self, missing_keyring, tmp_fallback):
         lost updates so every key survives the read-modify-write races."""
         import threading
 
-        import warnings as _w
-
         def writer(i):
-            with _w.catch_warnings():
-                _w.simplefilter("ignore")
-                secret_store.set_secret(f"key{i}", f"val{i}")
+            secret_store.set_secret(f"key{i}", f"val{i}")
 
         threads = [threading.Thread(target=writer, args=(i,)) for i in range(25)]
         for t in threads:
@@ -571,14 +578,14 @@ def test_healthy_set_no_resurrection(self, working_keyring, tmp_fallback):
         assert secret_store.get_secret("tok") is None
 
     def test_scrub_failure_warns_but_does_not_raise(
-        self, working_keyring, tmp_fallback
+        self, working_keyring, tmp_fallback, notices
     ):
         """F6: if the stale-copy scrub can't be written, warn -- but the set
         still succeeded (keyring holds the truth), so don't raise."""
         tmp_fallback.write_text(json.dumps({"tok": "OLD"}))
         with patch.object(secret_store, "_write_fallback", return_value=False):
-            with pytest.warns(UserWarning, match="stale"):
-                secret_store.set_secret("tok", "NEW")  # must not raise
+            secret_store.set_secret("tok", "NEW")  # must not raise
+        assert any("stale" in m for m in notices)
 
 
 # ---------------------------------------------------------------------------
@@ -662,19 +669,19 @@ def test_harden_windows_false_when_icacls_missing(self, monkeypatch, tmp_path):
         assert secret_store._harden_permissions(str(tmp_path / "s.json")) is False
         assert secret_store._windows_acl_hardened is False
 
-    def test_warning_is_plaintext_honest_when_acl_fails(self, monkeypatch):
-        """When the Windows ACL can't be applied, the warning must NOT claim
+    def test_warning_is_plaintext_honest_when_acl_fails(self, monkeypatch, notices):
+        """When the Windows ACL can't be applied, the notice must NOT claim
         owner-only protection -- it must say the file is plaintext."""
         monkeypatch.setattr(secret_store.sys, "platform", "win32")
         secret_store._windows_acl_hardened = False
-        with pytest.warns(UserWarning, match="PLAINTEXT"):
-            secret_store._warn_fallback_active()
+        secret_store._warn_fallback_active()
+        assert any("PLAINTEXT" in m for m in notices)
 
-    def test_warning_states_acl_when_hardened(self, monkeypatch):
+    def test_warning_states_acl_when_hardened(self, monkeypatch, notices):
         monkeypatch.setattr(secret_store.sys, "platform", "win32")
         secret_store._windows_acl_hardened = True
-        with pytest.warns(UserWarning, match="NTFS ACL"):
-            secret_store._warn_fallback_active()
+        secret_store._warn_fallback_active()
+        assert any("NTFS ACL" in m for m in notices)
 
     def test_posix_uses_chmod_not_icacls(self, monkeypatch, tmp_path):
         """Sanity: on POSIX the code path stays chmod-based."""
@@ -687,3 +694,29 @@ def test_posix_uses_chmod_not_icacls(self, monkeypatch, tmp_path):
         assert secret_store._harden_permissions(str(p)) is True
         called.assert_not_called()
         assert stat.S_IMODE(os.stat(p).st_mode) == 0o600
+
+
+# ---------------------------------------------------------------------------
+# F11 -- user-facing notices route through the messaging bus (visible in TUI)
+# ---------------------------------------------------------------------------
+
+
+class TestNotifyRouting:
+    def test_notify_uses_messaging_bus(self, monkeypatch):
+        seen = []
+        import code_puppy.messaging as _msg
+
+        monkeypatch.setattr(_msg, "emit_warning", lambda m, *a, **k: seen.append(m))
+        secret_store._notify("hello human")
+        assert seen == ["hello human"]
+
+    def test_notify_falls_back_to_warnings_when_bus_unavailable(self, monkeypatch):
+        """If the bus raises/imports fail, the notice must not be lost."""
+        import code_puppy.messaging as _msg
+
+        def boom(*a, **k):
+            raise RuntimeError("bus down")
+
+        monkeypatch.setattr(_msg, "emit_warning", boom)
+        with pytest.warns(UserWarning, match="last resort"):
+            secret_store._notify("last resort notice")

From 117a47f195c0a99271f4bc12ec618591e94f5568 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Thu, 9 Jul 2026 18:14:45 -0400
Subject: [PATCH 14/16] fix(secret-store): generation-numbered crash-safe
 chunking (F7)

The old chunked overwrite mutated the previous value's chunks in place while
the count key still pointed at them, so a mid-overwrite crash could yield a
'Frankenstein' value; prune-before-commit could leave count=N with chunks
missing; and the only cross-process lock lived in the macOS backend while
chunking activates on Windows, where no such lock exists.

Reworked to the generation-numbered scheme the reviewer proposed:
  1. Each chunked write picks a fresh random generation and writes chunks
     under name:cp:<gen>:<i>. The previously committed value is untouched.
  2. Commit is a single atomic flip of the pointer key to '<gen>:<count>'.
     Before the flip the old value is fully readable; after it, the new value
     is -- there is no window exposing a torn/mixed value, and no lock is
     required for crash-safety.
  3. The old generation (and any stale direct/legacy entries) are GC'd best
     effort after commit.

Random (not incrementing) generations mean two concurrent writers never share
a chunk namespace; the atomic pointer flip picks the winner. Reads and deletes
understand both the new '<gen>:<count>' pointer and the legacy bare-count
layout, so pre-existing chunked secrets keep working.

Addresses review finding: discussion_r3554814758
---
 code_puppy/secret_store.py | 160 +++++++++++++++++++++++++------------
 tests/test_secret_store.py | 128 +++++++++++++++++++++++++----
 2 files changed, 221 insertions(+), 67 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index c69e9aaf2..1bd896a80 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -180,15 +180,60 @@ def _validate_value(value: str) -> str:
 
 
 def _chunk_count_key(name: str) -> str:
-    """Keyring entry name for the chunk-count (commit) marker."""
+    """Keyring entry name for the commit pointer.
+
+    The pointer value is ``"<gen>:<count>"`` for generation-numbered writes,
+    or a bare ``"<count>"`` for legacy (pre-generation) writes.
+    """
     return f"{name}{_COUNT_SUFFIX}"
 
 
-def _chunk_key(name: str, i: int) -> str:
-    """Keyring entry name for chunk *i* of *name*."""
+def _chunk_key(name: str, gen: str, i: int) -> str:
+    """Keyring entry name for chunk *i* of generation *gen* of *name*."""
+    return f"{name}{_CHUNK_NS}{gen}:{i}"
+
+
+def _legacy_chunk_key(name: str, i: int) -> str:
+    """Keyring entry name for chunk *i* under the pre-generation layout."""
     return f"{name}{_CHUNK_NS}{i}"
 
 
+def _new_generation() -> str:
+    """A short random generation token (8 hex chars).
+
+    Random rather than incrementing so two concurrent writers never pick the
+    same generation and corrupt each other's chunk set: each writes under its
+    own namespace and the single atomic pointer flip decides the winner.
+    """
+    return os.urandom(4).hex()
+
+
+def _parse_pointer(raw: str | None) -> tuple[str | None, int] | None:
+    """Parse a commit pointer into ``(generation, count)``.
+
+    ``generation`` is ``None`` for legacy bare-count pointers.  Returns
+    ``None`` when the pointer is absent or corrupt (treated as no value).
+    """
+    if raw is None:
+        return None
+    if ":" in raw:
+        gen, _, count = raw.partition(":")
+        try:
+            return (gen, int(count))
+        except ValueError:
+            return None
+    try:
+        return (None, int(raw))
+    except ValueError:
+        return None
+
+
+def _gen_chunk_key(name: str, parsed: tuple[str | None, int], i: int) -> str:
+    """Chunk key for index *i* given a parsed pointer (handles legacy)."""
+    gen = parsed[0]
+    return _legacy_chunk_key(name, i) if gen is None else _chunk_key(name, gen, i)
+
+
 # ---------------------------------------------------------------------------
 # Keyring availability + low-level access
 # ---------------------------------------------------------------------------
@@ -257,21 +302,19 @@ def _kr_del_raw(name: str) -> bool:
 def _keyring_get(name: str) -> str | None:
     """Read a logical secret, transparently reassembling chunks if present.
 
-    Checks for a chunk-count key first.  When found, reads and concatenates
-    all chunks in order.  Falls back to a direct single-entry read for values
-    written by older builds or that never needed chunking.
+    Reads the commit pointer first.  When present, reassembles the chunks of
+    the generation it names (new layout) or the legacy flat chunk layout.
+    Falls back to a direct single-entry read for values that never needed
+    chunking.
     """
-    count_raw = _kr_get_raw(_chunk_count_key(name))
-    if count_raw is not None:
-        try:
-            n = int(count_raw)
-        except ValueError:
-            return None  # corrupt count -- treat as absent
+    parsed = _parse_pointer(_kr_get_raw(_chunk_count_key(name)))
+    if parsed is not None:
+        n = parsed[1]
         parts: list[str] = []
         for i in range(n):
-            chunk = _kr_get_raw(_chunk_key(name, i))
+            chunk = _kr_get_raw(_gen_chunk_key(name, parsed, i))
             if chunk is None:
-                return None  # partial write -- treat as absent
+                return None  # partial/torn read -- treat as absent
             parts.append(chunk)
         assembled = "".join(parts)
         return assembled or None
@@ -281,38 +324,51 @@ def _keyring_get(name: str) -> str | None:
 
 
 def _delete_chunks(name: str) -> None:
-    """Best-effort removal of all chunk entries for *name*."""
-    count_raw = _kr_get_raw(_chunk_count_key(name))
-    if count_raw is None:
+    """Best-effort removal of the committed chunk set (pointer + its chunks).
+
+    Only removes the generation named by the current pointer plus the pointer
+    itself; orphaned generations from crashed writers are swept by
+    ``_gc_generations``.
+    """
+    parsed = _parse_pointer(_kr_get_raw(_chunk_count_key(name)))
+    if parsed is None:
         return
-    try:
-        n = int(count_raw)
-    except ValueError:
-        n = 0
-    for i in range(n):
-        _kr_del_raw(_chunk_key(name, i))
+    for i in range(parsed[1]):
+        _kr_del_raw(_gen_chunk_key(name, parsed, i))
     _kr_del_raw(_chunk_count_key(name))
 
 
+def _gc_generation(name: str, gen: str, count: int) -> None:
+    """Best-effort removal of one generation's chunk entries."""
+    i = 0
+    # Delete the known count, then keep going while stray higher chunks exist
+    # (defensive against a prior larger write of the same generation).
+    while i < count or _kr_get_raw(_chunk_key(name, gen, i)) is not None:
+        _kr_del_raw(_chunk_key(name, gen, i))
+        i += 1
+
+
 def _keyring_set(name: str, value: str) -> bool:
     """Write a logical secret, chunking automatically when it exceeds _CHUNK_SIZE.
 
-    **Small values** (len <= _CHUNK_SIZE):
-      - Written as a single keyring entry under *name*.
-      - Any stale chunk entries from a prior oversized write are removed.
+    **Small values** (len <= _CHUNK_SIZE): written as a single entry under
+    *name*; any prior chunk set is removed.
 
-    **Large values** (len > _CHUNK_SIZE):
-      - Split into ceil(len / _CHUNK_SIZE) pieces.
-      - Chunk data entries are written first, then the count key last so a
-        mid-write crash never leaves a partially-readable value behind.
-      - Excess chunks from a prior write with more pieces are pruned.
-      - The direct *name* entry is removed to avoid ambiguity on read.
+    **Large values** (len > _CHUNK_SIZE): written with a *generation-numbered*
+    scheme that is crash-safe without relying on a lock (the chunked path only
+    activates on Windows, where the cross-process flock does not exist):
 
-    Returns ``True`` on success, ``False`` if any keyring write fails.
+      1. Pick a fresh random generation and write every chunk under
+         ``name:cp:<gen>:<i>``.  The previously committed value is untouched.
+      2. Atomically flip the single pointer key to ``"<gen>:<count>"``.  Until
+         this instant a crash leaves the old value fully readable; after it,
+         the new value is fully readable.  There is no window where the
+         pointer names a half-written or mixed ("Frankenstein") value.
+      3. Best-effort GC of the previously committed generation and the stale
+         direct entry.
 
-    The value is stored verbatim; callers reject empty/whitespace-only input
-    at the public boundary (``set_secret``), so a ``False`` here always means
-    a genuine backend failure -- never "you passed an empty string."
+    Returns ``True`` on success, ``False`` if any keyring write fails.  The
+    value is stored verbatim; empty input is rejected at the public boundary.
     """
     if len(value) <= _CHUNK_SIZE:
         if not _kr_set_raw(name, value):
@@ -320,30 +376,32 @@ def _keyring_set(name: str, value: str) -> bool:
         _delete_chunks(name)  # clean up any old chunked write
         return True
 
-    # --- Chunked write path ---
+    # --- Chunked write path (generation-numbered) ---
+    prev = _parse_pointer(_kr_get_raw(_chunk_count_key(name)))
+    gen = _new_generation()
     chunks = [value[i : i + _CHUNK_SIZE] for i in range(0, len(value), _CHUNK_SIZE)]
 
-    # Write data entries first.
+    # 1. Write the new generation's chunks. Old value stays intact.
     for idx, chunk in enumerate(chunks):
-        if not _kr_set_raw(_chunk_key(name, idx), chunk):
-            # Partial write -- roll back what we managed.
+        if not _kr_set_raw(_chunk_key(name, gen, idx), chunk):
+            # Roll back only our own (uncommitted) generation.
             for j in range(idx):
-                _kr_del_raw(_chunk_key(name, j))
+                _kr_del_raw(_chunk_key(name, gen, j))
             return False
 
-    # Prune stale trailing chunks from a prior larger write.
-    stale = len(chunks)
-    while _kr_get_raw(_chunk_key(name, stale)) is not None:
-        _kr_del_raw(_chunk_key(name, stale))
-        stale += 1
-
-    # Commit: write the count key last as the atomic marker.
-    if not _kr_set_raw(_chunk_count_key(name), str(len(chunks))):
+    # 2. Commit: atomically flip the pointer to the new generation.
+    if not _kr_set_raw(_chunk_count_key(name), f"{gen}:{len(chunks)}"):
         for idx in range(len(chunks)):
-            _kr_del_raw(_chunk_key(name, idx))
+            _kr_del_raw(_chunk_key(name, gen, idx))
         return False
 
-    _kr_del_raw(name)  # remove any stale single-entry write
+    # 3. Best-effort GC of the old generation and any stale direct entry.
+    if prev is not None and prev[0] is not None and prev[0] != gen:
+        _gc_generation(name, prev[0], prev[1])
+    elif prev is not None and prev[0] is None:
+        for i in range(prev[1]):
+            _kr_del_raw(_legacy_chunk_key(name, i))
+    _kr_del_raw(name)
     return True
 
 
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index f62a60237..8cfee9bd7 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -25,6 +25,24 @@ def _slice(fb, service=None):
     return doc.get(service or secret_store._service_name, {})
 
 
+def _chunk_entries(store, name, service=None):
+    """All chunk-data entry names for *name* in the fake keyring store (F7)."""
+    svc = service or secret_store._service_name
+    prefix = f"{name}{secret_store._CHUNK_NS}"
+    cnt = secret_store._chunk_count_key(name)
+    return {
+        k[1] for k in store if k[0] == svc and k[1].startswith(prefix) and k[1] != cnt
+    }
+
+
+def _pointer(store, name, service=None):
+    """The parsed commit pointer (gen, count) for *name*, or None."""
+    svc = service or secret_store._service_name
+    return secret_store._parse_pointer(
+        store.get((svc, secret_store._chunk_count_key(name)))
+    )
+
+
 @pytest.fixture(autouse=True)
 def _reset_state():
     """Reset process-global state between tests."""
@@ -251,10 +269,9 @@ def test_large_value_stored_as_chunks(self, working_keyring):
         big = "A" * (secret_store._CHUNK_SIZE * 2 + 100)  # 3 chunks
         secret_store.set_secret("tok", big)
 
-        count_key = secret_store._chunk_count_key("tok")
-        assert store.get((svc, count_key)) == "3"
-        for i in range(3):
-            assert (svc, secret_store._chunk_key("tok", i)) in store
+        assert _pointer(store, "tok") is not None
+        assert _pointer(store, "tok")[1] == 3
+        assert len(_chunk_entries(store, "tok")) == 3
         # Direct entry must NOT exist (unambiguous read path)
         assert (svc, "tok") not in store
 
@@ -273,21 +290,21 @@ def test_small_value_uses_direct_entry(self, working_keyring):
         assert (svc, secret_store._chunk_count_key("small")) not in store
 
     def test_stale_chunks_pruned_on_smaller_write(self, working_keyring):
-        """If a secret shrinks from 3 chunks to 2, the 3rd chunk is removed."""
+        """If a secret shrinks from 3 chunks to 2, the old generation is GC'd."""
         _, store = working_keyring
-        svc = secret_store._service_name
         big3 = "B" * (secret_store._CHUNK_SIZE * 2 + 100)  # 3 chunks
         secret_store.set_secret("tok", big3)
-        assert store.get((svc, secret_store._chunk_count_key("tok"))) == "3"
+        assert _pointer(store, "tok")[1] == 3
 
         big2 = "C" * (secret_store._CHUNK_SIZE + 100)  # 2 chunks
         secret_store.set_secret("tok", big2)
-        assert store.get((svc, secret_store._chunk_count_key("tok"))) == "2"
-        assert (svc, secret_store._chunk_key("tok", 2)) not in store
+        assert _pointer(store, "tok")[1] == 2
+        # Only the new generation's 2 chunks remain -- no orphans.
+        assert len(_chunk_entries(store, "tok")) == 2
         assert secret_store.get_secret("tok") == big2
 
     def test_delete_removes_all_chunk_keys(self, working_keyring):
-        """delete_secret wipes the count key and every chunk entry."""
+        """delete_secret wipes the pointer and every chunk entry."""
         _, store = working_keyring
         svc = secret_store._service_name
         big = "D" * (secret_store._CHUNK_SIZE * 2 + 1)  # 3 chunks
@@ -295,8 +312,7 @@ def test_delete_removes_all_chunk_keys(self, working_keyring):
         secret_store.delete_secret("tok")
 
         assert (svc, secret_store._chunk_count_key("tok")) not in store
-        for i in range(3):
-            assert (svc, secret_store._chunk_key("tok", i)) not in store
+        assert len(_chunk_entries(store, "tok")) == 0
 
     def test_old_single_entry_still_readable(self, working_keyring):
         """Pre-chunking entries written without a count key are still readable."""
@@ -306,12 +322,12 @@ def test_old_single_entry_still_readable(self, working_keyring):
         assert secret_store.get_secret("legacy") == "old-value"
 
     def test_missing_chunk_returns_none(self, working_keyring):
-        """A partially-written chunk sequence (count present, chunk missing)
-        is treated as absent rather than returning corrupt data."""
+        """A committed pointer whose chunk is missing (torn read) is treated as
+        absent rather than returning corrupt data. Uses the legacy layout."""
         _, store = working_keyring
         svc = secret_store._service_name
-        store[(svc, secret_store._chunk_count_key("tok"))] = "3"
-        store[(svc, secret_store._chunk_key("tok", 0))] = "part0"
+        store[(svc, secret_store._chunk_count_key("tok"))] = "3"  # legacy pointer
+        store[(svc, secret_store._legacy_chunk_key("tok", 0))] = "part0"
         # chunk 1 and 2 missing
         assert secret_store.get_secret("tok") is None
 
@@ -720,3 +736,83 @@ def boom(*a, **k):
         monkeypatch.setattr(_msg, "emit_warning", boom)
         with pytest.warns(UserWarning, match="last resort"):
             secret_store._notify("last resort notice")
+
+
+# ---------------------------------------------------------------------------
+# F7 -- generation-numbered chunking is crash-safe (old value never torn)
+# ---------------------------------------------------------------------------
+
+
+class TestGenerationChunking:
+    def _big(self, mult, ch="X"):
+        return ch * (secret_store._CHUNK_SIZE * mult + 10)
+
+    def test_overwrite_uses_new_generation_and_gcs_old(self, working_keyring):
+        _, store = working_keyring
+        secret_store.set_secret("tok", self._big(2, "1"))  # gen1, 3 chunks
+        gen1 = _pointer(store, "tok")[0]
+        assert gen1 is not None
+
+        secret_store.set_secret("tok", self._big(3, "2"))  # gen2, 4 chunks
+        gen2 = _pointer(store, "tok")[0]
+        assert gen2 is not None and gen2 != gen1
+        assert secret_store.get_secret("tok") == self._big(3, "2")
+        # Old generation swept; only the new generation's 4 chunks remain.
+        assert len(_chunk_entries(store, "tok")) == 4
+
+    def test_crash_before_pointer_flip_keeps_old_value(self, working_keyring):
+        """The core F7 guarantee: if the commit (pointer flip) fails mid-write,
+        the previously committed value is still fully readable -- no torn or
+        'Frankenstein' value."""
+        big1 = self._big(2, "1")
+        secret_store.set_secret("tok", big1)
+        orig = secret_store._kr_set_raw
+
+        def flaky(name, value):
+            if name.endswith(secret_store._COUNT_SUFFIX):
+                return False  # simulate a crash exactly at the commit point
+            return orig(name, value)
+
+        with patch.object(secret_store, "_kr_set_raw", side_effect=flaky):
+            assert secret_store._keyring_set("tok", self._big(3, "2")) is False
+
+        # Old value intact and reassembles correctly.
+        assert secret_store.get_secret("tok") == big1
+
+    def test_crash_before_flip_leaves_no_orphan_after_next_write(self, working_keyring):
+        _, store = working_keyring
+        secret_store.set_secret("tok", self._big(2, "1"))
+        orig = secret_store._kr_set_raw
+
+        def flaky(name, value):
+            if name.endswith(secret_store._COUNT_SUFFIX):
+                return False
+            return orig(name, value)
+
+        with patch.object(secret_store, "_kr_set_raw", side_effect=flaky):
+            secret_store._keyring_set("tok", self._big(3, "2"))
+        # The failed write rolled back its own generation's chunks.
+        assert len(_chunk_entries(store, "tok")) == 3  # only the committed gen1
+
+    def test_legacy_chunked_value_readable(self, working_keyring):
+        """A value written under the pre-generation layout is still readable."""
+        _, store = working_keyring
+        svc = secret_store._service_name
+        store[(svc, secret_store._chunk_count_key("tok"))] = "2"  # bare count
+        store[(svc, secret_store._legacy_chunk_key("tok", 0))] = "AAA"
+        store[(svc, secret_store._legacy_chunk_key("tok", 1))] = "BBB"
+        assert secret_store.get_secret("tok") == "AAABBB"
+
+    def test_overwrite_of_legacy_cleans_old_chunks(self, working_keyring):
+        _, store = working_keyring
+        svc = secret_store._service_name
+        store[(svc, secret_store._chunk_count_key("tok"))] = "2"
+        store[(svc, secret_store._legacy_chunk_key("tok", 0))] = (
+            "A" * secret_store._CHUNK_SIZE
+        )
+        store[(svc, secret_store._legacy_chunk_key("tok", 1))] = "B" * 10
+        big = self._big(2, "C")
+        secret_store.set_secret("tok", big)
+        assert secret_store.get_secret("tok") == big
+        assert (svc, secret_store._legacy_chunk_key("tok", 0)) not in store
+        assert (svc, secret_store._legacy_chunk_key("tok", 1)) not in store

From 1242b68521cbe15c1946819c7c6a0b3614adc572 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Fri, 10 Jul 2026 10:03:56 -0400
Subject: [PATCH 15/16] scrub: genericize enterprise references in secret store
 backend

Keep the OSS/enterprise secret store byte-identical while removing
corporate tells: reword the _BLOB_ACCOUNT comment to 'different
distributions' and use neutral service names in the distinct-services
backend test.

(cherry picked from commit 96da4dd4d526b42ab16fd638de688a82c1853637)
---
 code_puppy/secret_store_backends.py | 4 ++--
 tests/test_secret_store_backends.py | 8 ++++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/code_puppy/secret_store_backends.py b/code_puppy/secret_store_backends.py
index 1ac5fea95..4c515809f 100644
--- a/code_puppy/secret_store_backends.py
+++ b/code_puppy/secret_store_backends.py
@@ -39,8 +39,8 @@
 from code_puppy.config import CONFIG_DIR
 
 # All secrets for a given service live under this single, fixed account.
-# The service name is supplied per call by secret_store, so the public and
-# enterprise builds (distinct service names) get distinct blob items.
+# The service name is supplied per call by secret_store, so different
+# distributions (with distinct service names) get distinct blob items.
 _BLOB_ACCOUNT = "__code_puppy_secret_blob__"
 
 # Cross-process lock file guarding the read-modify-write of the blob.
diff --git a/tests/test_secret_store_backends.py b/tests/test_secret_store_backends.py
index 6f09933a1..47ccad813 100644
--- a/tests/test_secret_store_backends.py
+++ b/tests/test_secret_store_backends.py
@@ -116,11 +116,11 @@ def test_delete_removes_only_that_key(self, backend):
 
     def test_distinct_services_get_distinct_items(self, backend):
         be, store = backend
-        be.set_password("code-puppy", "tok", "oss")
-        be.set_password("code-puppy-enterprise", "tok", "ent")
+        be.set_password("code-puppy", "tok", "one")
+        be.set_password("other-app", "tok", "two")
         assert len(store) == 2
-        assert be.get_password("code-puppy", "tok") == "oss"
-        assert be.get_password("code-puppy-enterprise", "tok") == "ent"
+        assert be.get_password("code-puppy", "tok") == "one"
+        assert be.get_password("other-app", "tok") == "two"
 
 
 class TestCorruptionSafety:

From bba608f30d5db76f8dedf6219033665afdaace52 Mon Sep 17 00:00:00 2001
From: Gregory Kinne <gregory.kinne@walmart.com>
Date: Fri, 10 Jul 2026 14:34:50 -0400
Subject: [PATCH 16/16] fix(secret_store): gate keyring write on
 keyring_available() to prevent null-backend silent credential loss

set_secret() called _keyring_set() unconditionally before consulting
keyring_available(). keyring.backends.null.Keyring (priority -1) has a
silent no-op set_password, so the write appeared successful while the
credential was silently discarded -- and then the fallback was scrubbed,
guaranteeing loss.

Now the keyring attempt is gated on keyring_available(): priority <= 0
backends (null/fail/headless) route straight to the permission-hardened
file fallback instead of trusting an accept-and-discard backend.

Tests: new null_keyring fixture (accept-and-discard, distinct from the
raises-based missing_keyring), TestNullBackendNoSilentLoss (5 tests) +
TestFailBackendFallsBack (1 test). 77 passed, ruff clean.
---
 code_puppy/secret_store.py | 33 ++++++++++++-----
 tests/test_secret_store.py | 74 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 99 insertions(+), 8 deletions(-)

diff --git a/code_puppy/secret_store.py b/code_puppy/secret_store.py
index 1bd896a80..6689b906b 100644
--- a/code_puppy/secret_store.py
+++ b/code_puppy/secret_store.py
@@ -740,20 +740,37 @@ def get_secret(name: str) -> str | None:
 def set_secret(name: str, value: str) -> None:
     """Persist a secret by name.
 
-    Attempts three strategies in order:
+    When a usable keyring backend is present (``keyring_available()``)
+    attempts, in order:
       1. Direct keyring write (small values).
       2. Chunked keyring write (oversized values, e.g. Windows CM cap).
-      3. Permission-hardened JSON file fallback (only when both keyring
-         strategies fail -- unexpected backend error or no keyring at all).
+    If the backend is unavailable (priority <= 0, e.g. the null/fail
+    backends or a headless box) the keyring is skipped entirely and the
+    secret goes straight to the permission-hardened JSON file fallback --
+    never trusting a no-op backend's silent "success".
+      3. Permission-hardened JSON file fallback (backend unavailable, or
+         both keyring strategies failed despite a healthy backend).
     """
     _validate_name(name)
     _validate_value(value)
     _ensure_backend()
-    if _keyring_set(name, value):
-        # F6: scrub any stale plaintext copy so a rotated secret can't linger
-        # on disk and be resurrected later if the keyring entry vanishes.
-        _fallback_scrub(name)
-        return
+
+    # F12: Only trust the keyring when a usable backend is present. A backend
+    # with priority <= 0 (e.g. keyring.backends.null.Keyring) is treated as
+    # unavailable -- crucially, its set_password is a *silent no-op* that
+    # returns without raising, so an unconditional _keyring_set() would report
+    # success while the credential is discarded (and, worse, then scrub the
+    # fallback). Gating on keyring_available() routes straight to the hardened
+    # fallback instead of losing the secret. The fail backend (priority 0,
+    # raises) already fell through correctly; this closes the null-backend
+    # silent-loss hole and honors the availability contract for both.
+    if keyring_available():
+        if _keyring_set(name, value):
+            # F6: scrub any stale plaintext copy so a rotated secret can't
+            # linger on disk and be resurrected later if the keyring entry
+            # vanishes.
+            _fallback_scrub(name)
+            return
 
     if keyring_available():
         # Both direct and chunked writes failed despite a healthy backend.
diff --git a/tests/test_secret_store.py b/tests/test_secret_store.py
index 8cfee9bd7..86dc37c4f 100644
--- a/tests/test_secret_store.py
+++ b/tests/test_secret_store.py
@@ -122,6 +122,26 @@ def missing_keyring():
         yield fake
 
 
+@pytest.fixture
+def null_keyring():
+    """Model the null backend (F12): priority <= 0 AND writes silently no-op.
+
+    Unlike ``missing_keyring`` (fail backend, set_password *raises*), the null
+    backend's set_password/delete_password return None without raising and
+    get_password always returns None -- a black hole that must NOT be mistaken
+    for a successful write. Mirrors keyring.backends.null.Keyring (priority -1).
+    """
+    fake = MagicMock()
+    fake.get_password = MagicMock(return_value=None)
+    fake.set_password = MagicMock(return_value=None)  # silent no-op, no raise
+    fake.delete_password = MagicMock(return_value=None)
+    backend = MagicMock()
+    backend.priority = -1
+    fake.get_keyring = MagicMock(return_value=backend)
+    with patch.object(secret_store, "keyring", fake):
+        yield fake
+
+
 # ---------------------------------------------------------------------------
 # configure_service_name
 # ---------------------------------------------------------------------------
@@ -418,6 +438,60 @@ def test_fallback_warns_once(self, missing_keyring, tmp_fallback, notices):
         assert sum("fallback" in m for m in notices) == 1
 
 
+# ---------------------------------------------------------------------------
+# F12: null-backend silent-loss guard
+# ---------------------------------------------------------------------------
+
+
+class TestNullBackendNoSilentLoss:
+    """A backend that accepts-and-discards (null.Keyring, priority -1) must not
+    be mistaken for a successful keyring write. set_secret must route to the
+    hardened fallback so the credential is never silently lost.
+    """
+
+    def test_set_persists_to_fallback_not_the_void(self, null_keyring, tmp_fallback):
+        secret_store.set_secret("puppy_token", "REAL-TOKEN")
+        # The null backend's set_password is a no-op; the secret must have
+        # landed in the fallback file instead of vanishing.
+        assert tmp_fallback.exists()
+        assert _slice(tmp_fallback)["puppy_token"] == "REAL-TOKEN"
+
+    def test_set_then_get_roundtrip(self, null_keyring, tmp_fallback):
+        secret_store.set_secret("puppy_token", "REAL-TOKEN")
+        assert secret_store.get_secret("puppy_token") == "REAL-TOKEN"
+
+    def test_keyring_set_not_attempted_when_unavailable(
+        self, null_keyring, tmp_fallback
+    ):
+        # The availability gate must short-circuit before any keyring write,
+        # so the null backend's set_password is never even called.
+        secret_store.set_secret("puppy_token", "REAL-TOKEN")
+        null_keyring.set_password.assert_not_called()
+
+    def test_emits_fallback_active_notice(self, null_keyring, tmp_fallback, notices):
+        secret_store.set_secret("puppy_token", "REAL-TOKEN")
+        assert any("fallback" in m for m in notices)
+
+    def test_does_not_scrub_the_fresh_fallback_write(self, null_keyring, tmp_fallback):
+        # Regression: the old code took the keyring-success path and scrubbed
+        # the fallback -- guaranteeing loss. Ensure the value survives.
+        secret_store.set_secret("puppy_token", "REAL-TOKEN")
+        assert _slice(tmp_fallback).get("puppy_token") == "REAL-TOKEN"
+
+
+class TestFailBackendFallsBack:
+    """The fail backend (priority 0, every op raises) is the authentic
+    'no keyring on this box' case that keyring auto-selects on a headless
+    machine. It must route to the hardened fallback (companion to the null
+    guard so the two contracts stay pinned independently).
+    """
+
+    def test_set_persists_to_fallback(self, missing_keyring, tmp_fallback):
+        secret_store.set_secret("puppy_token", "REAL-TOKEN")
+        assert _slice(tmp_fallback)["puppy_token"] == "REAL-TOKEN"
+        assert secret_store.get_secret("puppy_token") == "REAL-TOKEN"
+
+
 # ---------------------------------------------------------------------------
 # Cross-platform fixes
 # ---------------------------------------------------------------------------