From 1b0d2d9b91575f7db44ef4ff58ac37fc9335e5f6 Mon Sep 17 00:00:00 2001 From: Byron Date: Wed, 5 Aug 2026 04:48:36 +0200 Subject: [PATCH 01/10] Block file-reading Git options Reject file-reading options passed to blame, diff, and tag APIs. Inspect positional values after resolving aliases, recognize unsafe options behind command-specific short-flag clusters, and retain the explicit allow_unsafe_options escape hatch. This closes GHSA-5xxx-qhh7-9287 and GHSA-3wxw-xv34-2frg and covers the adjacent diff order-file sink. Regression tests cover long, short, and clustered options, incremental blame, tag path/reference positionals, the ref keyword alias, both diff entry points, and preservation of diff pickaxe behavior. Git baseline: cf5497b14c; git-blame, git-diff, and git-tag document the relevant file-input options. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/cmd.py | 11 +++++++---- git/diff.py | 7 ++++--- git/index/base.py | 3 ++- git/refs/tag.py | 10 ++++++---- git/repo/base.py | 24 ++++++++++++++++++++---- test/test_diff.py | 14 ++++++++++++++ test/test_refs.py | 15 +++++++++++++++ test/test_remote.py | 3 +++ test/test_repo.py | 7 +++++-- 9 files changed, 76 insertions(+), 18 deletions(-) diff --git a/git/cmd.py b/git/cmd.py index 03ecd13f5..193dfd4f6 100644 --- a/git/cmd.py +++ b/git/cmd.py @@ -652,6 +652,7 @@ class Git(metaclass=_GitMeta): unsafe_git_ls_remote_options = [ # This option allows arbitrary command execution in git-ls-remote. "--upload-pack", + "--exec", ] unsafe_git_pathspec_from_file_options = [ @@ -976,7 +977,9 @@ def _canonicalize_option_name(cls, option: str) -> str: return dashify(option_tokens[0]) @classmethod - def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> None: + def check_unsafe_options( + cls, options: List[str], unsafe_options: List[str], clusterable_short_options: str = "46flnqsv" + ) -> None: """Raise :class:`~git.exc.UnsafeOptionError` for blocked option spellings. In addition to exact matches, this rejects abbreviated long options accepted @@ -1011,7 +1014,7 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> # These value-less Git flags can be clustered before another short option # (for example, ``-fuVALUE``). Stop at any other character because it may # begin an attached value, as ``o`` does in the safe option ``-oupstream``. - clusterable_short_options = frozenset("46flnqsv") + clusterable_short_options_set = frozenset(clusterable_short_options) options_are_kwargs = all(not option.startswith("-") for option in options) for option in options: candidate = cls._canonicalize_option_name(option) @@ -1028,7 +1031,7 @@ def check_unsafe_options(cls, options: List[str], unsafe_options: List[str]) -> raise UnsafeOptionError( f"{unsafe_option} is not allowed, use `allow_unsafe_options=True` to allow it." ) - if option_char not in clusterable_short_options: + if option_char not in clusterable_short_options_set: break if not (option.startswith("--") or (options_are_kwargs and len(candidate) > 1)): continue @@ -1133,7 +1136,7 @@ def ls_remote( """List references in a remote repository. :param allow_unsafe_options: - Allow unsafe options, like ``--upload-pack``. + Allow unsafe options, like ``--upload-pack`` or ``--exec``. """ if not allow_unsafe_options: candidate_options = self._option_candidates(args, kwargs) diff --git a/git/diff.py b/git/diff.py index 3628c815a..d1963b84f 100644 --- a/git/diff.py +++ b/git/diff.py @@ -220,8 +220,8 @@ def diff( to be read and diffed. :param allow_unsafe_options: - If ``True``, allow options such as ``--output`` that can write to arbitrary - filesystem paths. + If ``True``, allow options such as ``--output`` and ``-O`` that can write to + or read from arbitrary filesystem paths. :param kwargs: Additional arguments passed to :manpage:`git-diff(1)`, such as ``R=True`` to @@ -238,7 +238,8 @@ def diff( if not allow_unsafe_options: Git.check_unsafe_options( options=Git._option_candidates([other], kwargs), - unsafe_options=self.repo.unsafe_git_revision_options, + unsafe_options=self.repo.unsafe_git_diff_options, + clusterable_short_options="46abceflmnpqrstuvwzBCDMNRW", ) args: List[Union[PathLike, Diffable]] = [] diff --git a/git/index/base.py b/git/index/base.py index 0e7b5f918..a3c915242 100644 --- a/git/index/base.py +++ b/git/index/base.py @@ -1567,7 +1567,8 @@ def diff( if not allow_unsafe_options: Git.check_unsafe_options( options=Git._option_candidates([other], kwargs), - unsafe_options=self.repo.unsafe_git_revision_options, + unsafe_options=self.repo.unsafe_git_diff_options, + clusterable_short_options="46abceflmnpqrstuvwzBCDMNRW", ) # Only run if we are the default repository index. diff --git a/git/refs/tag.py b/git/refs/tag.py index 055722e3b..3a7d946c1 100644 --- a/git/refs/tag.py +++ b/git/refs/tag.py @@ -134,15 +134,17 @@ def create( :return: A new :class:`TagReference`. """ + legacy_ref = kwargs.pop("ref", None) + if legacy_ref: + reference = legacy_ref + if not allow_unsafe_options: Git.check_unsafe_options( - options=Git._option_candidates([], kwargs), + options=Git._option_candidates([path, reference], kwargs), unsafe_options=cls.unsafe_git_tag_options, + clusterable_short_options="46adefilnqsv", ) - if "ref" in kwargs and kwargs["ref"]: - reference = kwargs["ref"] - if "message" in kwargs and kwargs["message"]: kwargs["m"] = kwargs["message"] del kwargs["message"] diff --git a/git/repo/base.py b/git/repo/base.py index df61e2d28..583e96ca4 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -199,6 +199,19 @@ class Repo: "-o", ] + unsafe_git_blame_options = unsafe_git_revision_options + [ + # These options read from arbitrary files and expose their contents through blame output. + "--contents", + "-S", + "--ignore-revs-file", + ] + + unsafe_git_diff_options = unsafe_git_revision_options + [ + # Reads caller-controlled order patterns from an arbitrary file. + "-O", + "--orderfile", + ] + # Invariants config_level: ConfigLevels_Tup = ("system", "user", "global", "repository") """Represents the configuration level of a configuration file.""" @@ -1149,7 +1162,7 @@ def blame_incremental( :manpage:`git-rev-parse(1)` is a valid option. :param allow_unsafe_options: - Allow unsafe options in revision argument, like ``--output``. + Allow unsafe options in revision argument, like ``--output`` or ``--contents``. :return: Lazy iterator of :class:`BlameEntry` tuples, where the commit indicates the @@ -1161,7 +1174,9 @@ def blame_incremental( """ if not allow_unsafe_options: Git.check_unsafe_options( - options=Git._option_candidates([rev], kwargs), unsafe_options=self.unsafe_git_revision_options + options=Git._option_candidates([rev], kwargs), + unsafe_options=self.unsafe_git_blame_options, + clusterable_short_options="46bceflnpqstvw", ) data: bytes = self.git.blame(rev, "--", file, p=True, incremental=True, stdout_as_string=False, **kwargs) @@ -1253,7 +1268,7 @@ def blame( :manpage:`git-rev-parse(1)` is a valid option. :param allow_unsafe_options: - Allow unsafe options in revision argument, like ``--output``. + Allow unsafe options in revision argument, like ``--output`` or ``--contents``. :return: list: [git.Commit, list: []] @@ -1269,7 +1284,8 @@ def blame( if not allow_unsafe_options: Git.check_unsafe_options( options=Git._option_candidates([rev, rev_opts_list], kwargs), - unsafe_options=self.unsafe_git_revision_options, + unsafe_options=self.unsafe_git_blame_options, + clusterable_short_options="46bceflnpqstvw", ) data: bytes = self.git.blame(rev, *rev_opts_list, "--", file, p=True, stdout_as_string=False, **kwargs) commits: Dict[str, Commit] = {} diff --git a/test/test_diff.py b/test/test_diff.py index 7f2275f55..d5e14f3de 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -376,11 +376,25 @@ def test_diff_submodule(self): def test_diff_rejects_unsafe_output_options(self): commit = self.rorepo.head.commit + commit.diff(S="needle") + calls = ( lambda target: commit.diff(output=target), lambda target: commit.diff(other=f"--output={target}"), + lambda target: commit.diff(O=target), + lambda target: commit.diff(orderfile=target), + lambda target: commit.diff(other=f"--orderfile={target}"), + lambda target: commit.diff(other=f"-pO{target}"), + lambda target: commit.diff(other=f"-uO{target}"), + lambda target: commit.diff(other=f"-DO{target}"), lambda target: self.rorepo.index.diff(NULL_TREE, output=target), lambda target: self.rorepo.index.diff(f"--output={target}"), + lambda target: self.rorepo.index.diff(NULL_TREE, O=target), + lambda target: self.rorepo.index.diff(NULL_TREE, orderfile=target), + lambda target: self.rorepo.index.diff(f"--orderfile={target}"), + lambda target: self.rorepo.index.diff(f"-pO{target}"), + lambda target: self.rorepo.index.diff(f"-uO{target}"), + lambda target: self.rorepo.index.diff(f"-DO{target}"), ) for index, call in enumerate(calls): target = osp.join(self.repo_dir, f"diff-output-{index}") diff --git a/test/test_refs.py b/test/test_refs.py index a87134ab8..9a3f58c7b 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -70,6 +70,21 @@ def test_tag_create_rejects_unsafe_file_options(self, rw_repo): with self.assertRaises(UnsafeOptionError): TagReference.create(rw_repo, f"unsafe-{index}", **option) + for args in ( + ("unsafe-reference", f"--file={message.name}"), + (f"--file={message.name}", "HEAD"), + (f"-eF{message.name}", "HEAD"), + (f"-iF{message.name}", "HEAD"), + ): + with self.assertRaises(UnsafeOptionError): + TagReference.create(rw_repo, *args) + + with self.assertRaises(UnsafeOptionError): + TagReference.create(rw_repo, "unsafe-ref-kwarg", ref=f"--file={message.name}") + + tag = TagReference.create(rw_repo, "legacy-ref", ref="HEAD", allow_unsafe_options=True) + self.assertEqual(tag.commit, rw_repo.head.commit) + tag = TagReference.create(rw_repo, "allowed-file", F=message.name, allow_unsafe_options=True) self.assertEqual(tag.tag.message, "private tag message") diff --git a/test/test_remote.py b/test/test_remote.py index 505d283af..e1793214c 100644 --- a/test/test_remote.py +++ b/test/test_remote.py @@ -1035,6 +1035,7 @@ def test_ls_remote_unsafe_options(self, rw_repo): {"upload-pack": f"touch {tmp_file}"}, {"upload_pack": f"touch {tmp_file}"}, {"upl": f"touch {tmp_file}"}, + {"exec": f"touch {tmp_file}"}, ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -1047,6 +1048,8 @@ def test_ls_remote_unsafe_options(self, rw_repo): rw_repo.git.ls_remote(f"--upload-pack={tmp_file}", ".") with self.assertRaises(UnsafeOptionError): rw_repo.git.ls_remote(f"--upl={tmp_file}", ".") + with self.assertRaises(UnsafeOptionError): + rw_repo.git.ls_remote(f"--exec={tmp_file}", ".") with self.assertRaises(UnsafeOptionError): rw_repo.git.ls_remote("--upload-pack", "touch", ".") with self.assertRaises(UnsafeOptionError): diff --git a/test/test_repo.py b/test/test_repo.py index 0c97041f9..1dfec951a 100644 --- a/test/test_repo.py +++ b/test/test_repo.py @@ -590,8 +590,11 @@ def test_blame_real(self): def test_blame_rejects_unsafe_revision(self): with tempfile.TemporaryDirectory() as tdir: output_marker = osp.join(tdir, "pwn") - with self.assertRaises(UnsafeOptionError): - self.rorepo.blame(f"--output={output_marker}", "README.md") + for option in ("--output", "--contents", "-S", "-wS", "--ignore-revs-file"): + with self.assertRaises(UnsafeOptionError): + self.rorepo.blame(f"{option}={output_marker}", "README.md") + with self.assertRaises(UnsafeOptionError): + list(self.rorepo.blame_incremental(f"{option}={output_marker}", "README.md")) assert not osp.exists(output_marker) def test_blame_rejects_unsafe_options(self): From ce9d8e8d150e06ae2e2cc2efa229071cd3048a93 Mon Sep 17 00:00:00 2001 From: Byron Date: Wed, 5 Aug 2026 07:18:16 +0200 Subject: [PATCH 02/10] prepare next release --- doc/source/changes.rst | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/doc/source/changes.rst b/doc/source/changes.rst index ffddf56a1..714cc7ffc 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,20 @@ Changelog ========= +3.1.59 +====== + +Security fixes for + +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-5xxx-qhh7-9287 +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-3wxw-xv34-2frg + +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.1.59 + 3.1.58 ====== From 93677a00ab9dcb06cc08595fd1f88a4b4a0fa23b Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 10:23:51 +0200 Subject: [PATCH 03/10] fix: `index.add()` now supports filters (#2021) This is done by calling into `git hash-object` for correctness, instead of using a mostly incorrect custom implementation for this (lacks filters). GitCmdObjectDB inherited LooseObjectDB.store(), so despite its name, object writes bypassed Git and used gitdb's loose-object implementation. That path creates and chmods object files itself, which can fail during Index.add() on filesystems where those permission changes are unsupported. Override store() to stream new objects through `git hash-object -w --stdin`. This lets Git manage object creation and permissions consistently with the repository configuration. Retain the inherited implementation for pre-hashed objects and custom output streams, whose existing semantics hash-object cannot provide. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/db.py | 25 ++++++++++++++++++++++++- test/test_db.py | 16 +++++++++++++++- test/test_refs.py | 3 ++- 3 files changed, 41 insertions(+), 3 deletions(-) diff --git a/git/db.py b/git/db.py index cacd030d0..bd68a5157 100644 --- a/git/db.py +++ b/git/db.py @@ -5,10 +5,14 @@ __all__ = ["GitCmdObjectDB", "GitDB"] -from gitdb.base import OInfo, OStream +from subprocess import PIPE + +from gitdb.base import IStream, OInfo, OStream from gitdb.db import GitDB, LooseObjectDB from gitdb.exc import BadObject +from gitdb.fun import stream_copy +from git.compat import force_text from git.util import bin_to_hex, hex_to_bin from git.exc import GitCommandError @@ -46,6 +50,25 @@ def stream(self, binsha: bytes) -> OStream: hexsha, typename, size, stream = self._git.stream_object_data(bin_to_hex(binsha)) return OStream(hex_to_bin(hexsha), typename, size, stream) + def store(self, istream: IStream) -> IStream: + """Store an object using git itself.""" + if istream.binsha is not None or self.ostream() is not None: + return super().store(istream) + + proc = self._git.hash_object( + "-t", force_text(istream.type), "-w", "--stdin", "--literally", as_process=True, istream=PIPE + ) + assert proc.stdin is not None + try: + stream_copy(istream.read, proc.stdin.write, istream.size, self.stream_chunk_size) + finally: + proc.stdin.close() + assert proc.stdout is not None + hexsha = proc.stdout.read().strip() + proc.wait() + istream.binsha = hex_to_bin(hexsha) + return istream + # { Interface def partial_to_complete_sha_hex(self, partial_hexsha: str) -> bytes: diff --git a/test/test_db.py b/test/test_db.py index 72d63b44b..46580d84b 100644 --- a/test/test_db.py +++ b/test/test_db.py @@ -3,16 +3,30 @@ # This module is part of GitPython and is released under the # 3-Clause BSD License: https://opensource.org/license/bsd-3-clause/ +from io import BytesIO import os.path as osp +from unittest import mock + +from gitdb import IStream +from gitdb.db import LooseObjectDB +from gitdb.typ import str_blob_type from git.db import GitCmdObjectDB from git.exc import BadObject from git.util import bin_to_hex -from test.lib import TestBase +from test.lib import TestBase, with_rw_repo class TestDB(TestBase): + @with_rw_repo("HEAD") + def test_store_uses_hash_object(self, rw_repo): + data = b"hello world" + with mock.patch.object(LooseObjectDB, "store", side_effect=AssertionError("unexpected loose-object write")): + istream = rw_repo.odb.store(IStream(str_blob_type, len(data), BytesIO(data))) + + assert rw_repo.odb.stream(istream.binsha).read() == data + def test_base(self): gdb = GitCmdObjectDB(osp.join(self.rorepo.git_dir, "objects"), self.rorepo.git) diff --git a/test/test_refs.py b/test/test_refs.py index 9a3f58c7b..32c8fbe34 100644 --- a/test/test_refs.py +++ b/test/test_refs.py @@ -26,7 +26,7 @@ from git.exc import UnsafeOptionError from git.objects.tag import TagObject import git.refs as refs -from git.util import Actor +from git.util import Actor, rmtree from test.lib import TestBase, requires_symlinks, with_rw_repo, PathLikeMock @@ -43,6 +43,7 @@ def _repo_with_initial_commit(self, base_dir): yield repo finally: repo.git.clear_cache() + rmtree(repo_dir) def test_from_path(self): # Should be able to create any reference directly. From b68afff45af0f49e79a3e2d2162018986b37ad5d Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 11:20:03 +0200 Subject: [PATCH 04/10] Block separate git directories during clone Repo.clone() and Repo.clone_from() did not reject the clone option that redirects repository metadata to a caller-controlled path (GHSA-8mcc-hrx5-hvxc). Regression coverage exercises both keyword and multi-option input through both public clone APIs. Add the option to the existing clone denylist, matching Repo.init() and the documented allow_unsafe_options contract. Git itself registers The Python package and Alpine test workflows failed across the submodule suite because GitPython internally supplies --separate-git-dir when creating modern submodule layouts. The new public clone guard correctly rejected that option, but could not distinguish the library-generated path from caller input. Validate caller-provided keyword and multi-options before adding the library-controlled metadata path, then explicitly allow the resulting trusted clone invocation. This preserves rejection of unsafe clone_multi_options while restoring normal submodule creation. Update the one test that intentionally invokes Repo.clone_from() with its own separate git directory to opt in explicitly. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/objects/submodule/base.py | 10 ++++++++++ git/repo/base.py | 2 ++ test/test_clone.py | 4 ++++ test/test_submodule.py | 1 + 4 files changed, 17 insertions(+) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index da0e09af4..d52ee4459 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -9,6 +9,7 @@ import ntpath import os import os.path as osp +import shlex import stat import sys import uuid @@ -363,6 +364,15 @@ def _clone_repo( module_abspath = cls._module_abspath(repo, path, name) module_checkout_path = module_abspath if cls._need_gitfile_submodules(repo.git): + if not allow_unsafe_options: + Git.check_unsafe_options(Git._option_candidates([], kwargs), repo.unsafe_git_clone_options) + multi_options = kwargs.get("multi_options") + if multi_options: + Git.check_unsafe_options( + shlex.split(" ".join(cast("Sequence[str]", multi_options))), + repo.unsafe_git_clone_options, + ) + allow_unsafe_options = True kwargs["separate_git_dir"] = module_abspath module_abspath_dir = osp.dirname(module_abspath) if not osp.isdir(module_abspath_dir): diff --git a/git/repo/base.py b/git/repo/base.py index 583e96ca4..d0ec00ec9 100644 --- a/git/repo/base.py +++ b/git/repo/base.py @@ -159,6 +159,8 @@ class Repo: "-c", # Can install hooks that execute during clone: "--template", + # Redirects the repository metadata to a caller-controlled path: + "--separate-git-dir", # Fetches from an additional caller-controlled URI: "--bundle-uri", ] diff --git a/test/test_clone.py b/test/test_clone.py index 5af67613a..a6c3db6f9 100644 --- a/test/test_clone.py +++ b/test/test_clone.py @@ -133,6 +133,7 @@ def test_clone_unsafe_options(self, rw_repo): "-vcprotocol.ext.allow=always", f"--template={tmp_dir}", f"--bundle-uri=file://{tmp_dir}", + f"--separate-git-dir={tmp_dir / 'git-dir'}", ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -149,6 +150,7 @@ def test_clone_unsafe_options(self, rw_repo): {"c": "protocol.ext.allow=always"}, {"template": tmp_dir}, {"bundle_uri": f"file://{tmp_dir}"}, + {"separate_git_dir": tmp_dir / "git-dir"}, ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -258,6 +260,7 @@ def test_clone_from_unsafe_options(self, rw_repo): "-c protocol.ext.allow=always", "-cprotocol.ext.allow=always", "-vcprotocol.ext.allow=always", + f"--separate-git-dir={tmp_dir / 'git-dir'}", ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): @@ -270,6 +273,7 @@ def test_clone_from_unsafe_options(self, rw_repo): {"u": f"touch {tmp_file}"}, {"config": "protocol.ext.allow=always"}, {"c": "protocol.ext.allow=always"}, + {"separate_git_dir": tmp_dir / "git-dir"}, ] for unsafe_option in unsafe_options: with self.assertRaises(UnsafeOptionError): diff --git a/test/test_submodule.py b/test/test_submodule.py index 287986059..28e865ff2 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -954,6 +954,7 @@ def test_update_rejects_parent_component_in_name(self, rwdir): source.working_tree_dir, osp.join(clone.working_tree_dir, "module"), separate_git_dir=osp.join(rwdir, "escaped", "module"), + allow_unsafe_options=True, ) with pytest.raises(ValueError, match="submodule name"): clone.submodules[0].update(init=True) From 4b4e47fc1224e23b0c8ee7220a7192818f2e4abb Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 12:22:31 +0200 Subject: [PATCH 05/10] fix: preserve multiline config values when writing GitConfigParser decoded valid multiline values into embedded newlines, but _write() serialized those newlines as indented physical lines. Rewriting an otherwise unchanged config could therefore change its meaning to Git. Serialize resident multiline values with Git-compatible escapes inside a quoted continuation, preserving GitPython read compatibility while keeping each option structurally intact. This addresses GHSA-284h-m62q-gf8w. The regression starts with an inert multiline value, performs an unrelated write, and verifies with both GitPython and git config that it remains one value and does not create another option. Git baseline: config.c parse_value() and write_pair() at cf5497b14c5a escape embedded LF as \\n rather than emitting it as a physical config line. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/config.py | 6 +++++- test/test_config.py | 42 +++++++++++++++++++++++++++++++++++++++++- 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/git/config.py b/git/config.py index c300de499..6f26e58fc 100644 --- a/git/config.py +++ b/git/config.py @@ -705,7 +705,11 @@ def write_section(name: str, section_dict: _OMD) -> None: continue for v in values: - fp.write(("\t%s = %s\n" % (key, self._value_to_string(v).replace("\n", "\n\t"))).encode(defenc)) + value = self._value_to_string(v) + if any(char in value for char in '\n\t\b\\"'): + value = value.replace("\\", "\\\\").replace('"', '\\"') + value = '"%s\\\n"' % value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b") + fp.write(("\t%s = %s\n" % (key, value)).encode(defenc)) # END if key is not __name__ # END section writing diff --git a/test/test_config.py b/test/test_config.py index fd0d347a4..d664fdb6f 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -7,6 +7,7 @@ import io import os import os.path as osp +import subprocess import sys from unittest import mock @@ -15,7 +16,6 @@ from git import GitConfigParser from git.config import _OMD, cp from git.util import cwd, rmfile - from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory _tc_lock_fpaths = osp.join(osp.dirname(__file__), "fixtures/*.lock") @@ -150,6 +150,46 @@ def test_config_value_with_trailing_new_line(self): git_config = GitConfigParser(config_file) git_config.read() # This should not throw an exception + @with_rw_directory + def test_rewriting_multiline_value_does_not_create_option(self, rw_dir): + config_path = osp.join(rw_dir, "config") + with open(config_path, "wb") as config_file: + config_file.write(b'[core]\n\tzzz = "A\\nhooksPath = ../evil-hooks\\\n"\n') + + with GitConfigParser(config_path, read_only=False) as git_config: + self.assertEqual(git_config.get_value("core", "zzz"), "A\nhooksPath = ../evil-hooks") + git_config.set_value("user", "name", "Test User") + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertEqual(git_config.get_value("core", "zzz"), "A\nhooksPath = ../evil-hooks") + self.assertFalse(git_config.has_option("core", "hooksPath")) + self.assertEqual( + subprocess.run(["git", "config", "--file", config_path, "--get", "core.hooksPath"]).returncode, 1 + ) + + @with_rw_directory + def test_writer_escapes_special_characters_without_newline(self, rw_dir): + config_path = osp.join(rw_dir, "config") + values = {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"} + + with GitConfigParser(config_path, read_only=False) as git_config: + for key, value in values.items(): + git_config.set_value("section", key, value) + + with GitConfigParser(config_path, read_only=True) as git_config: + for key, value in values.items(): + self.assertEqual(git_config.get_value("section", key), value) + self.assertEqual( + subprocess.run( + ["git", "config", "--file", config_path, "--get", "section.%s" % key], + stdout=subprocess.PIPE, + check=True, + ).stdout, + value.encode() + b"\n", + ) + with open(config_path, "rb") as config_file: + self.assertNotIn(b"\x08", config_file.read()) + @with_rw_directory def test_set_value_rejects_config_injection(self, rw_dir): config_path = osp.join(rw_dir, "config") From ef7568e3b317ce617eacda39b8b54dcdff8c3b5c Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 12:32:24 +0200 Subject: [PATCH 06/10] fix: ignore includes in submodule configuration Submodule configuration is read from .gitmodules, whose contents may come from an untrusted repository. Its parser inherited merge_includes=True and could therefore open files named by include directives during ordinary submodule enumeration. Disable include merging at the SubmoduleConfigParser construction site. This matches Repo.config_writer() hardening from 41ecc6a4 and addresses GHSA-7833-fr7j-v32q without changing include behavior for trusted config parsers. The regression points .gitmodules at a non-config file and verifies the submodule entry remains readable without opening the included path. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/objects/submodule/base.py | 2 +- test/test_submodule.py | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/git/objects/submodule/base.py b/git/objects/submodule/base.py index da0e09af4..d116dd414 100644 --- a/git/objects/submodule/base.py +++ b/git/objects/submodule/base.py @@ -270,7 +270,7 @@ def _config_parser( raise ValueError("Cannot write blobs of 'historical' submodule configurations") # END handle writes of historical submodules - return SubmoduleConfigParser(fp_module, read_only=read_only) + return SubmoduleConfigParser(fp_module, read_only=read_only, merge_includes=False) def _clear_cache(self) -> None: """Clear the possibly changed values.""" diff --git a/test/test_submodule.py b/test/test_submodule.py index 287986059..d01c35298 100644 --- a/test/test_submodule.py +++ b/test/test_submodule.py @@ -1207,6 +1207,22 @@ def test_ignore_non_submodule_file(self, rwdir): assert len(parent.submodules) == 0 + @with_rw_directory + def test_gitmodules_does_not_merge_includes(self, rwdir): + parent = git.Repo.init(rwdir) + secret_path = osp.join(rwdir, "secret") + with open(secret_path, "w", encoding="utf-8") as secret: + secret.write("not git config\n") + with open(osp.join(rwdir, ".gitmodules"), "w", encoding="utf-8") as modules: + modules.write('[submodule "module"]\n') + modules.write("\tpath = module\n") + modules.write("\turl = https://example.com/module.git\n") + modules.write("[include]\n") + modules.write("\tpath = %s\n" % secret_path) + + parser = Submodule._config_parser(parent, None, read_only=True) + self.assertEqual(parser.get_value('submodule "module"', "path"), "module") + @with_rw_directory def test_remove_norefs(self, rwdir): parent = git.Repo.init(osp.join(rwdir, "parent")) From 66340d77aab9a7468f4aed3681d4ef1e3c0ec931 Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 13:59:44 +0200 Subject: [PATCH 07/10] prepare changelog prior to release --- VERSION | 2 +- doc/source/changes.rst | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/VERSION b/VERSION index dfabc766a..e5c812e68 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.58 +3.1.59 diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 714cc7ffc..1a1b8fa12 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -9,6 +9,9 @@ Security fixes for * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-5xxx-qhh7-9287 * https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-3wxw-xv34-2frg +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-8mcc-hrx5-hvxc +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-284h-m62q-gf8w +* 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. From eefa7e425ed5272eb1c1f7cc2c5f229bd552df98 Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 14:38:41 +0200 Subject: [PATCH 08/10] Preserve Git config value semantics Decode Git-supported quoted value escapes directly instead of routing UTF-8 text through Python unicode_escape. This preserves newlines, quotes, backslashes, and non-ASCII text when an unrelated config update rewrites existing values. Quote values containing Git comment delimiters (# and ;) or leading/trailing whitespace so Git does not truncate or trim their data. Escape LF, tab, backspace, quote, and backslash, while rejecting carriage returns and NULs before opening the destination. Regression coverage round-trips these values through both GitPython and git config and verifies unsafe control characters cannot alter the original file. Behavior follows Git config.c parse_value() and write_pair(). Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/config.py | 29 +++++++++++---- test/test_config.py | 88 ++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 102 insertions(+), 15 deletions(-) diff --git a/git/config.py b/git/config.py index 6f26e58fc..e7f64f7b5 100644 --- a/git/config.py +++ b/git/config.py @@ -462,7 +462,8 @@ def string_decode(v: str) -> str: v = v[:-1] # END cut trailing escapes to prevent decode error - return v.encode(defenc).decode("unicode_escape") + escapes = {"b": "\b", "n": "\n", "r": "\r", "t": "\t", '"': '"', "\\": "\\"} + return re.sub(r"\\(.)", lambda match: escapes.get(match.group(1), match.group(0)), v) # END string_decode @@ -517,10 +518,12 @@ def string_decode(v: str) -> str: # Opens quoting and does not close: appears to start multi-line quoting. is_multi_line = True optval = string_decode(optval[1:]) - elif optval.find("\\", 1, -1) == -1 and optval.find('"', 1, -1) == -1: - # Opens and closes quoting. Single line, and all we need is quote removal. - optval = optval[1:-1] - # TODO: Handle other quoted content, especially well-formed backslash escapes. + elif re.search(r'(?:^|[^\\])(?:\\\\)*"', optval[1:-1]): + # Preserve malformed values containing unescaped quotes. + pass + else: + # Opens and closes quoting. + optval = string_decode(optval[1:-1]) # Preserves multiple values for duplicate optnames. cursect.add(optname, optval) @@ -706,7 +709,7 @@ def write_section(name: str, section_dict: _OMD) -> None: for v in values: value = self._value_to_string(v) - if any(char in value for char in '\n\t\b\\"'): + if any(char in value for char in '\n\t\b\\"#;') or value[:1].isspace() or value[-1:].isspace(): value = value.replace("\\", "\\\\").replace('"', '\\"') value = '"%s\\\n"' % value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b") fp.write(("\t%s = %s\n" % (key, value)).encode(defenc)) @@ -768,6 +771,20 @@ def write(self) -> None: return # END stop if we have include files + sections: List[_OMD] = [self._defaults] + section: _OMD + stored_section: _OMD + values: List[Any] + raw_value: Any + for _, stored_section in self._sections.items(): + sections.append(stored_section) + for section in sections: + for key, values in section.items_all(): + if key != "__name__": + for raw_value in values: + if "\r" in self._value_to_string(raw_value) or "\x00" in self._value_to_string(raw_value): + raise ValueError("Git config values must not contain CR or NUL") + fp = self._file_or_files # We have a physical file on disk, so get a lock. diff --git a/test/test_config.py b/test/test_config.py index d664fdb6f..28bb12043 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -14,6 +14,7 @@ import pytest from git import GitConfigParser +from git.compat import defenc from git.config import _OMD, cp from git.util import cwd, rmfile from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory @@ -170,7 +171,16 @@ def test_rewriting_multiline_value_does_not_create_option(self, rw_dir): @with_rw_directory def test_writer_escapes_special_characters_without_newline(self, rw_dir): config_path = osp.join(rw_dir, "config") - values = {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"} + values = { + "tab": "\tvalue\t", + "backspace": "a\bb", + "quote": 'a"b', + "backslash": "a\\qb", + "hash": "value#fragment", + "semicolon": "value;fragment", + "leading": " value", + "trailing": "value ", + } with GitConfigParser(config_path, read_only=False) as git_config: for key, value in values.items(): @@ -185,11 +195,72 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir): stdout=subprocess.PIPE, check=True, ).stdout, - value.encode() + b"\n", + value.encode(defenc) + b"\n", ) with open(config_path, "rb") as config_file: self.assertNotIn(b"\x08", config_file.read()) + @with_rw_directory + def test_writer_preserves_escaped_and_non_ascii_values_safely(self, rw_dir): + config_path = osp.join(rw_dir, "config") + with open(config_path, "wb") as config_file: + config_file.write( + ( + '[section]\nnewline = "first\\nsecond"\nquote = "a\\"b"\nbackslash = "a\\\\b"\n' + 'unicode = "café\\\\path"\n' + ).encode(defenc) + ) + + with GitConfigParser(config_path, read_only=False) as config: + config.set_value("unrelated", "key", "value") + + expected = { + "newline": "first\nsecond", + "quote": 'a"b', + "backslash": "a\\b", + "unicode": "café\\path", + } + with GitConfigParser(config_path, read_only=True) as config: + for key, value in expected.items(): + self.assertEqual( + config.get_value("section", key), + value, + "GitPython should preserve values when rewriting unrelated entries", + ) + self.assertEqual( + subprocess.run( + ["git", "config", "--file", config_path, "--get", "section.%s" % key], + stdout=subprocess.PIPE, + check=True, + ).stdout, + value.encode(defenc) + b"\n", + "git should read rewritten values with the same semantics", + ) + + with open(config_path, "rb") as config_file: + contents = config_file.read() + self.assertNotIn(b"\r", contents, "the writer should never emit carriage returns") + self.assertNotIn(b"\x00", contents, "the writer should never emit NUL bytes") + + for name, value in (("return", b"first\\rsecond"), ("nul", b"first\x00second")): + unsafe_path = osp.join(rw_dir, "%s-config" % name) + unsafe_contents = b'[section]\nvalue = "' + value + b'"\n' + with open(unsafe_path, "wb") as config_file: + config_file.write(unsafe_contents) + with self.assertRaisesRegex( + ValueError, + "CR or NUL", + msg="unsafe existing values should abort rewrites", + ): + with GitConfigParser(unsafe_path, read_only=False) as config: + config.set_value("unrelated", "key", "value") + with open(unsafe_path, "rb") as config_file: + self.assertEqual( + config_file.read(), + unsafe_contents, + "rejected rewrites should leave the original file unchanged", + ) + @with_rw_directory def test_set_value_rejects_config_injection(self, rw_dir): config_path = osp.join(rw_dir, "config") @@ -745,15 +816,14 @@ def test_config_with_quotes_with_whitespace_outside_value(self): self.assertEqual(cr.get("init", "defaultBranch"), "trunk") def test_config_with_quotes_containing_escapes(self): - """For now just suppress quote removal. But it would be good to interpret most of these.""" + """Interpret Git's quoted escapes without changing malformed values.""" cr = GitConfigParser(fixture_path("git_config_with_quotes_escapes"), read_only=True) - # These can eventually be supported by substituting the represented character. - self.assertEqual(cr.get("custom", "hasnewline"), R'"first\nsecond"') - self.assertEqual(cr.get("custom", "hasbackslash"), R'"foo\\bar"') - self.assertEqual(cr.get("custom", "hasquote"), R'"ab\"cd"') - self.assertEqual(cr.get("custom", "hastrailingbackslash"), R'"word\\"') - self.assertEqual(cr.get("custom", "hasunrecognized"), R'"p\qrs"') + self.assertEqual(cr.get("custom", "hasnewline"), "first\nsecond") + self.assertEqual(cr.get("custom", "hasbackslash"), R"foo\bar") + self.assertEqual(cr.get("custom", "hasquote"), 'ab"cd') + self.assertEqual(cr.get("custom", "hastrailingbackslash"), "word\\") + self.assertEqual(cr.get("custom", "hasunrecognized"), R"p\qrs") # It is less obvious whether and what to eventually do with this. self.assertEqual(cr.get("custom", "hasunescapedquotes"), '"ab"cd"e"') From 9a92677171dfcca5e1a9bcecfd07aaceabeddee4 Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 11 Aug 2026 14:15:10 +0200 Subject: [PATCH 09/10] fix: decode quoted diff paths in one pass GHSA-v6xg-m7rh-r365 (closed) reports that quoted patch paths can crash or silently change when an escaped literal backslash precedes digits. Add regression coverage distinguishing literal backslashes from real octal byte escapes, then decode Git's C-style quoting sequentially so one escape cannot be reinterpreted by a later pass. Match Git baseline cf5497b14c5a24f10c13f7e0ee85cb95af13ea6a quote.c::unquote_c_style by accepting octal bytes only when all three digits are valid and the first is 0 through 3. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- git/diff.py | 39 +++++++++++++++++++++++++++++---------- test/test_diff.py | 6 ++++++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/git/diff.py b/git/diff.py index d1963b84f..f89f3126f 100644 --- a/git/diff.py +++ b/git/diff.py @@ -95,14 +95,35 @@ class DiffConstants(enum.Enum): :const:`git.INDEX` and :const:`Diffable.INDEX`, as well as :const:`Diffable.Index`. """ -_octal_byte_re = re.compile(rb"\\([0-9]{3})") - -def _octal_repl(matchobj: Match) -> bytes: - value = matchobj.group(1) - value = int(value, 8) - value = bytes(bytearray((value,))) - return value +def _unquote_path(path: bytes) -> bytes: + result = bytearray() + escapes = { + ord("a"): 7, + ord("b"): 8, + ord("f"): 12, + ord("n"): 10, + ord("r"): 13, + ord("t"): 9, + ord("v"): 11, + } + i = 0 + while i < len(path): + if path[i] != ord("\\") or i + 1 == len(path): + result.append(path[i]) + i += 1 + continue + if path[i + 1] in b"0123" and i + 3 < len(path) and all(c in b"01234567" for c in path[i + 2 : i + 4]): + result.append(int(path[i + 1 : i + 4], 8)) + i += 4 + continue + escaped = path[i + 1] + if escaped in escapes or escaped in b'\\"': + result.append(escapes.get(escaped, escaped)) + else: + result.extend(path[i : i + 2]) + i += 2 + return bytes(result) def decode_path(path: bytes, has_ab_prefix: bool = True) -> Optional[bytes]: @@ -110,9 +131,7 @@ def decode_path(path: bytes, has_ab_prefix: bool = True) -> Optional[bytes]: return None if path.startswith(b'"') and path.endswith(b'"'): - path = path[1:-1].replace(b"\\n", b"\n").replace(b"\\t", b"\t").replace(b'\\"', b'"').replace(b"\\\\", b"\\") - - path = _octal_byte_re.sub(_octal_repl, path) + path = _unquote_path(path[1:-1]) if has_ab_prefix: assert path.startswith(b"a/") or path.startswith(b"b/") diff --git a/test/test_diff.py b/test/test_diff.py index d5e14f3de..92f3876c7 100644 --- a/test/test_diff.py +++ b/test/test_diff.py @@ -14,6 +14,7 @@ from git import NULL_TREE, Diff, DiffIndex, Diffable, GitCommandError, Repo, Submodule from git.cmd import Git +from git.diff import decode_path from git.exc import UnsafeOptionError from test.lib import StringProcessAdapter, TestBase, fixture, with_rw_directory @@ -324,6 +325,11 @@ def test_diff_patch_format(self): Diff._index_from_patch_format(self.rorepo, diff_proc) # END for each fixture + def test_decode_path_distinguishes_escaped_backslashes_from_octal_bytes(self): + self.assertEqual(decode_path(b'"foo\\\\899bar"', False), b"foo\\899bar") + self.assertEqual(decode_path(b'"foo\\\\123bar"', False), b"foo\\123bar") + self.assertEqual(decode_path(b'"foo\\123bar"', False), b"fooSbar") + def test_diff_with_spaces(self): data = StringProcessAdapter(fixture("diff_file_with_spaces")) diff_index = Diff._index_from_patch_format(self.rorepo, data) From 751473a5f3221d6f989291cbebcc404353fd3ba8 Mon Sep 17 00:00:00 2001 From: Byron Date: Tue, 11 Aug 2026 14:18:25 +0200 Subject: [PATCH 10/10] fix: parse actor identities without regular expressions GHSA-g5vv-9gxw-82hx reports quadratic backtracking when an actor identity contains a long unterminated email delimiter. Add a regression that exercises a 20,000-character malformed identity, then replace both actor regexes with direct delimiter scans following Git's first-opening, first-closing delimiter behavior. Keep GitPython's whole-string fallback when either delimiter is absent. Reference Git baseline cf5497b14c5a24f10c13f7e0ee85cb95 ident.c::split_ident_line and its invalid-committer cases in t/t9300-fast-import.sh. Also reference gix-actor's signature decoder and lenient identity tests. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 --- doc/source/changes.rst | 13 +++++++++++++ git/util.py | 24 ++++++++---------------- test/test_actor.py | 20 ++++++++++++++++++++ 3 files changed, 41 insertions(+), 16 deletions(-) diff --git a/doc/source/changes.rst b/doc/source/changes.rst index 1a1b8fa12..bd6c471ff 100644 --- a/doc/source/changes.rst +++ b/doc/source/changes.rst @@ -2,6 +2,19 @@ Changelog ========= +3.1.60 +====== + +Security fixes for + +* https://github.com/gitpython-developers/GitPython/security/advisories/GHSA-g5vv-9gxw-82hx + +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.1.60 + 3.1.59 ====== diff --git a/git/util.py b/git/util.py index 02f57c132..b0593feea 100644 --- a/git/util.py +++ b/git/util.py @@ -858,10 +858,6 @@ class Actor: committers and authors or anything with a name and an email as mentioned in the git log entries.""" - # PRECOMPILED REGEX - name_only_regex = re.compile(r"<(.*)>") - name_email_regex = re.compile(r"(.*) <(.*?)>") - # ENVIRONMENT VARIABLES # These are read when creating new commits. env_author_name = "GIT_AUTHOR_NAME" @@ -906,18 +902,14 @@ def _from_string(cls, string: str) -> "Actor": :return: :class:`Actor` """ - m = cls.name_email_regex.search(string) - if m: - name, email = m.groups() - return Actor(name, email) - else: - m = cls.name_only_regex.search(string) - if m: - return Actor(m.group(1), None) - # Assume the best and use the whole string as name. - return Actor(string, None) - # END special case name - # END handle name/email matching + line = string.partition("\n")[0] + left_bracket = line.find("<") + right_bracket = line.find(">", left_bracket + 1) + if left_bracket >= 0 and right_bracket >= 0: + return Actor(line[:left_bracket].rstrip(), line[left_bracket + 1 : right_bracket]) + + # Assume the best and use the whole string as name. + return Actor(string, None) @classmethod def _main_actor( diff --git a/test/test_actor.py b/test/test_actor.py index 5e6635709..baf6545f1 100644 --- a/test/test_actor.py +++ b/test/test_actor.py @@ -27,6 +27,26 @@ def test_from_string_should_handle_just_name(self): self.assertEqual("Michael Trier", a.name) self.assertEqual(None, a.email) + def test_from_string_handles_unterminated_email_without_regex_backtracking(self): + value = "A" * 20_000 + " \n y "), Actor("x", "a")) + + def test_from_string_uses_git_delimiters(self): + for value, expected in ( + ("Name ", Actor("Name", "e>", Actor("Name", "email")), + ("Name", Actor("Name", "email")), + (" <>", Actor("", "")), + ("Name ", Actor("Name email>", None)), + ): + self.assertEqual(Actor._from_string(value), expected) + def test_should_display_representation(self): a = Actor._from_string("Michael Trier ") self.assertEqual('">', repr(a))