hermes-admin/hermes-agent
1
0
Fork 0
Code Issues Pull Requests Actions 29 Packages Projects Releases Wiki Activity

fix(gateway-windows): bind the attestation to process birth, not the ledger claim

Browse Source 
Review follow-ups on the identity binding:
- The sentinel's `start_time` is `time.time()` at `record_startup`, seconds
  after the process was born once imports finish, so comparing it with
  psutil's create_time within 2 s would have read every real gateway as
  undecidable and silently stopped the #109538 cold-start. `record_startup`
  now stamps `create_time` (psutil birth via the existing
  `process_identity._process_create_time`), `mark_exited` carries it, and
  the attestation compares birth to birth. A sentinel from a gateway older
  than the stamp falls back to the PID-only rule.
- A resume token written by pre-generation code and resumed by this code
  probes the marker again instead of skipping the spawn.
- Horizon allows a 60 s backwards clock step; the unused `now` parameter is
  gone; the create-time tolerance is a named constant; the read-then-unlink
  in `_consume_start_attestation` is documented as best-effort.
This commit is contained in:
kshitijk4poor
2026-09-14 20:45:36 +05:30
committed by kshitij
parent faf6e6889b
commit 8c3a35b69d
6 changed files with 54 additions and 25 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
gateway/lifecycle_ledger.py
View File

@@ -214,6 +214,13 @@ def record_startup(home: Optional[Path] = None) -> Optional[Dict[str, Any]]:
logger.debug("Unclean-exit detection failed", exc_info=True)
try:
claim: Dict[str, Any] = {"phase": "running", "pid": os.getpid(), "start_time": time.time(), "started_at": _now_iso()}
# Process birth (psutil), distinct from ``start_time`` (the ledger claim, seconds later once
# imports finish): the Windows start attestation binds PIDs to birth time (#110020 review).
from hermes_cli.process_identity import _process_create_time
create_time = _process_create_time(os.getpid())
if create_time is not None:
claim["create_time"] = create_time
# Carry the verdict on the PREVIOUS life on the new sentinel: it is the only
# machine-readable copy (/api/status reads it to report an OOM restart).
# Scoped to this life — the next clean exit or boot rewrites the sentinel.
@@ -242,8 +249,9 @@ def mark_exited(exit_code: Optional[int] = None, reason: str = "graceful_shutdow
"exited_at": _now_iso()}
# Carry the incarnation identity: the Windows start attestation matches a clean exit by
# PID *and* start time so a reused PID's exit cannot vouch for a different life (#110020).
if sentinel is not None and sentinel.get("start_time") is not None:
exited["start_time"] = sentinel["start_time"]
for key in ("start_time", "create_time"):
if sentinel is not None and sentinel.get(key) is not None:
exited[key] = sentinel[key]
_write_sentinel(exited, home)
except Exception:
logger.debug("Failed to mark lifecycle sentinel exited", exc_info=True)

18
hermes_cli/gateway_windows.py
View File

@@ -907,9 +907,13 @@ def _clear_start_attestation() -> None:
# meant to bridge the seconds between a ✓ and the next ``hermes gateway status``/``update``; a
# historical marker must never later override Desktop ownership into a duplicate gateway (#76129).
START_ATTESTATION_MAX_AGE_S = 24 * 3600
# Same slack process_identity uses for psutil create_time comparisons (PID reuse disambiguation).
_CREATE_TIME_TOLERANCE_S = 2.0
# A backwards clock step (NTP) between write and read must not kill a fresh marker.
_ATTESTATION_CLOCK_SLACK_S = 60.0
def _attestation_within_horizon(data: object, now: float | None = None) -> bool:
def _attestation_within_horizon(data: object) -> bool:
"""False for a marker whose ``ts`` is missing, unparsable or older than the horizon (fail closed)."""
try:
ts = datetime.fromisoformat(str(data["ts"])) if isinstance(data, dict) else None
@@ -917,8 +921,8 @@ def _attestation_within_horizon(data: object, now: float | None = None) -> bool:
return False
if ts.tzinfo is None:
ts = ts.replace(tzinfo=timezone.utc)
age = (time.time() if now is None else now) - ts.timestamp()
return 0 <= age <= START_ATTESTATION_MAX_AGE_S
age = time.time() - ts.timestamp()
return -_ATTESTATION_CLOCK_SLACK_S <= age <= START_ATTESTATION_MAX_AGE_S
except Exception:
return False
@@ -931,6 +935,7 @@ def _attestation_generation(data: object) -> str | None:
def _consume_start_attestation(generation: str) -> None:
"""Clear the marker only while it is still the ``generation`` that was acted on; a newer
marker belongs to a gateway start this caller knows nothing about and keeps its own report."""
# Best-effort read-then-unlink: a marker written in between loses one post-start report, never authority.
if _attestation_generation(_read_start_attestation()) == generation:
_clear_start_attestation()
@@ -980,10 +985,11 @@ def _attested_pid_exited_cleanly(pid: int, create_time: float | None = None) ->
if not isinstance(data, dict):
return create_time is not None
if create_time is not None:
start_time = data.get("start_time")
if data.get("pid") != pid or type(start_time) not in (int, float):
if data.get("pid") != pid:
return True
if abs(float(start_time) - create_time) > 2.0:
sentinel_birth = data.get("create_time")
# A sentinel from a gateway older than the identity stamp cannot be told apart: PID-only rule.
if type(sentinel_birth) in (int, float) and abs(float(sentinel_birth) - create_time) > _CREATE_TIME_TOLERANCE_S:
return True
return data.get("phase") == "exited" and data.get("pid") == pid

