From 06af009ccdcdcf2108e43687f0870ac235ce8158 Mon Sep 17 00:00:00 2001 From: "zhao.wang" <57819425+Excelius-Wang@users.noreply.github.com> Date: Sun, 30 Aug 2026 17:42:12 +0800 Subject: [PATCH] fix(filesystem): support cross-device moves --- .../__tests__/cross-device-move.test.ts | 86 +++++++++++ src/filesystem/__tests__/lib.test.ts | 133 ++++++++++++++++++ src/filesystem/lib.ts | 43 +++++- 3 files changed, 261 insertions(+), 1 deletion(-) create mode 100644 src/filesystem/__tests__/cross-device-move.test.ts diff --git a/src/filesystem/__tests__/cross-device-move.test.ts b/src/filesystem/__tests__/cross-device-move.test.ts new file mode 100644 index 0000000000..a5577a8fe4 --- /dev/null +++ b/src/filesystem/__tests__/cross-device-move.test.ts @@ -0,0 +1,86 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import fs from 'fs/promises'; +import net from 'net'; +import os from 'os'; +import path from 'path'; +import { moveFile } from '../lib.js'; + +describe.skipIf(process.platform === 'win32')('moveFile cross-device fallback', () => { + let testDirectory: string | undefined; + + afterEach(async () => { + vi.restoreAllMocks(); + if (testDirectory) { + await fs.rm(testDirectory, { recursive: true, force: true }); + testDirectory = undefined; + } + }); + + function simulateCrossDeviceRename() { + const rename = fs.rename.bind(fs); + vi.spyOn(fs, 'rename') + .mockRejectedValueOnce( + Object.assign(new Error('cross-device link'), { code: 'EXDEV' }), + ) + .mockImplementation(rename); + } + + it('preserves relative symbolic links', async () => { + testDirectory = await fs.mkdtemp( + path.join(os.tmpdir(), 'mcp-cross-device-symlink-'), + ); + const source = path.join(testDirectory, 'source'); + const destination = path.join(testDirectory, 'destination'); + await fs.mkdir(source, { mode: 0o751 }); + await fs.writeFile(path.join(source, 'target.txt'), 'payload', { + mode: 0o640, + }); + await fs.symlink('target.txt', path.join(source, 'link.txt')); + simulateCrossDeviceRename(); + + await moveFile(source, destination); + + await expect(fs.readlink(path.join(destination, 'link.txt'))).resolves.toBe( + 'target.txt', + ); + await expect( + fs.readFile(path.join(destination, 'link.txt'), 'utf8'), + ).resolves.toBe('payload'); + expect((await fs.stat(destination)).mode & 0o777).toBe(0o751); + expect( + (await fs.stat(path.join(destination, 'target.txt'))).mode & 0o777, + ).toBe(0o640); + await expect(fs.access(source)).rejects.toMatchObject({ code: 'ENOENT' }); + }); + + it('does not expose a partial destination when copying fails', async () => { + testDirectory = await fs.mkdtemp( + path.join(os.tmpdir(), 'mcp-cross-device-failure-'), + ); + const source = path.join(testDirectory, 'source'); + const destination = path.join(testDirectory, 'destination'); + await fs.mkdir(source); + await fs.writeFile(path.join(source, 'copied-first.txt'), 'payload'); + + const socketServer = net.createServer(); + await new Promise((resolve, reject) => { + socketServer.once('error', reject); + socketServer.listen(path.join(source, 'unsupported.sock'), resolve); + }); + simulateCrossDeviceRename(); + + try { + await expect(moveFile(source, destination)).rejects.toMatchObject({ + code: 'ERR_FS_CP_SOCKET', + }); + } finally { + await new Promise((resolve) => socketServer.close(() => resolve())); + } + + await expect(fs.access(source)).resolves.toBeUndefined(); + await expect(fs.access(destination)).rejects.toMatchObject({ + code: 'ENOENT', + }); + expect((await fs.readdir(testDirectory)).sort()).toEqual(['source']); + }); +}); diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts index eeee46b99d..bb77ae2273 100644 --- a/src/filesystem/__tests__/lib.test.ts +++ b/src/filesystem/__tests__/lib.test.ts @@ -393,6 +393,139 @@ describe('Lib Functions', () => { expect(mockFs.rename).not.toHaveBeenCalled(); }); + + it('falls back to copy and remove across filesystem boundaries', async () => { + const enoent = Object.assign(new Error('not found'), { code: 'ENOENT' }); + const exdev = Object.assign(new Error('cross-device link'), { + code: 'EXDEV', + }); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rename.mockRejectedValueOnce(exdev); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.cp.mockResolvedValueOnce(undefined); + mockFs.rename.mockResolvedValueOnce(undefined); + mockFs.rm.mockResolvedValueOnce(undefined); + + await moveFile('/source/file.txt', '/mounted-volume/file.txt'); + + expect(mockFs.cp).toHaveBeenCalledWith( + '/source/file.txt', + expect.stringMatching( + /^\/mounted-volume\/file\.txt\.[a-f0-9]+\.tmp$/, + ), + { + recursive: true, + errorOnExist: true, + force: false, + preserveTimestamps: true, + verbatimSymlinks: true, + }, + ); + const tempPath = mockFs.cp.mock.calls[0][1]; + expect(mockFs.rename).toHaveBeenLastCalledWith( + tempPath, + '/mounted-volume/file.txt', + ); + expect(mockFs.rm).toHaveBeenCalledWith('/source/file.txt', { + recursive: true, + }); + }); + + it('does not copy when rename fails for another reason', async () => { + const enoent = Object.assign(new Error('not found'), { code: 'ENOENT' }); + const permissionError = Object.assign(new Error('permission denied'), { + code: 'EACCES', + }); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rename.mockRejectedValueOnce(permissionError); + + await expect( + moveFile('/source/file.txt', '/mounted-volume/file.txt'), + ).rejects.toBe(permissionError); + + expect(mockFs.cp).not.toHaveBeenCalled(); + expect(mockFs.rm).not.toHaveBeenCalled(); + }); + + it('keeps the source when the cross-filesystem copy fails', async () => { + const enoent = Object.assign(new Error('not found'), { code: 'ENOENT' }); + const exdev = Object.assign(new Error('cross-device link'), { + code: 'EXDEV', + }); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rename.mockRejectedValueOnce(exdev); + mockFs.cp.mockRejectedValueOnce(new Error('copy failed')); + mockFs.rm.mockResolvedValueOnce(undefined); + + await expect( + moveFile('/source/file.txt', '/mounted-volume/file.txt'), + ).rejects.toThrow('copy failed'); + + const tempPath = mockFs.cp.mock.calls[0][1]; + expect(mockFs.rm).toHaveBeenCalledWith(tempPath, { + recursive: true, + force: true, + }); + expect(mockFs.rm).not.toHaveBeenCalledWith('/source/file.txt', { + recursive: true, + }); + }); + + it('does not overwrite a destination created while copying', async () => { + const enoent = Object.assign(new Error('not found'), { code: 'ENOENT' }); + const exdev = Object.assign(new Error('cross-device link'), { + code: 'EXDEV', + }); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rename.mockRejectedValueOnce(exdev); + mockFs.cp.mockResolvedValueOnce(undefined); + mockFs.lstat.mockResolvedValueOnce({} as any); + mockFs.rm.mockResolvedValueOnce(undefined); + + await expect( + moveFile('/source/file.txt', '/mounted-volume/file.txt'), + ).rejects.toThrow('Destination already exists'); + + const tempPath = mockFs.cp.mock.calls[0][1]; + expect(mockFs.rename).toHaveBeenCalledTimes(1); + expect(mockFs.rm).toHaveBeenCalledWith(tempPath, { + recursive: true, + force: true, + }); + expect(mockFs.rm).not.toHaveBeenCalledWith('/source/file.txt', { + recursive: true, + }); + }); + + it('cleans the staged copy when the final rename fails', async () => { + const enoent = Object.assign(new Error('not found'), { code: 'ENOENT' }); + const exdev = Object.assign(new Error('cross-device link'), { + code: 'EXDEV', + }); + const permissionError = Object.assign(new Error('permission denied'), { + code: 'EACCES', + }); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rename + .mockRejectedValueOnce(exdev) + .mockRejectedValueOnce(permissionError); + mockFs.cp.mockResolvedValueOnce(undefined); + mockFs.lstat.mockRejectedValueOnce(enoent); + mockFs.rm.mockResolvedValueOnce(undefined); + + await expect( + moveFile('/source/file.txt', '/mounted-volume/file.txt'), + ).rejects.toBe(permissionError); + + const tempPath = mockFs.cp.mock.calls[0][1]; + expect(mockFs.rm).toHaveBeenCalledWith(tempPath, { + recursive: true, + force: true, + }); + expect(mockFs.rm).not.toHaveBeenCalledWith('/source/file.txt', { + recursive: true, + }); + }); }); }); diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts index f2371a9a01..ca7300bafb 100644 --- a/src/filesystem/lib.ts +++ b/src/filesystem/lib.ts @@ -241,7 +241,48 @@ export async function moveFile(sourcePath: string, destinationPath: string): Pro await fs.lstat(destinationPath); } catch (error) { if ((error as NodeJS.ErrnoException).code === 'ENOENT') { - await fs.rename(sourcePath, destinationPath); + try { + await fs.rename(sourcePath, destinationPath); + } catch (renameError) { + if ((renameError as NodeJS.ErrnoException).code !== 'EXDEV') { + throw renameError; + } + + // rename cannot cross filesystem boundaries, which is common when + // moving between container volumes, network mounts, and local disks. + // Stage the copy beside the destination, then rename it into place so + // a failed copy never exposes a partial destination. This follows the + // same temporary-file pattern used by writeFileContent and + // applyFileEdits. + const tempPath = `${destinationPath}.${randomBytes(16).toString('hex')}.tmp`; + try { + await fs.cp(sourcePath, tempPath, { + recursive: true, + errorOnExist: true, + force: false, + preserveTimestamps: true, + verbatimSymlinks: true, + }); + + // The copy can be long-running. Recheck immediately before the final + // rename to minimize the existing lstat/rename race and avoid + // overwriting a path created during the copy. + try { + await fs.lstat(destinationPath); + } catch (destinationError) { + if ((destinationError as NodeJS.ErrnoException).code === 'ENOENT') { + await fs.rename(tempPath, destinationPath); + await fs.rm(sourcePath, { recursive: true }); + return; + } + throw destinationError; + } + throw new Error(`Destination already exists: ${destinationPath}`); + } catch (copyError) { + await fs.rm(tempPath, { recursive: true, force: true }).catch(() => {}); + throw copyError; + } + } return; } throw error;