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.
This commit is contained in:
parent
508764d384
commit
a1c4d99953
|
|
@ -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:
|
||||
|
|
|
|||
|
|
@ -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,
|
||||
|
|
|
|||
Loading…
Reference in New Issue