tempfile.TemporaryDirectory() cleanup exception if nonwriteable or non-searchable files or directories created

[LaurentMazuel]

BPO	26660
Nosy	@birkenfeld, @serhiy-storchaka, @eryksun, @vidartf, @altendky, @mayankasthana, @rtzoeller
PRs	bpo-26660, bpo-35144: Fix permission errors in TemporaryDirectory cleanup. #10320

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2020-03-17.17:47:52.590>
created_at = <Date 2016-03-28.22:22:28.720>
labels = ['3.7', '3.8', 'type-feature', 'library']
title = 'tempfile.TemporaryDirectory() cleanup exception if nonwriteable or non-searchable files or directories created'
updated_at = <Date 2020-03-17.17:47:52.589>
user = 'https://bugs.python.org/LaurentMazuel'

bugs.python.org fields:

activity = <Date 2020-03-17.17:47:52.589>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2020-03-17.17:47:52.590>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2016-03-28.22:22:28.720>
creator = 'Laurent.Mazuel'
dependencies = []
files = []
hgrepos = []
issue_num = 26660
keywords = ['patch']
message_count = 8.0
messages = ['262584', '329153', '329186', '329206', '329232', '329236', '344037', '364019']
nosy_count = 8.0
nosy_names = ['georg.brandl', 'Laurent.Mazuel', 'serhiy.storchaka', 'eryksun', 'vidartf', 'altendky', 'masthana', 'rtzoeller']
pr_nums = ['10320']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26660'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8']

[LaurentMazuel]

Using tempfile.TemporaryDirectory() in Windows, creating read-only files in this temp directory leads to PermissionError during the cleanup().
This is a direct cause of this one:
https://bugs.python.org/issue19643

And the workaround which was proposed in the bpo-19643 and added to the doc here:
https://docs.python.org/3/library/shutil.html?highlight=shutil#rmtree-example

is not used in the TemporaryDirectory implementation.

I don't know if the right solution is to modify the implementation to systematically delete read-only files using the cited workaround, or to add a 'remove_readonly' flag or to update the documentation to clearly says that cleanup will raise a PermissionError if the user creates a read-only file. At least documentation please :)

In my specific usecase I "git clone" in the temp directory, and the .git folder contains read-only files.

Full stacktrace:
Traceback (most recent call last):
  File "C:\mycode.py", line 149, in build_libraries
    update(generated_path, dest_folder)
  File "C:\Program Files\Python35\lib\tempfile.py", line 807, in __exit__
    self.cleanup()
  File "C:\Program Files\Python35\lib\tempfile.py", line 811, in cleanup
    _shutil.rmtree(self.name)
  File "C:\Program Files\Python35\lib\shutil.py", line 488, in rmtree
    return _rmtree_unsafe(path, onerror)
  File "C:\Program Files\Python35\lib\shutil.py", line 378, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Program Files\Python35\lib\shutil.py", line 378, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Program Files\Python35\lib\shutil.py", line 378, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Program Files\Python35\lib\shutil.py", line 378, in _rmtree_unsafe
    _rmtree_unsafe(fullname, onerror)
  File "C:\Program Files\Python35\lib\shutil.py", line 383, in _rmtree_unsafe
    onerror(os.unlink, fullname, sys.exc_info())
  File "C:\Program Files\Python35\lib\shutil.py", line 381, in _rmtree_unsafe
    os.unlink(fullname)
PermissionError: [WinError 5] Access is denied: 'C:\\Users\\me\\AppData\\Local\\Temp\\tmpk62cp34t\\readonly.file'

[mayankasthana]

I think it is OK to delete read-only files since they are in a TemporaryDirectory and should be expected to be temporary. A note in the documentation about this behaviour should be sufficient.

[serhiy-storchaka]

I am working on this. Left to test on Windows and analyze possible security issues.

[eryksun]

Serhiy, do you also plan to work around immutable files in POSIX? What about permissions on directories that prevent deleting files?

In BSD and macOS, we have os.chflags for modifying file flags. Modifying the immutable flag requires superuser access. In Linux, modifying the immutable attribute requires the CAP_LINUX_IMMUTABLE capability, and file attributes are accessed with an ioctl call. For example:

    import os
    import fcntl
    import array

    EXT2_IOC_GETFLAGS = 0x80086601
    EXT2_IOC_SETFLAGS = 0x40086602
    EXT2_IMMUTABLE_FL = 0x00000010 

    def make_mutable(filename):
        flags = array.array('l', [0])
        fd = os.open(filename, os.O_RDONLY)
        try:
            fcntl.ioctl(fd, EXT2_IOC_GETFLAGS, flags, True)
            flags[0] &= ~EXT2_IMMUTABLE_FL
            fcntl.ioctl(fd, EXT2_IOC_SETFLAGS, flags)
        finally:
            os.close(fd)

I assume for Windows this will use os.chmod. I need to rant a bit to see whether anyone else is bothered by this. Microsoft's chmod function modifies the readonly attribute as if it's a write/delete permission. I wish Python hadn't adopted this behavior, since it clashes with how chmod works on other platforms, none of which conflates native file attributes and permissions. (We can be granted permission to modify or delete an immutable file, but this doesn't enable us to modify the file until it's made mutable.) It would be nice to have os.get_file_attributes and os.set_file_attributes on Windows instead of this confused use of chmod.

[serhiy-storchaka]

PR 10320 solves problems with file mode and flags which can be set by os.chmod() and os.chflags() by unprivileged user. Was tested on Linux, FreeBSD and Windows.

It doesn't solve problem when files was made immutable by chattr on Linux.

[eryksun]

file mode and flags which can be set by os.chmod() and os.chflags()
by unprivileged user.

I remembered incorrectly about chflags. It's not a single immutable flag, but several (depending on the OS). The owner or superuser can modify UF_IMMUTABLE, UF_READONLY, and UF_NOUNLINK. The SF_* flags can only be unset by the superuser in single-user mode.

It doesn't solve problem when files was made immutable by chattr on
Linux.

We may not have CAP_LINUX_IMMUTABLE, but we can at least try to clear the immutable flag, no?

[serhiy-storchaka]

New changeset e9b51c0 by Serhiy Storchaka in branch 'master':
bpo-26660, bpo-35144: Fix permission errors in TemporaryDirectory cleanup. (GH-10320)
e9b51c0

[vidartf]

Seems as if this was resolved by the linked PR in 3.8, or am I missing something?