From a308fdd5e83b41e1307b65fbc78b5d809f3a19d6 Mon Sep 17 00:00:00 2001 From: nesquena-hermes Date: Thu, 18 Jun 2026 18:13:46 +0000 Subject: [PATCH] =?UTF-8?q?release:=20v0.51.494=20=E2=80=94=20TLS-aware=20?= =?UTF-8?q?launcher=20health=20probes=20(#4412)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Absorbs #4412 (tomas-forgac) rebased onto fresh master, plus two Codex-regression-gate fixes: (1) use the scheme that actually answered the probe for the ready/already-up URL + browser-open, (2) honor an explicit HERMES_WEBUI_HEALTH_URL override in the WSL autostart probe. Co-authored-by: tomas-forgac Closes #4412 --- CHANGELOG.md | 8 + Dockerfile | 4 +- bootstrap.py | 120 +++++++++-- ctl.sh | 44 ++-- scripts/lib/health_probe.sh | 153 ++++++++++++++ scripts/wsl/hermes_webui_autostart.sh | 34 +++- start.sh | 30 ++- tests/test_ctl_script.py | 14 +- tests/test_issue513_wsl_autostart.py | 18 +- tests/test_tls_aware_probe.py | 277 ++++++++++++++++++++++++++ 10 files changed, 658 insertions(+), 44 deletions(-) create mode 100644 scripts/lib/health_probe.sh create mode 100644 tests/test_tls_aware_probe.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dfcc1b3..57665312 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,14 @@ ## [Unreleased] +## [v0.51.494] — 2026-06-18 — Release RD (TLS-aware launcher health probes) + +### Fixed + +- **Launcher health probes now work over HTTPS when TLS is enabled.** When `HERMES_WEBUI_TLS_CERT` and `HERMES_WEBUI_TLS_KEY` are set the server serves HTTPS, but `start.sh`, `ctl.sh status`, `bootstrap.py`, the WSL autostart helper, and the Docker `HEALTHCHECK` previously probed `http://` and reported a healthy server as down. All five now share a single TLS-aware probe (`scripts/lib/health_probe.sh`) that mirrors the server scheme. Self-signed certificates (common for home setups) are accepted with a one-line warning; set `HERMES_WEBUI_TLS_INSECURE_PROBE=1` to skip verification up front silently. If the cert/key are present but unloadable, the server falls back to plain HTTP and the probes now follow it instead of polling HTTPS forever — and the "ready"/"already running" URL printed to the user (and the browser that `bootstrap.py` opens) now reflects the scheme the server is actually reachable on, not the configured one. An explicitly-set `HERMES_WEBUI_HEALTH_URL` (documented WSL-autostart override) remains the authoritative probe target. Thanks @tomas-forgac. +- **`ctl.sh status` no longer crashes when `.env` contains shell-readonly variables.** `status` now loads `.env` (to learn the TLS scheme) and, like `start.sh`, filters out `UID`/`GID`/`EUID`/`EGID`/`PPID` before sourcing so a `.env` carrying `UID=...` (documented for docker-compose) doesn't abort the command under `set -euo pipefail`. +- **TLS probe follow-up hardening now cleans `ctl.sh`'s filtered `.env` temp file on early aborts and locks the insecure-opt-in HTTP fallback with regression tests.** The `.env` loader now registers a `RETURN` trap right after `mktemp`, so a failing sourced line cannot leave stale `hermes-webui-ctl-env.*` files behind under `set -euo pipefail`. The TLS-aware probe tests now also cover the `HERMES_WEBUI_TLS_INSECURE_PROBE=1` + plain-HTTP server-fallback path for both `bootstrap.py` and the shared shell helper. + ## [v0.51.493] — 2026-06-18 — Release RC (ignore malformed background-process wakeup events) ### Fixed diff --git a/Dockerfile b/Dockerfile index 8d60c0bd..c792e9fc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -101,8 +101,8 @@ ENV HERMES_WEBUI_PORT=8787 EXPOSE 8787 -HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ - CMD curl -f http://localhost:8787/health || exit 1 +HEALTHCHECK --interval=30s --timeout=8s --start-period=10s --retries=3 \ + CMD bash /apptoo/scripts/lib/health_probe.sh localhost 8787 /health 2 >/dev/null || exit 1 # docker_init.bash performs root-only bind-mount setup, then drops to hermeswebui # before starting the WebUI server. The production image does not ship sudo. diff --git a/bootstrap.py b/bootstrap.py index d8e688d7..9bccf09e 100755 --- a/bootstrap.py +++ b/bootstrap.py @@ -11,6 +11,7 @@ import subprocess import sys import time import urllib.error +import urllib.parse import urllib.request import venv import webbrowser @@ -81,6 +82,10 @@ def info(msg: str) -> None: print(f"[bootstrap] {msg}", flush=True) +def warn(msg: str) -> None: + print(f"[bootstrap] [warn] {msg}", file=sys.stderr, flush=True) + + def is_wsl() -> bool: if platform.system() != "Linux": return False @@ -281,19 +286,97 @@ def install_hermes_agent() -> None: ) -def wait_for_health(url: str, timeout: float = 25.0) -> bool: - deadline = time.time() + timeout +def _truthy(value: str | None) -> bool: + return (value or "").strip().lower() in {"1", "true", "yes", "on"} + + +def _tls_probe_enabled() -> bool: + """Mirror api.config.TLS_ENABLED: HTTPS when both cert and key are set.""" + return bool(os.getenv("HERMES_WEBUI_TLS_CERT", "").strip()) and bool( + os.getenv("HERMES_WEBUI_TLS_KEY", "").strip() + ) + + +def _health_ok(url: str, verify: bool = True) -> bool: + """Single /health request. Returns True iff the server answered with ok. + + ``verify=False`` disables TLS certificate verification (self-signed certs). + """ # Validate URL scheme to prevent file:// and other dangerous schemes if not url.startswith(("http://", "https://")): raise ValueError(f"Invalid health check URL: {url}") + context = None + if url.startswith("https://") and not verify: + import ssl + + context = ssl.create_default_context() + context.check_hostname = False + context.verify_mode = ssl.CERT_NONE + try: + with urllib.request.urlopen(url, timeout=2, context=context) as response: # nosec B310 + return b'"status": "ok"' in response.read() + except Exception: + return False + + +def wait_for_health(url: str, timeout: float = 25.0) -> str: + """Poll /health until the server answers ok or the timeout elapses. + + Returns the scheme that actually answered ("https" or "http") on success, + or "" on timeout. The scheme string is truthy on success, so existing + ``if not wait_for_health(...)`` / ``assert wait_for_health(...)`` callers + keep working unchanged. + + TLS-aware: when TLS is configured (HERMES_WEBUI_TLS_CERT/KEY set) the server + serves HTTPS, so probe HTTPS first. Self-signed certs are handled by a + second, unverified attempt (with a one-line warning). server.py falls back + to plain HTTP when the cert/key are unloadable + (tests/test_tls_support.py::test_tls_startup_failure_fallback_to_http), so + HTTP is probed last to honor that contract instead of polling HTTPS forever. + The returned scheme reflects that fallback, so callers print the URL the + server is actually reachable on. + + HERMES_WEBUI_TLS_INSECURE_PROBE=1 is an explicit opt-in that skips the + verified attempt and stays silent by contract. + """ + # Validate URL scheme to prevent file:// and other dangerous schemes + if not url.startswith(("http://", "https://")): + raise ValueError(f"Invalid health check URL: {url}") + deadline = time.time() + timeout + https = _tls_probe_enabled() + insecure_optin = _truthy(os.getenv("HERMES_WEBUI_TLS_INSECURE_PROBE")) + # Derive host:port/path from the passed URL, then build scheme-correct URLs. + parsed = urllib.parse.urlsplit(url) + authority = parsed.netloc + path = parsed.path or "/health" + https_url = f"https://{authority}{path}" + http_url = f"http://{authority}{path}" + warned = False while time.time() < deadline: - try: - with urllib.request.urlopen(url, timeout=2) as response: # nosec B310 - if b'"status": "ok"' in response.read(): - return True - except Exception: - time.sleep(0.4) - return False + if not https: + if _health_ok(http_url, verify=True): + return "http" + else: + if insecure_optin: + if _health_ok(https_url, verify=False): + return "https" + else: + if _health_ok(https_url, verify=True): + return "https" + if _health_ok(https_url, verify=False): + if not warned: + warned = True + warn( + f"Health probe: TLS certificate at {https_url} is " + "self-signed or not trusted; proceeding without " + "verification." + ) + return "https" + # server.py may have fallen back to plain HTTP (cert/key unloadable). + if _health_ok(http_url, verify=True): + return "http" + time.sleep(0.4) + return "" def open_browser(url: str) -> None: @@ -424,6 +507,8 @@ def main() -> int: server_cwd = str(agent_dir or REPO_ROOT) server_path = str(REPO_ROOT / "server.py") + # Scheme the server will advertise (HTTPS when TLS cert+key are configured). + scheme = "https" if _tls_probe_enabled() else "http" # --foreground (or auto-detected supervisor): replace this process with the # server. The supervisor sees the long-lived server as the original child, @@ -432,7 +517,7 @@ def main() -> int: foreground_reason = "--foreground" if args.foreground else _detect_supervisor() if foreground_reason: info( - f"Starting Hermes Web UI on http://{args.host}:{args.port} " + f"Starting Hermes Web UI on {scheme}://{args.host}:{args.port} " f"(foreground mode: {foreground_reason})" ) try: @@ -472,7 +557,7 @@ def main() -> int: # /health, then return. Suitable for an interactive `bash start.sh` run. log_path = state_dir / f"bootstrap-{args.port}.log" - info(f"Starting Hermes Web UI on http://{args.host}:{args.port}") + info(f"Starting Hermes Web UI on {scheme}://{args.host}:{args.port}") with log_path.open("ab") as log_file: proc = subprocess.Popen( [python_exe, server_path], @@ -483,17 +568,22 @@ def main() -> int: start_new_session=True, ) - health_url = f"http://{args.host}:{args.port}/health" - if not wait_for_health(health_url): + health_url = f"{scheme}://{args.host}:{args.port}/health" + healthy_scheme = wait_for_health(health_url) + if not healthy_scheme: raise RuntimeError( f"Web UI did not become healthy at {health_url}. " f"Check the log at {log_path}. Server PID: {proc.pid}" ) + # server.py falls back to plain HTTP when the cert/key are unloadable, so the + # scheme that actually answered the probe is the one the server is reachable + # on — use it for the ready URL and browser-open, not the configured scheme. + ready_scheme = healthy_scheme or scheme app_url = ( - f"http://localhost:{args.port}" + f"{ready_scheme}://localhost:{args.port}" if args.host in ("127.0.0.1", "localhost") - else f"http://{args.host}:{args.port}" + else f"{ready_scheme}://{args.host}:{args.port}" ) info(f"Web UI is ready: {app_url}") info(f"Log file: {log_path}") diff --git a/ctl.sh b/ctl.sh index 71df395a..de62c3de 100755 --- a/ctl.sh +++ b/ctl.sh @@ -2,6 +2,8 @@ set -euo pipefail REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +# shellcheck source=scripts/lib/health_probe.sh +. "${REPO_ROOT}/scripts/lib/health_probe.sh" HERMES_HOME="${HERMES_HOME:-${HOME}/.hermes}" PID_FILE="${HERMES_WEBUI_PID_FILE:-${HERMES_HOME}/webui.pid}" LOG_FILE="${HERMES_WEBUI_LOG_FILE:-${HERMES_HOME}/webui.log}" @@ -41,6 +43,11 @@ _load_repo_dotenv_preserving_env() { key="${key#export }" key="${key//[[:space:]]/}" [[ "${key}" =~ ^[A-Za-z_][A-Za-z0-9_]*$ ]] || continue + # Skip shell-readonly names (UID/GID/EUID/EGID/PPID); re-exporting them + # below would abort under `set -euo pipefail` with "readonly variable". + case "${key}" in + UID | GID | EUID | EGID | PPID) continue ;; + esac if [[ -n "${!key+x}" ]]; then value="${!key}" preserved+=("${key}=${value}") @@ -48,8 +55,19 @@ _load_repo_dotenv_preserving_env() { done < "${env_file}" set -a + # Filter out shell-readonly vars (UID, GID, EUID, EGID, PPID) before + # sourcing. docker-compose.yml's macOS instructions document + # `echo "UID=$(id -u)" >> .env`; under `set -euo pipefail` a raw + # `source .env` then aborts with "UID: readonly variable" before + # `ctl.sh status` can print anything. Mirrors the guard start.sh uses. + local _ctl_env_filtered + _ctl_env_filtered="$(mktemp "${TMPDIR:-/tmp}/hermes-webui-ctl-env.XXXXXX")" + # shellcheck disable=SC2064 + trap "rm -f '${_ctl_env_filtered}'" RETURN + grep -vE '^[[:space:]]*(export[[:space:]]+)?(UID|GID|EUID|EGID|PPID)=' "${env_file}" > "${_ctl_env_filtered}" || true # shellcheck source=/dev/null - source "${env_file}" + source "${_ctl_env_filtered}" + rm -f "${_ctl_env_filtered}" set +a local assignment @@ -400,12 +418,16 @@ stop_cmd() { } _health_line() { - local host="$1" port="$2" url result - url="http://${host}:${port}/health" - if command -v curl >/dev/null 2>&1; then - if result="$(curl -fsS --max-time 2 "${url}" 2>/dev/null)"; then - if command -v python3 >/dev/null 2>&1; then - printf '%s' "${result}" | python3 -c 'import json,sys + local host="$1" port="$2" url scheme result + scheme="$(hermes_webui_probe_scheme)" + url="${scheme}://${host}:${port}/health" + if ! command -v curl >/dev/null 2>&1 && ! command -v wget >/dev/null 2>&1; then + echo "unknown (curl/wget not found; ${url})" + return 0 + fi + if result="$(hermes_webui_probe_health "${host}" "${port}" "/health" 2)"; then + if command -v python3 >/dev/null 2>&1; then + printf '%s' "${result}" | python3 -c 'import json,sys try: data=json.load(sys.stdin) sessions=data.get("sessions", data.get("session_count", "?")) @@ -414,19 +436,17 @@ try: print(f"ok ({sessions} sessions, {active} active streams)" if status == "ok" else status) except Exception: print("ok")' - else - echo "ok" - fi else - echo "unreachable (${url})" + echo "ok" fi else - echo "unknown (curl not found; ${url})" + echo "unreachable (${url})" fi } status_cmd() { ensure_home + _load_repo_dotenv_preserving_env _load_state_if_present local host="${HOST:-${HERMES_WEBUI_HOST:-127.0.0.1}}" local port="${PORT:-${HERMES_WEBUI_PORT:-8787}}" diff --git a/scripts/lib/health_probe.sh b/scripts/lib/health_probe.sh new file mode 100644 index 00000000..89141231 --- /dev/null +++ b/scripts/lib/health_probe.sh @@ -0,0 +1,153 @@ +#!/usr/bin/env bash +# Shared, TLS-aware /health probe used by every shell launcher (start.sh, +# ctl.sh, the WSL autostart helper) and the Docker HEALTHCHECK. +# +# The WebUI serves HTTPS when both HERMES_WEBUI_TLS_CERT and +# HERMES_WEBUI_TLS_KEY are set (see api/config.py:TLS_ENABLED). The probe must +# mirror that scheme, otherwise an http:// probe against an https listener (or +# vice-versa) reports a healthy server as down. +# +# Probe order when TLS is configured: +# 1. Verified HTTPS. +# 2. Self-signed fallback: if verification fails, retry without verification +# and print a one-line "self-signed certificate" warning (once). +# 3. Plain HTTP: server.py intentionally falls back to serving HTTP when the +# cert/key are present but unloadable (tests/test_tls_support.py:: +# test_tls_startup_failure_fallback_to_http). Probe HTTP last so that +# contract is honored instead of polling HTTPS forever. +# +# HERMES_WEBUI_TLS_INSECURE_PROBE=1 is an explicit opt-in that skips verified +# HTTPS and goes straight to the unverified attempt. By contract this is +# silent (the user already accepted the risk), so no warning is printed. +# +# This file is safe to `source` (defines functions only) and is also runnable +# directly as a standalone probe: +# bash scripts/lib/health_probe.sh [path] [max_time] +# On success it prints the response body to stdout and exits 0. +# +# Kept bash 3.2 compatible under `set -u` (ctl.sh sources this). + +# Guard so the self-signed warning is printed at most once per process even +# when the probe is retried in a wait loop. +_HERMES_WEBUI_SELF_SIGNED_WARNED="${_HERMES_WEBUI_SELF_SIGNED_WARNED:-0}" + +# Records the scheme that actually answered the most recent successful probe +# ("https" or "http"). Defaults empty; set by hermes_webui_probe_health. +_HERMES_WEBUI_PROBE_SCHEME="${_HERMES_WEBUI_PROBE_SCHEME:-}" + +_hermes_webui_truthy() { + case "${1:-}" in + 1 | true | TRUE | True | yes | YES | on | ON) return 0 ;; + *) return 1 ;; + esac +} + +# Echo "https" when TLS is configured (both cert and key present), else "http". +hermes_webui_probe_scheme() { + if [[ -n "${HERMES_WEBUI_TLS_CERT:-}" && -n "${HERMES_WEBUI_TLS_KEY:-}" ]]; then + printf 'https' + else + printf 'http' + fi +} + +_hermes_webui_warn_self_signed() { + [[ "${_HERMES_WEBUI_SELF_SIGNED_WARNED}" == "1" ]] && return 0 + _HERMES_WEBUI_SELF_SIGNED_WARNED=1 + printf '[warn] Health probe: TLS certificate at %s is self-signed or not trusted; proceeding without verification.\n' \ + "$1" >&2 +} + +# _hermes_webui_http_get +# mode: "insecure" disables certificate verification, anything else verifies. +# Prints the response body to stdout; returns the underlying client exit code. +_hermes_webui_http_get() { + local url="$1" max_time="$2" mode="$3" + if command -v curl >/dev/null 2>&1; then + if [[ "${mode}" == "insecure" ]]; then + curl -fsS -k --max-time "${max_time}" "${url}" 2>/dev/null + else + curl -fsS --max-time "${max_time}" "${url}" 2>/dev/null + fi + return $? + elif command -v wget >/dev/null 2>&1; then + if [[ "${mode}" == "insecure" ]]; then + wget -qO- --no-check-certificate "--timeout=${max_time}" --tries=1 "${url}" 2>/dev/null + else + wget -qO- "--timeout=${max_time}" --tries=1 "${url}" 2>/dev/null + fi + return $? + fi + return 127 +} + +# hermes_webui_probe_health [path] [max_time] +# Prints the response body to stdout on success; warnings go to stderr. +# Returns 0 if the server answered, 1 otherwise. +# +# Side effect: sets the global _HERMES_WEBUI_PROBE_SCHEME to the scheme that +# actually answered ("https" or "http"). Callers that print a ready/already-up +# URL should prefer this over the configured scheme, because server.py falls +# back to plain HTTP when the cert/key are unloadable — so the configured +# scheme can be https:// while the live server speaks http://. +hermes_webui_probe_health() { + local host="$1" port="$2" path="${3:-/health}" max_time="${4:-2}" + local scheme body + scheme="$(hermes_webui_probe_scheme)" + + local http_url="http://${host}:${port}${path}" + + if [[ "${scheme}" == "http" ]]; then + if body="$(_hermes_webui_http_get "${http_url}" "${max_time}" "")"; then + _HERMES_WEBUI_PROBE_SCHEME="http" + printf '%s' "${body}" + return 0 + fi + return 1 + fi + + # TLS configured: prefer HTTPS, then fall back to HTTP. + local https_url="https://${host}:${port}${path}" + + if _hermes_webui_truthy "${HERMES_WEBUI_TLS_INSECURE_PROBE:-}"; then + # Explicit opt-in: skip verification, stay silent by contract. + if body="$(_hermes_webui_http_get "${https_url}" "${max_time}" "insecure")"; then + _HERMES_WEBUI_PROBE_SCHEME="https" + printf '%s' "${body}" + return 0 + fi + else + # 1) Verified HTTPS. + if body="$(_hermes_webui_http_get "${https_url}" "${max_time}" "")"; then + _HERMES_WEBUI_PROBE_SCHEME="https" + printf '%s' "${body}" + return 0 + fi + # 2) Self-signed fallback: verification failed, retry unverified + warn. + if body="$(_hermes_webui_http_get "${https_url}" "${max_time}" "insecure")"; then + _hermes_webui_warn_self_signed "${https_url}" + _HERMES_WEBUI_PROBE_SCHEME="https" + printf '%s' "${body}" + return 0 + fi + fi + + # 3) server.py may have fallen back to plain HTTP (cert/key unloadable). + if body="$(_hermes_webui_http_get "${http_url}" "${max_time}" "")"; then + _HERMES_WEBUI_PROBE_SCHEME="http" + printf '%s' "${body}" + return 0 + fi + + return 1 +} + +# When executed directly (not sourced), run a single probe. +if [[ "${BASH_SOURCE[0]:-}" == "${0}" ]]; then + if [[ $# -lt 2 ]]; then + echo "usage: health_probe.sh [path] [max_time]" >&2 + exit 2 + fi + hermes_webui_probe_health "$@" + exit $? +fi diff --git a/scripts/wsl/hermes_webui_autostart.sh b/scripts/wsl/hermes_webui_autostart.sh index 90726c01..c823b37f 100755 --- a/scripts/wsl/hermes_webui_autostart.sh +++ b/scripts/wsl/hermes_webui_autostart.sh @@ -16,7 +16,22 @@ HERMES_WEBUI_LOG_DIR="${HERMES_WEBUI_LOG_DIR:-${HOME}/.hermes/webui/logs}" HERMES_WEBUI_HOST="${HERMES_WEBUI_HOST:-127.0.0.1}" HERMES_WEBUI_PORT="${HERMES_WEBUI_PORT:-8787}" HERMES_WEBUI_HEALTH_HOST="${HERMES_WEBUI_HEALTH_HOST:-127.0.0.1}" -HERMES_WEBUI_HEALTH_URL="${HERMES_WEBUI_HEALTH_URL:-http://${HERMES_WEBUI_HEALTH_HOST}:${HERMES_WEBUI_PORT}/health}" + +# Shared TLS-aware probe (mirrors the server scheme; handles self-signed certs +# and the HTTP-fallback contract). +# shellcheck source=../lib/health_probe.sh +. "${SCRIPT_DIR}/../lib/health_probe.sh" + +# Scheme-aware default health URL (https when TLS_CERT/KEY are set). When the +# user explicitly sets HERMES_WEBUI_HEALTH_URL, it remains the authoritative +# probe target (documented override) — see webui_healthy() below. Otherwise the +# generated default is used for both the probe and human-readable log messages. +if [[ -n "${HERMES_WEBUI_HEALTH_URL:-}" ]]; then + _HERMES_WEBUI_HEALTH_URL_EXPLICIT=1 +else + _HERMES_WEBUI_HEALTH_URL_EXPLICIT=0 +fi +HERMES_WEBUI_HEALTH_URL="${HERMES_WEBUI_HEALTH_URL:-$(hermes_webui_probe_scheme)://${HERMES_WEBUI_HEALTH_HOST}:${HERMES_WEBUI_PORT}/health}" HERMES_WEBUI_PID_FILE="${HERMES_WEBUI_PID_FILE:-${HERMES_WEBUI_LOG_DIR}/hermes-webui.pid}" HERMES_WEBUI_LOCK_FILE="${HERMES_WEBUI_LOCK_FILE:-/tmp/hermes-webui-autostart.lock}" AUTOSTART_LOG="${HERMES_WEBUI_LOG_DIR}/webui_autostart.log" @@ -33,8 +48,21 @@ log() { } webui_healthy() { - command -v curl >/dev/null 2>&1 \ - && curl -fsS --max-time 3 "${HERMES_WEBUI_HEALTH_URL}" >/dev/null 2>&1 + # Honor an explicit HERMES_WEBUI_HEALTH_URL override (documented escape hatch): + # probe that exact URL. The shared TLS-aware helper is used for the generated + # default, which mirrors the server scheme and handles self-signed certs + the + # HTTP-fallback contract. + if [[ "${_HERMES_WEBUI_HEALTH_URL_EXPLICIT}" == "1" ]]; then + if command -v curl >/dev/null 2>&1; then + curl -fsS -k --max-time 3 "${HERMES_WEBUI_HEALTH_URL}" >/dev/null 2>&1 + elif command -v wget >/dev/null 2>&1; then + wget -qO- --no-check-certificate --timeout=3 --tries=1 "${HERMES_WEBUI_HEALTH_URL}" >/dev/null 2>&1 + else + return 1 + fi + return $? + fi + hermes_webui_probe_health "${HERMES_WEBUI_HEALTH_HOST}" "${HERMES_WEBUI_PORT}" "/health" 3 >/dev/null 2>&1 } pid_is_alive() { diff --git a/start.sh b/start.sh index f3d1d14d..b5498dbf 100755 --- a/start.sh +++ b/start.sh @@ -120,21 +120,33 @@ case "${_hermes_host}" in 0.0.0.0|""|::|"[::]") _hermes_probe_host="127.0.0.1" ;; *) _hermes_probe_host="${_hermes_host}" ;; esac -_hermes_health_url="http://${_hermes_probe_host}:${_hermes_port}/health" -# Best-effort probe. If neither curl nor wget is present we skip the check and -# fall through to the normal launch (unchanged behavior). Short 2s timeout so a -# normal cold start is not delayed. +# Best-effort, TLS-aware probe via the shared helper. If neither curl nor wget +# is present the helper returns non-zero and we fall through to the normal +# launch (unchanged behavior). Short 2s timeout so a normal cold start is not +# delayed. The helper mirrors the server scheme (https when TLS_CERT/KEY are +# set) and handles self-signed certs and the HTTP-fallback contract. +# shellcheck source=scripts/lib/health_probe.sh +. "${REPO_ROOT}/scripts/lib/health_probe.sh" +_hermes_probe_scheme="$(hermes_webui_probe_scheme)" +# Run the probe in the CURRENT shell (redirect, not $(...)) so the helper's +# _HERMES_WEBUI_PROBE_SCHEME global survives — a command-substitution subshell +# would discard it. server.py falls back to plain HTTP when the cert/key are +# unloadable, so a TLS-configured instance can be live on http:// while the +# configured scheme is https://; prefer the scheme that actually answered. +_hermes_probe_body_file="$(mktemp 2>/dev/null || echo "/tmp/hermes-webui-probe.$$")" _hermes_already_up="" -if command -v curl >/dev/null 2>&1; then - _hermes_already_up="$(curl -fsS --max-time 2 "${_hermes_health_url}" 2>/dev/null || true)" -elif command -v wget >/dev/null 2>&1; then - _hermes_already_up="$(wget -qO- --timeout=2 --tries=1 "${_hermes_health_url}" 2>/dev/null || true)" +if hermes_webui_probe_health "${_hermes_probe_host}" "${_hermes_port}" "/health" 2 > "${_hermes_probe_body_file}" 2>/dev/null; then + _hermes_already_up="$(cat "${_hermes_probe_body_file}" 2>/dev/null || true)" +fi +rm -f "${_hermes_probe_body_file}" 2>/dev/null || true +if [[ -n "${_HERMES_WEBUI_PROBE_SCHEME:-}" ]]; then + _hermes_probe_scheme="${_HERMES_WEBUI_PROBE_SCHEME}" fi if [[ -n "${_hermes_already_up}" ]]; then cat >&2 < None: + """Copy ctl.sh plus its sourced dependencies into an isolated repo dir.""" + shutil.copy2(CTL, repo_root / "ctl.sh") + lib_dir = repo_root / "scripts" / "lib" + lib_dir.mkdir(parents=True, exist_ok=True) + shutil.copy2(HEALTH_PROBE, lib_dir / "health_probe.sh") + def run_ctl( @@ -242,7 +252,7 @@ def test_start_uses_nohup_so_daemon_survives_launcher_exit(): def test_start_can_ignore_repo_dotenv_for_authoritative_test_env(tmp_path): repo_root = tmp_path / "repo" repo_root.mkdir() - shutil.copy2(CTL, repo_root / "ctl.sh") + _seed_ctl_repo(repo_root) (repo_root / "bootstrap.py").write_text("# fake bootstrap target\n", encoding="utf-8") (repo_root / ".env").write_text( f"HERMES_WEBUI_STATE_DIR={tmp_path / 'host-specific-webui'}\n", @@ -279,7 +289,7 @@ def test_start_can_ignore_repo_dotenv_for_authoritative_test_env(tmp_path): def test_start_loads_dotenv_but_inline_overrides_win(tmp_path): repo_root = tmp_path / "repo" repo_root.mkdir() - shutil.copy2(CTL, repo_root / "ctl.sh") + _seed_ctl_repo(repo_root) (repo_root / "bootstrap.py").write_text("# fake bootstrap target\n", encoding="utf-8") fake_python = tmp_path / "fake-python" diff --git a/tests/test_issue513_wsl_autostart.py b/tests/test_issue513_wsl_autostart.py index 192f8d2a..dd97a724 100644 --- a/tests/test_issue513_wsl_autostart.py +++ b/tests/test_issue513_wsl_autostart.py @@ -41,7 +41,7 @@ def test_wsl_autostart_launcher_has_safe_duplicate_prevention_and_exports_runtim assert "flock -n" in script assert "HERMES_WEBUI_LOCK_FILE" in script assert "HERMES_WEBUI_PID_FILE" in script - assert "curl -fsS --max-time 3" in script + assert "hermes_webui_probe_health" in script assert "bash \"${HERMES_WEBUI_REPO}/start.sh\" --foreground" in script assert "nohup" in script @@ -57,6 +57,22 @@ def test_wsl_autostart_launcher_passes_bash_syntax_check(): subprocess.run(["bash", "-n", str(WSL_SCRIPT)], check=True, cwd=REPO_ROOT) +def test_wsl_autostart_honors_explicit_health_url_override(): + """#4412 regression: an explicitly-set HERMES_WEBUI_HEALTH_URL must remain the + actual probe target (documented escape hatch), not be silently ignored in + favor of the TLS-aware HOST/PORT helper after the probe refactor. + """ + script = _read(WSL_SCRIPT) + # The override must be detected and used for the probe, not just for logging. + assert "_HERMES_WEBUI_HEALTH_URL_EXPLICIT" in script + # webui_healthy must branch on the explicit flag and probe the exact URL. + assert re.search( + r'_HERMES_WEBUI_HEALTH_URL_EXPLICIT.*==.*"1"', + script, + ) + assert '"${HERMES_WEBUI_HEALTH_URL}"' in script + + def test_windows_task_scheduler_helper_is_idempotent_and_validates_wsl_script_path(): script = _read(POWERSHELL_SCRIPT) diff --git a/tests/test_tls_aware_probe.py b/tests/test_tls_aware_probe.py new file mode 100644 index 00000000..9bedd51c --- /dev/null +++ b/tests/test_tls_aware_probe.py @@ -0,0 +1,277 @@ +"""Tests for the TLS-aware /health probe shared by all launchers. + +Covers the shared shell helper (``scripts/lib/health_probe.sh``), the Python +probe in ``bootstrap.py`` (``wait_for_health``), and two regressions raised in +review of the original TLS-probe PR: + +1. ``ctl.sh status`` must not crash when ``.env`` contains shell-readonly + variable assignments such as ``UID=1000`` (it sources ``.env`` to learn the + scheme). +2. Probes must fall back to plain HTTP when TLS cert/key are configured but the + server fell back to serving HTTP (server.py's tested contract), instead of + polling HTTPS forever. +""" +from __future__ import annotations + +import http.server +import importlib.util +import os +import socket +import ssl +import subprocess +import threading +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[1] +HEALTH_PROBE = REPO_ROOT / "scripts" / "lib" / "health_probe.sh" +CTL = REPO_ROOT / "ctl.sh" +BOOTSTRAP = REPO_ROOT / "bootstrap.py" + + +# ── helpers ───────────────────────────────────────────────────────────────── + +def _free_port() -> int: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as s: + s.bind(("127.0.0.1", 0)) + return s.getsockname()[1] + + +class _HealthHandler(http.server.BaseHTTPRequestHandler): + def do_GET(self): # noqa: N802 + self.send_response(200) + self.send_header("Content-Type", "application/json") + self.end_headers() + self.wfile.write(b'{"status": "ok"}') + + def log_message(self, *args): # suppress server log noise during tests + pass + + +class _Server: + """Minimal /health server, optionally TLS-wrapped, on a background thread.""" + + def __init__(self, cert: str | None = None, key: str | None = None): + self.port = _free_port() + self.httpd = http.server.HTTPServer(("127.0.0.1", self.port), _HealthHandler) + if cert and key: + ctx = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + ctx.load_cert_chain(cert, key) + self.httpd.socket = ctx.wrap_socket(self.httpd.socket, server_side=True) + self.thread = threading.Thread(target=self.httpd.serve_forever, daemon=True) + + def __enter__(self): + self.thread.start() + return self + + def __exit__(self, *exc): + self.httpd.shutdown() + self.httpd.server_close() + self.thread.join(timeout=5) + + +@pytest.fixture +def self_signed_cert(tmp_path: Path) -> tuple[str, str]: + cert = str(tmp_path / "cert.pem") + key = str(tmp_path / "key.pem") + subprocess.run( + ["openssl", "req", "-x509", "-newkey", "rsa:2048", "-keyout", key, + "-out", cert, "-days", "1", "-nodes", "-subj", "/CN=localhost"], + check=True, capture_output=True, + ) + return cert, key + + +def _run_helper(host: str, port: int, env: dict[str, str]) -> subprocess.CompletedProcess: + merged = os.environ.copy() + merged.update(env) + return subprocess.run( + ["bash", str(HEALTH_PROBE), host, str(port), "/health", "3"], + capture_output=True, text=True, env=merged, timeout=30, + ) + + +def _load_bootstrap(): + spec = importlib.util.spec_from_file_location("hermes_bootstrap_probe", BOOTSTRAP) + mod = importlib.util.module_from_spec(spec) + spec.loader.exec_module(mod) + return mod + + +# ── shared shell helper ───────────────────────────────────────────────────── + +def test_helper_scheme_http_without_tls(): + out = subprocess.run( + ["bash", "-c", f". {HEALTH_PROBE}; hermes_webui_probe_scheme"], + capture_output=True, text=True, env={**os.environ, + "HERMES_WEBUI_TLS_CERT": "", + "HERMES_WEBUI_TLS_KEY": ""}, + ) + assert out.stdout == "http" + + +def test_helper_scheme_https_with_tls(): + out = subprocess.run( + ["bash", "-c", f". {HEALTH_PROBE}; hermes_webui_probe_scheme"], + capture_output=True, text=True, + env={**os.environ, "HERMES_WEBUI_TLS_CERT": "/c", "HERMES_WEBUI_TLS_KEY": "/k"}, + ) + assert out.stdout == "https" + + +def test_helper_plain_http_probe(): + with _Server() as srv: + res = _run_helper("127.0.0.1", srv.port, + {"HERMES_WEBUI_TLS_CERT": "", "HERMES_WEBUI_TLS_KEY": ""}) + assert res.returncode == 0 + assert '"status": "ok"' in res.stdout + + +def test_helper_self_signed_warns_and_succeeds(self_signed_cert): + cert, key = self_signed_cert + with _Server(cert, key) as srv: + res = _run_helper("127.0.0.1", srv.port, + {"HERMES_WEBUI_TLS_CERT": cert, "HERMES_WEBUI_TLS_KEY": key}) + assert res.returncode == 0, res.stderr + assert '"status": "ok"' in res.stdout + assert "self-signed" in res.stderr.lower() + + +def test_helper_insecure_optin_is_silent(self_signed_cert): + cert, key = self_signed_cert + with _Server(cert, key) as srv: + res = _run_helper("127.0.0.1", srv.port, { + "HERMES_WEBUI_TLS_CERT": cert, + "HERMES_WEBUI_TLS_KEY": key, + "HERMES_WEBUI_TLS_INSECURE_PROBE": "1", + }) + assert res.returncode == 0, res.stderr + assert '"status": "ok"' in res.stdout + # Explicit opt-in is silent by contract. + assert res.stderr.strip() == "" + + +def test_helper_https_falls_back_to_http_when_server_is_plain_http(self_signed_cert): + """TLS configured in env, but the server serves plain HTTP (server.py's + fallback-on-bad-cert contract). The probe must still succeed via HTTP.""" + cert, key = self_signed_cert + with _Server() as srv: # plain HTTP server + res = _run_helper("127.0.0.1", srv.port, + {"HERMES_WEBUI_TLS_CERT": cert, "HERMES_WEBUI_TLS_KEY": key}) + assert res.returncode == 0, res.stderr + assert '"status": "ok"' in res.stdout + + +def test_helper_insecure_optin_falls_back_to_http(self_signed_cert): + """Shell helper: insecure opt-in + server serving plain HTTP must still + succeed via the HTTP fallback (and stay silent per the opt-in contract).""" + cert, key = self_signed_cert + with _Server() as srv: # plain HTTP + res = _run_helper("127.0.0.1", srv.port, { + "HERMES_WEBUI_TLS_CERT": cert, + "HERMES_WEBUI_TLS_KEY": key, + "HERMES_WEBUI_TLS_INSECURE_PROBE": "1", + }) + assert res.returncode == 0, res.stderr + assert '"status": "ok"' in res.stdout + assert res.stderr.strip() == "" + + +# ── bootstrap.py wait_for_health ──────────────────────────────────────────── + +def test_bootstrap_probe_self_signed(monkeypatch, self_signed_cert): + cert, key = self_signed_cert + monkeypatch.setenv("HERMES_WEBUI_TLS_CERT", cert) + monkeypatch.setenv("HERMES_WEBUI_TLS_KEY", key) + monkeypatch.delenv("HERMES_WEBUI_TLS_INSECURE_PROBE", raising=False) + bs = _load_bootstrap() + with _Server(cert, key) as srv: + assert bs.wait_for_health(f"https://127.0.0.1:{srv.port}/health", timeout=8) + + +def test_bootstrap_probe_http_fallback(monkeypatch, self_signed_cert): + """TLS env set but server serves HTTP -> must succeed via HTTP fallback, + not poll HTTPS forever (regression for server.py's fallback contract).""" + cert, key = self_signed_cert + monkeypatch.setenv("HERMES_WEBUI_TLS_CERT", cert) + monkeypatch.setenv("HERMES_WEBUI_TLS_KEY", key) + bs = _load_bootstrap() + with _Server() as srv: # plain HTTP + # The returned scheme must reflect the ACTUAL successful scheme (http), + # not the configured https — so the ready URL / browser-open uses the + # scheme the server is reachable on, not a dead https:// (Codex gate). + assert bs.wait_for_health(f"https://127.0.0.1:{srv.port}/health", timeout=8) == "http" + + +def test_bootstrap_probe_returns_scheme_actually_used(monkeypatch, self_signed_cert): + """wait_for_health returns the scheme that answered, truthy on success.""" + cert, key = self_signed_cert + # HTTPS server with self-signed cert -> answered over https. + monkeypatch.setenv("HERMES_WEBUI_TLS_CERT", cert) + monkeypatch.setenv("HERMES_WEBUI_TLS_KEY", key) + monkeypatch.delenv("HERMES_WEBUI_TLS_INSECURE_PROBE", raising=False) + bs = _load_bootstrap() + with _Server(cert, key) as srv: + assert bs.wait_for_health(f"https://127.0.0.1:{srv.port}/health", timeout=8) == "https" + # Plain-HTTP, no TLS env -> answered over http. + monkeypatch.delenv("HERMES_WEBUI_TLS_CERT", raising=False) + monkeypatch.delenv("HERMES_WEBUI_TLS_KEY", raising=False) + bs = _load_bootstrap() + with _Server() as srv: + assert bs.wait_for_health(f"http://127.0.0.1:{srv.port}/health", timeout=8) == "http" + # Timeout -> empty string (falsy, so `if not wait_for_health(...)` still works). + assert bs.wait_for_health("http://127.0.0.1:1/health", timeout=0.5) == "" + + +def test_bootstrap_probe_insecure_optin_http_fallback(monkeypatch, self_signed_cert): + """Insecure opt-in is set AND the server fell back to plain HTTP. + + The probe must still succeed via the HTTP fallback rather than spinning on + unverified HTTPS until timeout. Locks the reachability of the HTTP fallback + from inside the insecure-opt-in branch (guards against a future refactor + that moves the fallback under the non-opt-in ``else``). + """ + cert, key = self_signed_cert + monkeypatch.setenv("HERMES_WEBUI_TLS_CERT", cert) + monkeypatch.setenv("HERMES_WEBUI_TLS_KEY", key) + monkeypatch.setenv("HERMES_WEBUI_TLS_INSECURE_PROBE", "1") + bs = _load_bootstrap() + with _Server() as srv: # plain HTTP server, no TLS + assert bs.wait_for_health(f"https://127.0.0.1:{srv.port}/health", timeout=8) + + +def test_bootstrap_probe_plain_http(monkeypatch): + monkeypatch.delenv("HERMES_WEBUI_TLS_CERT", raising=False) + monkeypatch.delenv("HERMES_WEBUI_TLS_KEY", raising=False) + bs = _load_bootstrap() + with _Server() as srv: + assert bs.wait_for_health(f"http://127.0.0.1:{srv.port}/health", timeout=8) + + +def test_bootstrap_probe_rejects_bad_scheme(monkeypatch): + bs = _load_bootstrap() + with pytest.raises(ValueError): + bs.wait_for_health("file:///etc/passwd", timeout=1) + + +# ── ctl.sh status regression: readonly .env vars ──────────────────────────── + +def test_ctl_status_survives_readonly_env_vars(tmp_path: Path): + """`.env` with UID=/GID= must not crash `ctl.sh status` (it sources .env).""" + repo = tmp_path / "repo" + (repo / "scripts" / "lib").mkdir(parents=True) + (repo / "ctl.sh").write_bytes(CTL.read_bytes()) + (repo / "scripts" / "lib" / "health_probe.sh").write_bytes(HEALTH_PROBE.read_bytes()) + (repo / ".env").write_text("UID=1000\nGID=1000\nHERMES_WEBUI_PORT=8787\n", + encoding="utf-8") + env = os.environ.copy() + env["HERMES_HOME"] = str(tmp_path / ".hermes") + res = subprocess.run( + ["bash", str(repo / "ctl.sh"), "status"], + capture_output=True, text=True, env=env, timeout=30, + ) + assert res.returncode == 0, res.stderr + assert "hermes-webui" in res.stdout + assert "readonly variable" not in res.stderr