refactor: make validatePath return resolved url (#2572)

This changes the internal responsibility: validatePath is the only place
that is expected to follow symlinks if it can. It then returns a
resolved path for all consumers in the MCP server who are expected not
to follow symlinks.

This is to be combined with the followSymlinks=false setting in
https://github.com/puppeteer/puppeteer/pull/15335

src/McpContext.ts

@@ -225,21 +225,18 @@ export class McpContext implements Context {
     this.#roots = roots;
   }
 
-  async validatePath(filePath?: string): Promise<void> {
+  /**
+   * Validates that the filePath is allowed according to the roots configuration.
+   * Tolerates if parts of the filePath do not exist yet but the file access to
+   * the resolved should only be allowed without following symlinks.
+   */
+  async validatePath(filePath: string): Promise<string>;
+  async validatePath(filePath?: undefined): Promise<undefined>;
+  async validatePath(filePath?: string): Promise<string | undefined>;
+  async validatePath(filePath?: string): Promise<string | undefined> {
     if (filePath === undefined) {
-      return;
+      return undefined;
     }
-    // If the client never negotiated roots and the operator has explicitly
-    // opted into unrestricted access via --allow-unrestricted-paths, restore
-    // the previous permissive behavior and skip validation.
-    if (this.#roots === undefined && this.#allowUnrestrictedPaths) {
-      return;
-    }
-    // roots() always returns at least the temp directory, even if the
-    // connecting client never negotiated the optional `roots` capability.
-    // Path validation must not be skipped just because no workspace roots
-    // were configured.
-    const roots = this.roots();
 
     let canonicalPath: string;
 
@@ -255,6 +252,20 @@ export class McpContext implements Context {
       );
     }
 
+    // If the client never negotiated roots and the operator has explicitly
+    // opted into unrestricted access via --allow-unrestricted-paths, restore
+    // the previous permissive behavior and skip validation.
+    if (this.#roots === undefined && this.#allowUnrestrictedPaths) {
+      // Canonical path might not exist yet so we fallback to
+      // path.resolve(filePath). Consumers should not follow symlinks.
+      return canonicalPath || path.resolve(filePath);
+    }
+    // roots() always returns at least the temp directory, even if the
+    // connecting client never negotiated the optional `roots` capability.
+    // Path validation must not be skipped just because no workspace roots
+    // were configured.
+    const roots = this.roots();
+
     let allowed = false;
     const resolvedRoots = await Promise.allSettled(
       roots.map(async root => {
@@ -293,19 +304,20 @@ export class McpContext implements Context {
         `Access denied: path ${filePath} (canonical: ${canonicalPath}) is not within any of the configured workspace roots.`,
       );
     }
+
+    return canonicalPath || path.resolve(filePath);
   }
 
   async ensureExtension<Extension extends `.${string}`>(
     filePath: string,
     extension: Extension,
   ): Promise<`${string}${Extension}`> {
-    const resolvedPath = path.resolve(filePath);
-    const currentExtension = path.extname(resolvedPath);
-    const outputPath: `${string}${Extension}` = `${resolvedPath.slice(
+    const resolved = await this.validatePath(filePath);
+    const currentExtension = path.extname(resolved);
+    const outputPath: `${string}${Extension}` = `${resolved.slice(
       0,
-      resolvedPath.length - currentExtension.length,
+      resolved.length - currentExtension.length,
     )}${extension}`;
-    await this.validatePath(outputPath);
     return outputPath;
   }
 
@@ -624,17 +636,17 @@ export class McpContext implements Context {
     filepath: string,
     data: Uint8Array<ArrayBufferLike>,
   ): Promise<void> {
-    await this.validatePath(filepath);
+    const resolved = await this.validatePath(filepath);
 
     try {
-      await fs.mkdir(path.dirname(filepath), {recursive: true});
+      await fs.mkdir(path.dirname(resolved), {recursive: true});
       // Open the file with flags to:
       // - O_WRONLY: Write-only
       // - O_CREAT: Create if it doesn't exist
       // - O_TRUNC: Truncate to zero length if it exists
       // - O_NOFOLLOW: DO NOT follow symlinks.
       // - 0o600: Permissions: read/write for owner, no permissions for others.
-      await fs.writeFile(filepath, data, {
+      await fs.writeFile(resolved, data, {
         flag:
           fs.constants.O_WRONLY |
           fs.constants.O_CREAT |
@@ -866,8 +878,8 @@ export class McpContext implements Context {
       }
 
       case 'file:': {
-        await this.validatePath(fileURLToPath(url));
-        return await fs.readFile(url, 'utf-8');
+        const resolved = await this.validatePath(fileURLToPath(url));
+        return await fs.readFile(resolved, 'utf-8');
       }
 
       default:

src/ToolHandler.ts

@@ -25,7 +25,7 @@ import type {
 import {pageIdSchema} from './tools/ToolDefinition.js';
 import {logger} from './utils/logger.js';
 import type {Mutex} from './third_party/index.js';
-import {fileURLToPath} from 'node:url';
+import {fileURLToPath, pathToFileURL} from 'node:url';
 import {isLocalhost} from './utils/url.js';
 
 export function buildFlag(category: ToolCategory) {
@@ -151,14 +151,21 @@ function buildUnknownArgumentsMessage(
   return `Unknown ${unknownLabel} for tool "${toolName}": ${formatArgumentNames(unknownArgumentNames)}. ${expectedArguments} ${correction} and retry.`;
 }
 
-function extractPaths(value: unknown): string[] {
-  if (typeof value === 'string') {
-    return [value];
+async function validateAndResolvePathOrUrl(
+  filePathOrUrl: string,
+  context: McpContext,
+): Promise<string> {
+  try {
+    const url = new URL(filePathOrUrl);
+    if (url.protocol === 'file:') {
+      return pathToFileURL(await context.validatePath(fileURLToPath(url))).href;
+    } else if (['http:', 'https:', 'ws:', 'wss:'].includes(url.protocol)) {
+      return filePathOrUrl;
+    }
+  } catch {
+    // Suppress parsing errors for regular file paths.
   }
-  if (Array.isArray(value)) {
-    return value.filter(item => typeof item === 'string');
-  }
-  return [];
+  return await context.validatePath(filePathOrUrl);
 }
 
 function isLocalBrowser(context: McpContext): boolean {
@@ -194,25 +201,25 @@ async function validateToolFiles(
   context: McpContext,
 ): Promise<void> {
   const isLocal = isLocalBrowser(context);
-  const pathsOrUrlsToValidate: string[] = [];
   for (const [key, option] of Object.entries(tool.verifyFilesSchema)) {
     if (shouldValidateFile(option, isLocal)) {
-      pathsOrUrlsToValidate.push(...extractPaths(params[key]));
-    }
-  }
-  for (const filePathOrUrl of pathsOrUrlsToValidate) {
-    let filePath = filePathOrUrl;
-    try {
-      const url = new URL(filePathOrUrl);
-      if (url.protocol === 'file:') {
-        filePath = fileURLToPath(url);
-      } else if (['http:', 'https:', 'ws:', 'wss:'].includes(url.protocol)) {
-        continue;
+      const val = params[key];
+      if (typeof val === 'string') {
+        params[key] = await validateAndResolvePathOrUrl(val, context);
+      } else if (Array.isArray(val)) {
+        const updated: unknown[] = [];
+        for (const item of val) {
+          if (typeof item === 'string') {
+            updated.push(await validateAndResolvePathOrUrl(item, context));
+          } else {
+            throw new Error(
+              'Unexpected non-string value as a file path or URL',
+            );
+          }
+        }
+        params[key] = updated;
       }
-    } catch {
-      // Suppress parsing errors for regular file paths.
     }
-    await context.validatePath(filePath);
   }
 }
 

src/tools/extensions.ts

@@ -23,7 +23,7 @@ export const installExtension = defineTool({
   },
   blockedByDialog: false,
   verifyFilesSchema: {
-    path: true,
+    path: {local: true},
   },
   handler: async (request, response, context) => {
     const {path} = request.params;

tests/McpContext.test.ts

@@ -20,6 +20,7 @@ import {McpPage} from '../src/McpPage.js';
 import {TextSnapshot} from '../src/TextSnapshot.js';
 import {type HTTPResponse} from '../src/third_party/index.js';
 import type {TraceResult} from '../src/processors/PerformanceTrace.js';
+import {resolveCanonicalPath} from '../src/utils/files.js';
 
 import {
   getMockRequest,
@@ -418,8 +419,14 @@ describe('McpContext', () => {
         ];
         context.setRoots(roots);
         // Valid path within root
-        await context.validatePath(path.join(workspacePath, 'test.txt'));
-        await context.validatePath(workspacePath);
+        const targetPath = path.join(workspacePath, 'test.txt');
+        const resolved = await context.validatePath(targetPath);
+        assert.strictEqual(resolved, await resolveCanonicalPath(targetPath));
+        const resolvedWorkspace = await context.validatePath(workspacePath);
+        assert.strictEqual(
+          resolvedWorkspace,
+          await resolveCanonicalPath(workspacePath),
+        );
 
         // Invalid path outside root and outside temp dir
         const outsidePath = path.resolve(os.homedir(), 'outside-test.txt');
@@ -443,9 +450,9 @@ describe('McpContext', () => {
         ];
         context.setRoots(roots);
         // Valid path within root with non-existent intermediate directories
-        await context.validatePath(
-          path.join(workspacePath, 'dir1', 'dir2', 'test.txt'),
-        );
+        const targetPath = path.join(workspacePath, 'dir1', 'dir2', 'test.txt');
+        const resolved = await context.validatePath(targetPath);
+        assert.strictEqual(resolved, await resolveCanonicalPath(targetPath));
       } finally {
         await fs.rm(workspacePath, {recursive: true, force: true});
       }
@@ -456,19 +463,42 @@ describe('McpContext', () => {
     await withMcpContext(
       async (_response, context) => {
         context.setRoots(undefined);
-        await context.validatePath(path.resolve(os.homedir(), 'anywhere.txt'));
+        const targetPath = path.resolve(os.homedir(), 'anywhere.txt');
+        const resolved = await context.validatePath(targetPath);
+        assert.strictEqual(resolved, await resolveCanonicalPath(targetPath));
       },
       {allowUnrestrictedPaths: true},
     );
   });
 
+  it('validatePath returns undefined if filePath is undefined', async () => {
+    await withMcpContext(async (_response, context) => {
+      const resolved = await context.validatePath(undefined);
+      assert.strictEqual(resolved, undefined);
+    });
+  });
+
+  it('validatePath returns resolved absolute path for relative paths', async () => {
+    await withMcpContext(async (_response, context) => {
+      const tmpDir = os.tmpdir();
+      const relativeTmpPath = path.relative(
+        process.cwd(),
+        path.join(tmpDir, 'test.txt'),
+      );
+      const resolved = await context.validatePath(relativeTmpPath);
+      assert.strictEqual(resolved, await resolveCanonicalPath(relativeTmpPath));
+    });
+  });
+
   it('validatePath denies paths outside tmpdir if roots are undefined and allowUnrestrictedPaths is not set', async () => {
     await withMcpContext(async (_response, context) => {
       // setRoots() never called — simulates a client that skips roots capability.
       const outsidePath = path.resolve(os.homedir(), 'anywhere.txt');
       await assert.rejects(context.validatePath(outsidePath), /Access denied/);
       // Temp dir must still be reachable.
-      await context.validatePath(path.join(os.tmpdir(), 'test.txt'));
+      const tmpPath = path.join(os.tmpdir(), 'test.txt');
+      const resolved = await context.validatePath(tmpPath);
+      assert.strictEqual(resolved, await resolveCanonicalPath(tmpPath));
     });
   });
 
@@ -476,7 +506,9 @@ describe('McpContext', () => {
     await withMcpContext(async (_response, context) => {
       context.setRoots([]);
       // Should allow temp dir
-      await context.validatePath(path.join(os.tmpdir(), 'test.txt'));
+      const tmpPath = path.join(os.tmpdir(), 'test.txt');
+      const resolved = await context.validatePath(tmpPath);
+      assert.strictEqual(resolved, await resolveCanonicalPath(tmpPath));
 
       // Should deny outside temp dir
       await assert.rejects(
@@ -492,7 +524,35 @@ describe('McpContext', () => {
       return;
     }
 
-    it('saveFile refuses to write through a symlink to an existing file', async () => {
+    it('validatePath resolves symlinks and returns the canonical path', async () => {
+      await withMcpContext(async (_response, context) => {
+        const tmpDir = await fs.mkdtemp(
+          path.join(os.tmpdir(), 'validate-symlink-test-'),
+        );
+        try {
+          const targetDir = path.join(tmpDir, 'target');
+          await fs.mkdir(targetDir);
+          const targetFile = path.join(targetDir, 'file.txt');
+          await fs.writeFile(targetFile, 'hello');
+
+          const symlinkDir = path.join(tmpDir, 'symlink_dir');
+          await fs.symlink(targetDir, symlinkDir, 'dir');
+
+          const canonicalTarget = await fs.realpath(targetDir);
+          context.setRoots([
+            {uri: pathToFileURL(canonicalTarget).href, name: 'target'},
+          ]);
+
+          const filePathWithSymlink = path.join(symlinkDir, 'file.txt');
+          const resolved = await context.validatePath(filePathWithSymlink);
+          assert.strictEqual(resolved, path.join(canonicalTarget, 'file.txt'));
+        } finally {
+          await fs.rm(tmpDir, {recursive: true, force: true});
+        }
+      });
+    });
+
+    it('saveFile allows writing to a symlinked file if it resolves to an allowed path', async () => {
       await withMcpContext(async (_response, context) => {
         const tmpDir = await fs.mkdtemp(
           path.join(os.tmpdir(), 'mcp-symlink-test-'),
@@ -506,14 +566,12 @@ describe('McpContext', () => {
           const symlinkPath = path.join(tmpDir, 'symlink.txt');
           await fs.symlink(targetPath, symlinkPath);
 
-          const data = new TextEncoder().encode('malicious content');
-          await assert.rejects(
-            context.saveFile(data, symlinkPath, '.txt'),
-            /Could not write/,
-          );
+          const data = new TextEncoder().encode('content');
+          await context.saveFile(data, symlinkPath, '.txt');
+          await context.saveFile(data, targetPath, '.txt');
 
           const content = await fs.readFile(targetPath, 'utf-8');
-          assert.strictEqual(content, 'original content');
+          assert.strictEqual(content, 'content');
         } finally {
           await fs.rm(tmpDir, {recursive: true, force: true});
         }
@@ -567,8 +625,10 @@ describe('McpContext', () => {
           const data = new TextEncoder().encode('allowed content');
 
           const result = await context.saveFile(data, targetFilePath, '.txt');
-          assert.strictEqual(result.filename, targetFilePath);
-
+          assert.strictEqual(
+            result.filename,
+            await resolveCanonicalPath(targetFilePath),
+          );
           const content = await fs.readFile(
             path.join(realDir, 'test.txt'),
             'utf-8',

tests/ToolHandler.test.ts

@@ -312,8 +312,9 @@ describe('ToolHandler', () => {
     assert.strictEqual(handlerCalled, false);
   });
 
-  it('validates files specified in verifyFilesSchema', async () => {
+  it('validates files specified in verifyFilesSchema and rewrites input with validated paths/URLs', async () => {
     let handlerCalled = false;
+    let receivedParams: Record<string, unknown> | undefined;
     const tool: ToolDefinition = {
       name: 'file_tool',
       description: 'A tool requiring file validation',
@@ -330,15 +331,24 @@ describe('ToolHandler', () => {
         filePath: true,
         fileList: true,
       },
-      handler: async () => {
+      handler: async request => {
         handlerCalled = true;
+        receivedParams = request.params;
       },
     };
 
     const mockContext = sinon.createStubInstance(McpContext);
     const mockProcess = sinon.createStubInstance(ChildProcess);
     mockContext.browser = getMockBrowser({process: mockProcess});
-    mockContext.validatePath.resolves();
+    mockContext.validatePath.callsFake(async p => {
+      if (!p) {
+        return undefined;
+      }
+      return path.resolve(
+        '/canonical',
+        path.relative(path.resolve('/workspace'), p),
+      );
+    });
 
     const toolMutex = new Mutex();
     const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
@@ -378,6 +388,14 @@ describe('ToolHandler', () => {
       mockContext.validatePath.calledWith(testListFile2),
       true,
     );
+    assert.deepStrictEqual(receivedParams, {
+      filePath: pathToFileURL(path.resolve('/canonical/url-file.txt')).href,
+      fileList: [
+        path.resolve('/canonical/list1.txt'),
+        path.resolve('/canonical/list2.txt'),
+        'https://example.com/remote.txt',
+      ],
+    });
   });
 
   it('returns error when file validation fails for verifyFilesSchema', async () => {
@@ -434,6 +452,7 @@ describe('ToolHandler', () => {
 
   it('validates verifyFilesSchema when local: true and browser is running locally via process', async () => {
     let handlerCalled = false;
+    let receivedParams: Record<string, unknown> | undefined;
     const tool: ToolDefinition = {
       name: 'upload_tool',
       description: 'A tool with local-only file verification',
@@ -451,15 +470,17 @@ describe('ToolHandler', () => {
           remote: false,
         },
       },
-      handler: async () => {
+      handler: async request => {
         handlerCalled = true;
+        receivedParams = request.params;
       },
     };
 
     const mockContext = sinon.createStubInstance(McpContext);
     const mockProcess = sinon.createStubInstance(ChildProcess);
     mockContext.browser = getMockBrowser({process: mockProcess});
-    mockContext.validatePath.resolves();
+    const canonicalPath = path.resolve('/canonical/workspace/upload.png');
+    mockContext.validatePath.resolves(canonicalPath);
 
     const toolMutex = new Mutex();
     const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
@@ -481,10 +502,14 @@ describe('ToolHandler', () => {
     assert.strictEqual(result.isError, undefined);
     assert.strictEqual(handlerCalled, true);
     assert.strictEqual(mockContext.validatePath.calledOnceWith(testPath), true);
+    assert.deepStrictEqual(receivedParams, {
+      filePaths: [canonicalPath],
+    });
   });
 
   it('validates verifyFilesSchema when local: true and browser is connected to localhost wsEndpoint', async () => {
     let handlerCalled = false;
+    let receivedParams: Record<string, unknown> | undefined;
     const tool: ToolDefinition = {
       name: 'install_pwa_tool',
       description: 'PWA tool with local-only file verification',
@@ -501,8 +526,9 @@ describe('ToolHandler', () => {
           local: true,
         },
       },
-      handler: async () => {
+      handler: async request => {
         handlerCalled = true;
+        receivedParams = request.params;
       },
     };
 
@@ -510,7 +536,8 @@ describe('ToolHandler', () => {
     mockContext.browser = getMockBrowser({
       wsEndpoint: 'ws://127.0.0.1:9222/devtools/browser/test',
     });
-    mockContext.validatePath.resolves();
+    const canonicalBundlePath = path.resolve('/canonical/workspace/app.swbn');
+    mockContext.validatePath.resolves(canonicalBundlePath);
 
     const toolMutex = new Mutex();
     const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
@@ -536,10 +563,14 @@ describe('ToolHandler', () => {
       mockContext.validatePath.calledOnceWith(bundlePath),
       true,
     );
+    assert.deepStrictEqual(receivedParams, {
+      installUrlOrBundleUrl: pathToFileURL(canonicalBundlePath).href,
+    });
   });
 
   it('skips local-only verifyFilesSchema when browser is remote', async () => {
     let handlerCalled = false;
+    let receivedParams: Record<string, unknown> | undefined;
     const tool: ToolDefinition = {
       name: 'upload_tool',
       description: 'A tool with local-only file verification',
@@ -557,8 +588,9 @@ describe('ToolHandler', () => {
           remote: false,
         },
       },
-      handler: async () => {
+      handler: async request => {
         handlerCalled = true;
+        receivedParams = request.params;
       },
     };
 
@@ -586,6 +618,9 @@ describe('ToolHandler', () => {
     assert.strictEqual(result.isError, undefined);
     assert.strictEqual(handlerCalled, true);
     assert.strictEqual(mockContext.validatePath.called, false);
+    assert.deepStrictEqual(receivedParams, {
+      filePaths: ['/remote/server/path.txt'],
+    });
   });
 
   it('skips local-only verifyFilesSchema when browser has no process', async () => {
@@ -685,6 +720,7 @@ describe('ToolHandler', () => {
 
   it('validates verifyFilesSchema with true but skips local: true on remote browser', async () => {
     let handlerCalled = false;
+    let receivedParams: Record<string, unknown> | undefined;
     const tool: ToolDefinition = {
       name: 'hybrid_tool',
       description: 'A tool with both schema file verifications',
@@ -704,8 +740,9 @@ describe('ToolHandler', () => {
           remote: false,
         },
       },
-      handler: async () => {
+      handler: async request => {
         handlerCalled = true;
+        receivedParams = request.params;
       },
     };
 
@@ -713,7 +750,8 @@ describe('ToolHandler', () => {
     mockContext.browser = getMockBrowser({
       wsEndpoint: 'ws://remote-host.com:9222/devtools/browser/test',
     });
-    mockContext.validatePath.resolves();
+    const canonicalOutputPath = path.resolve('/canonical/output.json');
+    mockContext.validatePath.resolves(canonicalOutputPath);
 
     const toolMutex = new Mutex();
     const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
@@ -739,6 +777,10 @@ describe('ToolHandler', () => {
       mockContext.validatePath.calledOnceWith(outputPath),
       true,
     );
+    assert.deepStrictEqual(receivedParams, {
+      outputFile: canonicalOutputPath,
+      inputFile: '/remote/input.json',
+    });
   });
 
   it('returns error when file validation fails for local: true on local browser', async () => {
@@ -860,4 +902,69 @@ describe('ToolHandler', () => {
     await localToolHandler.handle({remoteFile: remotePath});
     assert.strictEqual(mockLocalContext.validatePath.called, false);
   });
+
+  it('rewrites file paths in params for page scoped tools', async () => {
+    let receivedParams: Record<string, unknown> | undefined;
+    const tool: DefinedPageTool = {
+      name: 'page_file_tool',
+      description: 'A page scoped tool with file verification',
+      annotations: {
+        category: ToolCategory.DEBUGGING,
+        readOnlyHint: false,
+      },
+      schema: {
+        filePath: zod.string(),
+      },
+      blockedByDialog: false,
+      verifyFilesSchema: {
+        filePath: true,
+      },
+      pageScoped: true,
+      handler: async request => {
+        receivedParams = request.params;
+      },
+    };
+
+    const mockContext = sinon.createStubInstance(McpContext);
+    const mockProcess = sinon.createStubInstance(ChildProcess);
+    mockContext.browser = getMockBrowser({process: mockProcess});
+    mockContext.getDevToolsData.resolves({});
+    const mockPage = sinon.createStubInstance(McpPage);
+    mockPage.getDialog.returns(undefined);
+    sinon.stub(mockPage, 'networkConditions').get(() => undefined);
+    sinon.stub(mockPage, 'geolocation').get(() => undefined);
+    sinon.stub(mockPage, 'viewport').get(() => undefined);
+    sinon.stub(mockPage, 'userAgent').get(() => undefined);
+    sinon.stub(mockPage, 'colorScheme').get(() => undefined);
+    sinon.stub(mockPage, 'cpuThrottlingRate').get(() => 1);
+    mockContext.getSelectedMcpPage.returns(mockPage);
+    const canonicalFilePath = path.resolve('/canonical/output.png');
+    mockContext.validatePath.resolves(canonicalFilePath);
+
+    const toolMutex = new Mutex();
+    const serverArgs = parseArguments('1.0.0', ['node', 'script.js'], {
+      CHROME_DEVTOOLS_MCP_NO_USAGE_STATISTICS: 'true',
+    });
+
+    const toolHandler = new ToolHandler(
+      tool,
+      serverArgs,
+      async () => mockContext,
+      toolMutex,
+    );
+
+    const inputPath = path.resolve('/workspace/output.png');
+    const result = await toolHandler.handle({
+      filePath: inputPath,
+    });
+
+    assert.strictEqual(result.isError, undefined);
+    assert.strictEqual(
+      mockContext.validatePath.calledOnceWith(inputPath),
+      true,
+    );
+    assert.deepStrictEqual(receivedParams, {
+      filePath: canonicalFilePath,
+    });
+  });
 });

tests/roots.test.ts

@@ -11,6 +11,8 @@ import path from 'node:path';
 import {describe, it} from 'node:test';
 import {pathToFileURL} from 'node:url';
 
+import {resolveCanonicalPath} from '../src/utils/files.js';
+
 import {withMcpContext} from './utils.js';
 
 describe('McpContext Roots', () => {
@@ -18,8 +20,8 @@ describe('McpContext Roots', () => {
     await withMcpContext(async (_response, context) => {
       context.setRoots([]);
       const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
-      // This should not throw
-      await context.validatePath(tmpPath);
+      const resolved = await context.validatePath(tmpPath);
+      assert.strictEqual(resolved, await resolveCanonicalPath(tmpPath));
     });
   });
 
@@ -36,7 +38,8 @@ describe('McpContext Roots', () => {
       const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
       // The temp directory must remain reachable even with no negotiated
       // roots, matching the existing "empty roots" behavior above.
-      await context.validatePath(tmpPath);
+      const resolved = await context.validatePath(tmpPath);
+      assert.strictEqual(resolved, await resolveCanonicalPath(tmpPath));
     });
   });
 
@@ -51,11 +54,16 @@ describe('McpContext Roots', () => {
         context.setRoots([{uri: pathToFileURL(otherRoot).href, name: 'other'}]);
 
         const tmpPath = path.join(os.tmpdir(), 'test-file.txt');
-        // This should not throw.
-        await context.validatePath(tmpPath);
+        const resolvedTmp = await context.validatePath(tmpPath);
+        assert.strictEqual(resolvedTmp, await resolveCanonicalPath(tmpPath));
 
         // Other root should also be allowed.
-        await context.validatePath(path.join(otherRoot, 'file.txt'));
+        const otherFile = path.join(otherRoot, 'file.txt');
+        const resolvedOther = await context.validatePath(otherFile);
+        assert.strictEqual(
+          resolvedOther,
+          await resolveCanonicalPath(otherFile),
+        );
 
         // Outside should still be denied. Use a path that is definitely not a root or temp dir.
         const outsidePath = path.resolve(
@@ -122,7 +130,9 @@ describe('McpContext Roots', () => {
 
           assert.strictEqual(
             resolvedPath,
-            path.join(workspacePath, testCase.expected),
+            await resolveCanonicalPath(
+              path.join(workspacePath, testCase.expected),
+            ),
           );
         }
       } finally {

tests/tools/lighthouse.test.ts

@@ -11,6 +11,7 @@ import path from 'node:path';
 import {describe, it} from 'node:test';
 
 import {lighthouseAudit} from '../../src/tools/lighthouse.js';
+import {resolveCanonicalPath} from '../../src/utils/files.js';
 import {serverHooks} from '../server.js';
 import {html, withMcpContext} from '../utils.js';
 
@@ -177,8 +178,9 @@ describe('lighthouse', () => {
           assert.equal(data.summary.mode, 'snapshot');
           assert.equal(data.summary.device, 'mobile');
           assert.ok(data.reports.length === 2);
+          const canonicalFolderPath = await resolveCanonicalPath(folderPath);
           for (const report of data.reports) {
-            assert.ok(report.startsWith(folderPath));
+            assert.ok(report.startsWith(canonicalFolderPath));
           }
         });
       } finally {

tests/tools/memory.test.ts

@@ -26,6 +26,7 @@ import {
   getHeapSnapshotObjectDetails,
 } from '../../src/tools/memory.js';
 import {stableIdSymbol} from '../../src/utils/id.js';
+import {resolveCanonicalPath} from '../../src/utils/files.js';
 import {withMcpContext} from '../utils.js';
 
 describe('memory', () => {
@@ -39,9 +40,10 @@ describe('memory', () => {
             response,
             context,
           );
+          const canonicalFilePath = await resolveCanonicalPath(filePath);
           assert.equal(
             response.responseLines.at(0),
-            `Heap snapshot saved to ${filePath}`,
+            `Heap snapshot saved to ${canonicalFilePath}`,
           );
           assert.ok(existsSync(filePath));
         } finally {

tests/tools/screenshot.test.ts

@@ -15,6 +15,7 @@ import sinon from 'sinon';
 import type {ParsedArguments} from '../../src/config/mcp-options.js';
 import {TextSnapshot} from '../../src/TextSnapshot.js';
 import {screenshot} from '../../src/tools/screenshot.js';
+import {resolveCanonicalPath} from '../../src/utils/files.js';
 import {screenshots} from '../snapshot.js';
 import {html, withMcpContext} from '../utils.js';
 
@@ -274,9 +275,10 @@ describe('screenshot', () => {
             response.responseLines.at(0),
             "Took a screenshot of the current page's viewport.",
           );
+          const canonicalFilePath = await resolveCanonicalPath(filePath);
           assert.equal(
             response.responseLines.at(1),
-            `Saved screenshot to ${filePath}.`,
+            `Saved screenshot to ${canonicalFilePath}.`,
           );
 
           const stats = await stat(filePath);

tests/utils/files.test.ts

@@ -8,88 +8,113 @@ import assert from 'node:assert';
 import fs from 'node:fs/promises';
 import os from 'node:os';
 import path from 'node:path';
-import {describe, it} from 'node:test';
+import {afterEach, beforeEach, describe, it} from 'node:test';
 
 import {resolveCanonicalPath} from '../../src/utils/files.js';
 
 describe('resolveCanonicalPath', () => {
-  it('should resolve an existing standard file path', async () => {
-    const tmpDir = await fs.mkdtemp(
+  let tmpDir: string;
+  let canonicalTmpDir: string;
+
+  beforeEach(async () => {
+    tmpDir = await fs.mkdtemp(
       path.join(os.tmpdir(), 'resolve-canonical-test-'),
     );
-    try {
-      const filePath = path.join(tmpDir, 'test.txt');
-      await fs.writeFile(filePath, 'hello');
+    canonicalTmpDir = await fs.realpath(tmpDir);
+  });
 
-      const resolved = await resolveCanonicalPath(filePath);
-      const canonicalTmpDir = await fs.realpath(tmpDir);
-      assert.strictEqual(resolved, path.join(canonicalTmpDir, 'test.txt'));
-    } finally {
-      await fs.rm(tmpDir, {recursive: true, force: true});
-    }
+  afterEach(async () => {
+    await fs.rm(tmpDir, {recursive: true, force: true});
+  });
+
+  it('should resolve an existing standard file path', async () => {
+    const filePath = path.join(tmpDir, 'test.txt');
+    await fs.writeFile(filePath, 'hello');
+
+    const resolved = await resolveCanonicalPath(filePath);
+    assert.strictEqual(resolved, path.join(canonicalTmpDir, 'test.txt'));
   });
 
   it('should resolve a non-existent file whose parent directory exists', async () => {
-    const tmpDir = await fs.mkdtemp(
-      path.join(os.tmpdir(), 'resolve-canonical-test-'),
-    );
-    try {
-      const filePath = path.join(tmpDir, 'non-existent.txt');
+    const filePath = path.join(tmpDir, 'non-existent.txt');
 
-      const resolved = await resolveCanonicalPath(filePath);
-      const canonicalTmpDir = await fs.realpath(tmpDir);
-      assert.strictEqual(
-        resolved,
-        path.join(canonicalTmpDir, 'non-existent.txt'),
-      );
-    } finally {
-      await fs.rm(tmpDir, {recursive: true, force: true});
-    }
+    const resolved = await resolveCanonicalPath(filePath);
+    assert.strictEqual(
+      resolved,
+      path.join(canonicalTmpDir, 'non-existent.txt'),
+    );
   });
 
   it('should resolve a non-existent deeply nested file whose parent directories do not exist', async () => {
-    const tmpDir = await fs.mkdtemp(
-      path.join(os.tmpdir(), 'resolve-canonical-test-'),
+    const filePath = path.join(
+      tmpDir,
+      'nested1',
+      'nested2',
+      'non-existent.txt',
     );
-    try {
-      const filePath = path.join(
-        tmpDir,
-        'nested1',
-        'nested2',
-        'non-existent.txt',
-      );
 
-      const resolved = await resolveCanonicalPath(filePath);
-      const canonicalTmpDir = await fs.realpath(tmpDir);
-      assert.strictEqual(
-        resolved,
-        path.join(canonicalTmpDir, 'nested1', 'nested2', 'non-existent.txt'),
-      );
-    } finally {
-      await fs.rm(tmpDir, {recursive: true, force: true});
-    }
+    const resolved = await resolveCanonicalPath(filePath);
+    assert.strictEqual(
+      resolved,
+      path.join(canonicalTmpDir, 'nested1', 'nested2', 'non-existent.txt'),
+    );
   });
 
   it('should resolve existing files with symlinks in path', async () => {
-    const tmpDir = await fs.mkdtemp(
-      path.join(os.tmpdir(), 'resolve-canonical-test-'),
+    const targetDir = path.join(tmpDir, 'target');
+    await fs.mkdir(targetDir);
+    const targetFile = path.join(targetDir, 'file.txt');
+    await fs.writeFile(targetFile, 'hello');
+
+    const symlinkDir = path.join(tmpDir, 'symlink_dir');
+    await fs.symlink(targetDir, symlinkDir, 'dir');
+
+    const filePathWithSymlink = path.join(symlinkDir, 'file.txt');
+
+    const resolved = await resolveCanonicalPath(filePathWithSymlink);
+    const canonicalTargetDir = await fs.realpath(targetDir);
+    assert.strictEqual(resolved, path.join(canonicalTargetDir, 'file.txt'));
+  });
+
+  it('should resolve non-existent files with symlinks in path', async () => {
+    const targetDir = path.join(tmpDir, 'target');
+    await fs.mkdir(targetDir);
+
+    const symlinkDir = path.join(tmpDir, 'symlink_dir');
+    await fs.symlink(targetDir, symlinkDir, 'dir');
+
+    const filePathWithSymlink = path.join(symlinkDir, 'non-existent.txt');
+
+    const resolved = await resolveCanonicalPath(filePathWithSymlink);
+    const canonicalTargetDir = await fs.realpath(targetDir);
+    assert.strictEqual(
+      resolved,
+      path.join(canonicalTargetDir, 'non-existent.txt'),
     );
-    try {
-      const targetDir = path.join(tmpDir, 'target');
-      await fs.mkdir(targetDir);
-      const targetFile = path.join(targetDir, 'file.txt');
-      await fs.writeFile(targetFile, 'hello');
+  });
 
-      const symlinkDir = path.join(tmpDir, 'symlink_dir');
-      await fs.symlink(targetDir, symlinkDir, 'dir');
+  it('should resolve dangling symlink at the end of path', async () => {
+    const nonExistentTarget = path.join(tmpDir, 'non-existent-target.txt');
+    const danglingSymlink = path.join(tmpDir, 'dangling-symlink.txt');
+    await fs.symlink(nonExistentTarget, danglingSymlink);
 
-      const filePathWithSymlink = path.join(symlinkDir, 'file.txt');
+    const resolved = await resolveCanonicalPath(danglingSymlink);
+    assert.strictEqual(
+      resolved,
+      path.join(canonicalTmpDir, 'dangling-symlink.txt'),
+    );
+  });
 
-      const resolved = await resolveCanonicalPath(filePathWithSymlink);
-      const canonicalTargetDir = await fs.realpath(targetDir);
-      assert.strictEqual(resolved, path.join(canonicalTargetDir, 'file.txt'));
-    } finally {
-      await fs.rm(tmpDir, {recursive: true, force: true});
-    }
+  it('should resolve path with a dangling symlink directory in the middle', async () => {
+    const nonExistentTargetDir = path.join(tmpDir, 'non-existent-dir');
+    const danglingSymlinkDir = path.join(tmpDir, 'dangling-dir');
+    await fs.symlink(nonExistentTargetDir, danglingSymlinkDir, 'dir');
+
+    const filePath = path.join(danglingSymlinkDir, 'file.txt');
+    const resolved = await resolveCanonicalPath(filePath);
+    assert.strictEqual(
+      resolved,
+      path.join(canonicalTmpDir, 'dangling-dir', 'file.txt'),
+    );
   });
 });