Skip to content

fix(filesystem): reject existing move destinations#4531

Open
dev-willbird1936 wants to merge 1 commit into
modelcontextprotocol:mainfrom
dev-willbird1936:fix/filesystem-move-no-overwrite
Open

fix(filesystem): reject existing move destinations#4531
dev-willbird1936 wants to merge 1 commit into
modelcontextprotocol:mainfrom
dev-willbird1936:fix/filesystem-move-no-overwrite

Conversation

@dev-willbird1936

@dev-willbird1936 dev-willbird1936 commented Jul 15, 2026

Copy link
Copy Markdown

Description

Make the filesystem server's move_file honor its documented no-overwrite contract. Existing destination files, directories, and symlinks now return a tool error, including when another process creates the destination during the move.

Server Details

  • Server: filesystem
  • Changes to: move_file

Motivation and Context

POSIX rename() can replace an existing destination. A separate existence check followed by rename() still has a check-to-use race, so a concurrent destination could be overwritten. The implementation now copies with exclusive destination creation and removes the source only after the copy succeeds.

How Has This Been Tested?

  • Focused structured-content.test.ts suite: 13 passed.
  • Full filesystem package suite: 155 passed.
  • Filesystem package TypeScript build: passed.
  • git diff --check: passed.
  • LLM-client integration was not exercised; tests call the server tool through the MCP client test harness.

Breaking Changes

None to the documented API. A successful move now uses copy-then-remove rather than an atomic same-filesystem rename so that portable Node.js code can enforce no replacement. If copying fails, the source is retained.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update

Checklist

  • I have read the MCP Protocol Documentation
  • My changes follow MCP security best practices
  • I have updated the server's README accordingly (not required; the documented contract is unchanged)
  • I have tested this with an LLM client
  • My code follows the repository's style guidelines
  • New and existing tests pass locally
  • I have added appropriate error handling
  • I have documented all environment variables and configuration options (not applicable)

Additional context

fs.cp() is called with force: false and errorOnExist: true, which makes destination creation exclusive and closes the overwrite race. Directory success and rejection without tree merging are covered by regression tests.

@dev-willbird1936
dev-willbird1936 force-pushed the fix/filesystem-move-no-overwrite branch from 282c88c to fa89d69 Compare July 23, 2026 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant