diff --git a/CHANGELOG.md b/CHANGELOG.md index a12ed882..2e59c782 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,12 @@ ## [Unreleased] +## [v0.51.307] — 2026-06-06 — Release JW (stage-a3 — onboarding forwarded-IP spoof fix + update-check CSRF hardening) + +### Security +- **Unauthenticated first-run onboarding no longer trusts spoofable forwarded IP headers.** The local-network gate that lets onboarding run without a password now ignores `X-Forwarded-For` / `X-Real-IP` by default (a direct client can set them to a private address to bypass the gate), trusting them only when `HERMES_WEBUI_TRUST_FORWARDED_FOR=1` is explicitly set behind a trusted reverse proxy — and then using the rightmost, proxy-appended hop. When forwarded headers are present but untrusted, the request is treated as coming through a proxy and is denied (the raw socket is the proxy's, not the client's), so a public client behind any reverse proxy can't be read as local. **Reverse-proxy deployments that run onboarding without a password must set `HERMES_WEBUI_TRUST_FORWARDED_FOR=1` (or `HERMES_WEBUI_ONBOARDING_OPEN=1`).** Direct loopback clients are unaffected. (#3758, @fantasticsquirrel) +- **Update checks that hit the network/git are now CSRF-safe.** `GET /api/updates/check` is cache-only (no network or git mutation), so a state-changing update fetch can't be triggered by a bare cross-site navigation; the forced refresh moved to `POST /api/updates/check {force:true}`. Docker init env logging also masks `PASSWORD`/`SECRET`/`CREDENTIAL`/`COOKIE`/`SESSION` key names in addition to `TOKEN`/`API`/`KEY`. (Minor behavior tightening: `HERMES_WEBUI_ONBOARDING_OPEN` now only bypasses for canonical truthy values `1`/`true`/`yes`/`on` rather than any non-empty value.) (#3758, @fantasticsquirrel) + ## [v0.51.306] — 2026-06-06 — Release JV (stage-a2 — branchy compression lineage resolves to the freshest tip) ### Fixed diff --git a/api/routes.py b/api/routes.py index 069934f8..a38e01aa 100644 --- a/api/routes.py +++ b/api/routes.py @@ -1558,6 +1558,87 @@ def _client_ip_for_rate_limit(handler) -> str: return "unknown" +def _truthy_env(name: str) -> bool: + return os.getenv(name, "").strip().lower() in {"1", "true", "yes", "on"} + + +def _request_client_ip(handler) -> str: + try: + address = getattr(handler, "client_address", None) + if address: + return str(address[0] or "") + except Exception: + pass + return "" + + +def _onboarding_request_is_local(handler) -> bool: + """Return True when an unauthenticated onboarding request is local/private. + + Forwarded client-IP headers are ignored by default because direct clients can + spoof them. Operators behind a trusted reverse proxy may opt in with + HERMES_WEBUI_TRUST_FORWARDED_FOR=1, matching the explicit forwarded-header + trust model used elsewhere in the server. + + When forwarded headers are PRESENT but not trusted, the request arrived + through a proxy, so the raw socket address is the proxy's (typically + loopback/private) and tells us nothing about the real client's locality. + In that case we deny rather than fall back to the proxy socket — otherwise a + public client behind any reverse proxy would be treated as local. Operators + who front the WebUI with a trusted proxy must set + HERMES_WEBUI_TRUST_FORWARDED_FOR=1 (or HERMES_WEBUI_ONBOARDING_OPEN=1). + """ + import ipaddress + + trust_forwarded = _truthy_env("HERMES_WEBUI_TRUST_FORWARDED_FOR") + if trust_forwarded: + candidates = [ + handler.headers.get("X-Forwarded-For", "").split(",")[-1].strip(), + handler.headers.get("X-Real-IP", "").strip(), + _request_client_ip(handler), + ] + for raw in candidates: + if not raw: + continue + try: + addr = ipaddress.ip_address(raw) + except ValueError: + continue + return bool(addr.is_loopback or addr.is_private) + return False + + # Untrusted forwarded headers present → the request arrived through a proxy. + # Ignore the spoofable header and judge by the raw socket, but only LOOPBACK + # counts as local in that case: a loopback raw socket is a genuine same-host + # client (or a same-host proxy the operator controls), whereas a PRIVATE/LAN + # raw socket is a separate proxy box that could be forwarding an arbitrary + # (public) client we can't see without trusting the header. Operators who + # front the WebUI with a LAN proxy must set HERMES_WEBUI_TRUST_FORWARDED_FOR=1 + # (or HERMES_WEBUI_ONBOARDING_OPEN=1). + forwarded_present = bool( + handler.headers.get("X-Forwarded-For", "").strip() + or handler.headers.get("X-Real-IP", "").strip() + ) + raw = _request_client_ip(handler) + if not raw: + return False + try: + addr = ipaddress.ip_address(raw) + except ValueError: + return False + if forwarded_present: + return bool(addr.is_loopback) + return bool(addr.is_loopback or addr.is_private) + + +def _onboarding_gate_allows(handler) -> bool: + from api.auth import is_auth_enabled + + if is_auth_enabled() or _truthy_env("HERMES_WEBUI_ONBOARDING_OPEN"): + return True + return _onboarding_request_is_local(handler) + + def _csp_report_rate_limited(handler, *, now: float | None = None) -> bool: now = time.time() if now is None else now key = _client_ip_for_rate_limit(handler) @@ -5879,7 +5960,6 @@ def handle_get(handler, parsed) -> bool: return j(handler, {"disabled": True}) include_agent_updates = not bool(settings.get("ignore_agent_updates")) qs = parse_qs(parsed.query) - force = qs.get("force", ["0"])[0] == "1" # ?simulate=1 returns fake behind counts for UI testing (localhost only) if ( qs.get("simulate", ["0"])[0] == "1" @@ -5910,9 +5990,9 @@ def handle_get(handler, parsed) -> bool: "checked_at": 0, }, ) - from api.updates import check_for_updates + from api.updates import cached_update_status - return j(handler, check_for_updates(force=force, include_agent=include_agent_updates)) + return j(handler, cached_update_status(include_agent=include_agent_updates)) if parsed.path == "/api/chat/stream/status": stream_id = parse_qs(parsed.query).get("stream_id", [""])[0] @@ -6483,6 +6563,16 @@ def handle_post(handler, parsed) -> bool: diag.finish() raise + if parsed.path == "/api/updates/check": + settings = load_settings() + if not settings.get("check_for_updates", True): + return j(handler, {"disabled": True}) + include_agent_updates = not bool(settings.get("ignore_agent_updates")) + force = bool(body.get("force", False)) + from api.updates import check_for_updates + + return j(handler, check_for_updates(force=force, include_agent=include_agent_updates)) + if parsed.path == "/api/session/recovery/repair-safe": from api.session_recovery import repair_safe_session_recovery result = repair_safe_session_recovery(SESSION_DIR, state_db_path=_active_state_db_path()) @@ -7707,20 +7797,8 @@ def handle_post(handler, parsed) -> bool: return True if parsed.path == "/api/onboarding/oauth/start": - from api.auth import is_auth_enabled - import os as _os - if not is_auth_enabled() and not _os.getenv("HERMES_WEBUI_ONBOARDING_OPEN"): - import ipaddress - try: - _xff = handler.headers.get("X-Forwarded-For", "").split(",")[0].strip() - _xri = handler.headers.get("X-Real-IP", "").strip() - _raw = handler.client_address[0] - addr = ipaddress.ip_address(_xff or _xri or _raw) - is_local = addr.is_loopback or addr.is_private - except ValueError: - is_local = False - if not is_local: - return bad(handler, "Onboarding OAuth is only available from local networks when auth is not enabled. To bypass this on a remote server, set HERMES_WEBUI_ONBOARDING_OPEN=1.", 403) + if not _onboarding_gate_allows(handler): + return bad(handler, "Onboarding OAuth is only available from local networks when auth is not enabled. To bypass this on a remote server, set HERMES_WEBUI_ONBOARDING_OPEN=1.", 403) try: return j(handler, start_onboarding_oauth_flow(body), extra_headers={"Cache-Control": "no-store"}) except ValueError as e: @@ -7742,22 +7820,8 @@ def handle_post(handler, parsed) -> bool: # carries the real origin IP — read it first before falling back to the raw socket addr. # HERMES_WEBUI_ONBOARDING_OPEN=1 lets operators on remote servers explicitly bypass # the check when they control network access themselves (e.g. firewall + VPN). - from api.auth import is_auth_enabled - import os as _os - if not is_auth_enabled() and not _os.getenv("HERMES_WEBUI_ONBOARDING_OPEN"): - import ipaddress - try: - # Prefer forwarded headers set by reverse proxies - _xff = handler.headers.get("X-Forwarded-For", "").split(",")[0].strip() - _xri = handler.headers.get("X-Real-IP", "").strip() - _raw = handler.client_address[0] - _ip_str = _xff or _xri or _raw - addr = ipaddress.ip_address(_ip_str) - is_local = addr.is_loopback or addr.is_private - except ValueError: - is_local = False - if not is_local: - return bad(handler, "Onboarding setup is only available from local networks when auth is not enabled. To bypass this on a remote server, set HERMES_WEBUI_ONBOARDING_OPEN=1.", 403) + if not _onboarding_gate_allows(handler): + return bad(handler, "Onboarding setup is only available from local networks when auth is not enabled. To bypass this on a remote server, set HERMES_WEBUI_ONBOARDING_OPEN=1.", 403) try: return j(handler, apply_onboarding_setup(body)) except ValueError as e: @@ -7775,21 +7839,8 @@ def handle_post(handler, parsed) -> bool: # Read-only: no config.yaml or .env writes happen here. Same local- # network gate as /api/onboarding/setup (also writing-adjacent in # spirit because it carries an api_key the user typed). - from api.auth import is_auth_enabled - import os as _os - if not is_auth_enabled() and not _os.getenv("HERMES_WEBUI_ONBOARDING_OPEN"): - import ipaddress - try: - _xff = handler.headers.get("X-Forwarded-For", "").split(",")[0].strip() - _xri = handler.headers.get("X-Real-IP", "").strip() - _raw = handler.client_address[0] - _ip_str = _xff or _xri or _raw - addr = ipaddress.ip_address(_ip_str) - is_local = addr.is_loopback or addr.is_private - except ValueError: - is_local = False - if not is_local: - return bad(handler, "Onboarding probe is only available from local networks when auth is not enabled. To bypass this on a remote server, set HERMES_WEBUI_ONBOARDING_OPEN=1.", 403) + if not _onboarding_gate_allows(handler): + return bad(handler, "Onboarding probe is only available from local networks when auth is not enabled. To bypass this on a remote server, set HERMES_WEBUI_ONBOARDING_OPEN=1.", 403) provider = str((body or {}).get("provider") or "").strip().lower() base_url = str((body or {}).get("base_url") or "") api_key = str((body or {}).get("api_key") or "").strip() or None diff --git a/api/updates.py b/api/updates.py index 3547ae0c..5a3c0d3b 100644 --- a/api/updates.py +++ b/api/updates.py @@ -686,6 +686,19 @@ def _ignored_agent_update_info() -> dict: return {'name': 'agent', 'behind': 0, 'ignored': True} +def cached_update_status(*, include_agent=True): + """Return cached update status without performing network or git mutations.""" + include_agent = bool(include_agent) + with _cache_lock: + cached = dict(_update_cache) + if cached.get('include_agent') != include_agent: + cached['include_agent'] = include_agent + if not include_agent: + cached['agent'] = _ignored_agent_update_info() + cached['cached'] = True + return cached + + def check_for_updates(force=False, *, include_agent=True): """Return cached update status for webui and agent repos.""" global _check_in_progress diff --git a/docker_init.bash b/docker_init.bash index dc8cd740..4d000352 100644 --- a/docker_init.bash +++ b/docker_init.bash @@ -19,7 +19,7 @@ ok_exit() { # Ignore list: variables to ignore when loading environment variables from user to user export ENV_IGNORELIST="HOME PWD USER SHLVL TERM OLDPWD SHELL _ SUDO_COMMAND HOSTNAME LOGNAME MAIL SUDO_GID SUDO_UID SUDO_USER CHECK_NV_CUDNN_VERSION VIRTUAL_ENV VIRTUAL_ENV_PROMPT ENV_IGNORELIST ENV_OBFUSCATE_PART" # Obfuscate part: part of the key to obfuscate when loading environment variables from user to user, ex: HF_TOKEN, ... -export ENV_OBFUSCATE_PART="TOKEN API KEY" +export ENV_OBFUSCATE_PART="TOKEN API KEY PASSWORD SECRET CREDENTIAL COOKIE SESSION" # Check for ENV_IGNORELIST and ENV_OBFUSCATE_PART if [ -z "${ENV_IGNORELIST+x}" ]; then error_exit "ENV_IGNORELIST not set"; fi diff --git a/static/boot.js b/static/boot.js index 497e6cd6..527270e9 100644 --- a/static/boot.js +++ b/static/boot.js @@ -1842,7 +1842,7 @@ function applyBotName(){ const _testUpdates=new URLSearchParams(location.search).get('test_updates')==='1'; if(_testUpdates||(_bootSettings.check_for_updates!==false&&!sessionStorage.getItem('hermes-update-checked')&&!sessionStorage.getItem('hermes-update-dismissed'))){ const _checkUrl='api/updates/check'+(_testUpdates?'?simulate=1':''); - api(_checkUrl).then(d=>{if(!_testUpdates)sessionStorage.setItem('hermes-update-checked','1');if((d.webui&&d.webui.behind>0)||(d.agent&&d.agent.behind>0))_showUpdateBanner(d);}).catch(()=>{}); + api(_checkUrl,{method:_testUpdates?'GET':'POST',body:_testUpdates?undefined:JSON.stringify({force:false})}).then(d=>{if(!_testUpdates)sessionStorage.setItem('hermes-update-checked','1');if((d.webui&&d.webui.behind>0)||(d.agent&&d.agent.behind>0))_showUpdateBanner(d);}).catch(()=>{}); } // Fetch active profile try{const p=await api('/api/profile/active');S.activeProfile=p.name||'default';S.activeProfileIsDefault=!!p.is_default;}catch(e){S.activeProfile='default';S.activeProfileIsDefault=true;} diff --git a/static/panels.js b/static/panels.js index 8d5c31cc..87ab6080 100644 --- a/static/panels.js +++ b/static/panels.js @@ -7571,7 +7571,7 @@ async function checkUpdatesNow(){ if(label) label.textContent=t('settings_checking'); if(status) status.textContent=''; try { - const data=await api('/api/updates/check?force=1',{timeoutMs:60000}); + const data=await api('/api/updates/check',{method:'POST',body:JSON.stringify({force:true}),timeoutMs:60000}); if(data.disabled){ if(status){status.textContent=t('settings_updates_disabled');status.style.color='var(--muted)';} } else { diff --git a/tests/test_api_timeout.py b/tests/test_api_timeout.py index 61788883..d5fb671d 100644 --- a/tests/test_api_timeout.py +++ b/tests/test_api_timeout.py @@ -211,7 +211,7 @@ def test_update_flows_keep_explicit_longer_timeouts(): """Legitimately long update flows should not inherit the generic 30s guard.""" src = _source(UI_JS) panels = _source(PANELS_JS) - assert "api('/api/updates/check?force=1',{timeoutMs:60000})" in panels + assert "api('/api/updates/check',{method:'POST',body:JSON.stringify({force:true}),timeoutMs:60000})" in panels assert "api('/api/updates/summary',{method:'POST',body:JSON.stringify({updates:scopedUpdates,target:target||null}),timeoutMs:60000})" in src assert "api('/api/updates/apply',{method:'POST',body:JSON.stringify({target}),timeoutMs:120000})" in src assert "api('/api/updates/force',{method:'POST',body:JSON.stringify({target}),timeoutMs:120000})" in src diff --git a/tests/test_security_review_fixes.py b/tests/test_security_review_fixes.py new file mode 100644 index 00000000..37ce33fe --- /dev/null +++ b/tests/test_security_review_fixes.py @@ -0,0 +1,178 @@ +import io +from types import SimpleNamespace +from urllib.parse import urlsplit +from pathlib import Path + + +class _Headers(dict): + def get(self, key, default=None): + for k, v in self.items(): + if k.lower() == key.lower(): + return v + return default + + +class _Handler: + def __init__(self, *, client_ip="8.8.8.8", headers=None, body=b"{}"): + self.client_address = (client_ip, 12345) + self.headers = _Headers(headers or {}) + self.rfile = io.BytesIO(body) + self.wfile = io.BytesIO() + self.status = None + self.sent_headers = [] + + def send_response(self, code): + self.status = code + + def send_header(self, key, value): + self.sent_headers.append((key, value)) + + def end_headers(self): + pass + + +def test_onboarding_local_gate_ignores_forwarded_ip_unless_trusted(monkeypatch): + from api import routes + + monkeypatch.delenv("HERMES_WEBUI_TRUST_FORWARDED_FOR", raising=False) + handler = _Handler( + client_ip="8.8.8.8", + headers={"X-Forwarded-For": "127.0.0.1", "X-Real-IP": "10.0.0.2"}, + ) + + assert routes._onboarding_request_is_local(handler) is False + + +def test_onboarding_local_gate_uses_forwarded_ip_when_explicitly_trusted(monkeypatch): + from api import routes + + monkeypatch.setenv("HERMES_WEBUI_TRUST_FORWARDED_FOR", "1") + handler = _Handler( + client_ip="8.8.8.8", + headers={"X-Forwarded-For": "10.0.0.2", "X-Real-IP": "203.0.113.11"}, + ) + + assert routes._onboarding_request_is_local(handler) is True + + +def test_onboarding_trusted_forwarded_for_uses_proxy_appended_rightmost_ip(monkeypatch): + from api import routes + + monkeypatch.setenv("HERMES_WEBUI_TRUST_FORWARDED_FOR", "1") + handler = _Handler( + client_ip="10.0.0.10", + headers={"X-Forwarded-For": "127.0.0.1, 8.8.8.8"}, + ) + + assert routes._onboarding_request_is_local(handler) is False + + +def test_docker_env_log_obfuscates_password_and_secret_names(): + src = Path("docker_init.bash").read_text(encoding="utf-8") + line = next(l for l in src.splitlines() if l.startswith("export ENV_OBFUSCATE_PART=")) + + assert "PASSWORD" in line + assert "SECRET" in line + assert "TOKEN" in line + assert "API" in line + assert "KEY" in line + + +def test_get_update_check_returns_cache_without_fetch(monkeypatch): + from api import routes, updates + + monkeypatch.setattr(routes, "load_settings", lambda: {"check_for_updates": True}) + monkeypatch.setattr(updates, "cached_update_status", lambda include_agent=True: {"checked_at": 123, "webui": None, "agent": None, "include_agent": include_agent}) + monkeypatch.setattr(updates, "check_for_updates", lambda *a, **k: (_ for _ in ()).throw(AssertionError("GET must not fetch"))) + + handler = _Handler(client_ip="127.0.0.1") + routes.handle_get(handler, urlsplit("/api/updates/check?force=1")) + assert handler.status == 200 + + +def test_cached_update_status_does_not_drop_agent_info_when_reenabled(monkeypatch): + from api import updates + + cached_agent = {"name": "agent", "behind": 2} + monkeypatch.setattr( + updates, + "_update_cache", + { + "webui": {"name": "webui", "behind": 0}, + "agent": cached_agent, + "checked_at": 123, + "include_agent": False, + }, + ) + + result = updates.cached_update_status(include_agent=True) + + assert result["agent"] == cached_agent + + +def test_post_update_check_performs_forced_fetch(monkeypatch): + from api import routes + + calls = [] + monkeypatch.setattr(routes, "load_settings", lambda: {"check_for_updates": True}) + monkeypatch.setattr(routes, "_check_csrf", lambda handler: True) + + def fake_check(*, force=False, include_agent=True): + calls.append((force, include_agent)) + return {"checked_at": 456, "webui": None, "agent": None} + + monkeypatch.setattr("api.updates.check_for_updates", fake_check) + body = b'{"force": true}' + handler = _Handler(client_ip="127.0.0.1", body=body, headers={"Content-Length": str(len(body))}) + routes.handle_post(handler, SimpleNamespace(path="/api/updates/check", query="")) + assert handler.status == 200 + assert calls == [(True, True)] + + +def test_onboarding_untrusted_forwarded_header_denies_lan_proxy_socket(monkeypatch): + """Reverse-proxy regression (release-gate CORE fix): when forwarded headers + are present but HERMES_WEBUI_TRUST_FORWARDED_FOR is NOT set, the spoofable + header is ignored and locality is judged by the raw socket — but a PRIVATE/LAN + raw socket (a separate proxy box that could be forwarding an arbitrary public + client) is NOT treated as local. A loopback raw socket is still genuine + same-host and remains allowed (a remote attacker cannot forge a 127.0.0.1 TCP + source). Operators with a LAN proxy must set HERMES_WEBUI_TRUST_FORWARDED_FOR=1. + """ + from api import routes + + monkeypatch.delenv("HERMES_WEBUI_TRUST_FORWARDED_FOR", raising=False) + + # LAN proxy box (private raw socket) forwarding a public client → DENY + handler = _Handler(client_ip="10.0.0.5", headers={"X-Real-IP": "203.0.113.7"}) + assert routes._onboarding_request_is_local(handler) is False + handler2 = _Handler(client_ip="172.20.0.1", headers={"X-Forwarded-For": "8.8.8.8"}) + assert routes._onboarding_request_is_local(handler2) is False + + # Genuine same-host: loopback raw socket is local even if a forwarded header + # is present (the TCP source genuinely came from localhost; unspoofable). + handler3 = _Handler(client_ip="127.0.0.1", headers={"X-Forwarded-For": "8.8.8.8"}) + assert routes._onboarding_request_is_local(handler3) is True + + +def test_onboarding_spoofed_forwarded_header_from_public_socket_denied(monkeypatch): + """The original spoof hole: a public client setting X-Forwarded-For=127.0.0.1 + must NOT bypass the gate. The forwarded header is ignored; the public raw + socket governs → denied. + """ + from api import routes + + monkeypatch.delenv("HERMES_WEBUI_TRUST_FORWARDED_FOR", raising=False) + handler = _Handler(client_ip="8.8.8.8", headers={"X-Forwarded-For": "127.0.0.1"}) + assert routes._onboarding_request_is_local(handler) is False + + +def test_onboarding_direct_loopback_without_forwarded_headers_is_local(monkeypatch): + """A genuine direct local client (no proxy headers) is still allowed.""" + from api import routes + + monkeypatch.delenv("HERMES_WEBUI_TRUST_FORWARDED_FOR", raising=False) + handler = _Handler(client_ip="127.0.0.1", headers={}) + assert routes._onboarding_request_is_local(handler) is True + + handler_public = _Handler(client_ip="8.8.8.8", headers={}) + assert routes._onboarding_request_is_local(handler_public) is False