Skip to content

Commit e226e42

Browse files
codexByron
authored andcommitted
Validate config key 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. Validation: - uv run --with gitdb --with pytest --with pytest-cov --with ddt --with pytest-mock --with pytest-instafail --with pytest-sugar python -m pytest test/test_config.py::TestBase::test_set_value_rejects_unsafe_section_and_option_names --no-cov - uv run --with gitdb --with pytest --with pytest-cov --with ddt --with pytest-mock --with pytest-instafail --with pytest-sugar python -m pytest test/test_config.py --no-cov - uv run --with ruff ruff check git/config.py test/test_config.py - codex review --uncommitted
1 parent b7f5fde commit e226e42

2 files changed

Lines changed: 33 additions & 0 deletions

File tree

git/config.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@
7272
See: https://git-scm.com/docs/git-config#_conditional_includes
7373
"""
7474

75+
UNSAFE_CONFIG_NAME_RE = re.compile(r"[\r\n\x00]")
76+
"""Characters that cannot be safely written in section or option names."""
77+
7578

7679
class MetaParserBuilder(abc.ABCMeta): # noqa: B024
7780
"""Utility class wrapping base-class methods into decorators that assure read-only
@@ -778,6 +781,7 @@ def _assure_writable(self, method_name: str) -> None:
778781

779782
def add_section(self, section: "cp._SectionName") -> None:
780783
"""Assures added options will stay in order."""
784+
self._assure_config_name_safe(section, "section")
781785
return super().add_section(section)
782786

783787
@property
@@ -888,6 +892,10 @@ def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> s
888892
raise ValueError("Git config values must not contain CR, LF, or NUL")
889893
return value_str
890894

895+
def _assure_config_name_safe(self, name: str, label: str) -> None:
896+
if UNSAFE_CONFIG_NAME_RE.search(name):
897+
raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label)
898+
891899
@needs_values
892900
@set_dirty_and_flush_changes
893901
def set(
@@ -896,6 +904,8 @@ def set(
896904
option: str,
897905
value: Union[str, bytes, int, float, bool, None] = None,
898906
) -> None:
907+
self._assure_config_name_safe(section, "section")
908+
self._assure_config_name_safe(option, "option")
899909
if value is not None:
900910
value = self._value_to_string_safe(value)
901911
return super().set(section, option, value)
@@ -920,6 +930,8 @@ def set_value(self, section: str, option: str, value: Union[str, bytes, int, flo
920930
:return:
921931
This instance
922932
"""
933+
self._assure_config_name_safe(section, "section")
934+
self._assure_config_name_safe(option, "option")
923935
value_str = self._value_to_string_safe(value)
924936
if not self.has_section(section):
925937
self.add_section(section)
@@ -948,6 +960,8 @@ def add_value(self, section: str, option: str, value: Union[str, bytes, int, flo
948960
:return:
949961
This instance
950962
"""
963+
self._assure_config_name_safe(section, "section")
964+
self._assure_config_name_safe(option, "option")
951965
value_str = self._value_to_string_safe(value)
952966
if not self.has_section(section):
953967
self.add_section(section)
@@ -968,6 +982,7 @@ def rename_section(self, section: str, new_name: str) -> "GitConfigParser":
968982
"""
969983
if not self.has_section(section):
970984
raise ValueError("Source section '%s' doesn't exist" % section)
985+
self._assure_config_name_safe(new_name, "section")
971986
if self.has_section(new_name):
972987
raise ValueError("Destination section '%s' already exists" % new_name)
973988

test/test_config.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,24 @@ def test_set_value_rejects_config_injection(self, rw_dir):
163163
self.assertFalse(git_config.has_section("user"))
164164
self.assertFalse(git_config.has_section("core"))
165165

166+
@with_rw_directory
167+
def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir):
168+
config_path = osp.join(rw_dir, "config")
169+
bad_keys = ("user]\n[core", "user]\r[core", "user]\x00[core")
170+
171+
with GitConfigParser(config_path, read_only=False) as git_config:
172+
for bad_key in bad_keys:
173+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
174+
git_config.set_value(bad_key, "hooksPath", "/tmp/hooks")
175+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
176+
git_config.set_value("user", bad_key, "/tmp/hooks")
177+
178+
git_config.set_value("user", "name", "safe")
179+
180+
with GitConfigParser(config_path, read_only=True) as git_config:
181+
self.assertEqual(git_config.get_value("user", "name"), "safe")
182+
self.assertFalse(git_config.has_section("core"))
183+
166184
@with_rw_directory
167185
def test_set_and_add_value_reject_unsafe_value_characters(self, rw_dir):
168186
config_path = osp.join(rw_dir, "config")

0 commit comments

Comments
 (0)