Prevent concurrent file copying (#2841)

src/ipc/processors/response_processor.ts

@@ -423,6 +423,7 @@ export async function processFullResponseActions(
         const result = await executeCopyFile({
           from: tag.from,
           to: tag.to,
+          appId: chatWithApp.app.id,
           appPath,
           supabaseProjectId: chatWithApp.app.supabaseProjectId,
           supabaseOrganizationSlug: chatWithApp.app.supabaseOrganizationSlug,

src/ipc/utils/copy_file_utils.ts

@@ -4,6 +4,7 @@ import log from "electron-log";
 import { safeJoin } from "./path_utils";
 import { gitAdd } from "./git_utils";
 import { isWithinDyadMediaDir } from "./media_path_utils";
+import { withLock } from "./lock_utils";
 import { deploySupabaseFunction } from "../../supabase_admin/supabase_management_client";
 import {
   isServerFunction,
@@ -31,6 +32,7 @@ export interface CopyFileResult {
 export async function executeCopyFile({
   from,
   to,
+  appId,
   appPath,
   supabaseProjectId,
   supabaseOrganizationSlug,
@@ -38,11 +40,13 @@ export async function executeCopyFile({
 }: {
   from: string;
   to: string;
+  appId: number;
   appPath: string;
   supabaseProjectId?: string | null;
   supabaseOrganizationSlug?: string | null;
   isSharedModulesChanged?: boolean;
 }): Promise<CopyFileResult> {
+  return withLock(appId, async () => {
     // Resolve the source path: allow both .dyad/media paths and app-relative paths
     let fromFullPath: string;
     if (path.isAbsolute(from)) {
@@ -63,6 +67,29 @@ export async function executeCopyFile({
       throw new Error(`Source file does not exist: ${from}`);
     }
 
+    // Security: resolve symlinks and re-validate that paths remain within bounds.
+    // path.resolve() does not follow symlinks, so an attacker could place a
+    // symlink inside the allowed directory that points outside it.
+    const realFromPath = fs.realpathSync(fromFullPath);
+    const resolvedAppPath = fs.realpathSync(appPath);
+    if (
+      path.isAbsolute(from) &&
+      !isWithinDyadMediaDir(realFromPath, resolvedAppPath)
+    ) {
+      throw new Error(
+        `Source path resolves to a location outside the .dyad/media directory (possible symlink traversal)`,
+      );
+    }
+    if (
+      !path.isAbsolute(from) &&
+      !realFromPath.startsWith(resolvedAppPath + path.sep) &&
+      realFromPath !== resolvedAppPath
+    ) {
+      throw new Error(
+        `Source path resolves to a location outside the app directory (possible symlink traversal)`,
+      );
+    }
+
     // Track if this involves shared modules
     const sharedModuleChanged = isSharedServerModule(to);
 
@@ -70,7 +97,7 @@ export async function executeCopyFile({
     const dirPath = path.dirname(toFullPath);
     fs.mkdirSync(dirPath, { recursive: true });
 
-  // Copy the file
+    // Copy the file (do not follow symlinks at destination)
     fs.copyFileSync(fromFullPath, toFullPath);
     logger.log(`Successfully copied file: ${fromFullPath} -> ${toFullPath}`);
 
@@ -103,4 +130,5 @@ export async function executeCopyFile({
       sharedModuleChanged,
       deployError,
     };
+  });
 }

src/ipc/utils/lock_utils.ts

 const locks = new Map<number | string, Promise<void>>();
 
 /**
- * Acquires a lock for an app operation
- * @param lockId The app ID to lock
- * @returns An object with release function and promise
- */
-export function acquireLock(lockId: number | string): {
-  release: () => void;
-  promise: Promise<void>;
-} {
-  let release: () => void = () => {};
-
-  const promise = new Promise<void>((resolve) => {
-    release = () => {
-      locks.delete(lockId);
-      resolve();
-    };
-  });
-
-  locks.set(lockId, promise);
-  return { release, promise };
-}
-
-/**
- * Executes a function with a lock on the lock ID
+ * Executes a function with a lock on the lock ID.
+ * Uses promise-chaining so that queued operations execute serially,
+ * preventing the race where multiple waiters all acquire simultaneously.
+ *
  * @param lockId The lock ID to lock
  * @param fn The function to execute with the lock
  * @returns Result of the function
  */
-export async function withLock<T>(
+export function withLock<T>(
   lockId: number | string,
   fn: () => Promise<T>,
 ): Promise<T> {
-  // Wait for any existing operation to complete
-  const existingLock = locks.get(lockId);
-  if (existingLock) {
-    await existingLock;
-  }
+  const lastOperation = locks.get(lockId) ?? Promise.resolve();
 
-  // Acquire a new lock
-  const { release } = acquireLock(lockId);
+  let resolve: () => void;
+  const newLock = new Promise<void>((r) => {
+    resolve = r;
+  });
+  locks.set(lockId, newLock);
 
+  const result = lastOperation.then(async () => {
     try {
-    const result = await fn();
-    return result;
+      return await fn();
     } finally {
-    release();
+      resolve();
+      if (locks.get(lockId) === newLock) {
+        locks.delete(lockId);
       }
+    }
+  });
+
+  return result;
 }

src/pro/main/ipc/handlers/local_agent/tools/copy_file.ts

@@ -34,6 +34,7 @@ export const copyFileTool: ToolDefinition<z.infer<typeof copyFileSchema>> = {
     const result = await executeCopyFile({
       from: args.from,
       to: args.to,
+      appId: ctx.appId,
       appPath: ctx.appPath,
       supabaseProjectId: ctx.supabaseProjectId,
       supabaseOrganizationSlug: ctx.supabaseOrganizationSlug,