diff --git a/hermes_state.py b/hermes_state.py index d315559a17..7a70a3e07a 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -47,7 +47,8 @@ from hermes_state_schema import SessionSchemaMixin import hermes_state_holders as _state_holders from hermes_state_dbfile import ( _canonical_sqlite_path, _connect_tracked_db, _read_sqlite_application_id, _stat_sqlite_sidecar_identity, - _watched_sqlite_sidecar_paths, is_zeroed_state_db, quarantine_cross_process_lock, quarantine_zeroed_state_db, + _watched_sqlite_sidecar_paths, has_invalid_sqlite_header_preopen, is_zeroed_state_db, quarantine_cross_process_lock, + quarantine_invalid_state_db, refuse_deleted_wal_generation, ) from hermes_state_messages import SessionMessagesMixin @@ -495,17 +496,17 @@ class SessionDB( try: # Serialize zero-byte check, quarantine, connect and schema commit so concurrent # openers don't race the absent-path -> schema-commit window. - if not self.db_path.exists() or is_zeroed_state_db(self.db_path): + if not self.db_path.exists() or has_invalid_sqlite_header_preopen(self.db_path): with quarantine_cross_process_lock(self.db_path) as lock_acquired: if not lock_acquired: logger.warning( "startup quarantine lock for %s not acquired within 5s; proceeding", self.db_path, ) - self._handle_quarantine_if_zeroed(already_locked=lock_acquired) + self._handle_quarantine_if_invalid(already_locked=lock_acquired) self._connect_and_init_with_lock_patience() else: - self._handle_quarantine_if_zeroed(already_locked=False) + self._handle_quarantine_if_invalid(already_locked=False) self._connect_and_init_with_lock_patience() except sqlite3.DatabaseError as exc: # A malformed schema fails on the very first statement (before _init_schema), so the @@ -565,18 +566,18 @@ class SessionDB( conn.row_factory = sqlite3.Row return conn - def _handle_quarantine_if_zeroed(self, already_locked: bool = False) -> None: + def _handle_quarantine_if_invalid(self, already_locked: bool = False) -> None: """Quarantine a zero-byte/headerless state.db so a fresh one can open; if quarantine failed, raise the clear message instead of opening the zeroed file.""" - if not (self.db_path.exists() and is_zeroed_state_db(self.db_path)): + if not (self.db_path.exists() and has_invalid_sqlite_header_preopen(self.db_path)): return try: zsize = self.db_path.stat().st_size except OSError: zsize = -1 - qpath = quarantine_zeroed_state_db(self.db_path, already_locked=already_locked) + qpath = quarantine_invalid_state_db(self.db_path, already_locked=already_locked) msg = ( - f"state.db looks ZEROED ({zsize} bytes, no SQLite header). " + f"state.db has no SQLite header ({zsize} bytes). " f"Preserved at {qpath or '(quarantine failed — file left in place)'}. " f"Restore from {self.db_path.parent / 'state-snapshots'} via `hermes snapshot list` / " f"`hermes snapshot restore ` if available. " @@ -584,7 +585,7 @@ class SessionDB( ) logger.error(msg) _set_last_init_error(msg) - if qpath is None and self.db_path.exists() and is_zeroed_state_db(self.db_path): + if qpath is None and self.db_path.exists() and has_invalid_sqlite_header_preopen(self.db_path): raise sqlite3.DatabaseError(msg) def _open_writer_conn(self) -> sqlite3.Connection: diff --git a/hermes_state_dbfile.py b/hermes_state_dbfile.py index 5e10dd4f17..f81c1ac125 100644 --- a/hermes_state_dbfile.py +++ b/hermes_state_dbfile.py @@ -194,6 +194,23 @@ def is_zeroed_state_db(path: Path, *, probe_bytes: int = 100, force: bool = Fals return head is not None and not head.startswith(b"SQLite format 3") and all(b == 0 for b in head) +def has_invalid_sqlite_header_preopen(path: Path, *, probe_bytes: int = 100, force: bool = False) -> bool: + """Pre-open byte probe: a pre-existing state.db whose first page is not SQLite (0-byte, NUL, or + clobbered page 0 as in #102198). Same live-connection contract as :func:`is_zeroed_state_db`; + ``force=True`` only for offline files. Never raises.""" + try: + if not path.is_file(): + return False + path.stat() + from hermes_cli.sqlite_safe_read import has_live_connection, read_header_bytes_preopen + if not force and has_live_connection(path): + return False + head = read_header_bytes_preopen(path, length=max(16, probe_bytes), force=force) + except Exception: + return False + return head is not None and not head.startswith(b"SQLite format 3") + + @contextlib.contextmanager def quarantine_cross_process_lock(path: Path, timeout: float = 5.0): """Acquire the cross-process lock for path.quarantine.lock.""" @@ -235,23 +252,24 @@ def quarantine_cross_process_lock(path: Path, timeout: float = 5.0): handle.close() -def quarantine_zeroed_state_db(path: Path, *, already_locked: bool = False) -> Optional[Path]: - """Move a zeroed state.db aside (preserve bytes) and return quarantine path. A cross-process - lock stops two concurrent startups racing: the second re-checks under the lock and finds the - file gone (or fresh) instead of clobbering the quarantine.""" +def quarantine_invalid_state_db(path: Path, *, already_locked: bool = False) -> Optional[Path]: + """Move a non-SQLite state.db (zeroed or clobbered page 0) aside with its -wal/-shm sidecars, + preserving bytes; return the quarantine path. A cross-process lock stops two concurrent + startups racing: the second re-checks under the lock and finds the file gone (or fresh).""" def _do_quarantine(): if not path.exists(): - logger.info("quarantine_zeroed_state_db: %s already moved by another process", path) + logger.info("quarantine_invalid_state_db: %s already moved by another process", path) return None - if not is_zeroed_state_db(path): - logger.info("quarantine_zeroed_state_db: %s is no longer zeroed (another " + if not has_invalid_sqlite_header_preopen(path): + logger.info("quarantine_invalid_state_db: %s is no longer invalid (another " "process quarantined it and a fresh DB was created)", path) return None + kind = "zeroed" if is_zeroed_state_db(path) else "notadb" try: ts = time.strftime("%Y%m%d-%H%M%S") except Exception: ts = "unknown" - stem = f"{path.name}.zeroed-{ts}-{os.getpid()}" + stem = f"{path.name}.{kind}-{ts}-{os.getpid()}" dest = path.with_name(f"{stem}.bak") n = 0 while dest.exists(): @@ -260,7 +278,7 @@ def quarantine_zeroed_state_db(path: Path, *, already_locked: bool = False) -> O try: path.rename(dest) except OSError as exc: - logger.error("Failed to quarantine zeroed %s: %s", path, exc) + logger.error("Failed to quarantine invalid %s: %s", path, exc) return None for suffix in ("-wal", "-shm"): side = Path(str(path) + suffix) @@ -274,7 +292,7 @@ def quarantine_zeroed_state_db(path: Path, *, already_locked: bool = False) -> O with quarantine_cross_process_lock(path) as acquired: if not acquired: logger.error("quarantine lock for %s not acquired within 5s — refusing to " - "quarantine without the cross-process lock. The zeroed file " + "quarantine without the cross-process lock. The invalid file " "is left in place. If sessions fail to load, restore from " "state-snapshots via `hermes snapshot list` / `hermes snapshot restore `.", path) diff --git a/tests/test_zeroed_state_db.py b/tests/test_zeroed_state_db.py index 76100c60b7..510a1ee4b3 100644 --- a/tests/test_zeroed_state_db.py +++ b/tests/test_zeroed_state_db.py @@ -14,7 +14,7 @@ def test_is_zeroed_state_db_and_quarantine(tmp_path): db.write_bytes(bytes(1024)) assert hs.is_zeroed_state_db(db) is True - q = hs.quarantine_zeroed_state_db(db) + q = hs.quarantine_invalid_state_db(db) assert q is not None assert q.exists() assert not db.exists() @@ -61,6 +61,41 @@ def test_sessiondb_opens_fresh_after_zeroed_quarantine(tmp_path, monkeypatch): sdb.close() +def test_sessiondb_quarantines_page_zero_clobber_before_open(tmp_path, monkeypatch): + """#102198: preserve a non-SQLite page 0 instead of opening degraded.""" + import sqlite3 + + import hermes_state as hs + + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + db = tmp_path / "state.db" + clobbered = (b"bg_032237_0e4ce7A" * 256)[:4096] + db.write_bytes(clobbered) + (tmp_path / "state.db-wal").write_bytes(b"wal evidence") + (tmp_path / "state.db-shm").write_bytes(b"shm evidence") + + assert hs.has_invalid_sqlite_header_preopen(db) is True + assert hs.is_zeroed_state_db(db) is False + + sdb = hs.SessionDB(db_path=db) + try: + backups = list(tmp_path.glob("state.db.notadb-*.bak")) + assert len(backups) == 1 + assert backups[0].read_bytes() == clobbered + assert Path(str(backups[0]) + "-wal").read_bytes() == b"wal evidence" + assert Path(str(backups[0]) + "-shm").read_bytes() == b"shm evidence" + assert db.read_bytes().startswith(b"SQLite format 3\x00") + sdb.create_session(session_id="after-quarantine", source="test") + finally: + sdb.close() + + conn = sqlite3.connect(str(db)) + try: + assert conn.execute("PRAGMA integrity_check").fetchone()[0] == "ok" + finally: + conn.close() + + def test_is_zeroed_state_db_zero_byte_quarantine(tmp_path, monkeypatch): """#97568: a 0-byte file must be detected as zeroed and quarantined.""" import hermes_state as hs @@ -92,7 +127,6 @@ def test_is_zeroed_state_db_zero_byte_quarantine(tmp_path, monkeypatch): sdb.close() - def test_concurrent_quarantine_no_clobber(tmp_path): """#68805: two concurrent startups must not race on quarantine. @@ -181,19 +215,23 @@ def test_quarantine_fails_closed_when_lock_held(tmp_path): 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() @@ -207,11 +245,11 @@ def test_quarantine_fails_closed_when_lock_held(tmp_path): # 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) + result = hs.quarantine_invalid_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"quarantine_invalid_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" @@ -298,4 +336,3 @@ def test_live_connection_0_byte_not_quarantined_in_process(tmp_path, monkeypatch conn.commit() finally: conn.close() -