option to preserve the permissions

[untitaker]

option to preserve the permissions

[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]

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)

[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. ```python 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.