harden: runner client rejects non-http(s) base_url + no redirect-follow (#3073)

Defense-in-depth flagged by both pre-release reviewers (Opus + Codex), both
non-blocking but cheap on a credential-handling surface:
- reject any non-http(s) HERMES_WEBUI_RUNNER_BASE_URL scheme at construction
  (a misconfigured file:// / ftp:// can never reach urlopen);
- route requests through an opener that does NOT follow 3xx redirects, so a
  misbehaving/compromised runner cannot smuggle the Bearer token to another host.
Both operator-misconfiguration-only (not user-reachable). +2 regression tests.

Co-authored-by: AJV20 <AJV20@users.noreply.github.com>

api/runner_client.py

@@ -35,6 +35,14 @@ class HttpRunnerClient:
         self.base_url = str(base_url or "").strip().rstrip("/")
         if not self.base_url:
             raise ValueError("runner base_url is required")
+        # Hardening: the runner endpoint is operator-configured, but reject any
+        # non-HTTP(S) scheme so a misconfigured HERMES_WEBUI_RUNNER_BASE_URL
+        # (e.g. file:///etc/passwd or ftp://) can never be handed to urlopen.
+        _scheme = urllib.parse.urlsplit(self.base_url).scheme.lower()
+        if _scheme not in ("http", "https"):
+            raise ValueError(
+                f"runner base_url must be http(s); got scheme '{_scheme or '(none)'}'"
+            )
         self.api_key = str(api_key or "").strip()
 
     @classmethod
@@ -118,9 +126,18 @@ class HttpRunnerClient:
         )
         return self._request_json(req)
 
+    def _opener(self) -> urllib.request.OpenerDirector:
+        # Hardening: do NOT follow redirects. A misbehaving/compromised runner
+        # returning 3xx Location could otherwise smuggle the Bearer token to
+        # another host. Treat any redirect as an error instead.
+        class _NoRedirect(urllib.request.HTTPRedirectHandler):
+            def redirect_request(self, *args, **kwargs):
+                return None
+        return urllib.request.build_opener(_NoRedirect)
+
     def _request_json(self, req: urllib.request.Request) -> dict[str, Any]:
         try:
-            with urllib.request.urlopen(req, timeout=60) as resp:
+            with self._opener().open(req, timeout=60) as resp:
                 raw = resp.read().decode("utf-8", errors="replace")
         except urllib.error.HTTPError as exc:
             try:

tests/test_runner_client.py

@@ -1,4 +1,5 @@
 import json
+import urllib.request
 
 import pytest
 
@@ -20,6 +21,22 @@ class FakeResponse:
         return json.dumps(self.payload).encode("utf-8")
 
 
+class _FakeOpener:
+    """Stand-in for the no-redirect opener: routes .open() to a fake urlopen."""
+
+    def __init__(self, fake_urlopen):
+        self._fake = fake_urlopen
+
+    def open(self, req, timeout=0):
+        return self._fake(req, timeout=timeout)
+
+
+def _patch_opener(monkeypatch, fake_urlopen):
+    monkeypatch.setattr(
+        HttpRunnerClient, "_opener", lambda self: _FakeOpener(fake_urlopen)
+    )
+
+
 def test_runner_client_is_default_off_without_endpoint():
     assert runner_client_configured({}) is False
     with pytest.raises(NotImplementedError, match="runner-local chat backend is not configured"):
@@ -36,7 +53,7 @@ def test_runner_client_start_run_posts_explicit_boundary_payload(monkeypatch):
         captured["body"] = json.loads(req.data.decode("utf-8"))
         return FakeResponse({"run_id": "run-1", "stream_id": "run-1", "status": "running"})
 
-    monkeypatch.setattr("api.runner_client.urllib.request.urlopen", fake_urlopen)
+    _patch_opener(monkeypatch, fake_urlopen)
     client = HttpRunnerClient(base_url="http://runner.local/", api_key="secret")
 
     result = client.start_run(
@@ -79,7 +96,7 @@ def test_runner_client_maps_observe_status_and_controls(monkeypatch):
         calls.append((req.get_method(), req.full_url, json.loads(req.data.decode("utf-8")) if req.data else None))
         return FakeResponse({"ok": True, "status": "accepted"})
 
-    monkeypatch.setattr("api.runner_client.urllib.request.urlopen", fake_urlopen)
+    _patch_opener(monkeypatch, fake_urlopen)
     client = HttpRunnerClient(base_url="http://runner.local")
 
     client.observe_run("run/1", cursor="event:2")
@@ -106,6 +123,34 @@ def test_runner_client_rejects_non_object_json(monkeypatch):
         def read(self):
             return b"[]"
 
-    monkeypatch.setattr("api.runner_client.urllib.request.urlopen", lambda req, timeout=0: ArrayResponse({}))
+    _patch_opener(monkeypatch, lambda req, timeout=0: ArrayResponse({}))
     with pytest.raises(RunnerClientError, match="non-object"):
         HttpRunnerClient(base_url="http://runner.local").get_run("r1")
+
+
+def test_runner_client_rejects_non_http_scheme():
+    """Hardening: a misconfigured base_url with a non-http(s) scheme must be
+    rejected at construction so it can never reach urlopen (e.g. file://)."""
+    for bad in ("file:///etc/passwd", "ftp://runner.local/x", "/no/scheme"):
+        with pytest.raises(ValueError, match="http"):
+            HttpRunnerClient(base_url=bad)
+    # http and https are accepted.
+    assert HttpRunnerClient(base_url="http://runner.local").base_url == "http://runner.local"
+    assert HttpRunnerClient(base_url="https://runner.local/").base_url == "https://runner.local"
+
+
+def test_runner_client_opener_does_not_follow_redirects():
+    """Hardening: the request opener must NOT follow 3xx redirects, so a
+    misbehaving runner cannot smuggle the Bearer token to another host."""
+    opener = HttpRunnerClient(base_url="http://runner.local")._opener()
+    redirect_handlers = [
+        h for h in opener.handlers
+        if isinstance(h, urllib.request.HTTPRedirectHandler)
+    ]
+    assert redirect_handlers, "expected a redirect handler on the opener"
+    # The overridden handler returns None from redirect_request → urllib raises
+    # instead of following the redirect.
+    assert all(
+        h.redirect_request(None, None, 302, "Found", {}, "http://evil.example") is None
+        for h in redirect_handlers
+    )