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

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

Closed
LaurentMazuel mannequin opened this issue Mar 28, 2016 · 9 comments
Assignees
Labels
3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement

Comments

@LaurentMazuel
Copy link
Mannequin

LaurentMazuel mannequin commented Mar 28, 2016

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
    Copy link
    Mannequin Author

    LaurentMazuel mannequin commented Mar 28, 2016

    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'

    @LaurentMazuel LaurentMazuel mannequin added stdlib Python modules in the Lib dir type-feature A feature request or enhancement labels Mar 28, 2016
    @serhiy-storchaka serhiy-storchaka self-assigned this Nov 2, 2018
    @mayankasthana
    Copy link
    Mannequin

    mayankasthana mannequin commented Nov 2, 2018

    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
    Copy link
    Member

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

    @eryksun
    Copy link
    Contributor

    eryksun commented Nov 3, 2018

    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 serhiy-storchaka added 3.7 (EOL) end of life 3.8 only security fixes labels Nov 4, 2018
    @serhiy-storchaka serhiy-storchaka changed the title tempfile.TemporaryDirectory() cleanup exception on Windows if readonly files created tempfile.TemporaryDirectory() cleanup exception if nonwriteable or non-searchable files or directories created Nov 4, 2018
    @serhiy-storchaka
    Copy link
    Member

    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
    Copy link
    Contributor

    eryksun commented Nov 4, 2018

    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
    Copy link
    Member

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

    @vidartf
    Copy link
    Mannequin

    vidartf mannequin commented Mar 12, 2020

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

    @akaihola
    Copy link

    akaihola commented Jan 7, 2023

    For anyone still needing to support Python 3.7 and having this issue, you can find a recipe for a work-around in:

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.7 (EOL) end of life 3.8 only security fixes stdlib Python modules in the Lib dir type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants