MediaMetz/gemini-cli
1
0
Fork 0
mirror of https://github.com/google-gemini/gemini-cli.git synced 2026-06-10 02:52:54 -07:00
Code Issues Packages Projects Releases Wiki Activity

fix(core): prefer pwsh.exe over Windows PowerShell 5.1 (#25859) (#25900)

Browse Source 
Co-authored-by: Tommaso Sciortino <sciortino@gmail.com>
This commit is contained in:
kaluchi
2026-05-18 20:44:05 +03:00
committed by GitHub
parent 7ff60809ef
commit dc47aaa2d9
10 changed files with 203 additions and 94 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/cli/src/ui/components/SessionSummaryDisplay.test.tsx
+1 -18
View File
@@ -12,7 +12,6 @@ import { useConfig } from '../contexts/ConfigContext.js';
import { type SessionMetrics } from '../contexts/SessionContext.js';
import {
ToolCallDecision,
getShellConfiguration,
isWindows,
type WorktreeSettings,
} from '@google/gemini-cli-core';
@@ -22,7 +21,6 @@ vi.mock('@google/gemini-cli-core', async (importOriginal) => {
await importOriginal<typeof import('@google/gemini-cli-core')>();
return {
...actual,
getShellConfiguration: vi.fn(),
isWindows: vi.fn(),
};
});
@@ -45,7 +43,6 @@ vi.mock('../contexts/ConfigContext.js', async (importOriginal) => {
};
});
const getShellConfigurationMock = vi.mocked(getShellConfiguration);
const isWindowsMock = vi.mocked(isWindows);
const useSessionStatsMock = vi.mocked(SessionContext.useSessionStats);
@@ -104,11 +101,6 @@ describe('<SessionSummaryDisplay />', () => {
};
beforeEach(() => {
getShellConfigurationMock.mockReturnValue({
executable: 'bash',
argsPrefix: ['-c'],
shell: 'bash',
});
isWindowsMock.mockReturnValue(false);
});
@@ -173,11 +165,6 @@ describe('<SessionSummaryDisplay />', () => {
it('renders a standard UUID-formatted session ID in the footer (powershell) on Windows', async () => {
isWindowsMock.mockReturnValue(true);
getShellConfigurationMock.mockReturnValue({
executable: 'powershell.exe',
argsPrefix: ['-NoProfile', '-Command'],
shell: 'powershell',
});
const uuidSessionId = '1234-abcd-5678-efgh';
const { lastFrame, unmount } = await renderWithMockedStats(
@@ -192,11 +179,7 @@ describe('<SessionSummaryDisplay />', () => {
});
it('sanitizes a malicious session ID in the footer (powershell)', async () => {
getShellConfigurationMock.mockReturnValue({
executable: 'powershell.exe',
argsPrefix: ['-NoProfile', '-Command'],
shell: 'powershell',
});
isWindowsMock.mockReturnValue(true);
const maliciousSessionId = "'; rm -rf / #";
const { lastFrame, unmount } = await renderWithMockedStats(
packages/cli/src/ui/components/SessionSummaryDisplay.tsx
+2 -2
View File
@@ -10,8 +10,8 @@ import { useSessionStats } from '../contexts/SessionContext.js';
import { useConfig } from '../contexts/ConfigContext.js';
import {
escapeShellArg,
getShellConfiguration,
isWindows,
type ShellType,
} from '@google/gemini-cli-core';
interface SessionSummaryDisplayProps {
@@ -23,7 +23,7 @@ export const SessionSummaryDisplay: React.FC<SessionSummaryDisplayProps> = ({
}) => {
const { stats } = useSessionStats();
const config = useConfig();
const { shell } = getShellConfiguration();
const shell: ShellType = isWindows() ? 'powershell' : 'bash';
const worktreeSettings = config.getWorktreeSettings();
packages/core/src/hooks/hookRunner.test.ts
+3 -3
View File
@@ -365,7 +365,7 @@ describe('HookRunner', () => {
);
expect(spawn).toHaveBeenCalledWith(
expect.stringMatching(/bash|powershell/),
expect.stringMatching(/bash|pwsh|powershell/),
expect.arrayContaining([
expect.stringMatching(/['"]?\/test\/project['"]?\/hooks\/test\.sh/),
]),
@@ -408,7 +408,7 @@ describe('HookRunner', () => {
);
expect(spawn).toHaveBeenCalledWith(
expect.stringMatching(/bash|powershell/),
expect.stringMatching(/bash|pwsh|powershell/),
expect.arrayContaining([
expect.stringMatching(
/ls ['"]\/test\/project\/plans with spaces['"]/,
@@ -447,7 +447,7 @@ describe('HookRunner', () => {
// If secure, spawn will be called with the shell executable and escaped command
expect(spawn).toHaveBeenCalledWith(
expect.stringMatching(/bash|powershell/),
expect.stringMatching(/bash|pwsh|powershell/),
expect.arrayContaining([
expect.stringMatching(/ls (['"]).*echo.*pwned.*\1/),
]),
packages/core/src/services/shellExecutionService.test.ts
+2 -2
View File
@@ -213,7 +213,7 @@ describe('ShellExecutionService', () => {
mockSerializeTerminalToObject.mockReturnValue([]);
mockIsBinary.mockReturnValue(false);
mockPlatform.mockReturnValue('linux');
mockResolveExecutable.mockImplementation(async (exe: string) => exe);
mockResolveExecutable.mockImplementation((exe: string) => exe);
process.env['PATH'] = '/test/path';
mockGetPty.mockResolvedValue({
module: { spawn: mockPtySpawn },
@@ -2064,7 +2064,7 @@ describe('ShellExecutionService environment variables', () => {
sandboxManager: mockSandboxManager,
};
mockResolveExecutable.mockResolvedValue('/bin/bash/resolved');
mockResolveExecutable.mockReturnValue('/bin/bash/resolved');
const mockChild = new EventEmitter() as unknown as ChildProcess;
mockChild.stdout = new EventEmitter() as unknown as Readable;
mockChild.stderr = new EventEmitter() as unknown as Readable;
packages/core/src/services/shellExecutionService.ts
+1 -2
View File
@@ -414,8 +414,7 @@ export class ShellExecutionService {
executable = 'cmd.exe';
}
const resolvedExecutable =
(await resolveExecutable(executable)) ?? executable;
const resolvedExecutable = resolveExecutable(executable) ?? executable;
const guardedCommand = ensurePromptvarsDisabled(commandToExecute, shell);
const spawnArgs = [...argsPrefix, guardedCommand];
packages/core/src/services/shellExecutionService.windows.integration.test.ts
+89
View File
@@ -0,0 +1,89 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect } from 'vitest';
import os from 'node:os';
import { ShellExecutionService } from './shellExecutionService.js';
import { NoopSandboxManager } from './sandboxManager.js';
const isWindows = os.platform() === 'win32';
/**
* Real-shell integration tests that reproduce the regression class from
* issue #25859: commands with inline double quotes executed on Windows
* lose their quotes when they reach the native executable, because
* Windows PowerShell 5.1 mangles embedded " during native-command
* argument passing. PowerShell 7 (pwsh.exe) passes arguments correctly.
*
* These tests exercise the full pipeline end-to-end. They pass when
* gemini-cli selects pwsh.exe from PATH; they fail when the pipeline
* routes through Windows PowerShell 5.1.
*/
describe.skipIf(!isWindows)(
'ShellExecutionService Windows quoting (real shell)',
() => {
const baseConfig = {
sanitizationConfig: {
allowedEnvironmentVariables: [],
blockedEnvironmentVariables: [],
enableEnvironmentVariableRedaction: false,
},
sandboxManager: new NoopSandboxManager(),
};
async function runReal(command: string) {
const controller = new AbortController();
const handle = await ShellExecutionService.execute(
command,
process.cwd(),
() => {},
controller.signal,
false,
baseConfig,
);
const result = await handle.result;
return { result, output: result.output };
}
it('should preserve inline double quotes through node -e', async () => {
const { result, output } = await runReal(
`node -e 'console.log("preserved")'`,
);
expect(result.exitCode).toBe(0);
expect(output).toBe('preserved');
});
it('should preserve double quotes inside JSON output', async () => {
const { result, output } = await runReal(
`node -e 'console.log(JSON.stringify({ok:"yes"}))'`,
);
expect(result.exitCode).toBe(0);
expect(output).toBe('{"ok":"yes"}');
});
it('should handle quoted argument containing a space', async () => {
const { result, output } = await runReal(
`node -e "console.log('hello world')"`,
);
expect(result.exitCode).toBe(0);
expect(output).toBe('hello world');
});
it('should handle a mixed-quote regex literal', async () => {
const { result, output } = await runReal(
`node -e 'console.log(String("a").match(/"/))'`,
);
expect(result.exitCode).toBe(0);
expect(output).toBe('null');
});
it('should pass a literal double-quote byte through to stdout', async () => {
const { result, output } = await runReal(`node -e 'console.log("\\"")'`);
expect(result.exitCode).toBe(0);
expect(output).toBe('"');
});
},
);
packages/core/src/tools/ripGrep.test.ts
+6 -6
View File
@@ -1825,7 +1825,7 @@ describe('resolveRipgrepPath', () => {
it('should fall back to system PATH if both bundled paths are missing and system is trusted', async () => {
vi.mocked(fileExists).mockResolvedValue(false);
vi.mocked(resolveExecutable).mockResolvedValue('/usr/bin/rg');
vi.mocked(resolveExecutable).mockReturnValue('/usr/bin/rg');
vi.mocked(resolveToRealPath).mockReturnValue('/usr/bin/rg');
const resolvedPath = await resolveRipgrepPath();
@@ -1836,7 +1836,7 @@ describe('resolveRipgrepPath', () => {
it('should reject system PATH if it is in the current working directory', async () => {
vi.mocked(fileExists).mockResolvedValue(false);
const unsafePath = path.join(process.cwd(), 'rg');
vi.mocked(resolveExecutable).mockResolvedValue(unsafePath);
vi.mocked(resolveExecutable).mockReturnValue(unsafePath);
vi.mocked(resolveToRealPath).mockReturnValue(unsafePath);
const resolvedPath = await resolveRipgrepPath();
@@ -1848,7 +1848,7 @@ describe('resolveRipgrepPath', () => {
const trustedLink = '/usr/local/bin/rg';
const trustedRealPath = '/opt/homebrew/Cellar/ripgrep/13.0.0/bin/rg';
vi.mocked(resolveExecutable).mockResolvedValue(trustedLink);
vi.mocked(resolveExecutable).mockReturnValue(trustedLink);
vi.mocked(resolveToRealPath).mockReturnValue(trustedRealPath);
const resolvedPath = await resolveRipgrepPath();
@@ -1857,7 +1857,7 @@ describe('resolveRipgrepPath', () => {
it('should return null if binary is missing from both bundled paths and system PATH', async () => {
vi.mocked(fileExists).mockResolvedValue(false);
vi.mocked(resolveExecutable).mockResolvedValue(undefined);
vi.mocked(resolveExecutable).mockReturnValue(undefined);
const resolvedPath = await resolveRipgrepPath();
expect(resolvedPath).toBeNull();
@@ -1883,7 +1883,7 @@ describe('resolveRipgrepPath', () => {
it('should fall back to system PATH if system is trusted on Windows', async () => {
vi.mocked(fileExists).mockResolvedValue(false);
vi.mocked(resolveExecutable).mockResolvedValue(
vi.mocked(resolveExecutable).mockReturnValue(
'C:\\Windows\\System32\\rg.exe',
);
vi.mocked(resolveToRealPath).mockReturnValue(
@@ -1898,7 +1898,7 @@ describe('resolveRipgrepPath', () => {
it('should reject system PATH if it is untrusted on Windows', async () => {
vi.mocked(fileExists).mockResolvedValue(false);
const unsafePath = 'D:\\Downloads\\rg.exe';
vi.mocked(resolveExecutable).mockResolvedValue(unsafePath);
vi.mocked(resolveExecutable).mockReturnValue(unsafePath);
vi.mocked(resolveToRealPath).mockReturnValue(unsafePath);
const resolvedPath = await resolveRipgrepPath();
packages/core/src/tools/ripGrep.ts
+1 -1
View File
@@ -72,7 +72,7 @@ export async function resolveRipgrepPath(): Promise<string | null> {
}
// 3. Fallback: check system PATH
const systemRg = await resolveExecutable('rg');
const systemRg = resolveExecutable('rg');
if (systemRg) {
// Security: Validate the system executable to prevent Search Path Interruption.
const realPath = resolveToRealPath(systemRg);
packages/core/src/utils/shell-utils.test.ts
+61 -41
View File
@@ -38,17 +38,13 @@ vi.mock('os', () => ({
homedir: mockHomedir,
}));
const mockAccess = vi.hoisted(() => vi.fn());
const mockAccessSync = vi.hoisted(() => vi.fn());
vi.mock('node:fs', () => ({
default: {
promises: {
access: mockAccess,
},
accessSync: mockAccessSync,
constants: { X_OK: 1 },
},
promises: {
access: mockAccess,
},
accessSync: mockAccessSync,
constants: { X_OK: 1 },
}));
@@ -458,10 +454,8 @@ describe('escapeShellArg', () => {
});
describe('getShellConfiguration', () => {
const originalEnv = { ...process.env };
afterEach(() => {
process.env = originalEnv;
vi.unstubAllEnvs();
});
it('should return bash configuration on Linux', () => {
@@ -483,19 +477,38 @@ describe('getShellConfiguration', () => {
describe('on Windows', () => {
beforeEach(() => {
mockPlatform.mockReturnValue('win32');
vi.stubEnv('ComSpec', '');
mockAccessSync.mockImplementation(() => {
throw new Error('ENOENT');
});
});
it('should return PowerShell configuration by default', () => {
delete process.env['ComSpec'];
const config = getShellConfiguration();
expect(config.executable).toBe('powershell.exe');
expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']);
expect(config.shell).toBe('powershell');
});
it.skipIf(!isWindowsRuntime)(
'should prefer pwsh.exe over powershell.exe when pwsh is available in PATH',
() => {
const pwshDir = path.resolve('C:\\Program Files\\PowerShell\\7');
const pwshPath = path.join(pwshDir, 'pwsh.exe');
vi.stubEnv('PATH', pwshDir);
mockAccessSync.mockImplementation((p: string) => {
if (p === pwshPath) return;
throw new Error('ENOENT');
});
const config = getShellConfiguration();
expect(config.executable).toBe(pwshPath);
expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']);
expect(config.shell).toBe('powershell');
},
);
it('should ignore ComSpec when pointing to cmd.exe', () => {
const cmdPath = 'C:\\WINDOWS\\system32\\cmd.exe';
process.env['ComSpec'] = cmdPath;
vi.stubEnv('ComSpec', 'C:\\WINDOWS\\system32\\cmd.exe');
const config = getShellConfiguration();
expect(config.executable).toBe('powershell.exe');
expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']);
@@ -505,7 +518,7 @@ describe('getShellConfiguration', () => {
it('should return PowerShell configuration if ComSpec points to powershell.exe', () => {
const psPath =
'C:\\WINDOWS\\System32\\WindowsPowerShell\\v1.0\\powershell.exe';
process.env['ComSpec'] = psPath;
vi.stubEnv('ComSpec', psPath);
const config = getShellConfiguration();
expect(config.executable).toBe(psPath);
expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']);
@@ -514,7 +527,7 @@ describe('getShellConfiguration', () => {
it('should return PowerShell configuration if ComSpec points to pwsh.exe', () => {
const pwshPath = 'C:\\Program Files\\PowerShell\\7\\pwsh.exe';
process.env['ComSpec'] = pwshPath;
vi.stubEnv('ComSpec', pwshPath);
const config = getShellConfiguration();
expect(config.executable).toBe(pwshPath);
expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']);
@@ -522,7 +535,7 @@ describe('getShellConfiguration', () => {
});
it('should be case-insensitive when checking ComSpec', () => {
process.env['ComSpec'] = 'C:\\Path\\To\\POWERSHELL.EXE';
vi.stubEnv('ComSpec', 'C:\\Path\\To\\POWERSHELL.EXE');
const config = getShellConfiguration();
expect(config.executable).toBe('C:\\Path\\To\\POWERSHELL.EXE');
expect(config.argsPrefix).toEqual(['-NoProfile', '-Command']);
@@ -566,63 +579,70 @@ describe('hasRedirection (PowerShell via mock)', () => {
});
describe('resolveExecutable', () => {
const originalEnv = process.env;
beforeEach(() => {
process.env = { ...originalEnv };
mockAccess.mockReset();
mockAccessSync.mockReset();
});
afterEach(() => {
process.env = originalEnv;
vi.unstubAllEnvs();
});
it('should return the absolute path if it exists and is executable', async () => {
it('should return the absolute path if it exists and is executable', () => {
const absPath = path.resolve('/usr/bin/git');
mockAccess.mockResolvedValue(undefined); // success
expect(await resolveExecutable(absPath)).toBe(absPath);
expect(mockAccess).toHaveBeenCalledWith(absPath, 1);
mockAccessSync.mockImplementation(() => undefined);
expect(resolveExecutable(absPath)).toBe(absPath);
expect(mockAccessSync).toHaveBeenCalledWith(absPath, 1);
});
it('should return undefined for absolute path if it does not exist', async () => {
it('should return undefined for absolute path if it does not exist', () => {
const absPath = path.resolve('/usr/bin/nonexistent');
mockAccess.mockRejectedValue(new Error('ENOENT'));
expect(await resolveExecutable(absPath)).toBeUndefined();
mockAccessSync.mockImplementation(() => {
throw new Error('ENOENT');
});
expect(resolveExecutable(absPath)).toBeUndefined();
});
it('should resolve executable in PATH', async () => {
it('should resolve executable in PATH', () => {
const binDir = path.resolve('/bin');
const usrBinDir = path.resolve('/usr/bin');
process.env['PATH'] = `${binDir}${path.delimiter}${usrBinDir}`;
vi.stubEnv('PATH', `${binDir}${path.delimiter}${usrBinDir}`);
mockPlatform.mockReturnValue('linux');
const targetPath = path.join(usrBinDir, 'ls');
mockAccess.mockImplementation(async (p: string) => {
mockAccessSync.mockImplementation((p: string) => {
if (p === targetPath) return undefined;
throw new Error('ENOENT');
});
expect(await resolveExecutable('ls')).toBe(targetPath);
expect(resolveExecutable('ls')).toBe(targetPath);
});
it('should try extensions on Windows', async () => {
it('should try extensions on Windows', () => {
const sys32 = path.resolve('C:\\Windows\\System32');
process.env['PATH'] = sys32;
vi.stubEnv('PATH', sys32);
mockPlatform.mockReturnValue('win32');
mockAccess.mockImplementation(async (p: string) => {
// Use includes because on Windows path separators might differ
mockAccessSync.mockImplementation((p: string) => {
if (p.includes('cmd.exe')) return undefined;
throw new Error('ENOENT');
});
expect(await resolveExecutable('cmd')).toContain('cmd.exe');
expect(resolveExecutable('cmd')).toContain('cmd.exe');
});
it('should return undefined if not found in PATH', async () => {
process.env['PATH'] = path.resolve('/bin');
it('should return undefined if not found in PATH', () => {
vi.stubEnv('PATH', path.resolve('/bin'));
mockPlatform.mockReturnValue('linux');
mockAccess.mockRejectedValue(new Error('ENOENT'));
mockAccessSync.mockImplementation(() => {
throw new Error('ENOENT');
});
expect(await resolveExecutable('unknown')).toBeUndefined();
expect(resolveExecutable('unknown')).toBeUndefined();
});
it('should return undefined if PATH is unset', () => {
vi.stubEnv('PATH', '');
mockPlatform.mockReturnValue('linux');
expect(resolveExecutable('anything')).toBeUndefined();
});
});
packages/core/src/utils/shell-utils.ts
+37 -19
View File
@@ -76,29 +76,30 @@ export interface ShellConfiguration {
shell: ShellType;
}
export async function resolveExecutable(
exe: string,
): Promise<string | undefined> {
if (path.isAbsolute(exe)) {
try {
await fs.promises.access(exe, fs.constants.X_OK);
return exe;
} catch {
return undefined;
}
function isExecutable(filePath: string): boolean {
try {
fs.accessSync(filePath, fs.constants.X_OK);
return true;
} catch {
return false;
}
}
export function resolveExecutable(exe: string): string | undefined {
if (path.isAbsolute(exe)) {
return isExecutable(exe) ? exe : undefined;
}
const pathEnv = process.env['PATH'];
if (!pathEnv) {
return undefined;
}
const paths = (process.env['PATH'] || '').split(path.delimiter);
const extensions =
os.platform() === 'win32' ? ['.exe', '.cmd', '.bat', ''] : [''];
for (const p of paths) {
for (const dir of pathEnv.split(path.delimiter)) {
for (const ext of extensions) {
const fullPath = path.join(p, exe + ext);
try {
await fs.promises.access(fullPath, fs.constants.X_OK);
const fullPath = path.join(dir, exe + ext);
if (isExecutable(fullPath)) {
return fullPath;
} catch {
continue;
}
}
}
@@ -644,6 +645,14 @@ export function parseCommandDetails(
* This ensures we can execute command strings predictably and securely across platforms
* using the `spawn(executable, [...argsPrefix, commandString], { shell: false })` pattern.
*
* On Windows, PowerShell 7 (pwsh.exe) is preferred over Windows PowerShell 5.1
* (powershell.exe) when available on PATH. Windows PowerShell 5.1 silently
* strips embedded double quotes from arguments to native executables — see
* issue #25859. PowerShell 7 uses standards-compliant argument passing and
* does not exhibit this regression. When pwsh.exe is not installed, we fall
* back to powershell.exe to preserve the existing behavior and the full
* cmdlet surface users depend on.
*
* @returns The ShellConfiguration for the current environment.
*/
export function getShellConfiguration(): ShellConfiguration {
@@ -663,7 +672,16 @@ export function getShellConfiguration(): ShellConfiguration {
}
}
// Default to PowerShell for all other Windows configurations.
const pwshPath = resolveExecutable('pwsh.exe');
if (pwshPath) {
return {
executable: pwshPath,
argsPrefix: ['-NoProfile', '-Command'],
shell: 'powershell',
};
}
// Fall back to Windows PowerShell 5.1 when pwsh.exe is not installed.
return {
executable: 'powershell.exe',
argsPrefix: ['-NoProfile', '-Command'],