Bug #423
closed777 permissions
Added by Gabriel Lander over 14 years ago. Updated over 14 years ago.
100%
Description
Our sys admin is very uncomfortable with the default "0777" permissions set on all appion folders - is there a reason they can't be "2775"?
Updated by Eric Hou over 14 years ago
Why do you have permission issue at the beginning?
Updated by Gabriel Lander over 14 years ago
when appionlib/apParam.py creates a directory, it is set to 0777
Updated by Christopher Irving over 14 years ago
This bug surprised me because I knew that wasn't the default for us and I though Gabe must have made a mistake. However, when I looked at the source I found this horrible bit of code in the file apParam.py (source:/trunk/appion/appionlib/apParam.py@13489#L485)
def setUmask(msg=False): os.umask(0) os.umask(0) return if os.getgid() == 773: prev = os.umask(002) curr = os.umask(002) else: prev = os.umask(000) curr = os.umask(000) if msg is True: apDisplay.printMsg("Umask changed from "+str(prev)+" to "+str(curr))
So basically we're setting the umask to 002 if the user is in the Unix group 773, which is AMI here but could be anything somewhere else. That needs to be removed. Also, the umask should only be set to 000 if you want to ensure the file permission mode passed in a open() or other file/directory creation call isn't overridden by the users umask setting. However, you have to set that file permission mode when creating the file otherwise a file will have mode 666 and a directory will have mode 777 which is a very bad idea.
Updated by Amber Herold over 14 years ago
- Assignee changed from Amber Herold to Christopher Irving
Updated by Christopher Irving over 14 years ago
- Status changed from New to In Test
- % Done changed from 0 to 100
Applied in changeset r13702.
Updated by Gabriel Lander over 14 years ago
can we also change line 299 in apParam.py:
def createDirectory(path, mode=0777, warning=True):
to mode=0775?
Updated by Neil Voss over 14 years ago
- Status changed from In Test to In Code Review
Hey guys. I am sorry I added this code. I get so fed up with trying to help people process data and then I have to go the extra steps to give myself access. It was an error on my part.
Nonetheless, Christopher's fix does nothing! The umask function in python does not work as expected. You can set it all you want and it does nothing. This code has been present since 2007, but did not work so no effect was seen.
The code that is making the effect happen is here in appionScript.py (source:/trunk/appion/appionlib/appionScript.py@13705#L261):
### make directories writable sessiondir = os.path.abspath(os.path.join(self.params['rundir'], "../..")) cmd = "chmod 777 `find %s -type d`"%(sessiondir) subprocess.Popen(cmd, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
I have since removed this code in r13706 and the line that sets the umask we should just make sure everyone sets things to an appropriate umask in their .cshrc/.bashrc files.
Updated by Christopher Irving over 14 years ago
Actually python's umask function works just as it should. The only place the setUmask()function is called on line 37 appion/appionlib/basicScript.py and createDirectory() is called in several places. As long as they are used it will work. The umask set by calling setUmask() doesn't work in the above code because it's forking a new shell to run the command. So if the user sets the umask in his startup scripts, that's the one that will be in effect when the chmod is executed. Neil's bit of code is unnecessary if you use the createDirectory() to create any directories.
Updated by Amber Herold over 14 years ago
- Status changed from In Code Review to In Test
looks good. How to test?
Updated by Gabriel Lander over 14 years ago
- Assignee changed from Christopher Irving to Gabriel Lander
I can check out a testing version of myami so it doesn't mess up our current installation - would that work?