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
This commit is contained in:
@@ -57,25 +57,41 @@ def _is_corruption_error(error: BaseException) -> bool:
|
|||||||
return 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:
|
async def _preflight_existing_file(
|
||||||
|
hass: HomeAssistant, db_path: Path
|
||||||
|
) -> None:
|
||||||
"""Detect corruption in an EXISTING file before it is opened read-write.
|
"""Detect corruption in an EXISTING file before it is opened read-write.
|
||||||
|
|
||||||
Opening read-write persists WAL mode and can create -wal/-shm
|
Opening read-write persists WAL mode — for a rollback-journal file
|
||||||
sidecars, which would modify a corrupt file before detection —
|
that rewrites the header byte 18/19 and can create -wal/-shm
|
||||||
breaking the leave-untouched guarantee. This preflight reads the
|
sidecars — which would modify a corrupt file before detection and
|
||||||
file's header with a READ-ONLY connection (mode=ro URI): a corrupt
|
break the leave-untouched guarantee. This preflight therefore
|
||||||
header raises there, before anything is written. Missing files
|
validates the WHOLE file with a READ-ONLY connection (mode=ro URI,
|
||||||
skip the preflight (they are created fresh by the real open).
|
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:
|
if not db_path.exists() or db_path.stat().st_size == 0:
|
||||||
return
|
return
|
||||||
uri = f"file:{db_path}?mode=ro"
|
uri = f"file:{db_path}?mode=ro"
|
||||||
try:
|
|
||||||
|
def _probe() -> None:
|
||||||
conn = sqlite3.connect(uri, uri=True)
|
conn = sqlite3.connect(uri, uri=True)
|
||||||
try:
|
try:
|
||||||
conn.execute("SELECT count(*) FROM sqlite_master").fetchone()
|
row = conn.execute("PRAGMA integrity_check").fetchone()
|
||||||
finally:
|
finally:
|
||||||
conn.close()
|
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:
|
except sqlite3.DatabaseError as err:
|
||||||
if _is_corruption_error(err):
|
if _is_corruption_error(err):
|
||||||
raise
|
raise
|
||||||
@@ -111,7 +127,7 @@ async def _async_open_database(
|
|||||||
same way.
|
same way.
|
||||||
"""
|
"""
|
||||||
try:
|
try:
|
||||||
_preflight_existing_file(db_path)
|
await _preflight_existing_file(hass, db_path)
|
||||||
database = await NestQuestDatabase(hass).open(db_path)
|
database = await NestQuestDatabase(hass).open(db_path)
|
||||||
except asyncio.CancelledError:
|
except asyncio.CancelledError:
|
||||||
raise
|
raise
|
||||||
|
|||||||
@@ -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 leave-untouched guarantee must still hold byte-for-byte for
|
||||||
the main file AND no WAL/SHM sidecars may be left behind for the
|
the main file AND no WAL/SHM sidecars may be left behind for the
|
||||||
owner to clean up.
|
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"
|
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())
|
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
|
# Corrupt a page well past the 100-byte header (offset 4096, the
|
||||||
# second page region): the header itself stays valid, so the
|
# second page region): the header itself stays valid, so the
|
||||||
# read-write open succeeds and only migrations discover damage.
|
# read-write open succeeds and only the integrity probe/migrations
|
||||||
assert len(data) > 4200
|
# discover damage.
|
||||||
for offset in range(4096, 4128):
|
for offset in range(4096, 4128):
|
||||||
data[offset] = 0xFF
|
data[offset] = 0xFF
|
||||||
db_path.write_bytes(bytes(data))
|
db_path.write_bytes(bytes(data))
|
||||||
|
|||||||
Reference in New Issue
Block a user