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
4 changes: 2 additions & 2 deletions apps/frontend/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

84 changes: 84 additions & 0 deletions apps/frontend/src/main/ipc-handlers/settings-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -299,4 +299,88 @@ export function registerSettingsHandlers(
await shell.openExternal(url);
}
);

ipcMain.handle(
IPC_CHANNELS.SHELL_OPEN_PATH,
async (_, folderPath: string): Promise<void> => {
await shell.openPath(folderPath);
}
);
Comment on lines +303 to +308
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 | 🟠 Major

Add error handling and path validation.

The SHELL_OPEN_PATH handler lacks error handling and path validation, making it inconsistent with other IPC handlers and potentially unsafe.

🔎 Proposed fix
  ipcMain.handle(
    IPC_CHANNELS.SHELL_OPEN_PATH,
-    async (_, folderPath: string): Promise<void> => {
-      await shell.openPath(folderPath);
+    async (_, folderPath: string): Promise<IPCResult> => {
+      try {
+        // Validate path exists
+        if (!existsSync(folderPath)) {
+          return { success: false, error: `Path does not exist: ${folderPath}` };
+        }
+
+        const result = await shell.openPath(folderPath);
+        
+        // shell.openPath returns a string with an error message if it fails
+        if (result) {
+          return { success: false, error: result };
+        }
+        
+        return { success: true };
+      } catch (error) {
+        return {
+          success: false,
+          error: error instanceof Error ? error.message : 'Failed to open path'
+        };
+      }
    }
  );

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/settings-handlers.ts around lines
303–308, the SHELL_OPEN_PATH handler currently calls shell.openPath without
validating the folderPath or handling errors; update it to first validate the
input (ensure folderPath is a non-empty string and a valid absolute/path that
exists via fs.stat or fs.existsSync), then wrap the shell.openPath call in a
try/catch, await its result and treat a non-empty result string as a failure
(throw or return an error), and log errors (or return a standardized IPC error
response) so the renderer gets consistent success/failure behavior.


ipcMain.handle(
IPC_CHANNELS.SHELL_OPEN_IN_IDE,
async (_, folderPath: string, ide?: 'cursor' | 'vscode' | 'finder'): Promise<IPCResult> => {
try {
// Validate path exists
if (!existsSync(folderPath)) {
return { success: false, error: `Path does not exist: ${folderPath}` };
}

// Determine which IDE to use
const ideToUse = ide || 'cursor'; // Default to Cursor

if (ideToUse === 'finder') {
// Open in Finder (macOS) or Explorer (Windows)
await shell.openPath(folderPath);
return { success: true };
}

// Try to open in the specified IDE
const commands: Record<string, string[]> = {
cursor: ['cursor', '.'],
vscode: ['code', '.']
};

const [cmd, ...args] = commands[ideToUse] || commands.cursor;

try {
// Use execSync to run the command - this opens the IDE
execSync(`${cmd} ${args.join(' ')}`, {
cwd: folderPath,
stdio: 'ignore',
timeout: 5000
});
return { success: true };
Comment on lines +336 to +343
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

CRITICAL: Command injection vulnerability.

Using string interpolation with execSync creates a command injection vulnerability. If folderPath contains special shell characters (e.g., ; rm -rf /), it could execute arbitrary commands.

🔎 Proposed fix

Use an array-based approach or properly escape arguments:

        try {
-          // Use execSync to run the command - this opens the IDE
-          execSync(`${cmd} ${args.join(' ')}`, {
+          // Use spawn for safer command execution
+          const { spawnSync } = require('child_process');
+          const result = spawnSync(cmd, [...args, folderPath], {
-            cwd: folderPath,
             stdio: 'ignore',
             timeout: 5000
           });
+          
+          if (result.error) {
+            throw result.error;
+          }
+          
           return { success: true };

Alternatively, if you must use execSync, properly escape the path:

+          const { execSync } = require('child_process');
+          // Escape the folder path to prevent command injection
+          const escapedPath = folderPath.replace(/"/g, '\\"');
+          
-          execSync(`${cmd} ${args.join(' ')}`, {
-            cwd: folderPath,
+          execSync(`${cmd} "${escapedPath}"`, {
             stdio: 'ignore',
             timeout: 5000
           });

But spawnSync with an array of arguments is the safer approach.

Committable suggestion skipped: line range outside the PR's diff.

} catch (execError) {
// If the command fails, try alternative approaches
console.warn(`[SHELL_OPEN_IN_IDE] ${cmd} command failed:`, execError);

// On macOS, try using 'open' command with the app
if (process.platform === 'darwin') {
const appNames: Record<string, string[]> = {
cursor: ['Cursor', 'Cursor.app'],
vscode: ['Visual Studio Code', 'Visual Studio Code.app', 'VSCode', 'Code']
};

const apps = appNames[ideToUse] || appNames.cursor;
for (const appName of apps) {
try {
execSync(`open -a "${appName}" "${folderPath}"`, {
stdio: 'ignore',
timeout: 5000
});
return { success: true };
} catch {
// Try next app name
continue;
}
}
Comment on lines +356 to +367
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

CRITICAL: Command injection vulnerability in macOS fallback.

The same command injection vulnerability exists in the macOS fallback path. The folderPath variable is interpolated directly into the shell command without escaping.

🔎 Proposed fix
            const apps = appNames[ideToUse] || appNames.cursor;
            for (const appName of apps) {
              try {
-                execSync(`open -a "${appName}" "${folderPath}"`, {
+                const { spawnSync } = require('child_process');
+                const result = spawnSync('open', ['-a', appName, folderPath], {
                  stdio: 'ignore',
                  timeout: 5000
                });
+                
+                if (result.error) {
+                  continue;
+                }
+                
                 return { success: true };
               } catch {
                 // Try next app name
                 continue;
               }
             }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In apps/frontend/src/main/ipc-handlers/settings-handlers.ts around lines 356 to
367, the macOS fallback uses execSync with a shell-interpolated command
containing folderPath, which creates a command-injection risk; replace the
execSync shell call with a non-shell child process API (e.g. execFileSync or
spawnSync) that passes the executable and each argument separately (['-a',
appName, folderPath']) so folderPath is never interpreted by a shell, and
optionally validate/normalize appName against the known apps list before
invoking.

}

// Fallback: open in Finder/Explorer
console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
await shell.openPath(folderPath);
return {
success: true,
error: `${ideToUse} not found. Opened in file browser instead.`
};
}
Comment on lines +370 to +377
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Clarify the success/error return pattern.

Returning success: true with an error message is confusing. Consider either returning success: false or using a different field like warning to indicate the fallback behavior.

🔎 Proposed refactor
          // Fallback: open in Finder/Explorer
          console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
          await shell.openPath(folderPath);
          return {
            success: true,
-            error: `${ideToUse} not found. Opened in file browser instead.`
+            data: { 
+              warning: `${ideToUse} not found. Opened in file browser instead.` 
+            }
          };

Or if you want to treat it as a partial failure:

          // Fallback: open in Finder/Explorer
          console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
          await shell.openPath(folderPath);
          return {
-            success: true,
+            success: false,
             error: `${ideToUse} not found. Opened in file browser instead.`
          };
📝 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
// Fallback: open in Finder/Explorer
console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
await shell.openPath(folderPath);
return {
success: true,
error: `${ideToUse} not found. Opened in file browser instead.`
};
}
// Fallback: open in Finder/Explorer
console.warn(`[SHELL_OPEN_IN_IDE] Falling back to system file browser`);
await shell.openPath(folderPath);
return {
success: false,
error: `${ideToUse} not found. Opened in file browser instead.`
};

} catch (error) {
return {
success: false,
error: error instanceof Error ? error.message : 'Failed to open in IDE'
};
}
}
);
}
13 changes: 12 additions & 1 deletion apps/frontend/src/preload/api/modules/shell-api.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,28 @@
import { IPC_CHANNELS } from '../../../shared/constants';
import { invokeIpc } from './ipc-utils';

/**
* IDE types supported for opening folders
*/
export type IDEType = 'cursor' | 'vscode' | 'finder';

/**
* Shell Operations API
*/
export interface ShellAPI {
openExternal: (url: string) => Promise<void>;
openPath: (path: string) => Promise<void>;
openInIde: (path: string, ide?: IDEType) => Promise<{ success: boolean; error?: string }>;
}

/**
* Creates the Shell Operations API implementation
*/
export const createShellAPI = (): ShellAPI => ({
openExternal: (url: string): Promise<void> =>
invokeIpc(IPC_CHANNELS.SHELL_OPEN_EXTERNAL, url)
invokeIpc(IPC_CHANNELS.SHELL_OPEN_EXTERNAL, url),
openPath: (path: string): Promise<void> =>
invokeIpc(IPC_CHANNELS.SHELL_OPEN_PATH, path),
openInIde: (path: string, ide?: IDEType): Promise<{ success: boolean; error?: string }> =>
invokeIpc(IPC_CHANNELS.SHELL_OPEN_IN_IDE, path, ide)
});
40 changes: 35 additions & 5 deletions apps/frontend/src/renderer/components/Worktrees.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,9 @@ import {
Minus,
ChevronRight,
Check,
X
X,
Code2,
ExternalLink
} from 'lucide-react';
import { Button } from './ui/button';
import { Badge } from './ui/badge';
Expand Down Expand Up @@ -305,13 +307,41 @@ export function Worktrees({ projectId }: WorktreesProps) {
<Button
variant="outline"
size="sm"
onClick={() => {
// Copy worktree path to clipboard
navigator.clipboard.writeText(worktree.path);
onClick={async () => {
// Open in Cursor (default IDE)
const result = await window.electronAPI.openInIde(worktree.path, 'cursor');
if (!result.success) {
setError(result.error || 'Failed to open in Cursor');
}
}}
>
<Code2 className="h-3.5 w-3.5 mr-1.5" />
Open in Cursor
</Button>
<Button
variant="outline"
size="sm"
onClick={async () => {
// Open in VS Code
const result = await window.electronAPI.openInIde(worktree.path, 'vscode');
if (!result.success) {
setError(result.error || 'Failed to open in VS Code');
}
}}
>
<ExternalLink className="h-3.5 w-3.5 mr-1.5" />
VS Code
</Button>
<Button
variant="outline"
size="sm"
onClick={async () => {
// Open in Finder
await window.electronAPI.openInIde(worktree.path, 'finder');
}}
>
<FolderOpen className="h-3.5 w-3.5 mr-1.5" />
Copy Path
Finder
</Button>
Comment on lines +310 to 345
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

Replace hardcoded strings with translation keys.

All user-facing button text must use translation keys as per coding guidelines. Additionally, the Finder button lacks error handling that the other IDE buttons have.

🔎 Proposed fixes

First, ensure you have translation keys defined in apps/frontend/src/shared/i18n/locales/. For example, in your English locale file:

{
  "worktrees": {
    "actions": {
      "openInCursor": "Open in Cursor",
      "openInVSCode": "VS Code",
      "openInFinder": "Finder",
      "failedToOpen": "Failed to open in {{ide}}"
    }
  }
}

Then update the component:

+import { useTranslation } from 'react-i18next';
+
 export function Worktrees({ projectId }: WorktreesProps) {
+  const { t } = useTranslation();
   const projects = useProjectStore((state) => state.projects);
   // ... rest of component

   // In the JSX:
   <Button
     variant="outline"
     size="sm"
     onClick={async () => {
-      // Open in Cursor (default IDE)
       const result = await window.electronAPI.openInIde(worktree.path, 'cursor');
       if (!result.success) {
-        setError(result.error || 'Failed to open in Cursor');
+        setError(result.error || t('worktrees:actions.failedToOpen', { ide: 'Cursor' }));
       }
     }}
   >
     <Code2 className="h-3.5 w-3.5 mr-1.5" />
-    Open in Cursor
+    {t('worktrees:actions.openInCursor')}
   </Button>
   <Button
     variant="outline"
     size="sm"
     onClick={async () => {
-      // Open in VS Code
       const result = await window.electronAPI.openInIde(worktree.path, 'vscode');
       if (!result.success) {
-        setError(result.error || 'Failed to open in VS Code');
+        setError(result.error || t('worktrees:actions.failedToOpen', { ide: 'VS Code' }));
       }
     }}
   >
     <ExternalLink className="h-3.5 w-3.5 mr-1.5" />
-    VS Code
+    {t('worktrees:actions.openInVSCode')}
   </Button>
   <Button
     variant="outline"
     size="sm"
     onClick={async () => {
-      // Open in Finder
-      await window.electronAPI.openInIde(worktree.path, 'finder');
+      const result = await window.electronAPI.openInIde(worktree.path, 'finder');
+      if (!result.success) {
+        setError(result.error || t('worktrees:actions.failedToOpen', { ide: 'Finder' }));
+      }
     }}
   >
     <FolderOpen className="h-3.5 w-3.5 mr-1.5" />
-    Finder
+    {t('worktrees:actions.openInFinder')}
   </Button>

As per coding guidelines: "Always use translation keys with useTranslation() for all user-facing text in React/TypeScript frontend components - use format namespace:section.key (e.g., navigation:items.githubPRs). Never use hardcoded strings in JSX/TSX files for user-facing text."

Committable suggestion skipped: line range outside the PR's diff.

<Button
variant="outline"
Expand Down
15 changes: 14 additions & 1 deletion apps/frontend/src/renderer/lib/mocks/settings-mock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,18 @@ export const settingsMock = {
// App Update Event Listeners (no-op in browser mode)
onAppUpdateAvailable: () => () => {},
onAppUpdateDownloaded: () => () => {},
onAppUpdateProgress: () => () => {}
onAppUpdateProgress: () => () => {},

// Shell Operations (limited functionality in browser mode)
openExternal: async (url: string) => {
console.warn('[browser-mock] openExternal called with:', url);
window.open(url, '_blank');
},
openPath: async (path: string) => {
console.warn('[browser-mock] openPath called with:', path);
},
openInIde: async (path: string, ide?: string) => {
console.warn('[browser-mock] openInIde called with:', path, ide);
return { success: false, error: 'IDE opening not supported in browser mode' };
}
};
2 changes: 2 additions & 0 deletions apps/frontend/src/shared/constants/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,8 @@ export const IPC_CHANNELS = {

// Shell operations
SHELL_OPEN_EXTERNAL: 'shell:openExternal',
SHELL_OPEN_PATH: 'shell:openPath', // Open folder in Finder/Explorer
SHELL_OPEN_IN_IDE: 'shell:openInIde', // Open folder in VS Code/Cursor

// Roadmap operations
ROADMAP_GET: 'roadmap:get',
Expand Down
2 changes: 2 additions & 0 deletions apps/frontend/src/shared/types/ipc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,8 @@ export interface ElectronAPI {

// Shell operations
openExternal: (url: string) => Promise<void>;
openPath: (path: string) => Promise<void>;
openInIde: (path: string, ide?: 'cursor' | 'vscode' | 'finder') => Promise<IPCResult>;

// Auto Claude source environment operations
getSourceEnv: () => Promise<IPCResult<SourceEnvConfig>>;
Expand Down