Confine submodule paths to their parent repository

Author: codex

git/objects/submodule/base.py

@@ -28,6 +28,7 @@
 from git.util import (
     IterableList,
     RemoteProgress,
+    _to_relative_path,
     join_path_native,
     rmtree,
     to_native_path_linux,
@@ -379,23 +380,14 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike:
         :raise ValueError:
             If path is not contained in the parent repository's working tree.
         """
-        path = to_native_path_linux(path)
+        if parent_repo.working_tree_dir:
+            path = _to_relative_path(parent_repo.working_tree_dir, path)
+        else:
+            path = to_native_path_linux(path)
         if path.endswith("/"):
             path = path[:-1]
-        # END handle trailing slash
-
-        if osp.isabs(path) and parent_repo.working_tree_dir:
-            working_tree_linux = to_native_path_linux(parent_repo.working_tree_dir)
-            if not path.startswith(working_tree_linux):
-                raise ValueError(
-                    "Submodule checkout path '%s' needs to be within the parents repository at '%s'"
-                    % (working_tree_linux, path)
-                )
-            path = path[len(working_tree_linux.rstrip("/")) + 1 :]
-            if not path:
-                raise ValueError("Absolute submodule path '%s' didn't yield a valid relative path" % path)
-            # END verify converted relative path makes sense
-        # END convert to a relative path
+        if not path or path == ".":
+            raise ValueError("Submodule checkout path must not be the repository root")
 
         return path
 

test/test_submodule.py

@@ -1256,6 +1256,46 @@ class Repo:
         msg = '_to_relative_path should be "submodule_path" but was "%s"' % relative_path
         assert relative_path == "submodule_path", msg
 
+    @with_rw_directory
+    def test_to_relative_path_is_confined_to_parent(self, rwdir):
+        class Repo:
+            working_tree_dir = osp.join(rwdir, "parent")
+
+        os.makedirs(osp.join(Repo.working_tree_dir, "nested"))
+
+        assert Submodule._to_relative_path(Repo(), osp.join(Repo.working_tree_dir, "nested")) == "nested"
+        assert Submodule._to_relative_path(Repo(), osp.join("nested", "..", "module")) == "module"
+        self.assertRaises(ValueError, Submodule._to_relative_path, Repo(), Repo.working_tree_dir)
+        self.assertRaises(ValueError, Submodule._to_relative_path, Repo(), osp.join("..", "outside"))
+        self.assertRaises(
+            ValueError,
+            Submodule._to_relative_path,
+            Repo(),
+            osp.join(Repo.working_tree_dir + "-other", "module"),
+        )
+
+    @skipUnless(sys.platform == "win32", "Specifically for Windows.")
+    @with_rw_directory
+    def test_to_relative_path_windows_path_kinds(self, rwdir):
+        class Repo:
+            working_tree_dir = osp.join(rwdir, "parent")
+
+        inside_path = osp.join(Repo.working_tree_dir, "nested", "module")
+        _drive, rooted_inside_path = osp.splitdrive(inside_path)
+
+        assert Submodule._to_relative_path(Repo(), rooted_inside_path) == "nested/module"
+        assert Submodule._to_relative_path(Repo(), rooted_inside_path.replace("\\", "/")) == "nested/module"
+        assert Submodule._to_relative_path(Repo(), PathLikeMock(inside_path)) == "nested/module"
+        self.assertRaises(
+            ValueError,
+            Submodule._to_relative_path,
+            Repo(),
+            f"{osp.splitdrive(Repo.working_tree_dir)[0]}relative",
+        )
+        self.assertRaises(ValueError, Submodule._to_relative_path, Repo(), R"Z:\outside")
+        self.assertRaises(ValueError, Submodule._to_relative_path, Repo(), R"\\server\share\outside")
+        self.assertRaises(ValueError, Submodule._to_relative_path, Repo(), R"\\?\C:\outside")
+
     @pytest.mark.xfail(
         reason="for some unknown reason the assertion fails, even though it in fact is working in more common setup",
         raises=AssertionError,