mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-22 16:25:58 +00:00
fix(memory): write honcho config through the plugin's own resolution, locked
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.
This commit is contained in:
Erosika
parent
822c8226d7
commit
a9cce6d844
2 changed files with 142 additions and 30 deletions
|
|
@ -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")
|
||||
|
|
|
|||
|
|
@ -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.<profile>' 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))
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue