From 606481586a0666847a0882e41daa12e00ee3f7c1 Mon Sep 17 00:00:00 2001 From: Erosika Date: Mon, 10 Aug 2026 19:12:41 -0400 Subject: [PATCH] fix(honcho): honor explicit top-level apiKey on local base_urls; warn on keyless profile host blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two silent-auth-failure paths from #36098 (also #66125): - the local-URL guard only escaped the 'local' placeholder when the HOST BLOCK had apiKey. A top-level apiKey in honcho.json — explicit user intent, and what 'hermes honcho setup' writes for single-host configs — was dropped on the floor, so AUTH_USE_AUTH self-hosts 401'd on every request. Now any explicit key in honcho.json (host block or top level) is honored; only env-sourced keys are still treated as likely-cloud and skipped for local URLs. - named-profile host blocks do not inherit the default host's apiKey (credential isolation is by design), but the failure was silent: the profile ran unauthenticated and every tool said 'no context'. Affirm isolation and warn loudly at config-resolution time instead, the outcome #66125 proposed if inheritance is rejected. --- plugins/memory/honcho/client.py | 35 +++++++++++++++----- tests/honcho_plugin/test_client.py | 28 ++++++++++++++-- tests/test_honcho_client_config.py | 53 ++++++++++++++++++++++++++++++ 3 files changed, 105 insertions(+), 11 deletions(-) diff --git a/plugins/memory/honcho/client.py b/plugins/memory/honcho/client.py index 05614d3c69b21..8a2d019ddfccb 100644 --- a/plugins/memory/honcho/client.py +++ b/plugins/memory/honcho/client.py @@ -578,6 +578,23 @@ class HonchoClientConfig: or raw.get("apiKey") or get_secret("HONCHO_API_KEY") ) + # Named-profile host blocks do NOT inherit the default host's apiKey — + # profiles are isolated islands by design (see resolve_active_host). + # But the failure mode is silent: the profile runs unauthenticated and + # every write 401s while tools report "no context". Warn loudly so the + # operator learns the key must be set on THIS host block (#36098, #66125). + if ( + not api_key + and host_block + and resolved_host != HOST + and _host_block(raw, HOST).get("apiKey") + ): + logger.warning( + "Honcho host block '%s' has no apiKey; the default '%s' host's key " + "is NOT inherited (profiles are credential-isolated). Set apiKey on " + "hosts.%s in %s or this profile runs unauthenticated.", + resolved_host, HOST, resolved_host, path, + ) environment = ( host_block.get("environment") @@ -1299,19 +1316,19 @@ def get_honcho_client(config: HonchoClientConfig | None = None) -> Honcho: # Local Honcho instances don't require an API key, but the SDK # expects a non-empty string. Use a placeholder for local URLs. - # 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. + # For local: honor config.api_key when the user set it EXPLICITLY in + # honcho.json — host block or top-level (#36098 issue 2: the top-level + # key was dropped for the placeholder, 401ing AUTH_USE_AUTH=true + # self-hosts). Only an env-sourced key (HONCHO_API_KEY) is still + # treated as likely-cloud and skipped for local URLs. _is_local = _is_local_base_url(resolved_base_url) if _is_local: - # Check if the host block has its own apiKey (explicit local auth). - # 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_local = _host_block(_raw, config.host) # uses dot-form legacy fallback (#37436) - _host_has_key = bool(_host_block_local.get("apiKey")) - effective_api_key = config.api_key if _host_has_key else "local" + _explicit_key = bool( + _host_block_local.get("apiKey") or _raw.get("apiKey") + ) + effective_api_key = config.api_key if _explicit_key else "local" else: effective_api_key = config.api_key diff --git a/tests/honcho_plugin/test_client.py b/tests/honcho_plugin/test_client.py index 008a52b27aaec..630a5bf5cd13e 100644 --- a/tests/honcho_plugin/test_client.py +++ b/tests/honcho_plugin/test_client.py @@ -416,8 +416,9 @@ class TestGetHonchoClient: reason="honcho SDK not installed" ) def test_local_base_url_without_host_key_uses_placeholder(self): - """Without an explicit host-block apiKey, a local base_url still gets - the SDK's non-empty placeholder instead of the (likely cloud) root key.""" + """Without an explicit apiKey anywhere in honcho.json, a local + base_url gets the SDK's non-empty placeholder instead of the (likely + cloud, env-sourced) resolved key.""" fake_honcho = MagicMock(name="Honcho") cfg = HonchoClientConfig( api_key="cloud-root-key", @@ -432,6 +433,29 @@ class TestGetHonchoClient: assert mock_honcho.call_args.kwargs["api_key"] == "local" + @pytest.mark.skipif( + not importlib.util.find_spec("honcho"), + reason="honcho SDK not installed" + ) + def test_local_base_url_honors_top_level_api_key(self): + """Regression for #36098 issue 2: a top-level apiKey in honcho.json is + explicit user intent and must be honored for local base_urls (AUTH_USE_AUTH + self-hosts). Previously only a host-block apiKey escaped the 'local' + placeholder, so the top-level key was dropped and every request 401'd.""" + fake_honcho = MagicMock(name="Honcho") + cfg = HonchoClientConfig( + api_key="explicit-top-level-key", + base_url="http://localhost:8000", + host="hermes", + workspace_id="hermes", + raw={"apiKey": "explicit-top-level-key"}, + ) + + with patch("honcho.Honcho", return_value=fake_honcho) as mock_honcho: + get_honcho_client(cfg) + + assert mock_honcho.call_args.kwargs["api_key"] == "explicit-top-level-key" + @pytest.mark.skipif( not importlib.util.find_spec("honcho"), reason="honcho SDK not installed" diff --git a/tests/test_honcho_client_config.py b/tests/test_honcho_client_config.py index 667a4b092e7d7..b23ac219742e5 100644 --- a/tests/test_honcho_client_config.py +++ b/tests/test_honcho_client_config.py @@ -151,3 +151,56 @@ class TestHonchoBaseUrlSanitize: monkeypatch.delenv('HONCHO_API_KEY', raising=False) cfg = HonchoClientConfig.from_env() assert cfg.base_url is None + + +class TestProfileKeyIsolationWarning: + """#36098 / #66125: a named-profile host block without apiKey does NOT + inherit the default host's key (isolation by design), but the failure + must be loud, not silent.""" + + def test_keyless_profile_block_warns_when_default_has_key(self, tmp_path, monkeypatch, caplog): + import logging + monkeypatch.delenv('HONCHO_API_KEY', raising=False) + config_path = tmp_path / 'config.json' + config_path.write_text(json.dumps({ + 'hosts': { + 'hermes': {'apiKey': 'shared-key'}, + 'hermes_coder': {'baseUrl': 'http://192.168.1.50:8000'}, + }, + })) + with caplog.at_level(logging.WARNING, logger='plugins.memory.honcho.client'): + cfg = HonchoClientConfig.from_global_config( + host='hermes_coder', config_path=config_path, + ) + assert cfg.api_key is None # isolation preserved — no silent inheritance + assert any('NOT inherited' in r.message for r in caplog.records) + + def test_no_warning_when_profile_block_has_key(self, tmp_path, monkeypatch, caplog): + import logging + monkeypatch.delenv('HONCHO_API_KEY', raising=False) + config_path = tmp_path / 'config.json' + config_path.write_text(json.dumps({ + 'hosts': { + 'hermes': {'apiKey': 'shared-key'}, + 'hermes_coder': {'apiKey': 'coder-key'}, + }, + })) + with caplog.at_level(logging.WARNING, logger='plugins.memory.honcho.client'): + cfg = HonchoClientConfig.from_global_config( + host='hermes_coder', config_path=config_path, + ) + assert cfg.api_key == 'coder-key' + assert not any('NOT inherited' in r.message for r in caplog.records) + + def test_no_warning_for_default_host(self, tmp_path, monkeypatch, caplog): + import logging + monkeypatch.delenv('HONCHO_API_KEY', raising=False) + config_path = tmp_path / 'config.json' + config_path.write_text(json.dumps({ + 'hosts': {'hermes': {'baseUrl': 'http://localhost:8000'}}, + })) + with caplog.at_level(logging.WARNING, logger='plugins.memory.honcho.client'): + HonchoClientConfig.from_global_config( + host='hermes', config_path=config_path, + ) + assert not any('NOT inherited' in r.message for r in caplog.records)