Merge pull request #2278 from Keerthana-64/gitmodules-symlink-entries

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

Author: Byron

git/index/base.py

@@ -728,6 +728,8 @@ def _preprocess_add_items(
             else:
                 raise TypeError("Invalid Type: %r" % item)
         # END for each item
+        # Source paths must be safe to read, but their recorded names may be rewritten.
+        # Apply mode-dependent restrictions to the final entries in add().
         for entry in entries:
             _validate_repo_path(entry.path)
         return paths, entries
@@ -1026,7 +1028,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

@@ -29,63 +29,62 @@
 if sys.platform == "win32":
     __all__.append("to_native_path_windows")
 
-from abc import abstractmethod
 import contextlib
-from functools import wraps
 import getpass
 import logging
 import ntpath
 import os
 import os.path as osp
-from pathlib import Path
 import platform
 import re
 import shutil
 import stat
 import subprocess
 import time
-from urllib.parse import urlsplit, urlunsplit
 import warnings
-
-# NOTE: Unused imports can be improved now that CI testing has fully resumed. Some of
-# these be used indirectly through other GitPython modules, which avoids having to write
-# gitdb all the time in their imports. They are not in __all__, at least currently,
-# because they could be removed or changed at any time, and so should not be considered
-# conceptually public to code outside GitPython. Linters of course do not like it.
-from gitdb.util import (
-    LazyMixin,  # noqa: F401
-    LockedFD,  # noqa: F401
-    bin_to_hex,  # noqa: F401
-    file_contents_ro,  # noqa: F401
-    file_contents_ro_filepath,  # noqa: F401
-    hex_to_bin,  # noqa: F401
-    make_sha,
-    to_bin_sha,  # noqa: F401
-    to_hex_sha,  # noqa: F401
-)
+from abc import abstractmethod
+from functools import wraps
+from pathlib import Path
 
 # typing ---------------------------------------------------------
-
 from typing import (
+    IO,
+    TYPE_CHECKING,
     Any,
     AnyStr,
     Callable,
     Dict,
     Generator,
-    IO,
     Iterator,
     List,
     Optional,
     Pattern,
     Sequence,
     Tuple,
-    TYPE_CHECKING,
     Type,
     TypeVar,
     Union,
     cast,
     overload,
 )
+from urllib.parse import urlsplit, urlunsplit
+
+# NOTE: Unused imports can be improved now that CI testing has fully resumed. Some of
+# these be used indirectly through other GitPython modules, which avoids having to write
+# gitdb all the time in their imports. They are not in __all__, at least currently,
+# because they could be removed or changed at any time, and so should not be considered
+# conceptually public to code outside GitPython. Linters of course do not like it.
+from gitdb.util import (
+    LazyMixin,  # noqa: F401
+    LockedFD,  # noqa: F401
+    bin_to_hex,  # noqa: F401
+    file_contents_ro,  # noqa: F401
+    file_contents_ro_filepath,  # noqa: F401
+    hex_to_bin,  # noqa: F401
+    make_sha,
+    to_bin_sha,  # noqa: F401
+    to_hex_sha,  # noqa: F401
+)
 
 if TYPE_CHECKING:
     from git.cmd import Git
@@ -94,9 +93,9 @@
     from git.repo.base import Repo
 
 from git.types import (
+    HSH_TD,
     Files_TD,
     Has_id_attribute,
-    HSH_TD,
     Literal,
     PathLike,
     Protocol,
@@ -385,12 +384,27 @@ 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"
 )
 
+# Match Git's is_ntfs_dotgitmodules in path.c on a lowercased, trimmed name.
+# Besides gitmod~1..4, fallback aliases have exactly eight ASCII characters:
+# a shrinking prefix of "gi7eba", "~", and digits with no leading zero.
+# Explicit digit counts avoid accepting shorter/longer names or Unicode digits.
+_NTFS_DOTGITMODULES_SHORT_NAME = re.compile(
+    r"(?:gitmod~[1-4]|gi7eba~[1-9]|gi7eb~[1-9][0-9]|gi7e~[1-9][0-9]{2}|"
+    r"gi7~[1-9][0-9]{3}|gi~[1-9][0-9]{4}|g~[1-9][0-9]{5}|~[1-9][0-9]{6})"
+)
+
 
-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 +420,13 @@ 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)
+            if mode is not None and stat.S_ISLNK(mode):
+                if (
+                    ntfs_name == ".gitmodules"
+                    or hfs_name == ".gitmodules"
+                    or _NTFS_DOTGITMODULES_SHORT_NAME.fullmatch(ntfs_name) is not None
+                ):
+                    raise ValueError("Symbolic link aliases the submodule configuration: %r" % name)
 
 
 def assure_directory_exists(path: PathLike, is_file: bool = False) -> bool: