From 0878fd55795eaa00d65edac15af31e9b6fe92e2a Mon Sep 17 00:00:00 2001
From: Yufeng He <40085740+he-yufeng@users.noreply.github.com>
Date: Sun, 7 Jun 2026 20:13:18 +0800
Subject: [PATCH] fix(filesystem): validate canonical allowed paths

---
 src/filesystem/__tests__/lib.test.ts | 39 ++++++++++++++++++++++++++++
 src/filesystem/lib.ts                | 25 +++++++++---------
 2 files changed, 52 insertions(+), 12 deletions(-)

diff --git a/src/filesystem/__tests__/lib.test.ts b/src/filesystem/__tests__/lib.test.ts
index e0ae61224f..2d172b4e8d 100644
--- a/src/filesystem/__tests__/lib.test.ts
+++ b/src/filesystem/__tests__/lib.test.ts
@@ -172,6 +172,23 @@ describe('Lib Functions', () => {
           .rejects.toThrow('Access denied - path outside allowed directories');
       });
 
+      it('allows paths whose canonical form is in an allowed directory', async () => {
+        const requestedPath = process.platform === 'win32'
+          ? 'Y:\\project\\file.txt'
+          : '/mapped/project/file.txt';
+        const realPath = process.platform === 'win32'
+          ? '\\\\nas\\share\\project\\file.txt'
+          : '/canonical/project/file.txt';
+        const allowedDir = process.platform === 'win32'
+          ? '\\\\nas\\share\\project'
+          : '/canonical/project';
+
+        setAllowedDirectories([allowedDir]);
+        mockFs.realpath.mockResolvedValueOnce(realPath);
+
+        await expect(validatePath(requestedPath)).resolves.toBe(realPath);
+      });
+
       it('handles non-existent files by checking parent directory', async () => {
         const newFilePath = process.platform === 'win32' ? 'C:\\Users\\test\\newfile.txt' : '/home/user/newfile.txt';
         const parentPath = process.platform === 'win32' ? 'C:\\Users\\test' : '/home/user';
@@ -188,6 +205,28 @@ describe('Lib Functions', () => {
         expect(result).toBe(path.resolve(newFilePath));
       });
 
+      it('allows new files whose canonical parent is in an allowed directory', async () => {
+        const newFilePath = process.platform === 'win32'
+          ? 'Y:\\project\\newfile.txt'
+          : '/mapped/project/newfile.txt';
+        const realParentPath = process.platform === 'win32'
+          ? '\\\\nas\\share\\project'
+          : '/canonical/project';
+        const allowedDir = process.platform === 'win32'
+          ? '\\\\nas\\share\\project'
+          : '/canonical/project';
+        const enoentError = new Error('ENOENT') as NodeJS.ErrnoException;
+        enoentError.code = 'ENOENT';
+
+        setAllowedDirectories([allowedDir]);
+        mockFs.realpath
+          .mockRejectedValueOnce(enoentError)
+          .mockResolvedValueOnce(realParentPath);
+
+        const result = await validatePath(newFilePath);
+        expect(result).toBe(path.resolve(newFilePath));
+      });
+
       it('rejects when parent directory does not exist', async () => {
         const newFilePath = process.platform === 'win32' ? 'C:\\Users\\test\\nonexistent\\newfile.txt' : '/home/user/nonexistent/newfile.txt';
         
diff --git a/src/filesystem/lib.ts b/src/filesystem/lib.ts
index ce4af9f38a..06929f999a 100644
--- a/src/filesystem/lib.ts
+++ b/src/filesystem/lib.ts
@@ -103,12 +103,7 @@ export async function validatePath(requestedPath: string): Promise<string> {
     : resolveRelativePathAgainstAllowedDirectories(expandedPath);
 
   const normalizedRequested = normalizePath(absolute);
-
-  // Security: Check if path is within allowed directories before any file operations
   const isAllowed = isPathWithinAllowedDirectories(normalizedRequested, allowedDirectories);
-  if (!isAllowed) {
-    throw new Error(`Access denied - path outside allowed directories: ${absolute} not in ${allowedDirectories.join(', ')}`);
-  }
 
   // Security: Handle symlinks by checking their real path to prevent symlink attacks
   // This prevents attackers from creating symlinks that point outside allowed directories
@@ -116,7 +111,8 @@ export async function validatePath(requestedPath: string): Promise<string> {
     const realPath = await fs.realpath(absolute);
     const normalizedReal = normalizePath(realPath);
     if (!isPathWithinAllowedDirectories(normalizedReal, allowedDirectories)) {
-      throw new Error(`Access denied - symlink target outside allowed directories: ${realPath} not in ${allowedDirectories.join(', ')}`);
+      const reason = isAllowed ? `symlink target outside allowed directories: ${realPath}` : `path outside allowed directories: ${absolute}`;
+      throw new Error(`Access denied - ${reason} not in ${allowedDirectories.join(', ')}`);
     }
     return realPath;
   } catch (error) {
@@ -124,16 +120,21 @@ export async function validatePath(requestedPath: string): Promise<string> {
     // This ensures we can't create files in unauthorized locations
     if ((error as NodeJS.ErrnoException).code === 'ENOENT') {
       const parentDir = path.dirname(absolute);
+      let realParentPath: string;
       try {
-        const realParentPath = await fs.realpath(parentDir);
-        const normalizedParent = normalizePath(realParentPath);
-        if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) {
-          throw new Error(`Access denied - parent directory outside allowed directories: ${realParentPath} not in ${allowedDirectories.join(', ')}`);
-        }
-        return absolute;
+        realParentPath = await fs.realpath(parentDir);
       } catch {
         throw new Error(`Parent directory does not exist: ${parentDir}`);
       }
+      const normalizedParent = normalizePath(realParentPath);
+      if (!isPathWithinAllowedDirectories(normalizedParent, allowedDirectories)) {
+        const reason = isAllowed ? `parent directory outside allowed directories: ${realParentPath}` : `path outside allowed directories: ${absolute}`;
+        throw new Error(`Access denied - ${reason} not in ${allowedDirectories.join(', ')}`);
+      }
+      return absolute;
+    }
+    if (!isAllowed) {
+      throw new Error(`Access denied - path outside allowed directories: ${absolute} not in ${allowedDirectories.join(', ')}`);
     }
     throw error;
   }