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
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
+16
-8
@@ -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"
|
||||
)
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user