ClawX windows path robustness (#171)
Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Haze <hazeone@users.noreply.github.com>
This commit is contained in:
@@ -6,7 +6,7 @@ import { spawn } from 'child_process';
|
|||||||
import fs from 'fs';
|
import fs from 'fs';
|
||||||
import path from 'path';
|
import path from 'path';
|
||||||
import { app, shell } from 'electron';
|
import { app, shell } from 'electron';
|
||||||
import { getOpenClawConfigDir, ensureDir, getClawHubCliBinPath, getClawHubCliEntryPath } from '../utils/paths';
|
import { getOpenClawConfigDir, ensureDir, getClawHubCliBinPath, getClawHubCliEntryPath, quoteForCmd } from '../utils/paths';
|
||||||
|
|
||||||
export interface ClawHubSearchParams {
|
export interface ClawHubSearchParams {
|
||||||
query: string;
|
query: string;
|
||||||
@@ -87,17 +87,20 @@ export class ClawHubService {
|
|||||||
console.log(`Running ClawHub command: ${displayCommand}`);
|
console.log(`Running ClawHub command: ${displayCommand}`);
|
||||||
|
|
||||||
const isWin = process.platform === 'win32';
|
const isWin = process.platform === 'win32';
|
||||||
|
const useShell = isWin && !this.useNodeRunner;
|
||||||
const env = {
|
const env = {
|
||||||
...process.env,
|
...process.env,
|
||||||
CI: 'true',
|
CI: 'true',
|
||||||
FORCE_COLOR: '0', // Disable colors for easier parsing
|
FORCE_COLOR: '0',
|
||||||
};
|
};
|
||||||
if (this.useNodeRunner) {
|
if (this.useNodeRunner) {
|
||||||
env.ELECTRON_RUN_AS_NODE = '1';
|
env.ELECTRON_RUN_AS_NODE = '1';
|
||||||
}
|
}
|
||||||
const child = spawn(this.cliPath, commandArgs, {
|
const spawnCmd = useShell ? quoteForCmd(this.cliPath) : this.cliPath;
|
||||||
|
const spawnArgs = useShell ? commandArgs.map(a => quoteForCmd(a)) : commandArgs;
|
||||||
|
const child = spawn(spawnCmd, spawnArgs, {
|
||||||
cwd: this.workDir,
|
cwd: this.workDir,
|
||||||
shell: isWin && !this.useNodeRunner,
|
shell: useShell,
|
||||||
env: {
|
env: {
|
||||||
...env,
|
...env,
|
||||||
CLAWHUB_WORKDIR: this.workDir,
|
CLAWHUB_WORKDIR: this.workDir,
|
||||||
|
|||||||
@@ -13,7 +13,8 @@ import {
|
|||||||
getOpenClawDir,
|
getOpenClawDir,
|
||||||
getOpenClawEntryPath,
|
getOpenClawEntryPath,
|
||||||
isOpenClawBuilt,
|
isOpenClawBuilt,
|
||||||
isOpenClawPresent
|
isOpenClawPresent,
|
||||||
|
quoteForCmd,
|
||||||
} from '../utils/paths';
|
} from '../utils/paths';
|
||||||
import { getSetting } from '../utils/store';
|
import { getSetting } from '../utils/store';
|
||||||
import { getApiKey, getDefaultProvider, getProvider } from '../utils/secure-storage';
|
import { getApiKey, getDefaultProvider, getProvider } from '../utils/secure-storage';
|
||||||
@@ -755,11 +756,15 @@ export class GatewayManager extends EventEmitter {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
this.process = spawn(command, args, {
|
const useShell = !app.isPackaged && process.platform === 'win32';
|
||||||
|
const spawnCmd = useShell ? quoteForCmd(command) : command;
|
||||||
|
const spawnArgs = useShell ? args.map(a => quoteForCmd(a)) : args;
|
||||||
|
|
||||||
|
this.process = spawn(spawnCmd, spawnArgs, {
|
||||||
cwd: openclawDir,
|
cwd: openclawDir,
|
||||||
stdio: ['ignore', 'pipe', 'pipe'],
|
stdio: ['ignore', 'pipe', 'pipe'],
|
||||||
detached: false,
|
detached: false,
|
||||||
shell: !app.isPackaged && process.platform === 'win32', // shell only in dev on Windows
|
shell: useShell,
|
||||||
env: spawnEnv,
|
env: spawnEnv,
|
||||||
});
|
});
|
||||||
const child = this.process;
|
const child = this.process;
|
||||||
|
|||||||
@@ -8,6 +8,8 @@ import { homedir } from 'os';
|
|||||||
import { existsSync, mkdirSync, readFileSync, realpathSync } from 'fs';
|
import { existsSync, mkdirSync, readFileSync, realpathSync } from 'fs';
|
||||||
import { logger } from './logger';
|
import { logger } from './logger';
|
||||||
|
|
||||||
|
export { quoteForCmd, needsWinShell, prepareWinSpawn } from './win-shell';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Expand ~ to home directory
|
* Expand ~ to home directory
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { existsSync } from 'fs';
|
|||||||
import { join } from 'path';
|
import { join } from 'path';
|
||||||
import { getUvMirrorEnv } from './uv-env';
|
import { getUvMirrorEnv } from './uv-env';
|
||||||
import { logger } from './logger';
|
import { logger } from './logger';
|
||||||
|
import { quoteForCmd, needsWinShell } from './paths';
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Get the path to the bundled uv binary
|
* Get the path to the bundled uv binary
|
||||||
@@ -88,11 +89,12 @@ export async function installUv(): Promise<void> {
|
|||||||
*/
|
*/
|
||||||
export async function isPythonReady(): Promise<boolean> {
|
export async function isPythonReady(): Promise<boolean> {
|
||||||
const { bin: uvBin } = resolveUvBin();
|
const { bin: uvBin } = resolveUvBin();
|
||||||
|
const useShell = needsWinShell(uvBin);
|
||||||
|
|
||||||
return new Promise<boolean>((resolve) => {
|
return new Promise<boolean>((resolve) => {
|
||||||
try {
|
try {
|
||||||
const child = spawn(uvBin, ['python', 'find', '3.12'], {
|
const child = spawn(useShell ? quoteForCmd(uvBin) : uvBin, ['python', 'find', '3.12'], {
|
||||||
shell: process.platform === 'win32',
|
shell: useShell,
|
||||||
});
|
});
|
||||||
child.on('close', (code) => resolve(code === 0));
|
child.on('close', (code) => resolve(code === 0));
|
||||||
child.on('error', () => resolve(false));
|
child.on('error', () => resolve(false));
|
||||||
@@ -111,12 +113,13 @@ async function runPythonInstall(
|
|||||||
env: Record<string, string | undefined>,
|
env: Record<string, string | undefined>,
|
||||||
label: string,
|
label: string,
|
||||||
): Promise<void> {
|
): Promise<void> {
|
||||||
|
const useShell = needsWinShell(uvBin);
|
||||||
return new Promise<void>((resolve, reject) => {
|
return new Promise<void>((resolve, reject) => {
|
||||||
const stderrChunks: string[] = [];
|
const stderrChunks: string[] = [];
|
||||||
const stdoutChunks: string[] = [];
|
const stdoutChunks: string[] = [];
|
||||||
|
|
||||||
const child = spawn(uvBin, ['python', 'install', '3.12'], {
|
const child = spawn(useShell ? quoteForCmd(uvBin) : uvBin, ['python', 'install', '3.12'], {
|
||||||
shell: process.platform === 'win32',
|
shell: useShell,
|
||||||
env,
|
env,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -201,10 +204,11 @@ export async function setupManagedPython(): Promise<void> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// After installation, verify and log the Python path
|
// After installation, verify and log the Python path
|
||||||
|
const verifyShell = needsWinShell(uvBin);
|
||||||
try {
|
try {
|
||||||
const findPath = await new Promise<string>((resolve) => {
|
const findPath = await new Promise<string>((resolve) => {
|
||||||
const child = spawn(uvBin, ['python', 'find', '3.12'], {
|
const child = spawn(verifyShell ? quoteForCmd(uvBin) : uvBin, ['python', 'find', '3.12'], {
|
||||||
shell: process.platform === 'win32',
|
shell: verifyShell,
|
||||||
env: { ...process.env, ...uvEnv },
|
env: { ...process.env, ...uvEnv },
|
||||||
});
|
});
|
||||||
let output = '';
|
let output = '';
|
||||||
|
|||||||
65
electron/utils/win-shell.ts
Normal file
65
electron/utils/win-shell.ts
Normal file
@@ -0,0 +1,65 @@
|
|||||||
|
/**
|
||||||
|
* Windows shell quoting utilities for child_process.spawn().
|
||||||
|
*
|
||||||
|
* When spawn() is called with `shell: true` on Windows, the command and
|
||||||
|
* arguments are concatenated and passed to cmd.exe. Paths containing spaces
|
||||||
|
* must be wrapped in double-quotes to prevent cmd.exe from splitting them
|
||||||
|
* into separate tokens.
|
||||||
|
*
|
||||||
|
* This module is intentionally dependency-free so it can be unit-tested
|
||||||
|
* without mocking Electron.
|
||||||
|
*/
|
||||||
|
import path from 'path';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Quote a path/value for safe use with Windows cmd.exe (shell: true in spawn).
|
||||||
|
*
|
||||||
|
* When Node.js spawn is called with `shell: true` on Windows, cmd.exe
|
||||||
|
* interprets spaces as argument separators. Wrapping the value in double
|
||||||
|
* quotes prevents this. On non-Windows platforms the value is returned
|
||||||
|
* unchanged so this function can be called unconditionally.
|
||||||
|
*/
|
||||||
|
export function quoteForCmd(value: string): string {
|
||||||
|
if (process.platform !== 'win32') return value;
|
||||||
|
if (!value.includes(' ')) return value;
|
||||||
|
if (value.startsWith('"') && value.endsWith('"')) return value;
|
||||||
|
return `"${value}"`;
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Determine whether a spawn call needs `shell: true` on Windows.
|
||||||
|
*
|
||||||
|
* Full (absolute) paths can be executed directly by the OS via
|
||||||
|
* CreateProcessW, which handles spaces correctly without a shell.
|
||||||
|
* Simple command names (e.g. 'uv', 'node') need shell for PATH/PATHEXT
|
||||||
|
* resolution on Windows.
|
||||||
|
*/
|
||||||
|
export function needsWinShell(bin: string): boolean {
|
||||||
|
if (process.platform !== 'win32') return false;
|
||||||
|
return !path.win32.isAbsolute(bin);
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Prepare command and args for spawn(), handling Windows paths with spaces.
|
||||||
|
*
|
||||||
|
* Returns the shell option, the (possibly quoted) command, and the
|
||||||
|
* (possibly quoted) args array ready for child_process.spawn().
|
||||||
|
*/
|
||||||
|
export function prepareWinSpawn(
|
||||||
|
command: string,
|
||||||
|
args: string[],
|
||||||
|
forceShell?: boolean,
|
||||||
|
): { shell: boolean; command: string; args: string[] } {
|
||||||
|
const isWin = process.platform === 'win32';
|
||||||
|
const useShell = forceShell ?? (isWin && !path.win32.isAbsolute(command));
|
||||||
|
|
||||||
|
if (!useShell || !isWin) {
|
||||||
|
return { shell: useShell, command, args };
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
shell: true,
|
||||||
|
command: quoteForCmd(command),
|
||||||
|
args: args.map(a => quoteForCmd(a)),
|
||||||
|
};
|
||||||
|
}
|
||||||
@@ -74,10 +74,9 @@ async function setupTarget(id) {
|
|||||||
echo`📂 Extracting...`;
|
echo`📂 Extracting...`;
|
||||||
if (target.filename.endsWith('.zip')) {
|
if (target.filename.endsWith('.zip')) {
|
||||||
if (os.platform() === 'win32') {
|
if (os.platform() === 'win32') {
|
||||||
// Use .NET Framework for ZIP extraction (more reliable than Expand-Archive)
|
const { execFileSync } = await import('child_process');
|
||||||
const { execSync } = await import('child_process');
|
|
||||||
const psCommand = `Add-Type -AssemblyName System.IO.Compression.FileSystem; [System.IO.Compression.ZipFile]::ExtractToDirectory('${archivePath.replace(/'/g, "''")}', '${tempDir.replace(/'/g, "''")}')`;
|
const psCommand = `Add-Type -AssemblyName System.IO.Compression.FileSystem; [System.IO.Compression.ZipFile]::ExtractToDirectory('${archivePath.replace(/'/g, "''")}', '${tempDir.replace(/'/g, "''")}')`;
|
||||||
execSync(`powershell.exe -NoProfile -Command "${psCommand}"`, { stdio: 'inherit' });
|
execFileSync('powershell.exe', ['-NoProfile', '-Command', psCommand], { stdio: 'inherit' });
|
||||||
} else {
|
} else {
|
||||||
await $`unzip -q -o ${archivePath} -d ${tempDir}`;
|
await $`unzip -q -o ${archivePath} -d ${tempDir}`;
|
||||||
}
|
}
|
||||||
|
|||||||
162
tests/unit/win-shell.test.ts
Normal file
162
tests/unit/win-shell.test.ts
Normal file
@@ -0,0 +1,162 @@
|
|||||||
|
/**
|
||||||
|
* Windows shell quoting utilities tests
|
||||||
|
*/
|
||||||
|
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
|
||||||
|
|
||||||
|
// We test the pure functions directly by dynamically importing after
|
||||||
|
// patching process.platform, since the functions check it at call time.
|
||||||
|
const originalPlatform = process.platform;
|
||||||
|
|
||||||
|
function setPlatform(platform: string) {
|
||||||
|
Object.defineProperty(process, 'platform', { value: platform, writable: true });
|
||||||
|
}
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
Object.defineProperty(process, 'platform', { value: originalPlatform, writable: true });
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('quoteForCmd', () => {
|
||||||
|
let quoteForCmd: (value: string) => string;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const mod = await import('@electron/utils/win-shell');
|
||||||
|
quoteForCmd = mod.quoteForCmd;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns value unchanged on non-Windows', () => {
|
||||||
|
setPlatform('linux');
|
||||||
|
expect(quoteForCmd('C:\\Program Files\\uv.exe')).toBe('C:\\Program Files\\uv.exe');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns value unchanged on macOS', () => {
|
||||||
|
setPlatform('darwin');
|
||||||
|
expect(quoteForCmd('/Applications/My App/bin')).toBe('/Applications/My App/bin');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns value unchanged on Windows when no spaces', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(quoteForCmd('C:\\tools\\uv.exe')).toBe('C:\\tools\\uv.exe');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('wraps in double quotes on Windows when path has spaces', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(quoteForCmd('C:\\Program Files\\uv.exe')).toBe('"C:\\Program Files\\uv.exe"');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('wraps user home paths with spaces', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(quoteForCmd('C:\\Users\\John Doe\\AppData\\Local\\uv.exe'))
|
||||||
|
.toBe('"C:\\Users\\John Doe\\AppData\\Local\\uv.exe"');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not double-quote already quoted values', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(quoteForCmd('"C:\\Program Files\\uv.exe"')).toBe('"C:\\Program Files\\uv.exe"');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('handles simple command names without spaces', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(quoteForCmd('uv')).toBe('uv');
|
||||||
|
expect(quoteForCmd('node')).toBe('node');
|
||||||
|
expect(quoteForCmd('pnpm')).toBe('pnpm');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('handles empty string', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(quoteForCmd('')).toBe('');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('needsWinShell', () => {
|
||||||
|
let needsWinShell: (bin: string) => boolean;
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const mod = await import('@electron/utils/win-shell');
|
||||||
|
needsWinShell = mod.needsWinShell;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns false on non-Windows', () => {
|
||||||
|
setPlatform('linux');
|
||||||
|
expect(needsWinShell('uv')).toBe(false);
|
||||||
|
expect(needsWinShell('/usr/bin/uv')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns true on Windows for simple command names', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(needsWinShell('uv')).toBe(true);
|
||||||
|
expect(needsWinShell('node')).toBe(true);
|
||||||
|
expect(needsWinShell('pnpm')).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns false on Windows for absolute paths', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(needsWinShell('C:\\Program Files\\uv.exe')).toBe(false);
|
||||||
|
expect(needsWinShell('D:\\tools\\bin\\uv.exe')).toBe(false);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('returns true on Windows for relative paths', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
expect(needsWinShell('bin\\uv.exe')).toBe(true);
|
||||||
|
expect(needsWinShell('.\\uv.exe')).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('prepareWinSpawn', () => {
|
||||||
|
let prepareWinSpawn: (
|
||||||
|
command: string,
|
||||||
|
args: string[],
|
||||||
|
forceShell?: boolean,
|
||||||
|
) => { shell: boolean; command: string; args: string[] };
|
||||||
|
|
||||||
|
beforeEach(async () => {
|
||||||
|
const mod = await import('@electron/utils/win-shell');
|
||||||
|
prepareWinSpawn = mod.prepareWinSpawn;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('does not quote on non-Windows', () => {
|
||||||
|
setPlatform('linux');
|
||||||
|
const result = prepareWinSpawn('/usr/bin/uv', ['python', 'install', '3.12']);
|
||||||
|
expect(result.shell).toBe(false);
|
||||||
|
expect(result.command).toBe('/usr/bin/uv');
|
||||||
|
expect(result.args).toEqual(['python', 'install', '3.12']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('quotes command and args with spaces on Windows with shell', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
const result = prepareWinSpawn(
|
||||||
|
'C:\\Program Files\\uv.exe',
|
||||||
|
['python', 'install', '3.12'],
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
expect(result.shell).toBe(true);
|
||||||
|
expect(result.command).toBe('"C:\\Program Files\\uv.exe"');
|
||||||
|
expect(result.args).toEqual(['python', 'install', '3.12']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('quotes args that contain spaces on Windows with shell', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
const result = prepareWinSpawn(
|
||||||
|
'node',
|
||||||
|
['C:\\Users\\John Doe\\script.js', '--port', '18789'],
|
||||||
|
true,
|
||||||
|
);
|
||||||
|
expect(result.shell).toBe(true);
|
||||||
|
expect(result.command).toBe('node');
|
||||||
|
expect(result.args).toEqual(['"C:\\Users\\John Doe\\script.js"', '--port', '18789']);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('auto-detects shell need based on absolute path on Windows', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
const absResult = prepareWinSpawn(
|
||||||
|
'C:\\tools\\uv.exe',
|
||||||
|
['python', 'find', '3.12'],
|
||||||
|
);
|
||||||
|
expect(absResult.shell).toBe(false);
|
||||||
|
|
||||||
|
const relResult = prepareWinSpawn(
|
||||||
|
'uv',
|
||||||
|
['python', 'find', '3.12'],
|
||||||
|
);
|
||||||
|
expect(relResult.shell).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user