GeezerVison is showing github.com — Resolve Windows hook Bash through PATH by Byron · Pull Request #2199 · gitpython-developers/GitPython
Conversation
Byron
commented
Jul 30, 2026
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by Codex GPT-5.
Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Summary
Fixes #2198.
On Windows, resolve bash.exe from PATH before launching extensionless commit hooks. This prevents CreateProcess from selecting the WSL launcher in System32 ahead of Git for Windows Bash. The current directory is deliberately excluded from resolution so a repository-local executable cannot be selected. If no Bash is present on PATH, GitPython retains its existing bare-name fallback.
Baselines
Git for Windows v2.55.0.windows.3 resolves its shell through PATH: git var GIT_SHELL_PATH returned C:/Program Files/Git/usr/bin/sh.exe in the reference checkout at /c/Users/byron/Desktop/dev/git.
The gix-command reference delegates default-shell selection to gix_path::env::shell_command(). On Windows, gix first locates a Git-associated shell from the selected Git installation (bin, then usr/bin) and falls back to bare sh.exe. This fix adopts the same key safety property—resolve before spawning and retain a bare fallback—while preserving GitPython's established Bash invocation.
Validation
test/test_index.py: 30 passed, 1 expected xfail, 1 xpass- Focused hook tests: 7 passed
ruff check .ruff format --check .git diff --checkcodex review --commit 531f5dc242efc2b4f76668f3d322d5fd3940f213: no actionable findings
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
<!-- agent -->
Extensionless commit hooks on Windows were launched through a bare bash.exe.
Windows CreateProcess searches system directories before PATH for bare
executable names, so the WSL launcher in System32 could preempt Git for Windows
Bash and make every commit fail when no WSL distribution was installed.
A regression test mocks PATH resolution and demonstrates that the hook command
previously remained bare instead of using the resolved executable. Resolve
bash.exe with shutil.which before spawning, while retaining the bare-name
fallback when PATH has no Bash so existing WSL-based behavior remains available.
This matches Git for Windows v2.55.0.windows.3, whose git var GIT_SHELL_PATH
reports the absolute PATH-selected sh.exe. It also follows gix-command's
principle of resolving a Git-associated Windows shell before falling back to a
bare executable name.
Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this usually works, and I'm not sure it works well or often enough to be worth keeping. The good news is that this applies specifically to hooks, where the previous lookup was already not working, so I can see a case for keeping it until it can be improved. But even then I think the case is not very strong.
(Note that this is a very different situation from path lookup in general--applying something like this more broadly, e.g. as a modification of safer_popen, would break a lot of stuff and possibly introduce vulnerabilities.)
My concerns are:
-
This skips the current directory in PATH, which is not necessary outside the case of ensuring that the empty string is not interpreted to mean the current directory, and which produces wrong behavior if the user happens to be in the directory where bash.exe actually should and otherwise would be found.
(One does need to ensure the empty string is not interpreted to mean the current directory in Windows, since one can get it from splitting PATH, but on Windows it is supposed to simply be skipped. Facilities that automatically handle PATH should always automatically do this and it is always a severe bug if they do not. It's when handling it manually, or when implementing such a general facility, that one must take care to skip empty entries.)
I think the confusion here is that we do need to avoid automatically including the CWD in the search, since it's an untrusted search path vulnerability if we don't. But safer_popen already does this. Any method that searches only directories named in PATH also does this automatically. This is so long as it either has no fallback or, as here, falls back to a mechanism that does it--and so long as it does not wrongly interpret empty entries on Windows as designating the current directory.
-
This doesn't skip the System32 directory, which is usually present in PATH and early on, and which is where the WSL-associated bash.exe is actually located.
There's a test here, but I also don't see how the test is really verifying that this works--it makes sure the new function is used, but it doesn't check that the new function works as designed, nor is there an end-to-end check to see if this design is effective at causing a correct bash.exe to be run when an incorrect one would have run before.
On CI, in a Git Bash shell, and in some but not most other development environments, the Git for Windows bash.exe precedes the WSL-associated bash.exe in PATH. Usually it doesn't. So if this appears to work then I suspect it is one of two cases:
- The narrow case where the
bin directory containing the Git for Windows bash.exe is present in PATH without the System32 directory preceding it (and nothing going wrong such as being cdd to that directory). I think that's happening on CI.
- It not working in the current environment, but it appearing to work based on the new test passing. I expect that to happen in local testing if tested in most common local environments other than a Git Bash shell.
One thing that is right about this implementation is that it does not use shutil.which, which is unsuitable for this particular use case. I bring this up because the commit message here inaccurately states that it does use shutil.which, which might wrongly lead someone to attempt to reimplement it by using that.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Jul 30, 2026
Byron
pushed a commit
that referenced
this pull request
Jul 30, 2026
The resolver merged in #2199 only worked when Git for Windows Bash itself preceded System32 on PATH. That is typical in Git Bash and CI but not in a normal system-wide installation, where PATH commonly contains System32 followed by Git\cmd. It also skipped an explicitly listed current directory even though explicit PATH entries are trusted configuration, and its test mocked the resolver rather than exercising its precedence.
This machine provides Ubuntu under WSL 2, C:\Windows\System32\bash.exe, and Git for Windows. Direct CreateProcess-style invocation of bare bash.exe reported Linux 6.18.33.2-microsoft-standard-WSL2, while `git var GIT_SHELL_PATH` reported C:/Program Files/Git/usr/bin/sh.exe and that shell reported MINGW64. With PATH reordered to System32 followed by Git\usr\bin, the merged resolver selected the System32 WSL launcher and an actual GitPython hook wrote a Linux marker. With the more typical System32 followed by Git\cmd PATH, Bash was not present on PATH at all.
Locate the Bash associated with GitPython's selected Git executable before general PATH lookup. Recognize the standard Git for Windows layouts Git\cmd\git.exe, Git\bin\git.exe, and Git\<platform>\bin\git.exe, with the platform names used by MSYS2. Configured relative Git executable paths are resolved against the parent process working directory, matching measured CreateProcess behavior even when Popen supplies a different hook working directory. Root-level bin is accepted for a selected Git executable, while usr\bin is deliberately not used to infer an unbounded parent layout. From the trusted Git root, follow gix-path's precedence of bin/bash.exe before usr/bin/bash.exe. If the Git layout is unrecognized, search explicit PATH entries while excluding candidates below SystemRoot so the WSL launcher cannot win. Empty PATH entries are ignored according to Windows semantics, but an explicitly named directory remains eligible even when it is the current directory. Finally, retain the prior bare fallback for nonstandard installations; safer_popen sets NoDefaultCurrentDirectoryInExePath, so that fallback does not reintroduce current-directory lookup.
The main regression models System32 before Git\cmd with Bash absent from PATH and checks the real resolver selects the associated Git\bin\bash.exe. Additional tests distinguish an explicit current-directory entry from an empty entry and cover an explicitly configured Git\bin\git.exe. On this machine, an end-to-end hook run under exactly System32;Git\cmd selected C:\Program Files\Git\bin\bash.exe and wrote a MINGW64 marker instead of the earlier Linux/WSL marker.
Validated with the focused hook suite (6 passed), the complete test_index.py module (32 passed, one expected xfail, one existing xpass), repository-wide Ruff lint and format checks targeting Python 3.7, and git diff --check. A standalone Python 3.7 interpreter was not available for an additional py_compile run.
Byron
pushed a commit
that referenced
this pull request
Jul 30, 2026
The resolver merged in #2199 only worked when Git for Windows Bash itself preceded System32 on PATH. That is typical in Git Bash and CI but not in a normal system-wide installation, where PATH commonly contains System32 followed by Git\cmd. It also skipped an explicitly listed current directory even though explicit PATH entries are trusted configuration, and its test mocked the resolver rather than exercising its precedence.
This machine provides Ubuntu under WSL 2, C:\Windows\System32\bash.exe, and Git for Windows. Direct CreateProcess-style invocation of bare bash.exe reported Linux 6.18.33.2-microsoft-standard-WSL2, while `git var GIT_SHELL_PATH` reported C:/Program Files/Git/usr/bin/sh.exe and that shell reported MINGW64. With PATH reordered to System32 followed by Git\usr\bin, the merged resolver selected the System32 WSL launcher and an actual GitPython hook wrote a Linux marker. With the more typical System32 followed by Git\cmd PATH, Bash was not present on PATH at all.
Locate the Bash associated with GitPython's selected Git executable before general PATH lookup. Recognize the standard Git for Windows layouts Git\cmd\git.exe, Git\bin\git.exe, and Git\<platform>\bin\git.exe, with the platform names used by MSYS2. Configured relative Git executable paths are resolved against the parent process working directory, matching measured CreateProcess behavior even when Popen supplies a different hook working directory. Root-level bin is accepted for a selected Git executable, while usr\bin is deliberately not used to infer an unbounded parent layout. From the trusted Git root, follow gix-path's precedence of bin/bash.exe before usr/bin/bash.exe. If the Git layout is unrecognized, search explicit PATH entries while excluding candidates below SystemRoot so the WSL launcher cannot win. Empty PATH entries are ignored according to Windows semantics, but an explicitly named directory remains eligible even when it is the current directory. Finally, retain the prior bare fallback for nonstandard installations; safer_popen sets NoDefaultCurrentDirectoryInExePath, so that fallback does not reintroduce current-directory lookup.
The main regression models System32 before Git\cmd with Bash absent from PATH and checks the real resolver selects the associated Git\bin\bash.exe. Additional tests distinguish an explicit current-directory entry from an empty entry and cover an explicitly configured Git\bin\git.exe. On this machine, an end-to-end hook run under exactly System32;Git\cmd selected C:\Program Files\Git\bin\bash.exe and wrote a MINGW64 marker instead of the earlier Linux/WSL marker.
Validated with the focused hook suite (6 passed), the complete test_index.py module (32 passed, one expected xfail, one existing xpass), repository-wide Ruff lint and format checks targeting Python 3.7, and git diff --check. A standalone Python 3.7 interpreter was not available for an additional py_compile run.
Byron
added a commit
that referenced
this pull request
Aug 4, 2026
<!-- agent -->
The resolver merged in #2199 only worked when Git for Windows Bash itself
preceded System32 on PATH. That is typical in Git Bash and CI but not in a
normal system-wide installation, where PATH commonly contains System32 followed
by Git\cmd. It also skipped an explicitly listed current directory even though
explicit PATH entries are trusted configuration, and its test mocked the
resolver rather than exercising its precedence.
The machine producing this commit provides Ubuntu under WSL 2, C:\Windows\System32\bash.exe, and Git
for Windows. Direct CreateProcess-style invocation of bare bash.exe reported
Linux 6.18.33.2-microsoft-standard-WSL2, while `git var GIT_SHELL_PATH` reported
C:/Program Files/Git/usr/bin/sh.exe and that shell reported MINGW64. With PATH
reordered to System32 followed by Git\usr\bin, the merged resolver selected the
System32 WSL launcher and an actual GitPython hook wrote a Linux marker. With
the more typical System32 followed by Git\cmd PATH, Bash was not present on PATH
at all.
Locate the Bash associated with GitPython's selected Git executable
before general PATH lookup. Recognize the standard Git for Windows layouts
Git\cmd\git.exe, Git\bin\git.exe, and Git\<platform>\bin\git.exe, with the
platform names used by MSYS2. Configured relative Git executable paths are
resolved against the parent process working directory, matching measured
CreateProcess behavior even when Popen supplies a different hook working
directory. Root-level bin is accepted for a selected Git executable, while
usr\bin is deliberately not used to infer an unbounded parent layout. From
the trusted Git root, follow gix-path's precedence of bin/bash.exe before
usr/bin/bash.exe. If the Git layout is unrecognized, search explicit PATH
entries while excluding candidates below SystemRoot so the WSL launcher
cannot win. Empty PATH entries are ignored according to Windows semantics,
but an explicitly named directory remains eligible even when it is the
current directory. Finally, retain the prior bare fallback for nonstandard
installations; safer_popen sets NoDefaultCurrentDirectoryInExePath, so that
fallback does not reintroduce current-directory lookup.
The main regression models System32 before Git\cmd with Bash absent from
PATH and checks the real resolver selects the associated Git\bin\bash.exe.
Additional tests distinguish an explicit current-directory entry from an empty
entry and cover an explicitly configured Git\bin\git.exe. On this machine,
an end-to-end hook run under exactly System32;Git\cmd selected C:\Program
Files\Git\bin\bash.exe and wrote a MINGW64 marker instead of the earlier
Linux/WSL marker.
Validated with the focused hook suite (6 passed), the complete test_index.py
module (32 passed, one expected xfail, one existing xpass), repository-wide Ruff
lint and format checks targeting Python 3.7, and git diff --check. A standalone
Python 3.7 interpreter was not available for an additional py_compile run.
Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <[email protected]>
Byron
added a commit
that referenced
this pull request
Aug 4, 2026
<!-- agent -->
The resolver merged in #2199 only worked when Git for Windows Bash itself
preceded System32 on PATH. That is typical in Git Bash and CI but not in a
normal system-wide installation, where PATH commonly contains System32 followed
by Git\cmd. It also skipped an explicitly listed current directory even though
explicit PATH entries are trusted configuration, and its test mocked the
resolver rather than exercising its precedence.
The machine producing this commit provides Ubuntu under WSL 2, C:\Windows\System32\bash.exe, and Git
for Windows. Direct CreateProcess-style invocation of bare bash.exe reported
Linux 6.18.33.2-microsoft-standard-WSL2, while `git var GIT_SHELL_PATH` reported
C:/Program Files/Git/usr/bin/sh.exe and that shell reported MINGW64. With PATH
reordered to System32 followed by Git\usr\bin, the merged resolver selected the
System32 WSL launcher and an actual GitPython hook wrote a Linux marker. With
the more typical System32 followed by Git\cmd PATH, Bash was not present on PATH
at all.
Locate the Bash associated with GitPython's selected Git executable
before general PATH lookup. Recognize the standard Git for Windows layouts
Git\cmd\git.exe, Git\bin\git.exe, and Git\<platform>\bin\git.exe, with the
platform names used by MSYS2. Configured relative Git executable paths are
resolved against the parent process working directory, matching measured
CreateProcess behavior even when Popen supplies a different hook working
directory. Root-level bin is accepted for a selected Git executable, while
usr\bin is deliberately not used to infer an unbounded parent layout. From
the trusted Git root, follow gix-path's precedence of bin/bash.exe before
usr/bin/bash.exe. If the Git layout is unrecognized, search explicit PATH
entries while excluding candidates below SystemRoot so the WSL launcher
cannot win. Empty PATH entries are ignored according to Windows semantics,
but an explicitly named directory remains eligible even when it is the
current directory. Finally, retain the prior bare fallback for nonstandard
installations; safer_popen sets NoDefaultCurrentDirectoryInExePath, so that
fallback does not reintroduce current-directory lookup.
The main regression models System32 before Git\cmd with Bash absent from
PATH and checks the real resolver selects the associated Git\bin\bash.exe.
Additional tests distinguish an explicit current-directory entry from an empty
entry and cover an explicitly configured Git\bin\git.exe. On this machine,
an end-to-end hook run under exactly System32;Git\cmd selected C:\Program
Files\Git\bin\bash.exe and wrote a MINGW64 marker instead of the earlier
Linux/WSL marker.
Validated with the focused hook suite (6 passed), the complete test_index.py
module (32 passed, one expected xfail, one existing xpass), repository-wide Ruff
lint and format checks targeting Python 3.7, and git diff --check. A standalone
Python 3.7 interpreter was not available for an additional py_compile run.
Assisted-by: GPT 5.6
Co-authored-by: GPT 5.6 <[email protected]>
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
|