GeezerVison is showing github.com — Preserve Git config escape semantics by Byron · Pull Request #2213 · gitpython-developers/GitPython
Conversation
Byron
commented
Aug 10, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Reported issue
 Preserve Git semantics for quoted escaped values
When an existing config contains a standard quoted escape such as k = "first\nsecond", _read() retains the quoted spelling because it does not decode single-line quoted values containing escapes. Re-escaping that representation here rewrites it as data, so an unrelated update makes Git see literal outer quotes and a literal \n rather than the original newline; quoted backslashes and escaped quotes are similarly altered. Normalize these parsed values before escaping, or preserve their original representation.
Keep non-ASCII text intact through the escape decoder
When a value combines non-ASCII text with any trigger for this branch, such as café\path or a non-ASCII value containing a tab, this forced multiline form is later processed by _read()'s string_decode, which encodes using the filesystem encoding and decodes with unicode_escape. On UTF-8 systems that changes café to café; GitPython therefore returns corrupted data after the first write, and a later unrelated write persists the mojibake into the config file. Escape decoding needs to operate without reinterpreting the encoded non-ASCII bytes.
Also:
The new writer escapes LF/tab/backspace/quote/backslash, but it still allows carriage returns and NULs to be written verbatim. Because the reader’s string_decode(...).decode('unicode_escape') can materialize \r or \x00 into actual \r/\x00 characters in-memory, an attacker-controlled config could be rewritten into a file containing unsafe control characters when an unrelated write occurs. Please either (a) escape \r during serialization (similar to \n) and (b) reject NULs on write, or otherwise ensure \r/NUL cannot reach disk.
Fix
- Decode Git's quoted value escapes without passing non-ASCII text through
unicode_escape.
- Normalize existing single-line quoted escaped values before serialization.
- Reject parsed CR and NUL values before opening or truncating the destination config.
Git behavior was checked against cf5497b14c5a, specifically config.c::parse_value().
Validation
pytest -q test/test_config.py — 37 passed, 2 skippedruff check git/config.py test/test_config.pyruff format --check git/config.py test/test_config.pymypy git/config.pygit diff --checkpre-commit run --all-files- Codex review completed for
c35c2ac8; its P1 pre-truncation validation finding was fixed.
- Codex review completed without findings for
1c015bd5.
- Codex review completed without findings for
ef3fb518 after the CI lint fix.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
chatgpt-codex-connector[bot] reacted with thumbs up emoji
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR updates GitConfigParser’s read/write behavior to preserve Git’s config quoted-escape semantics (e.g., \n, \", \\) without corrupting non-ASCII text, and adds safety checks to prevent rewriting configs that would emit unsafe control characters.
Changes:
- Replace the
unicode_escape-based escape decoder with a Git-oriented backslash escape translation that avoids non-ASCII mojibake.
- Decode well-formed single-line quoted values with backslash escapes, while preserving malformed quoted values containing unescaped quotes.
- Add write-time validation to abort rewrites if any stored value contains CR or NUL; expand writer quoting triggers and add regression tests.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File |
Description |
git/config.py |
Adjusts quoted escape decoding, expands quoting rules during serialization, and rejects CR/NUL values before truncating/writing. |
test/test_config.py |
Extends writer escaping tests and adds coverage for preserving escaped + non-ASCII values, plus CR/NUL rewrite rejection. |
Suppressed comments (1)
test/test_config.py:236
- The subprocess assertion compares
git config --get output bytes to value.encode() (UTF-8). Since GitConfigParser writes the file using defenc, and git config will output the raw bytes it reads, this comparison should also use defenc to avoid false failures when defenc != 'utf-8'.
self.assertEqual(
subprocess.run(
["git", "config", "--file", config_path, "--get", "section.%s" % key],
stdout=subprocess.PIPE,
check=True,
).stdout,
value.encode() + b"\n",
"git should read rewritten values with the same semantics",
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
<!-- agent -->
Decode Git-supported quoted value escapes directly instead of routing UTF-8 text
through Python unicode_escape. This preserves newlines, quotes, backslashes, and
non-ASCII text when an unrelated config update rewrites existing values.
Quote values containing Git comment delimiters (# and ;) or leading/trailing
whitespace so Git does not truncate or trim their data. Escape LF, tab,
backspace, quote, and backslash, while rejecting carriage returns and NULs
before opening the destination.
Regression coverage round-trips these values through both GitPython and git
config and verifies unsafe control characters cannot alter the original file.
Behavior follows Git config.c parse_value() and write_pair().
Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
git/config.py:786
- In the CR/NUL preflight check,
_value_to_string(raw_value) is called twice per value. This is redundant work and can be surprisingly expensive if _value_to_string does non-trivial formatting. Compute it once and reuse it for both checks.
for raw_value in values:
if "\r" in self._value_to_string(raw_value) or "\x00" in self._value_to_string(raw_value):
raise ValueError("Git config values must not contain CR or NUL")
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Using # or ; in config values
Uh oh!
There was an error while loading. Please reload this page.
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.
|