From 569326577556d350739efedd2363a681359db5da Mon Sep 17 00:00:00 2001 From: HumphreySun98 Date: Wed, 8 Jul 2026 16:41:52 -0500 Subject: [PATCH] fix(web): don't crash on a null web/backend config value MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_load_web_config()` is typed `-> dict` but returned `load_config().get("web", {})`, which is `None` when the config has a present-but-null `web:` section (YAML `web:` with no body). Every caller then does `_load_web_config().get(...)` and raises `AttributeError: 'NoneType' object has no attribute 'get'` — this hits `_get_backend`, `check_web_api_key`, and the extract-char-limit reader. Separately, `check_web_api_key()` read the backend as `.get("backend", "").lower()`; a null `web.backend` value yields `None` (the `""` default only applies when the key is absent), so `None.lower()` raised. `check_web_api_key` is the `check_fn` gate for `web_search`/`web_extract`, so this surfaced as an exception during tool-availability checking. - Make `_load_web_config()` honor its `-> dict` contract (`... or {}`), fixing the null-`web:`-section crash at every call site. - Guard the backend value in `check_web_api_key` with `or ""`, mirroring the existing guard in `_get_backend`. Adds regression tests for both the null-backend-value and null-web-section cases. --- tests/tools/test_web_tools_config.py | 16 ++++++++++++++++ tools/web_tools.py | 10 ++++++++-- 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/tests/tools/test_web_tools_config.py b/tests/tools/test_web_tools_config.py index 5fb88807f54..aac919b7a32 100644 --- a/tests/tools/test_web_tools_config.py +++ b/tests/tools/test_web_tools_config.py @@ -576,6 +576,22 @@ class TestCheckWebApiKey: from tools.web_tools import check_web_api_key assert check_web_api_key() is True + def test_null_backend_value_does_not_crash(self): + # config.yaml with ``web:\n backend:`` yields backend=None. The gate + # must not raise AttributeError on None.lower() — mirrors _get_backend. + with patch("tools.web_tools._load_web_config", return_value={"backend": None}): + from tools.web_tools import check_web_api_key + assert check_web_api_key() is False + + def test_null_web_section_does_not_crash(self): + # config.yaml with a present-but-null ``web:`` section makes the raw + # ``.get("web", {})`` return None; _load_web_config must still yield a + # dict so no caller does None.get(...). + with patch("hermes_cli.config.load_config", return_value={"web": None}): + from tools.web_tools import _load_web_config, check_web_api_key + assert _load_web_config() == {} + assert check_web_api_key() is False + def test_firecrawl_key_only(self): with patch.dict(os.environ, {"FIRECRAWL_API_KEY": "fc-test"}): from tools.web_tools import check_web_api_key diff --git a/tools/web_tools.py b/tools/web_tools.py index 409b46fa606..132d78c7d74 100644 --- a/tools/web_tools.py +++ b/tools/web_tools.py @@ -132,7 +132,11 @@ def _load_web_config() -> dict: """Load the ``web:`` section from ~/.hermes/config.yaml.""" try: from hermes_cli.config import load_config - return load_config().get("web", {}) + # ``or {}``: a present-but-null ``web:`` section (YAML ``web:`` with no + # body) makes ``.get("web", {})`` return None, which would break every + # caller that does ``_load_web_config().get(...)``. Honor the ``-> dict`` + # contract so callers never see None. + return load_config().get("web") or {} except (ImportError, Exception): return {} @@ -1004,7 +1008,9 @@ def check_web_api_key() -> bool: :func:`_is_backend_available`, which delegates non-legacy names to the registry. """ - configured = _load_web_config().get("backend", "").lower().strip() + # ``or ""``: a null ``web.backend`` value yields None from ``.get``, and + # ``None.lower()`` would raise. Mirrors ``_get_backend``. + configured = (_load_web_config().get("backend") or "").lower().strip() if configured and _is_backend_available(configured): return True # Any built-in backend with credentials present. This is a boolean OR, so