From 25d4c5023a275e8c2fda474e0b4ef83d9175652d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Sousa=20Andrade?= <659718+joaosa@users.noreply.github.com> Date: Sun, 10 May 2026 14:14:50 +0100 Subject: [PATCH] Harden file explorer symlink handling (#847) --- packages/server/src/server/bootstrap.ts | 17 +- .../file-explorer/service.posix.test.ts | 53 +++++- .../src/server/file-explorer/service.ts | 168 +++++++++++------- 3 files changed, 162 insertions(+), 76 deletions(-) diff --git a/packages/server/src/server/bootstrap.ts b/packages/server/src/server/bootstrap.ts index eb505721f..e0713cc1b 100644 --- a/packages/server/src/server/bootstrap.ts +++ b/packages/server/src/server/bootstrap.ts @@ -1,7 +1,7 @@ import express from "express"; import { createServer as createHTTPServer, type IncomingMessage, type ServerResponse } from "http"; -import { createReadStream, unlinkSync, existsSync } from "fs"; -import { stat } from "fs/promises"; +import { constants, existsSync, unlinkSync } from "fs"; +import { open } from "fs/promises"; import { randomUUID } from "node:crypto"; import { hostname as getHostname } from "node:os"; import path from "node:path"; @@ -143,6 +143,8 @@ type AgentMcpTransportMap = Map; const MAX_MCP_DEBUG_BATCH_ITEMS = 10; const REDACTED_LOG_VALUE = "[redacted]"; +const DOWNLOAD_OPEN_FLAGS = + process.platform === "win32" ? constants.O_RDONLY : constants.O_RDONLY | constants.O_NOFOLLOW; function formatHostForHttpUrl(host: string): string { return host.includes(":") && !host.startsWith("[") ? `[${host}]` : host; @@ -415,8 +417,10 @@ export async function createPaseoDaemon( return; } + let fileHandle: Awaited> | null = null; try { - const fileStats = await stat(entry.absolutePath); + fileHandle = await open(entry.absolutePath, DOWNLOAD_OPEN_FLAGS); + const fileStats = await fileHandle.stat(); if (!fileStats.isFile()) { res.status(404).json({ error: "File not found" }); return; @@ -425,9 +429,10 @@ export async function createPaseoDaemon( const safeFileName = entry.fileName.replace(/["\r\n]/g, "_"); res.setHeader("Content-Type", entry.mimeType); res.setHeader("Content-Disposition", `attachment; filename="${safeFileName}"`); - res.setHeader("Content-Length", entry.size.toString()); + res.setHeader("Content-Length", fileStats.size.toString()); - const stream = createReadStream(entry.absolutePath); + const stream = fileHandle.createReadStream(); + fileHandle = null; stream.on("error", (err) => { logger.error({ err }, "Failed to stream download"); if (!res.headersSent) { @@ -442,6 +447,8 @@ export async function createPaseoDaemon( if (!res.headersSent) { res.status(404).json({ error: "File not found" }); } + } finally { + await fileHandle?.close().catch(() => undefined); } }; diff --git a/packages/server/src/server/file-explorer/service.posix.test.ts b/packages/server/src/server/file-explorer/service.posix.test.ts index 31a536fbd..e1e961b8e 100644 --- a/packages/server/src/server/file-explorer/service.posix.test.ts +++ b/packages/server/src/server/file-explorer/service.posix.test.ts @@ -1,10 +1,10 @@ // POSIX-only: symlink fixtures /* eslint-disable max-nested-callbacks */ -import { mkdtemp, mkdir, rm, symlink, writeFile } from "node:fs/promises"; +import { mkdtemp, mkdir, realpath, rm, symlink, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { describe, expect, it } from "vitest"; -import { listDirectoryEntries, readExplorerFile } from "./service.js"; +import { getDownloadableFileInfo, listDirectoryEntries, readExplorerFile } from "./service.js"; import { isPlatform } from "../../test-utils/platform.js"; async function createTempDir(prefix: string): Promise { @@ -55,4 +55,53 @@ describe.skipIf(isPlatform("win32"))("service POSIX-only", () => { await rm(outsideRoot, { recursive: true, force: true }); } }); + + it("skips listed symlink entries that resolve outside the workspace", async () => { + const root = await createTempDir("paseo-file-explorer-"); + const outsideRoot = await createTempDir("paseo-file-explorer-outside-"); + + try { + await writeFile(path.join(root, "visible.txt"), "visible\n", "utf-8"); + const externalFile = path.join(outsideRoot, "secret.txt"); + await writeFile(externalFile, "top secret\n", "utf-8"); + await symlink(externalFile, path.join(root, "secret-link.txt")); + + const result = await listDirectoryEntries({ root }); + + const names = result.entries.map((entry) => entry.name); + expect(names).toContain("visible.txt"); + expect(names).not.toContain("secret-link.txt"); + } finally { + await rm(root, { recursive: true, force: true }); + await rm(outsideRoot, { recursive: true, force: true }); + } + }); + + it("uses canonical paths for downloadable symlink targets inside the workspace", async () => { + const root = await createTempDir("paseo-file-explorer-"); + + try { + const target = path.join(root, "safe.txt"); + const link = path.join(root, "safe-link.txt"); + await writeFile(target, "safe\n", "utf-8"); + await symlink("safe.txt", link); + + const file = await readExplorerFile({ + root, + relativePath: "safe-link.txt", + }); + const info = await getDownloadableFileInfo({ + root, + relativePath: "safe-link.txt", + }); + + expect(file.path).toBe("safe-link.txt"); + expect(file.content).toBe("safe\n"); + expect(info.path).toBe("safe-link.txt"); + expect(info.fileName).toBe("safe-link.txt"); + expect(info.absolutePath).toBe(await realpath(target)); + } finally { + await rm(root, { recursive: true, force: true }); + } + }); }); diff --git a/packages/server/src/server/file-explorer/service.ts b/packages/server/src/server/file-explorer/service.ts index f63badead..d97a9e40e 100644 --- a/packages/server/src/server/file-explorer/service.ts +++ b/packages/server/src/server/file-explorer/service.ts @@ -1,4 +1,5 @@ -import { promises as fs } from "fs"; +import { constants, promises as fs } from "fs"; +import type { FileHandle } from "fs/promises"; import path from "path"; import { resolvePathFromBase } from "../path-utils.js"; @@ -54,6 +55,10 @@ const TEXT_MIME_TYPES: Record = { }; const DEFAULT_TEXT_MIME_TYPE = "text/plain"; +const FILE_TYPE_SAMPLE_BYTES = 8192; +const READ_FILE_OPEN_FLAGS = + process.platform === "win32" ? constants.O_RDONLY : constants.O_RDONLY | constants.O_NOFOLLOW; +const ACCESS_OUTSIDE_WORKSPACE_MESSAGE = "Access outside of workspace is not allowed"; const IMAGE_MIME_TYPES: Record = { ".png": "image/png", @@ -69,6 +74,11 @@ interface ScopedPathParams { relativePath?: string; } +interface ScopedPath { + requestedPath: string; + resolvedPath: string; +} + interface EntryPayloadParams { root: string; targetPath: string; @@ -81,17 +91,17 @@ export async function listDirectoryEntries({ relativePath = ".", }: ListDirectoryParams): Promise { const directoryPath = await resolveScopedPath({ root, relativePath }); - const stats = await fs.stat(directoryPath); + const stats = await fs.stat(directoryPath.resolvedPath); if (!stats.isDirectory()) { throw new Error("Requested path is not a directory"); } - const dirents = await fs.readdir(directoryPath, { withFileTypes: true }); + const dirents = await fs.readdir(directoryPath.resolvedPath, { withFileTypes: true }); const entriesWithNulls = await Promise.all( dirents.map(async (dirent) => { - const targetPath = path.join(directoryPath, dirent.name); + const targetPath = path.join(directoryPath.requestedPath, dirent.name); const kind: ExplorerEntryKind = dirent.isDirectory() ? "directory" : "file"; try { return await buildEntryPayload({ @@ -103,7 +113,7 @@ export async function listDirectoryEntries({ } catch (error) { // Directories can contain dangling links (e.g. AGENTS.md -> CLAUDE.md). // Skip entries whose targets disappeared instead of failing the whole listing. - if (isMissingEntryError(error)) { + if (isMissingEntryError(error) || isOutsideWorkspaceError(error)) { return null; } throw error; @@ -121,7 +131,7 @@ export async function listDirectoryEntries({ }); return { - path: normalizeRelativePath({ root, targetPath: directoryPath }), + path: normalizeRelativePath({ root, targetPath: directoryPath.requestedPath }), entries, }; } @@ -171,48 +181,53 @@ export async function readExplorerFileBytes({ relativePath, }: ReadFileParams): Promise { const filePath = await resolveScopedPath({ root, relativePath }); - const stats = await fs.stat(filePath); + const handle = await openFileForRead(filePath.resolvedPath); - if (!stats.isFile()) { - throw new Error("Requested path is not a file"); - } + try { + const stats = await handle.stat(); - const ext = path.extname(filePath).toLowerCase(); - const basePayload = { - path: normalizeRelativePath({ root, targetPath: filePath }), - size: stats.size, - modifiedAt: stats.mtime.toISOString(), - }; + if (!stats.isFile()) { + throw new Error("Requested path is not a file"); + } + + const ext = path.extname(filePath.resolvedPath).toLowerCase(); + const basePayload = { + path: normalizeRelativePath({ root, targetPath: filePath.requestedPath }), + size: stats.size, + modifiedAt: stats.mtime.toISOString(), + }; + + const buffer = await handle.readFile(); + if (ext in IMAGE_MIME_TYPES) { + return { + ...basePayload, + kind: "image", + encoding: "binary", + bytes: buffer, + mimeType: IMAGE_MIME_TYPES[ext], + }; + } + + if (isLikelyBinary(buffer)) { + return { + ...basePayload, + kind: "binary", + encoding: "binary", + bytes: buffer, + mimeType: "application/octet-stream", + }; + } - if (ext in IMAGE_MIME_TYPES) { - const buffer = await fs.readFile(filePath); return { ...basePayload, - kind: "image", - encoding: "binary", + kind: "text", + encoding: "utf-8", bytes: buffer, - mimeType: IMAGE_MIME_TYPES[ext], + mimeType: textMimeTypeForExtension(ext), }; + } finally { + await handle.close(); } - - const buffer = await fs.readFile(filePath); - if (isLikelyBinary(buffer)) { - return { - ...basePayload, - kind: "binary", - encoding: "binary", - bytes: buffer, - mimeType: "application/octet-stream", - }; - } - - return { - ...basePayload, - kind: "text", - encoding: "utf-8", - bytes: buffer, - mimeType: textMimeTypeForExtension(ext), - }; } export async function getDownloadableFileInfo({ root, relativePath }: ReadFileParams): Promise<{ @@ -223,47 +238,50 @@ export async function getDownloadableFileInfo({ root, relativePath }: ReadFilePa size: number; }> { const filePath = await resolveScopedPath({ root, relativePath }); - const stats = await fs.stat(filePath); + const handle = await openFileForRead(filePath.resolvedPath); - if (!stats.isFile()) { - throw new Error("Requested path is not a file"); - } + try { + const stats = await handle.stat(); - const ext = path.extname(filePath).toLowerCase(); - let mimeType = "application/octet-stream"; - if (ext in IMAGE_MIME_TYPES) { - mimeType = IMAGE_MIME_TYPES[ext]; - } else { - // Read only a small prefix to classify likely text vs binary. - const handle = await fs.open(filePath, "r"); - const sample = Buffer.alloc(8192); - try { + if (!stats.isFile()) { + throw new Error("Requested path is not a file"); + } + + const ext = path.extname(filePath.resolvedPath).toLowerCase(); + let mimeType = "application/octet-stream"; + if (ext in IMAGE_MIME_TYPES) { + mimeType = IMAGE_MIME_TYPES[ext]; + } else { + const sample = Buffer.alloc(FILE_TYPE_SAMPLE_BYTES); const { bytesRead } = await handle.read(sample, 0, sample.length, 0); const chunk = bytesRead < sample.length ? sample.subarray(0, bytesRead) : sample; if (!isLikelyBinary(chunk)) { mimeType = textMimeTypeForExtension(ext); } - } finally { - await handle.close(); } - } - return { - path: normalizeRelativePath({ root, targetPath: filePath }), - absolutePath: filePath, - fileName: path.basename(filePath), - mimeType, - size: stats.size, - }; + return { + path: normalizeRelativePath({ root, targetPath: filePath.requestedPath }), + absolutePath: filePath.resolvedPath, + fileName: path.basename(filePath.requestedPath), + mimeType, + size: stats.size, + }; + } finally { + await handle.close(); + } } -async function resolveScopedPath({ root, relativePath = "." }: ScopedPathParams): Promise { +async function resolveScopedPath({ + root, + relativePath = ".", +}: ScopedPathParams): Promise { const normalizedRoot = path.resolve(root); const requestedPath = resolvePathFromBase(normalizedRoot, relativePath); const relative = path.relative(normalizedRoot, requestedPath); if (relative !== "" && (relative.startsWith("..") || path.isAbsolute(relative))) { - throw new Error("Access outside of workspace is not allowed"); + throw new Error(ACCESS_OUTSIDE_WORKSPACE_MESSAGE); } const realRoot = await fs.realpath(normalizedRoot); @@ -272,24 +290,32 @@ async function resolveScopedPath({ root, relativePath = "." }: ScopedPathParams) const realPath = await fs.realpath(requestedPath); const realRelative = path.relative(realRoot, realPath); if (realRelative !== "" && (realRelative.startsWith("..") || path.isAbsolute(realRelative))) { - throw new Error("Access outside of workspace is not allowed"); + throw new Error(ACCESS_OUTSIDE_WORKSPACE_MESSAGE); } - return requestedPath; + return { requestedPath, resolvedPath: realPath }; } catch (error) { if (isMissingEntryError(error)) { - return requestedPath; + return { requestedPath, resolvedPath: requestedPath }; } throw error; } } +async function openFileForRead(filePath: string): Promise { + return fs.open(filePath, READ_FILE_OPEN_FLAGS); +} + async function buildEntryPayload({ root, targetPath, name, kind, }: EntryPayloadParams): Promise { - const stats = await fs.stat(targetPath); + const entryPath = await resolveScopedPath({ + root, + relativePath: normalizeRelativePath({ root, targetPath }), + }); + const stats = await fs.stat(entryPath.resolvedPath); return { name, path: normalizeRelativePath({ root, targetPath }), @@ -304,6 +330,10 @@ function isMissingEntryError(error: unknown): boolean { return code === "ENOENT" || code === "ENOTDIR" || code === "ELOOP"; } +function isOutsideWorkspaceError(error: unknown): boolean { + return error instanceof Error && error.message === ACCESS_OUTSIDE_WORKSPACE_MESSAGE; +} + function normalizeRelativePath({ root, targetPath }: { root: string; targetPath: string }): string { const normalizedRoot = path.resolve(root); const normalizedTarget = path.resolve(targetPath);