Preserve Git config value semantics

codex · Byron · commit fa66d1e1fda0 · 2026-08-11T13:29:32.000+02:00
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().
diff --git 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
@@ -170,7 +170,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():
@@ -190,6 +199,54 @@ def test_writer_escapes_special_characters_without_newline(self, rw_dir):
         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()
+            )
+
+        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)
+                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:
+            contents = config_file.read()
+        self.assertNotIn(b"\r", contents)
+        self.assertNotIn(b"\x00", contents)
+
+        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"):
+                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)
+
     @with_rw_directory
     def test_set_value_rejects_config_injection(self, rw_dir):
         config_path = osp.join(rw_dir, "config")
@@ -745,15 +802,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"')
