Add protection against cache poisoning.

changes/3045.bugfix.md

@@ -0,0 +1 @@
+Downloads that are already present in the download cache are now re-verified against their expected hash, rather than being trusted unconditionally. This provides protection against cache poisoning as a potential attack vector.

src/briefcase/integrations/file.py

@@ -282,6 +282,13 @@ def download(
           doesn't match, a `CorruptContentError` is raised. Any fixed-length
           hash algorithm provided by hashlib can be used.
 
+        If the file is already present in the download cache, it is *not*
+        re-downloaded; however, its content is still re-hashed and compared
+        against `expected_hash` every time, so a corrupted or tampered cache
+        entry will still raise `CorruptContentError`. A console message
+        confirms when a hash has been checked and verified, for both cached
+        and freshly downloaded files.
+
         :param url: The URL to download
         :param download_path: The path to the download cache folder. This path
             will be created if it doesn't exist.
@@ -331,14 +338,31 @@ def download(
                 cache_name = cache_full_name.split("/")[-1]
                 filename = download_path / cache_name
 
+                if expected_hash is None:
+                    self.tools.console.warning(
+                        f"The integrity of {cache_name} will not be verified "
+                        "as no reference hash been provided."
+                    )
+
                 if filename.exists():
-                    self.tools.console.info(f"{cache_name} already downloaded")
-                else:
-                    if expected_hash is None:
-                        self.tools.console.warning(
-                            f"The integrity of {cache_name} will not be verified "
-                            "as no reference hash been provided."
+                    if algorithm is None:
+                        self.tools.console.info(f"{cache_name} already downloaded.")
+                    else:
+                        with filename.open("rb") as f:
+                            hash = hashlib.file_digest(f, algorithm)
+
+                        actual_digest = hash.hexdigest()
+                        if actual_digest.lower() != digest.lower():
+                            raise CorruptContentError(
+                                role=role or filename.name,
+                                expected_hash=f"{algorithm}:{digest}",
+                                actual_hash=f"{algorithm}:{actual_digest}",
+                            )
+                        self.tools.console.info(
+                            f"{cache_name} already downloaded; "
+                            f"hash verified ({algorithm})."
                         )
+                else:
                     self.tools.console.info(f"Downloading {cache_name}...")
                     self._fetch_and_write_content(
                         response,
@@ -488,11 +512,15 @@ def _fetch_and_write_content(
                                 hasher.update(data)
                             progress_bar.update(task_id, advance=len(data))
 
-            if hasher is not None and hasher.hexdigest().lower() != digest.lower():
-                raise CorruptContentError(
-                    role=role or filename.name,
-                    expected_hash=f"{hasher.name}:{digest}",
-                    actual_hash=f"{hasher.name}:{hasher.hexdigest()}",
+            if hasher is not None:
+                if hasher.hexdigest().lower() != digest.lower():
+                    raise CorruptContentError(
+                        role=role or filename.name,
+                        expected_hash=f"{hasher.name}:{digest}",
+                        actual_hash=f"{hasher.name}:{hasher.hexdigest()}",
+                    )
+                self.tools.console.info(
+                    f"{filename.name} hash verified ({hasher.name})."
                 )
 
             # This file move short circuits to a file rename when the source and

tests/integrations/file/test_File__download.py

@@ -221,11 +221,17 @@ def test_new_download_oneshot(
     with (mock_tools.base_path / "downloads/something.zip").open(encoding="utf-8") as f:
         assert f.read() == "all content"
 
-    # Verify that the expected warnings are output
+    # Verify that the expected warnings/confirmations are output
+    output = capsys.readouterr().out
     if hash_algorithm is None:
-        assert "will not be verified" in capsys.readouterr().out
+        assert "will not be verified" in output
+        assert "hash verified" not in output
+    elif hash_algorithm == "unverified":
+        assert "will not be verified" not in output
+        assert "hash verified" not in output
     else:
-        assert "will not be verified" not in capsys.readouterr().out
+        assert "will not be verified" not in output
+        assert f"hash verified ({hash_algorithm})" in output
 
 
 @pytest.mark.parametrize(
@@ -299,16 +305,23 @@ def test_new_download_chunked(mock_tools, file_perms, hash_algorithm, capsys):
     with (mock_tools.base_path / "something.zip").open(encoding="utf-8") as f:
         assert f.read() == "chunk-1;chunk-2;chunk-3;"
 
-    # Verify that the expected warnings are output
+    # Verify that the expected warnings/confirmations are output
+    output = capsys.readouterr().out
     if hash_algorithm is None:
-        assert "will not be verified" in capsys.readouterr().out
+        assert "will not be verified" in output
+        assert "hash verified" not in output
+    elif hash_algorithm == "unverified":
+        assert "will not be verified" not in output
+        assert "hash verified" not in output
     else:
-        assert "will not be verified" not in capsys.readouterr().out
+        assert "will not be verified" not in output
+        assert f"hash verified ({hash_algorithm})" in output
 
 
 @pytest.mark.parametrize("hash_algorithm", [None, "unverified", "sha256"])
 def test_already_downloaded(mock_tools, hash_algorithm, capsys):
-    """If the file already exists on disk, it isn't re-downloaded.
+    """If the file already exists on disk, it isn't re-downloaded, but its hash is re-
+    verified against expected_hash.
 
     The request is still made to derive the filename, but the content is never streamed.
     """
@@ -367,8 +380,73 @@ def test_already_downloaded(mock_tools, hash_algorithm, capsys):
     mock_tools.os.chmod.assert_not_called()
     mock_tools.os.remove.assert_not_called()
 
-    # No mention of verification
-    assert "will not be verified" not in capsys.readouterr().out
+    output = capsys.readouterr().out
+    if hash_algorithm is None:
+        # No hash was provided, so a warning is logged, and there's nothing to
+        # confirm as verified.
+        assert "will not be verified" in output
+        assert "already downloaded." in output
+        assert "already downloaded; hash verified" not in output
+    elif hash_algorithm == "unverified":
+        # Verification was deliberately skipped; no warning, nothing verified.
+        assert "will not be verified" not in output
+        assert "already downloaded." in output
+        assert "already downloaded; hash verified" not in output
+    else:
+        # A real hash was provided and matched; confirm it was verified.
+        assert "will not be verified" not in output
+        assert "already downloaded." not in output
+        assert f"already downloaded; hash verified ({hash_algorithm})" in output
+
+
+def test_already_downloaded_hash_mismatch(mock_tools, capsys):
+    """If the cached file's content doesn't match expected_hash, CorruptContentError is
+    raised, even though the file is already on disk."""
+    content = b"existing content"
+
+    # Create an existing file whose content doesn't match the expected hash
+    existing_file = mock_tools.base_path / "something.zip"
+    with existing_file.open("w", encoding="utf-8") as f:
+        f.write(content.decode())
+
+    url = "https://example.com/path/to/something.zip"
+
+    response = _make_httpx_response(
+        status_code=200,
+        url=url,
+        headers={"content-length": "100", "content-encoding": "gzip"},
+        stream=[b"definitely not gzip content"],
+    )
+    mock_tools.httpx.stream.return_value.__enter__.return_value = response
+
+    expected_hash = f"sha256:{'0' * 64}"
+
+    with pytest.raises(CorruptContentError) as exc_info:
+        mock_tools.file.download(
+            url=url,
+            download_path=mock_tools.base_path,
+            role="something",
+            expected_hash=expected_hash,
+        )
+
+    assert exc_info.value.role == "something"
+    assert exc_info.value.expected_hash == expected_hash
+    assert exc_info.value.actual_hash == (
+        f"sha256:{hashlib.sha256(content).hexdigest()}"
+    )
+
+    # The cached file was not touched
+    assert existing_file.exists()
+    with existing_file.open(encoding="utf-8") as f:
+        assert f.read() == content.decode()
+
+    # Temporary file was not created, moved, or deleted
+    mock_tools.shutil.move.assert_not_called()
+    mock_tools.os.chmod.assert_not_called()
+    mock_tools.os.remove.assert_not_called()
+
+    # No success confirmation was logged
+    assert "hash verified" not in capsys.readouterr().out
 
 
 def test_missing_resource(mock_tools):