fix(util): reject symbolic links that alias .gitmodules

`_validate_repo_path` mirrors Git's `verify_path` for tree and index entry
paths, but it only ever saw the path. Git's check there is mode dependent: an
entry that makes `.gitmodules` a symbolic link is refused, because the
submodule configuration would then be read through the link, from outside the
repository. That is why `git update-index --add --cacheinfo
120000,<sha>,.gitmodules` fails with `Invalid path`, `git read-tree` fails with
`invalid path`, and `git fsck --strict` reports `gitmodulesSymlink`.

GitPython accepted such an entry in both directions. `IndexFile.add` with a
`BaseIndexEntry` of mode `120000` and path `.gitmodules` was stored,
`write_tree` serialized the tree, and `IndexFile.commit` wrote a commit Git
refuses to read back and a server with `transfer.fsckObjects` set rejects.
Coming the other way, `read_cache` and `tree_entries_from_data` accepted the
same entry out of an untrusted repository's index or tree.

`_validate_repo_path` now takes the entry mode and, for a symbolic link,
rejects every spelling Git recognizes: `.gitmodules` with trailing spaces or
periods, the HFS form with ignorable code points removed, and the NTFS short
names `gitmod~1` through `gitmod~4` and `gi7eba~1` through `gi7eba~9`. The
mode is passed at the boundaries that have one: `write_cache`, `read_cache`,
`write_tree_from_cache`, `_tree_entry_to_baseindexentry`,
`IndexFile._preprocess_add_items`, `IndexFile.add`, `tree_to_stream`,
`tree_entries_from_data` and `TreeModifier.add`. Paths reached without a mode,
such as the directories walked by `IndexFile._iter_expand_paths`, keep their
previous behavior, and a regular file named `.gitmodules` stays valid.

Checked against `git update-index --add --cacheinfo` on git 2.52.0 for modes
`100644`, `120000`, `160000` and `40000` over the alias corpus: no path is
left that Git rejects and GitPython accepts. Like the existing `.git` rule the
name is tested per component, so a link below a directory spelled like one of
those aliases is refused as well, which Git happens to allow. Adds regression
tests in `test/test_index.py` and `test/test_tree.py`; `mypy`,
`basedpyright --warnings` and `ruff` are clean.

Author: Keerthana-64

git/index/base.py

