diff --git a/packages/playwright-core/src/tools/backend/context.ts b/packages/playwright-core/src/tools/backend/context.ts index 068e8e8339dd5..1ab516d9a7a9d 100644 --- a/packages/playwright-core/src/tools/backend/context.ts +++ b/packages/playwright-core/src/tools/backend/context.ts @@ -22,7 +22,7 @@ import debug from 'debug'; import { escapeWithQuotes } from '@isomorphic/stringUtils'; import { disposeAll } from '@isomorphic/disposable'; import { eventsHelper } from '@utils/eventsHelper'; -import { isPathInside, isSystemDirectory, isWritable } from '@utils/fileUtils'; +import { isPathInside, isSystemDirectory, isWritable, resolveSymlinks } from '@utils/fileUtils'; import { playwright } from '../../inprocess'; import { dedent, languageGeneratorId, secretCode } from './codegen'; @@ -467,6 +467,12 @@ async function checkFile(options: ContextOptions, resolvedFilename: string, flag // Trust llm to use valid characters in file names. const output = outputDir(options); const workspace = options.cwd; - if (!isPathInside(output, resolvedFilename) && !isPathInside(workspace, resolvedFilename)) + // Follow symlinks, an unresolvable root cannot be traversed anyway. + const [realOutput, realWorkspace, realFilename] = await Promise.all([ + resolveSymlinks(output).catch(() => output), + resolveSymlinks(workspace).catch(() => workspace), + resolveSymlinks(resolvedFilename), + ]); + if (!isPathInside(realOutput, realFilename) && !isPathInside(realWorkspace, realFilename)) throw new Error(`File access denied: ${resolvedFilename} is outside allowed roots. Allowed roots: ${output}, ${workspace}`); } diff --git a/packages/utils/fileUtils.ts b/packages/utils/fileUtils.ts index 613f298cc4a8a..12c72160be3dd 100644 --- a/packages/utils/fileUtils.ts +++ b/packages/utils/fileUtils.ts @@ -88,6 +88,7 @@ export function trimLongString(s: string, length = 100) { return s.substring(0, start) + middle + s.slice(-end); } +// Does not follow symlinks, see resolveSymlinks. export function isPathInside(root: string, candidate: string): boolean { const resolvedRoot = path.resolve(root); const resolvedCandidate = path.resolve(candidate); @@ -96,6 +97,19 @@ export function isPathInside(root: string, candidate: string): boolean { return resolvedCandidate.startsWith(resolvedRoot + path.sep); } +// Like realpath, but tolerates a non-existent tail. +export async function resolveSymlinks(filePath: string): Promise { + const resolved = path.resolve(filePath); + try { + return await fs.promises.realpath(resolved); + } catch (e) { + const parent = path.dirname(resolved); + if (e.code !== 'ENOENT' || parent === resolved) + throw e; + return path.join(await resolveSymlinks(parent), path.basename(resolved)); + } +} + export function resolveWithinRoot(root: string, fileName: string): string | null { if (path.isAbsolute(fileName)) return null; diff --git a/tests/mcp/files.spec.ts b/tests/mcp/files.spec.ts index 4456529cac0ec..583923a56a04f 100644 --- a/tests/mcp/files.spec.ts +++ b/tests/mcp/files.spec.ts @@ -377,6 +377,63 @@ test('file upload unrestricted when flag is set', async ({ startClient, server } }); }); +test('file upload follows symlinks when checking workspace roots', async ({ startClient, server }, testInfo) => { + test.skip(process.platform === 'win32', 'Creating symlinks requires elevated privileges on Windows'); + + const rootDir = testInfo.outputPath('workspace'); + await fs.mkdir(rootDir, { recursive: true }); + const fileInsideRoot = path.join(rootDir, 'inside.txt'); + await fs.writeFile(fileInsideRoot, 'Inside root'); + await fs.symlink(fileInsideRoot, path.join(rootDir, 'inside-link.txt')); + const fileOutsideRoot = testInfo.outputPath('outside.txt'); + await fs.writeFile(fileOutsideRoot, 'Outside root'); + await fs.symlink(fileOutsideRoot, path.join(rootDir, 'outside-link.txt')); + // Root reached through a symlink, like /tmp on macOS. + const rootLink = testInfo.outputPath('workspace-link'); + await fs.symlink(rootDir, rootLink); + + const { client } = await startClient({ + roots: [ + { + name: 'workspace', + uri: `file://${rootLink}`, + } + ], + }); + + server.setContent('/', ``, 'text/html'); + await client.callTool({ + name: 'browser_navigate', + arguments: { url: server.PREFIX }, + }); + await client.callTool({ + name: 'browser_click', + arguments: { element: 'Textbox', target: 'e2' }, + }); + + // Should succeed - symlink points inside the root + expect(await client.callTool({ + name: 'browser_file_upload', + arguments: { paths: ['inside-link.txt'] }, + })).toHaveResponse({ + code: expect.stringContaining(JSON.stringify(path.join(rootLink, 'inside-link.txt'))), + }); + + await client.callTool({ + name: 'browser_click', + arguments: { element: 'Textbox', target: 'e2' }, + }); + + // Should fail - symlink points outside the root + expect(await client.callTool({ + name: 'browser_file_upload', + arguments: { paths: ['outside-link.txt'] }, + })).toHaveResponse({ + isError: true, + error: expect.stringMatching('File access denied: .* is outside allowed roots'), + }); +}); + const dropzoneHtml = `