From cdd3b84c134691e431ae3e85d0e46f3603956f78 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Mon, 31 Aug 2026 08:32:08 -0700 Subject: [PATCH] fix(state): reject special files in zeroed probe; real schema-bytes decode fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups on the salvage: regular-file guard before the zeroed byte-probe (a FIFO at the state.db path would block startup forever — #98017 review P2), plus an on-main-reproducing UnicodeDecodeError fixture for #98924 (raw bytes in sqlite_master, not messages.content, are what reach pysqlite error-message decode). --- hermes_cli/backup.py | 11 +++- hermes_state.py | 6 +- tests/test_98924_readonly_fts_decode_error.py | 56 +++++++++++++++++++ tests/test_zeroed_state_db.py | 20 +++++++ 4 files changed, 90 insertions(+), 3 deletions(-) diff --git a/hermes_cli/backup.py b/hermes_cli/backup.py index b2a068797b..29dc4d4382 100644 --- a/hermes_cli/backup.py +++ b/hermes_cli/backup.py @@ -441,11 +441,18 @@ def is_zeroed_sqlite_file( ) -> bool: """True when *path* looks like the #68474 zeroed-state.db signature. - Signature: size > 0, first *probe_bytes* are all NUL (no ``SQLite format 3`` - header). Used at SessionDB open and for snapshot diagnostics so a silent + Signature: no ``SQLite format 3`` header and no data — either empty + (size 0, the total-loss case, #97568) or first *probe_bytes* all NUL. + Used at SessionDB open and for snapshot diagnostics so a silent all-zero file becomes a guided recovery instead of a generic failure. + + Only regular files qualify: a special file at the path (FIFO, device, + socket) is never "zeroed" — and probing one could block indefinitely + (opening a FIFO for read waits for a writer), so refuse before any I/O. """ try: + if not path.is_file(): + return False size = path.stat().st_size except OSError: return False diff --git a/hermes_state.py b/hermes_state.py index d81db6e074..a6e8ab3645 100644 --- a/hermes_state.py +++ b/hermes_state.py @@ -3928,7 +3928,7 @@ def _connect_tracked_db(path, tracking_path=None, **kwargs): def is_zeroed_state_db( path: Path, *, probe_bytes: int = 100, force: bool = False ) -> bool: - """Detect the #68474 zeroed state.db signature (size>0, NUL header). + """Detect the #68474/#97568 zeroed state.db signature (0-byte or NUL header). Byte-level probe, so it is only safe BEFORE any connection to *path* exists in this process: ``close()`` cancels every POSIX advisory lock the @@ -3949,6 +3949,10 @@ def is_zeroed_state_db( except Exception: pass try: + if not path.is_file(): + # Special files (FIFO, device, socket) are never "zeroed", and + # probing a FIFO would block until a writer appears. + return False size = path.stat().st_size except OSError: return False diff --git a/tests/test_98924_readonly_fts_decode_error.py b/tests/test_98924_readonly_fts_decode_error.py index 669ef04a8e..fe5c55bd65 100644 --- a/tests/test_98924_readonly_fts_decode_error.py +++ b/tests/test_98924_readonly_fts_decode_error.py @@ -80,3 +80,59 @@ class TestReadOnlyFTSDecodeError: assert read_only2._conn is not None finally: read_only2.close() + + +def _corrupt_schema_with_raw_bytes(db_path: Path) -> None: + """Rewrite an FTS vtable's sqlite_master row with invalid UTF-8 bytes. + + This is the fixture that actually reproduces #98924 on main: pysqlite + raises a bare UnicodeDecodeError at execute() time when SQLite's own + error/schema text carries raw non-UTF-8 file bytes. (Invalid UTF-8 in + messages.content alone does NOT make the LIMIT 0 probe raise.) + """ + conn = sqlite3.connect(str(db_path), isolation_level=None) + try: + conn.execute("PRAGMA writable_schema=ON") + badname = b"tbl_\x81\x82" + conn.execute( + "UPDATE sqlite_master SET name=CAST(? AS TEXT), " + "tbl_name=CAST(? AS TEXT), sql=CAST(? AS TEXT) " + "WHERE name='messages_fts_trigram'", + (badname, badname, b"CREATE GARBAGE \x81\x82"), + ) + ver = conn.execute("PRAGMA schema_version").fetchone()[0] + conn.execute(f"PRAGMA schema_version = {ver + 1}") + conn.execute("PRAGMA writable_schema=OFF") + finally: + conn.close() + + +class TestSchemaBytesDecodeError: + def test_read_only_open_survives_raw_bytes_in_schema(self, tmp_path): + """Genuine on-main repro of #98924: schema-area corruption whose raw + bytes reach pysqlite's error-message decode. Before the fix the probe + re-raised the resulting UnicodeDecodeError and killed read-only init. + """ + db_path = tmp_path / "state.db" + writable = SessionDB(db_path=db_path) + writable.create_session("schema-bytes", source="cli") + writable.append_message("schema-bytes", role="user", content="hello") + writable.close() + + _corrupt_schema_with_raw_bytes(db_path) + + # Precondition: the raw probe really raises UnicodeDecodeError on + # this fixture (guards the test against silently stopping to + # exercise the bug on future SQLite versions). + raw = sqlite3.connect(str(db_path)) + try: + with pytest.raises(UnicodeDecodeError): + raw.execute("SELECT * FROM messages_fts LIMIT 0") + finally: + raw.close() + + read_only = SessionDB(db_path=db_path, read_only=True) + try: + assert read_only._conn is not None + finally: + read_only.close() diff --git a/tests/test_zeroed_state_db.py b/tests/test_zeroed_state_db.py index a3d05b2ab2..76100c60b7 100644 --- a/tests/test_zeroed_state_db.py +++ b/tests/test_zeroed_state_db.py @@ -21,6 +21,26 @@ def test_is_zeroed_state_db_and_quarantine(tmp_path): assert q.read_bytes() == bytes(1024) +@pytest.mark.skipif(not hasattr(__import__("os"), "mkfifo"), reason="POSIX only") +def test_is_zeroed_never_probes_special_files(tmp_path): + """A FIFO at the state.db path must be rejected without any blocking read. + + Opening a FIFO for reading blocks until a writer appears; the zeroed + probe must classify on file type alone (#98017 review, P2). + """ + import os + + import hermes_state as hs + from hermes_cli.backup import is_zeroed_sqlite_file + + fifo = tmp_path / "state.db" + os.mkfifo(fifo) + # Would hang forever before the regular-file guard if either probe + # attempted open()+read on the FIFO. + assert is_zeroed_sqlite_file(fifo) is False + assert hs.is_zeroed_state_db(fifo) is False + + def test_sessiondb_opens_fresh_after_zeroed_quarantine(tmp_path, monkeypatch): import hermes_state as hs