@@ -729,7 +729,7 @@ def _preprocess_add_items(
                 raise TypeError("Invalid Type: %r" % item)
         # END for each item
         for entry in entries:
-            _validate_repo_path(entry.path)
+            _validate_repo_path(entry.path, entry.mode)
         return paths, entries
 
     def _store_path(self, filepath: PathLike, fprogress: Callable) -> BaseIndexEntry:
@@ -1026,7 +1026,7 @@ def handle_null_entries(self: "IndexFile") -> None:
         # FINALIZE
         # Add the new entries to this instance.
         for entry in entries_added:
-            _validate_repo_path(entry.path)
+            _validate_repo_path(entry.path, entry.mode)
         for entry in entries_added:
             self.entries[(entry.path, 0)] = IndexEntry.from_base(entry)
 

git/index/fun.py

@@ -279,7 +279,7 @@ def write_cache(
 
     # Body
     for entry in entries:
-        _validate_repo_path(entry.path)
+        _validate_repo_path(entry.path, entry.mode)
         beginoffset = tell()
         write(entry.ctime_bytes)  # ctime
         write(entry.mtime_bytes)  # mtime
@@ -394,7 +394,7 @@ def read_cache(
         if terminator != b"\0":
             raise ValueError("Unterminated index entry path")
         path = path_bytes.decode(defenc)
-        _validate_repo_path(path)
+        _validate_repo_path(path, mode)
 
         real_size = (tell() - beginoffset + 7) & ~7
         padding_size = beginoffset + real_size - tell()
@@ -462,7 +462,7 @@ def write_tree_from_cache(
     """
     if si == 0:
         for entry in entries[sl]:
-            _validate_repo_path(entry.path)
+            _validate_repo_path(entry.path, entry.mode)
     tree_items: List["TreeCacheTup"] = []
 
     ci = sl.start
@@ -510,7 +510,7 @@ def write_tree_from_cache(
 
 
 def _tree_entry_to_baseindexentry(tree_entry: "TreeCacheTup", stage: int) -> BaseIndexEntry:
-    _validate_repo_path(tree_entry[2])
+    _validate_repo_path(tree_entry[2], tree_entry[1])
     return BaseIndexEntry((tree_entry[1], tree_entry[0], stage << CE_STAGESHIFT, tree_entry[2]))
 
 

git/objects/fun.py

@@ -39,11 +39,11 @@
 # ---------------------------------------------------
 
 
-def _validate_tree_entry_name(name: str) -> None:
+def _validate_tree_entry_name(name: str, mode: Union[int, None] = None) -> None:
     if "/" in name:
         raise ValueError("Tree entry names must not contain '/' characters")
     # A tree name is a component, not a rooted path; a colon cannot select a drive.
-    _validate_repo_path("tree/" + name)
+    _validate_repo_path("tree/" + name, mode)
 
 
 def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer"], Union[int, None]]) -> None:
@@ -82,7 +82,7 @@ def tree_to_stream(entries: Sequence[EntryTup], write: Callable[["ReadableBuffer
             name_bytes = name.encode(defenc)
         else:
             name_bytes = name  # type: ignore[unreachable]  # check runtime types - is always str?
-        _validate_tree_entry_name(safe_decode(name_bytes))
+        _validate_tree_entry_name(safe_decode(name_bytes), mode)
         write(b"".join((mode_str, b" ", name_bytes, b"\0", binsha)))
     # END for each item
 
@@ -112,7 +112,7 @@ def tree_entries_from_data(data: bytes) -> List[EntryTup]:
         if name_end < 0 or name_end + 21 > len(data):
             raise ValueError("Truncated tree entry")
         name = safe_decode(bytes(data[mode_end + 1 : name_end]))
-        _validate_tree_entry_name(name)
+        _validate_tree_entry_name(name, mode)
         offset = name_end + 21
         out.append((bytes(data[name_end + 1 : offset]), mode, name))
     return out

git/objects/tree.py

@@ -110,7 +110,7 @@ def add(self, sha: bytes, mode: int, name: str, force: bool = False) -> "TreeMod
         :return:
             self
         """
-        _validate_tree_entry_name(name)
+        _validate_tree_entry_name(name, mode)
         if (mode >> 12) not in Tree._map_id_to_type:
             raise ValueError("Invalid object type according to mode %o" % mode)
 

git/util.py

@@ -385,12 +385,25 @@ def _to_relative_path(root: PathLike, path: PathLike) -> str:
     "", "", "\u200c\u200d\u200e\u200f\u202a\u202b\u202c\u202d\u202e\u206a\u206b\u206c\u206d\u206e\u206f\ufeff"
 )
 
+# Windows derives an 8.3 short name for ".gitmodules" from its first six
+# characters followed by "~1" through "~4", and from a hashed stem once those
+# are taken. Git recognizes both spellings (is_ntfs_dotgitmodules in path.c).
+_NTFS_DOTGITMODULES_SHORT_NAMES = frozenset(
+    ["gitmod~%d" % index for index in range(1, 5)] + ["gi7eba~%d" % index for index in range(1, 10)]
+)
+
 
-def _validate_repo_path(path: PathLike) -> None:
+def _validate_repo_path(path: PathLike, mode: Union[int, None] = None) -> None:
     """Reject unsafe tree/index paths without normalizing away their components.
 
     Protect Git metadata aliases on NTFS and HFS even when writing on another
     platform. Other POSIX filename characters, including newlines, remain valid.
+
+    :param mode:
+        Mode of the index or tree entry the path belongs to, where one is known.
+        Git refuses a symbolic link that aliases ``.gitmodules``, since the
+        submodule configuration would then be read through the link, so that
+        name is only rejected once the mode says the entry is a link.
     """
     name = os.fspath(path)
     if not name or "\0" in name or ntpath.splitdrive(name)[0] or name.startswith("/"):
@@ -406,6 +419,11 @@ def _validate_repo_path(path: PathLike) -> None:
             hfs_name = part.translate(_HFS_IGNORABLES).lower()
             if ntfs_name in (".git", "git~1") or hfs_name == ".git":
                 raise ValueError("Repository path aliases Git metadata: %r" % name)
+            aliases_gitmodules = (
+                ntfs_name == ".gitmodules" or hfs_name == ".gitmodules" or ntfs_name in _NTFS_DOTGITMODULES_SHORT_NAMES
+            )
+            if aliases_gitmodules and mode is not None and stat.S_ISLNK(mode):
+                raise ValueError("Symbolic link aliases the submodule configuration: %r" % name)
 
 
 def assure_directory_exists(path: PathLike, is_file: bool = False) -> bool:

test/test_index.py

@@ -189,10 +189,10 @@ def _make_hook(git_dir, name, content, make_exec=True):
     return hp
 
 
-def _raw_index(path):
+def _raw_index(path, mode=0o100644):
     """Build an index without using the writer under test."""
     name = path.encode("utf-8")
-    entry = struct.pack(">10L20sH", 0, 0, 0, 0, 0, 0, 0o100644, 0, 0, 0, b"a" * 20, min(len(name), 0xFFF)) + name
+    entry = struct.pack(">10L20sH", 0, 0, 0, 0, 0, 0, mode, 0, 0, 0, b"a" * 20, min(len(name), 0xFFF)) + name
     entry += b"\0" * (8 - len(entry) % 8)
     data = b"DIRC" + struct.pack(">LL", 2, 1) + entry
     return data + sha1(data).digest()
@@ -340,6 +340,33 @@ def test_index_reader_and_writer_reject_unsafe_paths(self, path):
         with pytest.raises(ValueError):
             write_cache([entry], BytesIO())
 
+    @ddt.data(
+        ".gitmodules",
+        ".GITMODULES",
+        ".gitmodules.",
+        ".gitmodules ",
+        ".gi\u200ctmodules",
+        "gitmod~1",
+        "gitmod~4",
+        "gi7eba~1",
+        "gi7eba~9",
+        "sub/.gitmodules",
+    )
+    def test_index_reader_and_writer_reject_gitmodules_symlinks(self, path):
+        """An entry that turns .gitmodules into a symbolic link is rejected, while the
+        same name stays valid for a regular file. The spellings are those
+        `git update-index --add --cacheinfo 120000,<sha>,<path>` refuses on git 2.52.0."""
+        with pytest.raises(ValueError):
+            read_cache(BytesIO(_raw_index(path, mode=0o120000)))
+        with pytest.raises(ValueError):
+            write_cache([IndexEntry((0o120000, b"a" * 20, 0, path))], BytesIO())
+
+        assert next(iter(read_cache(BytesIO(_raw_index(path)))[1])) == (path, 0)
+        stream = BytesIO()
+        write_cache([IndexEntry((0o100644, b"a" * 20, 0, path))], stream)
+        stream.seek(0)
+        assert next(iter(read_cache(stream)[1])) == (path, 0)
+
     def test_valid_unusual_index_names_round_trip(self):
         names = ["a b", "a\nb", "a\tb", "name:value", "dir/.gitignore", "café"]
         if os.name != "nt":

test/test_tree.py

@@ -138,6 +138,28 @@ def test_tree_names_are_checked_at_construction_and_serialization(self, name):
         with pytest.raises(ValueError):
             tree_to_stream([(b"a" * 20, 0o100644, name)], BytesIO().write)
 
+    @ddt.data(".gitmodules", ".GITMODULES", ".gitmodules ", ".gi\u200ctmodules", "gitmod~1", "gi7eba~1")
+    def test_gitmodules_symlink_entries_are_rejected(self, name):
+        """A symbolic link named like the submodule configuration would make Git read
+        it from outside the repository, so such an entry is refused in both
+        directions. A regular file with the same name is the normal case."""
+        symlink_mode = 0o120000
+        cache = []
+        with pytest.raises(ValueError):
+            TreeModifier(cache).add(b"a" * 20, symlink_mode, name)
+        assert not cache
+        with pytest.raises(ValueError):
+            tree_to_stream([(b"a" * 20, symlink_mode, name)], BytesIO().write)
+        raw = b"120000 " + name.encode() + b"\0" + b"a" * 20
+        with pytest.raises(ValueError):
+            tree_entries_from_data(raw)
+
+        TreeModifier(cache).add(b"a" * 20, 0o100644, name)
+        assert cache == [(b"a" * 20, 0o100644, name)]
+        data = BytesIO()
+        tree_to_stream(cache, data.write)
+        assert tree_entries_from_data(data.getvalue()) == cache
+
     def test_traverse(self):
         root = self.rorepo.tree("0.1.6")
         num_recursive = 0