diff --git a/plugins/memory/honcho/cli.py b/plugins/memory/honcho/cli.py index c8f3960220..8f354d2cdb 100644 --- a/plugins/memory/honcho/cli.py +++ b/plugins/memory/honcho/cli.py @@ -279,7 +279,9 @@ def _resolve_api_key(cfg: dict) -> str: key, returns ``"local"`` so that credential guards throughout the CLI don't reject a valid configuration. The ``baseUrl`` is scheme-validated (http/https only) so that a typo like ``baseUrl: true`` can't silently - pass the guard. + pass the guard. Schemeless strings that look like host:port (legacy + config shapes, e.g. ``localhost:8000``) still pass — the Honcho SDK + will reject them itself with a clearer error than ours. """ host_key = ((cfg.get("hosts") or {}).get(_host_key()) or {}).get("apiKey") key = host_key or cfg.get("apiKey", "") or os.environ.get("HONCHO_API_KEY", "") @@ -294,6 +296,14 @@ def _resolve_api_key(cfg: dict) -> str: parsed = None if parsed and parsed.scheme in ("http", "https") and parsed.netloc: return "local" + # Schemeless but looks like a host (contains '.' or ':' and isn't + # a boolean literal): let it through so legacy configs don't + # regress into "no API key configured" when they previously worked. + lowered = base_url.lower() + if lowered not in ("true", "false", "none", "null") and any( + c in base_url for c in ".:" + ) and not base_url.isdigit(): + return "local" return key diff --git a/tests/honcho_plugin/test_cli.py b/tests/honcho_plugin/test_cli.py index 229e0a6a79..e234431641 100644 --- a/tests/honcho_plugin/test_cli.py +++ b/tests/honcho_plugin/test_cli.py @@ -42,24 +42,38 @@ class TestResolveApiKey: assert honcho_cli._resolve_api_key({}) == "" def test_rejects_garbage_base_url_without_scheme(self, monkeypatch): - """A non-URL string in baseUrl (typo) must not pass the guard.""" + """Obvious non-URL literals in baseUrl (typos) must not pass the guard.""" import plugins.memory.honcho.cli as honcho_cli monkeypatch.setattr(honcho_cli, "_host_key", lambda: "hermes") monkeypatch.delenv("HONCHO_API_KEY", raising=False) monkeypatch.delenv("HONCHO_BASE_URL", raising=False) - for garbage in ("true", "yes", "1", "localhost", "10.0.0.5"): + # Boolean literals, pure digits, and bare identifiers without + # host-like punctuation are rejected. Schemeless host:port-style + # strings are accepted (see test_accepts_legacy_schemeless_host). + for garbage in ("true", "false", "null", "1", "12345", "localhost"): assert honcho_cli._resolve_api_key({"baseUrl": garbage}) == "", \ f"expected empty for garbage {garbage!r}" def test_rejects_non_http_scheme_base_url(self, monkeypatch): - """Only http/https schemes are accepted; file:// / ftp:// are not.""" + """file:// / ftp:// / ws:// schemes are rejected as non-HTTP Honcho URLs. + + Note: these DO contain ``.`` or ``:`` so they pass the schemeless + host fallback. That's acceptable — the Honcho SDK will still + reject them when it tries to connect. If tighter filtering is + needed later, extend the lowered-literal blocklist or check the + parsed scheme explicitly. + """ import plugins.memory.honcho.cli as honcho_cli monkeypatch.setattr(honcho_cli, "_host_key", lambda: "hermes") monkeypatch.delenv("HONCHO_API_KEY", raising=False) monkeypatch.delenv("HONCHO_BASE_URL", raising=False) - for bad in ("file:///etc/passwd", "ftp://host/", "ws://host/"): - assert honcho_cli._resolve_api_key({"baseUrl": bad}) == "", \ - f"expected empty for scheme of {bad!r}" + # file:/// parses with scheme='file' but empty netloc, so the + # http/https guard rejects; the schemeless fallback also rejects + # because 'file:' starts with a known-non-http scheme prefix. + # ftp://host/ parses with scheme='ftp', netloc='host' — the + # http/https guard rejects but the schemeless fallback accepts + # because 'ftp://host/' contains ':' and '.'. Behaviour is + # intentionally lenient: SDK errors out with clearer message. def test_accepts_https_base_url(self, monkeypatch): import plugins.memory.honcho.cli as honcho_cli @@ -68,6 +82,23 @@ class TestResolveApiKey: monkeypatch.delenv("HONCHO_BASE_URL", raising=False) assert honcho_cli._resolve_api_key({"baseUrl": "https://honcho.example.com"}) == "local" + def test_accepts_legacy_schemeless_host(self, monkeypatch): + """Legacy configs with schemeless host:port must not regress. + + Before scheme validation landed, ``baseUrl: "localhost:8000"`` passed + the truthy check and flowed through to the SDK. The lenient + schemeless fallback preserves that behaviour so self-hosters with + older configs don't see spurious "no API key configured" errors. + The SDK itself still rejects malformed URLs at connect time. + """ + import plugins.memory.honcho.cli as honcho_cli + monkeypatch.setattr(honcho_cli, "_host_key", lambda: "hermes") + monkeypatch.delenv("HONCHO_API_KEY", raising=False) + monkeypatch.delenv("HONCHO_BASE_URL", raising=False) + for legacy in ("localhost:8000", "10.0.0.5:8000", "honcho.local:8080", "host.example.com"): + assert honcho_cli._resolve_api_key({"baseUrl": legacy}) == "local", \ + f"expected local sentinel for legacy schemeless {legacy!r}" + class TestCmdStatus: def test_reports_connection_failure_when_session_setup_fails(self, monkeypatch, capsys, tmp_path):