From 203aa09d2e4420a05550f4f966a3708b59434a26 Mon Sep 17 00:00:00 2001 From: BillionClaw Date: Tue, 17 Mar 2026 22:43:57 +0800 Subject: [PATCH] fix(filesystem): preserve UNC paths in access validation On Windows, path.resolve() converts UNC paths like \server\share to C:\server\share, corrupting the path and breaking access checks for network shares. Added normalizePathSafe() to detect and preserve UNC paths before normalization, ensuring subdirectories under UNC shares are correctly validated as being within allowed directories. Fixes #3527 --- .../__tests__/path-validation.test.ts | 34 +++++++++++++ src/filesystem/path-validation.ts | 51 +++++++++++++++++-- 2 files changed, 81 insertions(+), 4 deletions(-) diff --git a/src/filesystem/__tests__/path-validation.test.ts b/src/filesystem/__tests__/path-validation.test.ts index 81ad247ee2..438bfb4e71 100644 --- a/src/filesystem/__tests__/path-validation.test.ts +++ b/src/filesystem/__tests__/path-validation.test.ts @@ -439,6 +439,40 @@ describe('Path Validation', () => { expect(isPathWithinAllowedDirectories('\\\\other\\share\\project', allowed)).toBe(false); } }); + + it('allows UNC path subdirectories (GitHub issue #3527)', () => { + // This test verifies that UNC paths like \\server\share\subdir are correctly + // recognized as being within allowed directories like \\server\share. + // The bug was that path.resolve() on Windows corrupts UNC paths by prepending + // the current drive letter (e.g., \\server\share becomes C:\server\share). + + // Test exact match of UNC share root + const allowedRoot = ['\\\\192.168.4.96\\Mega']; + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Mega', allowedRoot)).toBe(true); + + // Test subdirectories under UNC share - this was failing before the fix + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Mega\\Drops', allowedRoot)).toBe(true); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Mega\\00R RPGs', allowedRoot)).toBe(true); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Mega\\deeply\\nested\\path', allowedRoot)).toBe(true); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Mega\\file.txt', allowedRoot)).toBe(true); + + // Test that different shares on same server are blocked + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Plex', allowedRoot)).toBe(false); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Plex\\Movies', allowedRoot)).toBe(false); + + // Test that different servers are blocked + expect(isPathWithinAllowedDirectories('\\\\otherserver\\share\\path', allowedRoot)).toBe(false); + + // Test with multiple UNC allowed directories + const allowedMultiple = ['\\\\192.168.4.96\\Mega', '\\\\192.168.4.96\\Plex']; + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Mega\\Drops', allowedMultiple)).toBe(true); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\Plex\\Movies', allowedMultiple)).toBe(true); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\OtherShare\\file', allowedMultiple)).toBe(false); + + // Test UNC path prefix attacks are blocked + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\MegaBackup', allowedRoot)).toBe(false); + expect(isPathWithinAllowedDirectories('\\\\192.168.4.96\\MegaExtra\\file', allowedRoot)).toBe(false); + }); }); describe('Symlink Tests', () => { diff --git a/src/filesystem/path-validation.ts b/src/filesystem/path-validation.ts index 972e9c49d0..ce5c8778e8 100644 --- a/src/filesystem/path-validation.ts +++ b/src/filesystem/path-validation.ts @@ -1,5 +1,33 @@ import path from 'path'; +/** + * Normalizes a path without corrupting UNC paths. + * On Windows, path.resolve() converts UNC paths like \\server\share to C:\server\share, + * which breaks UNC path handling. This function preserves UNC paths. + * + * @param p - The path to normalize + * @returns Normalized path + */ +function normalizePathSafe(p: string): string { + // Check if it's a UNC path (starts with \\ or //) BEFORE normalizing + // We must check this first because path.normalize on macOS corrupts UNC paths + if (p.startsWith('\\\\') || p.startsWith('//')) { + // UNC paths: normalize slashes to backslashes and remove redundant slashes + // but preserve the leading \\ + let normalized = p.replace(/\//g, '\\'); + // Ensure exactly two leading backslashes (not more, not less) + normalized = normalized.replace(/^\\+/, '\\\\'); + // Normalize any double backslashes in the rest of the path to single + // but be careful not to break the leading \\ + const rest = normalized.substring(2).replace(/\\+/g, '\\'); + return '\\\\' + rest; + } + + // For non-UNC paths, use path.normalize then path.resolve + const normalized = path.normalize(p); + return path.resolve(normalized); +} + /** * Checks if an absolute path is within any of the allowed directories. * @@ -27,13 +55,15 @@ export function isPathWithinAllowedDirectories(absolutePath: string, allowedDire // Normalize the input path let normalizedPath: string; try { - normalizedPath = path.resolve(path.normalize(absolutePath)); + normalizedPath = normalizePathSafe(absolutePath); } catch { return false; } // Verify it's absolute after normalization - if (!path.isAbsolute(normalizedPath)) { + // UNC paths are always absolute, as are resolved paths + const isUncPath = normalizedPath.startsWith('\\\\'); + if (!isUncPath && !path.isAbsolute(normalizedPath)) { throw new Error('Path must be absolute after normalization'); } @@ -51,13 +81,14 @@ export function isPathWithinAllowedDirectories(absolutePath: string, allowedDire // Normalize the allowed directory let normalizedDir: string; try { - normalizedDir = path.resolve(path.normalize(dir)); + normalizedDir = normalizePathSafe(dir); } catch { return false; } // Verify allowed directory is absolute after normalization - if (!path.isAbsolute(normalizedDir)) { + const isUncDir = normalizedDir.startsWith('\\\\'); + if (!isUncDir && !path.isAbsolute(normalizedDir)) { throw new Error('Allowed directories must be absolute paths after normalization'); } @@ -81,6 +112,18 @@ export function isPathWithinAllowedDirectories(absolutePath: string, allowedDire return pathDrive === dirDrive && normalizedPath.startsWith(normalizedDir.replace(/\\?$/, '\\')); } + // Special handling for UNC paths + // Both paths must be UNC paths for a valid match + if (isUncPath && isUncDir) { + // For UNC paths, use backslash as separator + return normalizedPath.startsWith(normalizedDir + '\\'); + } + + // If one is UNC and the other isn't, they can't match + if (isUncPath !== isUncDir) { + return false; + } + return normalizedPath.startsWith(normalizedDir + path.sep); }); }