style(honcho): hoist hashlib import; validate baseUrl scheme before 'local' sentinel

Two small follow-ups to the PR review:

- Hoist hashlib import from _enforce_session_id_limit() to module top.
  stdlib imports are free after first cache, but keeping all imports at
  module top matches the rest of the codebase.

- _resolve_api_key now URL-parses baseUrl and requires http/https +
  non-empty netloc before returning the 'local' sentinel.  A typo like
  baseUrl: 'true' (or bare 'localhost') no longer silently passes the
  credential guard; the CLI correctly reports 'not configured'.

Three new tests cover the new validation (garbage strings, non-http
schemes, valid https).
This commit is contained in:
Erosika
2026-04-24 18:34:16 -04:00
committed by kshitij
parent 3b2edb347d
commit 02ab255a0d
3 changed files with 40 additions and 5 deletions

View File

@@ -277,14 +277,23 @@ def _resolve_api_key(cfg: dict) -> str:
For self-hosted instances configured with ``baseUrl`` instead of an API For self-hosted instances configured with ``baseUrl`` instead of an API
key, returns ``"local"`` so that credential guards throughout the CLI key, returns ``"local"`` so that credential guards throughout the CLI
don't reject a valid configuration. 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.
""" """
host_key = ((cfg.get("hosts") or {}).get(_host_key()) or {}).get("apiKey") 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", "") key = host_key or cfg.get("apiKey", "") or os.environ.get("HONCHO_API_KEY", "")
if not key: if not key:
base_url = cfg.get("baseUrl") or cfg.get("base_url") or os.environ.get("HONCHO_BASE_URL", "") base_url = cfg.get("baseUrl") or cfg.get("base_url") or os.environ.get("HONCHO_BASE_URL", "")
if base_url.strip(): base_url = (base_url or "").strip()
return "local" if base_url:
from urllib.parse import urlparse
try:
parsed = urlparse(base_url)
except (TypeError, ValueError):
parsed = None
if parsed and parsed.scheme in ("http", "https") and parsed.netloc:
return "local"
return key return key

View File

@@ -16,6 +16,7 @@ from __future__ import annotations
import json import json
import os import os
import logging import logging
import hashlib
from dataclasses import dataclass, field from dataclasses import dataclass, field
from pathlib import Path from pathlib import Path
@@ -571,8 +572,6 @@ class HonchoClientConfig:
if len(sanitized) <= max_len: if len(sanitized) <= max_len:
return sanitized return sanitized
import hashlib
hash_len = cls._HONCHO_SESSION_ID_HASH_LEN hash_len = cls._HONCHO_SESSION_ID_HASH_LEN
digest = hashlib.sha256(original.encode("utf-8")).hexdigest()[:hash_len] digest = hashlib.sha256(original.encode("utf-8")).hexdigest()[:hash_len]
# max_len - hash_len - 1 (for the '-' separator) chars of the sanitized # max_len - hash_len - 1 (for the '-' separator) chars of the sanitized

View File

@@ -41,6 +41,33 @@ class TestResolveApiKey:
monkeypatch.delenv("HONCHO_BASE_URL", raising=False) monkeypatch.delenv("HONCHO_BASE_URL", raising=False)
assert honcho_cli._resolve_api_key({}) == "" 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."""
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"):
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."""
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}"
def test_accepts_https_base_url(self, monkeypatch):
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)
assert honcho_cli._resolve_api_key({"baseUrl": "https://honcho.example.com"}) == "local"
class TestCmdStatus: class TestCmdStatus:
def test_reports_connection_failure_when_session_setup_fails(self, monkeypatch, capsys, tmp_path): def test_reports_connection_failure_when_session_setup_fails(self, monkeypatch, capsys, tmp_path):