From 73a4574ede648ea28b072297ff896df061214c76 Mon Sep 17 00:00:00 2001 From: Hermes Pi Date: Fri, 10 Jul 2026 21:48:50 +0200 Subject: [PATCH] fix(honcho): preserve profiles for local IP config --- plugins/memory/honcho/client.py | 96 ++++++++++++++++-------- tests/honcho_plugin/test_client.py | 116 ++++++++++++++++++++++++++++- 2 files changed, 180 insertions(+), 32 deletions(-) diff --git a/plugins/memory/honcho/client.py b/plugins/memory/honcho/client.py index ca4352f27aff..c044d776d8d7 100644 --- a/plugins/memory/honcho/client.py +++ b/plugins/memory/honcho/client.py @@ -17,8 +17,10 @@ import json import os import logging import hashlib +import ipaddress from dataclasses import dataclass, field from pathlib import Path +from urllib.parse import urlparse from hermes_constants import get_hermes_home from hermes_cli.profiles import _get_default_hermes_home @@ -56,33 +58,37 @@ def resolve_active_host() -> str: Resolution order: 1. HERMES_HONCHO_HOST env var (explicit override) - 2. Active profile name via profiles system -> ``hermes.`` - 3. Fallback: ``"hermes"`` (default profile) + 2. Active profile name via profiles system -> ``hermes_`` + 3. defaultHost from the active config, but only for the default profile + 4. Fallback: ``"hermes"`` (default profile) """ explicit = os.environ.get("HERMES_HONCHO_HOST", "").strip() if explicit: return explicit - # Respect defaultHost from honcho.json when no profile override is set. - # This keeps setup-generated configs (e.g. host block "local") in sync with - # the host key the rest of the plugin resolves. - try: - path = resolve_config_path() - if path.exists(): - raw = json.loads(path.read_text(encoding="utf-8")) - default_host = raw.get("defaultHost", "").strip() - if default_host: - return default_host - except Exception: - pass - try: from hermes_cli.profiles import get_active_profile_name profile = get_active_profile_name() - return profile_host_key(profile) + profile_host = profile_host_key(profile) except Exception: - pass - return HOST + profile_host = HOST + + # Honcho's generic config can carry a defaultHost (for example "local"), + # but applying it before profile resolution makes every named Hermes + # profile share that same host. Keep named profiles isolated; only the + # default Hermes profile may opt into the config's default host. + if profile_host == HOST: + try: + path = resolve_config_path() + if path.exists(): + raw = json.loads(path.read_text(encoding="utf-8")) + default_host = str(raw.get("defaultHost", "")).strip() + if default_host: + return default_host + except Exception: + pass + + return profile_host def resolve_global_config_path() -> Path: @@ -239,6 +245,44 @@ def _parse_dialectic_depth_levels(host_val, root_val, depth: int) -> list[str] | _DEFAULT_HTTP_TIMEOUT = 30.0 +def _is_local_base_url(base_url: str | None) -> bool: + """Return True for loopback/LAN/VPN self-hosted Honcho URLs. + + Local Honcho deployments can run without auth, but the SDK requires a + non-empty api_key argument. Treat loopback plus RFC1918/link-local/ULA + and carrier-grade-NAT IPs as local so LAN/VPN URLs such as + ``http://192.168.2.112:8000`` get the same placeholder-key behavior as + localhost. + """ + if not base_url: + return False + + try: + parsed = urlparse(base_url) + host = (parsed.hostname or "").strip().lower() + except Exception: + host = "" + + if host in {"localhost", "127.0.0.1", "::1"}: + return True + if not host: + return False + + try: + ip = ipaddress.ip_address(host) + except ValueError: + return False + + if ip.is_loopback or ip.is_private or ip.is_link_local: + return True + + # Tailscale/other VPN setups often sit in carrier-grade NAT space. + if ip.version == 4 and ipaddress.ip_address("100.64.0.0") <= ip <= ipaddress.ip_address("100.127.255.255"): + return True + + return False + + def _resolve_optional_float(*values: Any) -> float | None: """Return the first non-empty value coerced to a positive float.""" for value in values: @@ -490,9 +534,7 @@ class HonchoClientConfig: ) base_url = ( - host_block.get("baseUrl") - or host_block.get("base_url") - or raw.get("baseUrl") + raw.get("baseUrl") or raw.get("base_url") or os.environ.get("HONCHO_BASE_URL", "").strip() or None @@ -937,16 +979,12 @@ def get_honcho_client(config: HonchoClientConfig | None = None) -> Honcho: # For local: only use config.api_key if the host block explicitly # sets apiKey (meaning the user wants local auth). Otherwise skip # the stored key -- it's likely a cloud key that would break local. - _is_local = resolved_base_url and ( - "localhost" in resolved_base_url - or "127.0.0.1" in resolved_base_url - or "::1" in resolved_base_url - ) + _is_local = _is_local_base_url(resolved_base_url) if _is_local: # Check if the host block has its own apiKey (explicit local auth). - # Auth-skipping is loopback-only: a stored key is likely a cloud key - # that would break a no-auth local server, so we substitute the SDK's - # required-non-empty placeholder unless the host block opts in. + # For local/LAN/VPN self-hosts, a stored root key is likely a cloud + # key that would break a no-auth local server, so we substitute the + # SDK's required-non-empty placeholder unless the host block opts in. _raw = config.raw or {} _host_block = (_raw.get("hosts") or {}).get(config.host, {}) _host_has_key = bool(_host_block.get("apiKey")) diff --git a/tests/honcho_plugin/test_client.py b/tests/honcho_plugin/test_client.py index 858b98a55545..85e87579260c 100644 --- a/tests/honcho_plugin/test_client.py +++ b/tests/honcho_plugin/test_client.py @@ -3,6 +3,8 @@ import importlib.util import json import os +import sys +import types from pathlib import Path from unittest.mock import patch, MagicMock @@ -439,7 +441,11 @@ class TestResolveActiveHost: with patch.dict(os.environ, {}, clear=True): os.environ.pop("HERMES_HONCHO_HOST", None) os.environ.pop("HERMES_HOME", None) - assert resolve_active_host() == "hermes" + with patch( + "plugins.memory.honcho.client.resolve_config_path", + return_value=Path("/nonexistent/honcho.json"), + ): + assert resolve_active_host() == "hermes" def test_explicit_env_var_wins(self): with patch.dict(os.environ, {"HERMES_HONCHO_HOST": "hermes.coder"}): @@ -451,16 +457,52 @@ class TestResolveActiveHost: with patch("hermes_cli.profiles.get_active_profile_name", return_value="coder"): assert resolve_active_host() == "hermes_coder" + def test_default_host_does_not_override_named_profile(self, tmp_path): + """defaultHost is not applied before active-profile resolution.""" + config_file = tmp_path / "honcho.json" + config_file.write_text(json.dumps({ + "defaultHost": "local", + "hosts": {"local": {"workspace": "local-ws"}}, + })) + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("HERMES_HONCHO_HOST", None) + with patch("hermes_cli.profiles.get_active_profile_name", return_value="coder"), \ + patch("plugins.memory.honcho.client.resolve_config_path", return_value=config_file): + assert resolve_active_host() == "hermes_coder" + + def test_default_host_applies_to_default_profile_only(self, tmp_path): + """default profile can use setup-generated defaultHost without leaking to other profiles.""" + config_file = tmp_path / "honcho.json" + config_file.write_text(json.dumps({ + "defaultHost": "local", + "hosts": {"local": {"workspace": "local-ws"}}, + })) + + with patch.dict(os.environ, {}, clear=False): + os.environ.pop("HERMES_HONCHO_HOST", None) + with patch("hermes_cli.profiles.get_active_profile_name", return_value="default"), \ + patch("plugins.memory.honcho.client.resolve_config_path", return_value=config_file): + assert resolve_active_host() == "local" + def test_default_profile_returns_hermes(self): with patch.dict(os.environ, {}, clear=False): os.environ.pop("HERMES_HONCHO_HOST", None) - with patch("hermes_cli.profiles.get_active_profile_name", return_value="default"): + with patch("hermes_cli.profiles.get_active_profile_name", return_value="default"), \ + patch( + "plugins.memory.honcho.client.resolve_config_path", + return_value=Path("/nonexistent/honcho.json"), + ): assert resolve_active_host() == "hermes" def test_custom_profile_returns_hermes(self): with patch.dict(os.environ, {}, clear=False): os.environ.pop("HERMES_HONCHO_HOST", None) - with patch("hermes_cli.profiles.get_active_profile_name", return_value="custom"): + with patch("hermes_cli.profiles.get_active_profile_name", return_value="custom"), \ + patch( + "plugins.memory.honcho.client.resolve_config_path", + return_value=Path("/nonexistent/honcho.json"), + ): assert resolve_active_host() == "hermes" def test_profiles_import_failure_falls_back(self): @@ -976,6 +1018,74 @@ class TestGetHonchoClientBaseUrlDoublePrefixFix: f"Expected 'http://localhost:38000', got {passed_base_url!r}" ) + def test_lan_default_host_empty_key_uses_local_placeholder(self, tmp_path): + """Regression for #61661: setup-style root baseUrl + defaultHost + LAN IP + must not pass an empty/None api_key to the SDK for a no-auth local server.""" + config_file = tmp_path / "honcho.json" + config_file.write_text(json.dumps({ + "defaultHost": "local", + "baseUrl": "http://192.168.2.112:8000", + "hosts": { + "local": { + "workspace": "local-ws", + "aiPeer": "local-ai", + "apiKey": "", + }, + }, + })) + + with patch.dict(os.environ, {}, clear=True), \ + patch("hermes_cli.profiles.get_active_profile_name", return_value="default"), \ + patch("plugins.memory.honcho.client.resolve_config_path", return_value=config_file): + cfg = HonchoClientConfig.from_global_config(config_path=config_file) + + assert cfg.host == "local" + assert cfg.workspace_id == "local-ws" + assert cfg.ai_peer == "local-ai" + assert cfg.api_key is None + assert cfg.base_url == "http://192.168.2.112:8000" + + fake_honcho = MagicMock(name="Honcho") + mock_honcho = MagicMock(return_value=fake_honcho) + fake_honcho_module = types.SimpleNamespace(Honcho=mock_honcho) + with patch.dict(sys.modules, {"honcho": fake_honcho_module}), \ + patch("hermes_cli.config.load_config", return_value={}): + get_honcho_client(cfg) + + mock_honcho.assert_called_once() + assert mock_honcho.call_args.kwargs["api_key"] == "local" + assert mock_honcho.call_args.kwargs["base_url"] == "http://192.168.2.112:8000" + + def test_lan_default_host_explicit_host_key_preserved(self, tmp_path): + """A host-block local JWT still wins for LAN/VPN local URLs.""" + config_file = tmp_path / "honcho.json" + config_file.write_text(json.dumps({ + "defaultHost": "local", + "baseUrl": "http://192.168.2.112:8000", + "hosts": { + "local": { + "workspace": "local-ws", + "aiPeer": "local-ai", + "apiKey": "local-jwt", + }, + }, + })) + + with patch.dict(os.environ, {}, clear=True), \ + patch("hermes_cli.profiles.get_active_profile_name", return_value="default"), \ + patch("plugins.memory.honcho.client.resolve_config_path", return_value=config_file): + cfg = HonchoClientConfig.from_global_config(config_path=config_file) + + fake_honcho = MagicMock(name="Honcho") + mock_honcho = MagicMock(return_value=fake_honcho) + fake_honcho_module = types.SimpleNamespace(Honcho=mock_honcho) + with patch.dict(sys.modules, {"honcho": fake_honcho_module}), \ + patch("hermes_cli.config.load_config", return_value={}): + get_honcho_client(cfg) + + mock_honcho.assert_called_once() + assert mock_honcho.call_args.kwargs["api_key"] == "local-jwt" + @pytest.mark.skipif( not importlib.util.find_spec("honcho"), reason="honcho SDK not installed"