Fix UNC path validation: cover all corruption sites on Windows

src/filesystem/__tests__/lib.test.ts

@@ -228,6 +228,64 @@ describe('Lib Functions', () => {
           process.cwd = originalCwd;
         }
       });
+
+      // Tests for UNC path resolution (bug #3 in the companion PR):
+      // validatePath calls path.resolve() on the absolute path before the
+      // allowed-directory check. On Windows, path.resolve corrupts UNC paths
+      // (\\server\share becomes C:\server\share via path.normalize stripping
+      // a leading backslash). Without the UNC guard, every UNC access throws
+      // "Access denied" even when the UNC directory is explicitly allowed.
+      describe('UNC path handling on Windows', () => {
+        beforeEach(() => {
+          if (process.platform !== 'win32') return;
+          setAllowedDirectories(['\\\\server\\share\\project']);
+          // Simulate fs.realpath returning the UNC path as-is
+          mockFs.realpath.mockImplementation(async (p: any) => p.toString());
+        });
+
+        it('accepts UNC file path within allowed UNC directory', async () => {
+          if (process.platform !== 'win32') return;
+          const testPath = '\\\\server\\share\\project\\file.txt';
+          const result = await validatePath(testPath);
+          expect(result).toBe(testPath);
+        });
+
+        it('accepts UNC directory path matching allowed UNC exactly', async () => {
+          if (process.platform !== 'win32') return;
+          const testPath = '\\\\server\\share\\project';
+          const result = await validatePath(testPath);
+          expect(result).toBe(testPath);
+        });
+
+        it('accepts nested UNC subdirectory within allowed UNC', async () => {
+          if (process.platform !== 'win32') return;
+          const testPath = '\\\\server\\share\\project\\src\\sub\\deep.ts';
+          const result = await validatePath(testPath);
+          expect(result).toBe(testPath);
+        });
+
+        it('rejects UNC path outside allowed UNC directory', async () => {
+          if (process.platform !== 'win32') return;
+          const testPath = '\\\\server\\share\\other\\file.txt';
+          await expect(validatePath(testPath))
+            .rejects.toThrow('Access denied - path outside allowed directories');
+        });
+
+        it('rejects UNC path on a different server', async () => {
+          if (process.platform !== 'win32') return;
+          const testPath = '\\\\other-server\\share\\project\\file.txt';
+          await expect(validatePath(testPath))
+            .rejects.toThrow('Access denied - path outside allowed directories');
+        });
+
+        it('accepts UNC file path when allowed UNC has trailing separator', async () => {
+          if (process.platform !== 'win32') return;
+          setAllowedDirectories(['\\\\server\\share\\project\\']);
+          const testPath = '\\\\server\\share\\project\\file.txt';
+          const result = await validatePath(testPath);
+          expect(result).toBe(testPath);
+        });
+      });
     });
   });
 

src/filesystem/__tests__/path-validation.test.ts

@@ -439,6 +439,68 @@ describe('Path Validation', () => {
         expect(isPathWithinAllowedDirectories('\\\\other\\share\\project', allowed)).toBe(false);
       }
     });
