Fix two remaining Windows untrusted search path cases #1792

EliahKagan · 2024-01-10T09:52:25Z

This fixes CVE-2024-22190.

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 the CVE-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 for CVE-2023-40590 (#1635) that I contributed in #1636 was unfortunately incomplete, missing two cases.

When git is run with a shell, setting the NoDefaultCurrentDirectoryInExePath 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 the env dict is based on os.environ, it is created before NoDefaultCurrentDirectoryInExePath 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 uses bash.exe to run hooks, it does not do anything to avoid finding and running bash.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 malicious bash.exe, this is exploitable. The description of CVE-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 to taskkill was removed (#1754), and a call to ps does not occur on Windows because it is part of the kill_after_timeout feature of Git.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 all subprocess module functions and I did not find other problematic calls.

The shell=True bug was not detected before, due to a combination of two flaws in the regression tests I wrote in #1636 for CVE-2023-40590. First, I did not commit a test with shell=True. Second, although I had also tried it locally with shell=True, the test contained a shortcoming that prevented it from revealing the vulnerability in that case. It entered a new temporary directory with an impostor git.exe in it. But when shell=True, the relevant CWD is that of the cmd.exe shell subprocess, which was the rorepo 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 devious git.exe is in the repository and git is run with a shell.

I have likewise undertaken efforts to make the newly introduced bash.exe impostor test robust, tried it out with each of the WinBashStatus cases, and set up the impostor scenario in a way that allows the effect to be observed whether or not a real bash.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 a safer_popen function that can be used like Popen 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 as shutil.which or the external where 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 on os.environ that irwand commented about in #1650. On non-Windows systems, safer_popen is simply an alias for Popen.

Because Git.USE_SHELL = True was useful for suppressing undesired console windows in graphical applications on Windows (#126, 1c2dd54, #360) prior to 2.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.

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 not be further special-cased for non-Windows systems, where argument lists 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 is actually very different, because when a shell is used, both the shell and the command the shell runs must be found and executed, and because the process creation GitPython performs is that of the shell process, with the state of the shell process being what is relevant to how the path search is done for the git (or other) command. The code change here does not itself demonstrate that the test is broken for shell=True, because that case passes. However, manually undoing the fix in cmd.py for CVE-2023-40590, which as expected causes 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 for CVE-2023-40590 is erroneous, because the cmd.exe shell does search the CWD first when nothing has been done to prevent it.

This shows that CVE-2023-40590 is only partially patched. This commit does not fix the vulnerbility, only the test. The problem is that, when shell=True, the environment of the shell subprocess, instead of the GitPython process, determines whether searching for the "git" command will use the current directory. Currently NoDefaultCurrentDirectoryInExePath is set only in the current process, and this is done after the environment for the subprocess (env) is computed. So when git is an indirect subprocess due to shell=True, Windows still checks a CWD when looking it up. (Note that this should not be a problem for indirect subprocesses started by git itself. When a standard git command is implemented as an external executable, when git runs a custom command, and when one git command delegates some of its work to another -- e.g. "git clone" running git-remote-https -- Git for Windows already does not search the current directory. Custom git commands that start their own git subprocesses could have an analogous path search bug, but this would be separate from the bug in GitPython.) This is an exploitable vulnerability in GitPython. Although shell=True is rarer and inherently less secure than the default of shell=False, it still ought to avoid automatically running an executable that may exist only due to having been cloned as part of an untrusted repository. In addition, historically programs on Windows had sometimes used shell=True as a workaround for console windows being displayed for subprocesses, and some such code may still be in use. Furthermore, even when GitPython's current working directory is outside the repository being worked on, the Git object in a Repo instance's "git" attribute holds the repository directory as its "_working_dir" attribute, which Git.execute consults to determine the value of "cwd" to pass to subprocess.Popen. When the created direct subprocess is really a shell, this is the CWD where git.exe is looked for before searching PATH directories. This is also why previous, more limited testing (including accompanying manual testing with shell=True) didn't discover the bug. Even when modified to test with shell=True, the old test had the "impostor" git.exe in the GitPython process's own current directory (which was changed to a temporary directory for the test, where the "impostor" was created, but this was separate from the working tree of the self.git repository). This was effective for the shell=False case, but not the shell=True case where the impostor would be found and run in the repository directory even when it differs from the GitPython process's CWD.

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

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

On Python versions for which python/cpython#101283 is not patched, using Popen with shell=True can find cmd.exe in the current directory on Windows, in the rare case that the ComSpec environment variable is not defined. This is not necessarily worth addressing, because it is a bug in CPython 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 repository also has a file cmd.exe and where ComSpec is undefined, showing that this case is not covered.

This covers the rare unpatched python/cpython#101283 case the previous commit added tests for, that only applies in the unusual situation that the ComSpec environment variable is unset and an old build (but this includes downloadable builds and current actions/setup-python builds) of Python <=3.9 for Windows is in use. The main benefit of this change is really to slightly simplify the code under test. (It might even be justified to remove the use_shell_impostor=True test cases at some point.)

This shows that run_commit_hook is vulnerable to an untrusted search path bug on Windows, when running script hooks: bash.exe is run without setting NoDefaultCurrentDirectoryInExePath or otherwise excluding the current directory from the path search. The new test uses a non-bare repo, even though the surrounding tests use bare repos. Although the test also works if the repo is initialized with `Repo.init(rw_dir, bare=True)`, using a bare repo would obscure how the bug this test reveals would typically be exploited, where a command that uses a hook is run after a malicious bash.exe is checked out into the working tree from an untrusted branch. Running hooks that are themselves provided by an untrusted repository or branch is of course never safe. If an attacker can deceive a user into doing that, then this vulnerability is not needed. Instead, an attack that leverages this untrusted search path 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 the malicious bash.exe found in the current directory (which is the working tree or perhaps some subdirectory of it). The test added in this commit should, if possible, be improved to be able to run and detect the bug (or its absence) even when bash is absent from the Windows system and, preferably, also even when the WSL bash.exe is present but no WSL distribution is installed.

This uses the same NoDefaultCurrentDirectoryInExePath technique for the Popen call in git.index.fun.run_commit_hook on Windows as is already used in git.cmd.Git.execute. The code is simpler in run_commit_hook because a shell is never used to run bash.exe. (bash.exe is itself a shell, but we never run 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.

- 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 clarity for the existing test in test_installation. The reason for this change is instead to support the use of new per-test virtual environments in at least one other test.

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

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

This creates git.util.safer_popen that, on non-Windows systems, is bound to subprocess.Popen (to avoid introducing unnecessary latency). On Windows, it is a function that wraps subprocess.Popen, consolidating two pieces of logic that had previously been duplicated: 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 these features were already present. Instead, this moves them to a single location. It also documents them by giving the function bound to safer_popen on Windows, _safer_popen_windows, a detailed docstring. Because there would otherwise be potential for confusion on the different ways to perform or customize path searches, I have also added a doctring to py_where noting its limited use case and its relationship to shutil.which and non-shell search. (The search in _safer_popen_windows is typically a non-shell search, which is why it cannot be reimplemented to do its own lookup by calling an only slightly modified version of shutil.which, without a risk of breaking some currently working uses. It may, however, be possible to fix the race condition by doing something analogous for Windows non-shell search behavior, which is largely but not entirely described in the documentation for CreateProcessW.)

I had originally written it in git.util because it is used from two separate modules (git.cmd and git.index.fun) and is arguably the same sort of thing as remove_password_if_present, in that both relate to running processes (and to security) and both are used from 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_present can be used for other sanitization. In addition, it also replaces git.cmd.PROC_CREATIONFLAGS (also imported in git.index.fun) by passing that to Popen on Windows (since the situations where a custom value of creationflags should be used are the same as those where safer_popen should be called for its primary benefit of avoiding an untrusted path search when creating the subprocess). safer_popen and its Windows implementation _safer_popen_windows are thus moved from git/util.py to git/cmd.py, with related changes, such as to imports, done everywhere they are needed.

Byron

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.

[![Mend Renovate](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 [Compare Source](https://togithub.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41) The details about the Windows security issue [can be found in this advisory](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx). Special thanks go to [@EliahKagan](https://togithub.com/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 Changed - Add `__all__` in git.exc by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1719 - Set submodule update cadence to weekly by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1721 - Never modify sys.path by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1720 - Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1722 - Revise comments, docstrings, some messages, and a bit of code by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1725 - Use zero-argument super() by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1726 - Remove obsolete note in \_iter_packed_refs by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1727 - Reorganize test_util and make xfail marks precise by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1729 - Clarify license and make module top comments more consistent by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1730 - Deprecate compat.is\_<platform>, rewriting all uses by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1732 - Revise and restore some module docstrings by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1735 - Make the rmtree callback Windows-only by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1739 - List all non-passing tests in test summaries by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1740 - Document some minor subtleties in test_util.py by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1749 - Always read metadata files as UTF-8 in setup.py by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1748 - Test native Windows on CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1745 - Test macOS on CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1752 - Let close_fds be True on all platforms by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1753 - Fix IndexFile.from_tree on Windows by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1751 - Remove unused TASKKILL fallback in AutoInterrupt by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1754 - Don't return with operand when conceptually void by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1755 - Group .gitignore entries by purpose by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1758 - Adding dubious ownership handling by [@marioaag](https://togithub.com/marioaag) in [gitpython-developers/GitPython#1746 - Avoid brittle assumptions about preexisting temporary files in tests by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1759 - Overhaul noqa directives by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1760 - Clarify some Git.execute kill_after_timeout limitations by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1761 - Bump actions/setup-python from 4 to 5 by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1763 - Don't install black on Cygwin by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1766 - Extract all "import gc" to module level by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1765 - Extract remaining local "import gc" to module level by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1768 - Replace xfail with gc.collect in TestSubmodule.test_rename by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1767 - Enable CodeQL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1769 - Replace some uses of the deprecated mktemp function by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1770 - Bump github/codeql-action from 2 to 3 by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1773 - Run some Windows environment variable tests only on Windows by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1774 - Fix TemporaryFileSwap regression where file_path could not be Path by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1776 - Improve hooks tests by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1777 - Fix if items of Index is of type PathLike by [@stegm](https://togithub.com/stegm) in [gitpython-developers/GitPython#1778 - Better document IterableObj.iter_items and improve some subclasses by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1780 - Revert "Don't install black on Cygwin" by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1783 - Add missing pip in $PATH on Cygwin CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1784 - Shorten Iterable docstrings and put IterableObj first by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1785 - Fix incompletely revised Iterable/IterableObj docstrings by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1786 - Pre-deprecate setting Git.USE_SHELL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1782 - Deprecate Git.USE_SHELL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1787 - In handle_process_output don't forward finalizer result by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1788 - Fix mypy warning "Missing return statement" by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1789 - Fix two remaining Windows untrusted search path cases by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1792 #### New Contributors - [@marioaag](https://togithub.com/marioaag) made their first contribution in [gitpython-developers/GitPython#1746 - [@stegm](https://togithub.com/stegm) made their first contribution in [gitpython-developers/GitPython#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 you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about these updates again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/lettuce-financial/github-bot-signed-commit).

[![Mend Renovate](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 [Compare Source](https://togithub.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41) The details about the Windows security issue [can be found in this advisory](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx). Special thanks go to [@EliahKagan](https://togithub.com/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 Changed - Add `__all__` in git.exc by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1719 - Set submodule update cadence to weekly by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1721 - Never modify sys.path by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1720 - Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1722 - Revise comments, docstrings, some messages, and a bit of code by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1725 - Use zero-argument super() by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1726 - Remove obsolete note in \_iter_packed_refs by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1727 - Reorganize test_util and make xfail marks precise by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1729 - Clarify license and make module top comments more consistent by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1730 - Deprecate compat.is\_<platform>, rewriting all uses by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1732 - Revise and restore some module docstrings by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1735 - Make the rmtree callback Windows-only by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1739 - List all non-passing tests in test summaries by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1740 - Document some minor subtleties in test_util.py by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1749 - Always read metadata files as UTF-8 in setup.py by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1748 - Test native Windows on CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1745 - Test macOS on CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1752 - Let close_fds be True on all platforms by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1753 - Fix IndexFile.from_tree on Windows by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1751 - Remove unused TASKKILL fallback in AutoInterrupt by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1754 - Don't return with operand when conceptually void by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1755 - Group .gitignore entries by purpose by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1758 - Adding dubious ownership handling by [@marioaag](https://togithub.com/marioaag) in [gitpython-developers/GitPython#1746 - Avoid brittle assumptions about preexisting temporary files in tests by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1759 - Overhaul noqa directives by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1760 - Clarify some Git.execute kill_after_timeout limitations by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1761 - Bump actions/setup-python from 4 to 5 by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1763 - Don't install black on Cygwin by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1766 - Extract all "import gc" to module level by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1765 - Extract remaining local "import gc" to module level by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1768 - Replace xfail with gc.collect in TestSubmodule.test_rename by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1767 - Enable CodeQL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1769 - Replace some uses of the deprecated mktemp function by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1770 - Bump github/codeql-action from 2 to 3 by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1773 - Run some Windows environment variable tests only on Windows by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1774 - Fix TemporaryFileSwap regression where file_path could not be Path by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1776 - Improve hooks tests by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1777 - Fix if items of Index is of type PathLike by [@stegm](https://togithub.com/stegm) in [gitpython-developers/GitPython#1778 - Better document IterableObj.iter_items and improve some subclasses by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1780 - Revert "Don't install black on Cygwin" by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1783 - Add missing pip in $PATH on Cygwin CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1784 - Shorten Iterable docstrings and put IterableObj first by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1785 - Fix incompletely revised Iterable/IterableObj docstrings by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1786 - Pre-deprecate setting Git.USE_SHELL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1782 - Deprecate Git.USE_SHELL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1787 - In handle_process_output don't forward finalizer result by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1788 - Fix mypy warning "Missing return statement" by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1789 - Fix two remaining Windows untrusted search path cases by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1792 #### New Contributors - [@marioaag](https://togithub.com/marioaag) made their first contribution in [gitpython-developers/GitPython#1746 - [@stegm](https://togithub.com/stegm) made their first contribution in [gitpython-developers/GitPython#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 the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/allenporter/flux-local).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

EliahKagan · 2024-01-16T23:32:35Z

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 at the advisory on this repository. For the global advisory, I cannot apply the change directly; I've submitted github/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 whose revision history shows the proposed changes.

Byron · 2024-01-17T06:15:20Z

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 · 2024-01-19T17:49:07Z

The global advisory has been successfully updated.

On Windows, Python's subprocess module contains constants useful to pass to the `creationflags` parameter of subprocess.Popen. These are absent on other platforms, where they are not meaningful. The code was already safe at runtime from any AttributeError related to these constants, because they were only used in git.cmd._safer_popen_windows, which was defined on Windows. But in gitpython-developers#1792 I did not write the code in a way where mypy can verify its correctness. So if a regression of the kind mypy can in principle catch were to occur, it would be harder to notice it. This refactors the code, keeping the same behavior but expressing it 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> in gitpython-developers#1732, revised in a0fa2bd in gitpython-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.

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 in 1c2dd54, 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 to gitpython-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).

[![Mend Renovate](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 Dependency Dashboard for more information. ### GitHub Vulnerability Alerts #### [CVE-2024-22190](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx) ### Summary This issue exists because of an incomplete fix for CVE-2023-40590. On Windows, GitPython uses an untrusted search path if it uses a shell to run `git`, as well as when it runs `bash.exe` to interpret hooks. If either of those features are used on Windows, a malicious `git.exe` or `bash.exe` may be run from an untrusted repository. ### Details Although GitPython often avoids executing programs found in an untrusted search 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 used GitPython can be told to run `git` commands through a shell rather than as direct subprocesses, by passing `shell=True` to any method that accepts it, or by both setting `Git.USE_SHELL = True` and not passing `shell=False`. Then the Windows `cmd.exe` shell process performs the path search, and GitPython does not prevent that shell from finding and running `git` in the current directory. When GitPython runs `git` directly rather than through a shell, the GitPython process performs the path search, and currently omits the current directory by setting `NoDefaultCurrentDirectoryInExePath` in its own environment during the `Popen` call. Although the `cmd.exe` shell will honor this environment variable when present, GitPython does not currently pass it into the shell subprocess's environment. Furthermore, because GitPython sets the subprocess CWD to the root of a repository's working tree, using a shell will run a malicious `git.exe` in an untrusted repository even if GitPython itself is run from a trusted 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 run On Windows, GitPython uses `bash.exe` to run hooks that appear to be scripts. However, unlike when running `git`, no steps are taken to avoid finding and running `bash.exe` in the current directory. This allows the author of an untrusted fork or branch to cause a malicious `bash.exe` to be run in some otherwise safe workflows. An example of such a scenario is if the user installs a trusted hook while on a trusted branch, then switches to an untrusted feature branch (possibly from a fork) to review proposed changes. If the untrusted feature branch contains a malicious `bash.exe` and the user's current working directory is the working tree, and the user performs an action that runs the hook, then although the hook itself is uncorrupted, it runs with the malicious `bash.exe`. Note that, while `bash.exe` is a shell, this is a separate scenario from when `git` is run using the unrelated Windows `cmd.exe` shell. ### PoC On 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`: ```powershell mkdir testrepo git init testrepo cp ... 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`: ```powershell cd testrepo python -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 that causes GitPython to attempt to run the hook: ```powershell mkdir testrepo cd testrepo git init mv .git/hooks/pre-commit.sample .git/hooks/pre-commit cp ... bash.exe # Replace "..." with any executable of choice. echo "Some text" >file.txt git add file.txt python -c "import git; git.Repo().index.commit('Some message')" ``` The `bash.exe` executable in the current directory will be run. ### Impact The greatest impact is probably in applications that set `Git.USE_SHELL = True` for historical reasons. (Undesired console windows had, in the past, been created in some kinds of applications, when it was not used.) Such an application may be vulnerable to arbitrary code execution from a malicious repository, even with no other exacerbating conditions. This is to say that, if a shell is used to run `git`, the full effect of CVE-2023-40590 is still present. Furthermore, as noted above, running the application itself from a trusted directory is not a sufficient mitigation. An application that does not direct GitPython to use a shell to run `git` subprocesses thus avoids most of the risk. However, there is no such straightforward way to prevent GitPython from running `bash.exe` to interpret hooks. So while the conditions needed for that to be exploited are more involved, it may be harder to mitigate decisively prior to patching. ### Possible solutions A straightforward approach would be to address each bug directly: - When a shell is used, pass `NoDefaultCurrentDirectoryInExePath` into the subprocess environment, because in that scenario the subprocess is the `cmd.exe` shell that itself performs the path search. - Set `NoDefaultCurrentDirectoryInExePath` in the GitPython process environment 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 [Compare Source](https://togithub.com/gitpython-developers/GitPython/compare/3.1.40...3.1.41) The details about the Windows security issue [can be found in this advisory](https://togithub.com/gitpython-developers/GitPython/security/advisories/GHSA-2mqj-m65w-jghx). Special thanks go to [@EliahKagan](https://togithub.com/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 Changed - Add `__all__` in git.exc by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1719 - Set submodule update cadence to weekly by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1721 - Never modify sys.path by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1720 - Bump git/ext/gitdb from `8ec2390` to `ec58b7e` by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1722 - Revise comments, docstrings, some messages, and a bit of code by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1725 - Use zero-argument super() by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1726 - Remove obsolete note in \_iter_packed_refs by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1727 - Reorganize test_util and make xfail marks precise by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1729 - Clarify license and make module top comments more consistent by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1730 - Deprecate compat.is\_<platform>, rewriting all uses by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1732 - Revise and restore some module docstrings by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1735 - Make the rmtree callback Windows-only by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1739 - List all non-passing tests in test summaries by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1740 - Document some minor subtleties in test_util.py by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1749 - Always read metadata files as UTF-8 in setup.py by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1748 - Test native Windows on CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1745 - Test macOS on CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1752 - Let close_fds be True on all platforms by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1753 - Fix IndexFile.from_tree on Windows by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1751 - Remove unused TASKKILL fallback in AutoInterrupt by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1754 - Don't return with operand when conceptually void by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1755 - Group .gitignore entries by purpose by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1758 - Adding dubious ownership handling by [@marioaag](https://togithub.com/marioaag) in [gitpython-developers/GitPython#1746 - Avoid brittle assumptions about preexisting temporary files in tests by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1759 - Overhaul noqa directives by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1760 - Clarify some Git.execute kill_after_timeout limitations by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1761 - Bump actions/setup-python from 4 to 5 by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1763 - Don't install black on Cygwin by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1766 - Extract all "import gc" to module level by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1765 - Extract remaining local "import gc" to module level by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1768 - Replace xfail with gc.collect in TestSubmodule.test_rename by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1767 - Enable CodeQL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1769 - Replace some uses of the deprecated mktemp function by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1770 - Bump github/codeql-action from 2 to 3 by [@dependabot](https://togithub.com/dependabot) in [gitpython-developers/GitPython#1773 - Run some Windows environment variable tests only on Windows by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1774 - Fix TemporaryFileSwap regression where file_path could not be Path by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1776 - Improve hooks tests by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1777 - Fix if items of Index is of type PathLike by [@stegm](https://togithub.com/stegm) in [gitpython-developers/GitPython#1778 - Better document IterableObj.iter_items and improve some subclasses by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1780 - Revert "Don't install black on Cygwin" by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1783 - Add missing pip in $PATH on Cygwin CI by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1784 - Shorten Iterable docstrings and put IterableObj first by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1785 - Fix incompletely revised Iterable/IterableObj docstrings by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1786 - Pre-deprecate setting Git.USE_SHELL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1782 - Deprecate Git.USE_SHELL by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1787 - In handle_process_output don't forward finalizer result by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1788 - Fix mypy warning "Missing return statement" by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1789 - Fix two remaining Windows untrusted search path cases by [@EliahKagan](https://togithub.com/EliahKagan) in [gitpython-developers/GitPython#1792 #### New Contributors - [@marioaag](https://togithub.com/marioaag) made their first contribution in [gitpython-developers/GitPython#1746 - [@stegm](https://togithub.com/stegm) made their first contribution in [gitpython-developers/GitPython#1778 **Full Changelog**: gitpython-developers/GitPython@3.1.40...3.1.41 </details> --- ### Configuration 📅 **Schedule**: Branch creation - "" (UTC), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/googleapis/sdk-platform-java).

EliahKagan added 16 commits December 26, 2023 11:06

Use SystemRoot instead of WINDIR, to fix tox

c1f6c17

In most cases they will be the same, but WINDIR may be absent (or have a different value?) in rare cases. The practical reason it matters in GitPython's tests is that tox automatically 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.

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

a42ea0a

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

Clarify comment in test_hook_uses_shell_not_from_cwd

15ebb25

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

Byron merged commit ef3192c into gitpython-developers:main Jan 10, 2024
22 checks passed

EliahKagan deleted the popen branch January 10, 2024 19:45

EliahKagan mentioned this pull request Jan 16, 2024

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

Merged

EliahKagan mentioned this pull request Jan 19, 2024

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

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix two remaining Windows untrusted search path cases #1792

Fix two remaining Windows untrusted search path cases #1792

EliahKagan commented Jan 10, 2024 •

edited

Byron left a comment

EliahKagan commented Jan 16, 2024 •

edited

Byron commented Jan 17, 2024

EliahKagan commented Jan 19, 2024

Fix two remaining Windows untrusted search path cases #1792

Fix two remaining Windows untrusted search path cases #1792

Conversation

EliahKagan commented Jan 10, 2024 • edited

Byron left a comment

Choose a reason for hiding this comment

EliahKagan commented Jan 16, 2024 • edited

Byron commented Jan 17, 2024

EliahKagan commented Jan 19, 2024

EliahKagan commented Jan 10, 2024 •

edited

EliahKagan commented Jan 16, 2024 •

edited