fix(honcho): keep legacy schemeless baseUrl configs working

The scheme-validation commit (e77a3f2c) was too strict: a user with
legacy ''baseUrl: localhost:8000'' (no ''http://'' prefix) in their
''~/.honcho/config.json'' would get ''No API key configured'' from the
CLI after that change, even though their setup worked before.

urlparse on a schemeless host:port treats the host segment as the
scheme and leaves netloc empty, so the http/https check rejected it.

Falls back to a lenient check for schemeless strings that look like
hosts: contain '.' or ':', aren't a boolean/null literal, aren't pure
digits. The SDK still rejects truly malformed URLs at connect time
with a clearer error than ours.

Three new tests: legacy schemeless hosts accepted; obvious garbage
literals (''true'', ''null'', ''12345'') still rejected.  Reviewer
noted concern #1: schemeless regression for self-hosters with old
configs.
This commit is contained in:
Erosika
2026-04-26 11:55:33 -04:00
committed by kshitij
parent cd276eef78
commit 5883df5574
2 changed files with 48 additions and 7 deletions

View File

@@ -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

View File

@@ -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):