7
hermes_cli/update_cmd_windows.py
View File

@@ -950,7 +950,12 @@ def _cold_start_windows_gateway_after_update(token: dict | None = None) -> bool:
with _abort_on_error("Could not re-check gateway liveness before cold-start"):
if list(find_gateway_pids(all_profiles=True)):
return True
generation = (token or {}).get("attested_generation")
token = token or {}
generation = token.get("attested_generation")
if generation is None and "attested_generation" not in token:
# Token written by pre-generation code and resumed across this very update: probe the marker.
with _abort_on_error("Could not re-read the start attestation before cold-start"):
generation = gateway_windows.attested_death_generation(current_pids=[])
with _abort_on_error("Could not re-check Desktop gateway-lifecycle ownership before cold-start"):
if _desktop_owns_gateway_lifecycle() and not generation:
logger.debug("Skipping Windows gateway cold-start: Desktop owns gateway lifecycle")

11
tests/gateway/test_lifecycle_ledger.py
View File

@@ -216,12 +216,15 @@ def test_prior_exit_label_survives_corrupt_sentinel(tmp_path: Path) -> None:
assert read_prior_exit_label(tmp_path) == "unknown"
def test_mark_exited_carries_start_time_onto_exited_sentinel(tmp_path: Path) -> None:
"""The exited sentinel keeps the life's ``start_time`` so the Windows start attestation can
match a clean exit by incarnation, not by reusable PID (#110020)."""
def test_sentinel_carries_process_birth_through_exit(tmp_path: Path, monkeypatch) -> None:
"""The running sentinel stamps the process ``create_time`` (psutil birth, not the later ledger
claim) and the exited sentinel keeps it, so the Windows start attestation can match a clean
exit by incarnation, not by reusable PID (#110020)."""
monkeypatch.setattr("hermes_cli.process_identity._process_create_time", lambda pid=None: 1234.5)
record_startup(home=tmp_path)
running = json.loads(get_lifecycle_sentinel_path(tmp_path).read_text(encoding="utf-8"))
assert running["create_time"] == 1234.5
mark_exited(0, reason="graceful_shutdown", home=tmp_path)
exited = json.loads(get_lifecycle_sentinel_path(tmp_path).read_text(encoding="utf-8"))
assert exited["phase"] == "exited"
assert exited["start_time"] == running["start_time"]
assert (exited["start_time"], exited["create_time"]) == (running["start_time"], 1234.5)

26
tests/hermes_cli/test_gateway_start_attestation.py
View File

