fix: hold a release asset to every declared hash

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"

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.