Conversation
git_add kept a special case for files == ["."] that predated the switch from repo.index.add to repo.git.add. GitPython's index.add walks the working tree in Python and has no knowledge of the .git directory, so it staged repository metadata as ./.git/config, ./.git/index and ./.git/logs/HEAD. Both branches now delegate to the git CLI through the same validated path, which skips .git and honours .gitignore. Adds a regression test asserting that staging "." never puts anything under .git into the index.
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.
Description
Fixes #628 (and #2732, which was closed as a duplicate of it).
Server Details
git_add)Motivation and Context
git_addused to call GitPython'srepo.index.add(files). That API walks the working tree in Python instead of shelling out to git, so it knows nothing about the.gitdirectory or.gitignore. Callinggit_addwith["."]staged repository metadata, which is what the reporter saw:I reproduced this against the old implementation. The index ends up holding
./.git/HEAD,./.git/config,./.git/index,./.git/logs/HEADand every loose object. The leading./is the giveaway that these came from the Python tree walk rather than from git.The underlying call was already changed to
repo.git.add("--", *files)in "fix(security): bump vulnerable deps; harden git_add", so the reported behaviour no longer occurs on main. What is left is theif files == ["."]special case that was added earlier to work around it. It is redundant now, and it also skips the path validation the other branch gained.This removes the special case so every input goes through the same validated call that delegates to git. I did not add a
.gitfilter. Git already skips.gitand honours.gitignore, and delegating matches howgit_status,git_diffand the rest of the module work. There was no test covering this, so I added one to keep theindex.addpattern from coming back.How Has This Been Tested?
cd src/git && uv run pytest: 48 passed.uv run ruff check .clean,uv run pyright0 errors.I confirmed the new test fails for the right reason by temporarily restoring the old
repo.index.add(files)body. It fails with the index containing./.git/HEAD,./.git/configand friends.Checked
["."],["./"],["file.txt"]and[".git"]directly. The first three stage working-tree files only, and[".git"]stages nothing. Same as current main.Breaking Changes
None. No client configuration changes needed.
Types of changes
Checklist
Additional context
If you would rather close #628 as already fixed by the earlier hardening commit, that is fair. The regression test is the part I would still want to land.