option to preserve the permissions

[pabs3]

When I overwrite a file, the permissions of the file are changed:

$ touch foo

$ chmod 0123 foo

$ stat foo | sed -n 's/^\(Access.*\)Uid.*$/\1/p'
Access: (0123/---x-w--wx)  

$ python3
Python 3.7.3rc1 (default, Mar 13 2019, 11:01:15) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> from atomicwrites import atomic_write
>>> with atomic_write('foo', overwrite=True) as f:
...  f.write('foo')
... 
3
>>> 

$ stat foo | sed -n 's/^\(Access.*\)Uid.*$/\1/p'
Access: (0600/-rw-------)  

The normal non-atomic method of overwriting a file does not change the mode:

$ touch foo

$ chmod 0777 foo

$ stat foo | sed -n 's/^\(Access.*\)Uid.*$/\1/p'
Access: (0777/-rwxrwxrwx)  

$ python3
Python 3.7.3rc1 (default, Mar 13 2019, 11:01:15) 
[GCC 8.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> with open('foo', 'w') as f:
...  f.write('foo')
... 
3
>>> 

$ stat foo | sed -n 's/^\(Access.*\)Uid.*$/\1/p'
Access: (0777/-rwxrwxrwx)  

It would be nice to have an option to preserve the mode of the original file.

[pabs3]

With the proposed option turned on, when creating new files I think it would also be good to obey the system's umask instead of creating files with restricted permissions.

Until there is an option for this, I'm using this code as a workaround:

class AtomicWriterBetterPermissions(AtomicWriter):
        def commit(self, f):
                try:
                        mode = stat(self._path).st_mode
                except FileNotFoundError:
                        # Creating a new file, emulate what os.open() does
                        umask = os.umask(0)
                        umask(umask)
                        mode = 0o777 & ~umask
                os.chmod(f.name, mode)
                super().commit(f)

[anarcat]

                os.chmod(f.name, mode)

isn't there a possibility of a race here too? it seems to me the chmod should happen on a file descriptor, not a path.

[pabs3]

Good point. I think I should also move the permission change time to before the temporary file is written to, since that could take a while and the original file could disappear or change modes in the meantime.

class AtomicWriterPerms(AtomicWriter):
	def get_fileobject(self, **kwargs):
		f = super().get_fileobject(**kwargs)
		try:
			mode = os.stat(self._path).st_mode
		except FileNotFoundError:
			# Creating a new file, emulate what os.open() does
			umask = os.umask(0)
			os.umask(umask)
			mode = 0o777 & ~umask
		fd = f.fileno()
		os.fchmod(fd, mode)
		return f

-- bye, pabs

https://wiki.debian.org/PaulWise

[untitaker]

If you figure out a way to do this without any races lmk. I personally have no usecase for this and would think that people who care about this just call chmod with a fixed mask after writing the file to force it to a hardcoded value.

[pabs3]

This is my current workaround. There will always be a race since the original file could change permissions between when the original file permissions are read and when the new file gets the permissions changed to the original permissions or it could even change permissions while the new file is being written to.

from os import stat, umask, fchmod
from atomicwrites import AtomicWriter, atomic_write

class AtomicWriterPerms(AtomicWriter):
	def get_fileobject(self, **kwargs):
		f = super().get_fileobject(**kwargs)
		try:
			mode = stat(self._path).st_mode
		except FileNotFoundError:
			# Creating a new file, emulate what os.open() does
			mask = umask(0)
			umask(mask)
			mode = 0o777 & ~mask
		fd = f.fileno()
		fchmod(fd, mode)
		return f

-- bye, pabs

https://bonedaddy.net/pabs3/

[scottj97]

+1 on this. Changing a file's permissions when overwriting is not expected behavior.

[andersk]

                        umask = os.umask(0)

Since the umask is per-process, not per-thread, changing it is a potential security problem in the presence of multiple threads. (See https://bugs.python.org/issue21082.)

That’s also why it’s hard for callers to “just call chmod with a fixed mask after writing the file”—it’s difficult to compute the correct mask in a threadsafe way.

My approach for the corresponding bug in Click avoids this problem by replacing tempfile with direct calls to os.open, which naturally respects the umask: pallets/click#1400.

[untitaker]

it’s difficult to compute the correct mask in a threadsafe way.

My recommendation was to set it to a value that is hardcoded in your software. I recognize that this is not always possible but it's certainly the least likely to cause races.