diff --git a/hermes_state_repair.py b/hermes_state_repair.py index 7e8cc95879..e90f67b48c 100644 --- a/hermes_state_repair.py +++ b/hermes_state_repair.py @@ -580,8 +580,28 @@ def _connect_repair_durable(db_path: Path, *, timeout: float = 5.0) -> sqlite3.C no ``checkpoint_fullfsync`` — on Darwin an interrupted ``REINDEX``/``VACUUM``/``writable_schema`` rewrite leaves half-written b-tree pages. Autocommit (``isolation_level=None``): DDL and ``VACUUM`` are illegal inside an implicit transaction. Barriers are best-effort: on a malformed schema even ``PRAGMA synchronous=FULL`` raises, - so whole-file rewrites call :func:`_reapply_durability_barriers` once the schema parses again.""" - conn = sqlite3.connect(str(db_path), timeout=timeout, isolation_level=None) + so whole-file rewrites call :func:`_reapply_durability_barriers` once the schema parses again. + + Opened through :func:`hermes_cli.sqlite_safe_read.connect_tracked` so the fd is registered for its whole + lifetime: the repair paths hold the strongest locks in the process (``_open_exclusive`` keeps + ``locking_mode=EXCLUSIVE`` across the snapshot → strategies → promotion window, and the write-health probe + opens a ``BEGIN IMMEDIATE`` reservation). While untracked, every byte-level probe of a *live* state.db — the + zeroed-file detector, header verification, kanban's post-commit page check — was allowed to ``open()``/ + ``close()`` the file, which cancels every POSIX advisory lock this process holds on it + (https://sqlite.org/howtocorrupt.html#_posix_advisory_locks_canceled_by_a_separate_thread_doing_close_) + and lets an external writer commit into a database the repair still believes it owns (#63386). + """ + try: + from hermes_cli.sqlite_safe_read import connect_tracked + except ImportError: + # Scaffold/embed installs without hermes_cli: the durable connection stays, only the guard is off. + logger.debug("hermes_cli.sqlite_safe_read unavailable; opening %s untracked " + "(byte-probe guard inactive in this install)", db_path) + conn = sqlite3.connect(str(db_path), timeout=timeout, isolation_level=None) + else: + # Open through THIS module's sqlite3.connect so tests patching it keep control of the fd. + conn = connect_tracked(db_path, tracking_path=db_path, connect_fn=sqlite3.connect, + timeout=timeout, isolation_level=None) _reapply_durability_barriers(conn) return conn diff --git a/tests/hermes_state/test_sqlite_lock_safe_inspection.py b/tests/hermes_state/test_sqlite_lock_safe_inspection.py index 45a2d05b82..9eaea87b53 100644 --- a/tests/hermes_state/test_sqlite_lock_safe_inspection.py +++ b/tests/hermes_state/test_sqlite_lock_safe_inspection.py @@ -285,6 +285,68 @@ def test_session_db_read_only_is_tracked(tmp_path, clean_registry, monkeypatch): assert read_header_bytes_preopen(db_path, length=16) is not None +def test_repair_connections_are_tracked_for_byte_probe_safety(tmp_path, clean_registry, monkeypatch): + """End-to-end: live repair/probe connections block byte-level reads (#63386). + + The repair paths never go through ``SessionDB`` and they hold the strongest + locks in the process: ``_open_exclusive`` keeps ``locking_mode=EXCLUSIVE`` + across the whole snapshot -> strategies -> promotion window, and the + write-health probe opens a ``BEGIN IMMEDIATE`` reservation. They used to + connect outside the registry, so the byte-probe guard could not see them and + every inspection ``open()``/``close()`` cancelled those POSIX advisory locks + (``howtocorrupt`` §2.2), letting an external writer commit into the database + the repair still believed it owned. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + from hermes_state import SessionDB + from hermes_state_repair import _connect_repair_durable, _repair_conn + + db_path = tmp_path / "state.db" + seed = SessionDB(db_path=db_path) + seed.create_session("s1", source="cli") + seed.close() + assert not has_live_connection(db_path) + + conn = _connect_repair_durable(db_path) + try: + assert has_live_connection(db_path) + assert read_header_bytes_preopen(db_path, length=16) is None + finally: + conn.close() + assert not has_live_connection(db_path) + + with _repair_conn(db_path): + assert has_live_connection(db_path) + assert read_header_bytes_preopen(db_path, length=16) is None + assert not has_live_connection(db_path) + # The guard is released with the connection, not for good. + assert read_header_bytes_preopen(db_path, length=16) is not None + + +def test_byte_probe_never_cancels_the_repair_exclusion(tmp_path, clean_registry): + """A live repair's EXCLUSIVE lock must survive Hermes' own inspection (#63386). + + With the connection tracked the probe is refused, so nothing closes an fd and + the exclusion keeps holding; if the probe were allowed through, its ``close()`` + would cancel the lock and the intruder would commit into the file mid-repair. + """ + import hermes_state_repair as repair + + db_path = tmp_path / "state.db" + _make_db(db_path, "DELETE") + + guard = repair._open_exclusive(db_path, "BEGIN EXCLUSIVE") + try: + assert _external_writer_can_break_in(db_path) is False + assert read_header_bytes_preopen(db_path, length=16) is None + assert _external_writer_can_break_in(db_path) is False, ( + "the repair's EXCLUSIVE lock was cancelled by a byte-level probe " + "on the database it holds" + ) + finally: + guard.close() + +