From 2a8226f8e2248b44e423acc7325448126e8e0640 Mon Sep 17 00:00:00 2001 From: Ayaan Zaidi Date: Wed, 15 Apr 2026 11:43:16 +0530 Subject: [PATCH] fix(postinstall): reject dist symlink escapes --- scripts/postinstall-bundled-plugins.mjs | 12 +++---- src/infra/package-dist-inventory.test.ts | 14 ++++++++ src/infra/package-dist-inventory.ts | 13 +++++-- .../postinstall-bundled-plugins.test.ts | 34 +++++++++++++++++++ 4 files changed, 65 insertions(+), 8 deletions(-) diff --git a/scripts/postinstall-bundled-plugins.mjs b/scripts/postinstall-bundled-plugins.mjs index 443b350b753..acfbbf1ccfd 100644 --- a/scripts/postinstall-bundled-plugins.mjs +++ b/scripts/postinstall-bundled-plugins.mjs @@ -17,7 +17,9 @@ import { readFileSync, realpathSync, renameSync, + rmdirSync, rmSync, + unlinkSync, writeFileSync, } from "node:fs"; import { basename, dirname, isAbsolute, join, relative } from "node:path"; @@ -197,7 +199,7 @@ function listInstalledDistFiles(params = {}) { function pruneEmptyDistDirectories(params = {}) { const readDir = params.readdirSync ?? readdirSync; - const removePath = params.rmSync ?? rmSync; + const removeDirectory = params.rmdirSync ?? rmdirSync; const distRoot = resolveInstalledDistRoot(params); if (distRoot === null) { return; @@ -227,13 +229,12 @@ function pruneEmptyDistDirectories(params = {}) { ); } if (readDir(currentDir).length === 0) { - removePath( + removeDirectory( assertSafeInstalledDistPath(normalizeRelativePath(relative(packageRoot, currentDir)), { packageRoot, distDirReal: distRoot.distDirReal, realpathSync: params.realpathSync, }), - { recursive: true, force: true }, ); } } @@ -243,7 +244,7 @@ function pruneEmptyDistDirectories(params = {}) { export function pruneInstalledPackageDist(params = {}) { const packageRoot = params.packageRoot ?? DEFAULT_PACKAGE_ROOT; - const removePath = params.rmSync ?? rmSync; + const removeFile = params.unlinkSync ?? unlinkSync; const log = params.log ?? console; const distRoot = resolveInstalledDistRoot(params); if (distRoot === null) { @@ -257,13 +258,12 @@ export function pruneInstalledPackageDist(params = {}) { if (expectedFiles.has(relativePath)) { continue; } - removePath( + removeFile( assertSafeInstalledDistPath(relativePath, { packageRoot, distDirReal: distRoot.distDirReal, realpathSync: params.realpathSync, }), - { recursive: true, force: true }, ); removed.push(relativePath); } diff --git a/src/infra/package-dist-inventory.test.ts b/src/infra/package-dist-inventory.test.ts index 459e3c9ef11..b8d564bc249 100644 --- a/src/infra/package-dist-inventory.test.ts +++ b/src/infra/package-dist-inventory.test.ts @@ -5,6 +5,7 @@ import { withTempDir } from "../test-helpers/temp-dir.js"; import { collectPackageDistInventoryErrors, PACKAGE_DIST_INVENTORY_RELATIVE_PATH, + collectPackageDistInventory, writePackageDistInventory, } from "./package-dist-inventory.js"; @@ -63,4 +64,17 @@ describe("package dist inventory", () => { ]); }); }); + + it("rejects symlinked dist entries", async () => { + await withTempDir({ prefix: "openclaw-dist-inventory-symlink-" }, async (packageRoot) => { + const distDir = path.join(packageRoot, "dist"); + await fs.mkdir(distDir, { recursive: true }); + await fs.writeFile(path.join(packageRoot, "escape.js"), "export {};\n", "utf8"); + await fs.symlink(path.join(packageRoot, "escape.js"), path.join(distDir, "entry.js")); + + await expect(collectPackageDistInventory(packageRoot)).rejects.toThrow( + "Unsafe package dist path: dist/entry.js", + ); + }); + }); }); diff --git a/src/infra/package-dist-inventory.ts b/src/infra/package-dist-inventory.ts index 1375ff28c53..ba52a0b4d7f 100644 --- a/src/infra/package-dist-inventory.ts +++ b/src/infra/package-dist-inventory.ts @@ -34,15 +34,24 @@ function isPackagedDistPath(relativePath: string): boolean { } async function collectRelativeFiles(rootDir: string, baseDir: string): Promise { try { + const rootStats = await fs.lstat(rootDir); + if (!rootStats.isDirectory() || rootStats.isSymbolicLink()) { + throw new Error( + `Unsafe package dist path: ${normalizeRelativePath(path.relative(baseDir, rootDir))}`, + ); + } const entries = await fs.readdir(rootDir, { withFileTypes: true }); const files = await Promise.all( entries.map(async (entry) => { const entryPath = path.join(rootDir, entry.name); + const relativePath = normalizeRelativePath(path.relative(baseDir, entryPath)); + if (entry.isSymbolicLink()) { + throw new Error(`Unsafe package dist path: ${relativePath}`); + } if (entry.isDirectory()) { return await collectRelativeFiles(entryPath, baseDir); } - if (entry.isFile() || entry.isSymbolicLink()) { - const relativePath = normalizeRelativePath(path.relative(baseDir, entryPath)); + if (entry.isFile()) { return isPackagedDistPath(relativePath) ? [relativePath] : []; } return []; diff --git a/test/scripts/postinstall-bundled-plugins.test.ts b/test/scripts/postinstall-bundled-plugins.test.ts index 65fb525ac40..b4b50b2beb8 100644 --- a/test/scripts/postinstall-bundled-plugins.test.ts +++ b/test/scripts/postinstall-bundled-plugins.test.ts @@ -262,6 +262,40 @@ describe("bundled plugin postinstall", () => { ).toThrow("unsafe dist entry: dist/escape"); }); + it("unlinks stale files instead of recursive pruning them", () => { + const unlinkSync = vi.fn(); + + expect( + pruneInstalledPackageDist({ + packageRoot: "/pkg", + expectedFiles: new Set(), + existsSync: vi.fn(() => true), + lstatSync: vi.fn(() => ({ + isDirectory: () => true, + isSymbolicLink: () => false, + })), + realpathSync: vi.fn((filePath) => filePath), + readdirSync: vi.fn((filePath, options) => { + if (filePath === "/pkg/dist" && options?.withFileTypes) { + return [ + { + name: "stale.js", + isDirectory: () => false, + isFile: () => true, + isSymbolicLink: () => false, + }, + ]; + } + return []; + }), + unlinkSync, + log: { log: vi.fn(), warn: vi.fn() }, + }), + ).toEqual(["dist/stale.js"]); + + expect(unlinkSync).toHaveBeenCalledWith("/pkg/dist/stale.js"); + }); + it("runs nested local installs with sanitized env when the sentinel package is missing", async () => { const extensionsDir = await createExtensionsDir(); const packageRoot = path.dirname(path.dirname(extensionsDir));