From 62266dabde03645a2389972f26338a324463d1f3 Mon Sep 17 00:00:00 2001 From: Sasha Mitchell Date: Thu, 1 Oct 2026 17:15:39 +0700 Subject: [PATCH 01/24] objects: decode a byte mode before reading its digits mode_str_to_int(b"100644") returned 1830740. A byte is an int, so the digit 4 was read as 52. A text mode was already 0o100644. --- git/objects/util.py | 4 +++- test/test_util.py | 8 ++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/git/objects/util.py b/git/objects/util.py index 64cdf71fd..1fa3b9df0 100644 --- a/git/objects/util.py +++ b/git/objects/util.py @@ -100,9 +100,11 @@ def mode_str_to_int(modestr: Union[bytes, str]) -> int: module regarding the rwx permissions for user, group and other, special flags and file system flags, such as whether it is a symlink. """ + if isinstance(modestr, bytes): + # A byte is an int. int(b"4"[0]) is 52, so b"100644" became 0o6767524. + modestr = modestr.decode("ascii") mode = 0 for iteration, char in enumerate(reversed(modestr[-6:])): - char = cast(Union[str, int], char) mode += int(char) << iteration * 3 # END for each char return mode diff --git a/test/test_util.py b/test/test_util.py index c6e68cf51..46417cc39 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -22,6 +22,7 @@ from git.objects.util import ( altz_to_utctz_str, from_timestamp, + mode_str_to_int, parse_actor_and_date, parse_date, tzoffset, @@ -760,3 +761,10 @@ def test_remove_password_from_command_line(self): redacted_cmd_6 = remove_password_if_present(cmd_6) assert authorization not in " ".join(redacted_cmd_6) assert "http.extraHeader=Authorization: *****" in redacted_cmd_6 + + +def test_mode_str_to_int_accepts_bytes(): + assert mode_str_to_int("100644") == 0o100644 + assert mode_str_to_int(b"100644") == 0o100644 + assert mode_str_to_int("644") == 0o644 + assert mode_str_to_int(b"120000") == 0o120000 From f24ed41dea663785f60cc4f7d5ece240a7d88174 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Thu, 1 Oct 2026 13:37:36 +0000 Subject: [PATCH 02/24] build(deps): bump https://github.com/astral-sh/ruff-pre-commit Bumps the pre-commit group with 1 update: [https://github.com/astral-sh/ruff-pre-commit](https://github.com/astral-sh/ruff-pre-commit). Updates `https://github.com/astral-sh/ruff-pre-commit` from v0.16.5 to 0.16.9 - [Release notes](https://github.com/astral-sh/ruff-pre-commit/releases) - [Commits](https://github.com/astral-sh/ruff-pre-commit/compare/v0.16.5...v0.16.9) --- updated-dependencies: - dependency-name: https://github.com/astral-sh/ruff-pre-commit dependency-version: 0.16.9 dependency-type: direct:production dependency-group: pre-commit ... Signed-off-by: dependabot[bot] --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 45115f60c..9471680e8 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -2,7 +2,7 @@ exclude: ^(?:gitdb|smmap)/ repos: - repo: https://github.com/astral-sh/ruff-pre-commit - rev: v0.16.5 + rev: v0.16.9 hooks: - id: ruff-check args: ["--fix"] From 97eadb888d2f99567e3113f4e455be4ca128f697 Mon Sep 17 00:00:00 2001 From: Byron Date: Thu, 1 Oct 2026 19:17:13 +0200 Subject: [PATCH 03/24] fix(submodule): validate destinations before mutation Pretty much a rubber-stamp. It won't be out there long as the replacement with CLI + Gix is already on the way. Submodule checkout destinations could pass the containment check and be rejected by the index only after cloning had changed the filesystem. This addresses `GHSA-83vg-56qc-22m7` at the shared destination boundaries, including initialization and moves as well as creation. Reuse `_validate_repo_path` before checkout mutations to enforce portable NTFS/HFS metadata-alias checks and invalid-path rejection. Validate Windows filenames and submodule-name NULs before creating directories. Compare path components with `Repo.git_dir` and `Repo.common_dir` by filesystem identity, so separately named metadata directories and their aliases are protected too. Reject metadata destinations nested inside another submodule's Git directory before cloning, reuse, or renaming. Repeat the check after cloning and disable a clone that became nested. Preflight implicit metadata renames during moves, while preserving supported metadata symlinks and relocation of a submodule's own metadata directory. Git reference: `d38352cd43ab9745686d697872408bc3249a153f`, particularly `read-cache.c::verify_path_internal`, NTFS/HFS recognition, `compat/mingw.c::is_valid_win32_path`, and `submodule.c::validate_submodule_git_dir`. Related Git tests are in `t/t7450-bad-git-dotfiles.sh` and `t/t7406-submodule-update.sh`. Regression tests first demonstrated writes before rejection and acceptance of nested and separately named metadata destinations. Tests use harmless file content and compare portable aliases with native Git index validation. Coverage includes all 16 HFS ignored characters, Windows filename rules, relative and absolute paths, metadata reuse, and nesting during cloning. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6.0 Astra --- doc/source/changes.rst | 13 ++ git/objects/submodule/base.py | 91 ++++++++++++-- test/test_submodule.py | 225 ++++++++++++++++++++++++++++++++++ 3 files changed, 318 insertions(+), 11 deletions(-) diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 2f537c99d..8447228db 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,19 @@ Changelog ========= +3.2.1 +===== + +Security fixes for + +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-83vg-56qc-22m7 + +If you can, also try and provide feedback on the upcoming v4 branch +https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. + +See the following for all changes. +https://github.com/gitpython-developers/GitPython/releases/tag/3.2.1 + 3.2.0 ===== diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index 15fe877e6..74fbd39fe 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -8,6 +8,7 @@ import ntpath import os import os.path as osp +import re import shlex import stat import sys @@ -47,6 +48,7 @@ IterableList, RemoteProgress, _to_relative_path, + _validate_repo_path, join_path_native, rmtree, to_native_path_linux, @@ -305,20 +307,55 @@ def _config_parser_constrained(self, read_only: bool) -> SectionConstraint: def _validated_name(cls, name: str) -> str: if ( not name + or "\0" in name or name.startswith(("/", "\\")) or ntpath.splitdrive(name)[0] or ".." in name.replace("\\", "/").split("/") ): raise ValueError("Invalid submodule name %r" % name) + cls._validate_windows_path(name) return name + @staticmethod + def _validate_windows_path(path: PathLike) -> None: + """Apply Git for Windows' filename checks before creating directories.""" + if sys.platform == "win32": + for component in ntpath.splitdrive(os.fspath(path))[1].replace("\\", "/").split("/"): + if component in (".", ".."): + continue + stem = component.split(".", 1)[0].rstrip(" ").upper() + if ( + component.endswith((" ", ".")) + or any(ord(char) < 32 or char in '<>:"|?*' for char in component) + or re.fullmatch(r"CON(?:IN\$|OUT\$)?|PRN|AUX|NUL|COM[1-9]|LPT[1-9]", stem) + ): + raise ValueError("Invalid submodule path on Windows: %r" % path) + @classmethod - def _module_abspath(cls, parent_repo: "Repo", path: PathLike, name: str) -> PathLike: + def _module_abspath( + cls, parent_repo: "Repo", path: PathLike, name: str, *, moving_from: Union[PathLike, None] = None + ) -> PathLike: + """Reject nested Git directories, allowing the source of a pending rename.""" + from git.repo.fun import is_git_dir + name = cls._validated_name(name) if cls._need_gitfile_submodules(parent_repo.git): + directory = osp.join(parent_repo.git_dir, "modules") + for component in to_native_path_linux(name).split("/")[:-1]: + directory = osp.join(directory, component) + if is_git_dir(directory) and ( + moving_from is None or Path(directory).resolve() != Path(moving_from).resolve() + ): + raise ValueError( + "Submodule metadata for %r is inside another Git directory: %r" % (name, directory) + ) return osp.join(parent_repo.git_dir, "modules", name) if parent_repo.working_tree_dir: - return cls._checked_abspath(parent_repo.working_tree_dir, cls._to_relative_path(parent_repo, path)) + return cls._checked_abspath( + parent_repo.working_tree_dir, + cls._to_relative_path(parent_repo, path), + git_dirs=(parent_repo.git_dir, parent_repo.common_dir), + ) raise NotADirectoryError() @classmethod @@ -360,7 +397,9 @@ def _clone_repo( path = cls._to_relative_path(repo, path) if repo.working_tree_dir is None: raise NotADirectoryError("Submodules require a working tree") - module_checkout_path = cls._checked_abspath(repo.working_tree_dir, path) + module_checkout_path = cls._checked_abspath( + repo.working_tree_dir, path, git_dirs=(repo.git_dir, repo.common_dir) + ) module_abspath = cls._module_abspath(repo, path, name) if cls._need_gitfile_submodules(repo.git): if not allow_unsafe_options: @@ -402,6 +441,16 @@ def _clone_repo( **kwargs, ) if cls._need_gitfile_submodules(repo.git): + # A concurrent clone may have turned a leading directory into a repository. + try: + cls._module_abspath(repo, path, name) + except ValueError: + clone.close() + try: + os.remove(osp.join(clone.git_dir, "HEAD")) + except FileNotFoundError: + pass + raise cls._write_git_file_and_module_config(module_checkout_path, module_abspath) return clone @@ -411,8 +460,9 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike: """:return: A path guaranteed to be relative to the given parent repository :raise ValueError: - If path is not contained in the parent repository's working tree. + If path is outside the working tree or is unsafe as a submodule checkout. """ + cls._validate_windows_path(path) if parent_repo.working_tree_dir: path = _to_relative_path(parent_repo.working_tree_dir, path) else: @@ -422,6 +472,7 @@ def _to_relative_path(cls, parent_repo: "Repo", path: PathLike) -> PathLike: if not path or path == ".": raise ValueError("Submodule checkout path must not be the repository root") + _validate_repo_path(path) return path @property @@ -433,22 +484,35 @@ def abspath(self) -> PathLike: def _checkout_abspath(self, relative_path: PathLike, allow_final_symlink: bool = False) -> PathLike: """Check a checkout path already normalized by :meth:`_to_relative_path`.""" - return self._checked_abspath(self.repo.working_tree_dir, relative_path, allow_final_symlink) + return self._checked_abspath( + self.repo.working_tree_dir, + relative_path, + allow_final_symlink, + git_dirs=(self.repo.git_dir, self.repo.common_dir), + ) @classmethod def _checked_abspath( - cls, root: Union[PathLike, None], relative_path: PathLike, allow_final_symlink: bool = False + cls, + root: Union[PathLike, None], + relative_path: PathLike, + allow_final_symlink: bool = False, + *, + git_dirs: Sequence[PathLike] = (), ) -> str: - """Reject symlinks below a trusted root before accessing submodule paths.""" + """Reject symlinks and checkout aliases of Git directories below a trusted root.""" if root is None: raise NotADirectoryError("Submodules require a working tree") path = os.fspath(root) + metadata_dirs = set(git_dirs) components = to_native_path_linux(relative_path).split("/") for index, component in enumerate(components): path = os.fspath(join_path_native(path, component)) - if allow_final_symlink and index == len(components) - 1: - break + if metadata_dirs and osp.exists(path) and any(osp.samefile(path, directory) for directory in metadata_dirs): + raise ValueError("Submodule checkout path aliases Git metadata: %r" % relative_path) if osp.islink(path): + if allow_final_symlink and index == len(components) - 1: + break raise ValueError("Submodule path %r contains a symbolic link" % relative_path) return path @@ -1122,6 +1186,11 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) # Validate the source before removing the destination. cur_path = self.abspath + module_abspath = self._module_abspath(self.repo, self.path, self.name) + if self.path == self.name: + self._module_abspath( + self.repo, module_checkout_path, os.fspath(module_checkout_path), moving_from=module_abspath + ) module_checkout_abspath = self._checkout_abspath(module_checkout_path, allow_final_symlink=True) if osp.isfile(module_checkout_abspath): raise ValueError("Cannot move repository onto a file: %s" % module_checkout_abspath) @@ -1160,7 +1229,6 @@ def move(self, module_path: PathLike, configuration: bool = True, module: bool = renamed_module = True if osp.isfile(osp.join(module_checkout_abspath, ".git")): - module_abspath = self._module_abspath(self.repo, self.path, self.name) self._write_git_file_and_module_config(module_checkout_abspath, module_abspath) # END handle git file rewrite # END move physical module @@ -1522,8 +1590,8 @@ def rename(self, new_name: str) -> "Submodule": self._validated_name(self.name) self._validated_name(new_name) - destination_module_abspath = self._module_abspath(self.repo, self.path, new_name) mod = self.module() + destination_module_abspath = self._module_abspath(self.repo, self.path, new_name, moving_from=mod.git_dir) self._checked_abspath(self.repo.working_tree_dir, self.k_modules_file) # .git/config @@ -1573,6 +1641,7 @@ def module(self) -> "Repo": """ self._validated_name(self.name) module_checkout_abspath = self.abspath + self._module_abspath(self.repo, self.path, self.name) try: repo = git.Repo(module_checkout_abspath) if repo != self.repo: diff --git a/test/test_submodule.py b/test/test_submodule.py index b1f4f5156..45b285c55 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -86,6 +86,7 @@ def test_submodule_update_preserves_literal_name(tmp_path, monkeypatch, caplog, def movable_submodule(tmp_path): """Create a committed local submodule whose logical name stays fixed when moved.""" with git.Repo.init(tmp_path / "source") as source, git.Repo.init(tmp_path / "parent") as parent: + source.git.symbolic_ref("HEAD", "refs/heads/master") (tmp_path / "source" / "file").write_text("content", encoding="utf-8") source.index.add(["file"]) source.index.commit("Create source") @@ -111,6 +112,207 @@ def _move_snapshot(submodule): ) +@pytest.mark.parametrize( + "path", + [ + ".git/child", + ".GiT/child", + "nested/.git/child", + "git~1/child", + "GIT~1 . /child", + ".git. /child", + ".git:stream/child", + ".git::$INDEX_ALLOCATION/child", + "nested\\.git\\child", + "C:relative", + "nul\0name", + ] + + [ + f".g{chr(codepoint)}it/child" + for codepoint in (*range(0x200C, 0x2010), *range(0x202A, 0x202F), *range(0x206A, 0x2070), 0xFEFF) + ], +) +@pytest.mark.parametrize("operation", ["add", "clone", "update", "move", "move-module", "move-config"]) +def test_submodule_rejects_unsafe_checkout_before_mutation(movable_submodule, tmp_path, path, operation): + sm = movable_submodule + root = Path(sm.repo.working_tree_dir) + before = _move_snapshot(sm) + paths = set(root.rglob("*")) + # Check the portable metadata aliases against Git's index validation as well. + if operation == "clone" and path not in ("C:relative", "nul\0name"): + with sm.repo.git.custom_environment(GIT_INDEX_FILE=str(tmp_path / "validation-index")): + with _patch_git_config("core.protectHFS", "true"), _patch_git_config("core.protectNTFS", "true"): + with pytest.raises(GitCommandError): + sm.repo.git.update_index("--add", "--cacheinfo", f"160000,{sm.hexsha},{path}") + with mock.patch.object(git.Repo, "clone_from", side_effect=AssertionError("clone attempted")): + with pytest.raises(ValueError): + if operation == "add": + sm.repo.create_submodule("new", path, sm.url) + elif operation == "clone": + Submodule._clone_repo(sm.repo, sm.url, path, "new") + elif operation == "update": + Submodule(sm.repo, sm.binsha, name="new", path=path, url=sm.url).update(init=True) + else: + sm.move(path, configuration=operation != "move-module", module=operation != "move-config") + assert _move_snapshot(sm) == before + assert set(root.rglob("*")) == paths + + +def test_add_rejects_metadata_checkout_without_writing_files(movable_submodule): + sm = movable_submodule + before = _move_snapshot(sm) + with pytest.raises(ValueError, match="Git metadata"): + sm.repo.create_submodule("new", ".git/new", sm.url) + assert not Path(sm.repo.git_dir, "new").exists() + assert not Path(sm.repo.git_dir, "modules/new").exists() + assert _move_snapshot(sm) == before + + +@pytest.mark.parametrize("absolute_path", [False, True]) +@pytest.mark.parametrize("metadata_name", ["metadata", "MeTaDaTa"]) +@pytest.mark.parametrize("operation", ["add", "clone", "update", "move"]) +def test_submodule_rejects_checkout_in_separate_metadata( + movable_submodule, tmp_path, absolute_path, metadata_name, operation +): + root = tmp_path / "separate" + with git.Repo.init(root, separate_git_dir=str(root / "metadata"), allow_unsafe_options=True) as parent: + if not (root / metadata_name).is_dir(): + pytest.skip("Requires a case-insensitive filesystem") + sm = parent.create_submodule("module", "module", movable_submodule.url) + parent.index.commit("Add submodule") + before = _move_snapshot(sm) + paths = set(root.rglob("*")) + path = root / metadata_name / "new" if absolute_path else f"{metadata_name}/new" + with pytest.raises(ValueError, match="Git metadata"): + if operation == "add": + parent.create_submodule("new", path, sm.url) + elif operation == "clone": + Submodule._clone_repo(parent, sm.url, path, "new") + elif operation == "update": + Submodule(parent, sm.binsha, name="new", path=path, url=sm.url).update(init=True) + else: + sm.move(path) + assert _move_snapshot(sm) == before + assert set(root.rglob("*")) == paths + + +@pytest.mark.parametrize("operation", ["add", "clone", "update", "rename", "move"]) +def test_submodule_rejects_nested_metadata_before_mutation(movable_submodule, operation): + sm = movable_submodule + other = sm.repo.create_submodule("other", "other", sm.url) + other.module().close() + root = Path(sm.repo.working_tree_dir) + before = _move_snapshot(sm), _move_snapshot(other) + paths = set(root.rglob("*")) + name = f"{sm.name}/child" + with pytest.raises(ValueError, match="inside.*Git directory"): + if operation == "add": + sm.repo.create_submodule(name, "new", sm.url) + elif operation == "clone": + Submodule._clone_repo(sm.repo, sm.url, "new", name) + elif operation == "update": + Submodule(sm.repo, sm.binsha, name=name, path="new", url=sm.url).update(init=True) + elif operation == "rename": + other.rename(name) + else: + other.move(name) + assert (_move_snapshot(sm), _move_snapshot(other)) == before + assert set(root.rglob("*")) == paths + + +@pytest.mark.parametrize("state", ["retained", "checked-out"]) +def test_update_rejects_existing_nested_metadata(movable_submodule, state): + sm = movable_submodule + name = f"{sm.name}/child" + checkout = Path(sm.repo.working_tree_dir, "new") + with git.Repo.clone_from( + sm.url, checkout, separate_git_dir=str(Path(sm.repo.git_dir, "modules", name)), allow_unsafe_options=True + ): + pass + if state == "retained": + shutil.rmtree(checkout) + before = _move_snapshot(sm) + paths = set(Path(sm.repo.working_tree_dir).rglob("*")) + with pytest.raises(ValueError, match="inside.*Git directory"): + Submodule(sm.repo, sm.binsha, name=name, path="new", url=sm.url).update(init=True, no_fetch=True) + assert _move_snapshot(sm) == before + assert set(Path(sm.repo.working_tree_dir).rglob("*")) == paths + + +@pytest.mark.parametrize( + "path", + [ + "CON", + "con.txt", + "CONIN$", + "conout$.txt", + "AUX .txt", + "PRN", + "NUL", + "COM9", + "LPT1", + "name:stream", + "space ", + "period.", + "line\nbreak", + "star*", + 'quote"', + "question?", + "angle<", + "angle>", + "pipe|", + ], +) +def test_submodule_rejects_windows_destination_names_before_mutation(movable_submodule, path): + sm = movable_submodule + before = _move_snapshot(sm) + paths = set(Path(sm.repo.working_tree_dir).rglob("*")) + with mock.patch("git.objects.submodule.base.sys", SimpleNamespace(platform="win32")): + with mock.patch.object(git.Repo, "clone_from", side_effect=AssertionError("clone attempted")): + with pytest.raises(ValueError): + sm.repo.create_submodule("new", f"nested/{path}", sm.url) + with pytest.raises(ValueError): + sm.repo.create_submodule(f"nested/{path}", "new", sm.url) + assert _move_snapshot(sm) == before + assert set(Path(sm.repo.working_tree_dir).rglob("*")) == paths + + +@pytest.mark.parametrize("path", ["nested/space ", "nested/period."]) +def test_windows_destination_validation_precedes_normalization(tmp_path, path): + parent = SimpleNamespace(working_tree_dir=str(tmp_path)) + # Windows' GetFullPathName removes trailing spaces and periods. + with mock.patch("git.objects.submodule.base._to_relative_path", return_value=path.rstrip(" .")): + with mock.patch("git.objects.submodule.base.sys", SimpleNamespace(platform="win32")): + with pytest.raises(ValueError, match="Invalid submodule path on Windows"): + Submodule._to_relative_path(parent, path) + + +def test_submodule_can_relocate_its_own_metadata(movable_submodule): + sm = movable_submodule + sm.rename(f"{sm.name}/child") + assert Path(sm.abspath, "file").read_text() == "content" + with sm.module() as module: + assert Path(module.git_dir) == Path(sm.repo.git_dir, "modules", sm.name) + + +def test_clone_disables_metadata_that_becomes_nested(movable_submodule, monkeypatch): + sm = movable_submodule + clone_from = git.Repo.clone_from + ancestor = Path(sm.repo.git_dir, "modules/new") + + def clone_and_create_ancestor(*args, **kwargs): + clone = clone_from(*args, **kwargs) + with git.Repo.init(ancestor, bare=True): + pass + return clone + + monkeypatch.setattr(git.Repo, "clone_from", clone_and_create_ancestor) + with pytest.raises(ValueError, match="inside.*Git directory"): + Submodule._clone_repo(sm.repo, sm.url, "new", "new/child") + assert not (ancestor / "child/HEAD").exists() + assert (ancestor / "HEAD").is_file() + + @pytest.mark.parametrize("target_kind", ["relative", "absolute", "internal", "dangling"]) @pytest.mark.parametrize("configuration,module", [(True, True), (False, True), (True, False)]) @pytest.mark.parametrize("absolute_path", [False, True]) @@ -167,6 +369,28 @@ def test_move_normal_destination(movable_submodule, absolute_path): assert _move_snapshot(submodule) == before +@pytest.mark.parametrize("metadata_dir", ["git_dir", "common_dir"]) +def test_move_rejects_leaf_symlink_to_metadata(movable_submodule, tmp_path, metadata_dir): + root = tmp_path / "worktree" + movable_submodule.repo.git.worktree("add", "--detach", str(root)) + with git.Repo(root) as parent: + assert not osp.samefile(parent.git_dir, parent.common_dir) + submodule = parent.submodules[0] + submodule.update(init=True) + target = Path(getattr(parent, metadata_dir)) + destination = root / "destination" + destination.symlink_to(target, target_is_directory=True) + before = _move_snapshot(submodule) + + with pytest.raises(ValueError, match="Git metadata"): + submodule.move("destination", module=False) + + assert _move_snapshot(submodule) == before + assert destination.is_symlink() + assert destination.samefile(target) + assert Path(submodule.abspath, "file").read_text(encoding="utf-8") == "content" + + @pytest.mark.parametrize("kind", ["empty", "nonempty", "file", "dangling"]) def test_move_leaf_symlink_compatibility(movable_submodule, tmp_path, kind): """Preserve leaf-symlink replacement without modifying the external target. @@ -1410,6 +1634,7 @@ def test_update_rejects_parent_component_in_name(self, rwdir): invalid_names = ( "", + "nul\0name", "..", "../module", R"..\module", From b0ae041305ec342625b1398a673d5b8ada946fa5 Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Fri, 2 Oct 2026 09:22:41 +0530 Subject: [PATCH 04/24] fix(util): create lock files in one exclusive step `LockFile._obtain_lock_or_raise()` tested for the lock with `osp.isfile()` and then created it with `open(lock_file, "w")`. Nothing keeps another holder out between the two calls, so several can pass the test and all of them set `_owns_lock`. Racing eight holders on one lock file leaves seven believing they own it, which removes the mutual exclusion that `GitConfigParser` in write mode and `RefLog.append_entry()` rely on. `osp.isfile()` also resolves symbolic links. A dangling symlink planted at `.lock` therefore reports no lock, and the following `open()` resolves it and creates the target, outside the repository. Create the lock with `os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600)` instead. That is the single-step exclusive create `gitdb`'s `LockedFD.open()` and Git's own `lock_file()` already use, and `O_EXCL` fails with `EEXIST` on a symbolic link rather than resolving it, so both problems close together. The creation mode matches `LockedFD`; nothing reads a lock file's contents, and breaking a stale lock needs write permission on the containing directory rather than on the file. `FileExistsError` is translated back into the existing "did already exist" `OSError`, so `BlockingLockFile`'s retry loop and the existing `test_lock_file` and `test_blocking_lock_file` cases are unaffected. A directory at the lock path is still reported through the generic `OSError` branch. This covers acquiring the lock only; writing the locked file stays the caller's concern, as before. Adds `test_lock_file_does_not_follow_a_symlink` and `test_lock_file_is_obtained_by_a_single_holder`. Both fail on the previous code (`1 != 7`, and the symlink target gets created) and pass here. Validation: full `pytest` suite green on Python 3.11 on macOS, plus `ruff check`, `ruff format`, `mypy` and `basedpyright --warnings` clean. --- git/util.py | 15 +++++++++------ test/test_util.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/git/util.py b/git/util.py index 6f1f38401..ca942340d 100644 --- a/git/util.py +++ b/git/util.py @@ -1166,17 +1166,20 @@ def _obtain_lock_or_raise(self) -> None: if self._has_lock(): return lock_file = self._lock_file_path() - if osp.isfile(lock_file): + # Create the lock in one step, the way Git and gitdb's LockedFD do. Testing + # for the file first leaves a window in which another holder creates it and + # both proceed, and O_CREAT|O_EXCL additionally refuses to follow a symbolic + # link planted at the lock path instead of writing through it. + try: + fd = os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + except FileExistsError as e: raise OSError( "Lock for file %r did already exist, delete %r in case the lock is illegal" % (self._file_path, lock_file) - ) - - try: - with open(lock_file, mode="w"): - pass + ) from e except OSError as e: raise OSError(str(e)) from e + os.close(fd) self._owns_lock = True diff --git a/test/test_util.py b/test/test_util.py index 46417cc39..203e3a54f 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -12,6 +12,7 @@ import subprocess import sys import tempfile +import threading import time from unittest import SkipTest, mock @@ -446,6 +447,44 @@ def test_lock_file(self): lock_file._obtain_lock_or_raise() lock_file._release_lock() + @requires_symlinks + def test_lock_file_does_not_follow_a_symlink(self): + with tempfile.TemporaryDirectory() as tdir: + my_file = os.path.join(tdir, "my-lock-file") + outside = os.path.join(tdir, "outside-the-lock") + os.symlink(outside, my_file + ".lock") + + lock_file = LockFile(my_file) + self.assertRaises(IOError, lock_file._obtain_lock_or_raise) + assert not lock_file._has_lock() + assert not os.path.exists(outside) + + def test_lock_file_is_obtained_by_a_single_holder(self): + with tempfile.TemporaryDirectory() as tdir: + my_file = os.path.join(tdir, "my-lock-file") + racers = 8 + at_the_line = threading.Barrier(racers) + holders = [] + guard = threading.Lock() + + def obtain(): + lock_file = LockFile(my_file) + at_the_line.wait() + try: + lock_file._obtain_lock_or_raise() + except OSError: + return + with guard: + holders.append(lock_file) + + threads = [threading.Thread(target=obtain) for _ in range(racers)] + for thread in threads: + thread.start() + for thread in threads: + thread.join() + + self.assertEqual(1, len(holders)) + def test_blocking_lock_file(self): with tempfile.TemporaryDirectory() as tdir: my_file = os.path.join(tdir, "my-lock-file") From 97a546899df8b41496c71c128b9b4154bacc5fe5 Mon Sep 17 00:00:00 2001 From: Byron Date: Fri, 2 Oct 2026 04:52:11 +0000 Subject: [PATCH 05/24] fix(util): acquire Windows locks without following symlinks Took brief look only. Hope this code soon won't be present anymore. Windows follows dangling symlinks even when `os.open` uses `O_CREAT | O_EXCL`, so acquiring a lock could create its symlink target and incorrectly report ownership. The `_winapi.CreateFile` workaround also uses the ANSI API on Python 3.8 through 3.10, causing failures in Unicode directories or creating locks under mangled filenames. Use `CreateFileW` with `CREATE_NEW` and `FILE_FLAG_OPEN_REPARSE_POINT` to create the lock atomically while rejecting existing links. Declare the `ctypes` argument and return types explicitly so Unicode paths and native handle sizes are preserved. Close the handle before recording ownership, propagate Windows errors, and reject embedded NULs before the native API can truncate a path. Preserve exclusive `os.open` creation on POSIX. Expand the lock tests to cover Unicode filenames and directories, including non-BMP characters, and verify that the requested lock path is actually created and removed. Check NUL rejection, preserve both existing and missing symlink targets, and explicitly release the concurrent test's acquired locks. Reproduced the CI failure in `test_clone_from_with_path_contains_unicode` on Windows/Python 3.8.10 before the fix. The affected utility, clone, and configuration modules pass on Python 3.8.10 and 3.13.14: 169 passed, 41 skipped, and 2 expected failures on each version. The new Unicode and NUL regressions also failed before their respective fixes. `ruff check`, `ruff format --check`, `mypy --python-version=3.13`, and `basedpyright --warnings` pass. Assisted-by: GPT 6.0 Astra Co-authored-by: GPT 6 --- git/util.py | 50 +++++++++++++++++++++++++++++++++++++++++------ test/test_util.py | 37 ++++++++++++++++++++++++++++++----- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/git/util.py b/git/util.py index ca942340d..017a94ad3 100644 --- a/git/util.py +++ b/git/util.py @@ -1166,12 +1166,51 @@ def _obtain_lock_or_raise(self) -> None: if self._has_lock(): return lock_file = self._lock_file_path() - # Create the lock in one step, the way Git and gitdb's LockedFD do. Testing - # for the file first leaves a window in which another holder creates it and - # both proceed, and O_CREAT|O_EXCL additionally refuses to follow a symbolic - # link planted at the lock path instead of writing through it. + # Create the lock in one step. Checking for it first would allow another + # holder to create it between the check and the open. try: - fd = os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + if sys.platform == "win32": + if "\0" in lock_file: + raise ValueError("embedded null character") + + import ctypes + from ctypes import wintypes + + # Unlike POSIX, Windows follows dangling symlinks even with O_EXCL. + # Open the reparse point itself so an existing link is rejected. + # Call the Unicode API directly: older _winapi.CreateFile wrappers + # use the ANSI API and can create a lock under the wrong filename. + kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) + create_file = kernel32.CreateFileW + create_file.argtypes = ( + wintypes.LPCWSTR, + wintypes.DWORD, + wintypes.DWORD, + wintypes.LPVOID, + wintypes.DWORD, + wintypes.DWORD, + wintypes.HANDLE, + ) + create_file.restype = wintypes.HANDLE + close_handle = kernel32.CloseHandle + close_handle.argtypes = (wintypes.HANDLE,) + close_handle.restype = wintypes.BOOL + handle = create_file( + lock_file, + 0x40000000, # GENERIC_WRITE + 0, + None, + 1, # CREATE_NEW + 0x00200000, # FILE_FLAG_OPEN_REPARSE_POINT + None, + ) + if handle == wintypes.HANDLE(-1).value: + raise ctypes.WinError(ctypes.get_last_error()) + if not close_handle(handle): + raise ctypes.WinError(ctypes.get_last_error()) + else: + fd = os.open(lock_file, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + os.close(fd) except FileExistsError as e: raise OSError( "Lock for file %r did already exist, delete %r in case the lock is illegal" @@ -1179,7 +1218,6 @@ def _obtain_lock_or_raise(self) -> None: ) from e except OSError as e: raise OSError(str(e)) from e - os.close(fd) self._owns_lock = True diff --git a/test/test_util.py b/test/test_util.py index 203e3a54f..eb520aac6 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -419,9 +419,11 @@ def test_it_should_dashify(self): self.assertEqual("this-is-my-argument", dashify("this_is_my_argument")) self.assertEqual("foo", dashify("foo")) - def test_lock_file(self): + @ddt.data("my-lock-file", "my-lock-file-\u0394", "\u0394/my-lock-file", "\U0001f680/my-lock-file") + def test_lock_file(self, filename): with tempfile.TemporaryDirectory() as tdir: - my_file = os.path.join(tdir, "my-lock-file") + my_file = os.path.join(tdir, filename) + os.makedirs(os.path.dirname(my_file), exist_ok=True) lock_file = LockFile(my_file) assert not lock_file._has_lock() # Release lock we don't have - fine. @@ -430,6 +432,7 @@ def test_lock_file(self): # Get lock. lock_file._obtain_lock_or_raise() assert lock_file._has_lock() + assert os.path.isfile(my_file + ".lock") # Concurrent access. other_lock_file = LockFile(my_file) @@ -438,6 +441,7 @@ def test_lock_file(self): lock_file._release_lock() assert not lock_file._has_lock() + assert not os.path.exists(my_file + ".lock") other_lock_file._obtain_lock_or_raise() self.assertRaises(IOError, lock_file._obtain_lock_or_raise) @@ -447,17 +451,36 @@ def test_lock_file(self): lock_file._obtain_lock_or_raise() lock_file._release_lock() + def test_lock_file_rejects_embedded_nul(self): + with tempfile.TemporaryDirectory() as tdir: + my_file = os.path.join(tdir, "my-lock-file") + lock_file = LockFile(my_file + "\0suffix") + self.assertRaises(ValueError, lock_file._obtain_lock_or_raise) + assert not lock_file._has_lock() + assert not os.path.exists(my_file) + + @ddt.data(False, True) @requires_symlinks - def test_lock_file_does_not_follow_a_symlink(self): + def test_lock_file_does_not_follow_a_symlink(self, target_exists): with tempfile.TemporaryDirectory() as tdir: my_file = os.path.join(tdir, "my-lock-file") outside = os.path.join(tdir, "outside-the-lock") + content = b"Do not modify the symlink target." + if target_exists: + with open(outside, "wb") as stream: + stream.write(content) os.symlink(outside, my_file + ".lock") lock_file = LockFile(my_file) self.assertRaises(IOError, lock_file._obtain_lock_or_raise) assert not lock_file._has_lock() - assert not os.path.exists(outside) + lock_file._release_lock() + assert os.path.islink(my_file + ".lock") + if target_exists: + with open(outside, "rb") as stream: + self.assertEqual(stream.read(), content) + else: + assert not os.path.exists(outside) def test_lock_file_is_obtained_by_a_single_holder(self): with tempfile.TemporaryDirectory() as tdir: @@ -483,7 +506,11 @@ def obtain(): for thread in threads: thread.join() - self.assertEqual(1, len(holders)) + try: + self.assertEqual(1, len(holders)) + finally: + for lock_file in holders: + lock_file._release_lock() def test_blocking_lock_file(self): with tempfile.TemporaryDirectory() as tdir: From 226654af2d560b2de87f83f9c86eb9a3312c9496 Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Sat, 3 Oct 2026 00:27:34 +0530 Subject: [PATCH 06/24] fix(commit): accumulate gpgsig continuation lines in linear time `Commit._deserialize` collected a `gpgsig` header by appending each continuation line to a `bytes` object with `+=`. `bytes` are immutable, so every line copied the whole signature read so far, and a signature with *n* continuation lines cost O(n^2) to parse. Commit headers are fully controlled by whoever wrote the commit, and `_deserialize` runs on the first access to any commit attribute (`author`, `message`, `parents`, ...). Reading the commits of an untrusted repository is therefore enough to hit it. Measured on Python 3.11 with a `gpgsig` header of `" x\n"` lines: | object size | before | after | | ----------- | ------- | ------ | | 2.4 MB | 9.5 s | 0.08 s | | 4.8 MB | 116.8 s | | | 9.6 MB | 547.9 s | 0.32 s | Collect the lines in a list and join them once. This is linear and produces byte-identical `gpgsig` values, so the existing `test_gpgsig` round trip is unchanged. `test_gpgsig_deserialization_is_linear` deserializes a 1.8 MB signature under a 1 s CPU-time bound. It fails on the previous code and passes here. The full suite has no new failures; the 8 tests that fail locally (non-UTF-8 trailer encoding and NUL submodule names) fail identically without this change. `ruff`, `mypy` and `basedpyright --warnings` are clean. --- git/objects/commit.py | 6 +++--- test/test_commit.py | 22 ++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/git/objects/commit.py b/git/objects/commit.py index 771f87976..307b8c59b 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -885,7 +885,7 @@ def _deserialize(self, stream: BytesIO) -> "Commit": if buf[0:10] == b"encoding ": self.encoding = buf[buf.find(b" ") + 1 :].decode(self.encoding, "ignore") elif buf[0:7] == b"gpgsig ": - sig = buf[buf.find(b" ") + 1 :] + b"\n" + sig_lines = [buf[buf.find(b" ") + 1 :] + b"\n"] is_next_header = False while True: sigbuf = readline() @@ -895,9 +895,9 @@ def _deserialize(self, stream: BytesIO) -> "Commit": buf = sigbuf.strip() is_next_header = True break - sig += sigbuf[1:] + sig_lines.append(sigbuf[1:]) # END read all signature - self.gpgsig = sig.rstrip(b"\n").decode(self.encoding, "ignore") + self.gpgsig = b"".join(sig_lines).rstrip(b"\n").decode(self.encoding, "ignore") if is_next_header: continue buf = readline().strip() diff --git a/test/test_commit.py b/test/test_commit.py index b5baf0e33..0a5bcdb7d 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -607,6 +607,28 @@ def test_commit_co_authors_bounds_malformed_trailer(self): # The malformed line yields nothing; the well-formed trailer still parses. assert result == [Actor("Real Name", "real@example.com")] + def test_gpgsig_deserialization_is_linear(self): + """A long gpgsig header must not make deserialization run in quadratic time.""" + num_lines = 600_000 + # Commit headers are fully attacker-controlled. Accumulating the signature with + # bytes concatenation copied it once per continuation line (O(n^2)). + data = ( + b"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" + b"author A 1700000000 +0000\n" + b"committer A 1700000000 +0000\n" + b"gpgsig -----BEGIN PGP SIGNATURE-----\n" + b" x\n" * num_lines + b" -----END PGP SIGNATURE-----\n" + b"\n" + b"message\n" + ) + cmt = copy.copy(self.rorepo.commit()) + start = time.process_time() + cmt._deserialize(BytesIO(data)) + elapsed = time.process_time() - start + # Leave ample CPU time for slow runners, but catch quadratic accumulation. + self.assertLess(elapsed, 1.0) + self.assertEqual(cmt.gpgsig.count("\n"), num_lines + 1) + self.assertEqual(cmt.message, "message\n") + @with_rw_directory def test_create_from_tree_with_trailers_dict(self, rw_dir): """Test that create_from_tree supports adding trailers via a dict.""" From 5fc5f3456a6816d4caa03fdb4a3b000d3170a290 Mon Sep 17 00:00:00 2001 From: Byron Date: Sat, 3 Oct 2026 04:31:59 +0200 Subject: [PATCH 07/24] review - remove test as it can totally fail on slow runners, and extending the time budget makes it hard to see if it's actually working. Let's trust what we see and look forward to a time when the in-python parsing is gone as well. --- test/test_commit.py | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/test/test_commit.py b/test/test_commit.py index 0a5bcdb7d..b5baf0e33 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -607,28 +607,6 @@ def test_commit_co_authors_bounds_malformed_trailer(self): # The malformed line yields nothing; the well-formed trailer still parses. assert result == [Actor("Real Name", "real@example.com")] - def test_gpgsig_deserialization_is_linear(self): - """A long gpgsig header must not make deserialization run in quadratic time.""" - num_lines = 600_000 - # Commit headers are fully attacker-controlled. Accumulating the signature with - # bytes concatenation copied it once per continuation line (O(n^2)). - data = ( - b"tree 4b825dc642cb6eb9a060e54bf8d69288fbee4904\n" - b"author A 1700000000 +0000\n" - b"committer A 1700000000 +0000\n" - b"gpgsig -----BEGIN PGP SIGNATURE-----\n" + b" x\n" * num_lines + b" -----END PGP SIGNATURE-----\n" - b"\n" - b"message\n" - ) - cmt = copy.copy(self.rorepo.commit()) - start = time.process_time() - cmt._deserialize(BytesIO(data)) - elapsed = time.process_time() - start - # Leave ample CPU time for slow runners, but catch quadratic accumulation. - self.assertLess(elapsed, 1.0) - self.assertEqual(cmt.gpgsig.count("\n"), num_lines + 1) - self.assertEqual(cmt.message, "message\n") - @with_rw_directory def test_create_from_tree_with_trailers_dict(self, rw_dir): """Test that create_from_tree supports adding trailers via a dict.""" From 6795806a4d02781d7150fc71e27b7e778bb65b55 Mon Sep 17 00:00:00 2001 From: kokotatan Date: Sat, 3 Oct 2026 23:45:30 +0900 Subject: [PATCH 08/24] fix(index): accept a single PathLike in checkout `IndexFile.checkout()` documents accepting a single path, but only wraps strings before iterating. Passing one `pathlib.Path` or custom `os.PathLike` therefore raises `TypeError`, while a list containing the same object already works. Wrap single `os.PathLike` objects along with strings and widen the public annotation. Keep existing path normalization and iterable behavior. Add 36 real-repository cases covering path types, absolute and relative paths, files and directories, and single/list/iterator inputs. Eight single-PathLike cases failed before this fix. The focused tests pass, as do all pre-commit hooks, `mypy`, `basedpyright`, and strict Sphinx HTML. The full suite has 1,410 passing tests and 121 Windows symlink-privilege failures, all reproduced with identical node IDs on the pristine base. Assisted-by: OpenAI GPT-6 --- git/index/base.py | 4 ++-- test/test_index.py | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index c9d99840d..4ecd0c5b1 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1303,7 +1303,7 @@ def _flush_stdin_and_wait(self, proc: "Popen[bytes]", ignore_stdout: bool = Fals @default_index def checkout( self, - paths: Union[None, Iterable[PathLike]] = None, + paths: Union[None, PathLike, Iterable[PathLike]] = None, force: bool = False, fprogress: Callable = lambda *args: None, allow_unsafe_options: bool = False, @@ -1442,7 +1442,7 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik handle_stderr(proc, rval_iter) return rval_iter else: - if isinstance(paths, str): + if isinstance(paths, (str, os.PathLike)): paths = [paths] # Make sure we have our entries loaded before we start checkout_index, which diff --git a/test/test_index.py b/test/test_index.py index 8c33fe29d..30cb54565 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1731,6 +1731,32 @@ def test_index_file_v3_with_git_command(self, tmp_dir): assert "A file2.txt" in status_lines +class TestIndexCheckout: + @pytest.mark.parametrize("path_type", [str, Path, PathLikeMock]) + @pytest.mark.parametrize("absolute", [False, True]) + @pytest.mark.parametrize("directory", [False, True]) + @pytest.mark.parametrize("container", ["single", "list", "iterator"]) + def test_checkout_pathlike(self, tmp_path, path_type, absolute, directory, container): + with Repo.init(tmp_path) as repo: + nested = tmp_path / "nested" + nested.mkdir() + files = {"nested/first": b"first", "nested/second": b"second", "outside": b"outside"} + for name, data in files.items(): + (tmp_path / name).write_bytes(data) + repo.index.add(list(files)) + + path_name = "nested" if directory else "nested/first" + path = path_type(str(tmp_path / path_name) if absolute else path_name) + paths = path if container == "single" else [path] if container == "list" else iter([path]) + expected = {"nested/first", "nested/second"} if directory else {"nested/first"} + for name in expected: + (tmp_path / name).unlink() + + assert set(repo.index.checkout(paths)) == expected + for name, data in files.items(): + assert (tmp_path / name).read_bytes() == data + + class TestIndexUtils: @pytest.mark.parametrize("file_path_type", [str, Path]) def test_temporary_file_swap(self, tmp_path, file_path_type): From 6fdfa315406170a0fbc4898123e0bfda67cb9070 Mon Sep 17 00:00:00 2001 From: Yunare Maia Date: Mon, 5 Oct 2026 02:14:59 +0000 Subject: [PATCH 09/24] fix(util): redact URL credentials without corrupting the host remove_password_if_present() redacted credentials by substituting them into url.netloc with str.replace(), which rewrites every occurrence anywhere in the netloc -- including the host. https://git@github.com/user/repo.git -> https://*****@*****hub.com/user/repo.git "git" is the most common Git username and a substring of "github.com", so this hits the common case. An empty username or password makes it worse: str.replace("", "*****") matches at every position, so https://:token@fakerepo.example.com/testrepo comes back shredded into a URL roughly ten times longer, which is what lands in GitCommandError messages when a fetch fails. Rebuild the netloc from its userinfo instead, so only the credentials are replaced. The existing test already covered the empty-password case but only asserted the password is absent, which any mangling satisfies. The function is the single redaction path used for logged command lines and for exception messages (git/cmd.py, git/exc.py, git/repo/base.py), so this fixes all of them at once. --- git/util.py | 12 ++++++++---- test/test_util.py | 13 +++++++++++++ 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/git/util.py b/git/util.py index 017a94ad3..deebdc8d0 100644 --- a/git/util.py +++ b/git/util.py @@ -658,10 +658,14 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]: if url.password is None and url.username is None: continue - if url.password is not None: - url = url._replace(netloc=url.netloc.replace(url.password, "*****")) - if url.username is not None: - url = url._replace(netloc=url.netloc.replace(url.username, "*****")) + # Redact the userinfo as a whole rather than substituting the + # username/password into the netloc: a substring replace also hits + # the host ("git" in "github.com") and an empty username or password + # matches at every position. + _, at, hostinfo = url.netloc.rpartition("@") + if at: + redacted = "*****:*****" if url.password is not None else "*****" + url = url._replace(netloc=f"{redacted}@{hostinfo}") new_cmdline[index] = urlunsplit(url) except ValueError: # This is not a valid URL. diff --git a/test/test_util.py b/test/test_util.py index eb520aac6..5b72cb432 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -828,6 +828,19 @@ def test_remove_password_from_command_line(self): assert authorization not in " ".join(redacted_cmd_6) assert "http.extraHeader=Authorization: *****" in redacted_cmd_6 + def test_remove_password_keeps_host_intact(self): + """Redaction must not touch the host, even when it contains the username.""" + redacted = remove_password_if_present(["git", "clone", "https://git@github.com/user/repo.git"]) + assert redacted == ["git", "clone", "https://*****@github.com/user/repo.git"] + + redacted = remove_password_if_present(["git", "clone", "ssh://git@github.com/u/r.git"]) + assert redacted == ["git", "clone", "ssh://*****@github.com/u/r.git"] + + def test_remove_empty_password_keeps_host_intact(self): + """An empty password must not expand into every position of the netloc.""" + redacted = remove_password_if_present(["git", "clone", "https://:@fakerepo.example.com/testrepo"]) + assert redacted == ["git", "clone", "https://*****:*****@fakerepo.example.com/testrepo"] + def test_mode_str_to_int_accepts_bytes(): assert mode_str_to_int("100644") == 0o100644 From 0f89e3621943a34c49d702061308ac90320780f5 Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 5 Oct 2026 04:42:18 +0200 Subject: [PATCH 10/24] review - add some edge cases to the tests. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- git/util.py | 13 +++++-------- test/test_util.py | 15 +++++++++++++++ 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/git/util.py b/git/util.py index deebdc8d0..75677b7d3 100644 --- a/git/util.py +++ b/git/util.py @@ -658,14 +658,11 @@ def remove_password_if_present(cmdline: Sequence[str]) -> List[str]: if url.password is None and url.username is None: continue - # Redact the userinfo as a whole rather than substituting the - # username/password into the netloc: a substring replace also hits - # the host ("git" in "github.com") and an empty username or password - # matches at every position. - _, at, hostinfo = url.netloc.rpartition("@") - if at: - redacted = "*****:*****" if url.password is not None else "*****" - url = url._replace(netloc=f"{redacted}@{hostinfo}") + # Match urllib.parse's userinfo boundary. Keeping the raw hostinfo + # preserves hostname case, IPv6 brackets, and port formatting. + _, _, hostinfo = url.netloc.rpartition("@") + redacted = "*****:*****" if url.password is not None else "*****" + url = url._replace(netloc=f"{redacted}@{hostinfo}") new_cmdline[index] = urlunsplit(url) except ValueError: # This is not a valid URL. diff --git a/test/test_util.py b/test/test_util.py index 5b72cb432..ca8e006f3 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -841,6 +841,21 @@ def test_remove_empty_password_keeps_host_intact(self): redacted = remove_password_if_present(["git", "clone", "https://:@fakerepo.example.com/testrepo"]) assert redacted == ["git", "clone", "https://*****:*****@fakerepo.example.com/testrepo"] + @ddt.data( + ( + "https://user%40example.com:p%40ss@GitHub.COM:00443/repo@name?q=a@b#c@d", + "https://*****:*****@GitHub.COM:00443/repo@name?q=a@b#c@d", + ), + ("//user:pass@[2001:db8::1]:0080/repo", "//*****:*****@[2001:db8::1]:0080/repo"), + ("https://user:p@ss@example.com/repo", "https://*****:*****@example.com/repo"), + ("https://user:@example.com/repo", "https://*****:*****@example.com/repo"), + ("https://@example.com/repo", "https://*****@example.com/repo"), + ("https://example.com/repo@name?q=a@b#c@d", "https://example.com/repo@name?q=a@b#c@d"), + ) + @ddt.unpack + def test_remove_password_preserves_url_components(self, url, expected): + assert remove_password_if_present([url]) == [expected] + def test_mode_str_to_int_accepts_bytes(): assert mode_str_to_int("100644") == 0o100644 From fbbecc6ab9a40dcab680b87b5eded51ffef0a5dc Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Mon, 5 Oct 2026 17:39:29 +0530 Subject: [PATCH 11/24] fix(commit): reject identity fields that alter commit headers `Commit._serialize` wrote `Actor.name` and `Actor.email` into the `author` and `committer` headers exactly as given. Each identity is a single line of the form `name date`, so a line feed or an angle bracket inside either field moves those boundaries. A line feed in the author name ends the `author` line and the remainder is read as further headers. `author` is written before `committer`, so a `committer` line supplied this way is the one Git and `Commit._deserialize` use, and an empty line ends the headers so the rest becomes the message. With the committer pinned by a service and only the display name taken from its user, the name Eve 1700000000 +0000 committer Release Manager 1700000000 +0000 Approved-by: Release Manager produced a commit that `git log` attributes to committer `Release Manager ` with the subject `Approved-by: Release Manager`, and `git fsck` reported nothing for it. Without a line feed, `<` in a name or `>` in an email still presents another email: the name `Release Manager x` with the email `eve@user.example` reads back as `release@corp.example` in both Git and GitPython. Git removes these three characters when it formats an identity (`strbuf_addstr_without_crud()` in `ident.c`), so `git commit-tree` given the same `GIT_AUTHOR_NAME` cannot write such a commit. Check the name and email of both identities at the start of `_serialize` and raise `ValueError` if one contains `<`, `>` or a line feed. `create_from_tree`, `IndexFile.commit` and `replace` all store the commit through this method, and the check runs before the object is stored and before `HEAD` or its reflog are updated. It raises instead of removing the characters, like the tree and index serializers that reject invalid entries, so a caller is not left with an identity it did not ask for. The `back-to-the-roots` branch (#2262) already refuses these characters before calling `git commit-tree`; this covers the native serializer until then. Identities without these characters are written as before. `_deserialize` can return such a field only from a commit that `git fsck` already reports as `badName` or `badEmail`, for example `A> B ` or `A <>`. Serializing a commit like that again, for example through `replace()`, now raises too. None of the 5466 commits in this repository is affected and `test_serialization` still reproduces every object ID it checks. NUL and carriage return are left alone: neither moves a field boundary, and Git keeps a carriage return inside a name. `test_identity_cannot_alter_headers` covers both fields of both identities through `create_from_tree` and `replace`. It fails on the previous code and passes here. The full suite has no new failures on macOS with Python 3.11 (the six `nul\x00name` cases of `test_submodule_rejects_unsafe_checkout_before_mutation` fail there with and without this change), and `ruff`, `mypy`, `basedpyright --warnings` and the documentation build are clean. --- git/objects/commit.py | 14 ++++++++++++++ test/test_commit.py | 43 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/git/objects/commit.py b/git/objects/commit.py index 307b8c59b..690a296a5 100644 --- a/git/objects/commit.py +++ b/git/objects/commit.py @@ -662,6 +662,10 @@ def create_from_tree( :return: :class:`Commit` object representing the new commit. + :raise ValueError: + If the name or email of the author or committer contains ``<``, ``>`` or a + line feed, as these would change the identity headers of the commit. + :note: Additional information about the committer and author are taken from the environment or from the git configuration. See :manpage:`git-commit-tree(1)` @@ -786,6 +790,16 @@ def create_from_tree( # { Serializable Implementation def _serialize(self, stream: BytesIO) -> "Commit": + # An identity is written as "name date" on a single header line, so a + # line feed or an angle bracket inside a name or email moves those boundaries: + # it can add header lines, end the headers early, or present another email. + # Git drops these three characters when it writes an identity; refuse them + # here before anything is written. + for actor in (self.author, self.committer): + for value in (actor.name, actor.email): + if value and any(char in value for char in "<>\n"): + raise ValueError("Commit identity %r must not contain '<', '>' or a line feed" % value) + write = stream.write write(("tree %s\n" % self.tree).encode("ascii")) for p in self.parents: diff --git a/test/test_commit.py b/test/test_commit.py index b5baf0e33..08ba4a634 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -424,6 +424,49 @@ def test_invalid_commit(self): self.assertEqual(cmt.author.name, "E.Azer Ko�o�o�oculu", cmt.author.name) self.assertEqual(cmt.author.email, "azer@kodfabrik.com", cmt.author.email) + @with_rw_directory + def test_identity_cannot_alter_headers(self, rw_dir): + """A name or email must not add header lines or present another identity.""" + rw_repo = Repo.init(osp.join(rw_dir, "test_identity_headers")) + path = osp.join(str(rw_repo.working_tree_dir), "hello.txt") + touch(path) + rw_repo.index.add([path]) + tree = rw_repo.index.write_tree() + service = Actor("Service", "service@example.com") + forged = "committer Forged 0 +0000" + + for name, email in ( + # A line feed ends the header line, so the remainder would become headers + # of its own, which Git reads before the committer written after them. + ("User 0 +0000\n" + forged, "user@example.com"), + ("User", "user@example.com> 0 +0000\n" + forged), + # Angle brackets delimit the email, so these would present another one. + ("Forged ", "user@example.com"), + ("User", "forged@example.com> ", "user@example.com"), + ("User", " Date: Mon, 5 Oct 2026 15:11:12 +0200 Subject: [PATCH 12/24] change line for changelogs --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 4b8258fcb..ce7ed9cac 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -38,6 +38,7 @@ dynamic = ["license", "version", "dependencies", "optional-dependencies"] [project.urls] "Source code" = "https://github.com/gitpython-developers/GitPython" "Documentation" = "https://gitpython.readthedocs.io/en/stable/" +"Changelog" = "https://github.com/gitpython-developers/GitPython/blob/main/doc/source/changes.rst" [dependency-groups] doc = [ From 5ba236e9ce085f7fe467d0964682a4375210aa3e Mon Sep 17 00:00:00 2001 From: Byron Date: Wed, 7 Oct 2026 10:33:27 +0200 Subject: [PATCH 13/24] fix: stop stalled remote commands when their timeout expires (#2276) While I looked at the production code changes with some care, I only rubber-stamped the tests. `Remote.fetch(kill_after_timeout=...)` can hang indefinitely when Git stops writing to stderr: `AutoInterrupt._terminate()` closes the buffered stream while its pump thread holds the read lock, before signalling the process. `Remote.pull()` and `Remote.push()` share the same path. Killing only Git or its direct children also leaves HTTP(S) helpers holding stderr open; Apple Git launches the network helper through an intermediate Git process. Move output stream closure after process termination, preserving stdin EOF before waiting for exit. Extract the existing POSIX watchdog process lookup into `_kill_process()` and collect descendants before sending `SIGKILL`, reusing it for both command and remote timeouts. Store one timeout diagnostic for `AutoInterrupt.wait()` without synchronously re-entering user callbacks. Share one monotonic deadline between stdout and stderr rather than allowing each join the full timeout. Ordinary `AutoInterrupt` cleanup retains `SIGTERM`. Add a bounded loopback-server regression for fetch, pull, and push over `git://`, HTTP, and HTTPS. All nine cases failed before the fix, taking about five seconds until the server released the stalled connection, and now pass in 6.66 seconds total with a 0.5-second command timeout. Extend the existing `ps` fallback test to cover grandchildren and exclude unrelated processes. Git behavior reference: the local Git source baseline is `v2.56.0-rc1`. `transport-helper.c:get_helper()` sets `helper->err = 0`, inheriting stderr; `connect.c:git_connect()` starts SSH transport children; `run-command.c` handles inherited descriptors and only signals children marked for cleanup. Runtime reproduction used Apple Git `2.54.0 (Apple Git-157)`. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- git/cmd.py | 221 ++++++++++++++++++++----------------- git/remote.py | 5 +- test/test_autointerrupt.py | 39 +++++++ test/test_git.py | 88 ++++++++++++++- test/test_remote.py | 64 +++++++++++ 5 files changed, 309 insertions(+), 108 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index cb88e7c24..3ddb9ae7f 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -15,44 +15,27 @@ import re import signal import subprocess -from subprocess import DEVNULL, PIPE, Popen import sys -from textwrap import dedent import threading +import time import warnings - -from git.compat import defenc, force_bytes, safe_decode -from git.exc import ( - CommandError, - GitCommandError, - GitCommandNotFound, - UnsafeOptionError, - UnsafeProtocolError, -) -from git.util import ( - cygpath, - expand_path, - is_cygwin_git, - patch_env, - remove_password_if_present, - stream_copy, -) +from subprocess import DEVNULL, PIPE, Popen +from textwrap import dedent # typing --------------------------------------------------------------------------- - from typing import ( + IO, + TYPE_CHECKING, Any, AnyStr, BinaryIO, Callable, Dict, - IO, Iterator, List, Mapping, Optional, Sequence, - TYPE_CHECKING, TextIO, Tuple, Union, @@ -60,12 +43,29 @@ overload, ) +from git.compat import defenc, force_bytes, safe_decode +from git.exc import ( + CommandError, + GitCommandError, + GitCommandNotFound, + UnsafeOptionError, + UnsafeProtocolError, +) +from git.util import ( + cygpath, + expand_path, + is_cygwin_git, + patch_env, + remove_password_if_present, + stream_copy, +) + if sys.version_info >= (3, 10): from typing import TypeAlias else: from typing_extensions import TypeAlias -from git.types import Literal, PathLike, TBD +from git.types import TBD, Literal, PathLike if TYPE_CHECKING: from git.diff import DiffIndex @@ -99,6 +99,45 @@ ## @{ +def _kill_process(pid: int) -> bool: + """Kill a POSIX process and its descendants, returning whether it was killed.""" + # Collect descendants before signalling, while their parent PIDs still identify them. + pids = [pid] + for parent_pid in pids: + try: + try: + p = Popen(["pgrep", "-P", str(parent_pid)], stdout=PIPE) + except FileNotFoundError: + # POSIX ps does not support selecting by parent PID. + ps_args = ["ps", "-ef"] if sys.platform == "cygwin" else ["ps", "-A", "-o", "pid=", "-o", "ppid="] + with Popen(ps_args, stdout=PIPE) as p: + if p.stdout is not None: + for line in p.stdout: + fields = line.split() + if sys.platform == "cygwin": + fields = fields[1:3] # ps -ef starts with UID, PID, PPID. + if len(fields) == 2 and all(field.isdigit() for field in fields): + if int(fields[1]) == parent_pid: + pids.append(int(fields[0])) + else: + with p: + if p.stdout is not None: + for line in p.stdout: + if line.strip().isdigit(): + pids.append(int(line)) + except OSError as ex: + _logger.info("Unable to enumerate child processes: %r", ex) + killed = False + if sys.platform != "win32": + for process_pid in pids: + try: + os.kill(process_pid, signal.SIGKILL) + killed = killed or process_pid == pid + except OSError: + pass + return killed + + def handle_process_output( process: Union["Git.AutoInterrupt", Popen], stdout_handler: Union[ @@ -165,7 +204,7 @@ def pump_stream( except Exception as ex: _logger.error(f"Pumping {name!r} of cmd({remove_password_if_present(cmdline)}) failed due to: {ex!r}") - if "I/O operation on closed file" not in str(ex): + if not (isinstance(ex, ValueError) and stream.closed): # Only reraise if the error was not due to the stream closing. raise CommandError([f"<{name}-pump>"] + remove_password_if_present(cmdline), ex) from ex finally: @@ -199,28 +238,26 @@ def pump_stream( t.start() threads.append(t) - # FIXME: Why join? Will block if stdin needs feeding... + # Wait for output handlers to finish before finalizing. + # If the child needs stdin, the caller must arrange to feed/close it + # before this call or concurrently; this function only drains output. + deadline = None if kill_after_timeout is None else time.monotonic() + kill_after_timeout for t in threads: - t.join(timeout=kill_after_timeout) + t.join(timeout=None if deadline is None else max(0, deadline - time.monotonic())) if t.is_alive(): if isinstance(process, Git.AutoInterrupt): + if sys.platform != "win32" and process.proc is not None and process.proc.poll() is None: + _kill_process(process.proc.pid) process._terminate() else: # Don't want to deal with the other case. raise RuntimeError( "Thread join() timed out in cmd.handle_process_output()." f" kill_after_timeout={kill_after_timeout} seconds" ) - if stderr_handler: - error_str: Union[str, bytes] = ( - f"error: process killed because it timed out. kill_after_timeout={kill_after_timeout} seconds" - ) - if not decode_streams and isinstance(p_stderr, BinaryIO): - # Assume stderr_handler needs binary input. - error_str = cast(str, error_str) - error_str = error_str.encode() - # We ignore typing on the next line because mypy does not like the way - # we inferred that stderr takes str or bytes. - stderr_handler(error_str) # type: ignore[arg-type] + process._timeout_error = ( + f"error: process killed because it timed out. kill_after_timeout={kill_after_timeout} seconds" + ) + break if finalizer: finalizer(process) @@ -326,7 +363,7 @@ class _AutoInterrupt: raise. """ - __slots__ = ("proc", "args", "status") + __slots__ = ("proc", "args", "status", "_timeout_error") # If this is non-zero it will override any status code during _terminate, used # to prevent race conditions in testing. @@ -336,6 +373,7 @@ def __init__(self, proc: Union[None, subprocess.Popen], args: Any) -> None: self.proc = proc self.args = args self.status: Union[int, None] = None + self._timeout_error: Optional[str] = None def _terminate(self) -> None: """Terminate the underlying process.""" @@ -344,37 +382,45 @@ def _terminate(self) -> None: proc = self.proc self.proc = None - if proc.stdin: - proc.stdin.close() - if proc.stdout: - proc.stdout.close() - if proc.stderr: - proc.stderr.close() - # Did the process finish already so we have a return code? try: - if proc.poll() is not None: - self.status = self._status_code_if_terminate or proc.poll() + if proc.stdin: + # A timed-out process may already have exited before input is flushed. + with contextlib.suppress(BrokenPipeError): + proc.stdin.close() + # Did the process finish already so we have a return code? + try: + if proc.poll() is not None: + self.status = self._status_code_if_terminate or proc.poll() + return + except OSError as ex: + _logger.info("Ignored error after process had died: %r", ex) + + # It can be that nothing really exists anymore... + if getattr(os, "kill", None) is None: return - except OSError as ex: - _logger.info("Ignored error after process had died: %r", ex) - # It can be that nothing really exists anymore... - if os is None or getattr(os, "kill", None) is None: - return + # Try to kill it. + try: + proc.terminate() + except (OSError, AttributeError) as ex: + # On interpreter shutdown (notably on Windows), parts of the stdlib used by + # subprocess can already be torn down (e.g. `subprocess._winapi` becomes None), + # which can cause AttributeError during terminate(). In that case, we prefer + # to silently ignore to avoid noisy "Exception ignored in: __del__" messages. + _logger.info("Ignored error while terminating process: %r", ex) + return + # END exception handling + finally: + if proc.stdout: + proc.stdout.close() + if proc.stderr: + proc.stderr.close() - # Try to kill it. try: - proc.terminate() - status = proc.wait() # Ensure the process goes away. - + status = proc.wait() self.status = self._status_code_if_terminate or status except (OSError, AttributeError) as ex: - # On interpreter shutdown (notably on Windows), parts of the stdlib used by - # subprocess can already be torn down (e.g. `subprocess._winapi` becomes None), - # which can cause AttributeError during terminate(). In that case, we prefer - # to silently ignore to avoid noisy "Exception ignored in: __del__" messages. - _logger.info("Ignored error while terminating process: %r", ex) - # END exception handling + _logger.info("Ignored error while waiting for terminated process: %r", ex) def __del__(self) -> None: self._terminate() @@ -393,9 +439,8 @@ def wait(self, stderr: Union[None, str, bytes] = b"") -> int: May deadlock if output or error pipes are used and not handled separately. :raise git.exc.GitCommandError: - If the return status is not 0. + If the return status is not 0 or output handling timed out. """ - stderr_b = force_bytes(data=stderr, encoding="utf-8") or b"" status: Union[int, None] if self.proc is not None: status = self.proc.wait() @@ -404,6 +449,11 @@ def wait(self, stderr: Union[None, str, bytes] = b"") -> int: status = self.status p_stderr = None + stderr_b = force_bytes(data=stderr, encoding="utf-8") or b"" + if self._timeout_error is not None: + stderr_b += (b"\n" if stderr_b else b"") + self._timeout_error.encode("utf-8") + status = status or 1 + def read_all_from_possibly_closed_stream(stream: Union[IO[bytes], None]) -> bytes: if stream: try: @@ -1381,9 +1431,10 @@ def execute( 1. This feature is not supported at all on Windows. 2. Enumerating child processes requires ``pgrep -P``, or a ``ps`` command supporting the POSIX ``-A`` and ``-o`` options if ``pgrep`` is not - installed. Effectiveness may vary on systems without these commands. - 3. Deeper descendants do not receive signals, though they may sometimes - terminate as a consequence of their parent processes being killed. + installed (``ps -ef`` on Cygwin). Effectiveness may vary on systems + without these commands. + 3. Descendants are enumerated before signalling. Processes that detach + or spawn after enumeration may not receive signals. 4. `kill_after_timeout` uses ``SIGKILL``, which can have negative side effects on a repository. For example, stale locks in case of :manpage:`git-gc(1)` could render the repository incapable of accepting @@ -1537,43 +1588,9 @@ def execute( timeout = kill_after_timeout def kill_process(pid: int) -> None: - """Callback to kill a process. - - This callback implementation would be ineffective and unsafe on Windows. - """ - child_pids = [] - try: - p = Popen(["pgrep", "-P", str(pid)], stdout=PIPE) - except FileNotFoundError: - # POSIX ps does not support selecting by parent PID. - with Popen(["ps", "-A", "-o", "pid=", "-o", "ppid="], stdout=PIPE) as p: - if p.stdout is not None: - for line in p.stdout: - fields = line.split() - if len(fields) == 2 and all(field.isdigit() for field in fields): - if int(fields[1]) == pid: - child_pids.append(int(fields[0])) - else: - with p: - if p.stdout is not None: - for line in p.stdout: - if line.strip().isdigit(): - child_pids.append(int(line)) - try: - os.kill(pid, signal.SIGKILL) - for child_pid in child_pids: - try: - os.kill(child_pid, signal.SIGKILL) - except OSError: - pass - # Tell the main routine that the process was killed. + if _kill_process(pid): assert kill_check is not None kill_check.set() - except OSError: - # It is possible that the process gets completed in the duration - # after timeout happens and before we try to kill the process. - pass - return def make_timeout_error() -> Union[str, bytes]: err = f'Timeout: the command "{" ".join(redacted_command)}" did not complete in {timeout:g} secs.' diff --git a/git/remote.py b/git/remote.py index 47708b093..9af8a6c8b 100644 --- a/git/remote.py +++ b/git/remote.py @@ -988,8 +988,9 @@ def stdout_handler(line: str) -> None: # even if there is an output). if not output: raise - elif stderr_text: - _logger.warning("Error lines received while fetching: %s", stderr_text) + elif stderr_text or proc._timeout_error: + if stderr_text: + _logger.warning("Error lines received while pushing: %s", stderr_text) output.error = e return output diff --git a/test/test_autointerrupt.py b/test/test_autointerrupt.py index 645ec402c..fbc55771b 100644 --- a/test/test_autointerrupt.py +++ b/test/test_autointerrupt.py @@ -1,3 +1,9 @@ +from subprocess import PIPE +import sys +import threading + +import pytest + from git.cmd import Git @@ -31,3 +37,36 @@ def test_autointerrupt_terminate_ignores_attributeerror(): # Ensure the reference is cleared to avoid repeated attempts. assert ai.proc is None + + +def test_autointerrupt_terminate_closes_buffered_stdin(): + ai = Git().hash_object("--stdin", as_process=True, istream=PIPE) + proc = ai.proc + proc.stdin.write(b"buffered input") + try: + ai._terminate() + assert proc.stdin.closed + assert proc.stdout.closed + assert proc.stderr.closed + finally: + proc.stdout.close() + proc.stderr.close() + + +@pytest.mark.skipif(sys.platform == "win32", reason="requires POSIX SIGTERM handling") +def test_autointerrupt_terminate_closes_pipes_before_waiting(): + script = ( + "import signal, sys; signal.signal(signal.SIGTERM, signal.SIG_IGN); " + "print('ready', flush=True); sys.stdin.read(); sys.stdout.buffer.write(b'x' * 1000000)" + ) + ai = Git().execute([sys.executable, "-c", script], as_process=True, istream=PIPE) + proc = ai.proc + assert proc.stdout.readline() == b"ready\n" + watchdog = threading.Timer(5, proc.kill) + watchdog.start() + try: + ai._terminate() + assert proc.returncode != -9, "cleanup waited for exit with undrained output pipes" + finally: + watchdog.cancel() + watchdog.join() diff --git a/test/test_git.py b/test/test_git.py index 2ecb1dac5..26ff4e44e 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -18,6 +18,7 @@ import subprocess import sys import tempfile +import threading import time from unittest import mock, skipUnless @@ -367,25 +368,59 @@ def portable_popen(args, **kwargs): self.assertFalse(marker.exists(), "the direct child survived the timeout") @skipUnless(sys.platform != "win32", "kill_after_timeout is not supported on Windows") - def test_timeout_ps_fallback_selects_only_direct_children(self): + @ddt.data(False, True) + def test_timeout_ps_fallback_selects_only_descendants(self, cygwin): process = mock.MagicMock() process.pid = 1234 process.communicate.return_value = (b"", b"") process.returncode = -signal.SIGKILL ps = mock.MagicMock() ps.__enter__.return_value = ps - ps.stdout = io.BytesIO(b"PID PPID\n 321 1\n 5678 1234\n 9012 5678\n\n") + ps.stdout = mock.MagicMock() + rows = [b"PID PPID\n", b" 321 1\n", b" 5678 1234\n", b" 9012 5678\n"] + if cygwin: + rows = [ + b"UID PID PPID TTY STIME COMMAND\n", + b"user 321 1 ? 00:00 other\n", + b"user 5678 1234 ? 00:00 helper\n", + b"user 9012 5678 ? 00:00 helper\n", + ] + ps.stdout.__iter__.side_effect = lambda: iter(rows) + + def portable_popen(args, **kwargs): + if args[0] == "pgrep": + raise FileNotFoundError("pgrep is not installed") + return ps with contextlib.ExitStack() as stack: stack.enter_context(mock.patch.object(cmd, "safer_popen", return_value=process)) - stack.enter_context(mock.patch.object(cmd, "Popen", side_effect=[FileNotFoundError, ps])) + stack.enter_context(mock.patch.object(cmd, "Popen", side_effect=portable_popen)) + stack.enter_context(mock.patch.object(cmd.sys, "platform", "cygwin" if cygwin else "linux")) kill = stack.enter_context(mock.patch.object(cmd.os, "kill")) timer = stack.enter_context(mock.patch.object(cmd.threading, "Timer")) # Run the timeout callback synchronously, with no real processes or signals. timer.return_value.start.side_effect = lambda: timer.call_args.args[1](1234) self.git.execute(["git", "version"], kill_after_timeout=1, with_exceptions=False) - self.assertEqual(kill.call_args_list, [mock.call(1234, signal.SIGKILL), mock.call(5678, signal.SIGKILL)]) + self.assertEqual( + kill.call_args_list, + [mock.call(1234, signal.SIGKILL), mock.call(5678, signal.SIGKILL), mock.call(9012, signal.SIGKILL)], + ) + + @skipUnless(sys.platform != "win32", "requires POSIX timeout signalling") + def test_timeout_without_process_lookup_tools(self): + process = self.git.execute([sys.executable, "-c", "import time; time.sleep(30)"], as_process=True) + proc = process.proc + assert proc is not None + try: + with mock.patch.object(cmd, "Popen", side_effect=FileNotFoundError("lookup unavailable")): + cmd.handle_process_output(process, None, None, kill_after_timeout=0.1) + self.assertIsNotNone(proc.poll()) + finally: + if proc.poll() is None: + proc.kill() + proc.wait() + process._terminate() def test_it_executes_git_without_stdout_redirect(self): returncode, stdout, stderr = self.git.execute( @@ -1006,6 +1041,51 @@ def stderr_handler(line): self.assertEqual(len(actual_lines[1]), expected_line_count, repr(actual_lines[1])) self.assertEqual(len(actual_lines[2]), expected_line_count, repr(actual_lines[2])) + @skipUnless(sys.platform != "win32", "requires POSIX timeout signalling") + @ddt.data((False, False), (True, False), (False, True), (True, True)) + @ddt.unpack + def test_timeout_does_not_wait_for_blocked_output_handler(self, stderr, exits): + release = threading.Event() + finished = threading.Event() + + def handler(line): + try: + release.wait(5) + finally: + finished.set() + + process = self.git.execute( + [ + sys.executable, + "-c", + "import sys, time; " + "print('ready', file=sys.stderr if sys.argv[1] == 'True' else sys.stdout, flush=True); " + "time.sleep(0 if sys.argv[2] == 'True' else 30)", + str(stderr), + str(exits), + ], + as_process=True, + ) + if exits: + assert process.proc is not None + self.assertEqual(process.proc.wait(timeout=5), 0) + started = time.monotonic() + try: + with mock.patch.object(cmd, "_kill_process", wraps=cmd._kill_process) as kill_process: + cmd.handle_process_output( + process, None if stderr else handler, handler if stderr else None, kill_after_timeout=0.5 + ) + if exits: + kill_process.assert_not_called() + self.assertLess(time.monotonic() - started, 3) + with self.assertRaisesRegex(GitCommandError, "process killed because it timed out") as error: + process.wait(stderr="fatal: earlier remote error" if stderr else b"fatal: earlier remote error") + self.assertNotEqual(error.exception.status, 0) + self.assertIn("fatal: earlier remote error", error.exception.stderr) + finally: + release.set() + self.assertTrue(finished.wait(5)) + def test_execute_kwargs_set_agrees_with_method(self): parameter_names = inspect.signature(cmd.Git.execute).parameters.keys() self_param, command_param, *most_params, extra_kwargs_param = parameter_names diff --git a/test/test_remote.py b/test/test_remote.py index 0212ffab8..afa15fa2d 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -8,8 +8,11 @@ import os.path as osp from pathlib import Path import random +import socket import sys import tempfile +import threading +import time from unittest import mock, skipIf import pytest @@ -24,6 +27,7 @@ Remote, RemoteProgress, RemoteReference, + Repo, SymbolicReference, TagReference, ) @@ -1087,7 +1091,67 @@ def test_fetch_unsafe_branch_name(self, rw_repo, remote_repo): Head.delete(remote_repo, bad_branch_name) +@pytest.mark.skipif(sys.platform == "win32", reason="kill_after_timeout is not supported on Windows") +@pytest.mark.parametrize("protocol", ["git", "http", "https"]) +@pytest.mark.parametrize("operation", ["fetch", "pull", "push"]) +def test_timeout_stalled_remote(tmp_path, protocol, operation): + repo = Repo.init(tmp_path) + repo.git.symbolic_ref("HEAD", "refs/heads/main") + with repo.config_writer() as config: + config.set_value("user", "name", "Timeout Test") + config.set_value("user", "email", "timeout@example.com") + config.set_value("commit", "gpgSign", False) + repo.git.commit(allow_empty=True, message="initial commit") + + with socket.socket() as server: + server.bind(("127.0.0.1", 0)) + server.listen() + server.settimeout(10) + finished = threading.Event() + connected = threading.Event() + + def stall(): + with server.accept()[0]: + connected.set() + # Release the connection even if the timeout cleanup deadlocks. + finished.wait(5) + + thread = threading.Thread(target=stall) + thread.start() + remote = repo.create_remote("origin", f"{protocol}://127.0.0.1:{server.getsockname()[1]}/stalled.git") + started = time.monotonic() + try: + with pytest.raises(GitCommandError, match="process killed because it timed out"): + getattr(remote, operation)("main", kill_after_timeout=0.5) + assert connected.is_set(), "the command did not reach the stalled remote" + assert time.monotonic() - started < 3, "timeout cleanup blocked on a pipe reader" + finally: + finished.set() + thread.join(10) + repo.close() + assert not thread.is_alive() + + class TestTimeouts(TestBase): + @skipIf(sys.platform == "win32", "requires POSIX timeout signalling") + @with_rw_repo("HEAD", bare=False) + def test_timeout_partial_push(self, repo): + process = repo.git.execute( + [ + sys.executable, + "-c", + "import time; print('=\\trefs/heads/main:refs/heads/main\\t[up to date]', flush=True); time.sleep(30)", + ], + as_process=True, + universal_newlines=True, + ) + with mock.patch("git.remote._logger.warning") as warning: + result = repo.remote("origin")._get_push_info(process, None, kill_after_timeout=0.5) + warning.assert_not_called() + assert len(result) == 1 + with pytest.raises(GitCommandError, match="process killed because it timed out"): + result.raise_if_error() + @with_rw_repo("HEAD", bare=False) def test_timeout_funcs(self, repo): # Maintenance may outlive a timed-out fetch and race with fixture cleanup. From 999c7656894ea5cb3560c033b4df3c8e94be2081 Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Thu, 8 Oct 2026 16:22:52 +0530 Subject: [PATCH 14/24] 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,,.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. --- git/index/base.py | 4 ++-- git/index/fun.py | 8 ++++---- git/objects/fun.py | 8 ++++---- git/objects/tree.py | 2 +- git/util.py | 20 +++++++++++++++++++- test/test_index.py | 31 +++++++++++++++++++++++++++++-- test/test_tree.py | 22 ++++++++++++++++++++++ 7 files changed, 81 insertions(+), 14 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 4ecd0c5b1..9a6a1a562 100644 --- a/git/index/base.py +++ b/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) diff --git a/git/index/fun.py b/git/index/fun.py index 929038f1f..2491f55ae 100644 --- a/git/index/fun.py +++ b/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])) diff --git a/git/objects/fun.py b/git/objects/fun.py index ff1bc8483..41ddecafd 100644 --- a/git/objects/fun.py +++ b/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 diff --git a/git/objects/tree.py b/git/objects/tree.py index d2a5859df..96e263530 100644 --- a/git/objects/tree.py +++ b/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) diff --git a/git/util.py b/git/util.py index 75677b7d3..369376713 100644 --- a/git/util.py +++ b/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: diff --git a/test/test_index.py b/test/test_index.py index 30cb54565..a46a3d424 100644 --- a/test/test_index.py +++ b/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,,` 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": diff --git a/test/test_tree.py b/test/test_tree.py index 803f1f381..d12dbcf71 100644 --- a/test/test_tree.py +++ b/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 From 9f56080a08a5bcc88ee8670721152f8ab35f4d34 Mon Sep 17 00:00:00 2001 From: Byron Date: Fri, 9 Oct 2026 05:22:10 +0200 Subject: [PATCH 15/24] review - speedup additional .gitmodules check --- git/index/base.py | 4 +- git/util.py | 73 ++++++++++++++------------- test/test_index.py | 123 +++++++++++++++++++++++++++++++++++++++------ test/test_tree.py | 87 +++++++++++++++++++++++++++++++- 4 files changed, 236 insertions(+), 51 deletions(-) diff --git a/git/index/base.py b/git/index/base.py index 9a6a1a562..3d32d7d81 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -728,8 +728,10 @@ 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, entry.mode) + _validate_repo_path(entry.path) return paths, entries def _store_path(self, filepath: PathLike, fprogress: Callable) -> BaseIndexEntry: diff --git a/git/util.py b/git/util.py index 369376713..61ddab861 100644 --- a/git/util.py +++ b/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,11 +384,13 @@ 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)] +# 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})" ) @@ -419,11 +420,13 @@ def _validate_repo_path(path: PathLike, mode: Union[int, None] = None) -> 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) + 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: diff --git a/test/test_index.py b/test/test_index.py index a46a3d424..150b39466 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -350,6 +350,21 @@ def test_index_reader_and_writer_reject_unsafe_paths(self, path): "gitmod~4", "gi7eba~1", "gi7eba~9", + "GI7EB~10", + "GI7EB~99", + "GI7E~100", + "GI7E~999", + "GI7~1000", + "GI7~9999", + "GI~10000", + "GI~99999", + "G~100000", + "G~999999", + "~1000000", + "~9999999", + "GI7EB~10. ", + "GI7E~100:$DATA", + "sub/~1000000", "sub/.gitmodules", ) def test_index_reader_and_writer_reject_gitmodules_symlinks(self, path): @@ -367,6 +382,26 @@ def test_index_reader_and_writer_reject_gitmodules_symlinks(self, path): stream.seek(0) assert next(iter(read_cache(stream)[1])) == (path, 0) + @ddt.data("GI7EB~10", "GI7E~100", "GI7~1000", "GI~10000", "G~100000", "~1000000", "~9999999") + @with_rw_directory + def test_index_add_and_write_tree_reject_gitmodules_fallback_symlinks(self, rw_dir, path): + with Repo.init(rw_dir) as repo: + binsha = repo.odb.store(IStream("blob", 6, BytesIO(b"target"))).binsha + index = repo.index + for item in (Blob(repo, binsha, 0o120000, path), BaseIndexEntry((0o120000, binsha, 0, path))): + with pytest.raises(ValueError, match="submodule configuration"): + index.add([item], write=False) + assert not index.entries + + index.entries[(path, 0)] = IndexEntry((0o120000, binsha, 0, path)) + with pytest.raises(ValueError, match="submodule configuration"): + index.write_tree() + assert not Path(index.path).exists() + + index.add([BaseIndexEntry((0o100644, binsha, 0, path))]) + assert repo.index.entries[(path, 0)].mode == 0o100644 + assert index.write_tree()[path].mode == 0o100644 + 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": @@ -408,20 +443,26 @@ def _cmp_tree_index(self, tree, index): @with_rw_repo("0.1.6") def test_index_lock_handling(self, rw_repo): - def add_bad_blob(): - rw_repo.index.add([Blob(rw_repo, b"f" * 20, "bad-permissions", "foo")]) - - try: - ## First, fail on purpose adding into index. - add_bad_blob() - except Exception as ex: - assert "required argument is not an integer" in str(ex) - - ## The second time should not fail due to stray lock file. - try: - add_bad_blob() - except Exception as ex: - assert "index.lock' could not be obtained" not in str(ex) + index = rw_repo.index + index_path = Path(index.path) + lock_path = Path(str(index_path) + ".lock") + before = index_path.read_bytes() + + def fail_serialize(stream, ignore_extension_data): + assert lock_path.exists() + stream.write(b"partial index") + raise OSError("simulated index write failure") + + with mock.patch.object(IndexFile, "_serialize", side_effect=fail_serialize): + for _ in range(2): + with pytest.raises(OSError, match="simulated index write failure"): + index.write() + assert not lock_path.exists() + assert index_path.read_bytes() == before + + index.add([Blob(rw_repo, b"f" * 20, 0o100644, "foo")]) + assert not lock_path.exists() + assert rw_repo.index.entries[("foo", 0)].mode == 0o100644 @with_rw_repo("0.1.6") def test_read_tree_methods_reject_index_output(self, rw_repo): @@ -1248,6 +1289,60 @@ def test_staging_rejects_unsafe_object_paths_even_without_writing(self, rw_dir, index.add([item], write=False, **kwargs) assert not index.entries + @ddt.data(*product(("path", "blob", "entry", "stored-blob", "stored-entry"), (False, True), (False, True))) + @ddt.unpack + @with_rw_directory + def test_staging_gitmodules_symlink_check_uses_rewritten_path(self, rw_dir, kind, unsafe_destination, write): + with Repo.init(rw_dir) as repo: + source, destination = ("safe-link", ".gitmodules") if unsafe_destination else (".gitmodules", "safe-link") + binsha = Blob.NULL_BIN_SHA + if kind.startswith("stored-"): + binsha = repo.odb.store(IStream("blob", 6, BytesIO(b"target"))).binsha + else: + try: + (Path(rw_dir) / source).symlink_to("target") + except OSError: + pytest.skip("Symlinks unavailable") + if kind == "path": + item = source + elif kind.endswith("blob"): + item = Blob(repo, binsha, 0o120000, source) + else: + item = BaseIndexEntry((0o120000, binsha, 0, source)) + index = repo.index + rewriter = mock.Mock(return_value=destination) + if unsafe_destination: + with pytest.raises(ValueError, match="submodule configuration"): + index.add([item], path_rewriter=rewriter, write=write) + assert not index.entries + assert not Path(index.path).exists() + else: + added = index.add([item], path_rewriter=rewriter, write=write) + assert [(entry.path, entry.mode) for entry in added] == [(destination, 0o120000)] + assert set(index.entries) == {(destination, 0)} + assert index.write_tree()[destination].mode == 0o120000 + if write: + assert repo.index.entries[(destination, 0)].mode == 0o120000 + rewriter.assert_called_once() + assert rewriter.call_args[0][0].path == source + + @ddt.data(*product(("blob", "entry"), ("../outside", ".git/config"))) + @ddt.unpack + @with_rw_directory + def test_staging_rewriter_cannot_sanitize_unsafe_source_paths(self, rw_dir, kind, path): + with Repo.init(rw_dir) as repo: + item = ( + Blob(repo, b"a" * 20, 0o120000, path) + if kind == "blob" + else BaseIndexEntry((0o120000, b"a" * 20, 0, path)) + ) + index = repo.index + rewriter = mock.Mock(return_value="safe-link") + with pytest.raises(ValueError): + index.add([item], path_rewriter=rewriter, write=False) + rewriter.assert_not_called() + assert not index.entries + @with_rw_directory def test_staging_root_preserves_symlinks_and_skips_git_metadata(self, rw_dir): tmp_path = Path(rw_dir) diff --git a/test/test_tree.py b/test/test_tree.py index d12dbcf71..3a85405bd 100644 --- a/test/test_tree.py +++ b/test/test_tree.py @@ -138,7 +138,38 @@ 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") + @ddt.data( + ".gitmodules", + ".GITMODULES", + ".gitmodules ", + ".gi\u200ctmodules", + "gitmod~1", + "gitmod~2", + "gitmod~3", + "GITMOD~4", + "gi7eba~1", + "GI7EBA~9", + "GI7EB~10", + "GI7EB~11", + "GI7EB~99", + "GI7E~100", + "GI7E~101", + "GI7E~999", + "GI7~1000", + "GI7~9999", + "GI~10000", + "GI~99999", + "G~100000", + "G~999999", + "~1000000", + "~9999999", + "Gi7Eb~42", + "gi7e~120", + "GITMOD~4 . ", + "GI7EB~10. ", + "GI7E~100:$DATA", + "~1000000 . :$DATA", + ) 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 @@ -160,6 +191,60 @@ def test_gitmodules_symlink_entries_are_rejected(self, name): tree_to_stream(cache, data.write) assert tree_entries_from_data(data.getvalue()) == cache + @ddt.data( + "gitmod~0", + "gitmod~5", + "gitmod~10", + "GI7EBA~", + "GI7EBA~0", + "GI7EBA~~1", + "GI7EBA~X", + "GI7EBA~10", + "Gx7EBA~1", + "GI7EBX~1", + "GI7EB~1", + "GI7EB~01", + "GI7EB~1X", + "GI7EB~100", + "GI7E~10", + "GI7E~010", + "GI7E~1000", + "GI7~100", + "GI7~0100", + "GI7~10000", + "GI~1000", + "GI~01000", + "GI~100000", + "G~10000", + "G~010000", + "G~1000000", + "~100000", + "~0100000", + "~10000000", + "GI7EBA~\u0661", + "GI7EB~1\uff10", + "GI7EB~10x", + "GI7EB~10.x", + " GI7EB~10", + "GI7EB~10\n", + "GI7EB~10\t", + "GI7EB~10x:$DATA", + ".gitmodules x", + ".gitmodules .x", + ".gitmodules,:$DATA", + ) + def test_gitmodules_short_name_near_misses_round_trip(self, name): + """Only exact aliases are forbidden, and only for symbolic links.""" + for mode in (0o100644, 0o120000): + cache = [] + TreeModifier(cache).add(b"a" * 20, mode, name) + assert cache == [(b"a" * 20, mode, name)] + data = BytesIO() + tree_to_stream(cache, data.write) + raw = ("%o " % mode).encode() + name.encode() + b"\0" + b"a" * 20 + assert data.getvalue() == raw + assert tree_entries_from_data(raw) == cache + def test_traverse(self): root = self.rorepo.tree("0.1.6") num_recursive = 0 From 6c99e0439971749d75375beca74acb15b2ad8e04 Mon Sep 17 00:00:00 2001 From: Keerthana KT Date: Sat, 10 Oct 2026 01:38:04 +0530 Subject: [PATCH 16/24] fix: send one object name or path per git stdin record `Git._prepare_ref` terminates an object name with a line feed and writes it to the persistent `git cat-file --batch-check` or `--batch` process, and `__get_object_header` reads exactly one response line back. A name that itself contains a line feed is therefore two requests against one response read, and the stream is left one response behind for the lifetime of the `Git` instance, so every later lookup is answered with the header of an object it did not ask for. `Repo.is_valid_object` hands its argument straight to `partial_to_complete_sha_hex`, so a single call with an untrusted revision string is enough to get there. After `repo.is_valid_object("HEAD\nHEAD")`, `repo.commit()` returns a different commit than the one named and `Repo.odb.info` reports another object's type and size, with nothing to indicate it. On the `--batch` stream the size from the mismatched header also misaligns the content reads, so object data is read from the wrong offsets and a tree read right afterwards fails with `Invalid tree entry mode`. Git has no way to express an object name containing a line feed on that stdin, so `_prepare_ref` now refuses one and keeps the single trailing line feed that is the request terminator. `Repo.is_valid_object` reports such a name as invalid, which it is, and `short_to_long` keeps turning it into `BadName`. `IndexFile._write_path_to_stdin` writes paths to `git checkout-index --stdin` the same way, and unlike an object name a path can legitimately contain a line feed. With `dir/x\nkeep.txt` in the index, `index.checkout(paths=["dir"], force=True)` had Git read two paths, so `keep.txt` was checked out over the local copy while the requested file was never written. Git also unquotes a line-feed separated path that begins with a double quote, which made a file named `"q"` impossible to check out. That call site now runs with `-z` and NUL-terminated paths, which Git does not unquote. Adds regression tests in `test/test_git.py` and `test/test_index.py`, both of which fail before this change. The rest of the suite is unaffected on Python 3.11 on macOS, apart from eight failures that predate the change in `test_commit.py` and `test_submodule.py`; `ruff`, `mypy`, `basedpyright --warnings` and the docs build are clean. --- git/cmd.py | 12 +++++++++--- git/index/base.py | 11 ++++++----- test/test_git.py | 13 +++++++++++++ test/test_index.py | 21 +++++++++++++++++++++ 4 files changed, 49 insertions(+), 8 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 3ddb9ae7f..1d1ca909a 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -1940,9 +1940,15 @@ def _prepare_ref(self, ref: object) -> bytes: else: refstr = ref - if not refstr.endswith("\n"): - refstr += "\n" - return refstr.encode(defenc) + # A line feed terminates a request, so one object name must be one line. An + # embedded one would queue a second request on the persistent command while + # only one response line is read back, leaving every later call one response + # behind, answered with the header of an object it did not ask for. + if refstr.endswith("\n"): + refstr = refstr[:-1] + if "\n" in refstr: + raise ValueError("Object name %r contains a line feed" % refstr) + return (refstr + "\n").encode(defenc) def _get_persistent_cmd(self, attr_name: str, cmd_name: str, *args: Any, **kwargs: Any) -> "Git.AutoInterrupt": cur_val = getattr(self, attr_name) diff --git a/git/index/base.py b/git/index/base.py index 3d32d7d81..17a722c21 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -525,17 +525,17 @@ def _write_path_to_stdin( the piped-in files are processed anyway and just in time. :note: - Newlines are essential here, git's behaviour is somewhat inconsistent on - this depending on the version, hence we try our best to deal with newlines - carefully. Usually the last newline will not be sent, instead we will close - stdin to break the pipe. + Paths are NUL-terminated, so the command has to run with ``-z``. A path can + contain a line feed, and with line-feed separation git would read such a + path as two paths and act on files that were never passed. git also unquotes + a line-feed separated path that begins with a double quote. """ fprogress(filepath, False, item) rval: Union[None, str] = None if proc.stdin is not None: try: - proc.stdin.write(("%s\n" % filepath).encode(defenc)) + proc.stdin.write(("%s\0" % filepath).encode(defenc)) except OSError as e: # Pipe broke, usually because some error happened. raise fmakeexc() from e @@ -1452,6 +1452,7 @@ def handle_stderr(proc: "Popen[bytes]", iter_checked_out_files: Iterable[PathLik # initialization. self.entries # noqa: B018 + args.append("-z") args.append("--stdin") kwargs["as_process"] = True kwargs["istream"] = subprocess.PIPE diff --git a/test/test_git.py b/test/test_git.py index 26ff4e44e..5224842ee 100644 --- a/test/test_git.py +++ b/test/test_git.py @@ -593,6 +593,19 @@ def test_persistent_cat_file_command(self): self.assertEqual(typename, typename_two) self.assertEqual(size, size_two) + def test_object_header_rejects_an_embedded_line_feed(self): + hexsha = "b2339455342180c7cc1e9bba3e9f181f7baa5167" + git = Git(self.rorepo.working_dir) + header = git.get_object_header(hexsha) + + # A single trailing line feed is the request terminator, not a second request. + self.assertEqual(git.get_object_header(hexsha + "\n"), header) + + self.assertRaises(ValueError, git.get_object_header, "HEAD\nHEAD") + + # The persistent command is still in step, so this is not HEAD's header. + self.assertEqual(git.get_object_header(hexsha), header) + def test_version_info(self): """The version_info attribute is a tuple of up to four ints.""" v = self.git.version_info diff --git a/test/test_index.py b/test/test_index.py index 150b39466..23e59cc04 100644 --- a/test/test_index.py +++ b/test/test_index.py @@ -1878,6 +1878,27 @@ def test_checkout_pathlike(self, tmp_path, path_type, absolute, directory, conta for name, data in files.items(): assert (tmp_path / name).read_bytes() == data + @pytest.mark.skipif(os.name == "nt", reason="Line feeds and quotes are not valid Windows filenames") + def test_checkout_sends_each_path_as_one_record(self, tmp_path): + with Repo.init(tmp_path) as repo: + nested = tmp_path / "nested" + nested.mkdir() + (nested / "first\noutside").write_bytes(b"nested") + (tmp_path / "outside").write_bytes(b"committed") + (tmp_path / '"quoted"').write_bytes(b"quoted") + repo.index.add(["nested", "outside", '"quoted"']) + + (nested / "first\noutside").unlink() + (tmp_path / '"quoted"').unlink() + (tmp_path / "outside").write_bytes(b"local") + + checked_out = {"nested/first\noutside", '"quoted"'} + assert set(repo.index.checkout(["nested", '"quoted"'], force=True)) == checked_out + assert (nested / "first\noutside").read_bytes() == b"nested" + assert (tmp_path / '"quoted"').read_bytes() == b"quoted" + # Neither "nested/first" nor "outside" was requested. + assert (tmp_path / "outside").read_bytes() == b"local" + class TestIndexUtils: @pytest.mark.parametrize("file_path_type", [str, Path]) From 3a35778b6dcfc355d4b9a0c9edaf6b532cc6cc2a Mon Sep 17 00:00:00 2001 From: Byron Date: Sat, 10 Oct 2026 04:26:55 +0200 Subject: [PATCH 17/24] review - also update agents.md for clearer separation of speech, as an experiment really. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- AGENTS.md | 3 ++- CONTRIBUTING.md | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 6b2963cf4..cdffa4764 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,7 +2,8 @@ Before starting work, read and follow [CONTRIBUTING.md](CONTRIBUTING.md), including the [Prevent agent impersonation](CONTRIBUTING.md#prevent-agent-impersonation) -section governing identification when communicating through a person's account. +section governing agent identification and separation of unaltered user statements +when communicating through a person's account. # Commit messages diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 76f276323..1a64a701d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -27,6 +27,13 @@ example in issue or PR descriptions and comments. AI assistance that does not re the person as the speaker, such as proofreading or wording polish, does not require identification. +Even when identifying themselves, agents must not assert on a person's behalf that +that person performed an action, such as reviewing or approving a PR. The person +must make any such statement themselves. If it is included alongside agent-authored +content, it must be supplied by the person and preserved verbatim in a clearly +labeled, separate user-authored section. Agents must not draft, paraphrase, or embed +such statements in their own narration. + Attributing AI assistance in commit metadata, for example with a `Co-authored-by` trailer, is welcome but not required. Code is reviewed the same way regardless of its origin. From 3e9611b9689597d86814d087d1b9b37be92e0451 Mon Sep 17 00:00:00 2001 From: v1shay Date: Sat, 10 Oct 2026 23:38:46 -0700 Subject: [PATCH 18/24] fix(stats): preserve whitespace in filenames Commit.stats strips leading and trailing whitespace from filenames after splitting the diff output into records. This merges distinct paths such as `name`, ` name`, and `name `, overwrites their per-file statistics, and turns an all-space filename into an empty key. Keep the filename field intact. Add an integration regression covering root commits, later modifications, empty commits, and deletions. Store blobs directly to avoid relying on trailing-space worktree filenames. The regression fails before this change and passes afterward. On Linux with Python 3.12, the full suite passes: 1690 passed, 89 skipped, 1 xfailed, and 64 subtests passed. Pre-commit, mypy, and basedpyright also pass. Prepared by OpenAI Codex (dot); human review is pending. --- git/util.py | 2 +- test/test_commit.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/git/util.py b/git/util.py index 61ddab861..7fd061c7c 100644 --- a/git/util.py +++ b/git/util.py @@ -1107,7 +1107,7 @@ def _list_from_string(cls, repo: "Repo", text: str) -> "Stats": "lines": insertions + deletions, "change_type": change_type, } - hsh["files"][filename.strip()] = files_dict + hsh["files"][filename] = files_dict return Stats(hsh["total"], hsh["files"]) diff --git a/test/test_commit.py b/test/test_commit.py index 08ba4a634..ac155e0df 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -17,6 +17,7 @@ from git import Actor, Commit, Repo from git.exc import UnsafeOptionError +from git.index.typ import BaseIndexEntry from git.objects.util import tzoffset, utc from git.repo.fun import touch @@ -166,6 +167,36 @@ def check_entries(d, has_change_type=False): self.assertEqual(commit.committer_tz_offset, 14400, commit.committer_tz_offset) self.assertEqual(commit.message, "initial project\n") + @with_rw_directory + def test_stats_preserve_filename_spaces(self, rw_dir): + repo = Repo.init(rw_dir) + actor = Actor("Test", "test@example.com") + paths = ("name", " name", "name ", " name ", " ") + # Store blobs directly so trailing spaces work on Windows as well. + for multiplier, change_type in ((1, "A"), (2, "M")): + for count, path in enumerate(paths, 1): + data = b"line\n" * count * multiplier + blob = repo.odb.store(IStream("blob", len(data), BytesIO(data))) + repo.index.add([BaseIndexEntry((0o100644, blob.binsha, 0, path))]) + commit = repo.index.commit("Update files", author=actor, committer=actor) + stats = commit.stats + assert stats.total == {"insertions": 15, "deletions": 0, "lines": 15, "files": 5} + assert stats.files == { + path: {"insertions": count, "deletions": 0, "lines": count, "change_type": change_type} + for count, path in enumerate(paths, 1) + } + empty_stats = repo.index.commit("No changes", author=actor, committer=actor).stats + assert empty_stats.files == {} + assert empty_stats.total == {"insertions": 0, "deletions": 0, "lines": 0, "files": 0} + + repo.index.remove(list(paths)) + removed_stats = repo.index.commit("Remove files", author=actor, committer=actor).stats + assert removed_stats.files == { + path: {"insertions": 0, "deletions": 2 * count, "lines": 2 * count, "change_type": "D"} + for count, path in enumerate(paths, 1) + } + assert removed_stats.total == {"insertions": 0, "deletions": 30, "lines": 30, "files": 5} + def test_renames(self): commit = self.rorepo.commit("185d847ec7647fd2642a82d9205fb3d07ea71715") files = commit.stats.files From 963ca1c7d3bccdfad66293d5417f065ce5829b15 Mon Sep 17 00:00:00 2001 From: v1shay Date: Sun, 11 Oct 2026 00:13:14 -0700 Subject: [PATCH 19/24] test(stats): avoid normalizing fixture paths on Windows The stats regression's root and modification assertions pass on Windows, but IndexFile.remove normalizes the all-space test path to `.` and makes `git rm` fail before testing deletion statistics. Clear and write the fixture's in-memory index instead, just as the test already stores its initial blobs directly. The resulting empty-tree commit still exercises deletion statistics for every original filename. The full Linux suite again passes: 1690 passed, 89 skipped, 1 xfailed, and 64 subtests. Focused commit/util/diff tests, pre-commit, mypy, and basedpyright pass. Windows CI will verify the portability correction. Prepared by OpenAI Codex (dot); human review is pending. --- test/test_commit.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/test_commit.py b/test/test_commit.py index ac155e0df..379bc831b 100644 --- a/test/test_commit.py +++ b/test/test_commit.py @@ -189,7 +189,10 @@ def test_stats_preserve_filename_spaces(self, rw_dir): assert empty_stats.files == {} assert empty_stats.total == {"insertions": 0, "deletions": 0, "lines": 0, "files": 0} - repo.index.remove(list(paths)) + # Avoid filesystem path normalization of space-only names on Windows. + index = repo.index + index.entries.clear() + index.write() removed_stats = repo.index.commit("Remove files", author=actor, committer=actor).stats assert removed_stats.files == { path: {"insertions": 0, "deletions": 2 * count, "lines": 2 * count, "change_type": "D"} From 04fa56f761db83e256ee9882adb255604887a169 Mon Sep 17 00:00:00 2001 From: Huanyi Xie <74809554+xiehuanyi@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:26:20 +0300 Subject: [PATCH 20/24] fix(config): preserve exact large integer values `GitConfigParser.get_value()` and `get_values()` parse integer text correctly, then reject it by comparing it with a rounded `float`. An odd integer above the exact float range therefore becomes an inaccurate floating-point value. Restrict the comparison to floating values. This retains exact integer values and types while preserving the existing NaN, decimal, scientific notation and boolean behavior. Three integer regression subtests fail on the baseline. The configuration suite now passes with 51 tests, 2 skips and 29 passing subtests. Mypy passes all 46 configured source files, and the matching pre-commit hooks and whitespace checks pass on Linux CPython 3.12.15. Agent: OpenAI Codex (GPT-6.1-sol). --- git/config.py | 4 ++-- test/test_config.py | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/git/config.py b/git/config.py index a3437d525..7109c09c3 100644 --- a/git/config.py +++ b/git/config.py @@ -1018,8 +1018,8 @@ def _string_to_value(self, valuestr: Union[str, None]) -> Union[int, float, str, for numtype in types: try: val = numtype(valuestr) - # truncated value ? - if val != float(valuestr): + # Reject NaN without coercing exact integer values to float. + if numtype is float and val != float(valuestr): continue return val except (ValueError, TypeError): diff --git a/test/test_config.py b/test/test_config.py index 274738075..8d526457b 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -37,6 +37,38 @@ def tearDown(self): if osp.isfile(lfp): raise AssertionError("Previous TC left hanging git-lock file: {}".format(lfp)) + def test_integer_config_values_keep_precision(self): + for text, expected in ( + ("9007199254740993", 9007199254740993), + ("9223372036854775807", 9223372036854775807), + ("-9007199254740993", -9007199254740993), + ): + with self.subTest(text=text): + config_file = io.BytesIO(("[limits]\nvalue = %s\n" % text).encode("ascii")) + config_file.name = "integer_precision.config" + with GitConfigParser(config_file) as config: + value = config.get_value("limits", "value") + self.assertEqual(value, expected) + self.assertIs(type(value), int) + self.assertEqual(config.get_values("limits", "value"), [expected]) + + def test_numeric_config_conversion_preserves_other_values(self): + for text, expected in ( + ("11.11", 11.11), + ("1e3", 1000.0), + ("nan", "nan"), + ("NaN", "NaN"), + ("true", True), + ("false", False), + ): + with self.subTest(text=text): + config_file = io.BytesIO(("[values]\nvalue = %s\n" % text).encode("ascii")) + config_file.name = "numeric_conversion.config" + with GitConfigParser(config_file) as config: + value = config.get_value("values", "value") + self.assertEqual(value, expected) + self.assertIs(type(value), type(expected)) + def _to_memcache(self, file_path): with open(file_path, "rb") as fp: sio = io.BytesIO(fp.read()) From fa45526e9d87ddb71de7dc0fbde24a916f2051b2 Mon Sep 17 00:00:00 2001 From: fan wu <336798967+FanWu-ai@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:19:24 -0700 Subject: [PATCH 21/24] fix: resolve IterableList names independently of list attributes Normal attribute lookup makes string indexing return list methods for item names such as `count`, and reports absent `append` identifiers as present. Use `__getattr__` explicitly for identifier lookup and remove its fallback to list attributes. Ordinary attribute access still exposes list methods. Add twelve present/absent collision cases with and without prefixes. The full suite passes 1701 tests, 89 conditional skips, one existing xfail, and 64 subtests; pinned hooks, type checks, docs and packaging also pass. --- git/util.py | 6 +++--- test/test_util.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/git/util.py b/git/util.py index 61ddab861..c007c93ed 100644 --- a/git/util.py +++ b/git/util.py @@ -1382,7 +1382,7 @@ def __contains__(self, attr: object) -> bool: # Otherwise make a full name search. try: - getattr(self, cast(str, attr)) # Use cast to silence mypy. + self.__getattr__(cast(str, attr)) # Use cast to silence mypy. return True except (AttributeError, TypeError): return False @@ -1394,7 +1394,7 @@ def __getattr__(self, attr: str) -> T_IterableObj: if getattr(item, self._id_attr) == attr: return item # END for each item - return list.__getattribute__(self, attr) + raise AttributeError(f"No item found with id {attr}") def __getitem__( # type: ignore[override] # pyright: ignore[reportIncompatibleMethodOverride] self, index: Union[SupportsIndex, int, slice, str] @@ -1405,7 +1405,7 @@ def __getitem__( # type: ignore[override] # pyright: ignore[reportIncompatible raise ValueError("Index should be an int or str") else: try: - return getattr(self, cast(str, index)) + return self.__getattr__(cast(str, index)) except AttributeError as e: raise IndexError(f"No item found with id {self._prefix}{index}") from e # END handle getattr diff --git a/test/test_util.py b/test/test_util.py index ca8e006f3..d759343f0 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -862,3 +862,23 @@ def test_mode_str_to_int_accepts_bytes(): assert mode_str_to_int(b"100644") == 0o100644 assert mode_str_to_int("644") == 0o644 assert mode_str_to_int(b"120000") == 0o120000 + + +@pytest.mark.parametrize("name", ["sort", "count", "append", "_prefix", "__class__", "ordinary"]) +@pytest.mark.parametrize("prefix", ["", "refs/"]) +def test_iterable_list_name_lookup_ignores_list_attributes(name, prefix): + values = IterableList("name", prefix) + member = _Member(prefix + name) + values.append(member) + + assert values[name] is member + assert name in values + assert member in values + assert values[0] is member + assert callable(values.append) + assert callable(values.sort) + + del values[name] + assert name not in values + with pytest.raises(IndexError, match="No item found with id"): + values[name] From 76150aae9810e0ab8c9d99d0391639a2c73cb97c Mon Sep 17 00:00:00 2001 From: fan wu <336798967+FanWu-ai@users.noreply.github.com> Date: Sun, 11 Oct 2026 11:08:58 -0700 Subject: [PATCH 22/24] fix(util): wait for contended locks with relative filenames `BlockingLockFile('resource')` incorrectly treats the empty `dirname` of `resource.lock` as a missing directory when another holder owns the lock. Use `os.curdir` for that case so the retry and timeout paths work normally. Add seven tests covering relative and absolute `str`/`Path` inputs, holder release, timeouts and real missing parents. The complete Linux/Python 3.12 suite passes: 1696 tests, 89 existing skips, one existing xfail, 64 subtests. Pinned hooks, mypy, basedpyright, docs, builds and strict Twine also pass. Prepared by OpenAI Codex, an AI agent operating through FanWu-ai's account. --- git/util.py | 2 +- test/test_util.py | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/git/util.py b/git/util.py index a694f4fe9..17a32c01a 100644 --- a/git/util.py +++ b/git/util.py @@ -1310,7 +1310,7 @@ def _obtain_lock(self) -> None: # synity check: if the directory leading to the lockfile is not # readable anymore, raise an exception curtime = time.time() - if not osp.isdir(osp.dirname(self._lock_file_path())): + if not osp.isdir(osp.dirname(self._lock_file_path()) or osp.curdir): msg = "Directory containing the lockfile %r was not readable anymore after waiting %g seconds" % ( self._lock_file_path(), curtime - starttime, diff --git a/test/test_util.py b/test/test_util.py index d759343f0..7fe507b8f 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -882,3 +882,40 @@ def test_iterable_list_name_lookup_ignores_list_attributes(name, prefix): assert name not in values with pytest.raises(IndexError, match="No item found with id"): values[name] + + +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +@pytest.mark.parametrize("relative", [True, False]) +def test_blocking_lock_waits_for_existing_holder(tmp_path, monkeypatch, path_type, relative): + monkeypatch.chdir(tmp_path) + path = path_type("resource" if relative else tmp_path / "resource") + owner = LockFile(path) + waiter = BlockingLockFile(path, check_interval_s=0.01, max_block_time_s=1) + owner._obtain_lock() + try: + with mock.patch.object(time, "sleep", side_effect=lambda _: owner._release_lock()) as sleep: + waiter._obtain_lock() + sleep.assert_called_once_with(0.01) + assert waiter._has_lock() + finally: + owner._release_lock() + waiter._release_lock() + + +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +def test_blocking_relative_lock_reports_timeout(tmp_path, monkeypatch, path_type): + monkeypatch.chdir(tmp_path) + owner = LockFile(path_type("resource")) + waiter = BlockingLockFile(path_type("resource"), max_block_time_s=0) + owner._obtain_lock() + try: + with pytest.raises(OSError, match="Waited .* seconds for lock"): + waiter._obtain_lock() + finally: + owner._release_lock() + + +def test_blocking_lock_still_rejects_missing_directory(tmp_path): + waiter = BlockingLockFile(tmp_path / "missing" / "resource") + with pytest.raises(OSError, match="Directory containing the lockfile"): + waiter._obtain_lock() From 3b9bb6df3c3caca808e93f77e3eac7b97d24c571 Mon Sep 17 00:00:00 2001 From: Byron Date: Wed, 14 Oct 2026 09:47:31 +0200 Subject: [PATCH 23/24] review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - [P2] Propagate non-contention errors before retrying relative locks — git/util.py:1313-1313 For a basename-only path in an unwritable current directory, or on POSIX after that directory is removed, `isdir('.')` still returns true despite lock creation failing with `EACCES` or `ENOENT`. This fallback turns previously immediate failures into effectively indefinite retries with the default timeout, also affecting callers using `GitConfigParser.t_lock = BlockingLockFile`. Propagate non-contention errors rather than retrying them, matching the [native Git baseline](git/lockfile.c#L234-L235), which retries only `EEXIST`. Assisted-by: GPT 6.1 Sol Co-authored-by: GPT 6.1 Sol --- git/util.py | 14 +++++---- test/test_config.py | 20 ++++++++++++- test/test_util.py | 72 +++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 95 insertions(+), 11 deletions(-) diff --git a/git/util.py b/git/util.py index 17a32c01a..874b764d5 100644 --- a/git/util.py +++ b/git/util.py @@ -1270,8 +1270,8 @@ class BlockingLockFile(LockFile): specified timeout. :note: - If the directory containing the lock was removed, an exception will be raised - during the blocking period, preventing hangs as the lock can never be obtained. + Only an existing lock causes retries. Other errors, including a removed or + unwritable parent directory, are raised immediately. """ __slots__ = ("_check_interval", "_max_block_time") @@ -1297,7 +1297,7 @@ def __init__( def _obtain_lock(self) -> None: """This method blocks until it obtained the lock, or raises :exc:`IOError` if it - ran out of time or if the parent directory was not available anymore. + ran out of time or lock creation failed for a reason other than an existing lock. If this method returns, you are guaranteed to own the lock. """ @@ -1307,8 +1307,12 @@ def _obtain_lock(self) -> None: try: super()._obtain_lock() except OSError as e: - # synity check: if the directory leading to the lockfile is not - # readable anymore, raise an exception + # LockFile retains the creation error as the cause of its OSError. + # Only an existing lock is contention; other errors must not be retried. + if not isinstance(e.__cause__, FileExistsError): + raise + + # Abort if the directory containing the existing lock has disappeared. curtime = time.time() if not osp.isdir(osp.dirname(self._lock_file_path()) or osp.curdir): msg = "Directory containing the lockfile %r was not readable anymore after waiting %g seconds" % ( diff --git a/test/test_config.py b/test/test_config.py index 8d526457b..e3c30421c 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -3,6 +3,7 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +import errno import glob import io import os @@ -17,7 +18,7 @@ from git import GitConfigParser, Repo from git.compat import defenc from git.config import _OMD, cp -from git.util import cwd, rmfile +from git.util import BlockingLockFile, cwd, rmfile from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory _tc_lock_fpaths = osp.join(osp.dirname(__file__), "fixtures/*.lock") @@ -28,6 +29,23 @@ def _rm_lock_files(): rmfile(lfp) +@pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows") +@pytest.mark.parametrize("error_number", [errno.EACCES, errno.ENOENT]) +def test_blocking_config_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number): + monkeypatch.chdir(tmp_path) + monkeypatch.setattr(GitConfigParser, "t_lock", BlockingLockFile) + error = OSError(error_number, os.strerror(error_number), "config.lock") + with mock.patch.object(os, "open", side_effect=error) as open_lock: + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + GitConfigParser("config", read_only=False) + sleep.assert_not_called() + open_lock.assert_called_once() + assert exc_info.value.__cause__ is error + assert str(exc_info.value) == str(error) + assert not (tmp_path / "config.lock").exists() + + class TestBase(TestCase): def setUp(self): _rm_lock_files() diff --git a/test/test_util.py b/test/test_util.py index 7fe507b8f..ca592aaf7 100644 --- a/test/test_util.py +++ b/test/test_util.py @@ -4,7 +4,7 @@ # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ import ast -from datetime import datetime +import errno import os import pathlib import pickle @@ -14,6 +14,7 @@ import tempfile import threading import time +from datetime import datetime from unittest import SkipTest, mock import ddt @@ -37,12 +38,11 @@ LockFile, cygpath, decygpath, - is_cygwin_git, get_user_id, + is_cygwin_git, remove_password_if_present, rmtree, ) - from test.lib import TestBase, requires_symlinks, with_rw_repo @@ -917,5 +917,67 @@ def test_blocking_relative_lock_reports_timeout(tmp_path, monkeypatch, path_type def test_blocking_lock_still_rejects_missing_directory(tmp_path): waiter = BlockingLockFile(tmp_path / "missing" / "resource") - with pytest.raises(OSError, match="Directory containing the lockfile"): - waiter._obtain_lock() + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + assert isinstance(exc_info.value.__cause__, FileNotFoundError) + assert not waiter._has_lock() + + +@pytest.mark.skipif(sys.platform == "win32", reason="Lock creation uses CreateFileW on Windows") +@pytest.mark.parametrize( + "error_number", [errno.EACCES, errno.ENOENT, errno.ENOTDIR, errno.EROFS, errno.ENOSPC, errno.EIO] +) +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +@pytest.mark.parametrize("relative", [True, False]) +def test_blocking_lock_propagates_non_contention_errors(tmp_path, monkeypatch, error_number, path_type, relative): + monkeypatch.chdir(tmp_path) + path = path_type("resource" if relative else tmp_path / "resource") + waiter = BlockingLockFile(path) + error = OSError(error_number, os.strerror(error_number), waiter._lock_file_path()) + with mock.patch.object(os, "open", side_effect=error) as open_lock: + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + open_lock.assert_called_once() + assert exc_info.value.__cause__ is error + assert str(exc_info.value) == str(error) + assert not waiter._has_lock() + assert not os.path.lexists(waiter._lock_file_path()) + + +@pytest.mark.skipif(os.name != "posix", reason="Directory write permissions require POSIX") +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +def test_blocking_relative_lock_rejects_unwritable_cwd(tmp_path, monkeypatch, path_type): + monkeypatch.chdir(tmp_path) + original_mode = tmp_path.stat().st_mode + tmp_path.chmod(stat.S_IRUSR | stat.S_IXUSR) + try: + if os.access(".", os.W_OK): + pytest.skip("Current user can write to read-only directories") + assert os.path.isdir(".") + waiter = BlockingLockFile(path_type("resource")) + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + assert isinstance(exc_info.value.__cause__, PermissionError) + assert not waiter._has_lock() + finally: + tmp_path.chmod(original_mode) + + +@pytest.mark.skipif(os.name != "posix", reason="Removing the current directory requires POSIX") +@pytest.mark.parametrize("path_type", [str, pathlib.Path]) +def test_blocking_relative_lock_rejects_removed_cwd(tmp_path, monkeypatch, path_type): + monkeypatch.chdir(tmp_path) + tmp_path.rmdir() + waiter = BlockingLockFile(path_type("resource")) + with mock.patch.object(time, "sleep", side_effect=AssertionError("Must not retry")) as sleep: + with pytest.raises(OSError) as exc_info: + waiter._obtain_lock() + sleep.assert_not_called() + assert isinstance(exc_info.value.__cause__, FileNotFoundError) + assert not waiter._has_lock() From c179357b38a8962f1c8e33153ac58b5ac0e6e96a Mon Sep 17 00:00:00 2001 From: Byron Date: Wed, 14 Oct 2026 14:19:03 +0200 Subject: [PATCH 24/24] prepare changelog prior to release --- VERSION | 2 +- doc/source/changes.rst | 24 ++++++++++++------------ 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/VERSION b/VERSION index 944880fa1..e4604e3af 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.2.0 +3.2.1 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 8447228db..95178088d 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -10,7 +10,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-83vg-56qc-22m7 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.2.1 @@ -35,7 +35,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-fx3j-rwgx-fr94 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.2.0 @@ -48,7 +48,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-59cr-6r3x-644w If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.62 @@ -60,7 +60,7 @@ A fixup release to avoid accidental removal of public class regex on Actor. It's now deprecated instead. If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.61 @@ -75,7 +75,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-239g-whfq-7xj9 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.60 @@ -92,7 +92,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-7833-fr7j-v32q If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.59 @@ -110,7 +110,7 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-hmq2-w58f-27jc If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.58 @@ -124,7 +124,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-539m-9xh6-q6rr If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.57 @@ -137,7 +137,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-p538-c434-8v24 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.56 @@ -150,7 +150,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-94p4-4cq8-9g67 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.55 @@ -165,7 +165,7 @@ A security fix for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-fjr4-x663-mwxc If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.54 @@ -177,7 +177,7 @@ A security fix for https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-3rp5-jjmw-4wv2 If you can, also try and provide feedback on the upcoming v4 branch -https://github.com/gitpython-developers/GitPython/pull/2177 - patches welcome. +https://github.com/gitpython-developers/GitPython/pull/2274 - patches welcome. See the following for all changes. https://github.com/gitpython-developers/GitPython/releases/tag/3.1.53