hermes-agent/tests/test_zeroed_state_db.py
Jasmine Naderi 22e5dac4b6 fix(state): quarantine lock fails closed when unacquireable (#68805)
Reviewer egilewski identified that the 5s lock timeout fell open:
quarantine_zeroed_state_db() logged 'proceeding without the cross-
process lock' and continued to re-check + rename state.db. A slow or
paused startup that still owns the lock can overlap this fallback and
the two processes can again act on the same live file.

Fix: fail closed — return None without moving the file when the lock
cannot be acquired within 5s. Log an error with recovery guidance
(restore from state-snapshots).

Test: test_quarantine_fails_closed_when_lock_held holds the cross-
process lock from a background thread, calls quarantine, and asserts
it returns None without moving the zeroed file.
2026-07-24 23:06:05 -07:00

170 lines
5.8 KiB
Python

"""#68474 hardening: zeroed state.db detection + quarantine."""
from __future__ import annotations
from pathlib import Path
import pytest
def test_is_zeroed_state_db_and_quarantine(tmp_path):
import hermes_state as hs
db = tmp_path / "state.db"
db.write_bytes(bytes(1024))
assert hs.is_zeroed_state_db(db) is True
q = hs.quarantine_zeroed_state_db(db)
assert q is not None
assert q.exists()
assert not db.exists()
assert q.read_bytes() == bytes(1024)
def test_sessiondb_opens_fresh_after_zeroed_quarantine(tmp_path, monkeypatch):
import hermes_state as hs
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
db = tmp_path / "state.db"
db.write_bytes(bytes(4096))
sdb = hs.SessionDB(db_path=db)
try:
# Fresh DB should open and accept schema
assert db.exists()
assert not hs.is_zeroed_state_db(db)
# Quarantine retained
backups = list(tmp_path.glob("state.db.zeroed-*.bak"))
assert len(backups) == 1
assert backups[0].stat().st_size == 4096
finally:
sdb.close()
def test_concurrent_quarantine_no_clobber(tmp_path):
"""#68805: two concurrent startups must not race on quarantine.
Without the cross-process lock, the second process could move its
newly-created empty DB over the first process's quarantine backup,
erasing the original damaged-file evidence. With the lock, the
second process re-checks under the lock, finds the file no longer
zeroed (or gone), and returns without clobbering.
"""
import hermes_state as hs
import threading
import sqlite3
db = tmp_path / "state.db"
db.write_bytes(bytes(4096)) # zeroed (all-NUL) 4 KB file
results: list = [None, None]
errors: list = [None, None]
def worker(idx):
try:
# Each worker opens its own SessionDB on the same path.
# The first one quarantines the zeroed file and creates a
# fresh DB. The second one should find a valid DB (or no
# file) under the lock and NOT clobber the quarantine.
sdb = hs.SessionDB(db_path=db)
try:
results[idx] = "ok"
finally:
sdb.close()
except Exception as exc:
errors[idx] = exc
t1 = threading.Thread(target=worker, args=(0,))
t2 = threading.Thread(target=worker, args=(1,))
t1.start()
t2.start()
t1.join(timeout=10)
t2.join(timeout=10)
# Both workers should complete without error
assert errors[0] is None, f"Worker 0 raised: {errors[0]}"
assert errors[1] is None, f"Worker 1 raised: {errors[1]}"
# The quarantine backup must survive — exactly one .bak file with
# the original 4096 zeroed bytes.
backups = list(tmp_path.glob("state.db.zeroed-*.bak"))
assert len(backups) >= 1, "At least one quarantine backup must exist"
for bak in backups:
assert bak.stat().st_size == 4096, (
f"Quarantine backup {bak} was clobbered: "
f"expected 4096 bytes, got {bak.stat().st_size}"
)
# The live state.db must be a valid (non-zeroed) SQLite database
assert db.exists()
assert not hs.is_zeroed_state_db(db)
conn = sqlite3.connect(str(db))
conn.execute("SELECT 1")
conn.close()
def test_quarantine_fails_closed_when_lock_held(tmp_path):
"""#68805 review: when the cross-process lock cannot be acquired within
the timeout, quarantine must FAIL CLOSED — return None without moving
the file. A fail-open fallback would let a slow/paused startup that
still owns the lock race with the fallback's re-check + rename.
"""
import hermes_state as hs
import platform
import threading
db = tmp_path / "state.db"
db.write_bytes(bytes(4096)) # zeroed (all-NUL) 4 KB file
lock_path = db.with_name(db.name + ".quarantine.lock")
lock_path.parent.mkdir(parents=True, exist_ok=True)
# Hold the cross-process lock from a background thread so the main
# thread's quarantine attempt cannot acquire it.
lock_held = threading.Event()
release_lock = threading.Event()
def hold_lock():
handle = lock_path.open("a+b")
try:
if platform.system() == "Windows":
import msvcrt
handle.seek(0)
msvcrt.locking(handle.fileno(), msvcrt.LK_NBLCK, 1)
else:
import fcntl
fcntl.flock(handle.fileno(), fcntl.LOCK_EX | fcntl.LOCK_NB)
lock_held.set()
release_lock.wait(timeout=15)
if platform.system() == "Windows":
import msvcrt
handle.seek(0)
msvcrt.locking(handle.fileno(), msvcrt.LK_UNLCK, 1)
else:
import fcntl
fcntl.flock(handle.fileno(), fcntl.LOCK_UN)
except OSError:
lock_held.clear()
finally:
handle.close()
holder = threading.Thread(target=hold_lock)
holder.start()
assert lock_held.wait(timeout=5), "Background thread failed to acquire lock"
# Reduce the quarantine lock timeout to keep the test fast. We patch
# the deadline by calling quarantine directly — it uses a 5s timeout,
# but we only need to verify it returns None without moving the file.
result = hs.quarantine_zeroed_state_db(db)
# Must fail closed: return None without moving the zeroed file
assert result is None, (
f"quarantine_zeroed_state_db returned {result} — expected None "
f"(fail-closed when lock is held)"
)
assert db.exists(), "Zeroed state.db was moved despite lock being held"
assert hs.is_zeroed_state_db(db), "File should still be zeroed (not moved)"
# Release the lock so the background thread can exit cleanly
release_lock.set()
holder.join(timeout=5)