diff --git a/src/fromager/downloads.py b/src/fromager/downloads.py index 82dd7b8d..e6ffa28a 100644 --- a/src/fromager/downloads.py +++ b/src/fromager/downloads.py @@ -3,6 +3,7 @@ import logging import os import pathlib +import stat import tarfile import tempfile import typing @@ -81,6 +82,14 @@ def _download_with_retry() -> pathlib.Path: for chunk in r.iter_content(chunk_size=64 * 1024): if chunk: tmp.write(chunk) + # NamedTemporaryFile creates files with mode 0o600. Widen to + # 0o644 so external wheel servers (e.g. nginx) running as a + # different user can read the file. Using fchmod before + # close+rename avoids a window where the final path exists + # with overly restrictive permissions. + os.fchmod( + tmp.fileno(), stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH + ) tmp.close() # Atomic rename on the same filesystem os.rename(temp_path, outfile) diff --git a/tests/test_downloads.py b/tests/test_downloads.py index 27ad1d97..94053ae5 100644 --- a/tests/test_downloads.py +++ b/tests/test_downloads.py @@ -1,7 +1,9 @@ from __future__ import annotations import io +import os import pathlib +import stat import tarfile import typing import zipfile @@ -72,6 +74,16 @@ def test_download_url_creates_parent_dirs( assert download_url(destination_dir=tmp_path / "a" / "b", url=_PKG_URL).exists() +def test_download_url_world_readable( + requests_mock: requests_mock.Mocker, tmp_path: pathlib.Path +) -> None: + """Downloaded files must be readable by other users (e.g. nginx).""" + requests_mock.get(_PKG_URL, content=b"data") + result = download_url(destination_dir=tmp_path, url=_PKG_URL) + mode = stat.S_IMODE(os.stat(result).st_mode) + assert mode == 0o644, f"expected 0o644, got {oct(mode)}" + + def test_download_url_cleans_up_on_failure( requests_mock: requests_mock.Mocker, tmp_path: pathlib.Path ) -> None: