fix(honcho): honor explicit top-level apiKey on local base_urls; warn on keyless profile host blocks
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.
This commit is contained in:
parent
32238f9942
commit
606481586a
|
|
@ -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
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue