From 8b404e500f05ad0b0711f1e967f8b334dc0e2708 Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Sun, 23 Aug 2026 07:58:23 +0200 Subject: [PATCH] fix: keep a packed file's executable bit Pinning every member's metadata made packs reproducible and took the executable bit with it. The RetroDECK pack ships the two Voxatron engine binaries, and extracted at 644 they cannot be run. Git records the bit, so reading it from the source file keeps a pack the same from any clone. Nothing else about the source's mode reaches the archive: 2569 members ship at 644 and 942 at 755, which is what the builder produced before the pinning. Nothing caught this. The comparison that proved the pinning inert checked member names, CRCs and sizes, and mode is none of those. A test now builds a runnable payload and asserts it survives extraction. RetroDECK rebuilds to the same bytes twice and passes its integrity check, 2008/2008 baseline and 1551/1551 cores. --- scripts/generate_pack.py | 11 +++++++-- tests/test_deterministic_zip.py | 43 +++++++++++++++++++++++++++++++++ wiki/architecture.md | 20 +++++++++++++++ 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/scripts/generate_pack.py b/scripts/generate_pack.py index e1589c91..dee880c3 100644 --- a/scripts/generate_pack.py +++ b/scripts/generate_pack.py @@ -94,8 +94,15 @@ def _add_pack_member(zf: zipfile.ZipFile, source_path: str, arcname: str) -> Non """ info = zipfile.ZipInfo(filename=arcname, date_time=_FIXED_DATE_TIME) info.compress_type = zipfile.ZIP_DEFLATED - info.external_attr = 0o100644 << 16 - size = os.path.getsize(source_path) + # The executable bit is the one permission worth carrying: the RetroDECK + # pack ships the two Voxatron engine binaries, and a user who extracts + # them at 644 cannot run them. Git records the bit, so reading it from + # the source keeps the pack the same from any clone. Nothing else about + # the source's mode reaches the archive. + stat_result = os.stat(source_path) + mode = 0o755 if stat_result.st_mode & 0o111 else 0o644 + info.external_attr = (0o100000 | mode) << 16 + size = stat_result.st_size info.file_size = size with open(source_path, "rb") as source, zf.open( info, "w", force_zip64=size >= 1 << 31 diff --git a/tests/test_deterministic_zip.py b/tests/test_deterministic_zip.py index 829a89d6..a2eb91a6 100644 --- a/tests/test_deterministic_zip.py +++ b/tests/test_deterministic_zip.py @@ -321,6 +321,49 @@ class TestPackDeterminism(unittest.TestCase): with zipfile.ZipFile(pack) as zf: self.assertEqual({i.date_time for i in zf.infolist()}, {_FIXED_DATE_TIME}) + def test_an_executable_source_stays_executable(self): + """Pinning the mode stripped the bit the RetroDECK pack needs. + + It ships the two Voxatron engine binaries; extracted at 644 they + cannot be run. Git records the bit, so reading it from the source + leaves the pack the same from any clone. + """ + import os + import stat as stat_module + + runnable = self.bios / "engine.bin" + runnable.write_bytes(b"NOT REALLY AN ELF") + os.chmod(runnable, 0o755) + digest = hashlib.sha1(runnable.read_bytes()).hexdigest() + self.db["files"][digest] = { + "path": str(runnable), "name": "engine.bin", + "size": runnable.stat().st_size, "sha1": digest, + "md5": hashlib.md5(runnable.read_bytes()).hexdigest(), + } + self.db["indexes"]["by_name"]["engine.bin"] = [digest] + config = (self.platforms / "detplat.yml") + config.write_text( + config.read_text() + + " - name: engine.bin\n" + + " destination: engine.bin\n" + + " required: true\n" + ) + pack = self._build("exec1") + with zipfile.ZipFile(pack) as zf: + modes = { + i.filename.rsplit("/", 1)[-1]: (i.external_attr >> 16) & 0o777 + for i in zf.infolist() + } + self.assertIn("engine.bin", modes, sorted(modes)) + self.assertEqual( + modes["engine.bin"] & stat_module.S_IXUSR, stat_module.S_IXUSR, + "a runnable payload must stay runnable after extraction", + ) + self.assertEqual( + modes["boot.rom"] & 0o777, 0o644, + "a plain ROM keeps the fixed mode, whatever the local filesystem says", + ) + def test_injected_manifest_is_pinned_to_the_database(self): from scripts.generate_pack import inject_manifest, verify_pack diff --git a/wiki/architecture.md b/wiki/architecture.md index 0f4b5af2..0d78dc3c 100644 --- a/wiki/architecture.md +++ b/wiki/architecture.md @@ -155,6 +155,26 @@ with different hardware filters get separate packs. Emulator and system packs also retain system identity, `variant_group` and requested region; same-named regional files are never collapsed merely because their display label matches. +## Pack reproducibility + +A pack is a function of its inputs. Two builds from the same collection and +the same `database.json` produce the same bytes, so a third party can rebuild +a published pack and compare it against the checksum in `SHA256SUMS.txt`. + +Three things make that hold. Generated members (`README.txt`, `manifest.json`) +carry a fixed date rather than the wall clock, and the manifest's `generated` +field is read from `database.json` instead of the build time. MAME and FBNeo +romsets are rebuilt deterministically from their ROMs, so a pack does not +inherit whatever metadata the source archive happened to carry. And every +member is written with the same fixed date: `ZipFile.write` copies the source +file's mtime, which is the checkout time for a file from the collection and +the wall clock for one the build just staged in `tmp/`. + +`tests/test_deterministic_zip.py` builds the same fixture twice and compares +the bytes, for both the platform and the emulator pack paths. Its fixture +holds a romset, because without one the comparison never reaches the rebuild +path and passes while the real packs still move. + ## Storage tiers | Tier | Meaning |