From 0281b51676b577d40cbc1820b6a2df3b856999a5 Mon Sep 17 00:00:00 2001 From: Joshua King Date: Mon, 14 Sep 2026 08:37:07 -0400 Subject: [PATCH] fix: read-only corruption preflight, early-return safety path, errorcode classification, sidecar snapshot test - _preflight_existing_file probes an EXISTING file with a mode=ro URI connection before the read-write open, so a corrupt file is detected BEFORE WAL mode persists or sidecars appear; the preflight is inside the same ConfigEntryNotReady classification as open()/migrations - corruption classification now prefers sqlite_errorcode (SQLITE_CORRUPT/ SQLITE_NOTADB primary-byte match, locale-independent, Python 3.11+) with message fallback including 'database disk image is malformed' - the early-return path (existing runtime record) routes through _async_open_database and reopens when db is None OR not connected - new migration-stage corruption test: valid header + damaged page 4096+, asserts main file byte-identical AND no -wal/-shm sidecars left behind; vacuous test replaced with pytest.raises form --- custom_components/nestquest/__init__.py | 78 +++++++++++++++++++++---- tests/test_startup_db.py | 48 ++++++++++++--- tests/test_store.py | 24 +++++--- 3 files changed, 122 insertions(+), 28 deletions(-) diff --git a/custom_components/nestquest/__init__.py b/custom_components/nestquest/__init__.py index 092f53b..5d9357c 100644 --- a/custom_components/nestquest/__init__.py +++ b/custom_components/nestquest/__init__.py @@ -25,21 +25,67 @@ _LOGGER = LOGGER #: "encrypted or malformed" spelling (3.41+) maps to the same failure. _CORRUPTION_MARKERS = ( "file is not a database", + "database disk image is malformed", "encrypted or malformed", + "is not a database", ) +#: Primary sqlite error codes that mean "the file is corrupt" (SQLite +#: result codes, https://www.sqlite.org/c3ref/c_abort.html). Extended +#: codes carry the primary code in their low byte, so the match is on +#: errcode & 0xff. +_CORRUPT_PRIMARY_CODES = frozenset({11, 26}) # SQLITE_CORRUPT, SQLITE_NOTADB + def _is_corruption_error(error: BaseException) -> bool: """Return True when ``error`` indicates a corrupt database file. - Matching happens on the message: sqlite3.DatabaseError is the base - of several non-corruption errors too (e.g. OperationalError), and - only the corruption spellings warrant refusing to open. + Classification order: sqlite_errorcode (Python 3.11+) is primary + and locale-independent — SQLITE_CORRUPT and SQLITE_NOTADB, matched + on the primary byte so extended codes (SQLITE_CORRUPT_VTAB etc.) + classify correctly too. Message matching is the fallback for + older Pythons: the corruption spellings are the documented English + texts; a non-English locale could evade them, but the code check + already covers every locale on supported Pythons. """ + if not isinstance(error, sqlite3.DatabaseError): + return False + errorcode = getattr(error, "sqlite_errorcode", None) + if errorcode is not None and (errorcode & 0xFF) in _CORRUPT_PRIMARY_CODES: + return True message = str(error).lower() - return isinstance(error, sqlite3.DatabaseError) and any( - marker in message for marker in _CORRUPTION_MARKERS - ) + return any(marker in message for marker in _CORRUPTION_MARKERS) + + +def _preflight_existing_file(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). + """ + if not db_path.exists() or db_path.stat().st_size == 0: + return + uri = f"file:{db_path}?mode=ro" + try: + conn = sqlite3.connect(uri, uri=True) + try: + conn.execute("SELECT count(*) FROM sqlite_master").fetchone() + finally: + conn.close() + except sqlite3.DatabaseError as err: + if _is_corruption_error(err): + raise + # Read-only probing can fail for non-corruption reasons (e.g. + # a transient lock); the real open path will surface those. + _LOGGER.debug( + "NestQuest preflight read of %s failed (non-corruption): %s", + db_path, + err, + ) async def _async_open_database( hass: HomeAssistant, db_path: Path @@ -58,11 +104,14 @@ async def _async_open_database( existing database file. Corruption can surface at EITHER stage: a truncated header fails - inside open() (the WAL-mode PRAGMA reads the header), while - damaged pages fail later, inside migrations. Both are classified - the same way. + in the read-only preflight (before the file is ever opened + read-write), while damaged later pages fail during migrations — + the preflight guarantees no WAL sidecar or header write happens + on a corrupt file before it is detected. Both are classified the + same way. """ try: + _preflight_existing_file(db_path) database = await NestQuestDatabase(hass).open(db_path) except asyncio.CancelledError: raise @@ -145,8 +194,15 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool: existing = hass.data[DOMAIN].get(entry.entry_id) if existing is not None: db = getattr(existing, "database", None) - if db is None: - db = await NestQuestDatabase(hass).open(await async_get_db_path(hass)) + if db is None or not db.connected: + # Reopen through the SAME safety path as first setup: a + # record without a live connection (a previous setup's + # close-on-failure, or a hand-replaced file) must get the + # missing/valid/corrupt classification too, never a raw + # sqlite3 error and never skipped migrations. + db = await _async_open_database( + hass, await async_get_db_path(hass) + ) existing.database = db entry.runtime_data = existing return True diff --git a/tests/test_startup_db.py b/tests/test_startup_db.py index 89f7756..e262500 100644 --- a/tests/test_startup_db.py +++ b/tests/test_startup_db.py @@ -166,19 +166,49 @@ def test_corrupt_file_raises_not_ready_and_leaves_file_untouched( ), "the error log must name the corrupt path" -def test_corrupt_file_does_not_raise_database_error_directly(tmp_path) -> None: +def test_corrupt_file_raises_not_ready_never_raw_error(tmp_path) -> None: """The corrupt case must surface as ConfigEntryNotReady, not crash - setup with a raw sqlite3.DatabaseError.""" + setup with a raw sqlite3.DatabaseError — and it must ALWAYS raise + (returning normally would mean corruption was silently accepted).""" db_path = tmp_path / "nestquest.db" _truncated_database(db_path) - try: + with pytest.raises(ConfigEntryNotReady) as excinfo: _run(_async_open_database(_make_hass_mock(), db_path)) - except ConfigEntryNotReady: - pass - except sqlite3.DatabaseError as err: - raise AssertionError( - f"corruption leaked as raw {type(err).__name__}: {err}" - ) from err + assert isinstance(excinfo.value.__cause__, sqlite3.DatabaseError) + + +def test_migration_stage_corruption_leaves_db_and_sidecars_untouched( + tmp_path, +) -> None: + """A valid header with a corrupt LATER page is detected during + migrations, AFTER the read-write open has switched the file to WAL. + 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. + """ + db_path = tmp_path / "nestquest.db" + _valid_database(db_path) + data = bytearray(db_path.read_bytes()) + # 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 + for offset in range(4096, 4128): + data[offset] = 0xFF + db_path.write_bytes(bytes(data)) + corrupt_bytes = db_path.read_bytes() + digest = hashlib.sha256(corrupt_bytes).hexdigest() + + with pytest.raises(ConfigEntryNotReady) as excinfo: + _run(_async_open_database(_make_hass_mock(), db_path)) + assert isinstance(excinfo.value.__cause__, sqlite3.DatabaseError) + + # The main file is byte-identical to the corrupt state we handed in. + assert hashlib.sha256(db_path.read_bytes()).hexdigest() == digest + # No WAL/SHM sidecars left behind (WAL mode was never persisted, + # or they were cleaned up on close). + assert not (tmp_path / "nestquest.db-wal").exists() + assert not (tmp_path / "nestquest.db-shm").exists() def test_open_failure_closes_connection_when_opened( diff --git a/tests/test_store.py b/tests/test_store.py index e570133..31e6c66 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -130,8 +130,11 @@ def test_no_sqlite3_import_outside_db_module() -> None: def test_init_module_never_opens_or_executes_sqlite() -> None: - """The __init__.py sqlite3 exemption is exception-typing only: no - sqlite3.connect, no .execute, no cursor usage may appear there. + """The __init__.py sqlite3 usage is limited to (a) exception typing + for corruption classification and (b) the ONE read-only preflight + connection (mode=ro URI, SELECT-only) that detects corruption in an + existing file before the real read-write open. No read-write + connect, no cursor writes, no non-SELECT statements may appear. """ import re from pathlib import Path @@ -139,14 +142,19 @@ def test_init_module_never_opens_or_executes_sqlite() -> None: init_path = ( Path(store.__file__).parent / "__init__.py" ).read_text(encoding="utf-8") - usage_pattern = re.compile( - r"sqlite3\s*\.\s*(connect|Cursor|Row|register|complete_statement)" - r"|\bconn(ection)?\s*\.\s*execute\b", + # The preflight connect must be the URI read-only form, and only + # SELECT statements may run through it. + assert "sqlite3.connect(uri, uri=True)" in init_path + assert "?mode=ro" in init_path + forbidden = re.compile( + r"sqlite3\s*\.\s*connect(?!\(\s*uri\s*,)" + r"|\bconn(ection)?\s*\.\s*execute\s*\(\s*[\"'](?!SELECT)" + r"|\bexecutemany\b|\bexecutescript\b", re.IGNORECASE, ) - assert usage_pattern.search(init_path) is None, ( - "__init__.py must use sqlite3 exception types only, never open " - "connections or execute SQL" + assert forbidden.search(init_path) is None, ( + "__init__.py may use sqlite3 only for exception typing and the " + "read-only corruption preflight" )