From 8f5abb6f9c8e5e259318447d0e82cdcc2d092374 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 7 May 2026 12:33:36 -0700 Subject: [PATCH 1/3] fix(server): add path traversal checks to static file serving routes The static file fallback in serveFolder() and the trace viewer's static route use path.join with URL path segments without validating the resolved path stays inside the served directory. A request with .. segments could read files outside the intended root. The /file route in the same file already validates paths with isPathInside. Apply the same check to the static fallback routes for consistency. --- .../src/server/trace/viewer/traceViewer.ts | 8 +++++++- packages/utils/httpServer.ts | 5 +++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index cd27f9b4cc95a..1d6756cdbd628 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -149,7 +149,13 @@ export async function startTraceViewerServer(options?: TraceViewerServerOptions) const relativePath = url.pathname.slice('/trace'.length); if (relativePath.startsWith('/file')) return serveTraceDataRoute(request, response, relativePath); - const absolutePath = path.join(libPath('vite', 'traceViewer'), ...relativePath.split('/')); + const traceViewerRoot = libPath('vite', 'traceViewer'); + const absolutePath = path.join(traceViewerRoot, ...relativePath.split('/')); + if (!isPathInside(traceViewerRoot, absolutePath)) { + response.statusCode = 403; + response.end(); + return true; + } return server.serveFile(request, response, absolutePath); }); } diff --git a/packages/utils/httpServer.ts b/packages/utils/httpServer.ts index 020dcfc597f77..e1d92f183f7cf 100644 --- a/packages/utils/httpServer.ts +++ b/packages/utils/httpServer.ts @@ -341,6 +341,11 @@ export function serveFolder(folder: string): HttpServer { if (relativePath === '/') relativePath = '/index.html'; const absolutePath = path.join(folder, ...relativePath.split('/')); + if (!isPathInside(folderRoot, absolutePath)) { + response.statusCode = 403; + response.end(); + return true; + } return server.serveFile(request, response, absolutePath); }); return server; From 065fe4fc030788c2d1abcb31055ab20de3753de1 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 7 May 2026 12:40:32 -0700 Subject: [PATCH 2/3] fix(html-reporter): replace startsWith with isPathInside for path validation The HTML reporter dev server used absolutePath.startsWith(folder) to validate static file paths. This check is bypassable: path.join with .. segments can produce paths like /tmp/reportSecrets/data that pass startsWith('/tmp/report'). Replace with isPathInside which checks against the resolved root + path separator. --- packages/playwright/src/reporters/html.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/playwright/src/reporters/html.ts b/packages/playwright/src/reporters/html.ts index 654bf23522dd9..24e43a6a3ff5b 100644 --- a/packages/playwright/src/reporters/html.ts +++ b/packages/playwright/src/reporters/html.ts @@ -25,7 +25,7 @@ import open from 'open'; import * as yazl from 'yazl'; import { MultiMap } from '@isomorphic/multimap'; import { calculateSha1 } from '@utils/crypto'; -import { copyFileAndMakeWritable, removeFolders, sanitizeForFilePath, toPosixPath } from '@utils/fileUtils'; +import { copyFileAndMakeWritable, isPathInside, removeFolders, sanitizeForFilePath, toPosixPath } from '@utils/fileUtils'; import { getPackageManagerExecCommand, isCodingAgent } from '@utils/env'; import { HttpServer, serveFolder } from '@utils/httpServer'; import { gracefullyProcessExitDoNotHang } from '@utils/processLauncher'; @@ -314,7 +314,7 @@ async function serveHtmlReportWithHMR(folder: string): Promise { // Serve attachments and the bundled trace-viewer copy from the generated // output folder first, falling through to Vite for source modules. const absolutePath = path.join(folder, ...url.pathname.split('/')); - if (absolutePath.startsWith(folder) && server.serveFile(request, response, absolutePath)) + if (isPathInside(folder, absolutePath) && server.serveFile(request, response, absolutePath)) return true; devServer.middlewares(request, response, HttpServer.notFoundFallback(response)); return true; From c6544c5942de3927c055cce2041401e2cace35e5 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 7 May 2026 13:37:05 -0700 Subject: [PATCH 3/3] fix(server): enforce static root in HttpServer.serveFile Rework per Pavel's feedback: instead of adding isPathInside checks at each call site, initialize HttpServer with an optional static root and enforce the path containment check inside serveFile itself. - Add optional staticRoot parameter to HttpServer constructor - serveFile rejects paths outside staticRoot with 403 - Call sites that serve files outside the root (e.g. trace /file route with its own allowedRoots validation) pass skipRootCheck - Remove per-call-site isPathInside/startsWith checks from serveFolder, traceViewer, html reporter, and dashboard - All four HttpServer construction sites now pass their root --- .../src/server/trace/viewer/traceViewer.ts | 12 +++------- .../src/tools/dashboard/dashboardApp.ts | 4 +--- packages/playwright/src/reporters/html.ts | 6 ++--- packages/utils/httpServer.ts | 24 ++++++++----------- 4 files changed, 17 insertions(+), 29 deletions(-) diff --git a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts index 1d6756cdbd628..d8060209f9aad 100644 --- a/packages/playwright-core/src/server/trace/viewer/traceViewer.ts +++ b/packages/playwright-core/src/server/trace/viewer/traceViewer.ts @@ -89,7 +89,7 @@ function validateTraceUrlOrPath(traceFileOrUrl: string | undefined): string | un } export async function startTraceViewerServer(options?: TraceViewerServerOptions): Promise { - const server = new HttpServer(); + const server = new HttpServer(libPath('vite', 'traceViewer')); const allowedRoots = (options?.allowedFileRoots ?? [process.cwd()]).map(r => path.resolve(r)); const isAllowed = (filePath: string) => allowedRoots.some(root => isPathInside(root, filePath)); @@ -105,7 +105,7 @@ export async function startTraceViewerServer(options?: TraceViewerServerOptions) return true; } if (fs.existsSync(filePath)) - return server.serveFile(request, response, filePath); + return server.serveFile(request, response, filePath, undefined, { skipRootCheck: true }); // If .json is requested, we'll synthesize it for zip-less operation. if (filePath.endsWith('.json')) { @@ -149,13 +149,7 @@ export async function startTraceViewerServer(options?: TraceViewerServerOptions) const relativePath = url.pathname.slice('/trace'.length); if (relativePath.startsWith('/file')) return serveTraceDataRoute(request, response, relativePath); - const traceViewerRoot = libPath('vite', 'traceViewer'); - const absolutePath = path.join(traceViewerRoot, ...relativePath.split('/')); - if (!isPathInside(traceViewerRoot, absolutePath)) { - response.statusCode = 403; - response.end(); - return true; - } + const absolutePath = path.join(libPath('vite', 'traceViewer'), ...relativePath.split('/')); return server.serveFile(request, response, absolutePath); }); } diff --git a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts index 34b5bd4cfaf56..cc8cc27c87129 100644 --- a/packages/playwright-core/src/tools/dashboard/dashboardApp.ts +++ b/packages/playwright-core/src/tools/dashboard/dashboardApp.ts @@ -49,8 +49,8 @@ type DashboardServer = { }; async function startDashboardServer(provider: SessionProvider, options: DashboardOptions): Promise { - const httpServer = new HttpServer(); const dashboardDir = libPath('vite', 'dashboard'); + const httpServer = new HttpServer(dashboardDir); const connections = new Set(); let currentReveal: DashboardOptions = options; @@ -152,8 +152,6 @@ function attachDashboardStaticServer(httpServer: HttpServer, dashboardDir: strin const pathname = new URL(request.url!, `http://${request.headers.host}`).pathname; const filePath = pathname === '/' ? 'index.html' : pathname.substring(1); const resolved = path.join(dashboardDir, filePath); - if (!resolved.startsWith(dashboardDir)) - return false; return httpServer.serveFile(request, response, resolved); }); } diff --git a/packages/playwright/src/reporters/html.ts b/packages/playwright/src/reporters/html.ts index 24e43a6a3ff5b..6f9e05d844b54 100644 --- a/packages/playwright/src/reporters/html.ts +++ b/packages/playwright/src/reporters/html.ts @@ -25,7 +25,7 @@ import open from 'open'; import * as yazl from 'yazl'; import { MultiMap } from '@isomorphic/multimap'; import { calculateSha1 } from '@utils/crypto'; -import { copyFileAndMakeWritable, isPathInside, removeFolders, sanitizeForFilePath, toPosixPath } from '@utils/fileUtils'; +import { copyFileAndMakeWritable, removeFolders, sanitizeForFilePath, toPosixPath } from '@utils/fileUtils'; import { getPackageManagerExecCommand, isCodingAgent } from '@utils/env'; import { HttpServer, serveFolder } from '@utils/httpServer'; import { gracefullyProcessExitDoNotHang } from '@utils/processLauncher'; @@ -289,7 +289,7 @@ export async function showHTMLReport(reportFolder: string | undefined, host: str // the generated index.html; we extract it and splice it into Vite's // transformed HTML so the client still finds it at runtime. async function serveHtmlReportWithHMR(folder: string): Promise { - const server = new HttpServer(); + const server = new HttpServer(folder); const reporterRoot = path.resolve(__dirname, '..', '..', '..', 'html-reporter'); const devServer = await server.createViteDevServer({ root: reporterRoot }); const generatedIndex = await fs.promises.readFile(path.join(folder, 'index.html'), 'utf-8'); @@ -314,7 +314,7 @@ async function serveHtmlReportWithHMR(folder: string): Promise { // Serve attachments and the bundled trace-viewer copy from the generated // output folder first, falling through to Vite for source modules. const absolutePath = path.join(folder, ...url.pathname.split('/')); - if (isPathInside(folder, absolutePath) && server.serveFile(request, response, absolutePath)) + if (server.serveFile(request, response, absolutePath)) return true; devServer.middlewares(request, response, HttpServer.notFoundFallback(response)); return true; diff --git a/packages/utils/httpServer.ts b/packages/utils/httpServer.ts index e1d92f183f7cf..4e36467248c14 100644 --- a/packages/utils/httpServer.ts +++ b/packages/utils/httpServer.ts @@ -54,9 +54,11 @@ export class HttpServer { private _wsGuid: string | undefined; // Allowed Host headers; null disables the check (host bound to a public address). private _allowedHosts: Set | null = null; + private _staticRoot: string | undefined; - constructor() { + constructor(staticRoot?: string) { this._server = createHttpServer(this._onRequest.bind(this)); + this._staticRoot = staticRoot ? path.resolve(staticRoot) : undefined; } server() { @@ -183,7 +185,12 @@ export class HttpServer { return purpose === 'human-readable' ? this._urlPrefixHumanReadable : this._urlPrefixPrecise; } - serveFile(request: http.IncomingMessage, response: http.ServerResponse, absoluteFilePath: string, headers?: { [name: string]: string }): boolean { + serveFile(request: http.IncomingMessage, response: http.ServerResponse, absoluteFilePath: string, headers?: { [name: string]: string }, options?: { skipRootCheck?: boolean }): boolean { + if (this._staticRoot && !options?.skipRootCheck && !isPathInside(this._staticRoot, absoluteFilePath)) { + response.statusCode = 403; + response.end(); + return true; + } try { for (const [name, value] of Object.entries(headers || {})) response.setHeader(name, value); @@ -317,8 +324,7 @@ export function hostnameFromHostHeader(host: string): string { } export function serveFolder(folder: string): HttpServer { - const server = new HttpServer(); - const folderRoot = path.resolve(folder); + const server = new HttpServer(folder); server.routePrefix('/', (request, response) => { let relativePath = new URL('http://localhost' + request.url).pathname; if (relativePath.startsWith('/trace/file')) { @@ -327,11 +333,6 @@ export function serveFolder(folder: string): HttpServer { if (!requested) return false; const resolved = path.resolve(requested); - if (!isPathInside(folderRoot, resolved)) { - response.statusCode = 403; - response.end(); - return true; - } try { return server.serveFile(request, response, resolved); } catch (e) { @@ -341,11 +342,6 @@ export function serveFolder(folder: string): HttpServer { if (relativePath === '/') relativePath = '/index.html'; const absolutePath = path.join(folder, ...relativePath.split('/')); - if (!isPathInside(folderRoot, absolutePath)) { - response.statusCode = 403; - response.end(); - return true; - } return server.serveFile(request, response, absolutePath); }); return server;