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

Extract source tarballs using permissions from umask #20481

Closed
embray opened this issue Apr 21, 2016 · 13 comments
Closed

Extract source tarballs using permissions from umask #20481

embray opened this issue Apr 21, 2016 · 13 comments

Comments

@embray
Copy link
Contributor

embray commented Apr 21, 2016

This is the lowest-hanging fruit in addressing the issues I raised in this comment.

It changes sage-uncompress-spkg to apply the umask to all files and directories extracted from tarballs.

This doesn't apply to zipfiles since they do not contain permission information, though in principle we could modify zipfiles to use the same policy. I don't think it matters much though.

Component: build

Author: Erik Bray

Branch/Commit: 4fe154a

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/20481

@embray embray added this to the sage-7.2 milestone Apr 21, 2016
@jdemeyer
Copy link
Contributor

Reviewer: Jeroen Demeyer

@jdemeyer
Copy link
Contributor

comment:2

Instead of ignoring permissions for dirs, would it be possible to simply apply the user's umask on everything which gets extracted (files and directories). This would mean changing tarinfo.mode to tarinfo.mode & CURRENT_UMASK.

@embray
Copy link
Contributor Author

embray commented Apr 21, 2016

comment:3

That was my first thought actually. I'd be fine with that too--this was just infinitesimally simpler :)

@embray
Copy link
Contributor Author

embray commented Apr 21, 2016

comment:4

Perhaps one reason not to prefer that is that if the umask is set to something useless then then applying it would be no less "safe". As it is this is only "unsafe" in the context of moving the extracted directory to someplace like /tmp. But it seems more reliable not to rely on the user's umask.

@jdemeyer
Copy link
Contributor

comment:5

Replying to @embray:

Perhaps one reason not to prefer that is that if the umask is set to something useless then then applying it would be no less "safe".

I would say that the umask defines what is safe. It is the thing which determines which permissions the user wants.

@novoselt
Copy link
Member

comment:6

0700 would break the cell server which uses a separate worker account and in general would not work for multiuser setup. Or am I missing the point? It also seems to me that using umask is just the right way for defaults in any case. Those who are not happy with umask can either adjust it or set some restrictive permission on a containing directory.

@jdemeyer
Copy link
Contributor

comment:7

I agree that building with too restrictive permissions could give problems. Some installation scripts probably just copy the directory over without changing the permissions. So 0700 is certainly too restrictive for that to work.

@embray
Copy link
Contributor Author

embray commented Apr 22, 2016

comment:8

Okay then, umask it is. I'll update this next week.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2016

Changed commit from d2d0d74 to 4fe154a

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Apr 27, 2016

Branch pushed to git repo; I updated commit sha1. New commits:

4fe154aModified the custom TarFile to simply apply the user's umask to all files and directories, regardless what it may be--consistent with the default behavior of 'tar'.

@embray
Copy link
Contributor Author

embray commented Apr 27, 2016

comment:10

As discussed, now the customized TarFile subclass merely applies the user's umask to all extracted files.

@jdemeyer

This comment has been minimized.

@jdemeyer jdemeyer modified the milestones: sage-7.2, sage-7.3 May 13, 2016
@jdemeyer jdemeyer changed the title Extract source tarballs using more restrictive permissions on directories Extract source tarballs using permissions from umask May 13, 2016
@vbraun
Copy link
Member

vbraun commented May 22, 2016

Changed branch from u/embray/uncompress-permissions to 4fe154a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants