From 8ddaabe07a2cbcf9833ad97b6ccb98a00ab317e6 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Thu, 5 Nov 2026 11:57:06 +0100 Subject: [PATCH] fix: check the core file at a refused pin path The integrity check let a platform file the frontend's hash refuses hold its path, so the core file the builder ships there was never compared (finding 3). --- scripts/packverify.py | 33 +++++++++++++++------ tests/test_pack_core_extras.py | 53 ++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/scripts/packverify.py b/scripts/packverify.py index c52f466b2..60486be1a 100644 --- a/scripts/packverify.py +++ b/scripts/packverify.py @@ -30,7 +30,7 @@ from packextras import platform_region_groups from packpaths import _flat from nativemode import reads_file_contents import region as region_mod -from packresolve import resolve_file +from packresolve import resolve_declaration, resolve_file from exporter.candidates import BIOS, EntryCategories from gamedata import without_game_data from common import resolve_platform_cores @@ -544,6 +544,21 @@ def _structural_errors(zf: zipfile.ZipFile, zip_set: set) -> list[str]: return errors +def _shipped_copy( + entry: dict, db: dict, data_registry: dict | None, mode: str, hash_rule: str, +) -> str | None: + """The file the builder packs for a declaration, or None. + + The verdict verify applies before it lets a declaration hold its path. + """ + local, status = resolve_declaration( + entry, db, DEFAULT_BIOS_DIR, None, data_registry, None, offline=True, + ) + if local and baseline_verdict(entry, local, status, mode, hash_rule)[1] is None: + return local + return None + + def _builder_extras( config: dict, emulators_dir: str, @@ -563,15 +578,17 @@ def _builder_extras( """{full destination: local copy} of each core extra the builder writes. The platform's paths come first, as in the builder: a destination the - pack holds whose declarations resolve to a file, or that the platform - keeps for a user_provided or external entry. One whose declarations - resolve to nothing is left free, since the builder then places a core's - file there. + pack holds whose declarations resolve to a file it ships, or that the + platform keeps for a user_provided or external entry. One whose + declarations resolve to nothing, or to a file the frontend's hash + refuses, is left free, since the builder then places a core's file + there: taken by the refused file, that core file was never compared. """ from packextras import ExtraResolver, hidden_extras - from packresolve import resolve_declaration from validation import _build_validation_index, destination_owners + mode = config.get("verification_mode", "existence") + hash_rule = config.get("hash_rule", "") shipped: dict[str, str | None] = {} for expected, declarations in baseline_groups.items(): if expected not in zip_set and expected.lower() not in zip_lower: @@ -587,9 +604,7 @@ def _builder_extras( local = next( ( found for fe in declarations - if (found := resolve_declaration( - fe, db, DEFAULT_BIOS_DIR, None, data_registry, None, offline=True, - )[0]) + if (found := _shipped_copy(fe, db, data_registry, mode, hash_rule)) ), None, ) diff --git a/tests/test_pack_core_extras.py b/tests/test_pack_core_extras.py index d997b8b97..82c10eed9 100644 --- a/tests/test_pack_core_extras.py +++ b/tests/test_pack_core_extras.py @@ -87,6 +87,59 @@ class CoreFilesAsTheBuilderPlacesThem(unittest.TestCase): ) +class ARefusedPinLeavesItsPathToTheCore(unittest.TestCase): + """EmuELEC pins naomi.zip by an md5 the collection does not hold: the + builder refuses the held copy and ships the core's file at that path. + The check marked the path taken by the refused platform file, so the + core file the pack carries there was never compared.""" + + def setUp(self): + tmp = tempfile.TemporaryDirectory(dir=REPO_ROOT / "tmp") + self.addCleanup(tmp.cleanup) + previous = os.getcwd() + os.chdir(tmp.name) + self.addCleanup(os.chdir, previous) + files: dict = {} + by_name: dict = {} + _held(files, by_name, "bios/dc/naomi.zip", b"the core's naomi") + self.db = {"files": files, "indexes": { + "by_name": by_name, "by_md5": {e["md5"]: s for s, e in files.items()}, + "by_path_suffix": {}, "by_crc32": {}, "by_sha256": {}}} + Path("platforms").mkdir() + config = {"platform": "P", "verification_mode": "md5", "cores": ["flycast"], + "systems": {"dc": {"files": [{ + "name": "naomi.zip", "destination": "dc/naomi.zip", + "md5": hashlib.md5(b"a bundle nobody holds").hexdigest(), + }]}}} + Path("platforms/p.yml").write_text(yaml.safe_dump(config), encoding="utf-8") + Path("emulators").mkdir() + self.profiles = {"flycast": { + "emulator": "flycast", "type": "libretro", "systems": ["dc"], "files": [ + {"name": "naomi.zip", "path": "dc/naomi.zip"}]}} + + def _check(self, member: bytes) -> tuple: + with zipfile.ZipFile("pack.zip", "w") as zf: + zf.writestr("dc/naomi.zip", member) + return packverify.verify_pack_against_platform( + "pack.zip", "p", "platforms", self.db, emulators_dir="emulators", + emu_profiles=self.profiles, + ) + + def test_the_core_file_at_the_refused_path_is_compared(self): + _ok, _c, _p, errors, *_mid, core_checked, _cp, _ex, _ab = self._check(b"other") + self.assertEqual(core_checked, 1) + self.assertTrue( + any(e.startswith("core file differs: dc/naomi.zip") for e in errors), errors + ) + + def test_the_right_core_file_passes(self): + _ok, _c, _p, errors, *_mid, core_checked, core_present, _ex, _ab = ( + self._check(b"the core's naomi") + ) + self.assertEqual((core_checked, core_present), (1, 1)) + self.assertFalse([e for e in errors if e.startswith("core")], errors) + + class NarrowedPacksAgainstTheirOwnList(unittest.TestCase): """--verify-packs after --all-variants said "hashes checked during the build" for the five narrowed packs and compared none of them: a narrowed