From 5db49b3d29ab4fd1422775bd017885b0ba9d80b3 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 08:39:58 -0400 Subject: [PATCH] fix: whole-file read-only integrity preflight on the executor, rollback-journal fixture - preflight now runs PRAGMA integrity_check (whole-file validation, not just sqlite_master) over a mode=ro connection, dispatched via hass.async_add_executor_job so the event loop never blocks - migration-stage corruption test fixture is now a ROLLBACK-JOURNAL file created with plain sqlite3 (default journal mode), so the WAL transition the guardrail protects against actually happens in the test instead of being masked by an already-WAL fixture --- custom_components/nestquest/__init__.py | 36 ++++++++++++++++++------- tests/test_startup_db.py | 26 +++++++++++++++--- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 5d9357c..8e7a341 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -57,25 +57,41 @@ def _is_corruption_error(error: BaseException) -> bool: return any(marker in message for marker in _CORRUPTION_MARKERS) -def _preflight_existing_file(db_path: Path) -> None: +async def _preflight_existing_file( + hass: HomeAssistant, db_path: Path +) -> None: """Detect corruption in an EXISTING file before it is opened read-write. - Opening read-write persists WAL mode and can create -wal/-shm - sidecars, which would modify a corrupt file before detection — - breaking the leave-untouched guarantee. This preflight reads the - file's header with a READ-ONLY connection (mode=ro URI): a corrupt - header raises there, before anything is written. Missing files - skip the preflight (they are created fresh by the real open). + Opening read-write persists WAL mode — for a rollback-journal file + that rewrites the header byte 18/19 and can create -wal/-shm + sidecars — which would modify a corrupt file before detection and + break the leave-untouched guarantee. This preflight therefore + validates the WHOLE file with a READ-ONLY connection (mode=ro URI, + PRAGMA integrity_check, run on the executor like every other DB + call): corruption anywhere fails there, before anything is + written. Missing/empty files skip the preflight (they are created + fresh by the real open). """ if not db_path.exists() or db_path.stat().st_size == 0: return uri = f"file:{db_path}?mode=ro" - try: + + def _probe() -> None: conn = sqlite3.connect(uri, uri=True) try: - conn.execute("SELECT count(*) FROM sqlite_master").fetchone() + row = conn.execute("PRAGMA integrity_check").fetchone() finally: conn.close() + verdict = row[0] if row else "missing" + if verdict != "ok": + raise sqlite3.DatabaseError( + f"integrity_check on read-only probe failed: {verdict}" + ) + + try: + # Every DB-touching call goes through the executor (the mock + # harness's async_add_executor_job is awaitable too). + await hass.async_add_executor_job(_probe) except sqlite3.DatabaseError as err: if _is_corruption_error(err): raise @@ -111,7 +127,7 @@ async def _async_open_database( same way. """ try: - _preflight_existing_file(db_path) + await _preflight_existing_file(hass, db_path) database = await NestQuestDatabase(hass).open(db_path) except asyncio.CancelledError: raise diff --git a/tests/test_startup_db.py b/tests/test_startup_db.py index e262500..04e650d 100644 --- a/tests/test_startup_db.py +++ b/tests/test_startup_db.py @@ -185,14 +185,34 @@ def test_migration_stage_corruption_leaves_db_and_sidecars_untouched( The leave-untouched guarantee must still hold byte-for-byte for the main file AND no WAL/SHM sidecars may be left behind for the owner to clean up. + + The fixture is deliberately a ROLLBACK-JOURNAL file (created with + plain sqlite3, default journal_mode=delete, NOT via the wrapper): + that is the case where the WAL transition actually rewrites the + header — a WAL-mode fixture would mask the write under test. """ db_path = tmp_path / "nestquest.db" - _valid_database(db_path) + conn = sqlite3.connect(db_path) + try: + conn.execute("PRAGMA journal_mode=DELETE") + conn.execute( + "CREATE TABLE filler (id INTEGER PRIMARY KEY, " + "payload TEXT NOT NULL)" + ) + conn.executemany( + "INSERT INTO filler (payload) VALUES (?)", + [(f"row-{index}" * 8,) for index in range(200)], + ) + conn.commit() + finally: + conn.close() data = bytearray(db_path.read_bytes()) + assert conn.execute is not None + assert len(data) > 4200, "fixture must span more than one page" # Corrupt a page well past the 100-byte header (offset 4096, the # second page region): the header itself stays valid, so the - # read-write open succeeds and only migrations discover damage. - assert len(data) > 4200 + # read-write open succeeds and only the integrity probe/migrations + # discover damage. for offset in range(4096, 4128): data[offset] = 0xFF db_path.write_bytes(bytes(data))