Skip to content

fix(windows): never resolve Git Bash to the WSL or Store bash.exe stubs - #328

Merged
kalvinnchau merged 2 commits into
block:mainfrom
corydouthat-sq:cory/windows-git-bash-discovery
Sep 16, 2026
Merged

kalvinnchau merged 2 commits into
block:mainfrom
corydouthat-sq:cory/windows-git-bash-discovery

Conversation

@corydouthat-sq

@corydouthat-sq corydouthat-sq commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Get-GitBashPath tried two Program Files\Git literals and then returned whatever bash resolved to on PATH. On a stock Windows 11 machine that is the Microsoft Store app-execution alias (%LOCALAPPDATA%\Microsoft\WindowsApps\bash.exe) or the WSL launcher (System32\bash.exe); both precede Git for Windows on PATH and neither can run repository scripts. Per-user Git installs (winget user scope, %LOCALAPPDATA%\Programs\Git) were never considered. #39 (c174a7d0) fixed the same confusion on the just side by pinning just 1.48.0 but left this diagnostic helper alone, so bootstrap-windows and doctor-windows kept reporting the stub as a healthy Git Bash.

Changes, modelled on Find-RunnablePython:

  • Get-GitBashPath gathers candidates from the machine-wide and per-user install roots, from the root of the git.exe on PATH (<root>\bin\bash.exe, <root>\usr\bin\bash.exe), and from every where.exe bash hit, then filters them through a new pure helper Test-GitBashCandidatePath (rejects \WindowsApps\, System32/SysWOW64 bash.exe, Codex runtimes). It returns $null when nothing usable exists.
  • Bootstrap-Windows.ps1 and Doctor-Windows.ps1 failure text no longer claims Git lives under Program Files\Git and names the stubs that do not count.
  • Test-WindowsDev.ps1 covers the filter (WSL launcher, 32-bit launcher, Store alias, Codex runtime, blank, machine-wide, per-user), checks the lookup applies the filter and derives from git.exe, and validates the live result on the machine running the tests.

Why now: an internal Windows build lane resolved bash.exe to the WSL stub and had to carry its own lookup; this makes the module's helper trustworthy so callers can rely on it.

Related issue

Follows up on #39; none open.

