From eefa7e425ed5272eb1c1f7cc2c5f229bd552df98 Mon Sep 17 00:00:00 2001 From: Byron Date: Mon, 10 Aug 2026 14:38:41 +0200 Subject: [PATCH 1/3] 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 2/3] 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 3/3] 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))