diff --git a/src/infra/archive.test.ts b/src/infra/archive.test.ts index ed09f030162..3624710c233 100644 --- a/src/infra/archive.test.ts +++ b/src/infra/archive.test.ts @@ -148,40 +148,37 @@ describe("archive utils", () => { }); }); - it.runIf(process.platform !== "win32")( - "does not clobber out-of-destination file when parent dir is symlink-rebound during zip extract", - async () => { - await withArchiveCase("zip", async ({ workDir, archivePath, extractDir }) => { - const outsideDir = path.join(workDir, "outside"); - await fs.mkdir(outsideDir, { recursive: true }); - const slotDir = path.join(extractDir, "slot"); - await fs.mkdir(slotDir, { recursive: true }); + it("does not clobber out-of-destination file when parent dir is symlink-rebound during zip extract", async () => { + await withArchiveCase("zip", async ({ workDir, archivePath, extractDir }) => { + const outsideDir = path.join(workDir, "outside"); + await fs.mkdir(outsideDir, { recursive: true }); + const slotDir = path.join(extractDir, "slot"); + await fs.mkdir(slotDir, { recursive: true }); - const outsideTarget = path.join(outsideDir, "target.txt"); - await fs.writeFile(outsideTarget, "SAFE"); + const outsideTarget = path.join(outsideDir, "target.txt"); + await fs.writeFile(outsideTarget, "SAFE"); - const zip = new JSZip(); - zip.file("slot/target.txt", "owned"); - await fs.writeFile(archivePath, await zip.generateAsync({ type: "nodebuffer" })); + const zip = new JSZip(); + zip.file("slot/target.txt", "owned"); + await fs.writeFile(archivePath, await zip.generateAsync({ type: "nodebuffer" })); - await withRealpathSymlinkRebindRace({ - shouldFlip: (realpathInput) => realpathInput === slotDir, - symlinkPath: slotDir, - symlinkTarget: outsideDir, - timing: "after-realpath", - run: async () => { - await expect( - extractArchive({ archivePath, destDir: extractDir, timeoutMs: 5_000 }), - ).rejects.toMatchObject({ - code: "destination-symlink-traversal", - } satisfies Partial); - }, - }); - - await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("SAFE"); + await withRealpathSymlinkRebindRace({ + shouldFlip: (realpathInput) => realpathInput === slotDir, + symlinkPath: slotDir, + symlinkTarget: outsideDir, + timing: "after-realpath", + run: async () => { + await expect( + extractArchive({ archivePath, destDir: extractDir, timeoutMs: 5_000 }), + ).rejects.toMatchObject({ + code: "destination-symlink-traversal", + } satisfies Partial); + }, }); - }, - ); + + await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("SAFE"); + }); + }); it("rejects tar path traversal (zip slip)", async () => { await withArchiveCase("tar", async ({ workDir, archivePath, extractDir }) => { diff --git a/src/infra/fs-safe.test.ts b/src/infra/fs-safe.test.ts index 18f6986ff7f..cae7bd418bf 100644 --- a/src/infra/fs-safe.test.ts +++ b/src/infra/fs-safe.test.ts @@ -1,7 +1,10 @@ import fs from "node:fs/promises"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { withRealpathSymlinkRebindRace } from "../test-utils/symlink-rebind-race.js"; +import { + createRebindableDirectoryAlias, + withRealpathSymlinkRebindRace, +} from "../test-utils/symlink-rebind-race.js"; import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js"; import { copyFileWithinRoot, @@ -269,100 +272,27 @@ describe("fs-safe", () => { } }); - it.runIf(process.platform !== "win32")( - "does not truncate out-of-root file when symlink retarget races write open", - async () => { - const root = await tempDirs.make("openclaw-fs-safe-root-"); - const inside = path.join(root, "inside"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); - await fs.mkdir(inside, { recursive: true }); - const insideTarget = path.join(inside, "target.txt"); - const outsideTarget = path.join(outside, "target.txt"); - await fs.writeFile(insideTarget, "inside"); - await fs.writeFile(outsideTarget, "X".repeat(4096)); - const slot = path.join(root, "slot"); - await fs.symlink(inside, slot); + it("does not truncate out-of-root file when symlink retarget races write open", async () => { + const root = await tempDirs.make("openclaw-fs-safe-root-"); + const inside = path.join(root, "inside"); + const outside = await tempDirs.make("openclaw-fs-safe-outside-"); + await fs.mkdir(inside, { recursive: true }); + const insideTarget = path.join(inside, "target.txt"); + const outsideTarget = path.join(outside, "target.txt"); + await fs.writeFile(insideTarget, "inside"); + await fs.writeFile(outsideTarget, "X".repeat(4096)); + const slot = path.join(root, "slot"); + await createRebindableDirectoryAlias({ + aliasPath: slot, + targetPath: inside, + }); - await withRealpathSymlinkRebindRace({ - shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), - symlinkPath: slot, - symlinkTarget: outside, - timing: "before-realpath", - run: async () => { - await expect( - writeFileWithinRoot({ - rootDir: root, - relativePath: path.join("slot", "target.txt"), - data: "new-content", - mkdir: false, - }), - ).rejects.toMatchObject({ code: "outside-workspace" }); - }, - }); - - await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); - }, - ); - - it.runIf(process.platform !== "win32")( - "does not clobber out-of-root file when symlink retarget races write-from-path open", - async () => { - const root = await tempDirs.make("openclaw-fs-safe-root-"); - const inside = path.join(root, "inside"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); - const sourceDir = await tempDirs.make("openclaw-fs-safe-source-"); - const sourcePath = path.join(sourceDir, "source.txt"); - await fs.writeFile(sourcePath, "new-content"); - await fs.mkdir(inside, { recursive: true }); - const outsideTarget = path.join(outside, "target.txt"); - await fs.writeFile(outsideTarget, "X".repeat(4096)); - const slot = path.join(root, "slot"); - await fs.symlink(inside, slot); - - await withRealpathSymlinkRebindRace({ - shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), - symlinkPath: slot, - symlinkTarget: outside, - timing: "before-realpath", - run: async () => { - await expect( - writeFileFromPathWithinRoot({ - rootDir: root, - relativePath: path.join("slot", "target.txt"), - sourcePath, - mkdir: false, - }), - ).rejects.toMatchObject({ code: "outside-workspace" }); - }, - }); - - await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); - }, - ); - - it.runIf(process.platform !== "win32")( - "cleans up created out-of-root file when symlink retarget races create path", - async () => { - const root = await tempDirs.make("openclaw-fs-safe-root-"); - const inside = path.join(root, "inside"); - const outside = await tempDirs.make("openclaw-fs-safe-outside-"); - await fs.mkdir(inside, { recursive: true }); - const outsideTarget = path.join(outside, "target.txt"); - const slot = path.join(root, "slot"); - await fs.symlink(inside, slot); - - const realOpen = fs.open.bind(fs); - let flipped = false; - const openSpy = vi.spyOn(fs, "open").mockImplementation(async (...args) => { - const [filePath] = args; - if (!flipped && String(filePath).endsWith(path.join("slot", "target.txt"))) { - flipped = true; - await fs.rm(slot, { recursive: true, force: true }); - await fs.symlink(outside, slot); - } - return await realOpen(...args); - }); - try { + await withRealpathSymlinkRebindRace({ + shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), + symlinkPath: slot, + symlinkTarget: outside, + timing: "before-realpath", + run: async () => { await expect( writeFileWithinRoot({ rootDir: root, @@ -371,13 +301,88 @@ describe("fs-safe", () => { mkdir: false, }), ).rejects.toMatchObject({ code: "outside-workspace" }); - } finally { - openSpy.mockRestore(); - } + }, + }); - await expect(fs.stat(outsideTarget)).rejects.toMatchObject({ code: "ENOENT" }); - }, - ); + await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); + }); + + it("does not clobber out-of-root file when symlink retarget races write-from-path open", async () => { + const root = await tempDirs.make("openclaw-fs-safe-root-"); + const inside = path.join(root, "inside"); + const outside = await tempDirs.make("openclaw-fs-safe-outside-"); + const sourceDir = await tempDirs.make("openclaw-fs-safe-source-"); + const sourcePath = path.join(sourceDir, "source.txt"); + await fs.writeFile(sourcePath, "new-content"); + await fs.mkdir(inside, { recursive: true }); + const outsideTarget = path.join(outside, "target.txt"); + await fs.writeFile(outsideTarget, "X".repeat(4096)); + const slot = path.join(root, "slot"); + await createRebindableDirectoryAlias({ + aliasPath: slot, + targetPath: inside, + }); + + await withRealpathSymlinkRebindRace({ + shouldFlip: (realpathInput) => realpathInput.endsWith(path.join("slot", "target.txt")), + symlinkPath: slot, + symlinkTarget: outside, + timing: "before-realpath", + run: async () => { + await expect( + writeFileFromPathWithinRoot({ + rootDir: root, + relativePath: path.join("slot", "target.txt"), + sourcePath, + mkdir: false, + }), + ).rejects.toMatchObject({ code: "outside-workspace" }); + }, + }); + + await expect(fs.readFile(outsideTarget, "utf8")).resolves.toBe("X".repeat(4096)); + }); + + it("cleans up created out-of-root file when symlink retarget races create path", async () => { + const root = await tempDirs.make("openclaw-fs-safe-root-"); + const inside = path.join(root, "inside"); + const outside = await tempDirs.make("openclaw-fs-safe-outside-"); + await fs.mkdir(inside, { recursive: true }); + const outsideTarget = path.join(outside, "target.txt"); + const slot = path.join(root, "slot"); + await createRebindableDirectoryAlias({ + aliasPath: slot, + targetPath: inside, + }); + + const realOpen = fs.open.bind(fs); + let flipped = false; + const openSpy = vi.spyOn(fs, "open").mockImplementation(async (...args) => { + const [filePath] = args; + if (!flipped && String(filePath).endsWith(path.join("slot", "target.txt"))) { + flipped = true; + await createRebindableDirectoryAlias({ + aliasPath: slot, + targetPath: outside, + }); + } + return await realOpen(...args); + }); + try { + await expect( + writeFileWithinRoot({ + rootDir: root, + relativePath: path.join("slot", "target.txt"), + data: "new-content", + mkdir: false, + }), + ).rejects.toMatchObject({ code: "outside-workspace" }); + } finally { + openSpy.mockRestore(); + } + + await expect(fs.stat(outsideTarget)).rejects.toMatchObject({ code: "ENOENT" }); + }); it("returns not-found for missing files", async () => { const dir = await tempDirs.make("openclaw-fs-safe-"); diff --git a/src/plugins/install.test.ts b/src/plugins/install.test.ts index 442f97c3bfd..248294da764 100644 --- a/src/plugins/install.test.ts +++ b/src/plugins/install.test.ts @@ -20,6 +20,7 @@ vi.mock("../process/exec.js", () => ({ let installPluginFromArchive: typeof import("./install.js").installPluginFromArchive; let installPluginFromDir: typeof import("./install.js").installPluginFromDir; let installPluginFromNpmSpec: typeof import("./install.js").installPluginFromNpmSpec; +let installPluginFromPath: typeof import("./install.js").installPluginFromPath; let runCommandWithTimeout: typeof import("../process/exec.js").runCommandWithTimeout; let suiteTempRoot = ""; let tempDirCounter = 0; @@ -308,8 +309,12 @@ afterAll(() => { }); beforeAll(async () => { - ({ installPluginFromArchive, installPluginFromDir, installPluginFromNpmSpec } = - await import("./install.js")); + ({ + installPluginFromArchive, + installPluginFromDir, + installPluginFromNpmSpec, + installPluginFromPath, + } = await import("./install.js")); ({ runCommandWithTimeout } = await import("../process/exec.js")); }); @@ -598,6 +603,37 @@ describe("installPluginFromDir", () => { }); }); +describe("installPluginFromPath", () => { + it("blocks hardlink alias overwrites when installing a plain file plugin", async () => { + const baseDir = makeTempDir(); + const extensionsDir = path.join(baseDir, "extensions"); + const outsideDir = path.join(baseDir, "outside"); + fs.mkdirSync(extensionsDir, { recursive: true }); + fs.mkdirSync(outsideDir, { recursive: true }); + + const sourcePath = path.join(baseDir, "payload.js"); + fs.writeFileSync(sourcePath, "console.log('SAFE');\n", "utf-8"); + const victimPath = path.join(outsideDir, "victim.js"); + fs.writeFileSync(victimPath, "ORIGINAL", "utf-8"); + + const targetPath = path.join(extensionsDir, "payload.js"); + fs.linkSync(victimPath, targetPath); + + const result = await installPluginFromPath({ + path: sourcePath, + extensionsDir, + mode: "update", + }); + + expect(result.ok).toBe(false); + if (result.ok) { + return; + } + expect(result.error.toLowerCase()).toMatch(/hardlink|path alias escape/); + expect(fs.readFileSync(victimPath, "utf-8")).toBe("ORIGINAL"); + }); +}); + describe("installPluginFromNpmSpec", () => { it("uses --ignore-scripts for npm pack and cleans up temp dir", async () => { const stateDir = makeTempDir(); diff --git a/src/plugins/install.ts b/src/plugins/install.ts index ed6a0c3755d..fbcd4bc2203 100644 --- a/src/plugins/install.ts +++ b/src/plugins/install.ts @@ -2,6 +2,7 @@ import fs from "node:fs/promises"; import path from "node:path"; import { MANIFEST_KEY } from "../compat/legacy-names.js"; import { fileExists, readJsonFile, resolveArchiveKind } from "../infra/archive.js"; +import { writeFileFromPathWithinRoot } from "../infra/fs-safe.js"; import { resolveExistingInstallPath, withExtractedArchiveRoot } from "../infra/install-flow.js"; import { resolveInstallModeOptions, @@ -401,7 +402,15 @@ export async function installPluginFromFile(params: { } logger.info?.(`Installing to ${targetFile}…`); - await fs.copyFile(filePath, targetFile); + try { + await writeFileFromPathWithinRoot({ + rootDir: extensionsDir, + relativePath: path.basename(targetFile), + sourcePath: filePath, + }); + } catch (err) { + return { ok: false, error: String(err) }; + } return buildFileInstallResult(pluginId, targetFile); } diff --git a/src/test-utils/symlink-rebind-race.ts b/src/test-utils/symlink-rebind-race.ts index 5ba92e27d31..f0f381c5f02 100644 --- a/src/test-utils/symlink-rebind-race.ts +++ b/src/test-utils/symlink-rebind-race.ts @@ -1,6 +1,17 @@ import fs from "node:fs/promises"; +import path from "node:path"; import { vi } from "vitest"; +export async function createRebindableDirectoryAlias(params: { + aliasPath: string; + targetPath: string; +}): Promise { + const aliasPath = path.resolve(params.aliasPath); + const targetPath = path.resolve(params.targetPath); + await fs.rm(aliasPath, { recursive: true, force: true }); + await fs.symlink(targetPath, aliasPath, process.platform === "win32" ? "junction" : undefined); +} + export async function withRealpathSymlinkRebindRace(params: { shouldFlip: (realpathInput: string) => boolean; symlinkPath: string; @@ -17,13 +28,17 @@ export async function withRealpathSymlinkRebindRace(params: { if (!flipped && params.shouldFlip(filePath)) { flipped = true; if (params.timing !== "after-realpath") { - await fs.rm(params.symlinkPath, { recursive: true, force: true }); - await fs.symlink(params.symlinkTarget, params.symlinkPath); + await createRebindableDirectoryAlias({ + aliasPath: params.symlinkPath, + targetPath: params.symlinkTarget, + }); return await realRealpath(...args); } const resolved = await realRealpath(...args); - await fs.rm(params.symlinkPath, { recursive: true, force: true }); - await fs.symlink(params.symlinkTarget, params.symlinkPath); + await createRebindableDirectoryAlias({ + aliasPath: params.symlinkPath, + targetPath: params.symlinkTarget, + }); return resolved; } return await realRealpath(...args);