From c267b5edf6752c0304a0f2bfc4b1d93461f931d6 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Wed, 25 Feb 2026 02:01:12 +0000 Subject: [PATCH] refactor(sandbox): unify tmp alias checks and dedupe hardlink tests --- src/agents/sandbox-paths.test.ts | 129 ++++++++++++++++--------------- src/agents/sandbox-paths.ts | 15 +++- 2 files changed, 79 insertions(+), 65 deletions(-) diff --git a/src/agents/sandbox-paths.test.ts b/src/agents/sandbox-paths.test.ts index e25fd2b3a89..305da9eb40a 100644 --- a/src/agents/sandbox-paths.test.ts +++ b/src/agents/sandbox-paths.test.ts @@ -24,6 +24,51 @@ function isPathInside(root: string, target: string): boolean { return relative === "" || (!relative.startsWith("..") && !path.isAbsolute(relative)); } +function makeTmpProbePath(prefix: string): string { + return `${prefix}-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`; +} + +async function withOutsideHardlinkInOpenClawTmp( + params: { + openClawTmpDir: string; + hardlinkPrefix: string; + symlinkPrefix?: string; + }, + run: (paths: { hardlinkPath: string; symlinkPath?: string }) => Promise, +): Promise { + const outsideDir = await fs.mkdtemp(path.join(process.cwd(), "sandbox-media-hardlink-outside-")); + const outsideFile = path.join(outsideDir, "outside-secret.txt"); + const hardlinkPath = path.join(params.openClawTmpDir, makeTmpProbePath(params.hardlinkPrefix)); + const symlinkPath = params.symlinkPrefix + ? path.join(params.openClawTmpDir, makeTmpProbePath(params.symlinkPrefix)) + : undefined; + try { + if (isPathInside(params.openClawTmpDir, outsideFile)) { + return; + } + await fs.writeFile(outsideFile, "secret", "utf8"); + await fs.mkdir(params.openClawTmpDir, { recursive: true }); + try { + await fs.link(outsideFile, hardlinkPath); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "EXDEV") { + return; + } + throw err; + } + if (symlinkPath) { + await fs.symlink(hardlinkPath, symlinkPath); + } + await run({ hardlinkPath, symlinkPath }); + } finally { + if (symlinkPath) { + await fs.rm(symlinkPath, { force: true }); + } + await fs.rm(hardlinkPath, { force: true }); + await fs.rm(outsideDir, { recursive: true, force: true }); + } +} + describe("resolveSandboxedMediaSource", () => { const openClawTmpDir = resolvePreferredOpenClawTmpDir(); @@ -154,76 +199,38 @@ describe("resolveSandboxedMediaSource", () => { if (process.platform === "win32") { return; } - const outsideDir = await fs.mkdtemp( - path.join(process.cwd(), "sandbox-media-hardlink-outside-"), + await withOutsideHardlinkInOpenClawTmp( + { + openClawTmpDir, + hardlinkPrefix: "sandbox-media-hardlink", + }, + async ({ hardlinkPath }) => { + await withSandboxRoot(async (sandboxDir) => { + await expectSandboxRejection(hardlinkPath, sandboxDir, /hard.?link|sandbox/i); + }); + }, ); - const outsideFile = path.join(outsideDir, "outside-secret.txt"); - const hardlinkPath = path.join( - openClawTmpDir, - `sandbox-media-hardlink-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`, - ); - try { - if (isPathInside(openClawTmpDir, outsideFile)) { - return; - } - await fs.writeFile(outsideFile, "secret", "utf8"); - await fs.mkdir(openClawTmpDir, { recursive: true }); - try { - await fs.link(outsideFile, hardlinkPath); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EXDEV") { - return; - } - throw err; - } - await withSandboxRoot(async (sandboxDir) => { - await expectSandboxRejection(hardlinkPath, sandboxDir, /hard.?link|sandbox/i); - }); - } finally { - await fs.rm(hardlinkPath, { force: true }); - await fs.rm(outsideDir, { recursive: true, force: true }); - } }); it("rejects symlinked OpenClaw tmp paths to hardlinked outside files", async () => { if (process.platform === "win32") { return; } - const outsideDir = await fs.mkdtemp( - path.join(process.cwd(), "sandbox-media-hardlink-outside-"), - ); - const outsideFile = path.join(outsideDir, "outside-secret.txt"); - const hardlinkPath = path.join( - openClawTmpDir, - `sandbox-media-hardlink-target-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`, - ); - const symlinkPath = path.join( - openClawTmpDir, - `sandbox-media-hardlink-symlink-${Date.now()}-${Math.random().toString(16).slice(2)}.txt`, - ); - try { - if (isPathInside(openClawTmpDir, outsideFile)) { - return; - } - await fs.writeFile(outsideFile, "secret", "utf8"); - await fs.mkdir(openClawTmpDir, { recursive: true }); - try { - await fs.link(outsideFile, hardlinkPath); - } catch (err) { - if ((err as NodeJS.ErrnoException).code === "EXDEV") { + await withOutsideHardlinkInOpenClawTmp( + { + openClawTmpDir, + hardlinkPrefix: "sandbox-media-hardlink-target", + symlinkPrefix: "sandbox-media-hardlink-symlink", + }, + async ({ symlinkPath }) => { + if (!symlinkPath) { return; } - throw err; - } - await fs.symlink(hardlinkPath, symlinkPath); - await withSandboxRoot(async (sandboxDir) => { - await expectSandboxRejection(symlinkPath, sandboxDir, /hard.?link|sandbox/i); - }); - } finally { - await fs.rm(symlinkPath, { force: true }); - await fs.rm(hardlinkPath, { force: true }); - await fs.rm(outsideDir, { recursive: true, force: true }); - } + await withSandboxRoot(async (sandboxDir) => { + await expectSandboxRejection(symlinkPath, sandboxDir, /hard.?link|sandbox/i); + }); + }, + ); }); // Group 4: Passthrough diff --git a/src/agents/sandbox-paths.ts b/src/agents/sandbox-paths.ts index 04f0230750c..761106e8574 100644 --- a/src/agents/sandbox-paths.ts +++ b/src/agents/sandbox-paths.ts @@ -186,12 +186,19 @@ async function resolveAllowedTmpMediaPath(params: { if (!isPathInside(openClawTmpDir, resolved)) { return undefined; } - await assertNoSymlinkEscape(path.relative(openClawTmpDir, resolved), openClawTmpDir); - await assertNoHardlinkedFinalPath(resolved, openClawTmpDir); + await assertNoTmpAliasEscape({ filePath: resolved, tmpRoot: openClawTmpDir }); return resolved; } -async function assertNoHardlinkedFinalPath(filePath: string, root: string): Promise { +async function assertNoTmpAliasEscape(params: { + filePath: string; + tmpRoot: string; +}): Promise { + await assertNoSymlinkEscape(path.relative(params.tmpRoot, params.filePath), params.tmpRoot); + await assertNoHardlinkedFinalPath(params.filePath, params.tmpRoot); +} + +async function assertNoHardlinkedFinalPath(filePath: string, tmpRoot: string): Promise { let stat: Awaited>; try { stat = await fs.stat(filePath); @@ -206,7 +213,7 @@ async function assertNoHardlinkedFinalPath(filePath: string, root: string): Prom } if (stat.nlink > 1) { throw new Error( - `Hardlinked tmp media path is not allowed under sandbox root (${shortPath(root)}): ${shortPath(filePath)}`, + `Hardlinked tmp media path is not allowed under tmp root (${shortPath(tmpRoot)}): ${shortPath(filePath)}`, ); } }