Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork937
Fix IndexFile.from_tree on Windows#1751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Merged
Merged
Uh oh!
There was an error while loading.Please reload this page.
Conversation
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
This removes the xfail marks from 8 tests that fail due togitpython-developers#1630,to be fixed in the subsequent commits.This reverts commit6e477e3.
On Windows, a file created by NamedTemporaryFile cannot be reopenedwhile already open, under most circumstances. This applies toattempts to open it both from the original process and from otherprocesses. This differs from the behavior on other operatingsystems, and it works this way to help ensure the file can beautomatically deleted (since having a file open on Windows usuallyprevents the file from being deleted, unlike on other OSes).However, this can cause problems when NamedTemporaryFile is used toproduce a safe filename for accessing with an external process, asIndexFile.from_tree does. On Windows, git subprocess can't open andwrite to the file. This breaks IndexFile.from_tree on Windows. Thisis the cause ofgitpython-developers#1630 -- IndexFile.reset uses IndexFile.from_tree,and the Repo.index property returns an IndexFile instance, soRepo.index.reset fails -- and some other breakages.While passing delete=False to NamedTemporaryFile (and deletingseparately) often overcomes that, it won't fix things here, becauseIndexFile.from_tree uses "git read-tree --index-output=<file>".That needs more than the ability to open the file for write. Itneeds to be able to create or overwrite the given path by renamingan existing file to it. The description of "--index-output=<file>"in git-read-tree(1) notes: The file must allow to be rename(2)ed into from a temporary file that is created next to the usual index file; typically this means it needs to be on the same filesystem as the index file itself, and you need write permission to the directories the index file and index output file are located in.On Windows, more is required: there must be no currently open filewith that path, because on Windows, open files typically cannot bereplaced, just as, on Windows, they typically cannot be deleted(nor renamed). This is to say that passing delete=False toNamedTemporaryFile isn't enough to solve this because it merelycauses NamedTemporaryFile to behave like the lower-level mkstempfunction, leaving the responsibility of deletion to the caller butstill *opening* the file -- and while the file is open, the gitsubprocess can't replace it. Switching to mkstemp, with no furtherchange, would likewise not solve this.This commit is an initial fix for the problem, but not the bestfix. On Windows, it closes the temporary file created byNamedTemporaryFile -- so the file can be opened by name, includingby a git subprocess, and also overwritten, include by a file move.As currently implemented, this is not ideal, as it creates a racecondition, because closing the file actually deletes it. During thetime the file does not exist, the filename may end up being reusedby another call to NamedTemporaryFile, mkstemp, or similar facility(in any process, written in any language) before the git subprocesscan recreate it. Passing delete=False would avoid this, but thendeletion would have to be handled separately, and at that point itis not obvious that NamedTemporaryFile is the best facility to use.This fix, though not yet adequate, confirms the impact ongitpython-developers#1630.The following 8 tests go from failing to passing due to this changeon a local Windows development machine (as listed in the summary atthe end of pytest output):- test/test_docs.py:210 Tutorials.test_references_and_objects- test/test_fun.py:37 TestFun.test_aggressive_tree_merge- test/test_index.py:781 TestIndex.test_compare_write_tree- test/test_index.py:294 TestIndex.test_index_file_diffing- test/test_index.py:182 TestIndex.test_index_file_from_tree- test/test_index.py:232 TestIndex.test_index_merge_tree- test/test_index.py:428 TestIndex.test_index_mutation- test/test_refs.py:218 TestRefs.test_head_resetOn CI, one test still fails, but later and for an unrelated reason:- test/test_index.py:428 TestIndex.test_index_mutationThat `open(fake_symlink_path, "rt")` call raises FileNotFoundError.
The test assumes symlinks are never created on Windows. This oftenturns out to be correct, because core.symlinks defaults to false onWindows. (On some Windows systems, creating them is a privilegedoperation; this can be reconfigured, and unprivileged creation ofsymlinks is often enabled on systems used for development.)However, on a Windows system with core.symlinks set to true, gitwill create symlinks if it can, when checking out entries committedto a repository as symlinks. GitHub Actions runners for Windows dothis ever sinceactions/runner-images#1186(the file is now at images/windows/scripts/build/Install-Git.ps1;the `/o:EnableSymlinks=Enabled` option continues to be passed,causing Git for Windows to be installed with core.symlinks set totrue in the system scope).For now, this adds an xfail marking to test_index_mutation, for theFileNotFoundError raised when a symlink, which is expected not tobe a symlink, is passed to `open`, causing an attempt to open itsnonexistent target. (The check itself might bear refinement: aswritten, it reads the core.symlinks variable from any scope,including the local scope, which at the time of the check willusually be the cloned GitPython directory, where pytest is run.)While adding an import, this commit also improves the grouping andsorting of existing ones.
This lets the Windows subprocess open or rename onto the temporaryfile using a more robust approach than inb12fd4a, avoiding therace condition described there, where the filename could beinadvertently reused between deletion and recreation of the file.This creates a context manager helper for the temporary index fileused in IndexFile.from_tree, whose implementation differs byoperating system:- Delegating straightforwardly to NamedTempoaryFile on POSIX systems where an open file can replaced by having another file renamed to it (just as it can be deleted).- Employing custom logic on Windows, using mkstemp, closing the temporary file without immediately deleting it (so it won't be reused by any process seeking to create a temporary file), and then deleting it on context manager exit.IndexFile.from_tree now calls this helper instead ofNamedTemporaryFile. For convenience, the helper provides the path,i.e. the "name", when entered, so tmp_index is now just that path.(At least for now, this helper is implemented as a nonpublicfunction in the git.index.base module, rather than in thegit.index.util module. If it were public in git.index.util like theother facilities there, then some later changes to it, or its laterremoval, would be a breaking change to GitPython. If it werenonpublic in git.index.util, then this would not be a concern, butit would be unintuitive for it to be accessed from code in thegit.index.base module. In the future, one way to address this mightbe to have one or more nonpublic _util modules with public members.Because it would still be a breaking change to drop existing publicutil modules, that would be more utility modules in total, so sucha change isn't included here just for this one used-once function.)
| "'git read-tree --index-output=...' fails with 'fatal: unable to write new index file'." | ||
| ), | ||
| raises=GitCommandError, | ||
| os.name == "nt" and Git().config("core.symlinks") == "true", |
MemberAuthor
EliahKaganDec 1, 2023 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
I'm unsure if this is the best way for thexfail condition to check thecore.symlinks configuration variable. This will typically run with the CWD as the GitPython repository, thereby including the local scope, and maybe only the system and global scopes should be used. (I had thought to useGitConfigParser, which I think can omit the local scope since#950, but it looks like its use of the system scope islimited on Windows.)
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.
I think GitPython has the same problem thatgitoxidewould have: It fails to see the configuration coming with the git installation itself, which has to be discoveredby askinggit itself.
That's the configuration file that contains these values, on Windows at least. Maybe Windows will also put that value into the repository-local configuration file, which might be the reason this condition works at all.
And since it works, I suppose it's good enough for now?
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.
Maybe Windows will also put that value into the repository-local configuration file, which might be the reason this condition works at all.
At least in the more commonfalse case, I've found that it is coped into the local repository's configuration. But usingGit().config(...) doesn't rely on that. It works outside any repository and finds the system configuration:
(.venv) C:\Users\ek\source\repos\GitPython [fromtree ≡]> pushd ~/tmp(.venv) C:\Users\ek\tmp> pythonPython 3.12.0 (tags/v3.12.0:0fb18b0, Oct 2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32Type "help", "copyright", "credits" or "license" for more information.>>> import git>>> git.Git().config("core.symlinks", show_origin=True)'file:C:/Users/ek/scoop/apps/git/2.43.0/etc/gitconfig\tfalse'(This is the same GitPython as when I run it from theGitPython directory, since the virtual environment is activated and the package is installed in it usingpip install -e ..)
So I think the condition is working becauseconfig is not being treated specially--unlike ifGitConfigParser is used, the command in that condition it really is runninggit config.
(.venv) C:\Users\ek\tmp> pythonPython 3.12.0 (tags/v3.12.0:0fb18b0, Oct 2 2023, 13:03:39) [MSC v.1935 64 bit (AMD64)] on win32Type "help", "copyright", "credits" or "license" for more information.>>> import logging>>> logging.basicConfig(level=logging.INFO)>>> import git>>> git.Git.GIT_PYTHON_TRACE = "full">>> git.Git().config("core.symlinks")INFO:git.cmd:git config core.symlinks -> 0; stdout: 'false''false'Byron approved these changesDec 1, 2023
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.
Thanks a lot for your incredible support!
It's always good to see a PR that removes more than it adds, and this one even doubly so as it adds test-coverage to windows while removingxfails in one fell swoop!
It would be interesting to know if GitPython was previously used on Windows at all, given its shortcomings there, and if there is a user-base that can benefit from these improvements.
| @@ -52,5 +52,6 @@ Contributors are: | |||
| -Joseph Hale <me _at_ jhale.dev> | |||
| -Santos Gallegos <stsewd _at_ proton.me> | |||
| -Wenhan Zhu <wzhu.cosmos _at_ gmail.com> | |||
| -Eliah Kagan <eliah.kagan _at_ gmail.com> | |||
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.
About time :D!
I usenbdime, including on Windows, whichdepends on GitPython. That tool ispretty popular, Ithink including on Windows. My guess is that there are numerous libraries and tools that use GitPython and are used on Windows by many. However, it may be that most of them are doing fine, using parts of GitPython that are working well. Even if many Windows developers use GitPython directly, I don't know what functionality of the library tends in practice to be the most widely used or important. Anyway, if the tests results are any indication--of course a rough one since it's not like each test tests the same amount of significant functionality--most of which are passing on Windows, then I would expect GitPython to be pretty usable on Windows already for many practical applications. |
This was referencedDec 21, 2023
lettuce-botbot referenced this pull request in lettuce-financial/github-bot-signed-commitJan 10, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.40` -> `==3.1.41` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.41`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.41):- fix Windows security issue[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41)The details about the Windows security issue [can be found in thisadvisory](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx).Special thanks go to[@​EliahKagan](https://togithub.com/EliahKagan) who reported theissue and fixed it in a single stroke, while being responsible for anincredible amount of improvements that he contributed over the lastcouple of months ❤️.#### What's Changed- Add `__all__` in git.exc by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1719](https://togithub.com/gitpython-developers/GitPython/pull/1719)- Set submodule update cadence to weekly by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1721](https://togithub.com/gitpython-developers/GitPython/pull/1721)- Never modify sys.path by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1720](https://togithub.com/gitpython-developers/GitPython/pull/1720)- Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1722](https://togithub.com/gitpython-developers/GitPython/pull/1722)- Revise comments, docstrings, some messages, and a bit of code by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1725](https://togithub.com/gitpython-developers/GitPython/pull/1725)- Use zero-argument super() by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1726](https://togithub.com/gitpython-developers/GitPython/pull/1726)- Remove obsolete note in \_iter_packed_refs by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1727](https://togithub.com/gitpython-developers/GitPython/pull/1727)- Reorganize test_util and make xfail marks precise by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1729](https://togithub.com/gitpython-developers/GitPython/pull/1729)- Clarify license and make module top comments more consistent by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1730](https://togithub.com/gitpython-developers/GitPython/pull/1730)- Deprecate compat.is\_<platform>, rewriting all uses by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1732](https://togithub.com/gitpython-developers/GitPython/pull/1732)- Revise and restore some module docstrings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1735](https://togithub.com/gitpython-developers/GitPython/pull/1735)- Make the rmtree callback Windows-only by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1739](https://togithub.com/gitpython-developers/GitPython/pull/1739)- List all non-passing tests in test summaries by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1740](https://togithub.com/gitpython-developers/GitPython/pull/1740)- Document some minor subtleties in test_util.py by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1749](https://togithub.com/gitpython-developers/GitPython/pull/1749)- Always read metadata files as UTF-8 in setup.py by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1748](https://togithub.com/gitpython-developers/GitPython/pull/1748)- Test native Windows on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1745](https://togithub.com/gitpython-developers/GitPython/pull/1745)- Test macOS on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1752](https://togithub.com/gitpython-developers/GitPython/pull/1752)- Let close_fds be True on all platforms by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1753](https://togithub.com/gitpython-developers/GitPython/pull/1753)- Fix IndexFile.from_tree on Windows by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1751](https://togithub.com/gitpython-developers/GitPython/pull/1751)- Remove unused TASKKILL fallback in AutoInterrupt by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1754](https://togithub.com/gitpython-developers/GitPython/pull/1754)- Don't return with operand when conceptually void by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1755](https://togithub.com/gitpython-developers/GitPython/pull/1755)- Group .gitignore entries by purpose by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1758](https://togithub.com/gitpython-developers/GitPython/pull/1758)- Adding dubious ownership handling by[@​marioaag](https://togithub.com/marioaag) in[https://github.com/gitpython-developers/GitPython/pull/1746](https://togithub.com/gitpython-developers/GitPython/pull/1746)- Avoid brittle assumptions about preexisting temporary files in testsby [@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1759](https://togithub.com/gitpython-developers/GitPython/pull/1759)- Overhaul noqa directives by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1760](https://togithub.com/gitpython-developers/GitPython/pull/1760)- Clarify some Git.execute kill_after_timeout limitations by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1761](https://togithub.com/gitpython-developers/GitPython/pull/1761)- Bump actions/setup-python from 4 to 5 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1763](https://togithub.com/gitpython-developers/GitPython/pull/1763)- Don't install black on Cygwin by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1766](https://togithub.com/gitpython-developers/GitPython/pull/1766)- Extract all "import gc" to module level by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1765](https://togithub.com/gitpython-developers/GitPython/pull/1765)- Extract remaining local "import gc" to module level by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1768](https://togithub.com/gitpython-developers/GitPython/pull/1768)- Replace xfail with gc.collect in TestSubmodule.test_rename by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1767](https://togithub.com/gitpython-developers/GitPython/pull/1767)- Enable CodeQL by [@​EliahKagan](https://togithub.com/EliahKagan)in[https://github.com/gitpython-developers/GitPython/pull/1769](https://togithub.com/gitpython-developers/GitPython/pull/1769)- Replace some uses of the deprecated mktemp function by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1770](https://togithub.com/gitpython-developers/GitPython/pull/1770)- Bump github/codeql-action from 2 to 3 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1773](https://togithub.com/gitpython-developers/GitPython/pull/1773)- Run some Windows environment variable tests only on Windows by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1774](https://togithub.com/gitpython-developers/GitPython/pull/1774)- Fix TemporaryFileSwap regression where file_path could not be Path by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1776](https://togithub.com/gitpython-developers/GitPython/pull/1776)- Improve hooks tests by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1777](https://togithub.com/gitpython-developers/GitPython/pull/1777)- Fix if items of Index is of type PathLike by[@​stegm](https://togithub.com/stegm) in[https://github.com/gitpython-developers/GitPython/pull/1778](https://togithub.com/gitpython-developers/GitPython/pull/1778)- Better document IterableObj.iter_items and improve some subclasses by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1780](https://togithub.com/gitpython-developers/GitPython/pull/1780)- Revert "Don't install black on Cygwin" by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1783](https://togithub.com/gitpython-developers/GitPython/pull/1783)- Add missing pip in $PATH on Cygwin CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1784](https://togithub.com/gitpython-developers/GitPython/pull/1784)- Shorten Iterable docstrings and put IterableObj first by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1785](https://togithub.com/gitpython-developers/GitPython/pull/1785)- Fix incompletely revised Iterable/IterableObj docstrings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1786](https://togithub.com/gitpython-developers/GitPython/pull/1786)- Pre-deprecate setting Git.USE_SHELL by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1782](https://togithub.com/gitpython-developers/GitPython/pull/1782)- Deprecate Git.USE_SHELL by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1787](https://togithub.com/gitpython-developers/GitPython/pull/1787)- In handle_process_output don't forward finalizer result by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1788](https://togithub.com/gitpython-developers/GitPython/pull/1788)- Fix mypy warning "Missing return statement" by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1789](https://togithub.com/gitpython-developers/GitPython/pull/1789)- Fix two remaining Windows untrusted search path cases by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1792](https://togithub.com/gitpython-developers/GitPython/pull/1792)#### New Contributors- [@​marioaag](https://togithub.com/marioaag) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1746](https://togithub.com/gitpython-developers/GitPython/pull/1746)- [@​stegm](https://togithub.com/stegm) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1778](https://togithub.com/gitpython-developers/GitPython/pull/1778)**Full Changelog**:gitpython-developers/GitPython@3.1.40...3.1.41</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Disabled by config. Please merge this manually once youare satisfied.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about theseupdates again.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/lettuce-financial/github-bot-signed-commit).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjcuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyNy4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
%5d(https%3a%2f%2frenovatebot.com){kind=link}
renovatebot referenced this pull request in allenporter/flux-localJan 11, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.40` -> `==3.1.41` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.41`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.41):- fix Windows security issue[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41)The details about the Windows security issue [can be found in thisadvisory](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx).Special thanks go to[@​EliahKagan](https://togithub.com/EliahKagan) who reported theissue and fixed it in a single stroke, while being responsible for anincredible amount of improvements that he contributed over the lastcouple of months ❤️.#### What's Changed- Add `__all__` in git.exc by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1719](https://togithub.com/gitpython-developers/GitPython/pull/1719)- Set submodule update cadence to weekly by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1721](https://togithub.com/gitpython-developers/GitPython/pull/1721)- Never modify sys.path by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1720](https://togithub.com/gitpython-developers/GitPython/pull/1720)- Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1722](https://togithub.com/gitpython-developers/GitPython/pull/1722)- Revise comments, docstrings, some messages, and a bit of code by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1725](https://togithub.com/gitpython-developers/GitPython/pull/1725)- Use zero-argument super() by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1726](https://togithub.com/gitpython-developers/GitPython/pull/1726)- Remove obsolete note in \_iter_packed_refs by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1727](https://togithub.com/gitpython-developers/GitPython/pull/1727)- Reorganize test_util and make xfail marks precise by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1729](https://togithub.com/gitpython-developers/GitPython/pull/1729)- Clarify license and make module top comments more consistent by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1730](https://togithub.com/gitpython-developers/GitPython/pull/1730)- Deprecate compat.is\_<platform>, rewriting all uses by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1732](https://togithub.com/gitpython-developers/GitPython/pull/1732)- Revise and restore some module docstrings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1735](https://togithub.com/gitpython-developers/GitPython/pull/1735)- Make the rmtree callback Windows-only by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1739](https://togithub.com/gitpython-developers/GitPython/pull/1739)- List all non-passing tests in test summaries by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1740](https://togithub.com/gitpython-developers/GitPython/pull/1740)- Document some minor subtleties in test_util.py by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1749](https://togithub.com/gitpython-developers/GitPython/pull/1749)- Always read metadata files as UTF-8 in setup.py by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1748](https://togithub.com/gitpython-developers/GitPython/pull/1748)- Test native Windows on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1745](https://togithub.com/gitpython-developers/GitPython/pull/1745)- Test macOS on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1752](https://togithub.com/gitpython-developers/GitPython/pull/1752)- Let close_fds be True on all platforms by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1753](https://togithub.com/gitpython-developers/GitPython/pull/1753)- Fix IndexFile.from_tree on Windows by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1751](https://togithub.com/gitpython-developers/GitPython/pull/1751)- Remove unused TASKKILL fallback in AutoInterrupt by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1754](https://togithub.com/gitpython-developers/GitPython/pull/1754)- Don't return with operand when conceptually void by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1755](https://togithub.com/gitpython-developers/GitPython/pull/1755)- Group .gitignore entries by purpose by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1758](https://togithub.com/gitpython-developers/GitPython/pull/1758)- Adding dubious ownership handling by[@​marioaag](https://togithub.com/marioaag) in[https://github.com/gitpython-developers/GitPython/pull/1746](https://togithub.com/gitpython-developers/GitPython/pull/1746)- Avoid brittle assumptions about preexisting temporary files in testsby [@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1759](https://togithub.com/gitpython-developers/GitPython/pull/1759)- Overhaul noqa directives by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1760](https://togithub.com/gitpython-developers/GitPython/pull/1760)- Clarify some Git.execute kill_after_timeout limitations by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1761](https://togithub.com/gitpython-developers/GitPython/pull/1761)- Bump actions/setup-python from 4 to 5 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1763](https://togithub.com/gitpython-developers/GitPython/pull/1763)- Don't install black on Cygwin by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1766](https://togithub.com/gitpython-developers/GitPython/pull/1766)- Extract all "import gc" to module level by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1765](https://togithub.com/gitpython-developers/GitPython/pull/1765)- Extract remaining local "import gc" to module level by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1768](https://togithub.com/gitpython-developers/GitPython/pull/1768)- Replace xfail with gc.collect in TestSubmodule.test_rename by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1767](https://togithub.com/gitpython-developers/GitPython/pull/1767)- Enable CodeQL by [@​EliahKagan](https://togithub.com/EliahKagan)in[https://github.com/gitpython-developers/GitPython/pull/1769](https://togithub.com/gitpython-developers/GitPython/pull/1769)- Replace some uses of the deprecated mktemp function by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1770](https://togithub.com/gitpython-developers/GitPython/pull/1770)- Bump github/codeql-action from 2 to 3 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1773](https://togithub.com/gitpython-developers/GitPython/pull/1773)- Run some Windows environment variable tests only on Windows by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1774](https://togithub.com/gitpython-developers/GitPython/pull/1774)- Fix TemporaryFileSwap regression where file_path could not be Path by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1776](https://togithub.com/gitpython-developers/GitPython/pull/1776)- Improve hooks tests by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1777](https://togithub.com/gitpython-developers/GitPython/pull/1777)- Fix if items of Index is of type PathLike by[@​stegm](https://togithub.com/stegm) in[https://github.com/gitpython-developers/GitPython/pull/1778](https://togithub.com/gitpython-developers/GitPython/pull/1778)- Better document IterableObj.iter_items and improve some subclasses by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1780](https://togithub.com/gitpython-developers/GitPython/pull/1780)- Revert "Don't install black on Cygwin" by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1783](https://togithub.com/gitpython-developers/GitPython/pull/1783)- Add missing pip in $PATH on Cygwin CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1784](https://togithub.com/gitpython-developers/GitPython/pull/1784)- Shorten Iterable docstrings and put IterableObj first by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1785](https://togithub.com/gitpython-developers/GitPython/pull/1785)- Fix incompletely revised Iterable/IterableObj docstrings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1786](https://togithub.com/gitpython-developers/GitPython/pull/1786)- Pre-deprecate setting Git.USE_SHELL by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1782](https://togithub.com/gitpython-developers/GitPython/pull/1782)- Deprecate Git.USE_SHELL by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1787](https://togithub.com/gitpython-developers/GitPython/pull/1787)- In handle_process_output don't forward finalizer result by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1788](https://togithub.com/gitpython-developers/GitPython/pull/1788)- Fix mypy warning "Missing return statement" by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1789](https://togithub.com/gitpython-developers/GitPython/pull/1789)- Fix two remaining Windows untrusted search path cases by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1792](https://togithub.com/gitpython-developers/GitPython/pull/1792)#### New Contributors- [@​marioaag](https://togithub.com/marioaag) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1746](https://togithub.com/gitpython-developers/GitPython/pull/1746)- [@​stegm](https://togithub.com/stegm) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1778](https://togithub.com/gitpython-developers/GitPython/pull/1778)**Full Changelog**:gitpython-developers/GitPython@3.1.40...3.1.41</details>---### Configuration📅 **Schedule**: Branch creation - At any time (no schedule defined),Automerge - At any time (no schedule defined).🚦 **Automerge**: Enabled.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/allenporter/flux-local).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4xMjcuMCIsInVwZGF0ZWRJblZlciI6IjM3LjEyNy4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
otc-zuulbot pushed a commit to opentelekomcloud-infra/eyes_on_docs that referenced this pull requestMar 6, 2024
Bump gitpython from 3.1.37 to 3.1.41Bumps gitpython from 3.1.37 to 3.1.41.Release notesSourced from gitpython's releases.3.1.41 - fix Windows security issueThe details about the Windows security issue can be found in this advisory.Special thanks go to @EliahKagan who reported the issue and fixed it in a single stroke, while being responsible for an incredible amount of improvements that he contributed over the last couple of months ❤️.What's ChangedAdd __all__ in git.exc by @EliahKagan ingitpython-developers/GitPython#1719Set submodule update cadence to weekly by @EliahKagan ingitpython-developers/GitPython#1721Never modify sys.path by @EliahKagan ingitpython-developers/GitPython#1720Bump git/ext/gitdb from 8ec2390 to ec58b7e by @dependabot ingitpython-developers/GitPython#1722Revise comments, docstrings, some messages, and a bit of code by @EliahKagan ingitpython-developers/GitPython#1725Use zero-argument super() by @EliahKagan ingitpython-developers/GitPython#1726Remove obsolete note in _iter_packed_refs by @EliahKagan ingitpython-developers/GitPython#1727Reorganize test_util and make xfail marks precise by @EliahKagan ingitpython-developers/GitPython#1729Clarify license and make module top comments more consistent by @EliahKagan ingitpython-developers/GitPython#1730Deprecate compat.is_, rewriting all uses by @EliahKagan ingitpython-developers/GitPython#1732Revise and restore some module docstrings by @EliahKagan ingitpython-developers/GitPython#1735Make the rmtree callback Windows-only by @EliahKagan ingitpython-developers/GitPython#1739List all non-passing tests in test summaries by @EliahKagan ingitpython-developers/GitPython#1740Document some minor subtleties in test_util.py by @EliahKagan ingitpython-developers/GitPython#1749Always read metadata files as UTF-8 in setup.py by @EliahKagan ingitpython-developers/GitPython#1748Test native Windows on CI by @EliahKagan ingitpython-developers/GitPython#1745Test macOS on CI by @EliahKagan ingitpython-developers/GitPython#1752Let close_fds be True on all platforms by @EliahKagan ingitpython-developers/GitPython#1753Fix IndexFile.from_tree on Windows by @EliahKagan ingitpython-developers/GitPython#1751Remove unused TASKKILL fallback in AutoInterrupt by @EliahKagan ingitpython-developers/GitPython#1754Don't return with operand when conceptually void by @EliahKagan ingitpython-developers/GitPython#1755Group .gitignore entries by purpose by @EliahKagan ingitpython-developers/GitPython#1758Adding dubious ownership handling by @marioaag ingitpython-developers/GitPython#1746Avoid brittle assumptions about preexisting temporary files in tests by @EliahKagan ingitpython-developers/GitPython#1759Overhaul noqa directives by @EliahKagan ingitpython-developers/GitPython#1760Clarify some Git.execute kill_after_timeout limitations by @EliahKagan ingitpython-developers/GitPython#1761Bump actions/setup-python from 4 to 5 by @dependabot ingitpython-developers/GitPython#1763Don't install black on Cygwin by @EliahKagan ingitpython-developers/GitPython#1766Extract all "import gc" to module level by @EliahKagan ingitpython-developers/GitPython#1765Extract remaining local "import gc" to module level by @EliahKagan ingitpython-developers/GitPython#1768Replace xfail with gc.collect in TestSubmodule.test_rename by @EliahKagan ingitpython-developers/GitPython#1767Enable CodeQL by @EliahKagan ingitpython-developers/GitPython#1769Replace some uses of the deprecated mktemp function by @EliahKagan ingitpython-developers/GitPython#1770Bump github/codeql-action from 2 to 3 by @dependabot ingitpython-developers/GitPython#1773Run some Windows environment variable tests only on Windows by @EliahKagan ingitpython-developers/GitPython#1774Fix TemporaryFileSwap regression where file_path could not be Path by @EliahKagan ingitpython-developers/GitPython#1776Improve hooks tests by @EliahKagan ingitpython-developers/GitPython#1777Fix if items of Index is of type PathLike by @stegm ingitpython-developers/GitPython#1778Better document IterableObj.iter_items and improve some subclasses by @EliahKagan ingitpython-developers/GitPython#1780Revert "Don't install black on Cygwin" by @EliahKagan ingitpython-developers/GitPython#1783Add missing pip in $PATH on Cygwin CI by @EliahKagan ingitpython-developers/GitPython#1784Shorten Iterable docstrings and put IterableObj first by @EliahKagan ingitpython-developers/GitPython#1785Fix incompletely revised Iterable/IterableObj docstrings by @EliahKagan ingitpython-developers/GitPython#1786Pre-deprecate setting Git.USE_SHELL by @EliahKagan ingitpython-developers/GitPython#1782... (truncated)Commitsf288738 bump patch levelef3192c Merge pull request #1792 from EliahKagan/popen1f3caa3 Further clarify comment in test_hook_uses_shell_not_from_cwd3eb7c2a Move safer_popen from git.util to git.cmdc551e91 Extract shared logic for using Popen safely on Windows15ebb25 Clarify comment in test_hook_uses_shell_not_from_cwdf44524a Avoid spurious "location may have moved" on Windowsa42ea0a Cover absent/no-distro bash.exe in hooks "not from cwd" test7751436 Extract venv management from test_installation66ff4c1 Omit CWD in search for bash.exe to run hooks on WindowsAdditional commits viewable in compare viewYou can trigger a rebase of this PR by commenting@dependabot rebase.Dependabot commands and optionsYou can trigger Dependabot actions by commenting on this PR:@dependabot rebase will rebase this PR@dependabot recreate will recreate this PR, overwriting any edits that have been made to it@dependabot merge will merge this PR after your CI passes on it@dependabot squash and merge will squash and merge this PR after your CI passes on it@dependabot cancel merge will cancel a previously requested merge and block automerging@dependabot reopen will reopen this PR if it is closed@dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency@dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)You can disable automated security fix PRs for this repo from the Security Alerts page.NoteAutomatic rebases have been disabled on this pull request as it has been open for over 30 days.Reviewed-by: Vladimir Vshivkov
JoeWang1127 referenced this pull request in googleapis/sdk-platform-javaApr 6, 2024
[](https://renovatebot.com)This PR contains the following updates:| Package | Change | Age | Adoption | Passing | Confidence ||---|---|---|---|---|---|| [GitPython](https://togithub.com/gitpython-developers/GitPython) |`==3.1.40` -> `==3.1.41` |[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|[](https://docs.renovatebot.com/merge-confidence/)|---> [!WARNING]> Some dependencies could not be looked up. Check the DependencyDashboard for more information.### GitHub Vulnerability Alerts####[CVE-2024-22190](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx)### SummaryThis issue exists because of an incomplete fix forCVE-2023-40590. OnWindows, GitPython uses an untrusted search path if it uses a shell torun `git`, as well as when it runs `bash.exe` to interpret hooks. Ifeither of those features are used on Windows, a malicious `git.exe` or`bash.exe` may be run from an untrusted repository.### DetailsAlthough GitPython often avoids executing programs found in an untrustedsearch path since 3.1.33, two situations remain where this still occurs.Either can allow arbitrary code execution under some circumstances.#### When a shell is usedGitPython can be told to run `git` commands through a shell rather thanas direct subprocesses, by passing `shell=True` to any method thataccepts it, or by both setting `Git.USE_SHELL = True` and not passing`shell=False`. Then the Windows `cmd.exe` shell process performs thepath search, and GitPython does not prevent that shell from finding andrunning `git` in the current directory.When GitPython runs `git` directly rather than through a shell, theGitPython process performs the path search, and currently omits thecurrent directory by setting `NoDefaultCurrentDirectoryInExePath` in itsown environment during the `Popen` call. Although the `cmd.exe` shellwill honor this environment variable when present, GitPython does notcurrently pass it into the shell subprocess's environment.Furthermore, because GitPython sets the subprocess CWD to the root of arepository's working tree, using a shell will run a malicious `git.exe`in an untrusted repository even if GitPython itself is run from atrusted location.This also applies if `Git.execute` is called directly with `shell=True`(or after `Git.USE_SHELL = True`) to run any command.#### When hook scripts are runOn Windows, GitPython uses `bash.exe` to run hooks that appear to bescripts. However, unlike when running `git`, no steps are taken to avoidfinding and running `bash.exe` in the current directory.This allows the author of an untrusted fork or branch to cause amalicious `bash.exe` to be run in some otherwise safe workflows. Anexample of such a scenario is if the user installs a trusted hook whileon a trusted branch, then switches to an untrusted feature branch(possibly from a fork) to review proposed changes. If the untrustedfeature branch contains a malicious `bash.exe` and the user's currentworking directory is the working tree, and the user performs an actionthat runs the hook, then although the hook itself is uncorrupted, itruns with the malicious `bash.exe`.Note that, while `bash.exe` is a shell, this is a separate scenario fromwhen `git` is run using the unrelated Windows `cmd.exe` shell.### PoCOn Windows, create a `git.exe` file in a repository. Then create a`Repo` object, and call any method through it (directly or indirectly)that supports the `shell` keyword argument with `shell=True`:```powershellmkdir testrepogit init testrepocp ... testrepo git.exe # Replace "..." with any executable of choice.python -c "import git; print(git.Repo('testrepo').git.version(shell=True))"```The `git.exe` executable in the repository directory will be run.Or use no `Repo` object, but do it from the location with the `git.exe`:```powershellcd testrepopython -c "import git; print(git.Git().version(shell=True))"```The `git.exe` executable in the current directory will be run.For the scenario with hooks, install a hook in a repository, create a`bash.exe` file in the current directory, and perform an operation thatcauses GitPython to attempt to run the hook:```powershellmkdir testrepocd testrepogit initmv .git/hooks/pre-commit.sample .git/hooks/pre-commitcp ... bash.exe # Replace "..." with any executable of choice.echo "Some text" >file.txtgit add file.txtpython -c "import git; git.Repo().index.commit('Some message')"```The `bash.exe` executable in the current directory will be run.### ImpactThe greatest impact is probably in applications that set `Git.USE_SHELL= True` for historical reasons. (Undesired console windows had, in thepast, been created in some kinds of applications, when it was not used.)Such an application may be vulnerable to arbitrary code execution from amalicious repository, even with no other exacerbating conditions. Thisis to say that, if a shell is used to run `git`, the full effect ofCVE-2023-40590 is still present. Furthermore, as noted above, runningthe application itself from a trusted directory is not a sufficientmitigation.An application that does not direct GitPython to use a shell to run`git` subprocesses thus avoids most of the risk. However, there is nosuch straightforward way to prevent GitPython from running `bash.exe` tointerpret hooks. So while the conditions needed for that to be exploitedare more involved, it may be harder to mitigate decisively prior topatching.### Possible solutionsA straightforward approach would be to address each bug directly:- When a shell is used, pass `NoDefaultCurrentDirectoryInExePath` intothe subprocess environment, because in that scenario the subprocess isthe `cmd.exe` shell that itself performs the path search.- Set `NoDefaultCurrentDirectoryInExePath` in the GitPython processenvironment during the `Popen` call made to run hooks with a `bash.exe`subprocess.These need only be done on Windows.---### Release Notes<details><summary>gitpython-developers/GitPython (GitPython)</summary>###[`v3.1.41`](https://togithub.com/gitpython-developers/GitPython/releases/tag/3.1.41):- fix Windows security issue[CompareSource](https://togithub.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41)The details about the Windows security issue [can be found in thisadvisory](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx).Special thanks go to[@​EliahKagan](https://togithub.com/EliahKagan) who reported theissue and fixed it in a single stroke, while being responsible for anincredible amount of improvements that he contributed over the lastcouple of months ❤️.#### What's Changed- Add `__all__` in git.exc by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1719](https://togithub.com/gitpython-developers/GitPython/pull/1719)- Set submodule update cadence to weekly by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1721](https://togithub.com/gitpython-developers/GitPython/pull/1721)- Never modify sys.path by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1720](https://togithub.com/gitpython-developers/GitPython/pull/1720)- Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1722](https://togithub.com/gitpython-developers/GitPython/pull/1722)- Revise comments, docstrings, some messages, and a bit of code by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1725](https://togithub.com/gitpython-developers/GitPython/pull/1725)- Use zero-argument super() by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1726](https://togithub.com/gitpython-developers/GitPython/pull/1726)- Remove obsolete note in \_iter_packed_refs by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1727](https://togithub.com/gitpython-developers/GitPython/pull/1727)- Reorganize test_util and make xfail marks precise by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1729](https://togithub.com/gitpython-developers/GitPython/pull/1729)- Clarify license and make module top comments more consistent by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1730](https://togithub.com/gitpython-developers/GitPython/pull/1730)- Deprecate compat.is\_<platform>, rewriting all uses by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1732](https://togithub.com/gitpython-developers/GitPython/pull/1732)- Revise and restore some module docstrings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1735](https://togithub.com/gitpython-developers/GitPython/pull/1735)- Make the rmtree callback Windows-only by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1739](https://togithub.com/gitpython-developers/GitPython/pull/1739)- List all non-passing tests in test summaries by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1740](https://togithub.com/gitpython-developers/GitPython/pull/1740)- Document some minor subtleties in test_util.py by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1749](https://togithub.com/gitpython-developers/GitPython/pull/1749)- Always read metadata files as UTF-8 in setup.py by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1748](https://togithub.com/gitpython-developers/GitPython/pull/1748)- Test native Windows on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1745](https://togithub.com/gitpython-developers/GitPython/pull/1745)- Test macOS on CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1752](https://togithub.com/gitpython-developers/GitPython/pull/1752)- Let close_fds be True on all platforms by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1753](https://togithub.com/gitpython-developers/GitPython/pull/1753)- Fix IndexFile.from_tree on Windows by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1751](https://togithub.com/gitpython-developers/GitPython/pull/1751)- Remove unused TASKKILL fallback in AutoInterrupt by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1754](https://togithub.com/gitpython-developers/GitPython/pull/1754)- Don't return with operand when conceptually void by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1755](https://togithub.com/gitpython-developers/GitPython/pull/1755)- Group .gitignore entries by purpose by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1758](https://togithub.com/gitpython-developers/GitPython/pull/1758)- Adding dubious ownership handling by[@​marioaag](https://togithub.com/marioaag) in[https://github.com/gitpython-developers/GitPython/pull/1746](https://togithub.com/gitpython-developers/GitPython/pull/1746)- Avoid brittle assumptions about preexisting temporary files in testsby [@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1759](https://togithub.com/gitpython-developers/GitPython/pull/1759)- Overhaul noqa directives by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1760](https://togithub.com/gitpython-developers/GitPython/pull/1760)- Clarify some Git.execute kill_after_timeout limitations by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1761](https://togithub.com/gitpython-developers/GitPython/pull/1761)- Bump actions/setup-python from 4 to 5 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1763](https://togithub.com/gitpython-developers/GitPython/pull/1763)- Don't install black on Cygwin by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1766](https://togithub.com/gitpython-developers/GitPython/pull/1766)- Extract all "import gc" to module level by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1765](https://togithub.com/gitpython-developers/GitPython/pull/1765)- Extract remaining local "import gc" to module level by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1768](https://togithub.com/gitpython-developers/GitPython/pull/1768)- Replace xfail with gc.collect in TestSubmodule.test_rename by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1767](https://togithub.com/gitpython-developers/GitPython/pull/1767)- Enable CodeQL by [@​EliahKagan](https://togithub.com/EliahKagan)in[https://github.com/gitpython-developers/GitPython/pull/1769](https://togithub.com/gitpython-developers/GitPython/pull/1769)- Replace some uses of the deprecated mktemp function by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1770](https://togithub.com/gitpython-developers/GitPython/pull/1770)- Bump github/codeql-action from 2 to 3 by[@​dependabot](https://togithub.com/dependabot) in[https://github.com/gitpython-developers/GitPython/pull/1773](https://togithub.com/gitpython-developers/GitPython/pull/1773)- Run some Windows environment variable tests only on Windows by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1774](https://togithub.com/gitpython-developers/GitPython/pull/1774)- Fix TemporaryFileSwap regression where file_path could not be Path by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1776](https://togithub.com/gitpython-developers/GitPython/pull/1776)- Improve hooks tests by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1777](https://togithub.com/gitpython-developers/GitPython/pull/1777)- Fix if items of Index is of type PathLike by[@​stegm](https://togithub.com/stegm) in[https://github.com/gitpython-developers/GitPython/pull/1778](https://togithub.com/gitpython-developers/GitPython/pull/1778)- Better document IterableObj.iter_items and improve some subclasses by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1780](https://togithub.com/gitpython-developers/GitPython/pull/1780)- Revert "Don't install black on Cygwin" by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1783](https://togithub.com/gitpython-developers/GitPython/pull/1783)- Add missing pip in $PATH on Cygwin CI by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1784](https://togithub.com/gitpython-developers/GitPython/pull/1784)- Shorten Iterable docstrings and put IterableObj first by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1785](https://togithub.com/gitpython-developers/GitPython/pull/1785)- Fix incompletely revised Iterable/IterableObj docstrings by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1786](https://togithub.com/gitpython-developers/GitPython/pull/1786)- Pre-deprecate setting Git.USE_SHELL by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1782](https://togithub.com/gitpython-developers/GitPython/pull/1782)- Deprecate Git.USE_SHELL by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1787](https://togithub.com/gitpython-developers/GitPython/pull/1787)- In handle_process_output don't forward finalizer result by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1788](https://togithub.com/gitpython-developers/GitPython/pull/1788)- Fix mypy warning "Missing return statement" by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1789](https://togithub.com/gitpython-developers/GitPython/pull/1789)- Fix two remaining Windows untrusted search path cases by[@​EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1792](https://togithub.com/gitpython-developers/GitPython/pull/1792)#### New Contributors- [@​marioaag](https://togithub.com/marioaag) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1746](https://togithub.com/gitpython-developers/GitPython/pull/1746)- [@​stegm](https://togithub.com/stegm) made their firstcontribution in[https://github.com/gitpython-developers/GitPython/pull/1778](https://togithub.com/gitpython-developers/GitPython/pull/1778)**Full Changelog**:gitpython-developers/GitPython@3.1.40...3.1.41</details>---### Configuration📅 **Schedule**: Branch creation - "" (UTC), Automerge - At any time (noschedule defined).🚦 **Automerge**: Disabled by config. Please merge this manually once youare satisfied.♻ **Rebasing**: Whenever PR becomes conflicted, or you tick therebase/retry checkbox.🔕 **Ignore**: Close this PR and you won't be reminded about this updateagain.---- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, checkthis box---This PR has been generated by [MendRenovate](https://www.mend.io/free-developer-tools/renovate/). Viewrepository job log[here](https://developer.mend.io/github/googleapis/sdk-platform-java).<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yNjkuMiIsInVwZGF0ZWRJblZlciI6IjM3LjI2OS4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9-->
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Labels
None yet
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.
Uh oh!
There was an error while loading.Please reload this page.
Fixes#1630
IndexFile.from_treeusesNamedTemporaryFileto create a temporary file in the repository's.gitdirectory to pass togit read-treeas the operand of--index-output. But on Windows, the command always fails: thegitsubprocess reports it cannot write to that path.IndexFile.from_treeand everything that uses it are thus broken on Windows. This is the cause of#1630, asIndexFile.resetusesIndexFile.from_tree.git read-treedoes not actually write to thefile itself. Instead, it attempts to rename its own separate temporary file to have its name.git read-treedoes not require that any file exist there already. The reason we create it is to get a filename that was definitely available, and prevent an unintended race condition on the name. Asgit-read-tree(1) says:Using
NamedTemporaryFilefor this works on Unix-like systems, but not on Windows, becausegitneeds to be able to replace the file by renaming its own temporary file to the same name, which is a form of deletion: it deletes the file being replaced.NamedTemporaryFileopens the file, which remains open until the context manager object is exited. But on Windows, one cannot typically delete an open file. (As noted inb12fd4a,NamedTemporaryFileis often unsuitable for files shared with other processes on Windows even when they do not attempt to delete the file.)This fixes the problem on Windows by using
mkstempto create the file, and managing cleanup explicitly. BothNamedTemporaryFileandmkstempwill create a file by opening it in a way that fails if the file already exists (and retrying other names if that happens). In this way they avoid the problems ofmktemp(see alsosmmap#41). Immediately after opening the file for creation to get the name, we close it. When we pass its name forgit read-treeto rename its own temporary file to,gitis able to do so.More information about the problem, and how it is solved, appears in the commit messages. Especially:
mktemp. But this verifies the nature of the bug.test_index_mutationstill fails (later than before, and unlike the other seven affected tests that now pass). It adds a conditionalxfailmark. This is due to a test bug, but I think there are tradeoffs to be considered in deciding how to fix it, so I have not attempted to do so in this PR.I have not added any new test cases related to this bug.
IndexFile.from_treedoes not seem to have any of its own tests, and perhaps should. However, as described in the commit messages (and viewable in diffs removingxfailmarkings), eight tests were already failing due to it. Of those, seven now pass, while one, which tests a number of things, gets past the point where it failed before and fails later, differently and for an unrelated reason.To further confirm specifically that thisfixes#1630, I followed the procedure described there, which currently shows the bug on the tip of the main branch, but produces no error and successfully resets the change in the working tree at the tip of this topic branch: