From fb3a8499f37542d05a40f4971e8c4b1e2e09fb80 Mon Sep 17 00:00:00 2001
From: Stefan de Vogelaere <email@stefandevogelaere.be>
Date: Sun, 11 Jan 2026 16:55:25 +0100
Subject: [PATCH] fix: address CodeRabbitAI security and UX review comments

Security improvements in open-in-editor.ts:
- Use execFile with argument arrays instead of shell interpolation
  in commandExists() to prevent command injection
- Replace shell `test -d` commands with Node.js fs/promises access()
  in findMacApp() for safer file system checks
- Add cache TTL (5 minutes) for editor detection to prevent stale data

UX improvements in worktree-actions-dropdown.tsx:
- Add error handling for clipboard copy operation
- Show success toast when path is copied
- Show error toast if clipboard write fails

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
---
 .../routes/worktree/routes/open-in-editor.ts  | 35 ++++++++++++-------
 .../components/worktree-actions-dropdown.tsx  | 10 +++++-
 2 files changed, 32 insertions(+), 13 deletions(-)

diff --git a/apps/server/src/routes/worktree/routes/open-in-editor.ts b/apps/server/src/routes/worktree/routes/open-in-editor.ts
index 25652efd..7751b9a5 100644
--- a/apps/server/src/routes/worktree/routes/open-in-editor.ts
+++ b/apps/server/src/routes/worktree/routes/open-in-editor.ts
@@ -4,25 +4,34 @@
  */
 
 import type { Request, Response } from 'express';
-import { exec, execFile } from 'child_process';
+import { execFile } from 'child_process';
 import { promisify } from 'util';
 import { homedir } from 'os';
-import { isAbsolute } from 'path';
+import { isAbsolute, join } from 'path';
+import { access } from 'fs/promises';
 import type { EditorInfo } from '@automaker/types';
 import { getErrorMessage, logError } from '../common.js';
 
-const execAsync = promisify(exec);
 const execFileAsync = promisify(execFile);
 
+// Cache with TTL for editor detection
 let cachedEditor: EditorInfo | null = null;
 let cachedEditors: EditorInfo[] | null = null;
+let cacheTimestamp: number = 0;
+const CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
+
+function isCacheValid(): boolean {
+  return Date.now() - cacheTimestamp < CACHE_TTL_MS;
+}
 
 /**
  * Check if a CLI command exists in PATH
+ * Uses execFile to avoid shell injection
  */
 async function commandExists(cmd: string): Promise<boolean> {
   try {
-    await execAsync(process.platform === 'win32' ? `where ${cmd}` : `which ${cmd}`);
+    const whichCmd = process.platform === 'win32' ? 'where' : 'which';
+    await execFileAsync(whichCmd, [cmd]);
     return true;
   } catch {
     return false;
@@ -31,24 +40,25 @@ async function commandExists(cmd: string): Promise<boolean> {
 
 /**
  * Check if a macOS app bundle exists and return the path if found
- * Checks both /Applications and ~/Applications
+ * Uses Node fs methods instead of shell commands for safety
  */
 async function findMacApp(appName: string): Promise<string | null> {
   if (process.platform !== 'darwin') return null;
 
   // Check /Applications first
+  const systemAppPath = join('/Applications', `${appName}.app`);
   try {
-    await execAsync(`test -d "/Applications/${appName}.app"`);
-    return `/Applications/${appName}.app`;
+    await access(systemAppPath);
+    return systemAppPath;
   } catch {
     // Not in /Applications
   }
 
   // Check ~/Applications (used by JetBrains Toolbox and others)
+  const userAppPath = join(homedir(), 'Applications', `${appName}.app`);
   try {
-    const homeDir = homedir();
-    await execAsync(`test -d "${homeDir}/Applications/${appName}.app"`);
-    return `${homeDir}/Applications/${appName}.app`;
+    await access(userAppPath);
+    return userAppPath;
   } catch {
     return null;
   }
@@ -81,8 +91,8 @@ async function findEditor(
 }
 
 async function detectAllEditors(): Promise<EditorInfo[]> {
-  // Return cached result if available
-  if (cachedEditors) {
+  // Return cached result if still valid
+  if (cachedEditors && isCacheValid()) {
     return cachedEditors;
   }
 
@@ -121,6 +131,7 @@ async function detectAllEditors(): Promise<EditorInfo[]> {
   }
 
   cachedEditors = editors;
+  cacheTimestamp = Date.now();
   return editors;
 }
 
diff --git a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
index 59fd8c6b..b94faed7 100644
--- a/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
+++ b/apps/ui/src/components/views/board-view/worktree-panel/components/worktree-actions-dropdown.tsx
@@ -25,6 +25,7 @@ import {
   AlertCircle,
   Copy,
 } from 'lucide-react';
+import { toast } from 'sonner';
 import { cn } from '@/lib/utils';
 import type { WorktreeInfo, DevServerInfo, PRInfo, GitRepoStatus } from '../types';
 import { TooltipWrapper } from './tooltip-wrapper';
@@ -249,7 +250,14 @@ export function WorktreeActionsDropdown({
               })}
               {otherEditors.length > 0 && <DropdownMenuSeparator />}
               <DropdownMenuItem
-                onClick={() => navigator.clipboard.writeText(worktree.path)}
+                onClick={async () => {
+                  try {
+                    await navigator.clipboard.writeText(worktree.path);
+                    toast.success('Path copied to clipboard');
+                  } catch {
+                    toast.error('Failed to copy path to clipboard');
+                  }
+                }}
                 className="text-xs"
               >
                 <Copy className="w-3.5 h-3.5 mr-2" />