+
+    it('allows UNC path within allowed UNC directory', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project'];
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project\\subdir\\file.txt', allowed)).toBe(true);
+      }
+    });
+
+    it('blocks UNC path outside allowed UNC directory', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project'];
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\other', allowed)).toBe(false);
+        expect(isPathWithinAllowedDirectories('\\\\other\\share\\project', allowed)).toBe(false);
+      }
+    });
+
+    it('allows exact UNC path match', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project'];
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project', allowed)).toBe(true);
+      }
+    });
+
+    // Tests for UNC trailing-slash regression (bug #2 in the companion PR):
+    // path.normalize preserves trailing separators on UNC paths on Windows,
+    // while path.resolve strips them for drive paths. Without harmonization,
+    // the `normalizedDir + path.sep` comparison produces a double separator
+    // that startsWith() never matches, making every access fail even when
+    // the allowed directory is configured with a trailing backslash.
+    it('allows file inside allowed UNC directory with trailing separator', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project\\'];
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project\\file.txt', allowed)).toBe(true);
+      }
+    });
+
+    it('allows subdirectories inside allowed UNC directory with trailing separator', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project\\'];
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project\\src\\index.ts', allowed)).toBe(true);
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project\\deeply\\nested\\file', allowed)).toBe(true);
+      }
+    });
+
+    it('allows exact match against allowed UNC directory with trailing separator', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project\\'];
+        // Path without trailing separator
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project', allowed)).toBe(true);
+        // Path with trailing separator
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project\\', allowed)).toBe(true);
+      }
+    });
+
+    it('blocks sibling UNC paths with common prefix when allowed has trailing separator', () => {
+      if (path.sep === '\\') {
+        const allowed = ['\\\\server\\share\\project\\'];
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project2', allowed)).toBe(false);
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\project_backup', allowed)).toBe(false);
+        expect(isPathWithinAllowedDirectories('\\\\server\\share\\projectile', allowed)).toBe(false);
+      }
+    });
   });
 
   describe('Symlink Tests', () => {

src/filesystem/index.ts

@@ -45,7 +45,10 @@ if (args.length === 0) {
 let allowedDirectories = (await Promise.all(
   args.map(async (dir) => {
     const expanded = expandHome(dir);
-    const absolute = path.resolve(expanded);
+    // UNC-aware: path.resolve corrupts UNC paths (\\server\share becomes
+    // C:\server\share on Windows). Skip it for UNC paths and let normalizePath
+    // handle them. Same fix applied in lib.ts validatePath.
+    const absolute = expanded.startsWith('\\\\') ? expanded : path.resolve(expanded);
     const normalizedOriginal = normalizePath(absolute);
     try {
       // Security: Resolve symlinks in allowed directories during startup

src/filesystem/lib.ts

@@ -98,8 +98,14 @@ function resolveRelativePathAgainstAllowedDirectories(relativePath: string): str
 // Security & Validation Functions
 export async function validatePath(requestedPath: string): Promise<string> {
   const expandedPath = expandHome(requestedPath);
+  // UNC-aware resolution: path.resolve() on Windows corrupts UNC paths
+  // (\\server\share becomes C:\server\share via path.normalize stripping
+  // a leading backslash, then path.resolve treating it as drive-relative).
+  // For UNC paths we skip path.resolve entirely and let normalizePath handle
+  // them. PR #3921 only patched isPathWithinAllowedDirectories; validatePath
+  // needs the same treatment upstream of it.
   const absolute = path.isAbsolute(expandedPath)
-    ? path.resolve(expandedPath)
+    ? (expandedPath.startsWith('\\\\') ? expandedPath : path.resolve(expandedPath))
     : resolveRelativePathAgainstAllowedDirectories(expandedPath);
 
   const normalizedRequested = normalizePath(absolute);

src/filesystem/path-validation.ts

@@ -1,8 +1,42 @@
 import path from 'path';
 
+/**
+ * Checks if a path is a UNC path (e.g. \\server\share).
+ */
+function isUNCPath(p: string): boolean {
+  return p.startsWith('\\\\');
+}
+
+/**
+ * Normalizes a path that may be a UNC path on Windows.
+ *
+ * On Windows, path.normalize can strip one leading backslash from UNC paths
+ * (e.g. \\server\share becomes \server\share), and then path.resolve
+ * interprets it as a drive-relative path (e.g. C:\server\share). This
+ * function preserves the UNC prefix through normalization.
+ */
+function normalizePossiblyUNCPath(p: string): string {
+  if (isUNCPath(p)) {
+    let normalized = path.normalize(p);
+    // path.normalize may strip a leading backslash from UNC paths
+    if (!normalized.startsWith('\\\\')) {
+      normalized = '\\' + normalized;
+    }
+    // path.normalize preserves trailing separators for UNC paths on Windows,
+    // while path.resolve (used for non-UNC paths below) strips them. Harmonize
+    // here: without stripping, `normalizedDir + path.sep` in the caller produces
+    // a double separator that startsWith() never matches.
+    if (normalized.length > 2 && normalized.endsWith(path.sep)) {
+      normalized = normalized.slice(0, -1);
+    }
+    return normalized;
+  }
+  return path.resolve(path.normalize(p));
+}
+
 /**
  * Checks if an absolute path is within any of the allowed directories.
- * 
+ *
  * @param absolutePath - The absolute path to check (will be normalized)
  * @param allowedDirectories - Array of absolute allowed directory paths (will be normalized)
  * @returns true if the path is within an allowed directory, false otherwise
@@ -27,7 +61,7 @@ export function isPathWithinAllowedDirectories(absolutePath: string, allowedDire
   // Normalize the input path
   let normalizedPath: string;
   try {
-    normalizedPath = path.resolve(path.normalize(absolutePath));
+    normalizedPath = normalizePossiblyUNCPath(absolutePath);
   } catch {
     return false;
   }
@@ -51,7 +85,7 @@ export function isPathWithinAllowedDirectories(absolutePath: string, allowedDire
     // Normalize the allowed directory
     let normalizedDir: string;
     try {
-      normalizedDir = path.resolve(path.normalize(dir));
+      normalizedDir = normalizePossiblyUNCPath(dir);
     } catch {
       return false;
     }