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

Conversation

JustinVanHeek
Copy link
Contributor

@JustinVanHeek JustinVanHeek commented Nov 8, 2024

Resolves #11012.

The Problem

In a multi-user environment, it would be nice to be able to share a single cache. The NamedTemporaryFile class that is used to create the files is hardcoded to use file mode 600 (owner read/write only). This makes it impossible to share a cache with other users as they would need read/write permissions set for group or others.

The Solution

This PR resolves the problem by changing the file permissions after the file has been written to have the same read/write permissions as those from the cache's directory. One exception is that the owner read/write permissions will always be set (just as it was prior to this change).

Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not that familiar with how the HTTP cache functions, but it's probably worth clearing the executable bit. There are likely easier ways to abuse pip to compromise a multi-user set up, but even if it's simply to mitigate future "this seems unsecure" bug reports, I'd clear the bit.

src/pip/_internal/network/cache.py Outdated Show resolved Hide resolved
@ichard26 ichard26 added this to the 25.0 milestone Nov 8, 2024
@ichard26
Copy link
Member

ichard26 commented Nov 8, 2024

Thanks for the PR by the way!

@JustinVanHeek
Copy link
Contributor Author

Both reasonable suggestions. Done 😄

Copy link
Member

@ichard26 ichard26 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

src/pip/_internal/network/cache.py Outdated Show resolved Hide resolved
src/pip/_internal/network/cache.py Outdated Show resolved Hide resolved
@notatallshaw notatallshaw dismissed their stale review December 8, 2024 23:41

Resolved in conversation

@ichard26 ichard26 self-requested a review December 9, 2024 03:48
@ichard26 ichard26 enabled auto-merge (squash) December 9, 2024 22:14
@ichard26 ichard26 changed the title Make network cache file permissions inherit from cache directory Inherit cache file read/write permissions from cache directory Dec 9, 2024
@ichard26 ichard26 changed the title Inherit cache file read/write permissions from cache directory Inherit network cache file read/write permissions from cache directory Dec 9, 2024
@ichard26 ichard26 disabled auto-merge December 9, 2024 22:18
@ichard26 ichard26 enabled auto-merge (squash) December 9, 2024 22:20
@ichard26 ichard26 changed the title Inherit network cache file read/write permissions from cache directory Inherit HTTP cache file read/write permissions from cache directory Dec 9, 2024
@ichard26 ichard26 merged commit 667acf4 into pypa:main Dec 9, 2024
32 checks passed
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

File permissions for cache shared between multiple users
4 participants