diff --git a/src/agents/sandbox-paths.test.ts b/src/agents/sandbox-paths.test.ts index 5d906222216d..9c43d97646d7 100644 --- a/src/agents/sandbox-paths.test.ts +++ b/src/agents/sandbox-paths.test.ts @@ -7,6 +7,7 @@ import { describe, expect, it, vi } from "vitest"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { withEnvAsync } from "../test-utils/env.js"; import { + assertSandboxPath, resolveAllowedManagedMediaPath, resolveSandboxedMediaSource, resolveSandboxPath, @@ -116,6 +117,118 @@ describe("resolveSandboxPath", () => { }); }); +describe("assertSandboxPath", () => { + it.runIf(process.platform !== "win32")( + "rejects symlink-then-dot-dot traversal for existing and new files", + async () => { + const parent = await fs.realpath( + await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-symlink-dotdot-")), + ); + const root = path.join(parent, "workspace"); + const outside = path.join(parent, "outside"); + try { + await fs.mkdir(path.join(root, "sub"), { recursive: true }); + await fs.mkdir(outside); + await fs.symlink("..", path.join(root, "sub", "up")); + await fs.writeFile(path.join(outside, "secret.txt"), "outside", "utf8"); + + const escapedRead = `${root}/sub/up/../outside/secret.txt`; + await expect(fs.readFile(escapedRead, "utf8")).resolves.toBe("outside"); + await expect(assertSandboxPath({ filePath: escapedRead, cwd: root, root })).rejects.toThrow( + /escapes sandbox root/i, + ); + await expect( + assertSandboxPath({ + filePath: `${root}/sub/up/../outside/new.txt`, + cwd: root, + root, + }), + ).rejects.toThrow(/escapes sandbox root/i); + await expect( + assertSandboxPath({ filePath: `${root}/sub/up/../..`, cwd: root, root }), + ).rejects.toThrow(/escapes sandbox root/i); + + await fs.mkdir(path.join(root, "a")); + await fs.mkdir(path.join(root, "b")); + await fs.symlink("../b", path.join(root, "a", "up")); + await fs.symlink(path.join(outside, "secret.txt"), path.join(root, "escape")); + const escapedFinalSymlink = `${root}/a/up/../escape`; + await expect(fs.readFile(escapedFinalSymlink, "utf8")).resolves.toBe("outside"); + await expect( + assertSandboxPath({ filePath: escapedFinalSymlink, cwd: root, root }), + ).rejects.toThrow(/symlink escapes sandbox root/i); + + await fs.symlink(outside, path.join(root, "outside-link")); + await expect( + assertSandboxPath({ + filePath: `${root}/outside-link/`, + cwd: root, + root, + allowFinalSymlinkForUnlink: true, + }), + ).rejects.toThrow(/escapes sandbox root/i); + + await fs.mkdir(path.join(root, "real")); + await fs.symlink(path.join(root, "real"), path.join(root, "in-root-link")); + await expect( + assertSandboxPath({ + filePath: `${root}/in-root-link/../real/new.txt`, + cwd: root, + root, + }), + ).resolves.toBeTruthy(); + await expect(assertSandboxPath({ filePath: root, cwd: root, root })).resolves.toBeTruthy(); + } finally { + await fs.rm(parent, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform === "win32")( + "pins Win32 junction-then-dot-dot to lexical traversal semantics", + async () => { + const parent = await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-junction-dotdot-")); + const root = path.join(parent, "workspace"); + const outside = path.join(parent, "outside"); + try { + await fs.mkdir(path.join(root, "sub"), { recursive: true }); + await fs.mkdir(outside); + await fs.symlink(root, path.join(root, "sub", "up"), "junction"); + await fs.writeFile(path.join(outside, "secret.txt"), "outside", "utf8"); + const attemptedEscape = `${root}\\sub\\up\\..\\outside\\secret.txt`; + + await expect(fs.readFile(attemptedEscape, "utf8")).rejects.toMatchObject({ + code: "ENOENT", + }); + } finally { + await fs.rm(parent, { recursive: true, force: true }); + } + }, + ); + + it("accepts not-yet-created and symlinked roots", async () => { + const parent = await fs.realpath( + await fs.mkdtemp(path.join(os.tmpdir(), "sandbox-missing-root-")), + ); + try { + const root = path.join(parent, "workspace"); + await expect( + assertSandboxPath({ filePath: "nested/new.txt", cwd: root, root }), + ).resolves.toMatchObject({ relative: path.join("nested", "new.txt") }); + + const realRoot = path.join(parent, "real-workspace"); + const linkedRoot = path.join(parent, "linked-workspace"); + await fs.mkdir(realRoot); + await fs.symlink(realRoot, linkedRoot); + await expect( + assertSandboxPath({ filePath: linkedRoot, cwd: linkedRoot, root: linkedRoot }), + ).resolves.toMatchObject({ relative: "" }); + } finally { + await fs.rm(parent, { recursive: true, force: true }); + } + }); +}); + describe("resolveSandboxedMediaSource", () => { const openClawTmpDir = resolvePreferredOpenClawTmpDir(); diff --git a/src/agents/sandbox-paths.ts b/src/agents/sandbox-paths.ts index b843542f9ea2..c53c21f8259c 100644 --- a/src/agents/sandbox-paths.ts +++ b/src/agents/sandbox-paths.ts @@ -3,9 +3,11 @@ * * Handles host paths, file URLs, temporary media paths, and workspace root assertions. */ +import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { URL } from "node:url"; +import { promisify } from "node:util"; import { isPassThroughRemoteMediaSource } from "@openclaw/media-core/media-source-url"; import { isWindowsDrivePath } from "../infra/archive-path.js"; import { @@ -14,7 +16,7 @@ import { safeFileURLToPath, } from "../infra/local-file-access.js"; import { assertNoPathAliasEscape, type PathAliasPolicy } from "../infra/path-alias-guards.js"; -import { isPathInside } from "../infra/path-guards.js"; +import { isNotFoundPathError, isPathInside } from "../infra/path-guards.js"; import { resolvePreferredOpenClawTmpDir } from "../infra/tmp-openclaw-dir.js"; import { resolveConfigDir, shortenHomePath } from "../utils.js"; @@ -87,6 +89,66 @@ export function resolveSandboxPath(params: { filePath: string; cwd: string; root return { resolved, relative }; } +const realpathNative = promisify(fs.realpath.native); + +async function resolveRawPathViaExistingAncestor(rawPath: string): Promise { + let cursor = rawPath; + const missingSuffix: string[] = []; + while (true) { + try { + return path.resolve(await realpathNative(cursor), ...missingSuffix); + } catch (error) { + if (!isNotFoundPathError(error)) { + throw error; + } + const parent = path.dirname(cursor); + if (parent === cursor) { + throw error; + } + missingSuffix.unshift(path.basename(cursor)); + cursor = parent; + } + } +} + +async function assertRawParentWithinRoot(params: { + filePath: string; + cwd: string; + root: string; +}): Promise { + // Win32 resolves reparse-point/.. paths lexically, so it has no equivalent escape. + // Avoid adding another realpath to this hot path on Windows, where it is expensive. + if (process.platform === "win32") { + return resolveSandboxInputPath(params.filePath, params.cwd); + } + const expanded = expandPath(params.filePath); + if (isWindowsDrivePath(expanded)) { + return path.win32.normalize(expanded); + } + // Do not use path.resolve here: it would erase the symlink-sensitive `..` before + // native realpath can traverse the raw parent chain. The final component stays + // unresolved so assertNoPathAliasEscape retains final-link policy ownership. + const rawAbsolute = path.isAbsolute(expanded) ? expanded : `${params.cwd}${path.sep}${expanded}`; + const hasTrailingSeparator = rawAbsolute.endsWith(path.sep); + const rawParent = hasTrailingSeparator ? rawAbsolute : path.dirname(rawAbsolute); + const finalSegment = hasTrailingSeparator ? "." : path.basename(rawAbsolute); + const rootResolved = path.resolve(params.root); + const [rootCanonical, parentCanonical] = await Promise.all([ + resolveRawPathViaExistingAncestor(rootResolved), + resolveRawPathViaExistingAncestor(rawParent), + ]); + const targetCanonical = + path.resolve(rawAbsolute) === rootResolved + ? await resolveRawPathViaExistingAncestor(rawAbsolute) + : path.resolve(parentCanonical, finalSegment); + if (targetCanonical !== rootCanonical && !isPathInside(rootCanonical, targetCanonical)) { + throw new Error( + `Path escapes sandbox root (${shortenHomePath(rootCanonical)}): ${params.filePath}`, + ); + } + return targetCanonical; +} + export async function assertSandboxPath(params: { filePath: string; cwd: string; @@ -105,6 +167,17 @@ export async function assertSandboxPath(params: { boundaryLabel: "sandbox root", policy, }); + // The alias guard owns its specific symlink/hardlink errors; this closes the raw + // symlink-then-`..` gap that lexical normalization hides from that guard. + const rawTarget = await assertRawParentWithinRoot(params); + if (path.resolve(rawTarget) !== path.resolve(resolved.resolved)) { + await assertNoPathAliasEscape({ + absolutePath: rawTarget, + rootPath: params.root, + boundaryLabel: "sandbox root", + policy, + }); + } return resolved; }