Release v0.51.234 — Release HB (stage-q4) (#3488)
Some checks failed
Release & Docker / release (push) Has been cancelled
Some checks failed
Release & Docker / release (push) Has been cancelled
## Release v0.51.234 — Release HB (stage-q4) Two medium-risk backend/infra fixes. All gates green. ### Fixes | PR | Author | Fix | |----|--------|-----| | #3289 | @rodboev | Refuse server startup when a live instance already serves the port (Windows/macOS silent port-sharing hazard). Live-listener probe (`GET /health`, 2s timeout) + Windows `SO_EXCLUSIVEADDRUSE` — **preserves fast restart** (POSIX keeps `allow_reuse_address=True`; a dying socket in the kernel backlog times out → startup proceeds). | | #3486 | @dso2ng | Allow remote/SSH terminal profiles to use target-side workspace paths under `terminal.cwd` without a server-local `stat()`. Local profiles unchanged — bypass only fires for remote backends and only for paths contained within `terminal.cwd`. | ### History note on #3289 This PR was **held earlier this sweep** — its original form globally disabled `SO_REUSEADDR`, which a Codex gate flagged as breaking fast restart (TIME_WAIT bricks rebind for ~60s). The contributor reworked it along the suggested lines (live-listener probe instead of the global disable). This release ships the reworked version. Unheld → full pickup → full gate. ### Gate results - **Full pytest suite**: 7458 passed, 8 skipped, 3 xpassed, **0 failed** - **ruff forward gate**: CLEAN - **browser-smoke gate**: CLEAN (real server boots fine with the new startup probe) - **Codex (regression)**: SAFE TO SHIP (verified fast-rebind preserved + remote bypass gated on backend+containment, local validation unchanged) - **Opus (correctness + security)**: SAFE TO SHIP (probe false-positive, `_is_within` containment, local-profile bypass all hold up; applied its one minor double-call cleanup note) Closes #3289. Co-authored-by: rodboev <rodboev@users.noreply.github.com> Co-authored-by: dso2ng <dso2ng@users.noreply.github.com>
This commit is contained in:
@@ -3,6 +3,12 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
## [v0.51.234] — 2026-06-03 — Release HB (stage-q4 — duplicate-instance startup guard + remote-terminal workspace paths)
|
||||
|
||||
### Fixed
|
||||
- The server now refuses to start when a live instance is already responding on the configured port, instead of silently sharing it (a Windows/macOS hazard where `SO_REUSEADDR` semantics let two processes bind 8787 at once, #3289). Rather than globally disabling `SO_REUSEADDR` (which would brick legitimate fast restarts — `ctl.sh restart` and the `os.execv` self-update path rebind immediately and would hit the TIME_WAIT window), startup now runs a live-listener probe (`_abort_if_already_serving`): a TCP connect + `GET /health` with a 2s timeout. A live instance answers and startup aborts with a clear message; a dying instance whose socket still lingers in the kernel backlog accepts the connection but never responds, so the probe times out and startup proceeds — preserving fast restart. On Windows, `SO_EXCLUSIVEADDRUSE` is set in a `server_bind()` override to get true exclusive binding (POSIX keeps the inherited `allow_reuse_address = True`) (#3289, @rodboev).
|
||||
- Remote/SSH terminal profiles can now use target-side workspace paths that don't exist on the WebUI host. Workspace validation/resolution previously `stat()`-ed every path against the WebUI server's local filesystem, so a `terminal.cwd` (or session workspace) living on the remote target was rejected as nonexistent. For profiles whose terminal backend is non-local, paths **under the configured `terminal.cwd`** now pass validation without a server-local existence check, and stale server-local `last_workspace` values are ignored unless they fall under the remote cwd. Local profiles are unchanged — the bypass only fires for remote backends and only for paths contained within `terminal.cwd` (#3486, @dso2ng).
|
||||
|
||||
## [v0.51.233] — 2026-06-03 — Release HA (stage-q3 — session-truncate keep_count guard against silent transcript loss)
|
||||
|
||||
### Fixed
|
||||
|
||||
@@ -57,6 +57,46 @@ def _last_workspace_file() -> Path:
|
||||
return _profile_state_dir() / 'last_workspace.txt'
|
||||
|
||||
|
||||
def _is_remote_terminal_backend(terminal_cfg: dict | None) -> bool:
|
||||
"""Return True when the active terminal backend runs outside this WebUI host."""
|
||||
if not isinstance(terminal_cfg, dict):
|
||||
return False
|
||||
backend = str(terminal_cfg.get('backend') or '').strip().lower()
|
||||
return backend not in ('', 'local')
|
||||
|
||||
|
||||
def _remote_terminal_cwd() -> str | None:
|
||||
"""Return target-side terminal cwd for remote profiles, without local stat()."""
|
||||
try:
|
||||
from api.config import get_config
|
||||
|
||||
terminal_cfg = get_config().get('terminal', {})
|
||||
if not _is_remote_terminal_backend(terminal_cfg):
|
||||
return None
|
||||
cwd = str(terminal_cfg.get('cwd') or '').strip()
|
||||
if not cwd or cwd == '.':
|
||||
return None
|
||||
return cwd
|
||||
except Exception:
|
||||
logger.debug("Failed to read remote terminal cwd", exc_info=True)
|
||||
return None
|
||||
|
||||
|
||||
def _remote_terminal_workspace_candidate(path: str | Path) -> Path | None:
|
||||
"""Return a non-stat'ed target-side Path when it is under terminal.cwd."""
|
||||
cwd = _remote_terminal_cwd()
|
||||
if not cwd:
|
||||
return None
|
||||
raw = _strip_surrounding_quotes(str(path)).strip()
|
||||
if not raw:
|
||||
return None
|
||||
candidate = Path(raw).expanduser().resolve()
|
||||
base = Path(cwd).expanduser().resolve()
|
||||
if candidate == base or _is_within(candidate, base):
|
||||
return candidate
|
||||
return None
|
||||
|
||||
|
||||
def _profile_default_workspace() -> str:
|
||||
"""Read the profile's default workspace from its config.yaml.
|
||||
|
||||
@@ -65,25 +105,31 @@ def _profile_default_workspace() -> str:
|
||||
2. 'default_workspace' — alternate explicit key
|
||||
3. 'terminal.cwd' — hermes-agent terminal working dir (most common)
|
||||
|
||||
For remote/SSH terminal profiles, ``terminal.cwd`` lives on the target
|
||||
machine, not on the WebUI server. In that case return it without a
|
||||
server-local existence check so WebUI can send the correct workspace hint
|
||||
to the agent/tool backend.
|
||||
|
||||
Falls back to the live DEFAULT_WORKSPACE from api.config.
|
||||
"""
|
||||
try:
|
||||
from api.config import get_config
|
||||
cfg = get_config()
|
||||
terminal_cfg = cfg.get('terminal', {})
|
||||
remote_terminal = _is_remote_terminal_backend(terminal_cfg)
|
||||
# Explicit webui workspace keys first
|
||||
for key in ('workspace', 'default_workspace'):
|
||||
ws = cfg.get(key)
|
||||
if ws:
|
||||
p = Path(str(ws)).expanduser().resolve()
|
||||
if p.is_dir():
|
||||
if remote_terminal or p.is_dir():
|
||||
return str(p)
|
||||
# Fall through to terminal.cwd — the agent's configured working directory
|
||||
terminal_cfg = cfg.get('terminal', {})
|
||||
if isinstance(terminal_cfg, dict):
|
||||
cwd = terminal_cfg.get('cwd', '')
|
||||
if cwd and str(cwd) not in ('.', ''):
|
||||
p = Path(str(cwd)).expanduser().resolve()
|
||||
if p.is_dir():
|
||||
if remote_terminal or p.is_dir():
|
||||
return str(p)
|
||||
except (ImportError, Exception):
|
||||
logger.debug("Failed to load profile default workspace config")
|
||||
@@ -225,19 +271,35 @@ def save_workspaces(workspaces: list) -> None:
|
||||
|
||||
|
||||
def get_last_workspace() -> str:
|
||||
remote_cwd = _remote_terminal_cwd()
|
||||
|
||||
def valid_last_workspace(raw: str) -> str | None:
|
||||
if not raw:
|
||||
return None
|
||||
if remote_cwd:
|
||||
# For remote/SSH profiles, last_workspace is target-side state. Do
|
||||
# not accept stale server-local paths merely because they exist on
|
||||
# the WebUI host; require the value to stay under terminal.cwd.
|
||||
if _remote_terminal_workspace_candidate(raw) is not None:
|
||||
return raw
|
||||
return None
|
||||
if Path(raw).is_dir():
|
||||
return raw
|
||||
return None
|
||||
|
||||
lw_file = _last_workspace_file()
|
||||
if lw_file.exists():
|
||||
try:
|
||||
p = lw_file.read_text(encoding='utf-8').strip()
|
||||
if p and Path(p).is_dir():
|
||||
p = valid_last_workspace(lw_file.read_text(encoding='utf-8').strip())
|
||||
if p:
|
||||
return p
|
||||
except Exception:
|
||||
logger.debug("Failed to read last workspace from %s", lw_file)
|
||||
# Fallback: try global file
|
||||
if _GLOBAL_LW_FILE.exists():
|
||||
try:
|
||||
p = _GLOBAL_LW_FILE.read_text(encoding='utf-8').strip()
|
||||
if p and Path(p).is_dir():
|
||||
p = valid_last_workspace(_GLOBAL_LW_FILE.read_text(encoding='utf-8').strip())
|
||||
if p:
|
||||
return p
|
||||
except Exception:
|
||||
logger.debug("Failed to read global last workspace")
|
||||
@@ -574,8 +636,17 @@ def resolve_trusted_workspace(path: str | Path | None = None) -> Path:
|
||||
candidate = Path(path).expanduser().resolve()
|
||||
|
||||
access_error = _workspace_access_error(candidate)
|
||||
remote_candidate = _remote_terminal_workspace_candidate(path)
|
||||
if access_error:
|
||||
raise ValueError(access_error)
|
||||
# For remote terminal profiles, workspace paths belong to the target
|
||||
# machine. Allow paths under terminal.cwd so session switching can
|
||||
# update the workspace hint even though this WebUI host cannot stat
|
||||
# the target-side path.
|
||||
if remote_candidate is None:
|
||||
raise ValueError(access_error)
|
||||
|
||||
if remote_candidate is not None:
|
||||
return remote_candidate
|
||||
|
||||
# (A) Trusted if under the user's home directory — cross-platform via Path.home()
|
||||
# Must be checked before system roots to allow symlinks like /var/home.
|
||||
@@ -658,8 +729,16 @@ def validate_workspace_to_add(path: str) -> Path:
|
||||
candidate = Path(path).expanduser().resolve()
|
||||
|
||||
access_error = _workspace_access_error(candidate)
|
||||
remote_candidate = _remote_terminal_workspace_candidate(path)
|
||||
if access_error:
|
||||
raise ValueError(access_error)
|
||||
# Remote terminal profiles validate workspace existence on the target
|
||||
# machine, not on the WebUI server. Permit target-side paths under
|
||||
# terminal.cwd.
|
||||
if remote_candidate is None:
|
||||
raise ValueError(access_error)
|
||||
|
||||
if remote_candidate is not None:
|
||||
return remote_candidate
|
||||
|
||||
# Home directory is always trusted regardless of where it lives on disk
|
||||
# (e.g. /var/home/... on systemd-homed Fedora/RHEL).
|
||||
|
||||
28
server.py
28
server.py
@@ -184,6 +184,13 @@ class QuietHTTPServer(ThreadingHTTPServer):
|
||||
self.accept_loop_requests_total = 0
|
||||
self.accept_loop_last_request_at = 0.0
|
||||
|
||||
def server_bind(self):
|
||||
if sys.platform == 'win32':
|
||||
self.allow_reuse_address = False
|
||||
SO_EXCLUSIVEADDRUSE = getattr(socket, 'SO_EXCLUSIVEADDRUSE', -5)
|
||||
self.socket.setsockopt(socket.SOL_SOCKET, SO_EXCLUSIVEADDRUSE, 1)
|
||||
super().server_bind()
|
||||
|
||||
def _handle_request_noblock(self):
|
||||
"""Record accept-loop progress before dispatching a request handler.
|
||||
|
||||
@@ -477,6 +484,25 @@ def _log_shutdown_audit(reason: str = "serve_forever_exit") -> None:
|
||||
)
|
||||
|
||||
|
||||
def _abort_if_already_serving(host: str, port: int) -> None:
|
||||
"""Refuse to start if a live HTTP server is already responding on this port."""
|
||||
probe_host = '127.0.0.1' if host in ('0.0.0.0', '', '::') else host
|
||||
try:
|
||||
with socket.create_connection((probe_host, port), timeout=2) as s:
|
||||
s.sendall(b'GET /health HTTP/1.0\r\nHost: localhost\r\n\r\n')
|
||||
s.settimeout(2)
|
||||
data = s.recv(512)
|
||||
if data:
|
||||
print(
|
||||
f'[!!] FATAL: Another server is already responding on'
|
||||
f' {probe_host}:{port}. Stop the existing instance first.',
|
||||
flush=True,
|
||||
)
|
||||
sys.exit(1)
|
||||
except (ConnectionRefusedError, ConnectionResetError, OSError, socket.timeout):
|
||||
pass
|
||||
|
||||
|
||||
def main() -> None:
|
||||
from api.config import print_startup_config, verify_hermes_imports, _HERMES_FOUND
|
||||
|
||||
@@ -572,6 +598,7 @@ def main() -> None:
|
||||
except Exception as e:
|
||||
print(f'[!!] WARNING: Plugin loading failed: {e}', flush=True)
|
||||
|
||||
_abort_if_already_serving(HOST, PORT)
|
||||
httpd = QuietHTTPServer((HOST, PORT), Handler)
|
||||
|
||||
# ── TLS/HTTPS setup (optional) ─────────────────────────────────────────
|
||||
@@ -597,6 +624,7 @@ def main() -> None:
|
||||
try:
|
||||
httpd.serve_forever()
|
||||
finally:
|
||||
httpd.server_close()
|
||||
_log_shutdown_audit()
|
||||
# Stop the gateway watcher on shutdown
|
||||
try:
|
||||
|
||||
57
tests/test_remote_terminal_workspace.py
Normal file
57
tests/test_remote_terminal_workspace.py
Normal file
@@ -0,0 +1,57 @@
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from api import config as api_config
|
||||
from api import workspace
|
||||
|
||||
|
||||
REMOTE_CWD = "/Users/joeyshiue"
|
||||
|
||||
|
||||
def _remote_config(**overrides):
|
||||
cfg = {"terminal": {"backend": "ssh", "cwd": REMOTE_CWD}}
|
||||
cfg.update(overrides)
|
||||
return cfg
|
||||
|
||||
|
||||
def test_remote_terminal_cwd_is_profile_default_without_local_stat(monkeypatch, tmp_path):
|
||||
fallback = tmp_path / "fallback"
|
||||
fallback.mkdir()
|
||||
|
||||
monkeypatch.setattr(api_config, "DEFAULT_WORKSPACE", fallback)
|
||||
monkeypatch.setattr(api_config, "get_config", lambda: _remote_config())
|
||||
|
||||
assert workspace._profile_default_workspace() == REMOTE_CWD
|
||||
|
||||
|
||||
def test_remote_terminal_last_workspace_ignores_stale_local_path(monkeypatch, tmp_path):
|
||||
stale_local = tmp_path / "stale-local"
|
||||
stale_local.mkdir()
|
||||
last_workspace = tmp_path / "last_workspace.txt"
|
||||
last_workspace.write_text(str(stale_local), encoding="utf-8")
|
||||
|
||||
monkeypatch.setattr(api_config, "get_config", lambda: _remote_config())
|
||||
monkeypatch.setattr(workspace, "_last_workspace_file", lambda: last_workspace)
|
||||
monkeypatch.setattr(workspace, "_GLOBAL_LW_FILE", tmp_path / "missing-global-last-workspace.txt")
|
||||
|
||||
assert workspace.get_last_workspace() == REMOTE_CWD
|
||||
|
||||
|
||||
def test_remote_terminal_workspace_paths_under_cwd_do_not_require_local_existence(monkeypatch):
|
||||
monkeypatch.setattr(api_config, "get_config", lambda: _remote_config())
|
||||
|
||||
target_side_project = f"{REMOTE_CWD}/projects/demo"
|
||||
|
||||
assert workspace.validate_workspace_to_add(target_side_project) == Path(target_side_project).resolve()
|
||||
assert workspace.resolve_trusted_workspace(target_side_project) == Path(target_side_project).resolve()
|
||||
|
||||
|
||||
def test_remote_terminal_workspace_paths_outside_cwd_still_reject(monkeypatch):
|
||||
monkeypatch.setattr(api_config, "get_config", lambda: _remote_config())
|
||||
|
||||
with pytest.raises(ValueError, match="Path does not exist"):
|
||||
workspace.validate_workspace_to_add("/Users/other/projects/demo")
|
||||
|
||||
with pytest.raises(ValueError, match="Path does not exist"):
|
||||
workspace.resolve_trusted_workspace("/Users/other/projects/demo")
|
||||
90
tests/test_server_port_exclusivity.py
Normal file
90
tests/test_server_port_exclusivity.py
Normal file
@@ -0,0 +1,90 @@
|
||||
"""Duplicate-instance guard: a second server on the same port must be detected
|
||||
and refused before bind, not silently shared (#3289)."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import socket
|
||||
import sys
|
||||
import threading
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
|
||||
import pytest
|
||||
|
||||
from tests._pytest_port import TEST_PORT
|
||||
|
||||
|
||||
# ── SO_EXCLUSIVEADDRUSE on Windows ──────────────────────────────────────────
|
||||
|
||||
@pytest.mark.skipif(sys.platform != 'win32', reason='Windows-only socket option')
|
||||
def test_exclusive_addr_use_set_on_windows():
|
||||
from server import QuietHTTPServer
|
||||
port = TEST_PORT + 901
|
||||
httpd = QuietHTTPServer(('127.0.0.1', port), BaseHTTPRequestHandler)
|
||||
try:
|
||||
val = httpd.socket.getsockopt(
|
||||
socket.SOL_SOCKET,
|
||||
getattr(socket, 'SO_EXCLUSIVEADDRUSE', -5),
|
||||
)
|
||||
assert val != 0, 'SO_EXCLUSIVEADDRUSE should be set on Windows'
|
||||
finally:
|
||||
httpd.server_close()
|
||||
|
||||
|
||||
# ── Live-listener probe ─────────────────────────────────────────────────────
|
||||
|
||||
def test_probe_detects_live_server():
|
||||
"""_abort_if_already_serving must call sys.exit when a live server responds."""
|
||||
from server import _abort_if_already_serving
|
||||
|
||||
port = TEST_PORT + 902
|
||||
|
||||
class Handler(BaseHTTPRequestHandler):
|
||||
def do_GET(self): # noqa: N802
|
||||
self.send_response(200)
|
||||
self.end_headers()
|
||||
self.wfile.write(b'ok')
|
||||
def log_message(self, *a):
|
||||
pass
|
||||
|
||||
httpd = HTTPServer(('127.0.0.1', port), Handler)
|
||||
t = threading.Thread(target=httpd.serve_forever, daemon=True)
|
||||
t.start()
|
||||
try:
|
||||
with pytest.raises(SystemExit):
|
||||
_abort_if_already_serving('127.0.0.1', port)
|
||||
finally:
|
||||
httpd.shutdown()
|
||||
httpd.server_close()
|
||||
|
||||
|
||||
def test_probe_allows_startup_when_nothing_listening():
|
||||
"""_abort_if_already_serving must return normally on a free port."""
|
||||
from server import _abort_if_already_serving
|
||||
|
||||
port = TEST_PORT + 903
|
||||
_abort_if_already_serving('127.0.0.1', port)
|
||||
|
||||
|
||||
def test_probe_allows_startup_on_unresponsive_socket():
|
||||
"""A socket that accepts but never responds (e.g. dying instance still in
|
||||
kernel backlog) should not block startup."""
|
||||
from server import _abort_if_already_serving
|
||||
|
||||
port = TEST_PORT + 904
|
||||
srv = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
|
||||
srv.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1)
|
||||
srv.bind(('127.0.0.1', port))
|
||||
srv.listen(1)
|
||||
try:
|
||||
_abort_if_already_serving('127.0.0.1', port)
|
||||
finally:
|
||||
srv.close()
|
||||
|
||||
|
||||
def test_probe_normalizes_wildcard_host():
|
||||
"""0.0.0.0 and :: should probe 127.0.0.1, not the literal wildcard."""
|
||||
from server import _abort_if_already_serving
|
||||
|
||||
port = TEST_PORT + 905
|
||||
_abort_if_already_serving('0.0.0.0', port)
|
||||
_abort_if_already_serving('::', port)
|
||||
Reference in New Issue
Block a user