Skip to content

Commit 7766597

Browse files
hongyeclaude
andcommitted
fix(utils): discover git-bash on non-default Windows install paths
`findGitBashPath` previously only checked `C:\Program Files\Git\` and `C:\Program Files (x86)\Git\` for git.exe, and assumed bash lived at `<git>/../../bin/bash.exe`. On non-default installs — for example `D:\software\Git\` (portableGit), scoop, chocolatey, or any manual placement — git is found via `where.exe` but the derived bash path doesn't exist, so the function fell through to `process.exit(1)`. This crashed entire test suites on Windows (e.g. `src/__tests__/queryAutonomyProviderBoundary.test.ts` transitively triggered the exit) and blocked every Windows user whose Git lived outside the two hard-coded paths from using the CLI at all. The fix: - Add `where.exe bash` as a primary discovery step (most reliable: it returns the actual bash.exe the user's shell will execute). - Try multiple derived layouts from git's location (standard, portable `usr/bin/bash.exe`, sibling `bin/`). - Expand default-locations fallback to include `usr/bin/bash.exe` and scoop install paths. - Extract the discovery logic into a pure `findGitBashPathOrNullWithDeps` function that takes its dependencies as parameters, so the branching can be unit-tested without `mock.module` polluting other tests in the same `bun test` process (see CLAUDE.md "跨文件 mock 污染"). Test coverage added in `src/utils/__tests__/windowsPaths.test.ts`: - env-var override (existing and missing file) - `where.exe bash` PATH lookup - All three git-path derivation layouts - Default-locations fallback (both standard and `usr/bin` variants) - SECURITY: filters malicious `where.exe` hits from cwd - Behavior: `where.exe git` is skipped once bash is already found Verified on Windows 11 + Bun 1.3.14: - `bun run precheck`: 5896/5896 pass (was 5883/5883, +13 new tests) - `bun run build`: succeeds - `findGitBashPath()` now returns `D:\software\Git\usr\bin\bash.exe` on the actual dev machine that previously hard-crashed Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0753daf commit 7766597

2 files changed

Lines changed: 383 additions & 55 deletions

File tree

src/utils/__tests__/windowsPaths.test.ts

Lines changed: 216 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
import { describe, expect, test } from 'bun:test'
2-
import { windowsPathToPosixPath, posixPathToWindowsPath } from '../windowsPaths'
2+
import {
3+
windowsPathToPosixPath,
4+
posixPathToWindowsPath,
5+
findGitBashPathOrNullWithDeps,
6+
type GitBashDiscoveryDeps,
7+
} from '../windowsPaths'
38

49
// ─── windowsPathToPosixPath ────────────────────────────────────────────
510

@@ -78,17 +83,17 @@ describe('posixPathToWindowsPath', () => {
7883
})
7984

8085
test('converts bare drive mount (no trailing slash)', () => {
81-
// /d matches the regex ^\/([A-Za-z])(\/|$) where $2 is empty
82-
expect(posixPathToWindowsPath('/d')).toBe('D:\\')
86+
expect(posixPathToWindowsPath('/e')).toBe('E:\\')
8387
})
8488

8589
test('converts relative path by flipping forward slashes', () => {
8690
expect(posixPathToWindowsPath('src/main.ts')).toBe('src\\main.ts')
8791
})
8892

89-
test('handles already-windows relative path', () => {
90-
// No leading / or //, just flips / to backslash
91-
expect(posixPathToWindowsPath('foo\\bar')).toBe('foo\\bar')
93+
test('handles deeply nested posix path', () => {
94+
expect(
95+
posixPathToWindowsPath('/c/Users/me/Documents/project/src/index.ts'),
96+
).toBe('C:\\Users\\me\\Documents\\project\\src\\index.ts')
9297
})
9398
})
9499

@@ -109,3 +114,208 @@ describe('round-trip conversions', () => {
109114
expect(back).toBe(original)
110115
})
111116
})
117+
118+
// ─── findGitBashPathOrNullWithDeps ─────────────────────────────────────
119+
120+
// These tests exercise the pure discovery helper with mock dependencies.
121+
// Using the DI variant (rather than mocking modules) keeps these tests
122+
// hermetic — no `mock.module` calls, so other tests in the same `bun test`
123+
// process are unaffected. See CLAUDE.md "跨文件 mock 污染" for context.
124+
125+
/** Build a deps object where only specific paths "exist". */
126+
function makeDeps(opts: {
127+
exists?: ReadonlyArray<string>
128+
bashInPath?: string | null
129+
gitInPath?: string | null
130+
cwd?: string
131+
execThrows?: boolean
132+
envOverride?: string
133+
}): GitBashDiscoveryDeps {
134+
const existsSet = new Set(opts.exists ?? [])
135+
return {
136+
checkExists: p => existsSet.has(p),
137+
execCommand: cmd => {
138+
if (opts.execThrows) throw new Error('where.exe not found')
139+
if (cmd.includes('where.exe bash')) return opts.bashInPath ?? ''
140+
if (cmd.includes('where.exe git')) return opts.gitInPath ?? ''
141+
return ''
142+
},
143+
cwdFn: () => opts.cwd ?? '/safe/cwd',
144+
// Default to empty string so we bypass `process.env.CLAUDE_CODE_GIT_BASH_PATH`
145+
// (the production function falls back to process.env when this is undefined).
146+
envOverride: opts.envOverride ?? '',
147+
}
148+
}
149+
150+
describe('findGitBashPathOrNullWithDeps', () => {
151+
test('honors envOverride when file exists', () => {
152+
const override = 'D:\\custom\\path\\bash.exe'
153+
const deps: GitBashDiscoveryDeps = {
154+
...makeDeps({}),
155+
envOverride: override,
156+
}
157+
deps.checkExists = p => p === override
158+
159+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(override)
160+
})
161+
162+
test('returns null when envOverride points to missing file', () => {
163+
const deps: GitBashDiscoveryDeps = {
164+
...makeDeps({ exists: [] }),
165+
envOverride: 'D:\\missing\\bash.exe',
166+
}
167+
expect(findGitBashPathOrNullWithDeps(deps)).toBeNull()
168+
})
169+
170+
test('returns bash from where.exe when bash is in PATH', () => {
171+
// Simulates the portable-install case (D:\software\Git\usr\bin\bash.exe)
172+
// where bash is in PATH but doesn't match the conventional
173+
// <git>/../../bin/bash.exe derivation.
174+
const bashPath = 'D:\\software\\Git\\usr\\bin\\bash.exe'
175+
const deps = makeDeps({
176+
exists: [bashPath],
177+
bashInPath: bashPath,
178+
})
179+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath)
180+
})
181+
182+
test('derives bash from git path using standard layout', () => {
183+
// Standard Git for Windows: git at <root>/cmd/git.exe, bash at <root>/bin/bash.exe
184+
const gitPath = 'C:\\Program Files\\Git\\cmd\\git.exe'
185+
const bashPath = 'C:\\Program Files\\Git\\bin\\bash.exe'
186+
const deps = makeDeps({
187+
exists: [bashPath],
188+
gitInPath: gitPath,
189+
})
190+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath)
191+
})
192+
193+
test('derives bash from git path using portable layout (usr/bin/bash.exe)', () => {
194+
// PortableGit / custom installs: git at <root>/cmd/git.exe,
195+
// bash at <root>/usr/bin/bash.exe — the case that previously caused
196+
// process.exit(1) on D:\software\Git\ installations.
197+
const gitPath = 'D:\\software\\Git\\cmd\\git.exe'
198+
const bashPath = 'D:\\software\\Git\\usr\\bin\\bash.exe'
199+
const deps = makeDeps({
200+
exists: [bashPath], // only portable layout bash exists
201+
gitInPath: gitPath,
202+
})
203+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath)
204+
})
205+
206+
test('derives bash from git path using sibling layout', () => {
207+
// Some installs put git and bash in the same bin/ directory.
208+
const gitPath = 'C:\\Some\\Install\\bin\\git.exe'
209+
const bashPath = 'C:\\Some\\Install\\bin\\bash.exe'
210+
const deps = makeDeps({
211+
exists: [bashPath],
212+
gitInPath: gitPath,
213+
})
214+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath)
215+
})
216+
217+
test('falls back to default bash locations when nothing else matches', () => {
218+
const defaultBash = 'C:\\Program Files\\Git\\bin\\bash.exe'
219+
const deps = makeDeps({
220+
exists: [defaultBash],
221+
execThrows: true,
222+
})
223+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(defaultBash)
224+
})
225+
226+
test('falls back to usr/bin default layout when standard is absent', () => {
227+
// Default-locations branch also tries the `usr/bin` variant of
228+
// Program Files paths.
229+
const bashPath = 'C:\\Program Files\\Git\\usr\\bin\\bash.exe'
230+
const deps = makeDeps({
231+
exists: [bashPath],
232+
execThrows: true,
233+
})
234+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath)
235+
})
236+
237+
test('returns null when no discovery method finds bash', () => {
238+
const deps = makeDeps({
239+
exists: [],
240+
execThrows: true,
241+
})
242+
expect(findGitBashPathOrNullWithDeps(deps)).toBeNull()
243+
})
244+
245+
test('prefers envOverride over where.exe result', () => {
246+
const override = 'D:\\env\\bash.exe'
247+
const fromPath = 'D:\\software\\Git\\usr\\bin\\bash.exe'
248+
const deps: GitBashDiscoveryDeps = {
249+
...makeDeps({
250+
exists: [override, fromPath],
251+
bashInPath: fromPath,
252+
}),
253+
envOverride: override,
254+
}
255+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(override)
256+
})
257+
258+
test('prefers where.exe bash over git-path derivation', () => {
259+
// where.exe bash is more reliable than the <git>/../../bin/bash.exe
260+
// derivation when git is at a non-standard location.
261+
const fromPath = 'D:\\software\\Git\\usr\\bin\\bash.exe'
262+
const gitPath = 'D:\\software\\Git\\cmd\\git.exe'
263+
const derivedBash = 'D:\\software\\Git\\bin\\bash.exe' // doesn't exist
264+
const deps = makeDeps({
265+
exists: [fromPath], // only fromPath exists; derived layout absent
266+
bashInPath: fromPath,
267+
gitInPath: gitPath,
268+
})
269+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(fromPath)
270+
// Derived path must not be probed — we already have a where.exe hit.
271+
expect(deps.checkExists(derivedBash)).toBe(false)
272+
})
273+
274+
test('skips where.exe git when bash is already found via PATH', () => {
275+
// Performance / behavior: once bash is found, we shouldn't probe git
276+
// or the derived paths.
277+
const bashPath = 'D:\\software\\Git\\usr\\bin\\bash.exe'
278+
let gitProbed = false
279+
const deps: GitBashDiscoveryDeps = {
280+
checkExists: p => p === bashPath,
281+
execCommand: cmd => {
282+
if (cmd.includes('where.exe bash')) return bashPath
283+
if (cmd.includes('where.exe git')) {
284+
gitProbed = true
285+
return ''
286+
}
287+
return ''
288+
},
289+
cwdFn: () => '/safe/cwd',
290+
}
291+
expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath)
292+
expect(gitProbed).toBe(false)
293+
})
294+
295+
test('filters malicious where.exe hits in current working directory', () => {
296+
// SECURITY: where.exe can return paths from the cwd if a malicious
297+
// git.exe/bat lives there. The discovery must skip such entries.
298+
// The legit git path is NOT one of the default locations (we use a
299+
// custom non-default path) so the test exercises the where.exe
300+
// branch rather than the default-locations short-circuit.
301+
const cwd = 'C:\\Users\\victim\\project'
302+
const maliciousPath = `${cwd}\\git.exe`
303+
const legitGit = 'C:\\Custom\\Git\\install\\cmd\\git.exe'
304+
const expectedBash = 'C:\\Custom\\Git\\install\\bin\\bash.exe'
305+
const deps: GitBashDiscoveryDeps = {
306+
checkExists: p => p === expectedBash,
307+
execCommand: cmd => {
308+
if (cmd.includes('where.exe bash')) return ''
309+
if (cmd.includes('where.exe git')) {
310+
// where.exe returns cwd entry first, then legit
311+
return `${maliciousPath}\r\n${legitGit}`
312+
}
313+
return ''
314+
},
315+
cwdFn: () => cwd,
316+
envOverride: '',
317+
}
318+
const result = findGitBashPathOrNullWithDeps(deps)
319+
expect(result).toBe(expectedBash)
320+
})
321+
})

0 commit comments

Comments
 (0)