Test macOS on CI by EliahKagan · Pull Request #1752 · gitpython-developers/GitPython (original) (raw)
added 5 commits
This now tests on macOS in as well as Ubuntu and Windows.
"macos-latest" is currently macOS 12. Using macOS 13 may be faster.
To compare how much extra wait time Windows and macOS usually need.
This undoes the testing breakage of test_blocking_lock_file for Windows, and gives macOS the same amount of extra time as Windows.
I had expected that macOS, even though it needs more time than Ubuntu, might not need as much extra time as Windows. That turned out not to be the case, in the limited testing done so far on CI. Windows, while still slower than Ubuntu and still too slow to reliably pass the test_blocking_lock_file test, was usually faster than macOS and passed more often.
These relative timings may turn out not to be a trend and only to apply to the current GHA runners. That's probably okay since the adjustment for macOS wasn't present before and is being added to allow newly introduced macOS CI test jobs to pass.
That is all in regard to the very specific issue of the extra time required for the test_blocking_lock_file test after the lock, which makes an assertion about that not taking too long. Regarding the overall time of entire test jobs, macOS 13 seems to have usually been a little faster than macOS 12, so it is retained. Unlike "macos-latest", which currently is macOS 12, "macos-13" will never refer to a later version of the operating system, so the version given in the workflow should be revisited later, at or after the time "macos-latest" becomes a synonym of "macos-13".
lettuce-bot bot referenced this pull request in lettuce-financial/github-bot-signed-commit
renovate bot referenced this pull request in allenporter/flux-local
otc-zuul bot pushed a commit to opentelekomcloud-infra/eyes_on_docs that referenced this pull request
JoeWang1127 referenced this pull request in googleapis/sdk-platform-java
This PR contains the following updates:
| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| GitPython | |||||
==3.1.40 -> ==3.1.41 |
|||||
[!WARNING] Some dependencies could not be looked up. Check the Dependency Dashboard for more information.
GitHub Vulnerability Alerts
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:
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:
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:
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
NoDefaultCurrentDirectoryInExePathinto the subprocess environment, because in that scenario the subprocess is thecmd.exeshell that itself performs the path search. - Set
NoDefaultCurrentDirectoryInExePathin the GitPython process environment during thePopencall made to run hooks with abash.exesubprocess.
These need only be done on Windows.
Release Notes
gitpython-developers/GitPython (GitPython)
- fix Windows security issue
The details about the Windows security issue can be found in this advisory.
Special thanks go to @EliahKagan who reported the issue and fixed it in a single stroke, while being responsible for an incredible amount of improvements that he contributed over the last couple of months ❤️.
What's Changed
- Add
__all__in git.exc by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1719 - Set submodule update cadence to weekly by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1721
- Never modify sys.path by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1720
- Bump git/ext/gitdb from
8ec2390toec58b7eby @dependabot in https://github.com/gitpython-developers/GitPython/pull/1722 - Revise comments, docstrings, some messages, and a bit of code by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1725
- Use zero-argument super() by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1726
- Remove obsolete note in _iter_packed_refs by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1727
- Reorganize test_util and make xfail marks precise by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1729
- Clarify license and make module top comments more consistent by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1730
- Deprecate compat.is_, rewriting all uses by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1732
- Revise and restore some module docstrings by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1735
- Make the rmtree callback Windows-only by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1739
- List all non-passing tests in test summaries by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1740
- Document some minor subtleties in test_util.py by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1749
- Always read metadata files as UTF-8 in setup.py by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1748
- Test native Windows on CI by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1745
- Test macOS on CI by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1752
- Let close_fds be True on all platforms by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1753
- Fix IndexFile.from_tree on Windows by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1751
- Remove unused TASKKILL fallback in AutoInterrupt by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1754
- Don't return with operand when conceptually void by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1755
- Group .gitignore entries by purpose by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1758
- Adding dubious ownership handling by @marioaag in https://github.com/gitpython-developers/GitPython/pull/1746
- Avoid brittle assumptions about preexisting temporary files in tests by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1759
- Overhaul noqa directives by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1760
- Clarify some Git.execute kill_after_timeout limitations by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1761
- Bump actions/setup-python from 4 to 5 by @dependabot in https://github.com/gitpython-developers/GitPython/pull/1763
- Don't install black on Cygwin by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1766
- Extract all "import gc" to module level by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1765
- Extract remaining local "import gc" to module level by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1768
- Replace xfail with gc.collect in TestSubmodule.test_rename by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1767
- Enable CodeQL by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1769
- Replace some uses of the deprecated mktemp function by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1770
- Bump github/codeql-action from 2 to 3 by @dependabot in https://github.com/gitpython-developers/GitPython/pull/1773
- Run some Windows environment variable tests only on Windows by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1774
- Fix TemporaryFileSwap regression where file_path could not be Path by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1776
- Improve hooks tests by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1777
- Fix if items of Index is of type PathLike by @stegm in https://github.com/gitpython-developers/GitPython/pull/1778
- Better document IterableObj.iter_items and improve some subclasses by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1780
- Revert "Don't install black on Cygwin" by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1783
- Add missing pip in $PATH on Cygwin CI by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1784
- Shorten Iterable docstrings and put IterableObj first by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1785
- Fix incompletely revised Iterable/IterableObj docstrings by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1786
- Pre-deprecate setting Git.USE_SHELL by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1782
- Deprecate Git.USE_SHELL by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1787
- In handle_process_output don't forward finalizer result by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1788
- Fix mypy warning "Missing return statement" by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1789
- Fix two remaining Windows untrusted search path cases by @EliahKagan in https://github.com/gitpython-developers/GitPython/pull/1792
New Contributors
- @marioaag made their first contribution in https://github.com/gitpython-developers/GitPython/pull/1746
- @stegm made their first contribution in https://github.com/gitpython-developers/GitPython/pull/1778
Full Changelog: gitpython-developers/GitPython@3.1.40...3.1.41
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. View repository job log here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})