Skip to content

fix: validate config section delimiters#2176

Merged
Byron merged 1 commit into
mainfrom
fix-config-injection
Jul 20, 2026
Merged

fix: validate config section delimiters#2176
Byron merged 1 commit into
mainfrom
fix-config-injection

Conversation

@Byron

@Byron Byron commented Jul 20, 2026

Copy link
Copy Markdown
Member

Tasks

This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.

  • refackiew

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.

Advisory

GHSA-3rp5-jjmw-4wv2

GHSA-3rp5-jjmw-4wv2

Advisory summary

  • Severity: High
  • Package: gitpython (pip)
  • Affected versions: <= 3.1.52
  • Patched versions: Not yet assigned
  • CVE: Not yet assigned

Summary

  • Validate serialized configuration section boundaries at the shared writer entry points.
  • Reject unquoted closing section delimiters while preserving delimiters inside valid quoted subsections.
  • Cover plain and quoted-subsection-shaped unsafe inputs across the public writer APIs, plus a valid quoted-subsection compatibility case.

Git baseline

Compared with Git at a23bace963d508bd96983cc637131392d3face18: basic section parsing terminates on an unquoted closing bracket, while the extended form permits brackets inside quoted subsection text and requires a final bracket after the closing quote.

Validation

  • .venv/bin/pytest test/test_config.py -q — 32 passed, 1 known skip
  • .venv/bin/ruff check git/config.py test/test_config.py
  • .venv/bin/ruff format --check git/config.py test/test_config.py
  • git diff --check
  • Codex commit review completed; its compatibility finding was addressed and the refreshed commit passed review.

@Byron
Byron force-pushed the fix-config-injection branch 2 times, most recently from e43d571 to ecd6844 Compare July 20, 2026 09:35
@Byron
Byron marked this pull request as ready for review July 20, 2026 10:19
Copilot AI review requested due to automatic review settings July 20, 2026 10:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This pull request hardens Git config parsing and writing to better match Git’s own config grammar, with a focus on preventing malformed section delimiters (and related injection-style patterns) from being serialized via GitPython’s public config writer APIs.

Changes:

  • Replaces the config file reader with a Git-grammar state-machine parser and tightens accepted syntax.
  • Canonicalizes/validates section names at write time (including quoted subsections) and serializes values in a canonical escaped form.
  • Expands test coverage for invalid syntax, delimiter-injection attempts, and writer round-trips (including quoted subsections and escape sequences).

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

File Description
git/config.py Implements Git-grammar parsing, canonical section validation, and canonical escaping for config writes.
test/test_config.py Adds/updates tests covering Git-grammar parsing, delimiter rejection, and writer round-trip behavior.
test/fixtures/git_config_with_quotes_escapes Adjusts fixtures to reflect supported escape handling and remove invalid cases.
test/fixtures/git_config_multiple Updates option naming in fixture to comply with stricter Git-grammar option rules.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread git/config.py Outdated
Comment thread git/config.py Outdated
Copilot AI review requested due to automatic review settings July 20, 2026 10:51

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 6 out of 6 changed files in this pull request and generated 1 comment.

Comment thread git/config.py Outdated
@Byron
Byron force-pushed the fix-config-injection branch from a89ad71 to ecd6844 Compare July 20, 2026 11:15
Copilot AI review requested due to automatic review settings July 20, 2026 11:15

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 comments.

Comment thread git/config.py
Comment thread test/test_config.py
Copilot AI review requested due to automatic review settings July 20, 2026 11:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 1 comment.

Comments suppressed due to low confidence (1)

git/config.py:911

  • The section-name quote/bracket scan treats any '"' as starting a quoted subsection, regardless of position. That means inputs like user"] [other" (quote not preceded by whitespace) would be treated as having a quoted ] and pass validation, even though they are not a valid Git quoted-subsection form (<section> "<subsection>"). Tighten the logic so ] is only allowed inside a quoted subsection that starts after whitespace, and reject quotes outside that form (or at least don’t let them suppress the ] check).
                elif in_quotes and char == "\\":
                    escaped = True
                elif char == '"':
                    in_quotes = not in_quotes
                elif char == "]" and not in_quotes:
                    raise ValueError("Git config section names must not contain an unquoted closing bracket")

Comment thread test/test_config.py
Copilot AI review requested due to automatic review settings July 20, 2026 11:50
@Byron
Byron force-pushed the fix-config-injection branch from 9bee003 to aa4c700 Compare July 20, 2026 11:50

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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.

GHSA-3rp5-jjmw-4wv2 identified that configuration section names could alter the
structure of serialized config despite the existing control-character checks.

Reject unquoted closing section delimiters across all writer entry points
while preserving valid delimiters inside quoted subsections and the
existing option-name behavior. Add regression coverage for unsafe plain and
quoted-subsection-shaped names as well as a valid quoted subsection.

Git baseline: a23bace963d508bd96983cc637131392d3face18. Git config parsing
treats an unquoted closing bracket as the end of a basic section header, while
its extended form permits brackets inside a quoted subsection and requires a
final bracket after the closing quote.

Co-authored-by: Sebastian Thiel <[email protected]>
@Byron
Byron force-pushed the fix-config-injection branch from aa4c700 to 1ed1b92 Compare July 20, 2026 12:13
@Byron
Byron merged commit 6a5eb6a into main Jul 20, 2026
30 checks passed
@Byron
Byron deleted the fix-config-injection branch July 20, 2026 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants