Refuse a download that was published without a checksum

The hash was checked when there was one and skipped when there was not,
which is the wrong way round: everything fetched here is either run or
parsed by something written in C++, and the case with no hash is the one
where that matters most and says least.

Not hypothetical. GitHub publishes a digest for whisper.cpp v1.9.1 and for
llama.cpp today, and publishes none for whisper.cpp v1.8.0 and older, so
anything reaching for an older tag would have installed an executable
nobody checked.

ggml.py

@@ -117,7 +117,7 @@ def human_size(count):
 # --- fetching -------------------------------------------------------------
 
 
-def download(item, target, on_progress=None, should_stop=None):
+def download(item, target, on_progress=None, should_stop=None, require_hash=True):
     """Fetch one hub.Item to `target`. True when it landed, False when stopped.
 
     The bytes go to a `.part` that is renamed only after both the length and the
@@ -125,8 +125,18 @@ def download(item, target, on_progress=None, should_stop=None):
     there looking installed and fail much later, inside a server, as a corrupt
     model; a file that is the right length but the wrong content is worse, and
     this is a program as often as it is a model.
+
+    A file whose index published no hash is refused rather than taken on trust.
+    Everything fetched here is either run or parsed by something written in C++,
+    and GitHub did not always publish a digest: a release old enough to predate
+    that would otherwise install unchecked, which is the one case where this
+    would matter most and say least.
     """
     target = pathlib.Path(target)
+    if require_hash and not item.sha256:
+        raise LocalError(t("{name} is published without a checksum, so there is "
+                           "no way to tell what arrived. Nothing was installed.",
+                           name=item.name))
     part = target.with_name(target.name + ".part")
     try:
         target.parent.mkdir(parents=True, exist_ok=True)

tests/test_ggml.py

@@ -119,12 +119,23 @@ class Download(Local):
                 ggml.download(item("m.bin", data), target)
         self.assertFalse(target.exists())
 
-    def test_a_file_with_no_published_checksum_is_still_taken(self):
+    def test_a_file_with_no_published_checksum_is_refused(self):
-        data = b"a README, say"
+        # Everything fetched here is run or parsed by something written in C++,
-        target = self.path("data", "models", "readme")
+        # and GitHub did not always publish a digest.
+        data = b"a program, say"
+        target = self.path("data", "models", "m.bin")
         with fake_urlopen(body(data)):
-            self.assertTrue(ggml.download(item("readme", data, sha=False), target))
+            with self.assertRaises(ggml.LocalError) as caught:
-        self.assertTrue(target.exists())
+                ggml.download(item("m.bin", data, sha=False), target)
+        self.assertIn("checksum", str(caught.exception))
+        self.assertFalse(target.exists())
+
+    def test_nothing_is_asked_for_before_it_is_refused(self):
+        # The refusal is not worth a gigabyte of somebody's bandwidth first.
+        with fake_urlopen(body(b"never read")) as calls:
+            with self.assertRaises(ggml.LocalError):
+                ggml.download(item("m.bin", b"x", sha=False), self.path("m.bin"))
+        self.assertEqual(calls, [])
 
     def test_stopping_leaves_nothing_behind(self):
         data = b"x" * (ggml.DOWNLOAD_CHUNK * 3)
@@ -163,21 +174,26 @@ class Download(Local):
 
 
 class InstallProgram(Local):
