Skip to content

Commit e16b86b

Browse files
smoparthclaude
andcommitted
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 <claude@anthropic.com> Signed-off-by: Shanmukh Pawan <smoparth@redhat.com>
1 parent ebbd889 commit e16b86b

2 files changed

Lines changed: 22 additions & 0 deletions

File tree

src/fromager/downloads.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import logging
44
import os
55
import pathlib
6+
import stat
67
import tarfile
78
import tempfile
89
import typing
@@ -81,6 +82,14 @@ def _download_with_retry() -> pathlib.Path:
8182
for chunk in r.iter_content(chunk_size=64 * 1024):
8283
if chunk:
8384
tmp.write(chunk)
85+
# NamedTemporaryFile creates files with mode 0o600. Widen to
86+
# 0o644 so external wheel servers (e.g. nginx) running as a
87+
# different user can read the file. Using fchmod before
88+
# close+rename avoids a window where the final path exists
89+
# with overly restrictive permissions.
90+
os.fchmod(
91+
tmp.fileno(), stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH
92+
)
8493
tmp.close()
8594
# Atomic rename on the same filesystem
8695
os.rename(temp_path, outfile)

tests/test_downloads.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
from __future__ import annotations
22

33
import io
4+
import os
45
import pathlib
6+
import stat
57
import tarfile
68
import typing
79
import zipfile
@@ -72,6 +74,17 @@ def test_download_url_creates_parent_dirs(
7274
assert download_url(destination_dir=tmp_path / "a" / "b", url=_PKG_URL).exists()
7375

7476

77+
def test_download_url_world_readable(
78+
requests_mock: requests_mock.Mocker, tmp_path: pathlib.Path
79+
) -> None:
80+
"""Downloaded files must be readable by other users (e.g. nginx)."""
81+
requests_mock.get(_PKG_URL, content=b"data")
82+
result = download_url(destination_dir=tmp_path, url=_PKG_URL)
83+
mode = stat.S_IMODE(os.stat(result).st_mode)
84+
assert mode & stat.S_IRGRP, "group-read bit must be set"
85+
assert mode & stat.S_IROTH, "other-read bit must be set"
86+
87+
7588
def test_download_url_cleans_up_on_failure(
7689
requests_mock: requests_mock.Mocker, tmp_path: pathlib.Path
7790
) -> None:

0 commit comments

Comments
 (0)