Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
5fcb13c
feat(#1119): Add Launch App button for Human Review tasks
youngmrz Jan 15, 2026
d304fb4
fix(types): add worktreeLaunchApp to ElectronAPI interface
youngmrz Jan 15, 2026
cfaafc9
fix(mock): add worktreeLaunchApp to workspace mock
youngmrz Jan 15, 2026
3b97476
fix(terminal): verify terminal exists before spawn on Linux
youngmrz Jan 15, 2026
c50eeed
fix(security): prevent command injection in worktreeLaunchApp
youngmrz Jan 15, 2026
6a166d8
fix: address automated review issues in launch-app handler
youngmrz Jan 15, 2026
447bf92
test: add comprehensive tests for escapePathForShell security function
youngmrz Jan 15, 2026
55624ea
fix(launch-app): address PR #1120 review issues
youngmrz Jan 16, 2026
3a5053c
fix(types): resolve TypeScript errors in worktree-handlers and Worksp…
youngmrz Jan 16, 2026
6483660
fix(security): improve shell escaping for terminal launch commands
youngmrz Jan 16, 2026
cb75b8b
fix(terminal): use --working-directory for xfce4-terminal path handling
youngmrz Jan 17, 2026
e97a8f7
fix(terminal): address PR #1120 bot review comments
youngmrz Jan 17, 2026
bbf87c6
fix: address remaining bot review feedback
youngmrz Jan 17, 2026
918be7b
fix(launch-app): fix Windows command quoting for Launch App button
youngmrz Jan 18, 2026
84640a3
Merge branch 'feat/launch-app-button' into feat/launch-app-auto-install
youngmrz Jan 18, 2026
8c888f2
feat(launch-app): Add auto-install dependencies with UI feedback
youngmrz Jan 18, 2026
7b27c8d
fix: Address bot feedback on launch app dependency handling
youngmrz Jan 18, 2026
04edc76
fix(launch-app): Improve process cleanup and dependency detection
youngmrz Jan 18, 2026
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
270 changes: 270 additions & 0 deletions apps/frontend/src/main/ipc-handlers/task/shell-escape.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,270 @@
/**
* Unit tests for shell-escape utilities.
* Tests command injection prevention via path escaping.
*/

import { describe, it, expect } from 'vitest';
import { escapePathForShell, escapePathForAppleScript } from './shell-escape';

describe('escapePathForShell', () => {
describe('null byte injection prevention', () => {
it('should reject paths containing null bytes', () => {
expect(escapePathForShell('/path/with\0null', 'linux')).toBeNull();
expect(escapePathForShell('C:\\path\\with\0null', 'win32')).toBeNull();
});

it('should reject null byte at start of path', () => {
expect(escapePathForShell('\0/path', 'darwin')).toBeNull();
});

it('should reject null byte at end of path', () => {
expect(escapePathForShell('/path\0', 'linux')).toBeNull();
});
});

describe('newline injection prevention', () => {
it('should reject paths containing LF newlines', () => {
expect(escapePathForShell('/path/with\nnewline', 'linux')).toBeNull();
expect(escapePathForShell('C:\\path\\with\nnewline', 'win32')).toBeNull();
});

it('should reject paths containing CR newlines', () => {
expect(escapePathForShell('/path/with\rnewline', 'darwin')).toBeNull();
expect(escapePathForShell('C:\\path\\with\rnewline', 'win32')).toBeNull();
});

it('should reject paths containing CRLF', () => {
expect(escapePathForShell('/path/with\r\nnewline', 'linux')).toBeNull();
});
});

describe('Windows platform (win32)', () => {
it('should reject paths with < character', () => {
expect(escapePathForShell('C:\\path<file', 'win32')).toBeNull();
});

it('should reject paths with > character', () => {
expect(escapePathForShell('C:\\path>file', 'win32')).toBeNull();
});

it('should reject paths with | pipe character', () => {
expect(escapePathForShell('C:\\path|command', 'win32')).toBeNull();
});

it('should reject paths with & ampersand', () => {
expect(escapePathForShell('C:\\path&command', 'win32')).toBeNull();
});

it('should reject paths with ^ caret', () => {
expect(escapePathForShell('C:\\path^file', 'win32')).toBeNull();
});

it('should reject paths with % percent', () => {
expect(escapePathForShell('C:\\path%VAR%', 'win32')).toBeNull();
});

it('should reject paths with ! exclamation', () => {
expect(escapePathForShell('C:\\path!file', 'win32')).toBeNull();
});

it('should reject paths with ` backtick', () => {
expect(escapePathForShell('C:\\path`command`', 'win32')).toBeNull();
});

it('should escape double quotes with double-double quotes', () => {
expect(escapePathForShell('C:\\path\\"file"', 'win32')).toBe('C:\\path\\""file""');
});

it('should pass through safe Windows paths unchanged', () => {
expect(escapePathForShell('C:\\Users\\name\\project', 'win32')).toBe('C:\\Users\\name\\project');
});

it('should allow paths with spaces', () => {
expect(escapePathForShell('C:\\Program Files\\app', 'win32')).toBe('C:\\Program Files\\app');
});

it('should allow paths with parentheses', () => {
expect(escapePathForShell('C:\\Program Files (x86)\\app', 'win32')).toBe('C:\\Program Files (x86)\\app');
});
});

describe('Unix platforms (linux, darwin)', () => {
it('should escape single quotes with quote-escape-quote pattern', () => {
const result = escapePathForShell("/path/with'quote", 'linux');
expect(result).toBe("/path/with'\\''quote");
});

it('should handle multiple single quotes', () => {
const result = escapePathForShell("it's a 'test'", 'darwin');
expect(result).toBe("it'\\''s a '\\''test'\\''");
});

it('should pass through safe Unix paths unchanged', () => {
expect(escapePathForShell('/usr/local/bin', 'linux')).toBe('/usr/local/bin');
expect(escapePathForShell('/Users/name/project', 'darwin')).toBe('/Users/name/project');
});

it('should allow paths with spaces', () => {
expect(escapePathForShell('/path/with spaces/file', 'linux')).toBe('/path/with spaces/file');
});

it('should allow paths with special characters (not injection vectors)', () => {
// These are safe when single-quoted in bash
expect(escapePathForShell('/path/$var', 'linux')).toBe('/path/$var');
expect(escapePathForShell('/path/`cmd`', 'darwin')).toBe('/path/`cmd`');
expect(escapePathForShell('/path/!history', 'linux')).toBe('/path/!history');
});

it('should allow paths with glob characters', () => {
// Safe when single-quoted
expect(escapePathForShell('/path/*.txt', 'linux')).toBe('/path/*.txt');
expect(escapePathForShell('/path/[abc]', 'darwin')).toBe('/path/[abc]');
});
});

describe('edge cases', () => {
it('should handle empty string', () => {
expect(escapePathForShell('', 'linux')).toBe('');
expect(escapePathForShell('', 'win32')).toBe('');
});

it('should handle path with only quotes', () => {
expect(escapePathForShell("'''", 'linux')).toBe("'\\'''\\'''\\''");
expect(escapePathForShell('"""', 'win32')).toBe('""""""');
});

it('should handle very long paths', () => {
const longPath = '/a'.repeat(1000);
expect(escapePathForShell(longPath, 'linux')).toBe(longPath);
});

it('should handle Unicode characters', () => {
expect(escapePathForShell('/path/日本語/файл', 'linux')).toBe('/path/日本語/файл');
expect(escapePathForShell('C:\\путь\\文件', 'win32')).toBe('C:\\путь\\文件');
});

it('should handle emoji in paths', () => {
expect(escapePathForShell('/path/📁/file', 'darwin')).toBe('/path/📁/file');
});
});

describe('command injection attack patterns', () => {
it('should block semicolon command chaining on Windows via newline', () => {
// Attacker might try: path; malicious_command
// But semicolons are actually safe on Windows with proper quoting
// The danger comes from newlines which we block
expect(escapePathForShell('path\n; malicious', 'win32')).toBeNull();
});

it('should block pipe injection on Windows', () => {
expect(escapePathForShell('path | evil-command', 'win32')).toBeNull();
});

it('should block command substitution on Windows', () => {
expect(escapePathForShell('path & cmd /c evil', 'win32')).toBeNull();
});

it('should escape quote escapes on Unix', () => {
// Attacker might try to break out with: ' ; evil '
const result = escapePathForShell("' ; evil '", 'linux');
expect(result).toBe("'\\'' ; evil '\\''");
});

it('should block null byte truncation attacks', () => {
// Attacker might try: /safe/path\0/../../etc/passwd
expect(escapePathForShell('/safe/path\0/../../etc/passwd', 'linux')).toBeNull();
});
});
});

describe('escapePathForAppleScript', () => {
describe('null byte injection prevention', () => {
it('should reject paths containing null bytes', () => {
expect(escapePathForAppleScript('/path/with\0null')).toBeNull();
});

it('should reject null byte at start of path', () => {
expect(escapePathForAppleScript('\0/path')).toBeNull();
});

it('should reject null byte at end of path', () => {
expect(escapePathForAppleScript('/path\0')).toBeNull();
});
});

describe('newline injection prevention', () => {
it('should reject paths containing LF newlines', () => {
expect(escapePathForAppleScript('/path/with\nnewline')).toBeNull();
});

it('should reject paths containing CR newlines', () => {
expect(escapePathForAppleScript('/path/with\rnewline')).toBeNull();
});

it('should reject paths containing CRLF', () => {
expect(escapePathForAppleScript('/path/with\r\nnewline')).toBeNull();
});
});

describe('AppleScript escaping', () => {
it('should escape backslashes', () => {
expect(escapePathForAppleScript('/path\\with\\backslashes')).toBe('/path\\\\with\\\\backslashes');
});

it('should escape double quotes', () => {
expect(escapePathForAppleScript('/path/with"quotes"')).toBe('/path/with\\"quotes\\"');
});

it('should escape both backslashes and double quotes', () => {
expect(escapePathForAppleScript('/path\\"complex"')).toBe('/path\\\\\\"complex\\"');
});

it('should pass through safe paths unchanged', () => {
expect(escapePathForAppleScript('/Users/name/project')).toBe('/Users/name/project');
});

it('should allow paths with spaces', () => {
expect(escapePathForAppleScript('/path/with spaces/file')).toBe('/path/with spaces/file');
});

it('should allow single quotes (safe in AppleScript double-quoted strings)', () => {
expect(escapePathForAppleScript("/path/with'quote")).toBe("/path/with'quote");
});
});

describe('edge cases', () => {
it('should handle empty string', () => {
expect(escapePathForAppleScript('')).toBe('');
});

it('should handle path with only double quotes', () => {
expect(escapePathForAppleScript('"""')).toBe('\\"\\"\\"');
});

it('should handle path with only backslashes', () => {
expect(escapePathForAppleScript('\\\\\\')).toBe('\\\\\\\\\\\\');
});

it('should handle Unicode characters', () => {
expect(escapePathForAppleScript('/path/日本語/файл')).toBe('/path/日本語/файл');
});

it('should handle emoji in paths', () => {
expect(escapePathForAppleScript('/path/📁/file')).toBe('/path/📁/file');
});
});

describe('command injection prevention', () => {
it('should escape double quotes used in injection attempts', () => {
// Attacker might try to break out of the AppleScript string
const result = escapePathForAppleScript('/path" & do shell script "evil"');
expect(result).toBe('/path\\" & do shell script \\"evil\\"');
});

it('should escape backslash escape attempts', () => {
// Attacker might try to use backslash to escape the escaping
const result = escapePathForAppleScript('/path\\"');
expect(result).toBe('/path\\\\\\"');
});
});
});
68 changes: 68 additions & 0 deletions apps/frontend/src/main/ipc-handlers/task/shell-escape.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
/**
* Shell path escaping utilities for secure command execution.
*
* These functions prevent command injection by properly escaping
* or rejecting dangerous characters in file paths.
*/

/**
* Escape a path for use in shell commands.
* Prevents command injection by escaping or rejecting dangerous characters.
*
* @param filePath - The path to escape
* @param platform - Target platform ('win32', 'darwin', 'linux')
* @returns Escaped path string safe for shell use, or null if path is invalid
*/
export function escapePathForShell(filePath: string, platform: NodeJS.Platform): string | null {
// Reject paths with null bytes (always dangerous)
if (filePath.includes('\0')) {
return null;
}

// Reject paths with newlines (can break command structure)
if (filePath.includes('\n') || filePath.includes('\r')) {
return null;
}

if (platform === 'win32') {
// Windows: Reject paths with characters that could escape cmd.exe quoting
// These characters can break out of double-quoted strings in cmd
const dangerousWinChars = /[<>|&^%!`]/;
if (dangerousWinChars.test(filePath)) {
return null;
}
// Double-quote the path (already done in caller, but escape any internal quotes)
return filePath.replace(/"/g, '""');
} else {
// Unix (macOS/Linux): Use single quotes and escape any internal single quotes
// Single-quoted strings in bash treat everything literally except single quotes
// Escape ' as '\'' (end quote, escaped quote, start quote)
return filePath.replace(/'/g, "'\\''");
}
}

/**
* Escape a path for use in AppleScript double-quoted strings.
* AppleScript uses different escaping rules than POSIX shells.
*
* @param filePath - The path to escape (should already be validated)
* @returns Escaped path string safe for AppleScript double-quoted context
*/
export function escapePathForAppleScript(filePath: string): string | null {
// Reject paths with null bytes (always dangerous)
if (filePath.includes('\0')) {
return null;
}

// Reject paths with newlines (can break command structure)
if (filePath.includes('\n') || filePath.includes('\r')) {
return null;
}

// AppleScript double-quoted strings require escaping:
// 1. Backslashes must be escaped as \\
// 2. Double quotes must be escaped as \"
return filePath
.replace(/\\/g, '\\\\') // Escape backslashes first
.replace(/"/g, '\\"'); // Then escape double quotes
}
Loading