Open
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens GitConfigParser against config injection by rejecting unsafe characters (CR, LF, NUL) in config section and option names before writing, closing a newline-based section header injection vector similar in impact to CVE-2026-42215.
Changes:
- Add a shared unsafe-name regex and
_assure_config_name_safe()guard for section/option identifiers. - Enforce the guard in write paths:
add_section(),set(),set_value(),add_value(), and destination names inrename_section(). - Add a regression test ensuring unsafe section/option names are rejected while safe writes still succeed.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
git/config.py |
Introduces and applies section/option name validation to prevent CR/LF/NUL injection during config writes. |
test/test_config.py |
Adds a regression test for rejecting unsafe section/option names via set_value(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The Python package workflow failed in the Ubuntu Python 3.14 job while running mypy. The error reported that GitConfigParser.add_section() may receive configparser's _SectionName type, which is `str | _UNNAMED_SECTION`, but _assure_config_name_safe() accepted only `str`. Keep the runtime validation unchanged for normal string section and option names, but type the guard for configparser section names and only apply the unsafe-character regex to strings. This preserves compatibility with configparser's section-name type while still rejecting CR, LF, and NUL in real names. 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 ruff ruff check git/config.py
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
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Codex created this PR on behalf of Byron. Byron will review it before any merge decision.
Summary
add_section(),set(),set_value(),add_value(), and destination names inrename_section().Git reference
Git itself rejects config keys containing newlines. I checked Git source commit
94f057755b7941b321fd11fec1b2e3ca5313a4e0, whereconfig.creports invalid keys containing newlines, and localgit version 2.50.1rejects newline-bearing config keys.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-covuv 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-covuv run --with ruff ruff check git/config.py test/test_config.pycodex review --uncommittedcodex review --commit e226e42b38c7684cb10e1d13f264b08977892039