Skip to content

fix(filesystem): handle Windows EPERM when renaming over locked files#3296

Open
MattBenesch wants to merge 1 commit intomodelcontextprotocol:mainfrom
MattBenesch:fix/issue-3199
Open

fix(filesystem): handle Windows EPERM when renaming over locked files#3296
MattBenesch wants to merge 1 commit intomodelcontextprotocol:mainfrom
MattBenesch:fix/issue-3199

Conversation

@MattBenesch
Copy link

On Windows, fs.rename() fails with EPERM when the target file is locked by another process (e.g., open in an editor). This adds a fallback that uses fs.cp() + fs.unlink() when EPERM is encountered, allowing edits to succeed even when the file is open.

Fixes #3199

Description

Adds atomicReplaceFile() helper that falls back to fs.cp() when fs.rename() fails with EPERM.

Publishing Your Server

Note: We are no longer accepting PRs to add servers to the README. Instead, please publish your server to the MCP Server Registry to make it discoverable to the MCP ecosystem.

To publish your server, follow the quickstart guide. You can browse published servers at https://registry.modelcontextprotocol.io/.

Server Details

  • Server: filesystem
  • Changes to: tools (edit_file, write_file)

Motivation and Context

On Windows, files open in editors (VS Code, Notepad, etc.) are locked. The current fs.rename() approach fails with EPERM when trying to atomically replace these locked files. This prevents users from editing files they have open.

How Has This Been Tested?

Unit tests added for EPERM fallback behavior. All 46 lib.test.ts tests pass.

Breaking Changes

None - existing behavior unchanged on Linux/macOS. Windows users will now have edits succeed where they previously failed.

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 follows MCP security best practices
  • I have updated the server's README accordingly
  • 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

Additional context

On Windows, fs.rename() fails with EPERM when the target file is locked
by another process (e.g., open in an editor). This adds a fallback that
uses fs.cp() + fs.unlink() when EPERM is encountered, allowing edits to
succeed even when the file is open.

Fixes modelcontextprotocol#3199
Copy link
Member

@olaservo olaservo left a comment

Choose a reason for hiding this comment

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

Thanks for this fix! I reproduced the bug on Windows 11 (Node v22.12.0) and confirmed fs.rename fails with EPERM when the target is locked, while the fallback succeeds. Build and all 145 tests pass.

I think you need to make one change to the fallback strategy to preserve the codebase's existing symlink safety guarantees, plus a small robustness fix:


Security: fs.cp with force: true introduces a TOCTOU window

The existing code comments at both call sites explicitly guard against this:

Security: Use atomic rename to prevent race conditions where symlinks could be created between validation and write. Rename operations replace the target file atomically and don't follow symlinks.

We verified in the Node.js source that fs.cp with { force: true } calls unlink(dest) before copyFile. This creates a brief window where the target path is empty and a symlink could be planted before copyFile runs. While practical risk is low (Windows-only path, symlinks require admin/Developer Mode, microsecond window), this is a regression from the current code's guarantees.

Fix: try fs.writeFile first. Writing directly to the locked file overwrites in-place without unlinking — no empty path, no TOCTOU window. We tested both approaches against all four Windows file-sharing flag combinations using PowerShell [System.IO.File]::Open():

Lock type rename writeFile fs.cp
READ|WRITE|DELETE (VS Code / Node.js) EPERM OK OK
READ|WRITE (no DELETE) EPERM OK EBUSY
READ|DELETE (no WRITE) EPERM EBUSY OK
READ only EPERM EBUSY EBUSY

Neither alone covers all cases, but cascading them (writeFile first, fs.cp as fallback) covers the most scenarios without introducing any new security risk:

async function atomicReplaceFile(tempPath: string, targetPath: string): Promise<void> {
  try {
    await fs.rename(tempPath, targetPath);
  } catch (error) {
    if ((error as NodeJS.ErrnoException).code === 'EPERM') {
      // Try direct overwrite first — no unlink means no TOCTOU symlink window.
      // Works when the locker grants FILE_SHARE_WRITE (most common case).
      try {
        const content = await fs.readFile(tempPath);
        await fs.writeFile(targetPath, content);
      } catch {
        // Fall back to fs.cp for the uncommon case where the locker grants
        // FILE_SHARE_DELETE but not WRITE. fs.cp calls unlink(dest) before
        // copyFile, which has a brief TOCTOU window, but this is acceptable:
        // Windows-only, symlinks need admin, and the window is microseconds.
        await fs.cp(tempPath, targetPath, { force: true });
      }
      // Best-effort temp cleanup — don't fail a successful write over this
      try {
        await fs.unlink(tempPath);
      } catch {
        // Target was already written successfully
      }
    } else {
      throw error;
    }
  }
}

Why this ordering is safe: In the common scenarios (VS Code, most editors), writeFile succeeds with zero TOCTOU window. The fs.cp fallback only runs in the uncommon FILE_SHARE_DELETE-without-WRITE case. So the cascading approach is strictly equal or better than fs.cp alone in every scenario.


(This review was assisted by Claude and edited by me)

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.

filesystem edit_file: EPERM error when renaming over locked files on Windows

2 participants