fix(gateway-windows): bind the start attestation to each PID's process create time
`_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.
This commit is contained in:
@@ -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)
|
||||
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
@@ -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"]
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user