From efef444aa4fdf390374334406a77e55417696b78 Mon Sep 17 00:00:00 2001 From: Shanmukh Pawan Date: Thu, 30 Jul 2026 08:38:36 -0400 Subject: [PATCH] fix(downloads): set 0o644 permissions on downloaded files `NamedTemporaryFile` creates files with mode 0o600, making them unreadable by external wheel servers (e.g. nginx) that run as a different user. This caused 403 Forbidden errors when `uv pip install` tried to fetch wheels from the local package index. Use `os.fchmod` before close+rename to widen permissions to 0o644, preserving the atomicity and thread-safety benefits of `NamedTemporaryFile`. Closes: #1281 Co-Authored-By: Claude Signed-off-by: Shanmukh Pawan --- src/fromager/downloads.py | 9 +++++++++ tests/test_downloads.py | 12 ++++++++++++ 2 files changed, 21 insertions(+) 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: