pip.utils.filesystem.check_path_owner should treat group membership properly

[sashkab]

I have two users who share same .pip/cache directory: a and b. Both users are members of the group pip. There is third user, pip who is also member of group pip and that user handles all download and caching for users a and b. .pip/cache directory configured to have group writeable permission and sticky bit, to make sure all newly created files and folders are owned by group pip.

Function check_path_owner only checks if owner is same user, but it is somewhat wrong. You should check group membership as well.

Another option, is to create new command-line option, which will allow to ignore this check.

Either solution will work.

[dstufft]

I think checking group is good too. Better yet would be generalizing it so that it actually checks permissions. Possibly even supporting a read only cache mode.

On Feb 2, 2015, at 1:19 PM, Aleks Bunin notifications@github.com wrote:

I have two users who share same .pip/cache directory: a and b. Both users are members of the group pip. There is third user, pip who is also member of group pip and that user handles all download and caching for users a and b. .pip/cache directory configured to have group writeable permission and sticky bit, to make sure all newly created files and folders are owned by group pip.

Function check_path_owner only checks if owner is same user, but it is somewhat wrong. You should check group membership as well.

Another option, is to create new command-line option, which will allow to ignore this check.

Either solution will work.

—
Reply to this email directly or view it on GitHub.

[dstufft]

Cross posting from #2396:

I know I suggested this, but I realized why I didn't do this to begin with. That directory is writable to the root user when using sudo without the sudo -H flag. So this can't actually work like this without rendering the warning I originally added useless.

So I guess the underlying question here is what check is reasonable (or if no check is reasonable we should just remove the warning and the check all together).