Harden config and Git option validation - #2204
Draft
Byron wants to merge 3 commits into
Draft
Conversation
GHSA-jm78-9fvv-mhgr reports that config option names containing Git syntax can be serialized as unintended directives. A regression test showed that set, set_value, and add_value accepted delimiter, comment, bracket, and whitespace characters in option names. Restrict written option names to GitPython's established safe character set of letters, digits, hyphens, underscores, and dots. This blocks characters that can change config syntax while preserving option names historically supported by the writer and SectionConstraint. A broader audit confirmed that every public option-creating config API and SectionConstraint delegate reaches this validator; no separate config writer sink was found. The behavior was checked against Git cf5497b14, and the full config test module plus dotted-option regression pass.
GHSA-wvpp-8hx9-p66j reports that unsafe-option checks omitted the value joined to a one-character option when split_single_char_options was false. A regression test reproduced the mismatch: GitPython checked only -n even though it emitted a joined -nVALUE token that Git parses as clustered short options. Collect the exact joined token for unsplit one-character keyword arguments so the existing clustered-short-option validation sees every option character. The split form and long-option behavior remain unchanged. A broader audit confirmed that all guarded keyword-forwarding APIs use _option_candidates, including clone, ls-remote, fetch, pull, push, archive, revision, diff, checkout-index, and tag paths. Git cf5497b14 confirms repeated short-option parsing within a joined token. Focused candidate and unsafe-option tests pass.
GHSA-4gmw-gg2m-w46p reports that caller-controlled treeish arguments could be parsed by git read-tree as --index-output and select an arbitrary output path. A regression test showed that from_tree reached Git instead of raising UnsafeOptionError; the same unchecked path was reachable through reset and both merge_tree treeish positions. Add the project-standard unsafe-option guard and explicit opt-out to from_tree, merge_tree, and reset. Check positional and keyword candidates so abbreviations and alternate forwarding forms are covered before read-tree runs. A broader audit found only two read-tree sinks in the codebase; both are now guarded, and reset delegates to the guarded from_tree path. The only remaining index-output use is GitPython's controlled temporary index. Git cf5497b14 confirms read-tree parses this path-taking option before tree arguments. Focused index tests and Ruff checks pass.
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
2 participants
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.
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.
Security advisories
<= 3.1.57. Rejects syntax-bearing config option names before serialization. Patched versions and CVE are not yet assigned.<= 3.1.57. Ensures unsafe-option validation sees joined short-option tokens. Patched versions and CVE are not yet assigned.<= 3.1.57. Guardsread-treeoutput redirection acrossIndexFileentry points. Patched versions and CVE are not yet assigned.Exploit payloads and unnecessary reproduction detail are intentionally omitted from this public summary.
Behavior
Config writers reject option-name characters that can alter config syntax while retaining GitPython's established safe character set. Unsafe Git option checks now inspect the exact joined short-option form emitted when splitting is disabled.
IndexFile.from_tree,reset, andmerge_treereject caller-controlled--index-outputforms unless explicitly opted out.Implementation
The config mutation APIs share a syntax-safe option-name validator.
_option_candidatesnow constructs joined candidates for unsplit one-character kwargs, covering every existing guarded caller. Bothread-treesinks use a shared unsafe-option list, withresetdelegating to the guardedfrom_treepath.Each change includes a regression test and a broader class audit. The audits covered all config option-creating APIs, all unsafe-option guard call sites, and all
read-treeinvocations in the repository; no additional same-class sink was found.Git baseline
Behavior was checked against the local Git reference checkout at commit
cf5497b14c5a24f10c13f7e0ee85cb95af13ea6a(v2.55.0.windows.3-16-gcf5497b14). Its config parser, clustered short-option loop, andread-tree --index-outputparsing informed the GitPython-specific implementations.Validation
pytest -q test/test_config.py test/test_git.py test/test_index.py: 164 passed, 2 skipped, 3 expected xfailedtyping_extensionsgit diff --check: passed