@@ -254,17 +254,17 @@ def _sentinel(attest_home, **fields):
def test_attestation_bound_to_create_time_is_no_authority_once_the_sentinel_moved_on(monkeypatch, attest_home):
"""#110020 review (gateway_windows.py:937): the sentinel used to be matched by numeric PID only, so a
stale marker for PID 111 flipped from clean to crash once an unrelated PID 222 lifecycle overwrote
the sentinel. A marker bound to 111's create time fails closed: another PID, another start time,
or a sentinel without a start time all read as undecidable, never as dead."""
the sentinel. A marker bound to 111's process birth fails closed: another PID or another birth
time reads as undecidable, never as dead. (Birth, not the ledger's ``start_time``: that is stamped
seconds later, once imports finish.)"""
monkeypatch.setattr("hermes_cli.process_identity._process_create_time", lambda pid=None: 1000.0)
gateway_windows._write_start_attestation([111], "direct spawn (PID 111)")
marker = json.loads((attest_home / "state" / "gateway.start-attestation.json").read_text(encoding="utf-8"))
assert marker["create_times"] == {"111": 1000.0}
for fields in (
{"phase": "exited", "pid": 222, "start_time": 5000.0}, # unrelated lifecycle overwrote it
{"phase": "running", "pid": 222, "start_time": 5000.0},
{"phase": "running", "pid": 111, "start_time": 1003.0}, # PID reuse: different incarnation
{"phase": "running", "pid": 111}, # sentinel carries no identity
{"phase": "exited", "pid": 222, "create_time": 5000.0}, # unrelated lifecycle overwrote it
{"phase": "running", "pid": 222, "create_time": 5000.0},
{"phase": "running", "pid": 111, "create_time": 1003.0}, # PID reuse: different incarnation
):
_sentinel(attest_home, **fields)
assert gateway_windows.attested_death_generation(current_pids=[]) is None, fields
@@ -274,14 +274,18 @@ def test_attestation_bound_to_create_time_is_no_authority_once_the_sentinel_move
def test_attestation_bound_to_create_time_keeps_authority_for_its_own_incarnation(monkeypatch, attest_home):
"""A running sentinel for the same PID within 2s of the bound create time is the attested
incarnation: gone with no clean exit → dead. Its own clean exit (start_time carried by
``mark_exited``) → planned stop. Older markers without ``create_times`` keep PID-only matching."""
incarnation: gone with no clean exit → dead. Its own clean exit (create_time carried by
``mark_exited``) → planned stop. A sentinel from a gateway older than the identity stamp, and
older markers without ``create_times``, keep PID-only matching."""
monkeypatch.setattr("hermes_cli.process_identity._process_create_time", lambda pid=None: 1000.0)
gateway_windows._write_start_attestation([111], "direct spawn (PID 111)")
_sentinel(attest_home, phase="running", pid=111, start_time=1001.5)
_sentinel(attest_home, phase="running", pid=111, create_time=1001.5)
assert gateway_windows.attested_death_generation(current_pids=[]) is not None
_sentinel(attest_home, phase="exited", pid=111, start_time=1001.5, exit_reason="graceful_shutdown")
_sentinel(attest_home, phase="exited", pid=111, create_time=1001.5, exit_reason="graceful_shutdown")
assert gateway_windows.attested_death_generation(current_pids=[]) is None
# Pre-identity sentinel (no create_time, start_time is the later ledger claim): PID-only rule.
_sentinel(attest_home, phase="running", pid=111, start_time=1007.0)
assert gateway_windows.attested_death_generation(current_pids=[]) is not None
# No sentinel at all: the process never booted far enough to claim one → dead.
(attest_home / "state" / "gateway.lifecycle.json").unlink()
assert gateway_windows.attested_death_generation(current_pids=[]) is not None
@@ -289,5 +293,5 @@ def test_attestation_bound_to_create_time_keeps_authority_for_its_own_incarnatio
path = attest_home / "state" / "gateway.start-attestation.json"
legacy = {k: v for k, v in json.loads(path.read_text(encoding="utf-8")).items() if k != "create_times"}
path.write_text(json.dumps(legacy), encoding="utf-8")
_sentinel(attest_home, phase="running", pid=111, start_time=1003.0)
_sentinel(attest_home, phase="running", pid=111, create_time=1003.0)
assert gateway_windows.attested_death_generation(current_pids=[]) is not None

5
tests/hermes_cli/test_windows_gateway_cold_start_desktop_lifecycle.py
View File

@@ -187,7 +187,10 @@ def test_attested_dead_gateway_survives_desktop_ownership_and_marker_is_consumed
monkeypatch.setattr(gateway_windows, "_wait_for_gateway_ready", lambda *a, **k: [4242])
monkeypatch.setattr(gateway_windows, "_write_start_attestation", lambda *a, **k: None)
assert update_cmd._cold_start_windows_gateway_after_update(token) is True
# A token written by pre-generation code and resumed across this very update carries no
# ``attested_generation`` key: the marker is probed again rather than the spawn skipped.
legacy_token = {k: v for k, v in token.items() if k != "attested_generation"}
assert update_cmd._cold_start_windows_gateway_after_update(legacy_token) is True
assert spawned == [1]
assert "Gateway started via cold-start after update (PID: 4242)" in capsys.readouterr().out
assert not marker.exists() # consumed by the spawn