fix: address PR review security and validation issues

- Add runtime type check for worktreePath in open-in-terminal handler
- Fix Windows Terminal detection using commandExists before spawn
- Fix xterm shell injection by using sh -c with escapeShellArg
- Use loose equality for null/undefined in useEffectiveDefaultTerminal
- Consolidate duplicate imports from open-in-terminal.js
This commit is contained in:
Stefan de Vogelaere
2026-01-18 15:38:54 +01:00
parent 9529afbbaa
commit 5d68e75541
5 changed files with 39 additions and 30 deletions

View File

@@ -29,8 +29,8 @@ import {
createGetAvailableEditorsHandler, createGetAvailableEditorsHandler,
createRefreshEditorsHandler, createRefreshEditorsHandler,
} from './routes/open-in-editor.js'; } from './routes/open-in-editor.js';
import { createOpenInTerminalHandler } from './routes/open-in-terminal.js';
import { import {
createOpenInTerminalHandler,
createGetAvailableTerminalsHandler, createGetAvailableTerminalsHandler,
createGetDefaultTerminalHandler, createGetDefaultTerminalHandler,
createRefreshTerminalsHandler, createRefreshTerminalsHandler,

View File

@@ -32,10 +32,10 @@ export function createOpenInTerminalHandler() {
worktreePath: string; worktreePath: string;
}; };
if (!worktreePath) { if (!worktreePath || typeof worktreePath !== 'string') {
res.status(400).json({ res.status(400).json({
success: false, success: false,
error: 'worktreePath required', error: 'worktreePath required and must be a string',
}); });
return; return;
} }

View File

@@ -82,8 +82,8 @@ export function useEffectiveDefaultTerminal(terminals: TerminalInfo[]): Terminal
const defaultTerminalId = useAppStore((s) => s.defaultTerminalId); const defaultTerminalId = useAppStore((s) => s.defaultTerminalId);
return useMemo(() => { return useMemo(() => {
// If user hasn't set a preference (null), they prefer integrated terminal // If user hasn't set a preference (null/undefined), they prefer integrated terminal
if (defaultTerminalId === null) { if (defaultTerminalId == null) {
return null; return null;
} }

View File

@@ -374,8 +374,9 @@ export async function openInTerminal(targetPath: string): Promise<{ terminalName
await execFileAsync('osascript', ['-e', script]); await execFileAsync('osascript', ['-e', script]);
return { terminalName: 'Terminal' }; return { terminalName: 'Terminal' };
} else if (isWindows) { } else if (isWindows) {
// Try Windows Terminal first // Try Windows Terminal first - check if it exists before trying to spawn
try { const hasWindowsTerminal = await commandExists('wt');
if (hasWindowsTerminal) {
return await new Promise((resolve, reject) => { return await new Promise((resolve, reject) => {
const child: ChildProcess = spawn('wt', ['-d', targetPath], { const child: ChildProcess = spawn('wt', ['-d', targetPath], {
shell: true, shell: true,
@@ -384,13 +385,13 @@ export async function openInTerminal(targetPath: string): Promise<{ terminalName
}); });
child.unref(); child.unref();
child.on('error', () => { child.on('error', (err) => {
reject(new Error('Windows Terminal not available')); reject(err);
}); });
setTimeout(() => resolve({ terminalName: 'Windows Terminal' }), 100); setTimeout(() => resolve({ terminalName: 'Windows Terminal' }), 100);
}); });
} catch { }
// Fall back to cmd // Fall back to cmd
return await new Promise((resolve, reject) => { return await new Promise((resolve, reject) => {
const child: ChildProcess = spawn( const child: ChildProcess = spawn(
@@ -410,7 +411,6 @@ export async function openInTerminal(targetPath: string): Promise<{ terminalName
setTimeout(() => resolve({ terminalName: 'Command Prompt' }), 100); setTimeout(() => resolve({ terminalName: 'Command Prompt' }), 100);
}); });
}
} else { } else {
// Linux: Try common terminal emulators in order // Linux: Try common terminal emulators in order
const terminals = [ const terminals = [
@@ -425,7 +425,11 @@ export async function openInTerminal(targetPath: string): Promise<{ terminalName
command: 'xfce4-terminal', command: 'xfce4-terminal',
args: ['--working-directory', targetPath], args: ['--working-directory', targetPath],
}, },
{ name: 'xterm', command: 'xterm', args: ['-e', `cd "${targetPath}" && $SHELL`] }, {
name: 'xterm',
command: 'xterm',
args: ['-e', 'sh', '-c', `cd ${escapeShellArg(targetPath)} && $SHELL`],
},
{ {
name: 'x-terminal-emulator', name: 'x-terminal-emulator',
command: 'x-terminal-emulator', command: 'x-terminal-emulator',

View File

@@ -556,7 +556,12 @@ async function executeTerminalCommand(terminal: TerminalInfo, targetPath: string
case 'xterm': case 'xterm':
// XTerm: uses -e to run a shell in the directory // XTerm: uses -e to run a shell in the directory
await spawnDetached(command, ['-e', `cd "${targetPath}" && $SHELL`]); await spawnDetached(command, [
'-e',
'sh',
'-c',
`cd ${escapeShellArg(targetPath)} && $SHELL`,
]);
break; break;
default: default: