* rebase #7160 onto master (report unverified Agent updates, no auto-restart) Co-authored-by: hrdwdmrbl <hrdwdmrbl@users.noreply.github.com> * harden agent-update marker read: no-follow, fstat regular-file, bounded (fixes gate CORE) The Agent update marker is attacker-adjacent shared state. _read_live_agent_update() used Path.read_text(), which follows symlinks, blocks forever on a FIFO, and reads an unbounded regular file — a rejected/stale-runtime request could hang or OOM. Now opens O_RDONLY|O_NONBLOCK|O_NOFOLLOW, fstat-verifies a small regular file, and reads a bounded max, classifying anything else as 'unknown' (fail-closed). Adds FIFO/oversized/symlink/ happy-path regression tests. Co-authored-by: hrdwdmrbl <hrdwdmrbl@users.noreply.github.com> * fix marker-read portability: gate os.open fast-path on O_NONBLOCK+O_NOFOLLOW os.O_NONBLOCK is Unix-only; accessing it unconditionally raised AttributeError on native Windows, breaking every stale-revision barrier (chat/compression/etc). Now both flags resolve via getattr and the os.open() fast path is taken only when BOTH are available (_MARKER_SAFE_OPEN_AVAILABLE); otherwise fall back to an lstat-only classification (absent vs unknown) so a fallback platform can never symlink-traverse or crash. Adds a Windows-fallback regression test. Co-authored-by: hrdwdmrbl <hrdwdmrbl@users.noreply.github.com> * docs(changelog): stamp #7160 stale-runtime report + hardened marker read --------- Co-authored-by: n <a@n> Co-authored-by: hrdwdmrbl <hrdwdmrbl@users.noreply.github.com>
This commit is contained in:
7 files changed
+687
-45
No files matched your search
+272
-4
@@ -8,20 +8,54 @@ mixed runtime and require a clean WebUI restart instead.
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import errno
|
||||
import math
|
||||
import os
|
||||
from pathlib import Path
|
||||
import stat
|
||||
import sys
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
|
||||
# Retain the discovered path as a diagnostic/test-visible compatibility value;
|
||||
# runtime identity is deliberately captured from the loaded module below.
|
||||
from api.config import _AGENT_DIR # noqa: F401
|
||||
from api.config import (
|
||||
PYTHON_EXE,
|
||||
_AGENT_DIR, # noqa: F401
|
||||
_DEFAULT_STATE_HOME,
|
||||
)
|
||||
from api.subprocess_utils import windows_hide_flags
|
||||
|
||||
_RESTART_MESSAGE = (
|
||||
_RESTART_REQUIRED_MESSAGE = (
|
||||
"Hermes Agent was updated while Hermes WebUI was running. "
|
||||
"Restart Hermes WebUI before retrying this action."
|
||||
"WebUI cannot verify that the Agent update completed safely. "
|
||||
"Check the Agent update outcome and environment first. "
|
||||
"Restart Hermes WebUI manually before retrying this action."
|
||||
)
|
||||
_AGENT_UPDATE_MARKER = ".hermes-update-in-progress"
|
||||
_AGENT_RECOVERY_MARKERS = (".update-incomplete", ".lazy-refresh-incomplete")
|
||||
_AGENT_UPDATE_MAX_AGE_SECONDS = 20 * 60
|
||||
# The update marker holds a PID and a start timestamp (two short numeric lines).
|
||||
# Anything larger is not a legitimate marker; cap the read so a huge or growing
|
||||
# regular file can never exhaust memory on the stale-runtime request path.
|
||||
_AGENT_UPDATE_MARKER_MAX_BYTES = 64 * 1024
|
||||
# O_NOFOLLOW is POSIX; on platforms that lack it the fast os.open() path is not
|
||||
# taken at all (see _MARKER_SAFE_OPEN_AVAILABLE below).
|
||||
# The marker read hardening relies on two POSIX-only open flags to stay both
|
||||
# non-blocking (never hang on a FIFO/device) and symlink-safe. O_NONBLOCK is
|
||||
# Unix-only and O_NOFOLLOW is absent on some platforms; accessing them
|
||||
# unconditionally raises AttributeError on native Windows. Resolve them safely
|
||||
# and only take the os.open() fast path when BOTH are genuinely available —
|
||||
# otherwise the read cannot prove non-blocking + no-follow and must fall back to
|
||||
# an lstat-only classification (see _read_live_agent_update).
|
||||
_O_NOFOLLOW = getattr(os, "O_NOFOLLOW", 0)
|
||||
_O_NONBLOCK = getattr(os, "O_NONBLOCK", 0)
|
||||
_MARKER_SAFE_OPEN_AVAILABLE = bool(getattr(os, "O_NOFOLLOW", 0)) and bool(
|
||||
getattr(os, "O_NONBLOCK", 0)
|
||||
)
|
||||
_HERMES_HOME = Path(_DEFAULT_STATE_HOME)
|
||||
_AGENT_PYTHON = Path(PYTHON_EXE).expanduser() if PYTHON_EXE else None
|
||||
|
||||
|
||||
def _read_agent_revision(
|
||||
@@ -100,6 +134,233 @@ _RUNTIME_LOCK = threading.Lock()
|
||||
class AgentRuntimeChangedError(RuntimeError):
|
||||
"""Raised when the loaded Agent runtime no longer matches its source tree."""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
message: str,
|
||||
*,
|
||||
agent_update_state: str | None = None,
|
||||
) -> None:
|
||||
super().__init__(message)
|
||||
self.agent_update_state = agent_update_state
|
||||
|
||||
|
||||
def agent_runtime_stale_payload(exc: AgentRuntimeChangedError) -> dict:
|
||||
"""Return the shared retry response for every stale-runtime entry point."""
|
||||
payload = {
|
||||
"error": str(exc),
|
||||
"type": "agent_runtime_stale",
|
||||
"retryable": True,
|
||||
"restart_scheduled": False,
|
||||
}
|
||||
if exc.agent_update_state is not None:
|
||||
payload["agent_update_state"] = exc.agent_update_state
|
||||
return payload
|
||||
|
||||
|
||||
def _pid_is_alive(pid: int) -> bool | None:
|
||||
"""Return PID liveness, or ``None`` when the platform cannot confirm it."""
|
||||
if pid <= 0:
|
||||
return False
|
||||
if pid.bit_length() > 32:
|
||||
return None
|
||||
if sys.platform == "win32":
|
||||
try:
|
||||
import ctypes
|
||||
from ctypes import wintypes
|
||||
|
||||
kernel32 = ctypes.WinDLL("kernel32", use_last_error=True)
|
||||
kernel32.OpenProcess.argtypes = (
|
||||
wintypes.DWORD,
|
||||
wintypes.BOOL,
|
||||
wintypes.DWORD,
|
||||
)
|
||||
kernel32.OpenProcess.restype = wintypes.HANDLE
|
||||
kernel32.GetExitCodeProcess.argtypes = (
|
||||
wintypes.HANDLE,
|
||||
ctypes.POINTER(wintypes.DWORD),
|
||||
)
|
||||
kernel32.GetExitCodeProcess.restype = wintypes.BOOL
|
||||
kernel32.CloseHandle.argtypes = (wintypes.HANDLE,)
|
||||
kernel32.CloseHandle.restype = wintypes.BOOL
|
||||
handle = kernel32.OpenProcess(0x1000, False, pid)
|
||||
if not handle:
|
||||
error = ctypes.get_last_error()
|
||||
if error == 5: # ERROR_ACCESS_DENIED still proves the PID exists.
|
||||
return True
|
||||
if error == 87: # ERROR_INVALID_PARAMETER for a missing PID.
|
||||
return False
|
||||
return None
|
||||
try:
|
||||
exit_code = wintypes.DWORD()
|
||||
if not kernel32.GetExitCodeProcess(handle, ctypes.byref(exit_code)):
|
||||
return None
|
||||
return exit_code.value == 259 # STILL_ACTIVE
|
||||
finally:
|
||||
kernel32.CloseHandle(handle)
|
||||
except (AttributeError, OSError, TypeError, ValueError):
|
||||
return None
|
||||
|
||||
try:
|
||||
os.kill(pid, 0)
|
||||
except ProcessLookupError:
|
||||
return False
|
||||
except PermissionError:
|
||||
return True
|
||||
except (OverflowError, ValueError):
|
||||
return None
|
||||
except OSError as exc:
|
||||
if exc.errno == errno.ESRCH:
|
||||
return False
|
||||
if exc.errno == errno.EPERM:
|
||||
return True
|
||||
return None
|
||||
return True
|
||||
|
||||
|
||||
def _read_live_agent_update(marker: Path) -> str:
|
||||
"""Classify the shared Agent update marker without changing Agent state.
|
||||
|
||||
The marker is attacker-adjacent shared state (any process that can write the
|
||||
Agent home can create it), so the read is hardened: never follow a symlink,
|
||||
never block on a FIFO/device, and never read an unbounded regular file.
|
||||
Anything that is not a small regular file is classified ``unknown`` rather
|
||||
than allowed to hang or exhaust memory on a stale-runtime request path.
|
||||
"""
|
||||
if not _MARKER_SAFE_OPEN_AVAILABLE:
|
||||
# Without both O_NONBLOCK and O_NOFOLLOW we cannot prove the read is
|
||||
# non-blocking and symlink-safe (e.g. native Windows), so never open the
|
||||
# marker: an unverifiable marker fails closed to ``unknown``, and only a
|
||||
# genuinely missing path is ``absent``.
|
||||
try:
|
||||
marker.lstat()
|
||||
except FileNotFoundError:
|
||||
return "absent"
|
||||
except (OSError, ValueError, TypeError):
|
||||
return "unknown"
|
||||
return "unknown"
|
||||
try:
|
||||
fd = os.open(marker, os.O_RDONLY | _O_NONBLOCK | _O_NOFOLLOW)
|
||||
except FileNotFoundError:
|
||||
try:
|
||||
marker.lstat()
|
||||
except FileNotFoundError:
|
||||
return "absent"
|
||||
except OSError:
|
||||
return "unknown"
|
||||
# Path exists to lstat (e.g. a dangling/looping symlink) but O_NOFOLLOW
|
||||
# refused to open it — treat as an unverifiable marker.
|
||||
return "unknown"
|
||||
except (OSError, ValueError, TypeError):
|
||||
# ELOOP (symlink under O_NOFOLLOW), ENXIO/EWOULDBLOCK (FIFO with no
|
||||
# writer under O_NONBLOCK), a non-path marker object, or any other open
|
||||
# failure — fail closed: an unreadable marker is never proof of safety.
|
||||
return "unknown"
|
||||
|
||||
try:
|
||||
try:
|
||||
st = os.fstat(fd)
|
||||
except OSError:
|
||||
return "unknown"
|
||||
if not stat.S_ISREG(st.st_mode):
|
||||
# FIFO, device, directory, socket — never a legitimate marker.
|
||||
return "unknown"
|
||||
if st.st_size > _AGENT_UPDATE_MARKER_MAX_BYTES:
|
||||
return "unknown"
|
||||
try:
|
||||
# Read one byte past the cap so an oversized file that lied about
|
||||
# st_size (or grew mid-read) is still rejected rather than truncated.
|
||||
data = os.read(fd, _AGENT_UPDATE_MARKER_MAX_BYTES + 1)
|
||||
except (OSError, BlockingIOError):
|
||||
return "unknown"
|
||||
finally:
|
||||
try:
|
||||
os.close(fd)
|
||||
except OSError:
|
||||
pass
|
||||
|
||||
if len(data) > _AGENT_UPDATE_MARKER_MAX_BYTES:
|
||||
return "unknown"
|
||||
try:
|
||||
raw = data.decode("utf-8")
|
||||
except UnicodeError:
|
||||
return "unknown"
|
||||
|
||||
lines = raw.splitlines()
|
||||
try:
|
||||
pid = int(lines[0].strip())
|
||||
started_at = float(lines[1].strip())
|
||||
except (IndexError, TypeError, ValueError):
|
||||
return "unknown"
|
||||
if pid <= 0 or not math.isfinite(started_at):
|
||||
return "unknown"
|
||||
|
||||
age_seconds = time.time() - started_at
|
||||
if age_seconds < 0:
|
||||
return "unknown"
|
||||
if age_seconds > _AGENT_UPDATE_MAX_AGE_SECONDS:
|
||||
return "stale"
|
||||
alive = _pid_is_alive(pid)
|
||||
if alive is None:
|
||||
return "unknown"
|
||||
return "active" if alive else "stale"
|
||||
|
||||
|
||||
def _marker_presence(marker: Path) -> str:
|
||||
"""Return ``present``, ``absent``, or ``unknown`` for a recovery marker."""
|
||||
try:
|
||||
marker.lstat()
|
||||
except FileNotFoundError:
|
||||
return "absent"
|
||||
except OSError:
|
||||
return "unknown"
|
||||
return "present"
|
||||
|
||||
|
||||
def _agent_install_roots() -> tuple[Path, ...]:
|
||||
"""Return portable roots that can own the Agent's venv recovery markers."""
|
||||
candidates: list[Path] = []
|
||||
if _AGENT_SOURCE_DIR is not None:
|
||||
candidates.append(_AGENT_SOURCE_DIR)
|
||||
if _AGENT_PYTHON is not None:
|
||||
# A venv Python is commonly a symlink to a shared interpreter. Keep the
|
||||
# configured venv path so its installation's recovery markers are read.
|
||||
python_path = _AGENT_PYTHON
|
||||
if python_path.parent.name.lower() in {"bin", "scripts"}:
|
||||
venv_dir = python_path.parent.parent
|
||||
if venv_dir.name.lower() in {"venv", ".venv"}:
|
||||
candidates.append(venv_dir.parent)
|
||||
|
||||
roots: list[Path] = []
|
||||
seen: set[str] = set()
|
||||
for candidate in candidates:
|
||||
key = os.path.normcase(os.path.abspath(str(candidate)))
|
||||
if key not in seen:
|
||||
seen.add(key)
|
||||
roots.append(candidate)
|
||||
return tuple(roots)
|
||||
|
||||
|
||||
def _agent_update_transaction_state() -> str:
|
||||
"""Report marker diagnostics, never proof of successful completion.
|
||||
|
||||
The Agent removes its active marker on failed/interrupted exits too. Neither
|
||||
its absence nor a stale PID proves the checkout or environment is healthy.
|
||||
"""
|
||||
live_state = _read_live_agent_update(_HERMES_HOME / _AGENT_UPDATE_MARKER)
|
||||
if live_state == "unknown":
|
||||
return "unknown"
|
||||
|
||||
recovery_present = False
|
||||
for root in _agent_install_roots():
|
||||
for marker_name in _AGENT_RECOVERY_MARKERS:
|
||||
presence = _marker_presence(root / marker_name)
|
||||
if presence == "unknown":
|
||||
return "unknown"
|
||||
recovery_present = recovery_present or presence == "present"
|
||||
if recovery_present:
|
||||
return "incomplete"
|
||||
return "unverified" if live_state == "absent" else live_state
|
||||
|
||||
|
||||
def _loaded_agent_source_identity() -> tuple[Path, Path] | None:
|
||||
"""Return the source directory and file that supplied ``run_agent``."""
|
||||
@@ -140,7 +401,14 @@ def ensure_agent_runtime_current() -> None:
|
||||
_read_agent_revision(_AGENT_SOURCE_DIR, module_path=_AGENT_MODULE_PATH)
|
||||
!= _AGENT_REVISION
|
||||
):
|
||||
raise AgentRuntimeChangedError(_RESTART_MESSAGE)
|
||||
# Automatic restart needs an Agent-owned success receipt bound to this
|
||||
# transaction, final revision and healthy environment, plus an atomic
|
||||
# handoff excluding mutations across replacement. Marker polling and a
|
||||
# final revision read supply neither contract. Keep this path manual.
|
||||
raise AgentRuntimeChangedError(
|
||||
_RESTART_REQUIRED_MESSAGE,
|
||||
agent_update_state=_agent_update_transaction_state(),
|
||||
)
|
||||
|
||||
|
||||
def require_ai_agent_class():
|
||||
|
||||
Reference in new issue
Block a user