From 54538428f79b0c91ba52cda5229856a6edf7ac06 Mon Sep 17 00:00:00 2001 From: "GPT 5.5" Date: Tue, 5 May 2026 21:34:42 +0800 Subject: [PATCH] Validate config key section names before writing GitConfigParser already rejected CR, LF, and NUL in config values before writing, but section and option names could still reach configparser. Because GitPython writes section headers itself, a newline-bearing section name could split the output into additional headers. Reject CR, LF, and NUL in section and option names on write paths that create or set config keys: add_section(), set(), set_value(), add_value(), and rename_section() destinations. This matches Git config key validation behavior; Git source commit 94f057755b7941b321fd11fec1b2e3ca5313a4e0 reports invalid keys containing newlines from config.c, and local git 2.50.1 rejects newline-bearing config keys. Add a regression test covering unsafe section and option names while preserving safe writes. Co-authored-by: Sebastian Thiel --- git/config.py | 17 ++++++++++++++++- test/test_config.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/git/config.py b/git/config.py index 97ae054e5..82747eadd 100644 --- a/git/config.py +++ b/git/config.py @@ -72,6 +72,9 @@ See: https://git-scm.com/docs/git-config#_conditional_includes """ +UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]") +"""Characters that cannot be safely written in config names or values.""" + class MetaParserBuilder(abc.ABCMeta): # noqa: B024 """Utility class wrapping base-class methods into decorators that assure read-only @@ -778,6 +781,7 @@ def _assure_writable(self, method_name: str) -> None: def add_section(self, section: "cp._SectionName") -> None: """Assures added options will stay in order.""" + self._assure_config_name_safe(section, "section") return super().add_section(section) @property @@ -884,10 +888,14 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str: def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str: value_str = self._value_to_string(value) - if re.search(r"[\r\n\x00]", value_str): + if UNSAFE_CONFIG_CHARS_RE.search(value_str): raise ValueError("Git config values must not contain CR, LF, or NUL") return value_str + def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None: + if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name): + raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label) + @needs_values @set_dirty_and_flush_changes def set( @@ -896,6 +904,8 @@ def set( option: str, value: Union[str, bytes, int, float, bool, None] = None, ) -> None: + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") if value is not None: value = self._value_to_string_safe(value) return super().set(section, option, value) @@ -920,6 +930,8 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) @@ -948,6 +960,8 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo :return: This instance """ + self._assure_config_name_safe(section, "section") + self._assure_config_name_safe(option, "option") value_str = self._value_to_string_safe(value) if not self.has_section(section): self.add_section(section) @@ -968,6 +982,7 @@ def rename_section(self, section: str, new_name: str) -> "GitConfigParser": """ if not self.has_section(section): raise ValueError("Source section '%s' doesn't exist" % section) + self._assure_config_name_safe(new_name, "section") if self.has_section(new_name): raise ValueError("Destination section '%s' already exists" % new_name) diff --git a/test/test_config.py b/test/test_config.py index a9dcdb087..3ddaf0a4b 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -163,6 +163,37 @@ def test_set_value_rejects_config_injection(self, rw_dir): self.assertFalse(git_config.has_section("user")) self.assertFalse(git_config.has_section("core")) + @with_rw_directory + def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir): + config_path = osp.join(rw_dir, "config") + bad_keys = ("user]\n[core", "user]\r[core", "user]\x00[core") + + with GitConfigParser(config_path, read_only=False) as git_config: + git_config.add_section("user") + for bad_key in bad_keys: + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_section(bad_key) + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.set_value("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value(bad_key, "hooksPath", "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.add_value("user", bad_key, "/tmp/hooks") + with pytest.raises(ValueError, match="CR, LF, or NUL"): + git_config.rename_section("user", bad_key) + + git_config.set_value("user", "name", "safe") + + with GitConfigParser(config_path, read_only=True) as git_config: + self.assertEqual(git_config.get_value("user", "name"), "safe") + self.assertFalse(git_config.has_section("core")) + @with_rw_directory def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir): config_path = osp.join(rw_dir, "config")