From a1c4d9995336dad1e606cc75a08b0b2b73482179 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 25 Jul 2026 21:59:36 -0700 Subject: [PATCH] fix(sessions): refuse to snapshot a live database during recovery Follow-up to the partial-recovery salvage. _copy_source_bundle() raw-copied the source state.db and its -wal/-shm sidecars with no live-connection check. Copying a database file is an open()/close() on it, and close() cancels every POSIX advisory lock the process holds on that file -- including a running VACUUM's EXCLUSIVE lock (fe431651c5). hermes_state._backup_db_file already refuses that situation; this path did not, leaving two policies for one hazard. Verified against the merged guard: with a tracked connection open, _backup_db_file returned None (refused) while _copy_source_bundle copied anyway. Recovery normally runs as its own short-lived CLI process against an offline/quarantined file, so this should never fire in practice. It is a consistency fix, not a live corruption path -- but it is exactly the drift that reintroduces the bug later. Regression test verified by sabotage: removing the guard fails it. --- hermes_cli/session_recovery.py | 21 ++++++++++++++++ tests/hermes_cli/test_session_recovery.py | 29 +++++++++++++++++++++++ 2 files changed, 50 insertions(+) diff --git a/hermes_cli/session_recovery.py b/hermes_cli/session_recovery.py index 00c21b2b4443d..fc679015a02db 100644 --- a/hermes_cli/session_recovery.py +++ b/hermes_cli/session_recovery.py @@ -237,6 +237,27 @@ def _disk_space_preflight( def _copy_source_bundle(source: Path, snapshot_dir: Path) -> tuple[Path, list[str]]: + """Copy the source DB bundle aside so SQLite never opens the original. + + Refuses when a connection to *source* is live in this process. Copying a + database file is an ``open()``/``close()`` on it, and ``close()`` cancels + every POSIX advisory lock the process holds on that file -- including a + running VACUUM's EXCLUSIVE lock (see ``hermes_cli.sqlite_safe_read``). + Recovery normally runs as its own short-lived CLI process against an + offline/quarantined file, so this should never fire; the check keeps this + path consistent with ``hermes_state._backup_db_file``, which refuses the + same situation, rather than leaving two policies for one hazard. + """ + from hermes_cli.sqlite_safe_read import has_live_connection + + if has_live_connection(source): + raise SessionRecoverySafetyError( + f"Refusing to snapshot {source}: a connection to it is still open " + "in this process, and copying the file would cancel that " + "connection's POSIX locks. Close all database handles (stop the " + "gateway/dashboard) and re-run." + ) + snapshot_source = snapshot_dir / source.name copied: list[str] = [] for suffix in _SIDECAR_SUFFIXES: diff --git a/tests/hermes_cli/test_session_recovery.py b/tests/hermes_cli/test_session_recovery.py index 5dbf3bd35bad8..a16fccf4d7611 100644 --- a/tests/hermes_cli/test_session_recovery.py +++ b/tests/hermes_cli/test_session_recovery.py @@ -288,6 +288,35 @@ def _corrupt_middle_table_leaf( return leaf_page +def test_recovery_refuses_to_snapshot_a_live_database(tmp_path: Path) -> None: + """Snapshotting must refuse while a connection to the source is live. + + Copying a database file is an open()/close() on it, and close() cancels + every POSIX advisory lock the process holds on that file (see + hermes_cli.sqlite_safe_read). Recovery normally runs against an offline + file, but the check must exist so this path cannot drift away from + hermes_state._backup_db_file, which refuses the same situation. + """ + from hermes_cli.sqlite_safe_read import connect_tracked + + source = tmp_path / "live-state.db" + output = tmp_path / "recovered.db" + _make_source(source) + + live = connect_tracked(source, isolation_level=None) + try: + with pytest.raises(SessionRecoverySafetyError, match="still open"): + recover_session_database(source, output, work_dir=tmp_path) + finally: + live.close() + + assert not output.exists() + + # With the connection closed the same call proceeds normally. + report = recover_session_database(source, output, work_dir=tmp_path) + assert report["verification"]["integrity_check"] == ["ok"] + + def test_recovery_rebuilds_canonical_data_without_opening_source( tmp_path: Path, monkeypatch: pytest.MonkeyPatch,