Skip to content

Commit

Permalink
Test that rmtree doesn't chmod outside the tree
Browse files Browse the repository at this point in the history
This adds a test in test_util that reveals the bug where
git.util.rmtree will change the permissions on files outside the
tree being removed, if the tree being removed contains a symlink to
a file outside the tree, and the symlink is in a (sub)directory
whose own permissions prevent the symlink itself from being
removed.

The new test failure shows how git.util.rmtree currently calls
os.chmod in a way that dereferences symlinks, including those that
point from inside the tree being deleted to outside it.

Another similar demonstration is temporarily included in the
perm.sh script. That script served as scratchwork for writing the
unit test, and it can be removed soon (while keeping the test).
  • Loading branch information
EliahKagan committed Nov 14, 2023
1 parent d5d897c commit 5335416
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 0 deletions.
17 changes: 17 additions & 0 deletions perm.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#!/bin/sh

set -e

mkdir dir1
touch dir1/file
chmod -w dir1/file
printf 'Permissions BEFORE rmtree call:\n'
ls -l dir1/file
printf '\n'

mkdir dir2
ln -s ../dir1/file dir2/symlink
chmod -w dir2
python -c 'from git.util import rmtree; rmtree("dir2")' || true
printf '\nPermissions AFTER rmtree call:\n'
ls -l dir1/file
29 changes: 29 additions & 0 deletions test/test_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,35 @@ def test_deletes_dir_with_readonly_files(self, tmp_path):

assert not td.exists()

@pytest.mark.skipif(
sys.platform == "cygwin",
reason="Cygwin can't set the permissions that make the test meaningful.",
)
def test_avoids_changing_permissions_outside_tree(self, tmp_path: pathlib.Path):
# Automatically works on Windows, but on Unix requires either special handling
# or refraining from attempting to fix PermissionError by making chmod calls.

dir1 = tmp_path / "dir1"
dir1.mkdir()
(dir1 / "file").write_bytes(b"")
(dir1 / "file").chmod(stat.S_IRUSR)
old_mode = (dir1 / "file").stat().st_mode

dir2 = tmp_path / "dir2"
dir2.mkdir()
(dir2 / "symlink").symlink_to(dir1 / "file")
dir2.chmod(stat.S_IRUSR | stat.S_IXUSR)

try:
rmtree(dir2)
except PermissionError:
pass # On Unix, dir2 is not writable, so dir2/symlink may not be deleted.
except SkipTest as ex:
self.fail(f"rmtree unexpectedly attempts skip: {ex!r}")

new_mode = (dir1 / "file").stat().st_mode
assert old_mode == new_mode, f"Should stay {old_mode:#o}, became {new_mode:#o}."

@pytest.mark.skipif(
sys.platform == "cygwin",
reason="Cygwin can't set the permissions that make the test meaningful.",
Expand Down

0 comments on commit 5335416

Please sign in to comment.