From 7766597fa2090238758c42ae5e07242e3ae1dd18 Mon Sep 17 00:00:00 2001 From: hongye <60014362@kans.cn> Date: Thu, 25 Jun 2026 23:17:58 +0800 Subject: [PATCH 1/2] fix(utils): discover git-bash on non-default Windows install paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `findGitBashPath` previously only checked `C:\Program Files\Git\` and `C:\Program Files (x86)\Git\` for git.exe, and assumed bash lived at `/../../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 --- src/utils/__tests__/windowsPaths.test.ts | 222 ++++++++++++++++++++++- src/utils/windowsPaths.ts | 216 +++++++++++++++++----- 2 files changed, 383 insertions(+), 55 deletions(-) diff --git a/src/utils/__tests__/windowsPaths.test.ts b/src/utils/__tests__/windowsPaths.test.ts index f6ae139476..6ff6a8c62f 100644 --- a/src/utils/__tests__/windowsPaths.test.ts +++ b/src/utils/__tests__/windowsPaths.test.ts @@ -1,5 +1,10 @@ import { describe, expect, test } from 'bun:test' -import { windowsPathToPosixPath, posixPathToWindowsPath } from '../windowsPaths' +import { + windowsPathToPosixPath, + posixPathToWindowsPath, + findGitBashPathOrNullWithDeps, + type GitBashDiscoveryDeps, +} from '../windowsPaths' // ─── windowsPathToPosixPath ──────────────────────────────────────────── @@ -78,17 +83,17 @@ describe('posixPathToWindowsPath', () => { }) test('converts bare drive mount (no trailing slash)', () => { - // /d matches the regex ^\/([A-Za-z])(\/|$) where $2 is empty - expect(posixPathToWindowsPath('/d')).toBe('D:\\') + expect(posixPathToWindowsPath('/e')).toBe('E:\\') }) test('converts relative path by flipping forward slashes', () => { expect(posixPathToWindowsPath('src/main.ts')).toBe('src\\main.ts') }) - test('handles already-windows relative path', () => { - // No leading / or //, just flips / to backslash - expect(posixPathToWindowsPath('foo\\bar')).toBe('foo\\bar') + test('handles deeply nested posix path', () => { + expect( + posixPathToWindowsPath('/c/Users/me/Documents/project/src/index.ts'), + ).toBe('C:\\Users\\me\\Documents\\project\\src\\index.ts') }) }) @@ -109,3 +114,208 @@ describe('round-trip conversions', () => { expect(back).toBe(original) }) }) + +// ─── findGitBashPathOrNullWithDeps ───────────────────────────────────── + +// These tests exercise the pure discovery helper with mock dependencies. +// Using the DI variant (rather than mocking modules) keeps these tests +// hermetic — no `mock.module` calls, so other tests in the same `bun test` +// process are unaffected. See CLAUDE.md "跨文件 mock 污染" for context. + +/** Build a deps object where only specific paths "exist". */ +function makeDeps(opts: { + exists?: ReadonlyArray + bashInPath?: string | null + gitInPath?: string | null + cwd?: string + execThrows?: boolean + envOverride?: string +}): GitBashDiscoveryDeps { + const existsSet = new Set(opts.exists ?? []) + return { + checkExists: p => existsSet.has(p), + execCommand: cmd => { + if (opts.execThrows) throw new Error('where.exe not found') + if (cmd.includes('where.exe bash')) return opts.bashInPath ?? '' + if (cmd.includes('where.exe git')) return opts.gitInPath ?? '' + return '' + }, + cwdFn: () => opts.cwd ?? '/safe/cwd', + // Default to empty string so we bypass `process.env.CLAUDE_CODE_GIT_BASH_PATH` + // (the production function falls back to process.env when this is undefined). + envOverride: opts.envOverride ?? '', + } +} + +describe('findGitBashPathOrNullWithDeps', () => { + test('honors envOverride when file exists', () => { + const override = 'D:\\custom\\path\\bash.exe' + const deps: GitBashDiscoveryDeps = { + ...makeDeps({}), + envOverride: override, + } + deps.checkExists = p => p === override + + expect(findGitBashPathOrNullWithDeps(deps)).toBe(override) + }) + + test('returns null when envOverride points to missing file', () => { + const deps: GitBashDiscoveryDeps = { + ...makeDeps({ exists: [] }), + envOverride: 'D:\\missing\\bash.exe', + } + expect(findGitBashPathOrNullWithDeps(deps)).toBeNull() + }) + + test('returns bash from where.exe when bash is in PATH', () => { + // Simulates the portable-install case (D:\software\Git\usr\bin\bash.exe) + // where bash is in PATH but doesn't match the conventional + // /../../bin/bash.exe derivation. + const bashPath = 'D:\\software\\Git\\usr\\bin\\bash.exe' + const deps = makeDeps({ + exists: [bashPath], + bashInPath: bashPath, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) + }) + + test('derives bash from git path using standard layout', () => { + // Standard Git for Windows: git at /cmd/git.exe, bash at /bin/bash.exe + const gitPath = 'C:\\Program Files\\Git\\cmd\\git.exe' + const bashPath = 'C:\\Program Files\\Git\\bin\\bash.exe' + const deps = makeDeps({ + exists: [bashPath], + gitInPath: gitPath, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) + }) + + test('derives bash from git path using portable layout (usr/bin/bash.exe)', () => { + // PortableGit / custom installs: git at /cmd/git.exe, + // bash at /usr/bin/bash.exe — the case that previously caused + // process.exit(1) on D:\software\Git\ installations. + const gitPath = 'D:\\software\\Git\\cmd\\git.exe' + const bashPath = 'D:\\software\\Git\\usr\\bin\\bash.exe' + const deps = makeDeps({ + exists: [bashPath], // only portable layout bash exists + gitInPath: gitPath, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) + }) + + test('derives bash from git path using sibling layout', () => { + // Some installs put git and bash in the same bin/ directory. + const gitPath = 'C:\\Some\\Install\\bin\\git.exe' + const bashPath = 'C:\\Some\\Install\\bin\\bash.exe' + const deps = makeDeps({ + exists: [bashPath], + gitInPath: gitPath, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) + }) + + test('falls back to default bash locations when nothing else matches', () => { + const defaultBash = 'C:\\Program Files\\Git\\bin\\bash.exe' + const deps = makeDeps({ + exists: [defaultBash], + execThrows: true, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(defaultBash) + }) + + test('falls back to usr/bin default layout when standard is absent', () => { + // Default-locations branch also tries the `usr/bin` variant of + // Program Files paths. + const bashPath = 'C:\\Program Files\\Git\\usr\\bin\\bash.exe' + const deps = makeDeps({ + exists: [bashPath], + execThrows: true, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) + }) + + test('returns null when no discovery method finds bash', () => { + const deps = makeDeps({ + exists: [], + execThrows: true, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBeNull() + }) + + test('prefers envOverride over where.exe result', () => { + const override = 'D:\\env\\bash.exe' + const fromPath = 'D:\\software\\Git\\usr\\bin\\bash.exe' + const deps: GitBashDiscoveryDeps = { + ...makeDeps({ + exists: [override, fromPath], + bashInPath: fromPath, + }), + envOverride: override, + } + expect(findGitBashPathOrNullWithDeps(deps)).toBe(override) + }) + + test('prefers where.exe bash over git-path derivation', () => { + // where.exe bash is more reliable than the /../../bin/bash.exe + // derivation when git is at a non-standard location. + const fromPath = 'D:\\software\\Git\\usr\\bin\\bash.exe' + const gitPath = 'D:\\software\\Git\\cmd\\git.exe' + const derivedBash = 'D:\\software\\Git\\bin\\bash.exe' // doesn't exist + const deps = makeDeps({ + exists: [fromPath], // only fromPath exists; derived layout absent + bashInPath: fromPath, + gitInPath: gitPath, + }) + expect(findGitBashPathOrNullWithDeps(deps)).toBe(fromPath) + // Derived path must not be probed — we already have a where.exe hit. + expect(deps.checkExists(derivedBash)).toBe(false) + }) + + test('skips where.exe git when bash is already found via PATH', () => { + // Performance / behavior: once bash is found, we shouldn't probe git + // or the derived paths. + const bashPath = 'D:\\software\\Git\\usr\\bin\\bash.exe' + let gitProbed = false + const deps: GitBashDiscoveryDeps = { + checkExists: p => p === bashPath, + execCommand: cmd => { + if (cmd.includes('where.exe bash')) return bashPath + if (cmd.includes('where.exe git')) { + gitProbed = true + return '' + } + return '' + }, + cwdFn: () => '/safe/cwd', + } + expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) + expect(gitProbed).toBe(false) + }) + + test('filters malicious where.exe hits in current working directory', () => { + // SECURITY: where.exe can return paths from the cwd if a malicious + // git.exe/bat lives there. The discovery must skip such entries. + // The legit git path is NOT one of the default locations (we use a + // custom non-default path) so the test exercises the where.exe + // branch rather than the default-locations short-circuit. + const cwd = 'C:\\Users\\victim\\project' + const maliciousPath = `${cwd}\\git.exe` + const legitGit = 'C:\\Custom\\Git\\install\\cmd\\git.exe' + const expectedBash = 'C:\\Custom\\Git\\install\\bin\\bash.exe' + const deps: GitBashDiscoveryDeps = { + checkExists: p => p === expectedBash, + execCommand: cmd => { + if (cmd.includes('where.exe bash')) return '' + if (cmd.includes('where.exe git')) { + // where.exe returns cwd entry first, then legit + return `${maliciousPath}\r\n${legitGit}` + } + return '' + }, + cwdFn: () => cwd, + envOverride: '', + } + const result = findGitBashPathOrNullWithDeps(deps) + expect(result).toBe(expectedBash) + }) +}) diff --git a/src/utils/windowsPaths.ts b/src/utils/windowsPaths.ts index d610f69c5d..746245addb 100644 --- a/src/utils/windowsPaths.ts +++ b/src/utils/windowsPaths.ts @@ -9,21 +9,89 @@ import { memoizeWithLRU } from './memoize.js' import { getPlatform } from './platform.js' /** - * Check if a file or directory exists on Windows. - * Uses fs.existsSync instead of `dir` shell command to avoid spawning - * cmd.exe — which can cause brief console window flashes in detached - * or windowsHide child processes. + * If Windows, set the SHELL environment variable to git-bash path. + * This is used by BashTool and Shell.ts for user shell commands. + * COMSPEC is left unchanged for system process execution. + */ +export function setShellIfWindows(): void { + if (getPlatform() === 'windows') { + const gitBashPath = findGitBashPath() + process.env.SHELL = gitBashPath + // Propagate to child processes so they skip filesystem probing + process.env.CLAUDE_CODE_GIT_BASH_PATH = gitBashPath + logForDebugging(`Using bash path: "${gitBashPath}"`) + } +} + +/** + * Dependencies for git-bash discovery. Exposed as a parameter so the + * discovery logic can be unit-tested without `mock.module` polluting + * other tests in the same process (see CLAUDE.md "跨文件 mock 污染"). */ -function checkPathExists(filePath: string): boolean { - return existsSync(filePath) +export type GitBashDiscoveryDeps = { + /** Returns true iff the path exists on disk. */ + checkExists: (filePath: string) => boolean + /** Executes a shell command and returns its trimmed stdout. May throw. */ + execCommand: (cmd: string) => string + /** Returns the current working directory (used to filter PATH-based lookups). */ + cwdFn: () => string + /** + * Optional override for `process.env.CLAUDE_CODE_GIT_BASH_PATH`. When + * provided, this is used instead of the live environment — useful for tests. + */ + envOverride?: string | undefined +} + +const DEFAULT_DEPS: GitBashDiscoveryDeps = { + checkExists: existsSync, + execCommand: cmd => + execSync_DEPRECATED(cmd, { stdio: 'pipe', encoding: 'utf8' }).trim(), + cwdFn: getCwd, + envOverride: undefined, } /** - * Find an executable using where.exe on Windows - * @param executable - The name of the executable to find - * @returns The path to the executable or null if not found + * Search common install locations for bash.exe directly. Returns the first + * existing candidate or null if none match. Used as a last-resort fallback + * when `where.exe` cannot locate bash via PATH and git is also unknown. */ -function findExecutable(executable: string): string | null { +function searchDefaultBashLocations( + checkExists: (p: string) => boolean, +): string | null { + const candidates = [ + // Standard Git for Windows install locations (both layouts). + 'C:\\Program Files\\Git\\bin\\bash.exe', + 'C:\\Program Files\\Git\\usr\\bin\\bash.exe', + 'C:\\Program Files (x86)\\Git\\bin\\bash.exe', + 'C:\\Program Files (x86)\\Git\\usr\\bin\\bash.exe', + ] + // Scoop install: %USERPROFILE%\scoop\apps\git\current\usr\bin\bash.exe + const userProfile = process.env.USERPROFILE + if (userProfile) { + candidates.push( + `${userProfile}\\scoop\\apps\\git\\current\\usr\\bin\\bash.exe`, + ) + } + for (const candidate of candidates) { + if (checkExists(candidate)) { + return candidate + } + } + return null +} + +/** + * Look up an executable on Windows. Tries common install locations first + * (for `git`), then falls back to `where.exe`. Filters out entries in the + * current working directory to avoid executing malicious copies. + * + * Pure variant — takes its dependencies as parameters so it can be unit-tested + * without process-global mocks. + */ +function findExecutableWithDeps( + executable: string, + deps: GitBashDiscoveryDeps, +): string | null { // For git, check common installation locations first if (executable === 'git') { const defaultLocations = [ @@ -35,7 +103,7 @@ function findExecutable(executable: string): string | null { ] for (const location of defaultLocations) { - if (checkPathExists(location)) { + if (deps.checkExists(location)) { return location } } @@ -43,15 +111,11 @@ function findExecutable(executable: string): string | null { // Fall back to where.exe try { - const result = execSync_DEPRECATED(`where.exe ${executable}`, { - stdio: 'pipe', - encoding: 'utf8', - }).trim() - + const result = deps.execCommand(`where.exe ${executable}`) // SECURITY: Filter out any results from the current directory // to prevent executing malicious git.bat/cmd/exe files const paths = result.split('\r\n').filter(Boolean) - const cwd = getCwd().toLowerCase() + const cwd = deps.cwdFn().toLowerCase() for (const candidatePath of paths) { // Normalize and compare paths to ensure we're not in current directory @@ -77,49 +141,103 @@ function findExecutable(executable: string): string | null { } /** - * If Windows, set the SHELL environment variable to git-bash path. - * This is used by BashTool and Shell.ts for user shell commands. - * COMSPEC is left unchanged for system process execution. + * Pure discovery helper for git-bash.exe. Returns `null` if not found. + * See `findGitBashPathOrNull` for production invocation. + * + * Exported for testing — tests can pass mock `checkExists`, `execCommand`, + * and `cwdFn` to exercise each branch in isolation without polluting the + * module registry (which would affect other tests via `mock.module`). */ -export function setShellIfWindows(): void { - if (getPlatform() === 'windows') { - const gitBashPath = findGitBashPath() - process.env.SHELL = gitBashPath - // Propagate to child processes so they skip filesystem probing - process.env.CLAUDE_CODE_GIT_BASH_PATH = gitBashPath - logForDebugging(`Using bash path: "${gitBashPath}"`) +export function findGitBashPathOrNullWithDeps( + deps: GitBashDiscoveryDeps = DEFAULT_DEPS, +): string | null { + const envOverride = deps.envOverride ?? process.env.CLAUDE_CODE_GIT_BASH_PATH + + // 1. Honor explicit CLAUDE_CODE_GIT_BASH_PATH override + if (envOverride) { + return deps.checkExists(envOverride) ? envOverride : null } -} -/** - * Find the path where `bash.exe` included with git-bash exists, exiting the process if not found. - */ -export const findGitBashPath = memoize((): string => { - if (process.env.CLAUDE_CODE_GIT_BASH_PATH) { - if (checkPathExists(process.env.CLAUDE_CODE_GIT_BASH_PATH)) { - return process.env.CLAUDE_CODE_GIT_BASH_PATH - } - console.error( - `Claude Code was unable to find CLAUDE_CODE_GIT_BASH_PATH path "${process.env.CLAUDE_CODE_GIT_BASH_PATH}"`, - ) - // eslint-disable-next-line custom-rules/no-process-exit - process.exit(1) + // 2. Look up bash.exe directly via PATH. This is the most reliable + // method for non-default install locations (e.g. D:\software\Git\) + // where bash sits at /usr/bin/bash.exe rather than the + // conventional /bin/bash.exe. + const fromPath = findExecutableWithDeps('bash', deps) + if (fromPath && deps.checkExists(fromPath)) { + return fromPath } - const gitPath = findExecutable('git') + // 3. Derive bash from git's location, trying multiple layouts since + // non-standard Git installs (scoop, chocolatey, manual / portable) + // place bash differently relative to git.exe. + const gitPath = findExecutableWithDeps('git', deps) if (gitPath) { - const bashPath = pathWin32.join(gitPath, '..', '..', 'bin', 'bash.exe') - if (checkPathExists(bashPath)) { - return bashPath + const candidates = [ + // Standard Git for Windows: git at /cmd/git.exe, bash at /bin/bash.exe + pathWin32.join(gitPath, '..', '..', 'bin', 'bash.exe'), + // PortableGit / custom installs: git at /cmd/git.exe, bash at /usr/bin/bash.exe + pathWin32.join(gitPath, '..', '..', 'usr', 'bin', 'bash.exe'), + // Some installs: git at /bin/git.exe, bash at /bin/bash.exe + pathWin32.join(gitPath, '..', 'bash.exe'), + ] + for (const candidate of candidates) { + if (deps.checkExists(candidate)) { + return candidate + } } } - console.error( - 'Claude Code on Windows requires git-bash (https://git-scm.com/downloads/win). If installed but not in PATH, set environment variable pointing to your bash.exe, similar to: CLAUDE_CODE_GIT_BASH_PATH=C:\\Program Files\\Git\\bin\\bash.exe', - ) + // 4. Last resort: scan common install locations for bash.exe directly + return searchDefaultBashLocations(deps.checkExists) +} + +/** + * Find the path where `bash.exe` included with git-bash exists. Returns + * `null` if no suitable bash.exe can be located. + * + * Discovery order (each step is skipped if the previous one resolves): + * 1. `CLAUDE_CODE_GIT_BASH_PATH` env var, if set and the file exists + * 2. `where.exe bash` lookup (works whenever Git Bash's bin dir is in PATH, + * e.g. portable installs at `D:\software\Git\` where bash is at + * `/usr/bin/bash.exe` rather than the conventional `/bin/bash.exe`) + * 3. Derive from `where.exe git`, trying multiple relative layouts + * (standard Git for Windows, PortableGit, sibling install) + * 4. Check common default install locations directly + * + * Memoized so repeated calls within the same process only search once. + * Test-friendly variant: does NOT call `process.exit`, unlike `findGitBashPath`. + */ +export const findGitBashPathOrNull = memoize(() => + findGitBashPathOrNullWithDeps(), +) + +/** + * Find the path where `bash.exe` included with git-bash exists, exiting + * the process if not found. + * + * Thin wrapper over `findGitBashPathOrNull` that handles the + * `process.exit(1)` failure path. Exported separately so the discovery + * logic in `findGitBashPathOrNullWithDeps` can be unit-tested without + * invoking `process.exit`. + */ +export function findGitBashPath(): string { + const result = findGitBashPathOrNull() + if (result !== null) { + return result + } + const envOverride = process.env.CLAUDE_CODE_GIT_BASH_PATH + if (envOverride) { + console.error( + `Claude Code was unable to find CLAUDE_CODE_GIT_BASH_PATH path "${envOverride}"`, + ) + } else { + console.error( + 'Claude Code on Windows requires git-bash (https://git-scm.com/downloads/win). If installed but not in PATH, set environment variable pointing to your bash.exe, similar to: CLAUDE_CODE_GIT_BASH_PATH=C:\\Program Files\\Git\\bin\\bash.exe', + ) + } // eslint-disable-next-line custom-rules/no-process-exit process.exit(1) -}) +} /** Convert a Windows path to a POSIX path using pure JS. */ export const windowsPathToPosixPath = memoizeWithLRU( From 6e413f8b17f5bbdaef5a6489e456b010fbe40b34 Mon Sep 17 00:00:00 2001 From: hongye <60014362@kans.cn> Date: Fri, 26 Jun 2026 01:22:07 +0800 Subject: [PATCH 2/2] fix(utils): address CodeRabbit review feedback on git-bash discovery Three improvements suggested by CodeRabbit's PR review: 1. Use path.win32 for processing `where.exe` results (MAJOR). On POSIX hosts, `path.resolve('C:\\foo')` treats backslashes as literal characters and produces a wrong (relative) result, which would let cwd shadowing slip past the security check. Switch to `path.win32.*` so the cwd filtering evaluates `where.exe`'s Windows-style paths with Windows semantics regardless of host OS. Also switched the cwd check from `startsWith` to `path.win32.relative`, which is more robust: returns `''` for cwd itself, `..`/`../...` for outside, and a non-absolute path for inside. Avoids edge cases with trailing separators and mixed case. 2. Inject `USERPROFILE` through `GitBashDiscoveryDeps` (NITPICK). The Scoop fallback in `searchDefaultBashLocations` was reading `process.env.USERPROFILE` directly, breaking hermetic testing of that branch. Threaded through as an optional deps field, defaults to `process.env.USERPROFILE` for production. 3. Add `envOverride: ''` to the manual `GitBashDiscoveryDeps` fixture in the "skips where.exe git when bash is already found via PATH" test (INLINE). Without this, the test would pass for the wrong reason (env override) on developer machines that have `CLAUDE_CODE_GIT_BASH_PATH` set, instead of exercising the PATH short-circuit behavior the test is named after. Co-Authored-By: Claude --- src/utils/__tests__/windowsPaths.test.ts | 4 +++ src/utils/windowsPaths.ts | 43 +++++++++++++++++++----- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/src/utils/__tests__/windowsPaths.test.ts b/src/utils/__tests__/windowsPaths.test.ts index 6ff6a8c62f..340eb3ac9f 100644 --- a/src/utils/__tests__/windowsPaths.test.ts +++ b/src/utils/__tests__/windowsPaths.test.ts @@ -287,6 +287,10 @@ describe('findGitBashPathOrNullWithDeps', () => { return '' }, cwdFn: () => '/safe/cwd', + // Bypass process.env.CLAUDE_CODE_GIT_BASH_PATH so the test exercises + // the intended PATH short-circuit behavior rather than passing for + // the wrong reason (env override). + envOverride: '', } expect(findGitBashPathOrNullWithDeps(deps)).toBe(bashPath) expect(gitProbed).toBe(false) diff --git a/src/utils/windowsPaths.ts b/src/utils/windowsPaths.ts index 746245addb..165379538f 100644 --- a/src/utils/windowsPaths.ts +++ b/src/utils/windowsPaths.ts @@ -35,6 +35,13 @@ export type GitBashDiscoveryDeps = { execCommand: (cmd: string) => string /** Returns the current working directory (used to filter PATH-based lookups). */ cwdFn: () => string + /** + * `USERPROFILE` used to derive Scoop Git install paths. When provided, + * this is used instead of `process.env.USERPROFILE` — keeps the pure + * helper hermetic so the Scoop fallback can be tested without + * depending on the live environment. + */ + userProfile?: string | undefined /** * Optional override for `process.env.CLAUDE_CODE_GIT_BASH_PATH`. When * provided, this is used instead of the live environment — useful for tests. @@ -47,6 +54,7 @@ const DEFAULT_DEPS: GitBashDiscoveryDeps = { execCommand: cmd => execSync_DEPRECATED(cmd, { stdio: 'pipe', encoding: 'utf8' }).trim(), cwdFn: getCwd, + userProfile: process.env.USERPROFILE, envOverride: undefined, } @@ -57,6 +65,7 @@ const DEFAULT_DEPS: GitBashDiscoveryDeps = { */ function searchDefaultBashLocations( checkExists: (p: string) => boolean, + userProfile?: string, ): string | null { const candidates = [ // Standard Git for Windows install locations (both layouts). @@ -66,7 +75,6 @@ function searchDefaultBashLocations( 'C:\\Program Files (x86)\\Git\\usr\\bin\\bash.exe', ] // Scoop install: %USERPROFILE%\scoop\apps\git\current\usr\bin\bash.exe - const userProfile = process.env.USERPROFILE if (userProfile) { candidates.push( `${userProfile}\\scoop\\apps\\git\\current\\usr\\bin\\bash.exe`, @@ -114,16 +122,35 @@ function findExecutableWithDeps( const result = deps.execCommand(`where.exe ${executable}`) // SECURITY: Filter out any results from the current directory // to prevent executing malicious git.bat/cmd/exe files - const paths = result.split('\r\n').filter(Boolean) - const cwd = deps.cwdFn().toLowerCase() + // + // Use path.win32.* here so that `where.exe`'s Windows-style backslash + // paths are evaluated with Windows semantics regardless of the host + // OS. On POSIX, `path.resolve('C:\\foo')` treats the backslashes as + // literal characters and produces a wrong (relative) result, which + // would let cwd shadowing slip past this check. pathWin32 is + // already imported for the bash derivation below. + const paths = result + .split(/\r?\n/) + .map(p => p.trim()) + .filter(Boolean) + const cwd = pathWin32.resolve(deps.cwdFn()).toLowerCase() for (const candidatePath of paths) { // Normalize and compare paths to ensure we're not in current directory - const normalizedPath = path.resolve(candidatePath).toLowerCase() - const pathDir = path.dirname(normalizedPath).toLowerCase() + const normalizedPath = pathWin32.resolve(candidatePath).toLowerCase() + const pathDir = pathWin32.dirname(normalizedPath).toLowerCase() + // path.win32.relative(cwd, pathDir) returns: + // '' → pathDir === cwd + // '..' / '../...' → pathDir is outside cwd (or above it) + // 'subdir/...' → pathDir is inside cwd + // We reject entries whose dir is cwd itself or anywhere inside cwd. + const relativePathDir = pathWin32.relative(cwd, pathDir) - // Skip if the executable is in the current working directory - if (pathDir === cwd || normalizedPath.startsWith(cwd + path.sep)) { + if ( + relativePathDir === '' || + (!relativePathDir.startsWith('..') && + !pathWin32.isAbsolute(relativePathDir)) + ) { logForDebugging( `Skipping potentially malicious executable in current directory: ${candidatePath}`, ) @@ -188,7 +215,7 @@ export function findGitBashPathOrNullWithDeps( } // 4. Last resort: scan common install locations for bash.exe directly - return searchDefaultBashLocations(deps.checkExists) + return searchDefaultBashLocations(deps.checkExists, deps.userProfile) } /**