fix(honcho): a transient read failure is not an empty credential store

_persist_credential promises "leaving all else intact", but it seeded
its write from _read_config, which returns {} on ANY read failure, and
_atomic_write_config replaces the whole file via os.replace — which
needs only a writable parent, so a present-but-unreadable honcho.json
did not stop the overwrite. One OSError (EACCES after a root-owned
write, EIO, a stalled mount) during an automatic token refresh or a
fresh login therefore replaced the store with a single-host file,
destroying every other host's credentials and honcho.json's root
config. No user action is required to trigger the refresh path.

Same defect class as #75206 (P1, fixed for the core auth store in

Add _read_config_strict for the write paths: a missing file still
bootstraps as {}; an unreadable file raises with the store untouched;
genuine corruption still degrades but preserves a .corrupt copy first,
since a truncated store usually holds the other hosts' tokens verbatim.
_rotate_and_persist now takes its strict read BEFORE the exchange —
rotation is single-use, so an exchange whose result cannot be persisted
loses the grant — and threads the dict through to _persist_credential,
which also closes the re-read race between the locked read and the
persist. install_grant seeds its root-merge from the strict reader for
the same reason. Read paths keep their fail-open contract untouched;
both readers now use utf-8-sig so a BOM'd store is not misclassified
as corruption (the wipe vector needing no filesystem fault at all).

Adds TestPersistReadFailure: six tests, four of which fail against the
previous source; the rotate-ordering test additionally pins that no
exchange is attempted against an unreadable store.

plugins/memory/honcho/oauth.py

@@ -107,6 +107,36 @@ def _mark_grant_dead(key: tuple[str, str], cred: OAuthCredential) -> None:
     _dead_grants[key] = hashlib.sha256(cred.refresh_token.encode("utf-8")).hexdigest()
     _reauth_check_cache.pop(key, None)  # verdict changed without a config rewrite
 
+def _read_config_strict(path: Path) -> dict[str, Any]:
+    """Reader for the WRITE paths: never lets a failed read become an empty store.
+
+    A missing file is the bootstrap case and reads as ``{}``. A file that EXISTS but cannot be read
+    (EACCES after a root-owned write, EIO, a stalled mount) raises instead: ``atomic_json_write`` replaces
+    the whole file and ``os.replace`` needs only a writable parent, so degrading here would let the next
+    persist erase every other host's credentials. Genuine corruption still degrades, but only after
+    preserving a ``.corrupt`` copy: a truncated store usually holds the other hosts' tokens verbatim.
+    """
+    try:
+        return json.loads(path.read_text(encoding="utf-8-sig"))
+    except FileNotFoundError:
+        return {}
+    except OSError:
+        logger.warning("Honcho config at %s could not be read; refusing to treat it as empty "
+                       "(a persist would erase every other host's credentials)", path, exc_info=True)
+        raise
+    except json.JSONDecodeError as exc:
+        corrupt = path.with_name(path.name + ".corrupt")
+        preserved = False
+        try:
+            import shutil
+            shutil.copy2(path, corrupt)
+            preserved = True
+        except Exception:
+            logger.debug("could not preserve a copy at %s", corrupt, exc_info=True)
+        logger.warning("Honcho config at %s is corrupt (%s); starting from an empty store. %s", path, exc,
+                       f"Original preserved at {corrupt}" if preserved else f"A copy could NOT be preserved at {corrupt}")
+        return {}
+
 def _load_cred(path: Path, host: str, raw: dict[str, Any] | None = None) -> OAuthCredential | None:
     """Credential from ``host``'s block in ``raw`` (or the file at ``path``)."""
     source = raw if raw is not None else read_json_or_empty(path)
@@ -252,6 +282,12 @@ def _rotate_and_persist(
 ) -> OAuthCredential | None:
     """Exchange ``cred`` and persist the rotation; ``None`` on failure (logged). A permanent OAuth error
     marks the grant dead so later calls skip the endpoint until a new login rotates the refresh token."""
+    try:
+        # Read before spending the single-use refresh token: an exchange that cannot be persisted loses the grant.
+        raw = _read_config_strict(path)
+    except OSError:
+        _refresh_failure_at[key] = time.monotonic()
+        return None
     try:
         rotated = _exchange_with_retry(cred, now=now)
     except Exception as exc:
