From 97396d4734e02ef6818a82a3696860ad7bf31bfe Mon Sep 17 00:00:00 2001 From: yusufipk Date: Sat, 1 Aug 2026 20:42:47 +0300 Subject: [PATCH] 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 | 12 +++++- tests/test_ggml.py | 94 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 91 insertions(+), 15 deletions(-) diff --git a/ggml.py b/ggml.py index 04e46f8..d8778b5 100644 --- a/ggml.py +++ b/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) diff --git a/tests/test_ggml.py b/tests/test_ggml.py index 48eff50..548d4c3 100644 --- a/tests/test_ggml.py +++ b/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): - data = b"a README, say" - target = self.path("data", "models", "readme") + def test_a_file_with_no_published_checksum_is_refused(self): + # Everything fetched here is run or parsed by something written in C++, + # 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)) - self.assertTrue(target.exists()) + with self.assertRaises(ggml.LocalError) as caught: + 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): - return {"tag_name": "v1.9.1", "assets": [ - {"name": name, "browser_download_url": f"https://example.invalid/{name}", - "size": 10, "digest": ""} for name in names]} - - def archive(self): - return tarball({ + def setUp(self): + super().setUp() + # Built once, because the release listing has to publish its checksum + # and a tarball is not the same bytes twice. + self.archive = 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), - self.archive() if archive is None else archive) as calls: + blob = self.archive if archive is None else archive + 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") +