Although#1636 caused GitPython to avoid using an untrusted search path on Windows under the most typical circumstances, it turns out that some cases remained. I believe only two remain at this time. This pull request fixes those. The CI status on all commits appears to be as I expected and intended.

This vulnerability was reported privately, initially without a full public writeup, since I wanted to get the pull request out as coordinated/requested, without extra delays. A summary of the vulnerability this fixes is now available in theCVE-2024-22190 advisory. Further information about these changes is present in the commit messages. For the benefit of future readers, I have also since added some more information below, lightly adapted from my original disclosure.

I have found that the fix forCVE-2023-40590 (#1635) that I contributed in#1636 was unfortunately incomplete, missing two cases.

Whengit is run with a shell, setting theNoDefaultCurrentDirectoryInExePath environment variable in the caller's environment is insufficient, because the shell subprocess is what performs the path search. When a shell is used, the child process's environment needs to have that variable set. This does not happen automatically, because while theenv dict is based onos.environ, it is created beforeNoDefaultCurrentDirectoryInExePath is set in the caller environment. Omitting that variable from the child process's environment was intentional, to avoid modifying the operation of child processes more than necessary. (git itself does not need that variable to be set to avoid looking in the current directory to run its own subprocesses, including custom commands.) But it has to be done when a shell is used.

In addition, when GitPython usesbash.exe to run hooks, it does not do anything to avoid finding and runningbash.exe in the current directory, which may be the working tree of an untrusted repository. Because in some reasonable workflows one checks out a branch from an untrusted remote to review it, and such a branch could contain a maliciousbash.exe, this is exploitable. The description ofCVE-2023-40590 had mentioned"And there are other commands executed that should probably be aware of this problem," but I had not given enough attention to that point when working on the fix, nor recognized its full ramifications.

Historically there have been other places in GitPython where a subprocess was created insecurely on Windows, but I believe that is no longer the case. The call totaskkill was removed (#1754), and a call tops does not occur on Windows because it is part of thekill_after_timeout feature ofGit.execute (#1756,#1761). This is not true of the test suite, but when running tests, a more trusted environment can reasonably be assumed: people trust the directory out of which they are intentionally running the test suite, and I don't think any test cases attempt git operations directly in/tmp anymore, as opposed to a subdirectory (#1744,#1759). Therefore I believe these are the only places that need to be changed. I searched for the names of allsubprocess module functions and I did not find other problematic calls.

Theshell=True bug was not detected before, due to a combination of two flaws in the regression tests I wrote in#1636 forCVE-2023-40590. First, I did not commit a test withshell=True. Second, although I had also tried it locally withshell=True, the test contained a shortcoming that prevented it from revealing the vulnerability in that case. It entered a new temporary directory with an impostorgit.exe in it. But whenshell=True, the relevant CWD is that of thecmd.exe shell subprocess, which was therorepo working tree rather than the GitPython process's CWD. My patch clarifies the test, creating a repository explicitly so that all the test logic is plain, and it parameterizes the test to cover substantially more cases, including when the deviousgit.exe is in the repository andgit is run with a shell.

I have likewise undertaken efforts to make the newly introducedbash.exe impostor test robust, tried it out with each of theWinBashStatus cases, and set up the impostor scenario in a way that allows the effect to be observed whether or not a realbash.exe is available and whether or not it is usable.

To avoid code duplication, make intent clearer, and make it easier to avoid introducing new untrusted search path bugs, this patch extracts both the shell and non-shell logic for preventing such bugs to asafer_popen function that can be used likePopen with no special forethought. This function also documents the relevant issues, including the strange but important distinction on Windows between shell and non-shell executable search behavior (i.e., why facilities such asshutil.which or the externalwhere command often don't do what we need, on Windows). Having this in one place should also aid in future improvements to the technique used, such as to mitigate or eliminate the race condition onos.environ thatirwand commented about in#1650. On non-Windows systems,safer_popen is simply an alias forPopen.

BecauseGit.USE_SHELL = True was useful for suppressing undesired console windows in graphical applications on Windows (#126,1c2dd54,#360) prior to2.0.8 (0d93908,#469), and is only much more recently longer documented as recommended for that (#1781,#1782,#1787), it is likely that a significant minority of applications use it on Windows. Searching on GitHub, and the web more broadly, turns up some uses and makes me think this may be so. In contrast, I doubt many people intentionally use GitPython to run hooks. But this happens automatically if it is used in a repository with a pre-commit hook enabled.

EliahKagan added16 commits

December 26, 2023 11:06

Show "not from cwd" test is broken for shell=True

1c65efb

This adds a test_it_executes_git_not_from_cwd case for shell=True.(This case also gives the command as a string, so the test need notbe further special-cased for non-Windows systems, where argumentlists aren't accepted with shell=True.)The test did not attempt to cover the shell=True case before,because I had erroneously assumed it worked similarity. It isactually very different, because when a shell is used, both theshell and the command the shell runs must be found and executed,and because the process creation GitPython performs is that of theshell process, with the state of the shell process being what isrelevant to how the path search is done for the git (or other)command.The code change here does not itself demonstrate that the test isbroken for shell=True, because that case passes. However, manuallyundoing the fix in cmd.py forCVE-2023-40590, which as expectedcauses the preexisting (implicitly shell=False case) to fail, does*not* cause the new shell=True case to fail. That case passes!That passing result in the absence of a fix forCVE-2023-40590 iserroneous, because the cmd.exe shell does search the CWD first whennothing has been done to prevent it.

Correct the "not from cwd" test and add more cases

2b47933

This shows thatCVE-2023-40590 is only partially patched. Thiscommit does not fix the vulnerbility, only the test.The problem is that, when shell=True, the environment of the shellsubprocess, instead of the GitPython process, determines whethersearching for the "git" command will use the current directory.Currently NoDefaultCurrentDirectoryInExePath is set only in thecurrent process, and this is done after the environment for thesubprocess (env) is computed. So when git is an indirect subprocessdue to shell=True, Windows still checks a CWD when looking it up.(Note that this should not be a problem for indirect subprocessesstarted by git itself. When a standard git command is implementedas an external executable, when git runs a custom command, and whenone git command delegates some of its work to another -- e.g."git clone" running git-remote-https -- Git for Windows alreadydoes not search the current directory. Custom git commands thatstart their own git subprocesses could have an analogous pathsearch bug, but this would be separate from the bug in GitPython.)This is an exploitable vulnerability in GitPython. Althoughshell=True is rarer and inherently less secure than the default ofshell=False, it still ought to avoid automatically running anexecutable that may exist only due to having been cloned as part ofan untrusted repository. In addition, historically programs onWindows had sometimes used shell=True as a workaround for consolewindows being displayed for subprocesses, and some such code maystill be in use.Furthermore, even when GitPython's current working directory isoutside the repository being worked on, the Git object in a Repoinstance's "git" attribute holds the repository directory as its"_working_dir" attribute, which Git.execute consults to determinethe value of "cwd" to pass to subprocess.Popen. When the createddirect subprocess is really a shell, this is the CWD where git.exeis looked for before searching PATH directories.This is also why previous, more limited testing (includingaccompanying manual testing with shell=True) didn't discover thebug. Even when modified to test with shell=True, the old test hadthe "impostor" git.exe in the GitPython process's own currentdirectory (which was changed to a temporary directory for the test,where the "impostor" was created, but this was separate from theworking tree of the self.git repository). This was effective forthe shell=False case, but not the shell=True case where theimpostor would be found and run in the repository directory evenwhen it differs from the GitPython process's CWD.

Use SystemRoot instead of WINDIR, to fix tox

c1f6c17

In most cases they will be the same, but WINDIR may be absent (orhave a different value?) in rare cases.The practical reason it matters in GitPython's tests is that toxautomatically passes SystemRoot through to environments on Windows.

Omit CWD in executable search even when shell=True

06bd2c7

Refactor "not from cwd" test for readability

7da9c3b

By using pathlib.Path instead of os.path functions.

Further expand "not from cwd" test re: cmd.exe

865c6e8

On Python versions for whichpython/cpython#101283 is not patched,using Popen with shell=True can find cmd.exe in the currentdirectory on Windows, in the rare case that the ComSpec environmentvariable is not defined.This is not necessarily worth addressing, because it is a bug inCPython rahter than GitPython, because that bug has been patched,and because it is very rare that ComSpec is undefined. However:- Changing the code to avoid it would also make that code simpler.- Patched versions of Python <=3.9 don't have python.org builds.This commit just expands the test to add cases where a repositoryalso has a file cmd.exe and where ComSpec is undefined, showingthat this case is not covered.

Make Git.execute a bit simpler and very slightly more robust

d2506c7

This covers the rare unpatchedpython/cpython#101283 case theprevious commit added tests for, that only applies in the unusualsituation that the ComSpec environment variable is unset and an oldbuild (but this includes downloadable builds and currentactions/setup-python builds) of Python <=3.9 for Windows is in use.The main benefit of this change is really to slightly simplify thecode under test. (It might even be justified to remove theuse_shell_impostor=True test cases at some point.)

Start on test_hook_uses_shell_not_from_cwd

61b4dda

This shows that run_commit_hook is vulnerable to an untrustedsearch path bug on Windows, when running script hooks: bash.exe isrun without setting NoDefaultCurrentDirectoryInExePath or otherwiseexcluding the current directory from the path search.The new test uses a non-bare repo, even though the surroundingtests use bare repos. Although the test also works if the repo isinitialized with `Repo.init(rw_dir, bare=True)`, using a bare repowould obscure how the bug this test reveals would typically beexploited, where a command that uses a hook is run after amalicious bash.exe is checked out into the working tree from anuntrusted branch.Running hooks that are themselves provided by an untrustedrepository or branch is of course never safe. If an attacker candeceive a user into doing that, then this vulnerability is notneeded. Instead, an attack that leverages this untrusted searchpath vulnerability would most likely be of roughly this form:1. The user clones a trusted repository and installs hooks.2. A malicious party offers a contribution to the project.3. The user checks out the malicious party's untrusted branch.4. The user performs an action that runs a hook.The hook the user runs is still trusted, but it runs with themalicious bash.exe found in the current directory (which is theworking tree or perhaps some subdirectory of it).The test added in this commit should, if possible, be improved tobe able to run and detect the bug (or its absence) even when bashis absent from the Windows system and, preferably, also even whenthe WSL bash.exe is present but no WSL distribution is installed.

Omit CWD in search for bash.exe to run hooks on Windows

66ff4c1

This uses the same NoDefaultCurrentDirectoryInExePath technique forthe Popen call in git.index.fun.run_commit_hook on Windows as isalready used in git.cmd.Git.execute.The code is simpler in run_commit_hook because a shell is neverused to run bash.exe. (bash.exe is itself a shell, but we neverrun it *via* a shell by passing shell=True to Popen.) Nonetheless,it may make sense to extract out a helper function both can call.This commit does not do that, so there is some code duplication.

Extract venv management from test_installation

7751436

- Create and use a test.lib.helper.VirtualEnvironment class.- Import and use venv module instead of running "python -m venv".These changes make no significant difference in speed or clarityfor the existing test in test_installation. The reason for thischange is instead to support the use of new per-test virtualenvironments in at least one other test.

Cover absent/no-distro bash.exe in hooks "not from cwd" test

a42ea0a

See comments for details on the test's new implementation and whatit achieves.

Avoid spurious "location may have moved" on Windows

f44524a

The main case where this happens is when tempfile.gettempdir() hasa component in it that uses an 8.3-encoded path, e.g.,C:\Users\Administrator\... -> C:\Users\ADMINI~1\...This is a workaround forpython/cpython#90329. I call realpath onlyonce, when the venv is created, and not on any paths inside thevenv, to make it less likely this masks the problems the warning ismeant for. (For example, if Scripts, or python.exe in it, producedthis even with the venv created as it is now, then that may indictean actual problem.)Note that copying python.exe from Scripts to one level up in thevenv, and changing its name to bash.exe to use it to simulate thebash.exe impostor, as is done in test_hook_uses_shell_not_from_cwd,should not (and does not) produce this warning. If that ever startsto do so, then that should be examined as a sign of brittleness.

Clarify comment in test_hook_uses_shell_not_from_cwd

15ebb25

Extract shared logic for using Popen safely on Windows

c551e91

This creates git.util.safer_popen that, on non-Windows systems, isbound to subprocess.Popen (to avoid introducing unnecessarylatency). On Windows, it is a function that wraps subprocess.Popen,consolidating two pieces of logic that had previously beenduplicated:1. Temporarily setting NoDefaultCurrentDirectoryInExePath in the   calling environment and, when shell=True is used, setting it in   the subprocess environment as well. This prevents executables   specified as single names (which are mainly "git" and, for   hooks, "bash.exe") from being searched for in the current   working directory of GitPython or, when a shell is used, the   current working directory of the shell used to run them.2. Passing the CREATE_NO_WINDOW and CREATE_NEW_PROCESS_GROUP flags   as creationflags. This is not a security measure. It is   indirectly related to safety in that CREATE_NO_WINDOW eliminated   at least some, and possibly all, cases where calling Git.execute   (directly, or indirectly via a dynamic method) with shell=True   conferred an advantage over the inherently more secure default   of shell=False; and CREATE_NEW_PROCESS facilitates some ways of   terminating subprocesses that would otherwise be unavailable,   thereby making resource exhaustion less likely. But really the   reason I included creationflags here is that it seems it should   always be used in the same situations as preventing the current   directory from being searched (and always was), and including it   further reduces code duplication and simplifies calling code.This commit does not improve security or robustness, because thesefeatures were already present. Instead, this moves them to a singlelocation. It also documents them by giving the function bound tosafer_popen on Windows, _safer_popen_windows, a detailed docstring.Because there would otherwise be potential for confusion on thedifferent ways to perform or customize path searches, I have alsoadded a doctring to py_where noting its limited use case and itsrelationship to shutil.which and non-shell search.(The search in _safer_popen_windows is typically a non-shellsearch, which is why it cannot be reimplemented to do its ownlookup by calling an only slightly modified version ofshutil.which, without a risk of breaking some currently workinguses. It may, however, be possible to fix the race condition bydoing something analogous for Windows non-shell search behavior,which is largely but not entirely described in the documentationfor CreateProcessW.)

Move safer_popen from git.util to git.cmd

3eb7c2a

I had originally written it in git.util because it is used from twoseparate modules (git.cmd and git.index.fun) and is arguably thesame sort of thing as remove_password_if_present, in that bothrelate to running processes (and to security) and both are usedfrom multiple modules yet are not meant for use outside GitPython.However, all of this is also true of git.cmd.handle_process_output,which this really has more in common with: it's a utility related*only* to the use of subprocesses, while remove_password_if_presentcan be used for other sanitization. In addition, it also replacesgit.cmd.PROC_CREATIONFLAGS (also imported in git.index.fun) bypassing that to Popen on Windows (since the situations where acustom value of creationflags should be used are the same as thosewhere safer_popen should be called for its primary benefit ofavoiding an untrusted path search when creating the subprocess).safer_popen and its Windows implementation _safer_popen_windows arethus moved from git/util.py to git/cmd.py, with related changes,such as to imports, done everywhere they are needed.

Further clarify comment in test_hook_uses_shell_not_from_cwd

1f3caa3

EliahKagan marked this pull request as ready for review

January 10, 2024 09:59

Byron approved these changes

Jan 10, 2024

View reviewed changes

Copy link

Member

Byron left a comment

Choose a reason for hiding this comment

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

Thanks so much for figuring this out (!!) and for providing not only the fix, but also the tests that exemplify the issue while assuring they are now fixed.

I will create a new release shortly.

Byron merged commitef3192c intogitpython-developers:main

Jan 10, 2024

lettuce-botbot referenced this pull request in lettuce-financial/github-bot-signed-commit

Jan 10, 2024

Update dependency GitPython to v3.1.41 (#25)

f54c26b

[![MendRenovate](https://app.renovatebot.com/images/banner.svg)](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` |[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.40/3.1.41?slim=true)](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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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 [@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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 [@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1792](https://togithub.com/gitpython-developers/GitPython/pull/1792)#### New Contributors- [@&#8203;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)- [@&#8203;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-->

EliahKagan deleted the popen branch

January 10, 2024 19:45

renovatebot referenced this pull request in allenporter/flux-local

Jan 11, 2024

chore(deps): update dependency gitpython to v3.1.41 (#499)

c8741a2

[![MendRenovate](https://app.renovatebot.com/images/banner.svg)](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` |[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.40/3.1.41?slim=true)](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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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 [@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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 [@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1792](https://togithub.com/gitpython-developers/GitPython/pull/1792)#### New Contributors- [@&#8203;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)- [@&#8203;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>

Copy link

MemberAuthor

EliahKagan commentedJan 16, 2024•
edited
Loading

I hope it's okay that I've made a small edit to the advisory to fix some confusing and slightly inaccurate wording that I had missed when first writing it. I'd be pleased to make any further changes if desired (or even undo this change if for some reason it is not considered beneficial).

I've applied the change already atthe advisory on this repository. Forthe global advisory, I cannot apply the change directly; I've submittedgithub/advisory-database#3290.

I've noticed that these changes may be hard to spot quickly in the public web-based interface, so I've also made a gist whoserevision history shows the proposed changes.

EliahKagan mentioned this pull request

Jan 16, 2024

[GHSA-2mqj-m65w-jghx] Untrusted search path under some conditions on Windows allows arbitrary code executiongithub/advisory-database#3290

Merged

Copy link

Member

Byron commentedJan 17, 2024

Thanks a lot, and since the first time I wasn't able to suggest improvements, now that it is further improved, I am even more out of my ability to do so. I am sure, however, that the one reviewing the advisory PR will come to the same conclusion and merge the improvements.

Thanks again for your relentless pursuit of excellence!

EliahKagan mentioned this pull request

Jan 19, 2024

Override bash executable, defaulting to Git for Windows git bash over WSL bash#1791

Open

Copy link

MemberAuthor

EliahKagan commentedJan 19, 2024

The global advisory has been successfully updated.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 7, 2024

Fix mypy error with creationflags in subprocess module

dc95a76

On Windows, Python's subprocess module contains constants useful topass to the `creationflags` parameter of subprocess.Popen. Theseare absent on other platforms, where they are not meaningful.The code was already safe at runtime from any AttributeErrorrelated to these constants, because they were only used ingit.cmd._safer_popen_windows, which was defined on Windows.But ingitpython-developers#1792 I did not write the code in a way where mypy canverify its correctness. So if a regression of the kind mypy can inprinciple catch were to occur, it would be harder to notice it.This refactors the code, keeping the same behavior but expressingit in a way mypy can understand. This consists of two changes:1. Only define _safer_popen_windows when the platform is Windows, placing it in the first branch of the `if` statement. This is needed because mypy will not take the only current call to that nonpublic function being on Windows as sufficient evidence that the platform is always Windows when it is run.2. Determine the platform, for this purpose, using sys.platform instead of os.name. These are far from equivalent in general (see the deprecation rationale for is_<platform> ingitpython-developers#1732, revised ina0fa2bd ingitpython-developers#1787). However, in Python 3 (GitPython no longer supports Python 2), in the specific case of Windows, we have a choice of which to use, as both `sys.platform == "win32"` and `os.name == "nt"`. os.name is "nt" on native Windows, and "posix" on Cygwin. sys.platform is "win32" on native Windows (including 64-bit systems with 64-bit Python builds), and "cygwin" on Cygwin. See:https://docs.python.org/3/library/sys.html#sys.platform This is needed because the type stubs for the subprocess module use this sys.platform check (rather than an os.name check) to determine if the platform is Windows for the purpose of deciding which constants to say the subprocess module defines.I have verified that neither of these changes is enough by itself.

EliahKagan added a commit to EliahKagan/GitPython that referenced this pull request

Mar 29, 2024

Expand USE_SHELL docstring; clarify a test usage

bdabb21

In the USE_SHELL docstring:- Restore the older wording "when executing git commands" rather  than "to execute git commands". I've realized that longer phrase,  which dates back to the introduction of USE_SHELL in1c2dd54, is  clearer, because readers less familiar with GitPython's overall  design and operation will still not be misled into thinking  USE_SHELL ever affects whether GitPython uses an external git  command, versus some other mechanism, to do something.- Give some more information about why USE_SHELL = True is unsafe  (though further revision or clarification may be called for).- Note some non-obvious aspects of USE_SHELL, that some of the way  it interacts with other aspects of GitPython is not part of what  is or has been documented about it, and in practice changes over  time. The first example relates togitpython-developers#1792; the second may help  users understand why code that enables USE_SHELL on Windows, in  addition to being unsafe there, often breaks immediately on other  platforms; the third is included so the warnings in the expanded  docstring are not interpreted as a new commitment that any shell  syntax that may have a *desired* effect in some application will  continue to have the same effect in the future.- Cover a second application that might lead, or have led, to  setting USE_SHELL to True, and explain what to do instead.In test_successful_default_refresh_invalidates_cached_version_info:- Rewrite the commented explanation of a special variant of that  second application, where the usual easy alternatives are not  used because part of the goal of the test is to check a "default"  scenario that does not include either of them. This better  explains why that choice is made in the test, and also hopefully  will prevent anyone from thinking that test is a model for  another situation (which, as noted, is unlikely to be the case  even in unit tests).

JoeWang1127 referenced this pull request in googleapis/sdk-platform-java

Apr 6, 2024

deps: update dependency gitpython to v3.1.41 [security] (#2625)

e41bd8f

[![MendRenovate](https://app.renovatebot.com/images/banner.svg)](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` |[![age](https://developer.mend.io/api/mc/badges/age/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/GitPython/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/GitPython/3.1.40/3.1.41?slim=true)](https://docs.renovatebot.com/merge-confidence/)|[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/GitPython/3.1.40/3.1.41?slim=true)](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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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 [@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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 [@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;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[@&#8203;EliahKagan](https://togithub.com/EliahKagan) in[https://github.com/gitpython-developers/GitPython/pull/1792](https://togithub.com/gitpython-developers/GitPython/pull/1792)#### New Contributors- [@&#8203;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)- [@&#8203;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-->

skupr-anaconda mentioned this pull request

Jul 5, 2024

gitpython 3.1.43AnacondaRecipes/gitpython-feedstock#10

Merged

Labels

None yet

2 participants

Movatterモバイル変換

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Fix two remaining Windows untrusted search path cases#1792

Fix two remaining Windows untrusted search path cases#1792

Uh oh!

Conversation

EliahKagan commentedJan 10, 2024•
edited
Loading

Uh oh!

Uh oh!

Byron left a comment

Choose a reason for hiding this comment

Uh oh!

EliahKagan commentedJan 16, 2024•
edited
Loading

Uh oh!

Uh oh!

Byron commentedJan 17, 2024

Uh oh!

EliahKagan commentedJan 19, 2024

Uh oh!

Uh oh!

Movatterモバイル変換

Uh oh!

Fix two remaining Windows untrusted search path cases#1792

Fix two remaining Windows untrusted search path cases#1792

Uh oh!

Conversation

EliahKagan commentedJan 10, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Byron left a comment

Choose a reason for hiding this comment

Uh oh!

EliahKagan commentedJan 16, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

Byron commentedJan 17, 2024

Uh oh!

EliahKagan commentedJan 19, 2024

Uh oh!

Uh oh!

EliahKagan commentedJan 10, 2024•
edited
Loading

EliahKagan commentedJan 16, 2024•
edited
Loading