From faf6e6889b6fd030f96f6f383169adf7072dfa14 Mon Sep 17 00:00:00 2001 From: kshitijk4poor <82637225+kshitijk4poor@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:38:03 +0530 Subject: [PATCH] fix(gateway-windows): bind the start attestation to each PID's process create time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `_attested_pid_exited_cleanly` matched the lifecycle sentinel by numeric PID only, so a stale marker for PID 111 flipped from "clean exit" to "crash" once an unrelated PID 222 lifecycle overwrote the sentinel, and a reused PID's clean exit could vouch for a different life (#110020 review, gateway_windows.py:937). `_write_start_attestation` now records `create_times: {pid: create_time}` via the existing `process_identity._process_create_time`; `mark_exited` carries the running sentinel's `start_time` onto the exited sentinel; the attested probes fail closed for a bound PID whenever the sentinel cannot be shown to describe that incarnation (other PID, start time off by > 2s, or no start time) — "unknown" never reads as "dead". A missing sentinel still reads as dead, and markers without `create_times` keep the PID-only rule. Tests: two attestation tests (stale marker vs. moved-on sentinel → no authority; own incarnation keeps authority / clean exit / legacy marker) and a ledger test for the carried `start_time`. Mutation: with HEAD's prod files the no-authority test and the ledger test fail. --- gateway/lifecycle_ledger.py | 9 +++- hermes_cli/gateway_windows.py | 45 +++++++++++++---- tests/gateway/test_lifecycle_ledger.py | 11 +++++ .../test_gateway_start_attestation.py | 48 +++++++++++++++++++ 4 files changed, 103 insertions(+), 10 deletions(-) diff --git a/gateway/lifecycle_ledger.py b/gateway/lifecycle_ledger.py index a12f6c5aae..33d80c4a21 100644 --- a/gateway/lifecycle_ledger.py +++ b/gateway/lifecycle_ledger.py @@ -238,8 +238,13 @@ def mark_exited(exit_code: Optional[int] = None, reason: str = "graceful_shutdow sentinel = _read_json(get_lifecycle_sentinel_path(home)) if sentinel is not None and sentinel.get("pid") != os.getpid(): return - _write_sentinel({"phase": "exited", "pid": os.getpid(), "exit_code": exit_code, "exit_reason": reason, - "exited_at": _now_iso()}, home) + exited: Dict[str, Any] = {"phase": "exited", "pid": os.getpid(), "exit_code": exit_code, "exit_reason": reason, + "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"] + _write_sentinel(exited, home) except Exception: logger.debug("Failed to mark lifecycle sentinel exited", exc_info=True) diff --git a/hermes_cli/gateway_windows.py b/hermes_cli/gateway_windows.py index 3d4758d192..fcc2c51262 100644 --- a/hermes_cli/gateway_windows.py +++ b/hermes_cli/gateway_windows.py @@ -879,10 +879,16 @@ def _write_start_attestation(pids: list[int], via: str) -> None: try: path = _start_attestation_path() path.parent.mkdir(parents=True, exist_ok=True) + from hermes_cli.process_identity import _process_create_time + payload = { "pids": [int(p) for p in pids], "via": via, "ts": datetime.now(timezone.utc).isoformat(), "generation": uuid.uuid4().hex, } + # Bind each PID to its incarnation (#110020 review): the ledger sentinel is matched by PID + # only otherwise, so a stale marker would be re-read against whatever lifecycle wrote last. + create_times = {str(int(p)): _process_create_time(int(p)) for p in pids} + payload["create_times"] = {k: v for k, v in create_times.items() if v is not None} tmp = path.with_suffix(".json.tmp") tmp.write_text(json.dumps(payload), encoding="utf-8") tmp.replace(path) @@ -950,21 +956,44 @@ def _attested_pids_from(data: object) -> list[int]: return list(pids) -def _attested_pid_exited_cleanly(pid: int) -> bool: - """True when the lifecycle ledger shows a clean exit for ``pid``.""" +def _attested_create_time(data: object, pid: int) -> float | None: + """The process create time the marker bound ``pid`` to, or ``None`` (older marker / psutil silent).""" + times = data.get("create_times") if isinstance(data, dict) else None + value = times.get(str(pid)) if isinstance(times, dict) else None + return float(value) if type(value) in (int, float) else None + + +def _attested_pid_exited_cleanly(pid: int, create_time: float | None = None) -> bool: + """True when the lifecycle ledger shows a clean exit for ``pid`` — or, for a marker that bound + ``pid`` to a ``create_time``, whenever the sentinel cannot be shown to describe THAT incarnation + (#110020 review): a sentinel for another PID or another start time means an unrelated lifecycle + has run since and the marker is stale; "unknown" must never read as "dead". A missing sentinel + still reads as dead (the attested process never booted far enough to claim it).""" try: from gateway.lifecycle_ledger import get_lifecycle_sentinel_path data = json.loads(get_lifecycle_sentinel_path(_hermes_home()).read_text(encoding="utf-8")) - except Exception: + except OSError: return False - return isinstance(data, dict) and data.get("phase") == "exited" and data.get("pid") == pid + except Exception: + return create_time is not 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): + return True + if abs(float(start_time) - create_time) > 2.0: + return True + return data.get("phase") == "exited" and data.get("pid") == pid -def _attested_dead(attested: list[int], current_pids: list[int]) -> bool: +def _attested_dead(attested: list[int], current_pids: list[int], data: object = None) -> bool: """The liveness rule shared by the consuming and read-only probes: attested PIDs are dead when no gateway runs now and the lifecycle ledger shows no clean exit for any of them.""" - return not current_pids and not any(_attested_pid_exited_cleanly(pid) for pid in attested) + return not current_pids and not any( + _attested_pid_exited_cleanly(pid, _attested_create_time(data, pid)) for pid in attested + ) def attested_death_generation(current_pids: list[int]) -> str | None: @@ -980,7 +1009,7 @@ def attested_death_generation(current_pids: list[int]) -> str | None: exit): "unknown" must never read as "dead".""" data = _read_start_attestation() attested = _attested_pids_from(data) - if not attested or not _attestation_within_horizon(data) or not _attested_dead(attested, current_pids): + if not attested or not _attestation_within_horizon(data) or not _attested_dead(attested, current_pids, data): return None return _attestation_generation(data) @@ -1005,7 +1034,7 @@ def check_start_attestation(current_pids: list[int] | None = None) -> str | None return None _clear_start_attestation() - if not _attested_dead(attested, current_pids): + if not _attested_dead(attested, current_pids, data): return None return _format_attestation_warning(attested, data) diff --git a/tests/gateway/test_lifecycle_ledger.py b/tests/gateway/test_lifecycle_ledger.py index 016a8b2bcd..c824b33a78 100644 --- a/tests/gateway/test_lifecycle_ledger.py +++ b/tests/gateway/test_lifecycle_ledger.py @@ -214,3 +214,14 @@ def test_prior_exit_label_survives_corrupt_sentinel(tmp_path: Path) -> None: path.parent.mkdir(parents=True, exist_ok=True) path.write_text("garbage", encoding="utf-8") 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).""" + record_startup(home=tmp_path) + running = json.loads(get_lifecycle_sentinel_path(tmp_path).read_text(encoding="utf-8")) + 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"] diff --git a/tests/hermes_cli/test_gateway_start_attestation.py b/tests/hermes_cli/test_gateway_start_attestation.py index cf6e9a2558..15c6bd60a7 100644 --- a/tests/hermes_cli/test_gateway_start_attestation.py +++ b/tests/hermes_cli/test_gateway_start_attestation.py @@ -243,3 +243,51 @@ def test_attested_probe_treats_a_marker_past_the_horizon_as_no_authority(attest_ marker.write_text(json.dumps(stale), encoding="utf-8") assert gateway_windows.attested_death_generation(current_pids=[]) is None, ts assert marker.exists() # not consumed either + + +def _sentinel(attest_home, **fields): + state = attest_home / "state" + state.mkdir(exist_ok=True) + (state / "gateway.lifecycle.json").write_text(json.dumps(fields), encoding="utf-8") + + +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.""" + 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 + ): + _sentinel(attest_home, **fields) + assert gateway_windows.attested_death_generation(current_pids=[]) is None, fields + assert gateway_windows.check_start_attestation(current_pids=[]) is None, fields + gateway_windows._write_start_attestation([111], "direct spawn (PID 111)") # consumed above + + +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.""" + 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) + 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") + assert gateway_windows.attested_death_generation(current_pids=[]) is 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 + # Legacy marker (no create_times): PID-only rule unchanged. + 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) + assert gateway_windows.attested_death_generation(current_pids=[]) is not None