Fix allowlist rejecting paths under symlinked directories (#590)

src/tools/filesystem.ts

@@ -168,6 +168,41 @@ async function validateParentDirectories(directoryPath: string): Promise<boolean
     }
 }
 
+/**
+ * Resolves a path to its canonical form, following symlinks.
+ *
+ * Mirrors how validatePath() canonicalizes the requested path: if the path
+ * exists its realpath is returned, otherwise the deepest existing ancestor is
+ * resolved and the remaining segments are re-appended. This lets a configured
+ * allowlist entry that has not been created yet still be compared on equal
+ * terms with an already-resolved requested path. Best-effort: if nothing along
+ * the chain resolves, the input is returned unchanged.
+ *
+ * @param absolutePath An absolute path to canonicalize
+ * @returns Promise<string> The canonical path
+ */
+async function canonicalizePath(absolutePath: string): Promise<string> {
+    try {
+        return await fs.realpath(absolutePath, { encoding: 'utf8' });
+    } catch {
+        let current = absolutePath;
+        const remaining: string[] = [];
+        while (true) {
+            const parent = path.dirname(current);
+            if (parent === current) break; // reached the filesystem root
+            remaining.unshift(path.basename(current));
+            current = parent;
+            try {
+                const resolvedAncestor = await fs.realpath(current, { encoding: 'utf8' });
+                return path.join(resolvedAncestor, ...remaining);
+            } catch {
+                // keep walking up until an existing ancestor is found
+            }
+        }
+        return absolutePath;
+    }
+}
+
 /**
  * Checks if a path is within any of the allowed directories
  *
@@ -186,8 +221,23 @@ async function isPathAllowed(pathToCheck: string): Promise<boolean> {
         normalizedPathToCheck = normalizedPathToCheck.slice(0, -1);
     }
 
+    // Canonicalize the configured allowlist entries the same way validatePath()
+    // canonicalizes the requested path before it reaches here. On macOS
+    // fs.realpath("/tmp/x") resolves to "/private/tmp/x"; if the allowlist entry
+    // is left unresolved the resolved requested path can never match its own
+    // configured directory and every access is rejected (#590).
+    const canonicalAllowedDirs = await Promise.all(
+        allowedDirectories.map((allowedDir) => {
+            const expanded = expandHome(allowedDir);
+            const absolute = path.isAbsolute(expanded)
+                ? path.resolve(expanded)
+                : path.resolve(process.cwd(), expanded);
+            return canonicalizePath(absolute);
+        })
+    );
+
     // Check if the path is within any allowed directory
-    const isAllowed = allowedDirectories.some(allowedDir => {
+    const isAllowed = canonicalAllowedDirs.some(allowedDir => {
         let normalizedAllowedDir = normalizePath(allowedDir);
         if (normalizedAllowedDir.slice(-1) === path.sep) {
             normalizedAllowedDir = normalizedAllowedDir.slice(0, -1);

test/test-allowed-directories-realpath.js

@@ -0,0 +1,105 @@
+import assert from 'assert';
+import os from 'os';
+import path from 'path';
+import fsp from 'fs/promises';
+import { validatePath } from '../dist/tools/filesystem.js';
+import { configManager } from '../dist/config-manager.js';
+
+/**
+ * Regression test for #590: a configured allowlist entry that points at a
+ * symlink was rejected for its own children.
+ *
+ * validatePath() canonicalizes the requested path with fs.realpath (so on macOS
+ * "/tmp/x" becomes "/private/tmp/x"), but isPathAllowed() used to compare that
+ * against the raw, unresolved allowlist entry. The two never matched and every
+ * access to the allowed directory failed.
+ *
+ * This reproduces the mismatch portably with an explicit symlink instead of the
+ * macOS /tmp alias: the allowlist entry is the symlink, the requested paths
+ * resolve to its target. If directory symlinks cannot be created (Windows
+ * without the privilege), the test skips rather than failing.
+ */
+
+let passed = 0;
+const ok = (msg) => { passed++; console.log(`✓ ${msg}`); };
+
+async function run() {
+  const base = await fsp.realpath(await fsp.mkdtemp(path.join(os.tmpdir(), 'dc-allow-realpath-')));
+  const target = path.join(base, 'real-target');
+  const link = path.join(base, 'link');
+  const outside = path.join(base, 'outside');
+  await fsp.mkdir(target);
+  await fsp.mkdir(outside);
+
+  try {
+    await fsp.symlink(target, link, 'dir');
+  } catch (e) {
+    // Only skip when the platform genuinely won't create the symlink; a real
+    // setup error should fail the test rather than masquerade as a skip.
+    const skippableCodes = new Set(['EPERM', 'EACCES', 'ENOTSUP', 'ENOSYS']);
+    if (!skippableCodes.has(e.code)) {
+      await fsp.rm(base, { recursive: true, force: true });
+      throw e;
+    }
+    console.log(`SKIP: cannot create directory symlink on this platform (${e.code})`);
+    await fsp.rm(base, { recursive: true, force: true });
+    return 'skipped';
+  }
+
+  const original = await configManager.getConfig();
+  const originalAllowed = original.allowedDirectories;
+  try {
+    // Allowlist the symlink itself, not its resolved target.
+    await configManager.setValue('allowedDirectories', [link]);
+
+    const resolvedTarget = await fsp.realpath(target);
+
+    // 1) The allowed directory itself validates and resolves to the target.
+    {
+      const validated = await validatePath(link);
+      assert.strictEqual(await fsp.realpath(validated), resolvedTarget,
+        'allowed symlink directory should validate and resolve to its target');
+      ok('allowlisted symlink directory is accepted');
+    }
+
+    // 2) An existing child under the symlink validates (read/list/search path).
+    {
+      const childFile = path.join(link, 'child.txt');
+      await fsp.writeFile(path.join(target, 'child.txt'), 'hello');
+      const validated = await validatePath(childFile);
+      assert.strictEqual(await fsp.realpath(validated), path.join(resolvedTarget, 'child.txt'),
+        'existing child under the allowed symlink should validate');
+      ok('existing child under the allowlisted symlink is accepted');
+    }
+
+    // 3) A not-yet-created child under the symlink validates (write path).
+    {
+      const newChild = path.join(link, 'new-file.txt');
+      const validated = await validatePath(newChild);
+      assert.strictEqual(await fsp.realpath(path.dirname(validated)), resolvedTarget,
+        'new child under the allowed symlink should validate inside the target itself');
+      ok('not-yet-created child under the allowlisted symlink is accepted');
+    }
+
+    // 4) A path outside the allowed directory is still rejected — the fix must
+    //    not widen access beyond the configured entry.
+    {
+      await assert.rejects(
+        () => validatePath(path.join(outside, 'secret.txt')),
+        /Path not allowed/,
+        'paths outside the allowed directory must still be rejected');
+      ok('path outside the allowlisted directory is still rejected');
+    }
+  } finally {
+    await configManager.setValue('allowedDirectories', originalAllowed);
+    await fsp.rm(base, { recursive: true, force: true });
+  }
+}
+
+run()
+  .then((result) => {
+    if (result === 'skipped') { console.log('\nSKIPPED'); process.exit(0); }
+    console.log(`\nPASS (${passed}/4)`);
+    process.exit(0);
+  })
+  .catch((e) => { console.error(`\nFAIL: ${e.message}`); process.exit(1); });