From 8983f9392a2df451e29d6d40646321cd2b89534c Mon Sep 17 00:00:00 2001 From: Abdessamad Derraz <3028866+Abdess@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:03:00 +0200 Subject: [PATCH] fix: hold a release asset to every declared hash --- scripts/packresolve.py | 70 ++++++++++++++++++++++++++------ tests/test_game_data_homonyms.py | 42 +++++++++++++++++++ 2 files changed, 100 insertions(+), 12 deletions(-) diff --git a/scripts/packresolve.py b/scripts/packresolve.py index d1c6a4d2..93170131 100644 --- a/scripts/packresolve.py +++ b/scripts/packresolve.py @@ -10,8 +10,7 @@ import re _HEX_RE = re.compile(r"\b([0-9a-fA-F]{8,40})\b") from largefiles import fetch_large_file import os -from hashing import parse_md5_list -from common import resolve_local_file +from common import declared_hash_verdict, resolve_local_file # CLI-wide network policy. Public helpers can override it explicitly, while # every CLI mode inherits --offline (including large-file release fallback). _OFFLINE = False @@ -22,6 +21,59 @@ def set_offline(value: bool) -> None: global _OFFLINE _OFFLINE = value +def _values(raw) -> list[str]: + """A hash field's values: one, a list, or a comma-separated string.""" + items = raw if isinstance(raw, list) else [raw] if raw else [] + return [ + value.strip().lower().removeprefix("0x") + for item in items + for value in str(item).split(",") + if value.strip() + ] + + +def _declared_hashes(file_entry: dict) -> dict[str, list[str]]: + return { + field: _values(file_entry.get(field)) + for field in ("sha1", "md5", "sha256", "crc32") + } + + +def _verified_asset( + name: str, declared: dict[str, list[str]], file_entry: dict, offline: bool +) -> str | None: + """The release asset of that name, if it is a copy the entry accepts. + + The store is asked once per accepted sha1 (or md5), since it keeps one + revision under a name; what it returns is held against every hash the + entry declares, sha256 and crc32 included. + """ + attempts = ( + [(sha1, "") for sha1 in declared["sha1"]] + or [("", md5) for md5 in declared["md5"]] + or [("", "")] + ) + for sha1, md5 in attempts: + cached = fetch_large_file( + name, expected_sha1=sha1, expected_md5=md5, offline=offline, + ) + if not cached: + continue + verdict = declared_hash_verdict( + cached, + has_strong_hash=any(declared.values()), + sha1_candidates=set(declared["sha1"]), + sha256_candidates=set(declared["sha256"]), + md5_list=declared["md5"], + crc_candidates=set(declared["crc32"]), + zipped_file=file_entry.get("zipped_file"), + declared_size=file_entry.get("size"), + ) + if verdict != "hash_mismatch": + return cached + return None + + def _detect_hash_type(h: str) -> str: n = len(h) if n == 40: @@ -151,20 +203,14 @@ def resolve_file( # Large files from GitHub release assets -tried when local file is # missing OR has a hash mismatch (wrong variant on disk) name = file_entry.get("name", "") - sha1 = file_entry.get("sha1") - first_sha1 = (sha1[0] if sha1 else "") if isinstance(sha1, list) else (sha1 or "") - md5_list = parse_md5_list(file_entry.get("md5")) - first_md5 = md5_list[0] if md5_list else "" - if file_entry.get("unsourceable") and not (first_sha1 or first_md5): + declared = _declared_hashes(file_entry) + if file_entry.get("unsourceable") and not any(declared.values()): # The asset store is addressed by file name: with no hash to check, # it would hand back the same-named file of another game, which is # exactly what the flag forbids. return (path, status) if path else (None, "not_found") - cached = fetch_large_file( - name, - expected_sha1=first_sha1, - expected_md5=first_md5, - offline=_OFFLINE if offline is None else offline, + cached = _verified_asset( + name, declared, file_entry, _OFFLINE if offline is None else offline, ) if cached: return cached, "release_asset" diff --git a/tests/test_game_data_homonyms.py b/tests/test_game_data_homonyms.py index afa1318c..0fbd45d0 100644 --- a/tests/test_game_data_homonyms.py +++ b/tests/test_game_data_homonyms.py @@ -12,6 +12,7 @@ carries `unsourceable:` until its own bytes are held. from __future__ import annotations +import hashlib import os import re import sys @@ -212,6 +213,47 @@ class ReleaseAssetIsNotANameMatch(unittest.TestCase): fetch.assert_not_called() +class ReleaseAssetAnswersEveryDeclaredHash(unittest.TestCase): + """The fallback passed only the first sha1 or md5 to the asset store: an + entry declaring sha256 or crc32 alone got the same-named asset back + unchecked, and one accepting two sha1s lost a cached copy of the second.""" + + def _resolve(self, entry: dict, cached_bytes: bytes): + import tempfile + from unittest import mock + + import packresolve + + db = {"files": {}, "indexes": {"by_name": {}, "by_md5": {}, "by_path_suffix": {}}} + with tempfile.TemporaryDirectory() as tmp: + cached = Path(tmp) / entry["name"] + cached.write_bytes(cached_bytes) + + def fetch(name, expected_sha1="", expected_md5="", offline=True): + digest = hashlib.sha1(cached_bytes).hexdigest() + return str(cached) if expected_sha1 in ("", digest) else None + + with mock.patch.object(packresolve, "fetch_large_file", side_effect=fetch): + path, status = packresolve.resolve_file(entry, db, "bios", {}, offline=True) + return (Path(path).name if path else None), status + + def test_a_sha256_alone_is_checked(self): + wanted = hashlib.sha256(b"right").hexdigest() + self.assertEqual( + self._resolve({"name": "big.bin", "sha256": wanted}, b"wrong"), + (None, "not_found"), + ) + self.assertEqual( + self._resolve({"name": "big.bin", "sha256": wanted}, b"right"), + ("big.bin", "release_asset"), + ) + + def test_the_second_accepted_sha1_is_found(self): + second = hashlib.sha1(b"second").hexdigest() + entry = {"name": "big.bin", "sha1": [hashlib.sha1(b"first").hexdigest(), second]} + self.assertEqual(self._resolve(entry, b"second"), ("big.bin", "release_asset")) + + class NameStepPrefersTheOwnersCopy(unittest.TestCase): """Same-named files are ordered by where the asking profile keeps its own.