Skip to content

feat(filesystem): add copy_file tool #2012

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
nandsha wants to merge 3 commits into from
+55 −0
Closed

feat(filesystem): add copy_file tool #2012

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
81cf93a
feat(filesystem): add copy_file tool for efficient file/directory cop…
nandsha Jun 4, 2025
e0270d4
Merge branch 'main' into feat/filesystem-copy-tool-1581
nandsha Jul 9, 2025
5206cc7
Merge branch 'main' into feat/filesystem-copy-tool-1581
olaservo Aug 7, 2025
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
8 changes: 8 additions & 0 deletions src/filesystem/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,14 @@ The server's directory access control follows this flow:
- `destination` (string)
- Fails if destination exists

- **copy_file**
- Copy a file or directory from source to destination
- Inputs:
- `source` (string)
- `destination` (string)
- For directories, copies all contents recursively
- Fails if destination exists

- **search_files**
- Recursively search for files/directories
- Inputs:
Expand Down
47 changes: 47 additions & 0 deletions src/filesystem/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,11 @@ const MoveFileArgsSchema = z.object({
destination: z.string(),
});

const CopyFileArgsSchema = z.object({
source: z.string(),
destination: z.string(),
});

const SearchFilesArgsSchema = z.object({
path: z.string(),
pattern: z.string(),
Expand Down Expand Up @@ -548,6 +553,13 @@ server.setRequestHandler(ListToolsRequestSchema, async () => {
"Only works within allowed directories.",
inputSchema: zodToJsonSchema(EditFileArgsSchema) as ToolInput,
},
{
name: "copy_file",
description:
"Copy a file or directory from source to destination. For directories, copies all contents recursively. " +
"If destination exists, the operation will fail. Both source and destination must be within allowed directories.",
inputSchema: zodToJsonSchema(CopyFileArgsSchema) as ToolInput,
},
{
name: "create_directory",
description:
Expand Down Expand Up @@ -944,6 +956,41 @@ server.setRequestHandler(CallToolRequestSchema, async (request) => {
};
}

case "copy_file": {
Copy link
Member

@olaservo olaservo Jul 8, 2025

Choose a reason for hiding this comment

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

This new tool needs to incorporate the same security fix that was added to the write_file tool, something like:

case "copy_file": {
  const parsed = CopyFileArgsSchema.safeParse(args);
  if (!parsed.success) {
    throw new Error(`Invalid arguments for copy_file: ${parsed.error}`);
  }
  const validSourcePath = await validatePath(parsed.data.source);
  const validDestPath = await validatePath(parsed.data.destination);

  // Check if source exists and get stats
  const sourceStats = await fs.stat(validSourcePath);
  
  // Use atomic temp file + rename approach (like write_file)
  const tempPath = `${validDestPath}.${randomBytes(16).toString('hex')}.tmp`;
  
  try {
    if (sourceStats.isDirectory()) {
      // For directories, use fs.cp with options to prevent symlink following
      await fs.cp(validSourcePath, tempPath, { 
        recursive: true,
        dereference: false,  // Don't follow symlinks
        preserveTimestamps: true
      });
    } else {
      // For files, copy to temp location first
      await fs.copyFile(validSourcePath, tempPath);
    }
    
    // Atomic rename - this will fail if destination exists and won't follow symlinks
    await fs.rename(tempPath, validDestPath);
    
  } catch (error) {
    // Cleanup temp file/directory
    try {
      const tempStats = await fs.stat(tempPath);
      if (tempStats.isDirectory()) {
        await fs.rm(tempPath, { recursive: true });
      } else {
        await fs.unlink(tempPath);
      }
    } catch {}
    
    if ((error as NodeJS.ErrnoException).code === 'EEXIST') {
      throw new Error(`Destination already exists: ${parsed.data.destination}`);
    }
    throw error;
  }

  return {
    content: [{ type: "text", text: `Successfully copied ${parsed.data.source} to ${parsed.data.destination}` }],
  };
}

The tldr; version of what needs to be addressed, based on the previous PR:

  • Atomic Operation: fs.rename() is atomic - either succeeds completely or fails completely
  • No Symlink Following: fs.rename() doesn't follow symlinks at the destination
  • Fail-Safe: If destination exists (including symlinks), fs.rename() fails with EEXIST
  • Race Condition Proof: No window between "check" and "action" - the action itself is the check

const parsed = CopyFileArgsSchema.safeParse(args);
if (!parsed.success) {
throw new Error(`Invalid arguments for copy_file: ${parsed.error}`);
}
const validSourcePath = await validatePath(parsed.data.source);
const validDestPath = await validatePath(parsed.data.destination);

// Check if destination already exists
try {
await fs.access(validDestPath);
throw new Error(`Destination already exists: ${parsed.data.destination}`);
} catch (error) {
// If error is NOT about file not existing, re-throw it
if (error instanceof Error && !error.message.includes('ENOENT')) {
throw error;
}
}

// Check if source is a directory
const sourceStats = await fs.stat(validSourcePath);

if (sourceStats.isDirectory()) {
// For directories, use fs.cp with recursive option
await fs.cp(validSourcePath, validDestPath, { recursive: true });
} else {
// For files, use copyFile which is optimized for large files
await fs.copyFile(validSourcePath, validDestPath);
}

return {
content: [{ type: "text", text: `Successfully copied ${parsed.data.source} to ${parsed.data.destination}` }],
};
}

case "list_allowed_directories": {
return {
content: [{
Expand Down