Skip to content

Commit 74a0d56

Browse files
codexByron
authored andcommitted
Address review feedback for config validation
Copilot review asked to avoid duplicating the CR/LF/NUL pattern between name validation and value validation, and to add regression coverage for all newly guarded write paths. Reuse a single compiled unsafe-character pattern for both config values and section/option names. Extend the regression test to cover add_section(), set(), add_value(), and rename_section() destination names in addition to set_value(). Validation: - uv run --with gitdb --with mypy --with pytest --with pytest-cov --with ddt --with pytest-mock --with pytest-instafail --with pytest-sugar --with pre-commit mypy --python-version=3.14 - 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
1 parent 9c92f22 commit 74a0d56

2 files changed

Lines changed: 17 additions & 4 deletions

File tree

git/config.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@
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."""
75+
UNSAFE_CONFIG_CHARS_RE = re.compile(r"[\r\n\x00]")
76+
"""Characters that cannot be safely written in config names or values."""
7777

7878

7979
class MetaParserBuilder(abc.ABCMeta): # noqa: B024
@@ -888,12 +888,12 @@ def _value_to_string(self, value: Union[str, bytes, int, float, bool]) -> str:
888888

889889
def _value_to_string_safe(self, value: Union[str, bytes, int, float, bool]) -> str:
890890
value_str = self._value_to_string(value)
891-
if re.search(r"[\r\n\x00]", value_str):
891+
if UNSAFE_CONFIG_CHARS_RE.search(value_str):
892892
raise ValueError("Git config values must not contain CR, LF, or NUL")
893893
return value_str
894894

895895
def _assure_config_name_safe(self, name: "cp._SectionName", label: str) -> None:
896-
if isinstance(name, str) and UNSAFE_CONFIG_NAME_RE.search(name):
896+
if isinstance(name, str) and UNSAFE_CONFIG_CHARS_RE.search(name):
897897
raise ValueError("Git config %s names must not contain CR, LF, or NUL" % label)
898898

899899
@needs_values

test/test_config.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,11 +169,24 @@ def test_set_value_rejects_unsafe_section_and_option_names(self, rw_dir):
169169
bad_keys = ("user]\n[core", "user]\r[core", "user]\x00[core")
170170

171171
with GitConfigParser(config_path, read_only=False) as git_config:
172+
git_config.add_section("user")
172173
for bad_key in bad_keys:
174+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
175+
git_config.add_section(bad_key)
176+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
177+
git_config.set(bad_key, "hooksPath", "/tmp/hooks")
178+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
179+
git_config.set("user", bad_key, "/tmp/hooks")
173180
with pytest.raises(ValueError, match="CR, LF, or NUL"):
174181
git_config.set_value(bad_key, "hooksPath", "/tmp/hooks")
175182
with pytest.raises(ValueError, match="CR, LF, or NUL"):
176183
git_config.set_value("user", bad_key, "/tmp/hooks")
184+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
185+
git_config.add_value(bad_key, "hooksPath", "/tmp/hooks")
186+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
187+
git_config.add_value("user", bad_key, "/tmp/hooks")
188+
with pytest.raises(ValueError, match="CR, LF, or NUL"):
189+
git_config.rename_section("user", bad_key)
177190

178191
git_config.set_value("user", "name", "safe")
179192

0 commit comments

Comments
 (0)