diff --git a/hermes_cli/doctor_live.py b/hermes_cli/doctor_live.py index 1c8dcb4d0e..7ddd130cb4 100644 --- a/hermes_cli/doctor_live.py +++ b/hermes_cli/doctor_live.py @@ -13,6 +13,7 @@ from typing import Callable, List, Optional from hermes_cli.browser_runtime import chromium_executable from hermes_cli.doctor import _section, check_info from hermes_cli.doctor_report import check_fail, check_ok, check_warn +from utils import normalize_proxy_env_vars DEFAULT_PROBE_TIMEOUT = 10.0 @@ -44,6 +45,10 @@ class ProbeResult: def _http_get(url: str, headers: Optional[dict] = None, timeout: Optional[float] = None): """Single HTTP GET seam for all metadata probes.""" import httpx + # Sanitize at the seam (idempotent, no-op on a clean env): a bracketed-IPv6 NO_PROXY entry + # ([::1], Clash Verge/mihomo) makes this bare trust_env client raise InvalidURL at + # construction (#118159), which _run_one's catch-all would dress up as backend downtime. + normalize_proxy_env_vars() return httpx.get(url, headers=headers or {}, timeout=timeout) diff --git a/tests/hermes_cli/test_doctor_live.py b/tests/hermes_cli/test_doctor_live.py index b4283d9897..e7f8e1fd65 100644 --- a/tests/hermes_cli/test_doctor_live.py +++ b/tests/hermes_cli/test_doctor_live.py @@ -6,6 +6,9 @@ All probes are mocked at the HTTP/client layer; no real network calls are made. from __future__ import annotations import argparse +import os +import threading +from http.server import BaseHTTPRequestHandler, HTTPServer from types import SimpleNamespace import pytest @@ -21,6 +24,30 @@ from tools import browser_tool_install as bt_install # to a constant, so TestBrowserAvailableNpxRung can exercise the real function. _real_browser_available = doctor_live._browser_available +# The exact environment Clash Verge / mihomo exports (issue #118159): a bracketed-IPv6 +# NO_PROXY entry that httpx 0.28.1 cannot parse into a proxy mount. +_CLASH_NO_PROXY = "127.0.0.1,localhost,::1,[::1]" + + +@pytest.fixture +def clash_env(monkeypatch): + for key in ("NO_PROXY", "no_proxy", "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", + "http_proxy", "https_proxy", "all_proxy"): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv("NO_PROXY", _CLASH_NO_PROXY) + monkeypatch.setenv("no_proxy", _CLASH_NO_PROXY) + + +class _Probes(BaseHTTPRequestHandler): + """Minimal 200-origin for live-path tests; every request is a bare metadata GET.""" + + def do_GET(self): + self.send_response(200) + self.end_headers() + + def log_message(self, format, *args): + pass # keep test output clean + def _args(live: bool = True) -> argparse.Namespace: return argparse.Namespace(live=live) @@ -154,6 +181,80 @@ class TestConfiguredOnlySelection: assert results["Browser"].status == "pass" +@pytest.mark.parametrize("kind", ["NO_PROXY", "no_proxy"]) +@pytest.mark.parametrize("entry", ["[::1]", "[::1]:8080", "::1/128"]) +def test_http_get_sanitizes_bracketed_ipv6_no_proxy(monkeypatch, kind, entry): + # The Clash Verge / mihomo export from #118159: httpx 0.28.1 turns a bracketed or + # CIDR IPv6 NO_PROXY entry into an unparseable mount pattern and raises InvalidURL + # at client construction, before any request. doctor --live's _http_get must not. + for key in ("NO_PROXY", "no_proxy", "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", + "http_proxy", "https_proxy", "all_proxy"): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv(kind, f"127.0.0.1,localhost,::1,{entry}") + # A bare local-socket GET proves the client was constructible; 127.0.0.1's own + # NO_PROXY membership keeps it out of any proxy's reach. + with HTTPServer(("127.0.0.1", 0), _Probes) as httpd: + thread = threading.Thread(target=httpd.serve_forever, daemon=True) + thread.start() + try: + resp = doctor_live._http_get( + f"http://127.0.0.1:{httpd.server_port}/", timeout=5.0) + assert resp.status_code == 200 + finally: + httpd.shutdown() + thread.join(timeout=5) + + +def test_run_live_checks_sanitizes_clash_env_and_probes_pass(clash_env, monkeypatch): + # Red-on-base end-to-end: _keyed_probe -> _http_get -> bare httpx.get raised + # InvalidURL under this env; _run_one's catch-all dressed it up as + # "(Invalid port: ':1]')" — a backend-failure verdict for a healthy service. + monkeypatch.setenv("FIRECRAWL_API_KEY", "fc-test") + + # Real HTTP origin: a 200 from an actual socket proves both client construction + # and the request reaching a reachable origin. The remote-credential probes would + # send real auth material or 401 without proving either. + with HTTPServer(("127.0.0.1", 0), _Probes) as httpd: + thread = threading.Thread(target=httpd.serve_forever, daemon=True) + thread.start() + try: + monkeypatch.setitem( + doctor_live._KEYED_PROBES, "Firecrawl", + (f"http://127.0.0.1:{httpd.server_port}/v2/team/credit-usage", + "FIRECRAWL_API_KEY", "Bearer")) + issues: list[str] = [] + results = {r.name: r for r in run_live_checks(issues)} + assert results["Firecrawl"].status == "pass" + assert issues == [] + assert "Invalid port" not in (results["Firecrawl"].detail or "") + finally: + httpd.shutdown() + thread.join(timeout=5) + # The seam rewrote the entry in place, the established pattern of this seam + # (chat transports and run_probes already do exactly this). + assert "[::1]" not in os.environ["NO_PROXY"] + assert os.environ["no_proxy"] == "127.0.0.1,localhost,::1" + + +def test_run_live_checks_leaves_a_clean_environment_untouched(monkeypatch): + for key in ("NO_PROXY", "no_proxy", "HTTP_PROXY", "HTTPS_PROXY", "ALL_PROXY", + "http_proxy", "https_proxy", "all_proxy"): + monkeypatch.delenv(key, raising=False) + monkeypatch.setenv("NO_PROXY", "example.com,10.0.0.0/8") + # _http_get is untouched by the sanitize-once design; a socket-level 200 proves + # the live path still works end to end when there is nothing to sanitize. + with HTTPServer(("127.0.0.1", 0), _Probes) as httpd: + thread = threading.Thread(target=httpd.serve_forever, daemon=True) + thread.start() + try: + assert doctor_live._http_get( + f"http://127.0.0.1:{httpd.server_port}/", timeout=5.0).status_code == 200 + finally: + httpd.shutdown() + thread.join(timeout=5) + assert os.environ["NO_PROXY"] == "example.com,10.0.0.0/8" + + class TestBrowserAvailable: """Live probes use the same passive selection as browser execution."""