Commit 4b4e47f
fix: preserve multiline config values when writing
<!-- agent -->
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 <codex@openai.com>1 parent 9729ed3 commit 4b4e47f
2 files changed
Lines changed: 46 additions & 2 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -705,7 +705,11 @@ def write_section(name: str, section_dict: _OMD) -> None: | |||
| 705 | 705 | continue | |
| 706 | 706 | ||
| 707 | 707 | for v in values: | |
| 708 | - fp.write(("\t%s = %s\n" % (key, self._value_to_string(v).replace("\n", "\n\t"))).encode(defenc)) | ||
| 708 | + value = self._value_to_string(v) | ||
| 709 | + if any(char in value for char in '\n\t\b\\"'): | ||
| 710 | + value = value.replace("\\", "\\\\").replace('"', '\\"') | ||
| 711 | + value = '"%s\\\n"' % value.replace("\n", "\\n").replace("\t", "\\t").replace("\b", "\\b") | ||
| 712 | + fp.write(("\t%s = %s\n" % (key, value)).encode(defenc)) | ||
| 709 | 713 | # END if key is not __name__ | |
| 710 | 714 | ||
| 711 | 715 | # END section writing | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -7,6 +7,7 @@ | |||
| 7 | 7 | import io | |
| 8 | 8 | import os | |
| 9 | 9 | import os.path as osp | |
| 10 | + import subprocess | ||
| 10 | 11 | import sys | |
| 11 | 12 | from unittest import mock | |
| 12 | 13 | ||
@@ -15,7 +16,6 @@ | |||
| 15 | 16 | from git import GitConfigParser | |
| 16 | 17 | from git.config import _OMD, cp | |
| 17 | 18 | from git.util import cwd, rmfile | |
| 18 | - | ||
| 19 | 19 | from test.lib import SkipTest, TestCase, fixture_path, with_rw_directory | |
| 20 | 20 | ||
| 21 | 21 | _tc_lock_fpaths = osp.join(osp.dirname(__file__), "fixtures/*.lock") | |
@@ -150,6 +150,46 @@ def test_config_value_with_trailing_new_line(self): | |||
| 150 | 150 | git_config = GitConfigParser(config_file) | |
| 151 | 151 | git_config.read() # This should not throw an exception | |
| 152 | 152 | ||
| 153 | + @with_rw_directory | ||
| 154 | + def test_rewriting_multiline_value_does_not_create_option(self, rw_dir): | ||
| 155 | + config_path = osp.join(rw_dir, "config") | ||
| 156 | + with open(config_path, "wb") as config_file: | ||
| 157 | + config_file.write(b'[core]\n\tzzz = "A\\nhooksPath = ../evil-hooks\\\n"\n') | ||
| 158 | + | ||
| 159 | + with GitConfigParser(config_path, read_only=False) as git_config: | ||
| 160 | + self.assertEqual(git_config.get_value("core", "zzz"), "A\nhooksPath = ../evil-hooks") | ||
| 161 | + git_config.set_value("user", "name", "Test User") | ||
| 162 | + | ||
| 163 | + with GitConfigParser(config_path, read_only=True) as git_config: | ||
| 164 | + self.assertEqual(git_config.get_value("core", "zzz"), "A\nhooksPath = ../evil-hooks") | ||
| 165 | + self.assertFalse(git_config.has_option("core", "hooksPath")) | ||
| 166 | + self.assertEqual( | ||
| 167 | + subprocess.run(["git", "config", "--file", config_path, "--get", "core.hooksPath"]).returncode, 1 | ||
| 168 | + ) | ||
| 169 | + | ||
| 170 | + @with_rw_directory | ||
| 171 | + def test_writer_escapes_special_characters_without_newline(self, rw_dir): | ||
| 172 | + config_path = osp.join(rw_dir, "config") | ||
| 173 | + values = {"tab": "\tvalue\t", "backspace": "a\bb", "quote": 'a"b', "backslash": "a\\qb"} | ||
| 174 | + | ||
| 175 | + with GitConfigParser(config_path, read_only=False) as git_config: | ||
| 176 | + for key, value in values.items(): | ||
| 177 | + git_config.set_value("section", key, value) | ||
| 178 | + | ||
| 179 | + with GitConfigParser(config_path, read_only=True) as git_config: | ||
| 180 | + for key, value in values.items(): | ||
| 181 | + self.assertEqual(git_config.get_value("section", key), value) | ||
| 182 | + self.assertEqual( | ||
| 183 | + subprocess.run( | ||
| 184 | + ["git", "config", "--file", config_path, "--get", "section.%s" % key], | ||
| 185 | + stdout=subprocess.PIPE, | ||
| 186 | + check=True, | ||
| 187 | + ).stdout, | ||
| 188 | + value.encode() + b"\n", | ||
| 189 | + ) | ||
| 190 | + with open(config_path, "rb") as config_file: | ||
| 191 | + self.assertNotIn(b"\x08", config_file.read()) | ||
| 192 | + | ||
| 153 | 193 | @with_rw_directory | |
| 154 | 194 | def test_set_value_rejects_config_injection(self, rw_dir): | |
| 155 | 195 | config_path = osp.join(rw_dir, "config") | |
0 commit comments