fix(gateway): served_profiles bind to a verified gateway identity, not bare PID existence

`live_default_gateway_pid()` trusted `gateway.pid` + `_pid_exists`, so a stale
default record whose PID an unrelated process had recycled kept its old
`served_profiles` authoritative: `hermes -p X gateway start` exited 78 and
`status` said "running via multiplexer" for a gateway long gone (review of
#108352, finding D). The salvaged #110167 fallback inherited the same bare
check for the pid-file branch.

One helper now answers "which live gateway owns this home?" for every reader:
`gateway.status.live_gateway_pid_for_home` = scoped `get_running_pid` (pid file
+ runtime lock, start-time reuse guard, live gateway command line, home match)
then `get_runtime_status_running_pid(..., expected_home=home)` (honours
`gateway_state` stopped/startup_failed). `gateway_multiplex_served`,
`gateway_migrate._live_gateway_pid` and the `hermes update` inventory's
gateway_state.json fallback (#109680: a `stopped` record + recycled PID
fabricated a phantom runtime, so the update exited partial) all route through
it. Tests that impersonated a gateway with this pytest PID now wear a gateway
command line instead of stubbing `_pid_exists`.
This commit is contained in:
teknium1
2026-09-13 12:57:25 -07:00
committed by Teknium
parent 52dbe0a7cd
commit 9188e708b3
12 changed files with 131 additions and 151 deletions
+17
View File
@@ -1091,6 +1091,23 @@ def get_runtime_status_running_pid(
return pid
def live_gateway_pid_for_home(home: Path) -> Optional[int]:
"""Verified PID of the gateway owned by ``home`` (pid file + runtime lock first, then the runtime
status record), or None. Every reader of another home's gateway identity goes through this so
they all prove the same thing: the PID passes the start-time reuse guard, its live command line is
a gateway's belonging to ``home``, and the record is not ``stopped``. Bare PID existence is not
identity -- a stale record whose PID was recycled by an unrelated process lent it ``served_profiles``
and put phantom gateways into the update inventory (#109680) -- while a launch-service gateway whose
``gateway.pid`` was unlinked is still live (#110166). Never unlinks ``home``'s identity files."""
home = Path(home)
# Cached: dashboard surfaces poll this for every served profile; the cache invalidates on any
# pid/lock file change, so a stopped or replaced gateway is seen at once.
pid = get_running_pid_cached(home / "gateway.pid", cleanup_stale=False)
if pid is not None:
return pid
return get_runtime_status_running_pid(read_runtime_status(home / "gateway_state.json"), expected_home=home)
def remove_pid_file() -> None:
"""Remove the PID file only if it belongs to this process: during --replace the old process's
atexit can fire AFTER the new process wrote its own record."""
+4 -7
View File
@@ -157,14 +157,11 @@ def _profile_homes() -> list[tuple[str, Path]]:
def _live_gateway_pid(home: Path) -> Optional[int]:
"""PID of a standalone gateway owned by ``home`` (pid file, then runtime status), else None."""
from gateway.status import get_running_pid, get_runtime_status_running_pid, read_runtime_status
"""Verified PID of a standalone gateway owned by ``home``, else None (never raises: a probe
failure must not abort a migration plan)."""
from gateway.status import live_gateway_pid_for_home
with contextlib.suppress(Exception):
pid = get_running_pid(home / "gateway.pid", cleanup_stale=False)
if pid is not None:
return pid
with contextlib.suppress(Exception):
return get_runtime_status_running_pid(read_runtime_status(home / "gateway_state.json"), expected_home=home)
return live_gateway_pid_for_home(home)
return None
+8 -23
View File
@@ -17,32 +17,17 @@ logger = logging.getLogger(__name__)
def live_default_gateway_pid() -> Optional[int]:
"""PID of the default profile's gateway when it names a live process, else None.
"""PID of the default profile's gateway when a VERIFIED live process owns it, else None.
``gateway.pid`` first, then the runtime record the gateway process itself writes: a
launch-service-managed gateway can be live with no PID file at all (a replace/cleanup path unlinks
it while the process keeps serving), and ``get_running_pid()`` cannot answer for this scoped home --
an explicit ``pid_path`` deliberately suppresses its own runtime-status fallback. Same order and
same call as ``hermes_cli.gateway_migrate._live_gateway_pid``. Never key this off the record's
``updated_at``: an idle gateway never advances it, so "recent" would read a live-but-quiet
multiplexer as stopped.
``gateway.status.live_gateway_pid_for_home``: pid file + lock, then the runtime record the gateway
itself writes, each proven against the live process (start time, gateway command line, home). A
launch-service gateway can be live with no ``gateway.pid`` at all, and a stale record whose PID was
recycled by an unrelated process must not make its ``served_profiles`` authoritative. Never key this
off the record's ``updated_at``: an idle gateway never advances it.
"""
from hermes_constants import get_default_hermes_root
from gateway.status import (
_pid_exists,
_pid_from_record,
_read_pid_record,
get_runtime_status_running_pid,
read_runtime_status,
)
default_root = get_default_hermes_root()
rec = _read_pid_record(default_root / "gateway.pid")
pid = _pid_from_record(rec) if rec else None
if pid and _pid_exists(pid):
return pid
return get_runtime_status_running_pid(
read_runtime_status(default_root / "gateway_state.json"), expected_home=default_root
)
from gateway.status import live_gateway_pid_for_home
return live_gateway_pid_for_home(get_default_hermes_root())
def recorded_served_profiles(default_root: Optional[Path] = None) -> Optional[list[str]]:
+7 -7
View File
@@ -172,7 +172,7 @@ def _collect_gateway_runtimes(plan: UpdatePlan, profile_homes: list, seen: set[i
mapped gateways no status record covers."""
supervisor = _supervisor_classifier()
with _probe("Gateway-state inventory"):
from gateway.status import _pid_exists, read_runtime_status
from gateway.status import live_gateway_pid_for_home, read_runtime_status
from hermes_cli.update_receipt import _socket_identity
for profile, home in profile_homes:
@@ -185,13 +185,13 @@ def _collect_gateway_runtimes(plan: UpdatePlan, profile_homes: list, seen: set[i
declared = record.get("supervisor")
sup = str(declared) if declared else supervisor(pid)
else:
# Verified identity, not bare PID existence: a ``stopped`` record whose PID was recycled
# by an unrelated process fabricated a phantom gateway the restart phase could never
# touch, so `hermes update` exited partial (#109680).
pid = live_gateway_pid_for_home(home)
if pid is None or pid in seen:
continue
record = read_runtime_status(home / "gateway_state.json") or {}
try:
pid = int(record.get("pid"))
except (TypeError, ValueError):
continue
if not _pid_exists(pid):
continue
seen.add(pid)
sup = supervisor(pid)
plan.runtimes.append(_runtime("gateway", profile, pid, sup, record.get("code_sha"), record.get("code_version")))
+13 -9
View File
@@ -89,10 +89,14 @@ class TestNamedProfileMultiplexerGuard:
monkeypatch.setattr(
"hermes_constants.get_default_hermes_root", lambda: tmp_path
)
(tmp_path / "gateway.pid").write_text("12345", encoding="utf-8")
monkeypatch.setattr(status, "_read_pid_record", lambda p: {"pid": 12345})
monkeypatch.setattr(status, "_pid_from_record", lambda rec: 12345)
monkeypatch.setattr(status, "_pid_exists", lambda pid: True)
import json
import os
# Liveness is a verified identity (live PID + gateway command line + home), so this pytest
# process stands in for the gateway by wearing a gateway command line.
(tmp_path / "gateway.pid").write_text(str(os.getpid()), encoding="utf-8")
(tmp_path / "gateway_state.json").write_text(json.dumps(
{"pid": os.getpid(), "hermes_home": str(tmp_path), "gateway_state": "running"}))
monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "hermes gateway run")
def test_unset_allowlist_preserves_historical_guard(self, monkeypatch, tmp_path):
self._fake_running_default_gateway(monkeypatch, tmp_path)
@@ -115,11 +119,11 @@ class TestNamedProfileMultiplexerGuard:
"gateway:\n multiplex_profiles: true\n",
encoding="utf-8",
)
import gateway.status as status
monkeypatch.setattr(
status, "read_runtime_status",
lambda path=None: {"gateway_state": "running", "served_profiles": ["default", "worker"]},
)
import json
import os
(tmp_path / "gateway_state.json").write_text(json.dumps({
"pid": os.getpid(), "hermes_home": str(tmp_path), "gateway_state": "running",
"served_profiles": ["default", "worker"]}))
from hermes_cli import gateway as gw
@@ -65,6 +65,9 @@ def fleet(tmp_path, monkeypatch):
runtime["served_profiles"] = ["default", "coder", "ops"]
runtime_path.write_text(json.dumps(runtime))
import gateway.status as status
# The default gateway the fixture "starts" is this process; the served probe verifies identity.
monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "hermes gateway run")
monkeypatch.setattr(gm, "_installed_service", lambda home: state.services.get(_name(home)))
monkeypatch.setattr(gm, "_live_gateway_pid", lambda home: state.pids.get(_name(home)))
monkeypatch.setattr(gm, "_service_op", _service_op)
@@ -27,11 +27,18 @@ def served_root(tmp_path, monkeypatch):
(root / "config.yaml").write_text("model: {default: x}\n") # NO multiplex flag: env-only opt-in
(root / "gateway.pid").write_text(json.dumps({"pid": os.getpid(), "hermes_home": str(root)}))
(root / "gateway_state.json").write_text(json.dumps(
{"pid": os.getpid(), "hermes_home": str(root), "served_profiles": ["default", "coder"]}))
{"pid": os.getpid(), "hermes_home": str(root), "gateway_state": "running",
"served_profiles": ["default", "coder"]}))
monkeypatch.setenv("HERMES_HOME", str(root / "profiles" / "coder"))
monkeypatch.delenv("GATEWAY_MULTIPLEX_PROFILES", raising=False)
import hermes_constants
import gateway.status as status
monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None)
# Liveness is a VERIFIED identity: this pytest process stands in for the default gateway only
# because its command line reads as one; any other PID keeps its real command line.
real_cmdline = status._read_process_cmdline
monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: (
"python -m hermes_cli.main gateway run" if pid == os.getpid() else real_cmdline(pid)))
return root
@@ -46,62 +53,46 @@ def test_probe_trusts_live_record_over_cli_side_config(served_root):
def test_probe_falls_back_to_config_only_without_recorded_key(served_root):
from hermes_cli.gateway import named_profile_served_by_running_multiplexer
(served_root / "gateway_state.json").write_text(json.dumps({"pid": os.getpid()}))
(served_root / "gateway_state.json").write_text(json.dumps(
{"pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": "running"}))
assert named_profile_served_by_running_multiplexer("coder") is False
(served_root / "config.yaml").write_text("gateway: {multiplex_profiles: true}\n")
assert named_profile_served_by_running_multiplexer("coder") is True
def test_probe_survives_a_missing_default_pid_file(served_root, monkeypatch):
def test_probe_survives_a_missing_default_pid_file(served_root):
"""A launch-service-managed multiplexer can be live with no ``gateway.pid``: a replace/cleanup path
unlinks it while the process keeps serving. Keying liveness off that file alone made every surface
(``hermes -p X status``, ``cron list``, the dashboard ladder) say "not running" about the gateway
that was in fact serving the profile."""
import gateway.status as status
from hermes_cli.gateway import named_profile_served_by_running_multiplexer
from hermes_cli.gateway_multiplex_served import live_default_gateway_pid
(served_root / "gateway_state.json").write_text(json.dumps({
"pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": "running",
"served_profiles": ["default", "coder"]}))
(served_root / "gateway.pid").unlink()
# The PID is this test process, so the record's identity check has to see a gateway command line:
# without it the fallback correctly refuses (see the recycled-PID test below).
monkeypatch.setattr(
status, "_read_process_cmdline", lambda pid: "python -m hermes_cli.main gateway run --replace"
)
assert live_default_gateway_pid() == os.getpid()
assert named_profile_served_by_running_multiplexer("coder") is True
@pytest.mark.parametrize(
("gateway_state", "pid_alive"), [("running", False), ("stopped", True), ("startup_failed", True)]
)
def test_missing_pid_file_still_never_reports_a_dead_gateway(
served_root, monkeypatch, gateway_state, pid_alive
):
"""Fail closed: the runtime fallback must not resurrect a dead PID or a stopped/failed record."""
def test_recycled_pid_does_not_lend_a_stale_record_its_served_profiles(served_root):
"""A stale default record whose PID now belongs to an unrelated process (start time differs, command
line is not a gateway's) must not make its ``served_profiles`` authoritative: bare PID existence
once did, so `hermes -p coder gateway start` exited 78 for a multiplexer that was long gone."""
import subprocess
import gateway.status as status
from hermes_cli.gateway_multiplex_served import live_default_gateway_pid
(served_root / "gateway_state.json").write_text(json.dumps({
"pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": gateway_state,
"served_profiles": ["default", "coder"]}))
(served_root / "gateway.pid").unlink()
if not pid_alive:
monkeypatch.setattr(status, "_pid_exists", lambda pid: False)
assert live_default_gateway_pid() is None
def test_missing_pid_file_ignores_a_recycled_pid(served_root, monkeypatch):
"""A PID recycled onto a non-gateway process must not lend a stale record an identity: the live
command line decides, so the fallback cannot report a foreign process as the multiplexer."""
import gateway.status as status
from hermes_cli.gateway_multiplex_served import live_default_gateway_pid
(served_root / "gateway_state.json").write_text(json.dumps({
"pid": os.getpid(), "hermes_home": str(served_root), "gateway_state": "running",
"served_profiles": ["default", "coder"]}))
(served_root / "gateway.pid").unlink()
monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "/usr/bin/pytest tests/")
assert live_default_gateway_pid() is None
from hermes_cli.gateway import named_profile_served_by_running_multiplexer
from hermes_cli.gateway_multiplex_served import live_default_gateway_pid, recorded_served_profiles
child = subprocess.Popen(["sleep", "60"])
try:
stale_start = (status._get_process_start_time(child.pid) or 10**9) - 4242
for name in ("gateway.pid", "gateway_state.json"):
(served_root / name).write_text(json.dumps({
"pid": child.pid, "hermes_home": str(served_root), "gateway_state": "running",
"start_time": stale_start, "served_profiles": ["default", "coder"]}))
assert live_default_gateway_pid() is None
assert recorded_served_profiles(served_root) is None
assert named_profile_served_by_running_multiplexer("coder") is False
finally:
child.kill()
child.wait()
@pytest.mark.parametrize("verb", ["start", "install", "restart"])
@@ -15,10 +15,13 @@ import os
from contextlib import redirect_stdout
from types import SimpleNamespace
import pytest
def _fake_multiplexer(monkeypatch, tmp_path, *, multiplex: bool, pid_file: bool = True):
"""A live default gateway at ``tmp_path`` whose runtime record names this process; the process passes
the identity check because its command line reads as a gateway's. ``pid_file=False`` models a
launch-service gateway whose ``gateway.pid`` was unlinked while it kept serving."""
import json
def _fake_multiplexer(monkeypatch, tmp_path, *, multiplex: bool):
import hermes_constants
import gateway.status as status
@@ -26,10 +29,17 @@ def _fake_multiplexer(monkeypatch, tmp_path, *, multiplex: bool):
(tmp_path / "config.yaml").write_text(
f"gateway:\n multiplex_profiles: {'true' if multiplex else 'false'}\n"
)
(tmp_path / "gateway.pid").write_text(str(os.getpid()))
if pid_file:
(tmp_path / "gateway.pid").write_text(str(os.getpid()))
(tmp_path / "gateway_state.json").write_text(json.dumps({
"pid": os.getpid(), "kind": "hermes-gateway", "gateway_state": "running",
"start_time": status._get_process_start_time(os.getpid()), "hermes_home": str(tmp_path),
}))
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "profiles" / "beta"))
monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None)
monkeypatch.setattr(status, "_pid_exists", lambda pid: True)
monkeypatch.setattr(
status, "_read_process_cmdline", lambda pid: "python -m hermes_cli.main gateway run --replace"
)
def _run_status():
@@ -63,63 +73,12 @@ def test_unserved_named_profile_still_reports_stopped(monkeypatch, tmp_path):
assert _run_status().startswith("✗ Gateway is not running")
def _fake_launchd_multiplexer(
monkeypatch, tmp_path, *, multiplex: bool = True, gateway_state: str = "running", pid_alive: bool = True
):
"""A launch-service-managed default gateway: live process + runtime status record, no gateway.pid.
The PID file is absent (a replace/cleanup path unlinks it while the process keeps serving); the
process is the live multiplexer the ``gateway_state.json`` record points at.
"""
import json
import hermes_constants
import gateway.status as status
(tmp_path / "profiles" / "beta").mkdir(parents=True)
(tmp_path / "config.yaml").write_text(
f"gateway:\n multiplex_profiles: {'true' if multiplex else 'false'}\n"
)
(tmp_path / "gateway_state.json").write_text(json.dumps({
"pid": os.getpid(),
"kind": "hermes-gateway",
"gateway_state": gateway_state,
# Same call the production PID-reuse guard makes, so the guard compares like with like.
"start_time": status._get_process_start_time(os.getpid()),
"argv": ["hermes", "gateway", "run", "--replace"],
"hermes_home": str(tmp_path),
}))
monkeypatch.setenv("HERMES_HOME", str(tmp_path / "profiles" / "beta"))
monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None)
monkeypatch.setattr(status, "_pid_exists", lambda pid: pid_alive)
monkeypatch.setattr(
status, "_read_process_cmdline", lambda pid: "python -m hermes_cli.main gateway run --replace"
)
def test_served_named_profile_reports_running_without_default_pid_file(monkeypatch, tmp_path):
"""A live multiplexer whose PID file is missing still serves the profile it ticks."""
"""A live multiplexer whose PID file is missing still serves the profile it ticks (#110166)."""
from hermes_cli.profiles import list_profiles
_fake_launchd_multiplexer(monkeypatch, tmp_path)
_fake_multiplexer(monkeypatch, tmp_path, multiplex=True, pid_file=False)
beta = next(p for p in list_profiles() if p.name == "beta")
assert beta.gateway_running is True
assert _run_status().startswith("✓ Gateway is running via the default-profile multiplexer")
@pytest.mark.parametrize(
("gateway_state", "pid_alive"),
[("stopped", True), ("startup_failed", True), ("running", False)],
)
def test_not_live_multiplexer_without_default_pid_file_reports_stopped(
monkeypatch, tmp_path, gateway_state, pid_alive
):
"""Fails closed: a stopped/failed state or a dead PID must never be reported as running."""
from hermes_cli.profiles import list_profiles
_fake_launchd_multiplexer(monkeypatch, tmp_path, gateway_state=gateway_state, pid_alive=pid_alive)
beta = next(p for p in list_profiles() if p.name == "beta")
assert beta.gateway_running is False
assert _run_status().startswith("✗ Gateway is not running")
@@ -31,6 +31,10 @@ def pooled_served_process(tmp_path, monkeypatch):
monkeypatch.setenv("HERMES_HOME", str(root / "profiles" / "alpha"))
monkeypatch.delenv("GATEWAY_MULTIPLEX_PROFILES", raising=False)
import hermes_constants
import gateway.status as status
# Liveness is a verified identity; this pytest process passes as the default gateway only by
# wearing a gateway command line.
monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "hermes gateway run")
monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None)
from hermes_cli import profiles as profiles_mod
monkeypatch.setattr(profiles_mod, "_check_gateway_running", lambda home: False)
@@ -31,6 +31,10 @@ def served_root(tmp_path, monkeypatch):
monkeypatch.setenv("HERMES_HOME", str(root))
monkeypatch.delenv("GATEWAY_MULTIPLEX_PROFILES", raising=False)
import hermes_constants
import gateway.status as status
# Liveness is a verified identity; this pytest process passes as the default gateway only by
# wearing a gateway command line.
monkeypatch.setattr(status, "_read_process_cmdline", lambda pid: "hermes gateway run")
monkeypatch.setattr(hermes_constants, "_default_hermes_root_memo", None)
return root
+16 -2
View File
@@ -8,8 +8,9 @@ import pytest
import hermes_cli.update_inventory as ui
def _write_state(home: Path, pid: int, sha: str | None = None, version: str | None = None):
record = {"pid": pid}
def _write_state(home: Path, pid: int, sha: str | None = None, version: str | None = None,
gateway_state: str = "running"):
record = {"pid": pid, "gateway_state": gateway_state}
if sha:
record["code_sha"] = sha
if version:
@@ -31,6 +32,9 @@ def fleet(monkeypatch, tmp_path):
monkeypatch.setattr("hermes_cli.profiles._get_profiles_root", lambda: default_home / "profiles")
monkeypatch.setattr("hermes_cli.profiles._PROFILE_ID_RE", re.compile(r"^[a-z0-9][a-z0-9_-]*$"), raising=False)
monkeypatch.setattr("gateway.status._pid_exists", lambda pid: pid in (100, 200))
# A runtime is a VERIFIED gateway identity: live PID whose command line is a gateway's for that home.
monkeypatch.setattr("gateway.status._read_process_cmdline", lambda pid: {
100: "hermes gateway run", 200: "hermes --profile work gateway run"}.get(pid))
monkeypatch.setattr("hermes_cli.gateway._get_service_pids", lambda all_profiles=False: {100})
monkeypatch.setattr("hermes_cli.gateway.supports_systemd_services", lambda: True)
monkeypatch.setattr("hermes_cli.gateway.find_profile_gateway_processes", lambda exclude_pids=None: [])
@@ -81,6 +85,16 @@ class TestCollectInventory:
plan = ui.collect_runtime_inventory()
assert plan.runtimes == []
def test_stopped_record_with_recycled_pid_is_not_a_runtime(self, fleet, monkeypatch):
"""#109680: a ``stopped`` record whose PID an unrelated process now holds must not fabricate a
gateway the restart phase can never touch (that phantom made `hermes update` exit partial)."""
work_home = fleet / "home" / "profiles" / "work"
_write_state(work_home, 200, gateway_state="stopped")
monkeypatch.setattr("gateway.status._read_process_cmdline", lambda pid: {
100: "hermes gateway run", 200: "C:/Windows/system32/dllhost.exe /Processid:{X}"}.get(pid))
plan = ui.collect_runtime_inventory()
assert [r.profile for r in plan.runtimes] == ["default"]
def test_pid_file_fallback_covers_unstamped_profiles(self, fleet, monkeypatch):
"""Gateways with a PID file but no runtime-status record still appear."""
from hermes_cli.gateway import ProfileGatewayProcess
+8 -6
View File
@@ -775,15 +775,17 @@ class TestWebServerEndpoints:
seen = {}
def _pid(pid_path=None, **kw):
seen["pid_path"] = pid_path
# The served-profile probe also verifies the DEFAULT home's gateway identity; the
# contract here is that the worker's OWN pid file is what the scoped rung reads.
seen.setdefault("pid_paths", []).append(pid_path)
return None
def _runtime(path=None):
seen["status_path"] = path
seen.setdefault("status_paths", []).append(path)
return None
def _runtime_pid(runtime=None, *, expected_home=None):
seen["expected_home"] = expected_home
seen.setdefault("expected_homes", []).append(expected_home)
return None
monkeypatch.setattr(_gw_status, "get_running_pid_cached", _pid)
@@ -795,9 +797,9 @@ class TestWebServerEndpoints:
resp = self.client.get("/api/messaging/platforms?profile=worker")
assert resp.status_code == 200
assert seen["pid_path"] == worker_home / "gateway.pid"
assert seen["status_path"] == worker_home / "gateway_state.json"
assert seen["expected_home"] == worker_home
assert worker_home / "gateway.pid" in seen["pid_paths"]
assert worker_home / "gateway_state.json" in seen["status_paths"]
assert worker_home in seen["expected_homes"]