Testing

  • just test-windows-dev under PowerShell 7 on Windows 11: all new assertions pass. (Under Windows PowerShell 5.1 the suite still trips on the pre-existing [semver] use at lines 162-165, which fix(windows): keep the Windows script tests runnable on Windows PowerShell 5.1 #329 fixes; this branch inherits that fix from main once it merges.)
  • With %LOCALAPPDATA%\Microsoft\WindowsApps moved to the front of PATH (so where.exe bash lists the Store alias first), Get-GitBashPath returns C:\Program Files\Git\bin\bash.exe under both hosts; Get-WindowsPrerequisiteSnapshot reports GitBash.Found = True with that path.
  • Parser check of the four touched scripts: no parse errors.

Get-GitBashPath tried two Program Files literals and then fell back to
whatever `bash` resolved to on PATH. On a stock Windows 11 machine that is
the Microsoft Store app-execution alias under WindowsApps, or the WSL
launcher in System32, both of which precede Git for Windows on PATH and
cannot run repository scripts. Per-user Git installs (winget user scope,
%LOCALAPPDATA%\Programs\Git) were never considered at all. c174a7d fixed
the same confusion on the `just` side by pinning just 1.48.0 but left this
diagnostic helper alone, so bootstrap and doctor kept reporting the stub as
a healthy Git Bash.

Model the lookup on Find-RunnablePython: gather candidates from the
machine-wide and per-user install roots, from the root of the git.exe on
PATH (<root>\bin and <root>\usr\bin), and from every `where.exe bash`
hit, then drop anything a new pure helper, Test-GitBashCandidatePath,
recognises as a stub (WindowsApps, System32 or SysWOW64 bash.exe, Codex
runtimes). When nothing usable remains the helper returns $null and
bootstrap/doctor say so instead of pointing at Program Files\Git.

Test-WindowsDev.ps1 covers the filter and checks the lookup applies it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@corydouthat-sq
corydouthat-sq requested a review from a team September 15, 2026 20:42

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Automated code review

REQUEST_CHANGES — one blocking finding. The fallback still accepts unrelated Bash installations as Git Bash, so Windows prerequisite checks can report false readiness. Supplied GitHub evidence was inspected; some checks were still in progress, so required checks remain a separate merge-readiness gate.

Deterministic publication result: 1 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

if ($whereBash.ExitCode -eq 0) {
foreach ($line in ($whereBash.Output -split "`r?`n")) {
if (-not [string]::IsNullOrWhiteSpace($line)) {
$candidates.Add($line.Trim())

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 P1 · Unrelated Bash passes as Git Bash (blocking)

The lookup adds every path returned by where.exe bash, while the candidate filter rejects only WindowsApps, System32/SysWOW64 stubs, and Codex runtimes. A Cygwin, MSYS2, or standalone Bash path therefore passes as the Git-for-Windows prerequisite even when Git Bash is absent, preserving the original false-positive failure through another fallback.

User effect: Windows bootstrap and doctor can say Git Bash is ready when it is not, so people may continue into setup without the required tool and fail later without the expected installation guidance.

Recommended fix: Remove the unrestricted Bash-on-PATH fallback, or positively verify that every accepted candidate belongs to a Git for Windows installation associated with a trusted install root or the resolved git.exe layout.

Test: Add deterministic selection coverage where the only PATH result is Cygwin or MSYS2 Bash and assert that discovery returns null, plus coverage showing a portable Git installation derived from git.exe is accepted.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 792becc. Get-GitBashPath now positively verifies every candidate (including the ones from where.exe bash and the one derived from git.exe) with a new pure Test-GitForWindowsLayout check: the path must be <root>\bin\bash.exe or <root>\usr\bin\bash.exe and <root>\cmd\git.exe must exist. That file is present in every Git for Windows install (installed and portable) and absent from Cygwin and MSYS2, so an unrelated Bash on PATH can no longer satisfy the prerequisite. The file-exists probe is injectable, and Test-WindowsDev.ps1 now asserts Cygwin (C:\cygwin64\bin\bash.exe) and MSYS2 (C:\msys64\usr\bin\bash.exe) are rejected, a portable Git derived from git.exe (D:\PortableGit\usr\bin\bash.exe) is accepted, a Git-shaped tree missing cmd\git.exe is rejected, and that the lookup applies the layout check to every candidate. Bootstrap/doctor failure text now names what counts.

…sh.exe

Get-GitBashPath added every path returned by `where.exe bash` and only
filtered out the WSL/Store stubs and Codex runtimes, so a Cygwin, MSYS2,
or standalone bash.exe on PATH passed as the Git for Windows prerequisite
even when Git Bash was absent.

Add Test-GitForWindowsLayout, a pure check that accepts a bash.exe only
when it sits at <root>\bin\bash.exe or <root>\usr\bin\bash.exe and
<root>\cmd\git.exe exists beside it. That file is shipped by every Git
for Windows install (installed and portable) and by neither Cygwin nor
MSYS2, so it positively identifies the layout instead of trusting PATH.
The file-exists probe is injectable so the check is deterministic in
tests. Get-GitBashPath now requires both the string filter and the
layout check for every candidate; the git.exe-derived and where.exe
candidates stay, since the layout check makes them safe.

Tests cover Cygwin and MSYS2 rejection, portable Git acceptance, the
machine-wide install, a Git-shaped tree missing cmd\git.exe, and a
bash.exe outside a bin directory. Bootstrap and doctor messages name the
locations that are searched and the bash flavors that do not count.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

@morgmart morgmart left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Automated code review

APPROVE — no publishable findings. The previous false-readiness issue is fixed: every discovered Bash candidate must now match a Git for Windows layout, with coverage for unsupported and supported layouts. Supplied GitHub check evidence was inspected; all reported check runs completed successfully, while required checks remain the merge-readiness authority.

Deterministic publication result: 0 blocking and 0 non-blocking inline finding(s) publishable; 0 duplicate(s) suppressed; 0 blocking screenshot-evidence requirement(s) in this review body.

Pending checks: 1 check(s) are not complete.

This approval reflects the completed code review only; merge readiness remains governed by the repository's required checks.

@kalvinnchau
kalvinnchau merged commit 500cd13 into block:main Sep 16, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants