Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inherit HTTP cache file read/write permissions from cache directory #13070

Merged
merged 11 commits into from
Dec 9, 2024
Prev Previous commit
Next Next commit
Guard against symlink related attack
JustinVanHeek committed Nov 15, 2024
commit c0f5b0a3e7715ab993ad5f886b265e503f88214f
18 changes: 15 additions & 3 deletions src/pip/_internal/network/cache.py
Original file line number Diff line number Diff line change
@@ -77,9 +77,21 @@ def _write(self, path: str, data: bytes) -> None:
with adjacent_tmp_file(path) as f:
f.write(data)

# `& 0o666` selects the read/write permissions of the cache directory.
# `| 0o600` sets owner read/write permissions.
os.chmod(f.name, os.stat(self.directory).st_mode & 0o666 | 0o600)
# Change permissions only if there is no risk of following a symlink
if (
os.chmod in os.supports_fd
or os.chmod in os.supports_follow_symlinks
):
os.chmod(
path=f.fileno() if os.chmod in os.supports_fd else f.name,
mode=(
os.stat(self.directory).st_mode
& 0o666 # select read/write permissions of cache directory
| 0o600 # set owner read/write permissions
),
follow_symlinks=os.chmod not in os.supports_follow_symlinks,
)

replace(f.name, path)

def set(
18 changes: 17 additions & 1 deletion tests/unit/test_network_cache.py
Original file line number Diff line number Diff line change
@@ -84,14 +84,30 @@ def test_cache_hashes_are_same(self, cache_tmpdir: Path) -> None:
assert cache._get_cache_path(key) == FileCache._fn(fake_cache, key)

@pytest.mark.skipif("sys.platform == 'win32'")
@pytest.mark.skipif(
os.chmod not in os.supports_fd and os.chmod not in os.supports_follow_symlinks,
reason="requires os.chmod to support file descriptors or not follow symlinks",
)
@pytest.mark.parametrize(
"perms, expected_perms", [(0o300, 0o600), (0o700, 0o600), (0o777, 0o666)]
)
def test_cache_inherits_perms(
def test_cache_inherit_perms(
self, cache_tmpdir: Path, perms: int, expected_perms: int
) -> None:
key = "foo"
with chmod(cache_tmpdir, perms):
cache = SafeFileCache(os.fspath(cache_tmpdir))
cache.set(key, b"bar")
assert (os.stat(cache._get_cache_path(key)).st_mode & 0o777) == expected_perms

@pytest.mark.skipif("sys.platform == 'win32'")
def test_cache_not_inherit_perms(self, cache_tmpdir: Path, monkeypatch) -> None:
monkeypatch.setattr(os, "supports_fd", os.supports_fd - {os.chmod})
monkeypatch.setattr(
os, "supports_follow_symlinks", os.supports_follow_symlinks - {os.chmod}
)
key = "foo"
with chmod(cache_tmpdir, 0o777):
cache = SafeFileCache(os.fspath(cache_tmpdir))
cache.set(key, b"bar")
assert (os.stat(cache._get_cache_path(key)).st_mode & 0o777) == 0o600