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

bpo-42782: Fail fast for permission errors in shutil.move() (GH-24001) #413

Merged
merged 1 commit into from
Mar 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Lib/shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -813,6 +813,12 @@ def move(src, dst, copy_function=copy2):
if _destinsrc(src, dst):
raise Error("Cannot move a directory '%s' into itself"
" '%s'." % (src, dst))
if (_is_immutable(src)
or (not os.access(src, os.W_OK) and os.listdir(src)
and sys.platform == 'darwin')):
raise PermissionError("Cannot move the non-empty directory "
"'%s': Lacking write permission to '%s'."
% (src, src))
copytree(src, real_dst, copy_function=copy_function,
symlinks=True)
rmtree(src)
Expand All @@ -830,6 +836,11 @@ def _destinsrc(src, dst):
dst += os.path.sep
return dst.startswith(src)

def _is_immutable(src):
st = _stat(src)
immutable_states = [stat.UF_IMMUTABLE, stat.SF_IMMUTABLE]
return hasattr(st, 'st_flags') and st.st_flags in immutable_states

def _get_gid(name):
"""Returns a gid, given a group name."""
if getgrnam is None or name is None:
Expand Down
37 changes: 37 additions & 0 deletions Lib/test/test_shutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
from test.support.os_helper import TESTFN, FakePath

TESTFN2 = TESTFN + "2"
TESTFN_SRC = TESTFN + "_SRC"
TESTFN_DST = TESTFN + "_DST"
MACOS = sys.platform.startswith("darwin")
AIX = sys.platform[:3] == 'aix'
try:
Expand Down Expand Up @@ -2085,6 +2087,41 @@ def test_move_dir_caseinsensitive(self):
os.rmdir(dst_dir)


@unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0
and hasattr(os, 'lchflags')
and hasattr(stat, 'SF_IMMUTABLE')
and hasattr(stat, 'UF_OPAQUE'),
'root privileges required')
def test_move_dir_permission_denied(self):
# bpo-42782: shutil.move should not create destination directories
# if the source directory cannot be removed.
try:
os.mkdir(TESTFN_SRC)
os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)

# Testing on an empty immutable directory
# TESTFN_DST should not exist if shutil.move failed
self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
self.assertFalse(TESTFN_DST in os.listdir())

# Create a file and keep the directory immutable
os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
os_helper.create_empty_file(os.path.join(TESTFN_SRC, 'child'))
os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)

# Testing on a non-empty immutable directory
# TESTFN_DST should not exist if shutil.move failed
self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
self.assertFalse(TESTFN_DST in os.listdir())
finally:
if os.path.exists(TESTFN_SRC):
os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
os_helper.rmtree(TESTFN_SRC)
if os.path.exists(TESTFN_DST):
os.lchflags(TESTFN_DST, stat.UF_OPAQUE)
os_helper.rmtree(TESTFN_DST)


class TestCopyFile(unittest.TestCase):

class Faux(object):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fail fast in :func:`shutil.move()` to avoid creating destination directories on
failure.