From ffde6194477f8e9e205d690f22523842369d3b5c Mon Sep 17 00:00:00 2001 From: stephamie7 <1223696150@qq.com> Date: Wed, 15 Jul 2026 17:38:00 +0800 Subject: [PATCH] fix: guard unsupported SQLite recovery --- flocks/storage/storage.py | 41 ++++++++++++++++- scripts/recover_raw_flocks_db.py | 32 +++++++++++++- tests/scripts/test_recover_raw_flocks_db.py | 49 ++++++++++++++++++--- tests/storage/test_storage.py | 45 ++++++++++++++++--- 4 files changed, 153 insertions(+), 14 deletions(-) diff --git a/flocks/storage/storage.py b/flocks/storage/storage.py index 4bf9124f..debf1bf1 100644 --- a/flocks/storage/storage.py +++ b/flocks/storage/storage.py @@ -404,6 +404,33 @@ async def _assert_integrity_check_ok(cls, db_path: Path) -> None: f"PRAGMA integrity_check failed for {db_path}: {detail}" ) + @classmethod + def _sqlite_recover_capability_sync(cls, sqlite_bin: str) -> tuple[bool, str]: + """Return whether *sqlite_bin* was built with support for ``.recover``.""" + + try: + completed = subprocess.run( + [ + sqlite_bin, + ":memory:", + "SELECT sqlite_compileoption_used('ENABLE_DBPAGE_VTAB');", + ], + check=False, + capture_output=True, + text=True, + encoding="utf-8", + timeout=cls._sqlite_recover_timeout_s, + ) + except Exception as exc: + return False, str(exc) + + if completed.returncode != 0: + detail = completed.stderr.strip() or completed.stdout.strip() + return False, detail or f"capability probe exited with {completed.returncode}" + if completed.stdout.strip() != "1": + return False, "SQLite CLI was built without SQLITE_ENABLE_DBPAGE_VTAB" + return True, "" + @classmethod def _try_sqlite_recover_sync(cls, quarantined_path: Path, target_path: Path) -> Optional[Path]: """Recover from an isolated copy without allowing SQLite to mutate evidence.""" @@ -420,6 +447,18 @@ def _try_sqlite_recover_sync(cls, quarantined_path: Path, target_path: Path) -> ) return None + supported, reason = cls._sqlite_recover_capability_sync(sqlite_bin) + if not supported: + cls._log.warn( + "storage.corruption.recovery.skipped", + { + "db_path": str(target_path), + "quarantined_path": str(quarantined_path), + "reason": f"sqlite3 .recover is unavailable: {reason}", + }, + ) + return None + working_path = quarantined_path.with_name( f".{quarantined_path.name}.recovery-source-{uuid.uuid4().hex}" ) @@ -517,7 +556,7 @@ def _try_sqlite_recover_working_copy_sync( recover_sql = completed.stdout or "" sql_path.write_text(recover_sql, encoding="utf-8") - if completed.returncode != 0 and not recover_sql.strip(): + if completed.returncode != 0: cls._log.warn( "storage.corruption.recovery.failed", { diff --git a/scripts/recover_raw_flocks_db.py b/scripts/recover_raw_flocks_db.py index 846ef56b..fe1a18b9 100644 --- a/scripts/recover_raw_flocks_db.py +++ b/scripts/recover_raw_flocks_db.py @@ -569,6 +569,32 @@ def reconstruct_sqlite_candidate( } +def _sqlite_recover_capability(sqlite_bin: str = "sqlite3") -> tuple[bool, str]: + """Return whether *sqlite_bin* was built with support for ``.recover``.""" + + try: + completed = subprocess.run( + [ + sqlite_bin, + ":memory:", + "SELECT sqlite_compileoption_used('ENABLE_DBPAGE_VTAB');", + ], + check=False, + capture_output=True, + text=True, + encoding="utf-8", + ) + except OSError as exc: + return False, str(exc) + + if completed.returncode != 0: + detail = completed.stderr.strip() or completed.stdout.strip() + return False, detail or f"capability probe exited with {completed.returncode}" + if completed.stdout.strip() != "1": + return False, "SQLite CLI was built without SQLITE_ENABLE_DBPAGE_VTAB" + return True, "" + + def _run_sqlite_recover( candidate_db: Path, recover_sql_path: Path, @@ -577,6 +603,10 @@ def _run_sqlite_recover( ) -> None: """Write `sqlite3 .recover` output to a SQL file.""" + supported, reason = _sqlite_recover_capability() + if not supported: + raise RuntimeError(f"sqlite3 .recover is unavailable: {reason}") + completed = subprocess.run( [ "sqlite3", @@ -588,7 +618,7 @@ def _run_sqlite_recover( text=True, encoding="utf-8", ) - if completed.returncode != 0 and not completed.stdout.strip(): + if completed.returncode != 0: stderr = completed.stderr.strip() or completed.stdout.strip() raise RuntimeError(f"sqlite3 .recover failed: {stderr or completed.returncode}") with _atomic_output_path(recover_sql_path) as temporary_path: diff --git a/tests/scripts/test_recover_raw_flocks_db.py b/tests/scripts/test_recover_raw_flocks_db.py index b53fdeae..1fe47088 100644 --- a/tests/scripts/test_recover_raw_flocks_db.py +++ b/tests/scripts/test_recover_raw_flocks_db.py @@ -9,6 +9,7 @@ import struct import sys from pathlib import Path +from types import SimpleNamespace import pytest @@ -28,6 +29,15 @@ def load_module(): recover_raw_flocks_db = load_module() +def _require_sqlite_recover() -> None: + sqlite_bin = shutil.which("sqlite3") + if sqlite_bin is None: + pytest.skip("sqlite3 CLI is required for the recovery integration test") + supported, reason = recover_raw_flocks_db._sqlite_recover_capability(sqlite_bin) + if not supported: + pytest.skip(f"sqlite3 .recover is unavailable: {reason}") + + def _wal_checksum(data, *, byte_order, state=(0, 0)): assert len(data) % 8 == 0 words = struct.unpack(f"{byte_order}{len(data) // 4}I", data) @@ -694,9 +704,38 @@ def fake_recovery(raw_path, wal_path, recovery_dir, *, prefix): assert "removed_sidecars=none\n" in capsys.readouterr().out +def test_sqlite_recover_rejects_partial_output_on_failure(tmp_path, monkeypatch): + candidate_db = tmp_path / "candidate.db" + candidate_db.write_bytes(b"candidate") + recover_sql = tmp_path / "recover.sql" + + monkeypatch.setattr( + recover_raw_flocks_db, + "_sqlite_recover_capability", + lambda: (True, ""), + ) + monkeypatch.setattr( + recover_raw_flocks_db.subprocess, + "run", + lambda *_args, **_kwargs: SimpleNamespace( + returncode=1, + stdout="BEGIN;\n", + stderr="sql error: no such table: sqlite_dbpage", + ), + ) + + with pytest.raises(RuntimeError, match="no such table: sqlite_dbpage"): + recover_raw_flocks_db._run_sqlite_recover( + candidate_db, + recover_sql, + lost_and_found_table="lost_and_found", + ) + + assert recover_sql.exists() is False + + def test_real_recovery_preserves_auth_and_custom_tables(tmp_path, monkeypatch): - if shutil.which("sqlite3") is None: - pytest.skip("sqlite3 CLI is required for the recovery integration test") + _require_sqlite_recover() data_dir = tmp_path / "live-data" data_dir.mkdir() @@ -794,8 +833,7 @@ def test_real_recovery_preserves_auth_and_custom_tables(tmp_path, monkeypatch): def test_real_wal_recovery_preserves_committed_row(tmp_path, monkeypatch): - if shutil.which("sqlite3") is None: - pytest.skip("sqlite3 CLI is required for the recovery integration test") + _require_sqlite_recover() data_dir = tmp_path / "live-data" data_dir.mkdir() @@ -863,8 +901,7 @@ def test_real_wal_recovery_preserves_committed_row(tmp_path, monkeypatch): def test_real_recovery_preserves_fts_virtual_table(tmp_path, monkeypatch): - if shutil.which("sqlite3") is None: - pytest.skip("sqlite3 CLI is required for the recovery integration test") + _require_sqlite_recover() source_db = tmp_path / "fts-source.db" with sqlite3.connect(source_db) as db: diff --git a/tests/storage/test_storage.py b/tests/storage/test_storage.py index 62ddb0c6..063731c2 100644 --- a/tests/storage/test_storage.py +++ b/tests/storage/test_storage.py @@ -28,6 +28,15 @@ class StorageTestModel(BaseModel): value: int +def _require_sqlite_recover() -> None: + sqlite_bin = shutil.which("sqlite3") + if sqlite_bin is None: + pytest.skip("sqlite3 CLI is not available") + supported, reason = Storage._sqlite_recover_capability_sync(sqlite_bin) + if not supported: + pytest.skip(f"sqlite3 .recover is unavailable: {reason}") + + @pytest.fixture async def storage(): """Create a temporary storage for testing""" @@ -324,10 +333,36 @@ async def test_storage_get_defers_corruption_recovery_until_restart(tmp_path): assert not any(".corrupt." in name for name in siblings), siblings +def test_try_sqlite_recover_rejects_partial_output_on_failure(tmp_path): + quarantined = tmp_path / "flocks.db.corrupt.test" + quarantined.write_bytes(b"quarantined") + target = tmp_path / "flocks.db" + partial_failure = SimpleNamespace( + returncode=1, + stdout="BEGIN;\n", + stderr="sql error: no such table: sqlite_dbpage", + ) + + with patch( + "flocks.storage.storage.shutil.which", + return_value="/usr/bin/sqlite3", + ), patch.object( + Storage, + "_sqlite_recover_capability_sync", + return_value=(True, ""), + ), patch( + "flocks.storage.storage.subprocess.run", + return_value=partial_failure, + ): + assert Storage._try_sqlite_recover_sync(quarantined, target) is None + + assert target.exists() is False + assert target.with_name("flocks.db.recovered").exists() is False + + def test_try_sqlite_recover_installs_recovered_db(tmp_path): """The lightweight `.recover` path should install a readable recovered DB.""" - if shutil.which("sqlite3") is None: - pytest.skip("sqlite3 CLI is not available") + _require_sqlite_recover() quarantined = tmp_path / "flocks.db.corrupt.test" target = tmp_path / "flocks.db" @@ -371,8 +406,7 @@ def test_try_sqlite_recover_installs_recovered_db(tmp_path): def test_sqlite_recover_reads_wal_paired_with_quarantined_main(tmp_path): """A committed WAL must stay paired with the renamed main DB during recovery.""" - if shutil.which("sqlite3") is None: - pytest.skip("sqlite3 CLI is not available") + _require_sqlite_recover() source = tmp_path / "source.db" source_conn = sqlite3.connect(source) @@ -637,8 +671,7 @@ def blocked_recovery(_quarantined: Path, _target: Path) -> None: @pytest.mark.asyncio async def test_storage_init_recovers_real_malformed_sqlite_file(tmp_path): """Startup should recover a real DB that fails SQLite integrity checks.""" - if shutil.which("sqlite3") is None: - pytest.skip("sqlite3 CLI is not available") + _require_sqlite_recover() db_path = tmp_path / "flocks.db" conn = sqlite3.connect(db_path)