fix(security): handle broken symlinks in validatePath to close write-outside-allowlist gap

When fs.realpath() fails with ENOENT, the previous fallback resolved the parent
directory and rejoined the basename (the symlink name itself). Since fs.writeFile()
follows symlinks, a broken symlink inside an allowed directory could redirect a write
to an arbitrary path outside the allowlist.

Fix: before falling back to parent-directory resolution, use fs.lstat() to detect
whether the unreachable path is a symlink. If it is, read the symlink target with
fs.readlink() and resolve it to an absolute canonical path so that the allowlist check
and all subsequent file operations use the actual destination, not the symlink entry.

Also tighten the ancestor-walk loop to stop on non-ENOENT errors (e.g. EPERM) rather
than silently swallowing them, preventing a degraded-resolution bypass on restricted
directory trees.

Closes coderabbitai review comment on PR #398.

Author: sorlen008

src/tools/filesystem.ts

@@ -245,32 +245,61 @@ export async function validatePath(requestedPath: string): Promise<string> {
                 throw new Error(`Failed to resolve symlink for path: ${absoluteOriginal}. Error: ${err.message}`);
             }
 
+            // SECURITY FIX: Check if the path is a broken symlink (symlink whose target
+            // doesn't exist yet). fs.realpath() fails with ENOENT for broken symlinks, but
+            // fs.lstat() succeeds because the symlink itself exists.
+            // Without this check, the parent-directory fallback below would return the
+            // symlink name rather than its target, allowing fs.writeFile() (which follows
+            // symlinks) to write outside the allowlist.
+            try {
+                const lstat = await fs.lstat(absoluteOriginal);
+                if (lstat.isSymbolicLink()) {
+                    // Read the raw symlink target and resolve it to an absolute path
+                    const linkTarget = await fs.readlink(absoluteOriginal);
+                    const resolvedTarget = path.isAbsolute(linkTarget)
+                        ? path.resolve(linkTarget)
+                        : path.resolve(path.dirname(absoluteOriginal), linkTarget);
+                    // Use the resolved target as the canonical path for allowlist checking
+                    // and as the actual path for all subsequent file operations.
+                    resolvedRealPath = resolvedTarget;
+                }
+            } catch {
+                // Not a symlink or unreadable; fall through to parent-directory resolution
+            }
+
             // SECURITY FIX: When the full path doesn't exist (e.g., writing a new file),
             // resolve the parent directory to detect symlinks in the path chain.
             // Without this, an attacker could create a symlink inside an allowed directory
             // pointing to a restricted location, then write to a non-existent file through
             // that symlink — bypassing the directory restriction check.
-            try {
-                const parentDir = path.dirname(absoluteOriginal);
-                const resolvedParent = await fs.realpath(parentDir, { encoding: 'utf8' });
-                const basename = path.basename(absoluteOriginal);
-                resolvedRealPath = path.join(resolvedParent, basename);
-            } catch {
-                // Parent also doesn't exist — walk up the tree to find
-                // the deepest existing ancestor and resolve it
-                let current = absoluteOriginal;
-                let remaining: string[] = [];
-                while (true) {
-                    const parent = path.dirname(current);
-                    if (parent === current) break; // reached filesystem root
-                    remaining.unshift(path.basename(current));
-                    current = parent;
-                    try {
-                        const resolvedAncestor = await fs.realpath(current, { encoding: 'utf8' });
-                        resolvedRealPath = path.join(resolvedAncestor, ...remaining);
-                        break;
-                    } catch {
-                        // keep walking up
+            if (resolvedRealPath === null) {
+                try {
+                    const parentDir = path.dirname(absoluteOriginal);
+                    const resolvedParent = await fs.realpath(parentDir, { encoding: 'utf8' });
+                    const basename = path.basename(absoluteOriginal);
+                    resolvedRealPath = path.join(resolvedParent, basename);
+                } catch {
+                    // Parent also doesn't exist — walk up the tree to find
+                    // the deepest existing ancestor and resolve it
+                    let current = absoluteOriginal;
+                    let remaining: string[] = [];
+                    while (true) {
+                        const parent = path.dirname(current);
+                        if (parent === current) break; // reached filesystem root
+                        remaining.unshift(path.basename(current));
+                        current = parent;
+                        try {
+                            const resolvedAncestor = await fs.realpath(current, { encoding: 'utf8' });
+                            resolvedRealPath = path.join(resolvedAncestor, ...remaining);
+                            break;
+                        } catch (walkErr) {
+                            const walkError = walkErr as NodeJS.ErrnoException;
+                            // Non-ENOENT error (e.g., EPERM) means we cannot safely
+                            // resolve this ancestor — stop walking to avoid returning
+                            // an unresolved path that could bypass allowlist checks.
+                            if (walkError.code && walkError.code !== 'ENOENT') break;
+                            // ENOENT: keep walking up
+                        }
                     }
                 }
             }