From baf635cf4e9616195a87a1686b9c0e6f7388c436 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 18 Oct 2026 05:15:15 +0200 Subject: [PATCH] fix: refuse to pack an archive that fails its CRC The deterministic rebuild raises on a member whose bytes fail their CRC, and the builder caught it and copied the damaged archive into the pack, expecting verify to flag it, which nothing did. The run now stops on it and names the file to restore. --- scripts/generate_pack.py | 21 +++++++++++++++++---- tests/test_audit_regressions.py | 26 ++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index 94f6304d..9e1fbf6d 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -1217,6 +1217,10 @@ def _add_zip_to_pack( _normalize_zip_for_pack(source_zip, dest_path, target_zf) +class DamagedArchiveError(OSError): + """An archive of the collection whose members fail their own CRC.""" + + def _normalize_zip_for_pack( source_zip: str, dest_path: str, target_zf: zipfile.ZipFile ) -> None: @@ -1241,9 +1245,12 @@ def _normalize_zip_for_pack( try: rebuild_zip_deterministic(source_zip, tmp_path) _add_pack_member(target_zf, tmp_path, dest_path) - except zipfile.BadZipFile: - # Corrupt source ZIP: copy as-is (will be flagged by verify) - _add_pack_member(target_zf, source_zip, dest_path) + except zipfile.BadZipFile as exc: + # The rebuild checks every member's CRC. Copied as-is "for verify to + # flag", a damaged archive went into the pack and nothing flagged it. + raise DamagedArchiveError( + f"{source_zip}: {exc}; restore it from git or the release before packing" + ) from exc finally: os.unlink(tmp_path) @@ -2847,7 +2854,13 @@ _MISSING_DATA_DIRS: set[str] = set() def main() -> None: with contextlib.ExitStack() as held: - _main(held) + try: + _main(held) + except DamagedArchiveError as exc: + # Emulator, system and hash modes have no per-platform loop to + # report it: the run stops with the file to restore. + print(f"ERROR: {exc}", file=sys.stderr) + sys.exit(1) if _MISSING_DATA_DIRS: print( "ERROR: packs written without data directories that are not cached: " diff --git a/tests/test_audit_regressions.py b/tests/test_audit_regressions.py index 8e695eda..001bd445 100644 --- a/tests/test_audit_regressions.py +++ b/tests/test_audit_regressions.py @@ -534,6 +534,32 @@ class StandaloneCopiesNameTheirSource(unittest.TestCase): self.assertEqual(placed, copies) +class ADamagedArchiveIsNotPacked(unittest.TestCase): + """The deterministic rebuild raises on a member whose bytes fail their + CRC; the builder caught it and copied the damaged archive into the pack + "for verify to flag", which nothing did.""" + + def test_the_rebuild_refusal_reaches_the_run(self): + import zipfile + + from generate_pack import DamagedArchiveError, _normalize_zip_for_pack + + with tempfile.TemporaryDirectory(dir=TMP_ROOT) as directory: + source = Path(directory) / "set.zip" + with zipfile.ZipFile(source, "w", zipfile.ZIP_STORED) as zf: + zf.writestr("rom.bin", b"A" * 64) + data = bytearray(source.read_bytes()) + data[data.index(b"A" * 64)] ^= 0xFF + source.write_bytes(bytes(data)) + pack = Path(directory) / "pack.zip" + with zipfile.ZipFile(pack, "w") as target: + with self.assertRaises(DamagedArchiveError) as caught: + _normalize_zip_for_pack(str(source), "set.zip", target) + self.assertNotIn("set.zip", target.namelist()) + self.assertIsInstance(caught.exception, OSError) + self.assertIn("set.zip", str(caught.exception)) + + class FaqRegressions(unittest.TestCase): """The FAQ states facts the code owns; these tie it back to the source.