From a9cce6d8448394b7b2c66f62b123ae40edcea596 Mon Sep 17 00:00:00 2001 From: Erosika Date: Thu, 2 Jul 2026 18:39:23 -0400 Subject: [PATCH] fix(memory): write honcho config through the plugin's own resolution, locked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The panel's PUT hand-rolled its target: hardcoded profile-local path while reads used resolve_config_path(), always the underscore host key while reads honor legacy dot-form blocks, apiKey to the env store which the client ranks below a JSON-stored key, and an unlocked read-modify-write of the file the OAuth refresh loop guards with an advisory lock because refresh tokens are single-use. Any of the first three silently shadowed or bypassed live settings on save; the fourth could revoke the OAuth grant. Route the write through resolve_config_path() and _host_block (updating the resolved block in place), persist a saved apiKey into the host block where the client reads it — never over an OAuth access token, which the refresh loop owns — and take _config_refresh_lock around the read-modify-write. Tolerate hosts:null instead of 500ing. --- hermes_cli/web_server.py | 78 +++++++++++++++--------- tests/hermes_cli/test_web_server.py | 94 ++++++++++++++++++++++++++++- 2 files changed, 142 insertions(+), 30 deletions(-) diff --git a/hermes_cli/web_server.py b/hermes_cli/web_server.py index 2fc695ef4a56..23199b5875af 100644 --- a/hermes_cli/web_server.py +++ b/hermes_cli/web_server.py @@ -4152,41 +4152,63 @@ def _write_provider_honcho(provider: ProviderConfigSchema, values: Dict[str, str text clears a key so it falls back to the host/default mapping. """ + from plugins.memory.honcho.oauth import ACCESS_TOKEN_PREFIX, _config_refresh_lock from utils import atomic_json_write - resolve_active_host, _resolve_config_path, _host_block = _honcho_resolvers() + resolve_active_host, resolve_config_path, host_block_of = _honcho_resolvers() host = resolve_active_host() + # Write the same file reads resolve — a hardcoded path would shadow a + # config living at one of resolve_config_path's fallbacks with a sparse + # copy holding only the submitted keys. + path = resolve_config_path() - path = get_hermes_home() / "honcho.json" - cfg: Dict[str, Any] = {} - if path.exists(): - try: - loaded = json.loads(path.read_text(encoding="utf-8")) - cfg = loaded if isinstance(loaded, dict) else {} - except Exception: - _log.warning("Failed to read Honcho config from %s", path, exc_info=True) + # OAuth refresh rotates single-use tokens in this file; an unlocked + # read-modify-write racing it would persist a stale refresh token and + # revoke the grant, so serialize on the same advisory lock. + with _config_refresh_lock(path): + cfg: Dict[str, Any] = {} + if path.exists(): + try: + loaded = json.loads(path.read_text(encoding="utf-8")) + cfg = loaded if isinstance(loaded, dict) else {} + except Exception: + _log.warning("Failed to read Honcho config from %s", path, exc_info=True) - host_block = cfg.setdefault("hosts", {}).setdefault(host, {}) + hosts = cfg.get("hosts") + cfg["hosts"] = hosts = hosts if isinstance(hosts, dict) else {} + # Target the block reads resolve — including a legacy dot-form key — + # so a save updates the live block instead of shadowing it. + existing = host_block_of(cfg, host) + host_key = next((k for k, v in hosts.items() if v is existing), host) if existing else host + host_block = hosts.setdefault(host_key, existing) - for field in provider.fields: - if field.is_secret: - submitted = (values.get(field.key) or "").strip() - if submitted and field.env_key: - save_env_value(field.env_key, submitted) - continue - if field.key not in values: - continue - target = host_block if field.scope == "host" else cfg - coerced = _coerce_field_value(field, values[field.key]) - if coerced is _UNSET: - target.pop(field.key, None) - for alias in field.aliases: - target.pop(alias, None) - else: - target[field.key] = coerced + for field in provider.fields: + if field.is_secret: + submitted = (values.get(field.key) or "").strip() + if not submitted: + continue + if field.env_key: + save_env_value(field.env_key, submitted) + # The client reads the JSON-stored key before the env store, so + # persist where honcho setup does — but never overwrite an OAuth + # access token; the refresh loop owns that slot. + stored = host_block.get(field.key) + if not (isinstance(stored, str) and stored.startswith(ACCESS_TOKEN_PREFIX)): + host_block[field.key] = submitted + continue + if field.key not in values: + continue + target = host_block if field.scope == "host" else cfg + coerced = _coerce_field_value(field, values[field.key]) + if coerced is _UNSET: + target.pop(field.key, None) + for alias in field.aliases: + target.pop(alias, None) + else: + target[field.key] = coerced - path.parent.mkdir(parents=True, exist_ok=True) - atomic_json_write(path, cfg, mode=0o600) + path.parent.mkdir(parents=True, exist_ok=True) + atomic_json_write(path, cfg, mode=0o600) @app.get("/api/memory/providers/{name}/config") diff --git a/tests/hermes_cli/test_web_server.py b/tests/hermes_cli/test_web_server.py index eb1a32a7af4f..451aae461897 100644 --- a/tests/hermes_cli/test_web_server.py +++ b/tests/hermes_cli/test_web_server.py @@ -512,6 +512,15 @@ class TestWebServerEndpoints: # ── Memory provider config (Honcho host-block backend) ────────────── + @staticmethod + def _seed_local_honcho(cfg=None): + from hermes_constants import get_hermes_home + + path = get_hermes_home() / "honcho.json" + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(json.dumps(cfg if cfg is not None else {}), encoding="utf-8") + return path + def test_get_honcho_config_returns_safe_defaults(self, monkeypatch, tmp_path): # HOME isn't isolated by the suite; pin it so ~/.honcho can't leak in. monkeypatch.setenv("HOME", str(tmp_path)) @@ -541,6 +550,7 @@ class TestWebServerEndpoints: def test_put_honcho_writes_host_block_root_and_secret(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() from hermes_constants import get_hermes_home from hermes_cli.config import load_config, load_env @@ -571,11 +581,13 @@ class TestWebServerEndpoints: assert cfg["hosts"]["hermes"]["peerName"] == "eri" assert cfg["hosts"]["hermes"]["environment"] == "local" assert cfg["hosts"]["hermes"]["sessionStrategy"] == "per-repo" - # The secret must never be written to the JSON config. - assert "hch-test-key" not in json.dumps(cfg) + # The key persists where the client actually reads it (the host block + # outranks the env store) — GET keeps it write-only regardless. + assert cfg["hosts"]["hermes"]["apiKey"] == "hch-test-key" def test_put_honcho_blank_text_clears_key(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() from hermes_constants import get_hermes_home self.client.put( @@ -592,6 +604,7 @@ class TestWebServerEndpoints: def test_put_honcho_partial_save_preserves_other_keys(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() from hermes_constants import get_hermes_home self.client.put( @@ -619,6 +632,7 @@ class TestWebServerEndpoints: def test_get_honcho_config_does_not_return_secret(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() self.client.put( "/api/memory/providers/honcho/config", @@ -636,6 +650,7 @@ class TestWebServerEndpoints: def test_put_honcho_bool_stored_natively_and_false_survives(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() from hermes_constants import get_hermes_home self.client.put( @@ -654,6 +669,7 @@ class TestWebServerEndpoints: def test_put_honcho_number_stored_as_native_number(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() from hermes_constants import get_hermes_home self.client.put( @@ -672,6 +688,7 @@ class TestWebServerEndpoints: def test_put_honcho_json_round_trips_object(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path)) + self._seed_local_honcho() from hermes_constants import get_hermes_home self.client.put( @@ -685,6 +702,79 @@ class TestWebServerEndpoints: fields = self._provider_field_map(self.client.get("/api/memory/providers/honcho/config").json()) assert json.loads(fields["userPeerAliases"]["value"]) == {"telegram_1": "eri"} + def test_put_honcho_first_save_merges_into_resolved_config(self, monkeypatch, tmp_path): + # No profile-local honcho.json: reads and writes both resolve to the + # global config, so a save merges instead of shadowing it with a + # sparse profile-local copy. + monkeypatch.setenv("HOME", str(tmp_path)) + from hermes_constants import get_hermes_home + + global_path = tmp_path / ".honcho" / "config.json" + global_path.parent.mkdir(parents=True) + global_path.write_text( + json.dumps({"baseUrl": "https://kept.example", "hosts": {"hermes": {"workspace": "kept"}}}), + encoding="utf-8", + ) + + resp = self.client.put( + "/api/memory/providers/honcho/config", + json={"values": {"peerName": "eri"}}, + ) + + assert resp.status_code == 200 + assert not (get_hermes_home() / "honcho.json").exists() + cfg = json.loads(global_path.read_text(encoding="utf-8")) + assert cfg["baseUrl"] == "https://kept.example" + assert cfg["hosts"]["hermes"] == {"workspace": "kept", "peerName": "eri"} + + def test_put_honcho_updates_legacy_dot_form_host_block(self, monkeypatch, tmp_path): + # A legacy 'hermes.' block that reads resolve must be updated + # in place, not shadowed by a fresh underscore-form block. + monkeypatch.setenv("HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_HONCHO_HOST", "hermes_work") + + path = self._seed_local_honcho({"hosts": {"hermes.work": {"workspace": "w", "peerName": "eri"}}}) + + resp = self.client.put( + "/api/memory/providers/honcho/config", + json={"values": {"sessionStrategy": "per-repo"}}, + ) + + assert resp.status_code == 200 + hosts = json.loads(path.read_text(encoding="utf-8"))["hosts"] + assert set(hosts) == {"hermes.work"} + assert hosts["hermes.work"] == {"workspace": "w", "peerName": "eri", "sessionStrategy": "per-repo"} + + def test_put_honcho_api_key_never_overwrites_oauth_token(self, monkeypatch, tmp_path): + monkeypatch.setenv("HOME", str(tmp_path)) + from hermes_cli.config import load_env + + path = self._seed_local_honcho({"hosts": {"hermes": {"apiKey": "hch-at-oauth-token"}}}) + + resp = self.client.put( + "/api/memory/providers/honcho/config", + json={"values": {"apiKey": "manual-key"}}, + ) + + assert resp.status_code == 200 + cfg = json.loads(path.read_text(encoding="utf-8")) + # The OAuth grant owns the JSON slot; the manual key lands in the env store. + assert cfg["hosts"]["hermes"]["apiKey"] == "hch-at-oauth-token" + assert load_env()["HONCHO_API_KEY"] == "manual-key" + + def test_put_honcho_tolerates_null_hosts(self, monkeypatch, tmp_path): + monkeypatch.setenv("HOME", str(tmp_path)) + + path = self._seed_local_honcho({"hosts": None}) + + resp = self.client.put( + "/api/memory/providers/honcho/config", + json={"values": {"workspace": "myws"}}, + ) + + assert resp.status_code == 200 + assert json.loads(path.read_text(encoding="utf-8"))["hosts"]["hermes"]["workspace"] == "myws" + def test_put_honcho_rejects_malformed_number_and_json(self, monkeypatch, tmp_path): monkeypatch.setenv("HOME", str(tmp_path))