From 9858b3dd8ea2501824e5b5b1d2dbfffdc35df0c8 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Sun, 28 Jun 2026 02:17:06 +0000 Subject: [PATCH] Fail closed on passwordless public bind in containers (#3758) Salvages the "public-bind-requires-auth" hardening (GAP 2) from the deserted PR #3758: refuse to start (sys.exit(1)) instead of merely warning when the WebUI would bind a public/non-loopback address with no password or passkey configured inside a container. - api/bind_guard.py: new module with `_public_bind_requires_auth(host, *, within_container, auth_enabled)` (auth-enabled and loopback never trip; honors HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND on/off override; otherwise defaults to within_container) and a crystal-clear multi-line refusal message (`public_bind_refusal_message`) that states what happened, why it is dangerous, the detected host, and the three fix paths: set a password (recommended), bind to 127.0.0.1, or explicitly opt out because another layer enforces access. Lives in its own module so server.py stays under its 750-line architectural budget. - server.py main(): fail closed for the dangerous case; the existing warn-only path is preserved unchanged for loopback, auth-enabled, and bare-metal/dev hosts. - Dockerfile: enable HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND by default so containers are protected out of the box (override with =0). - tests/test_security_review_fixes.py: added public-bind coverage (container blocks, loopback allows, auth disables, bare-metal warn-only, explicit flag on/off, Dockerfile default, message clarity). Does NOT include the /tmp media-root change from the original PR. Co-authored-by: fantasticsquirrel --- CHANGELOG.md | 2 + Dockerfile | 4 + api/bind_guard.py | 90 +++++++++++++++++++ server.py | 14 ++- tests/test_security_review_fixes.py | 133 ++++++++++++++++++++++++++++ 5 files changed, 240 insertions(+), 3 deletions(-) create mode 100644 api/bind_guard.py diff --git a/CHANGELOG.md b/CHANGELOG.md index ad7f49cd548..b25a158ccae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ - **Three new opt-in appearance skins: GitHub, Codex, and Terracotta.** All are CSS-only, namespaced under `[data-skin]`, and selectable from Settings → Appearance — they change nothing unless you pick them. GitHub uses a restrained graphite + Primer-blue palette; Codex a minimal editor look with a muted sage accent; Terracotta a warm clay accent on a soft neutral background. Thanks @gottipx (GitHub #4634, Codex #4636, Terracotta #4635 — renamed from the originally-proposed name to a descriptive material name). +- **A passwordless WebUI bound to a public address inside a container now refuses to start (fail-closed) instead of only warning.** Previously, starting the server on a non-loopback/public address (`0.0.0.0`, `::`, a public host) with no password or passkey configured printed a warning and continued — silently exposing an unauthenticated WebUI (filesystem + agent) to the network. Startup now hard-stops (`sys.exit(1)`) for the dangerous case and prints a crystal-clear block explaining what happened and exactly how to fix it: set a password (`HERMES_WEBUI_PASSWORD`, recommended), bind to localhost (`HERMES_WEBUI_HOST=127.0.0.1`), or explicitly opt out (`HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND=0`) if another layer (reverse proxy with auth, private network, VPN) already enforces access control. Loopback binds, auth-enabled servers, and bare-metal/dev hosts are unaffected — the guard defaults to on **only inside containers** (and the Docker image sets `HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND=1` so containers are protected out of the box); a bare-metal host keeps the historical warn-only behavior unless it explicitly opts in. Thanks @fantasticsquirrel. (#3758) + ### Fixed - **Settled assistant turns with interleaved text and tool calls now keep their original order, and no post-tool text is dropped.** When an assistant turn mixed prose with tool calls (text → tool → more text), the settled/reloaded transcript could lose the chronological ordering or silently drop post-tool text/thinking from a non-final assistant message. The Stable Assistant Turn Anchor now promotes a settled turn's mixed `content[]` into ordered scene rows (prose, tool, and thinking rows in sequence) instead of only recovering the raw fallback, and the backend hydration path mirrors the same scene model so a cold reload reconstructs an identical transcript. Only the turn-final assistant message's post-last-tool text is treated as the final answer; earlier assistant messages keep their post-tool content as activity rows. Tool-row de-duplication is conservative (it only merges rows it can positively confirm are the same call), biasing toward an extra visible card over ever silently losing one. Thanks @franksong2702. (#4958) diff --git a/Dockerfile b/Dockerfile index c792e9fc295..482d83da083 100644 --- a/Dockerfile +++ b/Dockerfile @@ -98,6 +98,10 @@ RUN echo "__version__ = '${HERMES_VERSION}'" > /apptoo/api/_version.py # Default to binding all interfaces (required for container networking) ENV HERMES_WEBUI_HOST=0.0.0.0 ENV HERMES_WEBUI_PORT=8787 +# Security: containers commonly publish 0.0.0.0 beyond the local machine, +# so fail closed (refuse to start) on a passwordless public bind by default. +# Operators who terminate auth at another layer can override with =0. +ENV HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND=1 EXPOSE 8787 diff --git a/api/bind_guard.py b/api/bind_guard.py new file mode 100644 index 00000000000..94982404894 --- /dev/null +++ b/api/bind_guard.py @@ -0,0 +1,90 @@ +"""Startup guard: refuse to expose a passwordless WebUI on a public address. + +Salvaged from PR #3758 (GAP 2 — "public bind requires auth"). This lives in its +own module so server.py stays within its architectural line budget while the +user-facing refusal message can be as long and clear as it needs to be. + +The decision (whether to refuse) and the ``sys.exit(1)`` still happen in +``server.main()``; this module only provides the predicate and the message text. +""" +from __future__ import annotations + +import os + +#: Env var that explicitly forces (``1``/on) or waives (``0``/off) the guard. +REQUIRE_AUTH_ENV = "HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND" + +#: Loopback hosts are local-only and never trip the guard. +_LOOPBACK_HOSTS = ('127.0.0.1', '::1', 'localhost') + + +def _public_bind_requires_auth(host: str, *, within_container: bool, auth_enabled: bool) -> bool: + """Whether startup should refuse a public (non-loopback) bind with no auth. + + Fails CLOSED for the dangerous case — a passwordless server bound to a + public/network address — instead of merely warning, so a WebUI can't be + unknowingly exposed to the network. Rules, in order: + + - If authentication is configured (password or passkey), never block. + - Loopback binds (127.0.0.1 / ::1 / localhost) are local-only, never block. + - ``HERMES_WEBUI_`` + ``REQUIRE_AUTH_FOR_PUBLIC_BIND`` is an explicit override: + off (0/false/no/off) -> never block (operator secured access elsewhere) + on (1/true/yes/on) -> always block a passwordless public bind + - Otherwise default to ``within_container``: containers commonly publish + 0.0.0.0 beyond the local machine, so they fail closed by default, while + bare-metal/dev hosts keep the historical warn-only behavior. + """ + if auth_enabled: + return False + if host in _LOOPBACK_HOSTS: + return False + flag = os.getenv(REQUIRE_AUTH_ENV, "").strip().lower() + if flag in ("0", "false", "no", "off"): + return False + if flag in ("1", "true", "yes", "on"): + return True + return bool(within_container) + + +def public_bind_refusal_message(host: str) -> str: + """Return the multi-line fatal message printed right before ``sys.exit(1)``. + + Crystal-clear by design: it states what was about to happen and why it is + dangerous, then lays out the three concrete fix paths (set a password, bind + to localhost, or explicitly opt out because another layer enforces access). + """ + bar = "=" * 74 + lines = [ + "", + bar, + " REFUSING TO START: passwordless server would be exposed on a public", + " address.", + bar, + "", + " WHAT HAPPENED", + " Hermes WebUI was about to bind to host %r with NO password" % (host,), + " and NO passkey configured. That address is public / reachable from", + " the network (it is not localhost), so anyone who can reach this", + " host could open the WebUI without logging in — reading your", + " sessions, files, and memory, and running commands as you.", + " Startup was stopped so this cannot happen by accident.", + "", + " HOW TO FIX IT (choose ONE, then restart)", + "", + " 1) Set a password [RECOMMENDED]", + " export HERMES_WEBUI_PASSWORD=your-s...rd", + " (or configure a password in Settings)", + "", + " 2) Bind to localhost only [if you only need local access,", + " e.g. reaching it over an SSH tunnel]", + " export HERMES_WEBUI_HOST=127.0.0.1", + "", + " 3) Already protected by another layer? [a reverse proxy that", + " enforces auth, a private network, or a VPN] — explicitly", + " opt out to acknowledge you have secured access another way:", + " export HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND=0", + "", + bar, + "", + ] + return "\n".join(lines) diff --git a/server.py b/server.py index 44dd9a2d8ec..460b3f6d877 100644 --- a/server.py +++ b/server.py @@ -136,6 +136,9 @@ def _blocked_socket_connect(self, address): logger = logging.getLogger(__name__) from api.auth import check_auth +# Re-export so `server._public_bind_requires_auth` stays a stable reference +# (the predicate + message live in api.bind_guard to keep server.py thin). +from api.bind_guard import _public_bind_requires_auth # noqa: F401 from api.config import HOST, PORT, STATE_DIR, SESSION_DIR, DEFAULT_WORKSPACE from api.helpers import ( j, @@ -586,16 +589,21 @@ def main() -> None: if within_container: print('[ok] Running within container.', flush=True) - # Security: warn if binding non-loopback without authentication + # Security: refuse (fail closed) or warn if binding non-loopback without auth from api.auth import is_auth_enabled - if HOST not in ('127.0.0.1', '::1', 'localhost') and not is_auth_enabled(): + from api.bind_guard import public_bind_refusal_message + auth_enabled = is_auth_enabled() + if _public_bind_requires_auth(HOST, within_container=within_container, auth_enabled=auth_enabled): + print(public_bind_refusal_message(HOST), flush=True) + sys.exit(1) + if HOST not in ('127.0.0.1', '::1', 'localhost') and not auth_enabled: print(f'[!!] WARNING: Binding to {HOST} with NO PASSWORD SET.', flush=True) print(f' Anyone on the network can access your filesystem and agent.', flush=True) print(f' Set a password via Settings or HERMES_WEBUI_PASSWORD env var.', flush=True) print(f' To suppress: bind to 127.0.0.1 or set a password.', flush=True) if within_container: print(f' Note: You are running within a container, must bind to 0.0.0.0 (IPv4) or :: (IPv6) to publish the port.', flush=True) - elif not is_auth_enabled(): + elif not auth_enabled: print(f' [tip] No password set. Any process on this machine can read sessions', flush=True) print(f' and memory via the local API. Set HERMES_WEBUI_PASSWORD to', flush=True) print(f' enable authentication.', flush=True) diff --git a/tests/test_security_review_fixes.py b/tests/test_security_review_fixes.py index d33d57c8012..476ee6a88e2 100644 --- a/tests/test_security_review_fixes.py +++ b/tests/test_security_review_fixes.py @@ -353,3 +353,136 @@ def fake_save_settings(body): assert handler.status == 200 assert any(key.lower() == "set-cookie" for key, _ in handler.sent_headers) + + +# ── Public-bind-requires-auth fail-closed guard (PR #3758, GAP 2) ────────── +# These cover server._public_bind_requires_auth and the Dockerfile default. +# The guard fails CLOSED (server sys.exit(1)s) for a passwordless server about +# to bind a public/non-loopback address inside a container, while keeping +# loopback, auth-enabled, and bare-metal-dev behavior unchanged. + +_REQUIRE_AUTH_FLAG = 'HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND' + + +def test_public_bind_blocks_passwordless_public_container(monkeypatch): + """Container + public host + no auth + no explicit flag -> BLOCK (fail closed).""" + import server + + monkeypatch.delenv(_REQUIRE_AUTH_FLAG, raising=False) + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=True, auth_enabled=False + ) is True + assert server._public_bind_requires_auth( + "::", within_container=True, auth_enabled=False + ) is True + assert server._public_bind_requires_auth( + "203.0.113.7", within_container=True, auth_enabled=False + ) is True + + +def test_public_bind_allows_loopback_even_in_container(monkeypatch): + """Loopback binds are local-only and must NEVER trip the guard.""" + import server + + monkeypatch.delenv(_REQUIRE_AUTH_FLAG, raising=False) + for host in ("127.0.0.1", "::1", "localhost"): + assert server._public_bind_requires_auth( + host, within_container=True, auth_enabled=False + ) is False + assert server._public_bind_requires_auth( + host, within_container=False, auth_enabled=False + ) is False + + +def test_public_bind_allows_when_auth_enabled(monkeypatch): + """Configured auth (password or passkey) disables the guard entirely.""" + import server + + monkeypatch.delenv(_REQUIRE_AUTH_FLAG, raising=False) + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=True, auth_enabled=True + ) is False + # Even with the explicit on-flag set, auth short-circuits to allow. + monkeypatch.setenv(_REQUIRE_AUTH_FLAG, '1') + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=True, auth_enabled=True + ) is False + + +def test_public_bind_bare_metal_dev_is_warn_only_by_default(monkeypatch): + """Bare-metal (non-container) dev host binding 0.0.0.0 passwordless does NOT + fail closed by default — it keeps the historical warn-only behavior.""" + import server + + monkeypatch.delenv(_REQUIRE_AUTH_FLAG, raising=False) + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=False, auth_enabled=False + ) is False + + +def test_public_bind_explicit_flag_on_blocks_even_bare_metal(monkeypatch): + """The explicit opt-in flag forces fail-closed even on a bare-metal host.""" + import server + + monkeypatch.setenv(_REQUIRE_AUTH_FLAG, '1') + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=False, auth_enabled=False + ) is True + for truthy in ("1", "true", "yes", "on", "ON", "True"): + monkeypatch.setenv(_REQUIRE_AUTH_FLAG, truthy) + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=False, auth_enabled=False + ) is True + + +def test_public_bind_explicit_flag_off_allows_even_in_container(monkeypatch): + """The explicit opt-out lets operators who secured access elsewhere (reverse + proxy / private network / VPN) run passwordless public binds in a container.""" + import server + + for falsy in ('0', "false", "no", "off", "OFF", "False"): + monkeypatch.setenv(_REQUIRE_AUTH_FLAG, falsy) + assert server._public_bind_requires_auth( + "0.0.0.0", within_container=True, auth_enabled=False + ) is False + + +def test_dockerfile_enables_require_auth_for_public_bind_by_default(): + """The Docker image must enable the fail-closed guard by default so a + container that publishes 0.0.0.0 is protected out of the box.""" + dockerfile = Path("Dockerfile").read_text(encoding="utf-8") + expected = "ENV " + _REQUIRE_AUTH_FLAG + "=" + '1' + assert expected in dockerfile, ( + "Dockerfile must set " + _REQUIRE_AUTH_FLAG + " on by default so " + "passwordless public binds in containers fail closed." + ) + + +def test_public_bind_refusal_message_is_clear_and_actionable(): + """Pin the crystal-clear fatal message: it must say what happened, why it + is dangerous, name the detected host, and lay out all three fix paths.""" + from api.bind_guard import public_bind_refusal_message + + msg = public_bind_refusal_message("0.0.0.0") + # Unmistakable lead-in + assert "REFUSING TO START" in msg + assert "passwordless server would be exposed on a public" in msg + # States WHAT happened + the detected host + that no auth is configured + assert "'0.0.0.0'" in msg + assert "NO password" in msg and "NO passkey" in msg + # Fix path 1: set a password (recommended) + assert "RECOMMENDED" in msg + assert ("HERMES_WEBUI_PASSWORD=") in msg + # Fix path 2: bind to localhost + assert ("HERMES_WEBUI_HOST=127.0.0.1") in msg + # Fix path 3: explicit opt-out for an externally-secured deployment + assert ("HERMES_WEBUI_REQUIRE_AUTH_FOR_PUBLIC_BIND=" + "0") in msg + assert "reverse proxy" in msg and "VPN" in msg + + +def test_public_bind_refusal_message_reflects_detected_host(): + """The refusal message echoes whatever public host was about to be bound.""" + from api.bind_guard import public_bind_refusal_message + + assert "'::'" in public_bind_refusal_message("::") + assert "'203.0.113.7'" in public_bind_refusal_message("203.0.113.7")