fix: improve file writing (#2447)

src/McpContext.ts

@@ -5,7 +5,6 @@
  */
 
 import fs from 'node:fs/promises';
-import fsPromises from 'node:fs/promises';
 import os from 'node:os';
 import path from 'node:path';
 import {fileURLToPath, pathToFileURL} from 'node:url';
@@ -249,7 +248,7 @@ export class McpContext implements Context {
       roots.map(async root => {
         const rootPathUri = root.uri;
         const rootPath = path.resolve(fileURLToPath(rootPathUri));
-        return await fsPromises.realpath(rootPath);
+        return await fs.realpath(rootPath);
       }),
     );
 
@@ -573,17 +572,39 @@ export class McpContext implements Context {
     return this.#extensionServiceWorkerMap.get(extensionServiceWorker.target);
   }
 
+  async #writeFile(
+    filepath: string,
+    data: Uint8Array<ArrayBufferLike>,
+  ): Promise<void> {
+    await this.validatePath(filepath);
+
+    try {
+      await fs.mkdir(path.dirname(filepath), {recursive: true});
+      // Open the file with flags to:
+      // - O_WRONLY: Write-only
+      // - O_CREAT: Create if it doesn't exist
+      // - O_TRUNC: Truncate to zero length if it exists
+      // - O_NOFOLLOW: DO NOT follow symlinks.
+      // - 0o600: Permissions: read/write for owner, no permissions for others.
+      await fs.writeFile(filepath, data, {
+        flag:
+          fs.constants.O_WRONLY |
+          fs.constants.O_CREAT |
+          fs.constants.O_TRUNC |
+          fs.constants.O_NOFOLLOW,
+        mode: 0o600,
+      });
+    } catch (err) {
+      throw new Error(`Could not write ${filepath}`, {cause: err});
+    }
+  }
+
   async saveTemporaryFile(
     data: Uint8Array<ArrayBufferLike>,
     filename: string,
   ): Promise<{filepath: string}> {
     const filepath = await getTempFilePath(filename);
-    await this.validatePath(filepath);
-    try {
-      await fs.writeFile(filepath, data);
-    } catch (err) {
-      throw new Error('Could not save a file', {cause: err});
-    }
+    await this.#writeFile(filepath, data);
     return {filepath};
   }
 
@@ -596,14 +617,8 @@ export class McpContext implements Context {
       clientProvidedFilePath,
       extension,
     );
-    try {
-      await fs.mkdir(path.dirname(filePath), {recursive: true});
-      await fs.writeFile(filePath, data);
-      return {filename: filePath};
-    } catch (err) {
-      this.logger?.(err);
-      throw new Error('Could not save a file', {cause: err});
-    }
+    await this.#writeFile(filePath, data);
+    return {filename: filePath};
   }
 
   storeTraceRecording(result: TraceResult): void {
@@ -796,7 +811,7 @@ export class McpContext implements Context {
 
       case 'file:': {
         await this.validatePath(fileURLToPath(url));
-        return await fsPromises.readFile(url, 'utf-8');
+        return await fs.readFile(url, 'utf-8');
       }
 
       default:

tests/McpContext.test.ts

@@ -485,6 +485,101 @@ describe('McpContext', () => {
     });
   });
 
+  describe('symlink security checks', () => {
+    // Symlinks are not followed on Windows by default.
+    if (os.platform() === 'win32') {
+      return;
+    }
+
+    it('saveFile refuses to write through a symlink to an existing file', async () => {
+      await withMcpContext(async (_response, context) => {
+        const tmpDir = await fs.mkdtemp(
+          path.join(os.tmpdir(), 'mcp-symlink-test-'),
+        );
+        try {
+          context.setRoots([{uri: pathToFileURL(tmpDir).href, name: 'temp'}]);
+
+          const targetPath = path.join(tmpDir, 'target.txt');
+          await fs.writeFile(targetPath, 'original content', 'utf-8');
+
+          const symlinkPath = path.join(tmpDir, 'symlink.txt');
+          await fs.symlink(targetPath, symlinkPath);
+
+          const data = new TextEncoder().encode('malicious content');
+          await assert.rejects(
+            context.saveFile(data, symlinkPath, '.txt'),
+            /Could not write/,
+          );
+
+          const content = await fs.readFile(targetPath, 'utf-8');
+          assert.strictEqual(content, 'original content');
+        } finally {
+          await fs.rm(tmpDir, {recursive: true, force: true});
+        }
+      });
+    });
+
+    it('saveFile refuses to write through a dangling symlink to a non-existent file', async () => {
+      await withMcpContext(async (_response, context) => {
+        const tmpDir = await fs.mkdtemp(
+          path.join(os.tmpdir(), 'mcp-symlink-test-'),
+        );
+        const outsideDir = await fs.mkdtemp(
+          path.join(os.tmpdir(), 'mcp-outside-test-'),
+        );
+        try {
+          context.setRoots([{uri: pathToFileURL(tmpDir).href, name: 'temp'}]);
+
+          const outsideTarget = path.join(outsideDir, 'target.txt');
+          const symlinkPath = path.join(tmpDir, 'symlink.txt');
+          await fs.symlink(outsideTarget, symlinkPath);
+
+          const data = new TextEncoder().encode('malicious content');
+          await assert.rejects(
+            context.saveFile(data, symlinkPath, '.txt'),
+            /Could not write/,
+          );
+
+          await assert.rejects(fs.stat(outsideTarget));
+        } finally {
+          await fs.rm(tmpDir, {recursive: true, force: true});
+          await fs.rm(outsideDir, {recursive: true, force: true});
+        }
+      });
+    });
+
+    it('saveFile allows writing to a file within an allowed symlinked directory', async () => {
+      await withMcpContext(async (_response, context) => {
+        const tmpDir = await fs.mkdtemp(
+          path.join(os.tmpdir(), 'mcp-symlink-test-'),
+        );
+        try {
+          context.setRoots([{uri: pathToFileURL(tmpDir).href, name: 'temp'}]);
+
+          const realDir = path.join(tmpDir, 'real_dir');
+          await fs.mkdir(realDir, {recursive: true});
+
+          const symlinkedDir = path.join(tmpDir, 'symlinked_dir');
+          await fs.symlink(realDir, symlinkedDir);
+
+          const targetFilePath = path.join(symlinkedDir, 'test.txt');
+          const data = new TextEncoder().encode('allowed content');
+
+          const result = await context.saveFile(data, targetFilePath, '.txt');
+          assert.strictEqual(result.filename, targetFilePath);
+
+          const content = await fs.readFile(
+            path.join(realDir, 'test.txt'),
+            'utf-8',
+          );
+          assert.strictEqual(content, 'allowed content');
+        } finally {
+          await fs.rm(tmpDir, {recursive: true, force: true});
+        }
+      });
+    });
+  });
+
   describe('loadResource', () => {
     describe('file protocol', () => {
       it('calls validatePath', async () => {