Harden config and Git option validation - #2204
Merged
Merged
Conversation
<!-- agent --> 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. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
<!-- agent --> 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. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
<!-- agent --> 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. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
<!-- agent --> GHSA-9rj7-rf2p-w77r reports that Repo.init forwarded git-init options without applying GitPython's unsafe-option policy. A regression showed template and abbreviated option spellings reached Git without an UnsafeOptionError and could create the destination before validation. Add a git-init denylist for template installation and separate Git directory redirection, check keyword options before any path or directory mutation, and provide the standard explicit allow_unsafe_options escape hatch. This preserves trusted uses while rejecting untrusted forwarding by default. An audit against Git cf5497b14c5a24f10c13f7e0ee85cb95af13ea6a (v2.55.0.windows.3-16-gcf5497b14c) confirmed that init and clone are the built-in commands that consume repository template directories. Clone, clone_from, and submodule cloning already share the guarded clone helper; the similarly named commit option only reads a commit-message template. Validated with the focused init regression, the clone/init unsafe-option suite, 185 config/Git/index/clone tests, Ruff, and basedpyright. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
<!-- agent --> GHSA-hh9p-6wh2-4mfc reports that high-level rm and checkout wrappers forwarded pathspec file options without GitPython's unsafe-option policy. A regression showed that both commands surfaced multi-line pathspec data in Git errors, while reset consumed the same caller-selected file without a validation error. The audit also found that reset's positional commit could carry the option before its argument separator. Define one shared unsafe pathspec-file option list and apply it to IndexFile.remove, Head.checkout, and HEAD.reset before invoking Git. Check reset's positional commit as well as keyword options, retain the standard allow_unsafe_options escape hatch for trusted callers, and cover abbreviated long-option spellings. An audit against Git cf5497b14c5a24f10c13f7e0ee85cb95af13ea6a (v2.55.0.windows.3-16-gcf5497b14c) found pathspec-file support in add, checkout/restore, commit, reset, rm, and stash. GitPython has no arbitrary high-level option forwarding to the other commands, and git mv does not support this option. Validated with focused rejection and opt-in tests, 214 affected-module regressions, Ruff, basedpyright, and git diff --check. Assisted-by: GPT 5.6 Co-authored-by: GPT 5.6 <codex@openai.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request hardens GitPython’s higher-level APIs against option-based injection and unsafe filesystem/code-execution vectors by adding targeted unsafe-option validation, stricter config key validation, and regression tests to cover these cases.
Changes:
- Add unsafe-option guarding (with
allow_unsafe_options=Trueescape hatch) forRepo.init,Head.checkout/HEAD.reset, andIndexFileentry points (from_tree,merge_tree,remove,reset). - Reject syntax-bearing Git config option names via a shared validator in
GitConfigParser. - Extend option-candidate generation and add tests to ensure unsafe-option detection considers additional spellings/encodings (including abbreviations and unsplit short-option forms).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
git/cmd.py |
Adds pathspec-from-file unsafe option list and adjusts _option_candidates for improved unsafe-option detection. |
git/config.py |
Enforces a stricter allowed character set for config option names to prevent syntax-altering keys. |
git/index/base.py |
Adds unsafe-option checks and allow_unsafe_options to read-tree-based and pathspec-related index operations. |
git/repo/base.py |
Adds unsafe-option checks and allow_unsafe_options to Repo.init for template / separate git-dir options. |
git/refs/head.py |
Adds unsafe-option checks and allow_unsafe_options to checkout and reset to block pathspec-from-file. |
test/test_config.py |
Adds regression tests for invalid config option names. |
test/test_git.py |
Adds regression tests for _option_candidates behavior and unsafe-option validation coverage. |
test/test_index.py |
Adds regression tests for --index-output and pathspec-from-file blocking/opt-in. |
test/test_refs.py |
Adds regression tests for Head.checkout / HEAD.reset pathspec-from-file blocking/opt-in. |
test/test_repo.py |
Adds regression tests for Repo.init unsafe options blocking/opt-in. |
💡 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.
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.<= 3.1.57. Guards repository-template installation and repository-directory redirection duringRepo.init. Patched versions and CVE are not yet assigned.<= 3.1.57. Prevents high-level pathspec commands from reading caller-selected files through forwarded options. 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.Repo.initrejects unsafe template and separate-Git-directory options before creating or changing filesystem paths.IndexFile.remove,Head.checkout, andHEAD.resetreject caller-controlled pathspec-file input, including abbreviated and option-shaped positional forms. Trusted callers retain an explicitallow_unsafe_options=Trueescape hatch.Implementation
The config mutation APIs share a syntax-safe option-name validator.
_option_candidatesconstructs joined candidates for unsplit one-character kwargs, covering every existing guarded caller. Bothread-treesinks use a shared unsafe-option list, withIndexFile.resetdelegating to the guardedfrom_treepath.Repo.initapplies a dedicated unsafe-option list through the same normalized checker. The pathspec-file fix defines one shared denylist and applies it beforerm,checkout, orreset; reset validates both kwargs and its pre-separator commit argument.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, all
read-treeinvocations, every repository-template consumer exposed by GitPython's high-level APIs, and all native Git commands supporting pathspec-file input. Native Git supports the latter inadd,checkout/restore,commit,reset,rm, andstash; onlyrm,checkout, andresethave GitPython high-level arbitrary-option forwarding, and all three are guarded. GitPython implements high-level add/commit without such forwarding and has no high-level restore/stash wrapper;git mvdoes not accept this option. The intentionally rawGitinterface remains caller-controlled by design.For repository templates, Native Git's built-in consumers are
initandclone; clone, clone-from, and submodule clone paths converge on the guarded clone helper.git svn initis exposed only through the rawGitinterface. Thecommit --templateoption is a commit-message input and is not the same hook-installation class.Git baseline
Behavior was checked against the local Git reference checkout at commit
cf5497b14c5a24f10c13f7e0ee85cb95af13ea6a(v2.55.0.windows.3-16-gcf5497b14c). Its config parser, clustered short-option loop,read-tree --index-outputparsing, repository-template definitions, and pathspec-file implementations informed the GitPython-specific changes.Validation
master-fixture tests deselected, 5 expected xfailedtyping_extensionsgit diff --check: passed