@@ -272,7 +308,7 @@ def _rotate_and_persist(
         _refresh_failure_at[key] = time.monotonic()
         logger.warning("Honcho OAuth %s failed for host %s: %s", op_label, host, redact_sensitive_text(str(exc), force=True))
         return None
-    _persist_credential(path, host, rotated)
+    _persist_credential(path, host, rotated, raw=raw)
     return rotated
 
 def _deep_merge(base: dict[str, Any], overlay: dict[str, Any]) -> dict[str, Any]:
@@ -284,11 +320,12 @@ def _deep_merge(base: dict[str, Any], overlay: dict[str, Any]) -> dict[str, Any]
     return base
 
 def _persist_credential(path: Path, host: str, cred: OAuthCredential, raw: dict[str, Any] | None = None) -> None:
-    """Write ``cred`` into ``host``'s block (apiKey + oauth) of ``raw`` (default:
-    the file's current content), leaving the rest intact; marks the grant live."""
+    """Write ``cred`` into ``host``'s block (apiKey + oauth) of ``raw`` (default: a strict read of the
+    file), leaving the rest intact; marks the grant live. "Leaving the rest intact" is only true when the
+    existing store was actually read, so an unreadable file raises instead of becoming a single-host file."""
     from utils import atomic_json_write
 
-    raw = read_json_or_empty(path) if raw is None else raw
+    raw = _read_config_strict(path) if raw is None else raw
     block = raw.setdefault("hosts", {}).setdefault(host, {})
     block["apiKey"], block["oauth"] = cred.access_token, cred.oauth_block()
     atomic_json_write(path, raw, mode=0o600)
@@ -374,7 +411,8 @@ def install_grant(
     ``apiKey`` and ``oauth`` block. ``apply_config=False`` stores tokens only."""
     now = time.time() if now is None else now
     cred = OAuthCredential.from_token_response(grant, now=now, client_id=client_id, token_endpoint=token_endpoint)
-    raw = read_json_or_empty(path)
+    # Strict: a fresh login must not seed its root-merge from a store that exists but could not be read.
+    raw = _read_config_strict(path)
     granted_config = grant.get("config")
     if isinstance(granted_config, dict):
         cred.consent_peer_name = granted_config.get("peerName")

tests/honcho_plugin/test_oauth.py

@@ -199,3 +199,119 @@ class TestApplyTokenToClient:
 
     def test_returns_false_when_shape_unknown(self):
         assert oauth.apply_token_to_client(object(), "hch-at-new") is False
+
+
+class TestPersistReadFailure:
+    """One unreadable honcho.json must never become a single-host store.
+
+    ``_persist_credential`` promises "leaving all else intact", but it seeded
+    the write from a reader that returned ``{}`` on ANY read failure, and
+    ``_atomic_write_config`` replaces the whole file via ``os.replace`` —
+    which needs only a writable parent, so a present-but-unreadable store did
+    not stop the overwrite. Same class as the auth-store fix in #75206.
+    """
+
+    @staticmethod
+    def _seed_two_hosts(path: Path) -> bytes:
+        _write(path, {
+            "hosts": {
+                "keep.example": _host_block(refresh="hch-rt-keep"),
+                "rotate.example": _host_block(refresh="hch-rt-rot"),
+            },
+            "defaults": {"workspace": "w1"},
+        })
+        return path.read_bytes()
+
+    def _unreadable(self, monkeypatch, target: Path):
+        real = Path.read_text
+
+        def boom(self, *a, **kw):
+            if self.name == target.name:
+                raise PermissionError(13, "Permission denied")
+            return real(self, *a, **kw)
+
+        monkeypatch.setattr(Path, "read_text", boom)
+
+    def test_persist_raises_and_preserves_store_when_unreadable(
+        self, tmp_path, monkeypatch
+    ):
+        path = tmp_path / "honcho.json"
+        before = self._seed_two_hosts(path)
+        cred = OAuthCredential.from_host_block(_host_block(refresh="hch-rt-new"))
+
+        with monkeypatch.context() as m:
+            self._unreadable(m, path)
+            with pytest.raises(OSError):
+                oauth._persist_credential(path, "rotate.example", cred)
+
+        assert path.read_bytes() == before
+
+    def test_rotate_refuses_before_spending_the_refresh_token(
+        self, tmp_path, monkeypatch
+    ):
+        # Rotation is single-use: an exchange whose result cannot be persisted
+        # loses the grant, so an unreadable store must fail the refresh BEFORE
+        # the exchange, not after.
+        path = tmp_path / "honcho.json"
+        before = self._seed_two_hosts(path)
+        cred = OAuthCredential.from_host_block(_host_block(refresh="hch-rt-rot"))
+        key = (str(path), "rotate.example")
+        oauth._refresh_failure_at.pop(key, None)
+
+        def no_exchange(*a, **kw):  # pragma: no cover - must not run
+            raise AssertionError("exchange ran against an unreadable store")
+
+        with monkeypatch.context() as m:
+            m.setattr(oauth, "_exchange_with_retry", no_exchange)
+            self._unreadable(m, path)
+            assert oauth._rotate_and_persist(
+                path, "rotate.example", key, cred, now=1.0
+            ) is None
+
+        assert key in oauth._refresh_failure_at
+        oauth._refresh_failure_at.pop(key, None)
+        assert path.read_bytes() == before
+
+    def test_corrupt_store_degrades_but_preserves_a_copy(self, tmp_path):
+        path = tmp_path / "honcho.json"
+        truncated = json.dumps(
+            {"hosts": {"keep.example": _host_block(refresh="hch-rt-keep")}}
+        )[:-5]
+        path.write_text(truncated, encoding="utf-8")
+
+        assert oauth._read_config_strict(path) == {}
+        corrupt = path.with_name(path.name + ".corrupt")
+        assert corrupt.exists()
+        assert corrupt.read_text(encoding="utf-8") == truncated
+
+    def test_missing_store_still_bootstraps_empty(self, tmp_path):
+        assert oauth._read_config_strict(tmp_path / "honcho.json") == {}
+
+    def test_bom_prefixed_store_is_not_corruption(self, tmp_path):
+        path = tmp_path / "honcho.json"
+        path.write_text(
+            json.dumps({"hosts": {"keep.example": _host_block()}}),
+            encoding="utf-8-sig",
+        )
+        assert "keep.example" in oauth._read_config(path).get("hosts", {})
+        assert "keep.example" in oauth._read_config_strict(path).get("hosts", {})
+        assert not path.with_name(path.name + ".corrupt").exists()
+
+    def test_install_grant_raises_and_preserves_store_when_unreadable(
+        self, tmp_path, monkeypatch
+    ):
+        path = tmp_path / "honcho.json"
+        before = self._seed_two_hosts(path)
+
+        with monkeypatch.context() as m:
+            self._unreadable(m, path)
+            with pytest.raises(OSError):
+                oauth.install_grant(
+                    path, "new.example",
+                    {"access_token": "hch-at-new", "refresh_token": "hch-rt-new",
+                     "expires_in": 3600},
+                    client_id="hermes-desktop",
+                    token_endpoint="http://localhost:8000/oauth/token",
+                )
+
+        assert path.read_bytes() == before