fix(mcp): isolate orphan-reap sleep so test patches don't catch other threads

hermes_cli/proxy/adapters/nous_portal.py

@@ -15,6 +15,8 @@
 import threading
 from typing import Any, Dict, FrozenSet, Optional
 
+from urllib.parse import urlparse
+
 from hermes_cli.auth import (
     DEFAULT_NOUS_INFERENCE_URL,
     _load_auth_store,
@@ -26,6 +28,46 @@
 
 logger = logging.getLogger(__name__)
 
+# Allowlist of hosts the Nous Portal proxy is willing to forward minted
+# bearer tokens to. The bearer is a long-lived agent_key minted by
+# portal.nousresearch.com — sending it anywhere else would leak it.
+_ALLOWED_INFERENCE_HOSTS: FrozenSet[str] = frozenset({
+    "inference-api.nousresearch.com",
+})
+
+
+def _validate_nous_inference_url(url: str) -> str:
+    """Return *url* if it points at an allowlisted Nous inference host,
+    otherwise fall back to the documented default.
+
+    Defense-in-depth: ``inference_base_url`` comes from the Portal's
+    refresh / agent-key-mint response and from on-disk ``auth.json``.
+    Both are normally trustworthy, but a compromised refresh response
+    (MITM on portal.nousresearch.com, malicious local process writing
+    auth.json) could otherwise redirect every subsequent proxy request
+    — bearing the user's minted agent_key — to an attacker-controlled
+    endpoint. Validating scheme + host closes that loop.
+    """
+    try:
+        parsed = urlparse(url)
+    except Exception:
+        return DEFAULT_NOUS_INFERENCE_URL
+    if parsed.scheme != "https":
+        logger.warning(
+            "proxy: refusing non-https inference_base_url scheme %r; "
+            "falling back to default",
+            parsed.scheme,
+        )
+        return DEFAULT_NOUS_INFERENCE_URL
+    if parsed.hostname not in _ALLOWED_INFERENCE_HOSTS:
+        logger.warning(
+            "proxy: refusing inference_base_url host %r not in allowlist; "
+            "falling back to default",
+            parsed.hostname,
+        )
+        return DEFAULT_NOUS_INFERENCE_URL
+    return url.rstrip("/")
+
 # Endpoints inference-api.nousresearch.com actually serves. Anything else
 # the proxy will reject with 404 — keeps stray clients from leaking weird
 # requests to the upstream.
@@ -95,8 +137,8 @@ def get_credential(self) -> UpstreamCredential:
                     "Try `hermes login nous` to re-authenticate."
                 )
 
-            base_url = refreshed.get("inference_base_url") or DEFAULT_NOUS_INFERENCE_URL
-            base_url = base_url.rstrip("/")
+            raw_base_url = refreshed.get("inference_base_url") or DEFAULT_NOUS_INFERENCE_URL
+            base_url = _validate_nous_inference_url(raw_base_url)
 
             return UpstreamCredential(
                 bearer=agent_key,

tests/tools/test_mcp_stability.py

@@ -135,7 +135,7 @@ def test_kill_orphaned_uses_sigkill_when_available(self, monkeypatch):
         # bpo-14484). Return True so the SIGKILL escalation fires.
         with patch("tools.mcp_tool.os.kill") as mock_kill, \
              patch("gateway.status._pid_exists", return_value=True), \
-             patch("tools.mcp_tool.time.sleep") as mock_sleep:
+             patch("tools.mcp_tool._orphan_reap_sleep") as mock_sleep:
             _kill_orphaned_mcp_children()
 
         # SIGTERM then SIGKILL; the alive check no longer touches os.kill.
@@ -163,12 +163,12 @@ def test_kill_orphaned_falls_back_without_sigkill(self, monkeypatch):
         monkeypatch.delattr(signal, "SIGKILL", raising=False)
 
         with patch("tools.mcp_tool.os.kill") as mock_kill, \
-             patch("tools.mcp_tool.time.sleep") as mock_sleep:
+             patch("tools.mcp_tool._orphan_reap_sleep") as mock_sleep:
             _kill_orphaned_mcp_children()
 
         # SIGTERM phase, alive check raises (process gone), no escalation
         mock_kill.assert_any_call(fake_pid, signal.SIGTERM)
-        assert mock_sleep.called
+        mock_sleep.assert_called_once_with(2)
 
         with _lock:
             assert fake_pid not in _orphan_stdio_pids

tools/mcp_tool.py

@@ -3500,6 +3500,18 @@ async def _shutdown():
     _stop_mcp_loop()
 
 
+def _orphan_reap_sleep(seconds: float) -> None:
+    """Indirection over ``time.sleep`` for the orphan-reap SIGTERM→SIGKILL gap.
+
+    Tests patch this symbol instead of ``time.sleep`` so unrelated background
+    sleepers (pytest-xdist workers, MCP heartbeat threads, etc.) don't pollute
+    the mock's call list — patching ``time.sleep`` (or ``tools.mcp_tool.time.sleep``,
+    which resolves to the same module attribute) intercepts every thread in the
+    process.
+    """
+    time.sleep(seconds)
+
+
 def _kill_orphaned_mcp_children(include_active: bool = False) -> None:
     """Best-effort graceful shutdown of stdio MCP subprocesses to reap orphans.
 
@@ -3542,7 +3554,7 @@ def _kill_orphaned_mcp_children(include_active: bool = False) -> None:
             pass
 
     # Phase 2: Wait for graceful exit
-    time.sleep(2)
+    _orphan_reap_sleep(2)
 
     # Phase 3: SIGKILL any survivors
     _sigkill = getattr(_signal, "SIGKILL", _signal.SIGTERM)