From fa89d69f5c8e92abd3479b3673f8e7bfe7a99bf5 Mon Sep 17 00:00:00 2001
From: dev-willbird1936 <dev-willbird1936@users.noreply.github.com>
Date: Thu, 23 Jul 2026 15:33:06 +0100
Subject: [PATCH] fix(filesystem): reject existing move destinations

---
 .../__tests__/structured-content.test.ts      | 75 +++++++++++++++++++
 src/filesystem/index.ts                       | 22 +++++-
 2 files changed, 96 insertions(+), 1 deletion(-)

diff --git a/src/filesystem/__tests__/structured-content.test.ts b/src/filesystem/__tests__/structured-content.test.ts
index 4605b72a8f..240feec777 100644
--- a/src/filesystem/__tests__/structured-content.test.ts
+++ b/src/filesystem/__tests__/structured-content.test.ts
@@ -122,6 +122,81 @@ describe('structuredContent schema compliance', () => {
       // The content should contain success message
       expect(structuredContent.content).toContain('Successfully moved');
     });
+
+    it('should reject an existing destination without replacing it', async () => {
+      const sourcePath = path.join(testDir, 'source.txt');
+      const destPath = path.join(testDir, 'existing.txt');
+      await fs.writeFile(sourcePath, 'source content');
+      await fs.writeFile(destPath, 'existing content');
+
+      const result = await client.callTool({
+        name: 'move_file',
+        arguments: {
+          source: sourcePath,
+          destination: destPath
+        }
+      });
+
+      expect(result.isError).toBe(true);
+      expect(result.content[0]).toMatchObject({ type: 'text' });
+      expect((result.content[0] as { text: string }).text).toContain(
+        'Destination already exists'
+      );
+      await expect(fs.readFile(sourcePath, 'utf8')).resolves.toBe('source content');
+      await expect(fs.readFile(destPath, 'utf8')).resolves.toBe('existing content');
+    });
+
+    it('should move directories recursively without replacing an existing tree', async () => {
+      const sourcePath = path.join(testDir, 'source-dir');
+      const destPath = path.join(testDir, 'moved-dir');
+      await fs.mkdir(path.join(sourcePath, 'nested'), { recursive: true });
+      await fs.writeFile(path.join(sourcePath, 'nested', 'file.txt'), 'source content');
+
+      const result = await client.callTool({
+        name: 'move_file',
+        arguments: {
+          source: sourcePath,
+          destination: destPath
+        }
+      });
+
+      expect(result.isError).not.toBe(true);
+      await expect(fs.stat(sourcePath)).rejects.toMatchObject({ code: 'ENOENT' });
+      await expect(
+        fs.readFile(path.join(destPath, 'nested', 'file.txt'), 'utf8')
+      ).resolves.toBe('source content');
+    });
+
+    it('should reject an existing directory destination without merging trees', async () => {
+      const sourcePath = path.join(testDir, 'source-dir');
+      const destPath = path.join(testDir, 'existing-dir');
+      await fs.mkdir(sourcePath);
+      await fs.mkdir(destPath);
+      await fs.writeFile(path.join(sourcePath, 'source.txt'), 'source content');
+      await fs.writeFile(path.join(destPath, 'existing.txt'), 'existing content');
+
+      const result = await client.callTool({
+        name: 'move_file',
+        arguments: {
+          source: sourcePath,
+          destination: destPath
+        }
+      });
+
+      expect(result.isError).toBe(true);
+      expect((result.content[0] as { text: string }).text).toContain(
+        'Destination already exists'
+      );
+      await expect(
+        fs.readFile(path.join(sourcePath, 'source.txt'), 'utf8')
+      ).resolves.toBe('source content');
+      await expect(
+        fs.readFile(path.join(destPath, 'existing.txt'), 'utf8')
+      ).resolves.toBe('existing content');
+      await expect(fs.stat(path.join(destPath, 'source.txt'))).rejects.toMatchObject({
+        code: 'ENOENT'
+      });
+    });
   });
 
   describe('list_directory (control - already working)', () => {
diff --git a/src/filesystem/index.ts b/src/filesystem/index.ts
index 234605bb13..5e0cf30fb0 100644
--- a/src/filesystem/index.ts
+++ b/src/filesystem/index.ts
@@ -631,7 +631,27 @@ server.registerTool(
   async (args: z.infer<typeof MoveFileArgsSchema>) => {
     const validSourcePath = await validatePath(args.source);
     const validDestPath = await validatePath(args.destination);
-    await fs.rename(validSourcePath, validDestPath);
+
+    try {
+      // `rename()` can overwrite a destination created after a separate
+      // existence check. `cp()` creates each destination entry exclusively
+      // when these options are set, so a concurrent creator cannot be clobbered.
+      await fs.cp(validSourcePath, validDestPath, {
+        recursive: true,
+        force: false,
+        errorOnExist: true,
+        preserveTimestamps: true,
+      });
+    } catch (error) {
+      if (
+        (error as NodeJS.ErrnoException).code === "EEXIST" ||
+        (error as NodeJS.ErrnoException).code === "ERR_FS_CP_EEXIST"
+      ) {
+        throw new Error(`Destination already exists: ${args.destination}`);
+      }
+      throw error;
+    }
+    await fs.rm(validSourcePath, { recursive: true });
     const text = `Successfully moved ${args.source} to ${args.destination}`;
     const contentBlock = { type: "text" as const, text };
     return {