Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
151 changes: 151 additions & 0 deletions apps/frontend/src/main/__tests__/python-detector.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,151 @@
import { parsePythonCommand, validatePythonPath } from '../python-detector';

describe('parsePythonCommand', () => {
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Add Jest type definitions to fix pipeline failure.

The CI pipeline reports Cannot find name 'describe'. The test file needs Jest type definitions to compile.

🔎 Proposed fix

Add the import at the top of the file or install @types/jest:

+/// <reference types="jest" />
 import { parsePythonCommand, validatePythonPath } from '../python-detector';

 describe('parsePythonCommand', () => {

Alternatively, ensure @types/jest is installed and included in tsconfig.json:

npm i --save-dev @types/jest
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
import { parsePythonCommand, validatePythonPath } from '../python-detector';
describe('parsePythonCommand', () => {
/// <reference types="jest" />
import { parsePythonCommand, validatePythonPath } from '../python-detector';
describe('parsePythonCommand', () => {
🧰 Tools
🪛 GitHub Actions: CI

[error] 3-3: Cannot find name 'describe'. Do you need to install type definitions for a test runner? Try 'npm i --save-dev @types/jest' or 'npm i --save-dev @types/mocha'.

🤖 Prompt for AI Agents
In apps/frontend/src/main/__tests__/python-detector.test.ts around lines 1 to 3,
the test file fails to compile because Jest globals like "describe" are not
recognized; fix by either adding Jest type definitions to the file (e.g., add a
single line at the top referencing Jest types) or install and enable @types/jest
project-wide (npm i --save-dev @types/jest) and ensure tsconfig.json includes
"jest" in the "types" array or the test tsconfig includes the Jest types so the
compiler recognizes Jest globals.

describe('paths with spaces', () => {
it('should preserve paths with spaces when they contain path separators', () => {
const [cmd, args] = parsePythonCommand('/Users/user/Library/Application Support/MyApp/venv/bin/python');
expect(cmd).toBe('/Users/user/Library/Application Support/MyApp/venv/bin/python');
expect(args).toEqual([]);
});

it('should preserve Windows paths with spaces', () => {
const [cmd, args] = parsePythonCommand('C:\\Program Files\\Python310\\python.exe');
expect(cmd).toBe('C:\\Program Files\\Python310\\python.exe');
expect(args).toEqual([]);
});

it('should preserve macOS Application Support paths', () => {
const [cmd, args] = parsePythonCommand('/Users/testuser/Library/Application Support/Auto-Claude/python-venv/bin/python3');
expect(cmd).toBe('/Users/testuser/Library/Application Support/Auto-Claude/python-venv/bin/python3');
expect(args).toEqual([]);
});

it('should preserve paths with multiple spaces', () => {
const [cmd, args] = parsePythonCommand('/path/with multiple spaces/python3');
expect(cmd).toBe('/path/with multiple spaces/python3');
expect(args).toEqual([]);
});
});

describe('simple commands', () => {
it('should split "py -3" into command and args', () => {
const [cmd, args] = parsePythonCommand('py -3');
expect(cmd).toBe('py');
expect(args).toEqual(['-3']);
});

it('should handle plain python command', () => {
const [cmd, args] = parsePythonCommand('python3');
expect(cmd).toBe('python3');
expect(args).toEqual([]);
});

it('should handle python with version', () => {
const [cmd, args] = parsePythonCommand('python');
expect(cmd).toBe('python');
expect(args).toEqual([]);
});
});

describe('quoted paths', () => {
it('should strip double quotes from paths', () => {
const [cmd, args] = parsePythonCommand('"/path/to/python"');
expect(cmd).toBe('/path/to/python');
expect(args).toEqual([]);
});

it('should strip single quotes from paths', () => {
const [cmd, args] = parsePythonCommand("'/path/to/python'");
expect(cmd).toBe('/path/to/python');
expect(args).toEqual([]);
});

it('should handle quoted paths with spaces', () => {
const [cmd, args] = parsePythonCommand('"/Users/user/My Apps/venv/bin/python"');
expect(cmd).toBe('/Users/user/My Apps/venv/bin/python');
expect(args).toEqual([]);
});
});

describe('edge cases', () => {
it('should throw on empty string', () => {
expect(() => parsePythonCommand('')).toThrow('Python command cannot be empty');
});

it('should throw on whitespace-only string', () => {
expect(() => parsePythonCommand(' ')).toThrow('Python command cannot be empty');
});

it('should throw on empty quoted string', () => {
expect(() => parsePythonCommand('""')).toThrow('Python command cannot be empty');
});

it('should handle path with trailing spaces', () => {
const [cmd, args] = parsePythonCommand('/path/to/python ');
expect(cmd).toBe('/path/to/python');
expect(args).toEqual([]);
});

it('should handle path with leading spaces', () => {
const [cmd, args] = parsePythonCommand(' /path/to/python');
expect(cmd).toBe('/path/to/python');
expect(args).toEqual([]);
});
});
});

describe('validatePythonPath', () => {
describe('shell metacharacter rejection', () => {
it('should reject paths with command substitution $()', () => {
const result = validatePythonPath('/path/$(whoami)/python');
expect(result.valid).toBe(false);
expect(result.reason).toContain('dangerous shell metacharacters');
});

it('should reject paths with backticks', () => {
const result = validatePythonPath('/path/`id`/python');
expect(result.valid).toBe(false);
expect(result.reason).toContain('dangerous shell metacharacters');
});

it('should reject paths with semicolons', () => {
const result = validatePythonPath('/path/python; rm -rf /');
expect(result.valid).toBe(false);
expect(result.reason).toContain('dangerous shell metacharacters');
});

it('should reject paths with pipes', () => {
const result = validatePythonPath('/path/python | cat');
expect(result.valid).toBe(false);
expect(result.reason).toContain('dangerous shell metacharacters');
});

it('should reject paths with newlines', () => {
const result = validatePythonPath('/path/python\nrm -rf /');
expect(result.valid).toBe(false);
expect(result.reason).toContain('dangerous shell metacharacters');
});
});

describe('path traversal rejection', () => {
it('should reject paths with directory traversal', () => {
const result = validatePythonPath('/usr/bin/../../../etc/passwd');
expect(result.valid).toBe(false);
// Path traversal is caught by allowlist check (normalized path won't match patterns)
expect(result.reason).toContain('does not match allowed Python locations');
});
});

describe('empty/invalid input', () => {
it('should reject empty string', () => {
const result = validatePythonPath('');
expect(result.valid).toBe(false);
expect(result.reason).toContain('empty or invalid');
});

it('should reject null-like values', () => {
const result = validatePythonPath(null as unknown as string);
expect(result.valid).toBe(false);
});
});
});
60 changes: 48 additions & 12 deletions apps/frontend/src/main/ipc-handlers/terminal-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,13 @@ import { TerminalManager } from '../terminal-manager';
import { projectStore } from '../project-store';
import { terminalNameGenerator } from '../terminal-name-generator';
import { debugLog, debugError } from '../../shared/utils/debug-logger';
import { escapeShellArg, escapeShellArgWindows } from '../../shared/utils/shell-escape';
import {
escapeShellArg,
escapeShellArgWindows,
escapeShellArgPowerShell,
detectShellType,
type ShellType
} from '../../shared/utils/shell-escape';


/**
Expand Down Expand Up @@ -327,19 +333,49 @@ export function registerTerminalHandlers(
await new Promise(resolve => setTimeout(resolve, 500));

// Build the login command with the profile's config dir
// Use platform-specific syntax and escaping for environment variables
// Detect the shell type to use appropriate escaping and syntax
const shell = process.platform === 'win32'
? process.env.COMSPEC || 'cmd.exe'
: process.env.SHELL || '/bin/zsh';
const shellType: ShellType = detectShellType(shell);

debugLog('[IPC] Detected shell for login command:', { shell, shellType });

let loginCommand: string;
if (!profile.isDefault && profile.configDir) {
if (process.platform === 'win32') {
// SECURITY: Use Windows-specific escaping for cmd.exe
const escapedConfigDir = escapeShellArgWindows(profile.configDir);
// Windows cmd.exe syntax: set "VAR=value" with %VAR% for expansion
loginCommand = `set "CLAUDE_CONFIG_DIR=${escapedConfigDir}" && echo Config dir: %CLAUDE_CONFIG_DIR% && claude setup-token`;
} else {
// SECURITY: Use POSIX escaping for bash/zsh
const escapedConfigDir = escapeShellArg(profile.configDir);
// Unix/Mac bash/zsh syntax: export VAR=value with $VAR for expansion
loginCommand = `export CLAUDE_CONFIG_DIR=${escapedConfigDir} && echo "Config dir: $CLAUDE_CONFIG_DIR" && claude setup-token`;
// Build shell-appropriate command based on detected shell type
switch (shellType) {
case 'powershell': {
// SECURITY: Use PowerShell-specific escaping
const escapedConfigDir = escapeShellArgPowerShell(profile.configDir);
// PowerShell syntax: $env:VAR = 'value'; command
loginCommand = `$env:CLAUDE_CONFIG_DIR = ${escapedConfigDir}; Write-Host "Config dir: $env:CLAUDE_CONFIG_DIR"; claude setup-token`;
break;
}
case 'cmd': {
// SECURITY: Use Windows cmd.exe escaping
const escapedConfigDir = escapeShellArgWindows(profile.configDir);
// cmd.exe syntax: set "VAR=value" && command
loginCommand = `set "CLAUDE_CONFIG_DIR=${escapedConfigDir}" && echo Config dir: %CLAUDE_CONFIG_DIR% && claude setup-token`;
break;
}
case 'fish': {
// SECURITY: Use POSIX escaping (fish uses similar single-quote escaping)
const escapedConfigDir = escapeShellArg(profile.configDir);
// fish syntax: set -x VAR value; command
loginCommand = `set -x CLAUDE_CONFIG_DIR ${escapedConfigDir}; echo "Config dir: $CLAUDE_CONFIG_DIR"; and claude setup-token`;
break;
}
case 'bash':
case 'zsh':
case 'sh':
default: {
// SECURITY: Use POSIX escaping for bash/zsh/sh
const escapedConfigDir = escapeShellArg(profile.configDir);
// POSIX syntax: export VAR=value && command
loginCommand = `export CLAUDE_CONFIG_DIR=${escapedConfigDir} && echo "Config dir: $CLAUDE_CONFIG_DIR" && claude setup-token`;
break;
}
}
} else {
loginCommand = 'claude setup-token';
Expand Down
80 changes: 56 additions & 24 deletions apps/frontend/src/main/python-detector.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { execSync, execFileSync } from 'child_process';
import { execFileSync } from 'child_process';
import { existsSync, accessSync, constants } from 'fs';
import path from 'path';
import { app } from 'electron';
Expand Down Expand Up @@ -121,16 +121,19 @@ export function findPythonCommand(): string | null {

/**
* Extract Python version from a command.
* Uses execFileSync to safely handle paths with spaces and prevent shell injection.
*
* @param pythonCmd - The Python command to check (e.g., "python3", "py -3")
* @param pythonCmd - The Python command to check (e.g., "python3", "py -3", "/path/with spaces/python")
* @returns The version string (e.g., "3.10.5") or null if unable to detect
*/
function getPythonVersion(pythonCmd: string): string | null {
try {
const version = execSync(`${pythonCmd} --version`, {
const [cmd, args] = parsePythonCommand(pythonCmd);
const version = execFileSync(cmd, [...args, '--version'], {
stdio: 'pipe',
timeout: 5000,
windowsHide: true
windowsHide: true,
shell: false
}).toString().trim();

// Extract version number from "Python 3.10.5" format
Expand Down Expand Up @@ -208,6 +211,10 @@ export function getDefaultPythonCommand(): string {
* Parse a Python command string into command and base arguments.
* Handles space-separated commands like "py -3" and file paths with spaces.
*
* IMPORTANT: This function must correctly handle paths with spaces (e.g., macOS
* Application Support paths). It should NEVER split a path at spaces if it looks
* like a file path rather than a command with arguments.
*
* @param pythonPath - The Python command string (e.g., "python3", "py -3", "/path/with spaces/python")
* @returns Tuple of [command, baseArgs] ready for use with spawn()
* @throws Error if pythonPath is empty or only whitespace
Expand All @@ -230,23 +237,36 @@ export function parsePythonCommand(pythonPath: string): [string, string[]] {
}
}

// If the path points to an actual file, use it directly (handles paths with spaces)
if (existsSync(cleanPath)) {
return [cleanPath, []];
}

// Check if it's a path (contains path separators but not just at the start)
// Paths with spaces should be treated as a single command, not split
// Check if it's a path (contains path separators)
// This MUST be checked FIRST because paths may contain spaces that we should NOT split on
// Examples of paths with spaces:
// - /Users/user/Library/Application Support/AppName/python-venv/bin/python
// - C:\Users\user\Program Files\Python\python.exe
const hasPathSeparators = cleanPath.includes('/') || cleanPath.includes('\\');

// A path is any string with path separators that doesn't start with a dash
// We check path separators BEFORE checking file existence to handle:
// 1. Paths that exist
// 2. Paths that don't exist yet (during setup)
// 3. Paths that might not be accessible
const isLikelyPath = hasPathSeparators && !cleanPath.startsWith('-');

if (isLikelyPath) {
// This looks like a file path, don't split it
// Even if the file doesn't exist (yet), treat the whole thing as the command
// This looks like a file path - NEVER split it on spaces
// Return as-is even if the file doesn't exist yet
return [cleanPath, []];
}

// If the path points to an actual file (without path separators, e.g., in PATH)
if (existsSync(cleanPath)) {
return [cleanPath, []];
}

// Otherwise, split on spaces for commands like "py -3"
// Only split on spaces for simple commands like "py -3"
// At this point we know it's NOT a path (no path separators) and NOT an existing file
// EDGE CASE: If someone has a file literally named "py -3" (with a space), the existsSync
// check above will catch it and return it as a single command. Only if no such file exists
// do we treat it as "py" command with "-3" argument.
const parts = cleanPath.split(' ').filter(p => p.length > 0);
if (parts.length === 0) {
// This shouldn't happen after earlier validation, but guard anyway
Expand Down Expand Up @@ -283,12 +303,18 @@ const ALLOWED_PATH_PATTERNS: RegExp[] = [
// Homebrew Python (macOS)
/^\/opt\/homebrew\/bin\/python\d*(\.\d+)?$/,
/^\/opt\/homebrew\/opt\/python@[\d.]+\/bin\/python\d*(\.\d+)?$/,
// pyenv
/^.*\/\.pyenv\/versions\/[\d.]+\/bin\/python\d*(\.\d+)?$/,
// pyenv - only allow paths under user home directories (stricter than .*)
/^\/(?:Users|home)\/[^/]+\/\.pyenv\/versions\/[\d.]+\/bin\/python\d*(\.\d+)?$/,
// pyenv for root user on Linux (no username directory)
/^\/root\/\.pyenv\/versions\/[\d.]+\/bin\/python\d*(\.\d+)?$/,
// Virtual environments (various naming conventions)
/^.*\/\.?venv\/bin\/python\d*(\.\d+)?$/,
/^.*\/\.?virtualenv\/bin\/python\d*(\.\d+)?$/,
/^.*\/env\/bin\/python\d*(\.\d+)?$/,
// Only allow alphanumeric, dash, underscore, dot, and space in directory names
/^(?:\/[a-zA-Z0-9._\- ]+)+\/\.?venv\/bin\/python\d*(\.\d+)?$/,
/^(?:\/[a-zA-Z0-9._\- ]+)+\/\.?virtualenv\/bin\/python\d*(\.\d+)?$/,
/^(?:\/[a-zA-Z0-9._\- ]+)+\/env\/bin\/python\d*(\.\d+)?$/,
// macOS Application Support paths (Electron userData with spaces)
// Matches: /Users/<user>/Library/Application Support/<app>/(python-venv|.venv|venv)/bin/python
/^\/Users\/[^/]+\/Library\/Application Support\/[^/]+\/(python-venv|\.?venv)\/bin\/python\d*(\.\d+)?$/,
// Windows virtual environments
/^.*\\\.?venv\\Scripts\\python\.exe$/i,
/^.*\\\.?virtualenv\\Scripts\\python\.exe$/i,
Expand All @@ -298,11 +324,17 @@ const ALLOWED_PATH_PATTERNS: RegExp[] = [
/^[A-Za-z]:\\Program Files\\Python\d+\\python\.exe$/i,
/^[A-Za-z]:\\Program Files \(x86\)\\Python\d+\\python\.exe$/i,
/^[A-Za-z]:\\Users\\[^\\]+\\AppData\\Local\\Programs\\Python\\Python\d+\\python\.exe$/i,
// Conda environments
/^.*\/anaconda\d*\/bin\/python\d*(\.\d+)?$/,
/^.*\/miniconda\d*\/bin\/python\d*(\.\d+)?$/,
/^.*\/anaconda\d*\/envs\/[^/]+\/bin\/python\d*(\.\d+)?$/,
/^.*\/miniconda\d*\/envs\/[^/]+\/bin\/python\d*(\.\d+)?$/,
// Windows Application Data paths (Electron userData)
// Matches: C:\Users\<user>\AppData\Roaming\<app>\(python-venv|.venv|venv)\Scripts\python.exe
/^[A-Za-z]:\\Users\\[^\\]+\\AppData\\Roaming\\[^\\]+\\(python-venv|\.?venv)\\Scripts\\python\.exe$/i,
// Conda environments - only allow paths under user home directories
/^\/(?:Users|home)\/[^/]+\/anaconda\d*\/bin\/python\d*(\.\d+)?$/,
/^\/(?:Users|home)\/[^/]+\/miniconda\d*\/bin\/python\d*(\.\d+)?$/,
/^\/(?:Users|home)\/[^/]+\/anaconda\d*\/envs\/[^/]+\/bin\/python\d*(\.\d+)?$/,
/^\/(?:Users|home)\/[^/]+\/miniconda\d*\/envs\/[^/]+\/bin\/python\d*(\.\d+)?$/,
// System-wide conda installations
/^\/opt\/(?:anaconda|miniconda)\d*\/bin\/python\d*(\.\d+)?$/,
/^\/opt\/(?:anaconda|miniconda)\d*\/envs\/[^/]+\/bin\/python\d*(\.\d+)?$/,
];

/**
Expand Down
11 changes: 7 additions & 4 deletions apps/frontend/src/main/python-env-manager.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { spawn, execSync, ChildProcess } from 'child_process';
import { spawn, execSync, execFileSync, ChildProcess } from 'child_process';
import { existsSync, readdirSync } from 'fs';
import path from 'path';
import { EventEmitter } from 'events';
import { app } from 'electron';
import { findPythonCommand, getBundledPythonPath } from './python-detector';
import { findPythonCommand, getBundledPythonPath, parsePythonCommand } from './python-detector';

export interface PythonEnvStatus {
ready: boolean;
Expand Down Expand Up @@ -201,9 +201,12 @@ if sys.version_info >= (3, 12):
try {
// Get the actual executable path from the command
// For commands like "py -3", we need to resolve to the actual executable
const pythonPath = execSync(`${pythonCmd} -c "import sys; print(sys.executable)"`, {
// Use execFileSync to avoid shell interpretation issues with paths containing spaces
const [cmd, cmdArgs] = parsePythonCommand(pythonCmd);
const pythonPath = execFileSync(cmd, [...cmdArgs, '-c', 'import sys; print(sys.executable)'], {
stdio: 'pipe',
timeout: 5000
timeout: 5000,
shell: false
}).toString().trim();

console.log(`[PythonEnvManager] Found Python at: ${pythonPath}`);
Expand Down
Loading
Loading