mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-10-26 13:16:24 +00:00
fix(memory): reject a Honcho base_url that can never resolve (#5607)
* fix(memory): reject a Honcho base_url that can never resolve
HonchoConfig only did str(...).rstrip('/') on backend_config.base_url, so a
scheme-less or non-http value was accepted at Gateway startup and then made
every memory read return empty and every write drop silently, with only a
per-operation WARNING. httpx turns 'localhost:8000' into the bogus scheme
'localhost:'. The same check already exists in the mem0 and OpenViking
backends, and honcho/README.md plus HonchoMemoryManager.from_config both
promise a bad URL fails fast at startup.
* fix(memory): cover the scheme-case and host-less variants of the guard
- drive the api_key-over-plain-HTTP check off urlsplit(...).scheme: urlsplit
lowercases the scheme while startswith("http://") does not, so
"HTTP://internal:8000" reached httpx over plain HTTP with the key set and
allow_insecure_http still False
- read hostname instead of netloc in the base_url guard, so an authority
without a host ("http://", "http://:8000") is rejected at config time like
the scheme-less shape this PR already removes
- test direct HonchoConfig(base_url=...) construction, the path the guard's
placement in __post_init__ exists to protect
This commit is contained in:
parent
39726573aa
commit
f250d11e73
@ -13,6 +13,7 @@ from collections.abc import Callable
|
||||
from dataclasses import dataclass, field
|
||||
from math import isfinite
|
||||
from typing import Any
|
||||
from urllib.parse import urlsplit
|
||||
|
||||
_ID_RE = re.compile(r"[^a-zA-Z0-9_-]+")
|
||||
|
||||
@ -87,6 +88,16 @@ class HonchoConfig:
|
||||
storage_path: str = ""
|
||||
|
||||
def __post_init__(self) -> None:
|
||||
# A scheme-less or non-http value is not a relative address: httpx turns
|
||||
# "localhost:8000" into the bogus scheme "localhost:", so every later
|
||||
# request fails while Gateway startup stays green. An authority without a
|
||||
# host ("http://", "http://:8000") is the same accepted-now, dead-later
|
||||
# shape, so the guard reads hostname rather than netloc. The mem0 and
|
||||
# OpenViking backends already reject this shape; README.md and
|
||||
# HonchoMemoryManager.from_config both promise a bad URL fails fast.
|
||||
parsed_base_url = urlsplit(self.base_url)
|
||||
if parsed_base_url.scheme not in {"http", "https"} or not parsed_base_url.hostname:
|
||||
raise ValueError("Honcho backend: base_url must be an absolute http:// or https:// URL")
|
||||
if not isfinite(self.timeout_seconds) or self.timeout_seconds <= 0:
|
||||
raise ValueError("Honcho backend: timeout_seconds must be a finite value > 0")
|
||||
if not isfinite(self.connect_timeout_seconds) or self.connect_timeout_seconds <= 0:
|
||||
@ -105,7 +116,10 @@ class HonchoConfig:
|
||||
base_url = str(cfg.get("base_url", "http://localhost:8000")).rstrip("/")
|
||||
api_key = cfg.get("api_key") or None
|
||||
allow_insecure = bool(cfg.get("allow_insecure_http", False))
|
||||
if api_key and base_url.startswith("http://") and not allow_insecure:
|
||||
# The parsed scheme, not startswith("http://"): a caller can write
|
||||
# "HTTP://internal:8000" and urlsplit/httpx both treat that as plain
|
||||
# HTTP, so a case-sensitive prefix test leaks the key unencrypted.
|
||||
if api_key and urlsplit(base_url).scheme == "http" and not allow_insecure:
|
||||
raise ValueError("Honcho backend: api_key over plain http requires backend_config.allow_insecure_http: true (the key would be sent unencrypted). Use https, or set the opt-in for local development.")
|
||||
return cls(
|
||||
base_url=base_url,
|
||||
|
||||
@ -59,6 +59,39 @@ class TestHonchoConfig:
|
||||
cfg = HonchoConfig.from_backend_config({"base_url": "http://host.docker.internal:8000"})
|
||||
assert cfg.api_key is None
|
||||
|
||||
@pytest.mark.parametrize("value", ["localhost:8000", "honcho.internal", "not a url", "", "ftp://localhost:8000", "http://", "http://:8000", "https://:8000", 12345])
|
||||
def test_malformed_base_url_rejected_as_config_error(self, value):
|
||||
with pytest.raises(ValueError, match="base_url must be an absolute"):
|
||||
HonchoConfig.from_backend_config({"base_url": value})
|
||||
|
||||
@pytest.mark.parametrize("value", ["localhost:8000", "http://", "http://:8000", "ftp://localhost:8000"])
|
||||
def test_direct_construction_hits_the_same_guard(self, value):
|
||||
"""The guard sits in ``__post_init__`` exactly so this path is covered: a
|
||||
caller that builds ``HonchoConfig`` without ``from_backend_config`` must
|
||||
not end up with a base_url httpx can never resolve."""
|
||||
with pytest.raises(ValueError, match="base_url must be an absolute"):
|
||||
HonchoConfig(base_url=value)
|
||||
|
||||
@pytest.mark.parametrize("value", ["HTTP://internal:8000", "HtTp://internal:8000", "http://internal:8000"])
|
||||
def test_http_scheme_case_does_not_bypass_the_insecure_key_guard(self, value):
|
||||
"""``urlsplit`` lowercases the scheme and httpx sends such a URL over plain
|
||||
HTTP, so the api_key guard has to read the parsed scheme rather than a
|
||||
case-sensitive ``http://`` prefix."""
|
||||
with pytest.raises(ValueError, match="allow_insecure_http"):
|
||||
HonchoConfig.from_backend_config({"base_url": value, "api_key": "sk-x"})
|
||||
cfg = HonchoConfig.from_backend_config(
|
||||
{"base_url": value, "api_key": "sk-x", "allow_insecure_http": True},
|
||||
)
|
||||
assert cfg.api_key == "sk-x"
|
||||
|
||||
@pytest.mark.parametrize("value", ["http://localhost:8000", "https://api.honcho.dev", "http://host.docker.internal:8000", "https://honcho.internal:8443"])
|
||||
def test_absolute_base_urls_still_accepted(self, value):
|
||||
assert HonchoConfig.from_backend_config({"base_url": f"{value}/"}).base_url == value
|
||||
|
||||
def test_manager_from_config_fails_fast_on_malformed_base_url(self):
|
||||
with pytest.raises(ValueError, match="base_url must be an absolute"):
|
||||
HonchoMemoryManager.from_config({"base_url": "localhost:8000"})
|
||||
|
||||
def test_empty_override_values_rejected(self):
|
||||
"""An override entry with an empty/null value is a config mistake: silently
|
||||
falling through to the default derivation (empty string is falsy) or
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user