-    def release(self, *names):
+    def setUp(self):
-        return {"tag_name": "v1.9.1", "assets": [
+        super().setUp()
-            {"name": name, "browser_download_url": f"https://example.invalid/{name}",
+        # Built once, because the release listing has to publish its checksum
-             "size": 10, "digest": ""} for name in names]}
+        # and a tarball is not the same bytes twice.
-
+        self.archive = tarball({
-    def archive(self):
-        return tarball({
             "whisper-bin-ubuntu-x64/whisper-server": b"#!/bin/sh\nexit 0\n",
             "whisper-bin-ubuntu-x64/libwhisper.so": b"not really a library",
         })
 
+    def release(self, *names, archive=None):
+        digest = hashlib.sha256(self.archive if archive is None else archive)
+        return {"tag_name": "v1.9.1", "assets": [
+            {"name": name, "browser_download_url": f"https://example.invalid/{name}",
+             "size": 10, "digest": "sha256:" + digest.hexdigest()}
+            for name in names]}
+
     def install(self, *names, archive=None):
         self.patch_attr(ggml, "_arch", lambda: "x64")
-        with serving(self.release(*names),
+        blob = self.archive if archive is None else archive
-                     self.archive() if archive is None else archive) as calls:
+        with serving(self.release(*names, archive=blob), blob) as calls:
             path = ggml.install_program(ggml.WHISPER)
         return path, [call.args[0].full_url for call in calls.call_args_list]
 
@@ -229,6 +245,55 @@ class InstallProgram(Local):
             self.install("whisper-bin-ubuntu-x64.tar.gz", archive=empty)
         self.assertIn("whisper-server", str(caught.exception))
 
+
+    def test_a_release_without_a_published_checksum_is_refused(self):
+        # GitHub did not always publish one, and whisper.cpp v1.8.0 and older
+        # still have none.
+        self.patch_attr(ggml, "_arch", lambda: "x64")
+        listing = {"tag_name": "v1.8.0", "assets": [
+            {"name": "whisper-bin-ubuntu-x64.tar.gz",
+             "browser_download_url": "https://example.invalid/w.tar.gz",
+             "size": 10}]}
+        with serving(listing, self.archive):
+            with self.assertRaises(ggml.LocalError) as caught:
+                ggml.install_program(ggml.WHISPER)
+        self.assertIn("checksum", str(caught.exception))
+        self.assertEqual(ggml.installed_program(ggml.WHISPER), "")
+
+    def test_an_archive_that_is_not_what_was_promised_installs_nothing(self):
+        listing = self.release("whisper-bin-ubuntu-x64.tar.gz")
+        other = tarball({"whisper-bin-ubuntu-x64/whisper-server": b"#!/bin/sh\nrm -rf\n"})
+        self.patch_attr(ggml, "_arch", lambda: "x64")
+        with serving(listing, other):
+            with self.assertRaises(ggml.LocalError) as caught:
+                ggml.install_program(ggml.WHISPER)
+        self.assertIn("checksum", str(caught.exception))
+        self.assertEqual(ggml.installed_program(ggml.WHISPER), "")
+
+    def test_an_archive_cannot_write_outside_the_directory_it_is_opened_in(self):
+        # An archive is not a trusted thing to unpack: a member named ../../ is
+        # how one writes over a file it was never given.
+        escape = tarball({"../../../escaped": b"should not land"})
+        path = self.path("data", "bin", "whisper", "v1.9.1")
+        with self.assertRaises(ggml.LocalError):
+            self.install("whisper-bin-ubuntu-x64.tar.gz", archive=escape)
+        self.assertFalse(self.path("escaped").exists())
+        self.assertFalse((path.parent.parent / "escaped").exists())
+
+    def test_a_symlink_out_of_the_directory_does_not_survive_either(self):
+        buf = io.BytesIO()
+        with tarfile.open(fileobj=buf, mode="w:gz") as tar:
+            info = tarfile.TarInfo("whisper-bin-ubuntu-x64/whisper-server")
+            info.type, info.linkname = tarfile.SYMTYPE, "/etc/passwd"
+            tar.addfile(info)
+        with self.assertRaises(ggml.LocalError):
+            self.install("whisper-bin-ubuntu-x64.tar.gz", archive=buf.getvalue())
+
+    def test_everything_is_asked_for_over_tls(self):
+        for url in (hub.GITHUB_API, hub.HF_API, hub.HF_FILES):
+            with self.subTest(url=url):
+                self.assertTrue(url.startswith("https://"))
+
     def test_llama_takes_the_vulkan_build_when_there_is_a_loader(self):
         self.patch_attr(ggml, "_arch", lambda: "x64")
         self.patch_attr(ggml, "_has_vulkan", lambda: True)
@@ -596,3 +661,4 @@ class Sizes(DikteTest):
         self.assertEqual(ggml.human_size(512), "512 B")
         self.assertEqual(ggml.human_size(574041195), "547.4 MB")
         self.assertEqual(ggml.human_size(3_095_033_483), "2.9 GB")
+