diff --git a/CHANGELOG.md b/CHANGELOG.md index 2e0274c5a38..f051950d860 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -84,6 +84,7 @@ Docs: https://docs.openclaw.ai - Gateway/auth: allow one trusted device-token retry on shared-token mismatch with recovery hints to prevent reconnect churn during token drift. (#42507) Thanks @joshavant. - Channels/allowlists: remove stale matcher caching so same-array allowlist edits and wildcard replacements take effect immediately, with regression coverage for in-place mutation cases. - Gateway/auth: fail closed when local `gateway.auth.*` SecretRefs are configured but unavailable, instead of silently falling back to `gateway.remote.*` credentials in local mode. Thanks @tdjackey. +- Sandbox/fs bridge: pin staged writes to verified parent directories so temporary write files cannot materialize outside the allowed mount before atomic replace. Thanks @tdjackey. ## 2026.3.8 diff --git a/src/agents/sandbox/fs-bridge-path-safety.ts b/src/agents/sandbox/fs-bridge-path-safety.ts index a18ed500287..dabba7319b6 100644 --- a/src/agents/sandbox/fs-bridge-path-safety.ts +++ b/src/agents/sandbox/fs-bridge-path-safety.ts @@ -23,6 +23,12 @@ export type AnchoredSandboxEntry = { basename: string; }; +export type PinnedSandboxWriteEntry = { + mountRootPath: string; + relativeParentPath: string; + basename: string; +}; + type RunCommand = ( script: string, options?: { @@ -144,6 +150,26 @@ export class SandboxFsPathGuard { }; } + resolvePinnedWriteEntry(target: SandboxResolvedFsPath, action: string): PinnedSandboxWriteEntry { + const basename = path.posix.basename(target.containerPath); + if (!basename || basename === "." || basename === "/") { + throw new Error(`Invalid sandbox entry target: ${target.containerPath}`); + } + const parentPath = normalizeContainerPath(path.posix.dirname(target.containerPath)); + const mount = this.resolveRequiredMount(parentPath, action); + const relativeParentPath = path.posix.relative(mount.containerRoot, parentPath); + if (relativeParentPath.startsWith("..") || path.posix.isAbsolute(relativeParentPath)) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${action}: ${target.containerPath}`, + ); + } + return { + mountRootPath: mount.containerRoot, + relativeParentPath: relativeParentPath === "." ? "" : relativeParentPath, + basename, + }; + } + private pathIsExistingDirectory(hostPath: string): boolean { try { return fs.statSync(hostPath).isDirectory(); diff --git a/src/agents/sandbox/fs-bridge-shell-command-plans.ts b/src/agents/sandbox/fs-bridge-shell-command-plans.ts index 4c1a9b8d64f..e821f6cea7a 100644 --- a/src/agents/sandbox/fs-bridge-shell-command-plans.ts +++ b/src/agents/sandbox/fs-bridge-shell-command-plans.ts @@ -10,18 +10,6 @@ export type SandboxFsCommandPlan = { allowFailure?: boolean; }; -export function buildWriteCommitPlan( - target: SandboxResolvedFsPath, - tempPath: string, -): SandboxFsCommandPlan { - return { - checks: [{ target, options: { action: "write files", requireWritable: true } }], - recheckBeforeCommand: true, - script: 'set -eu; mv -f -- "$1" "$2"', - args: [tempPath, target.containerPath], - }; -} - export function buildMkdirpPlan( target: SandboxResolvedFsPath, anchoredTarget: AnchoredSandboxEntry, diff --git a/src/agents/sandbox/fs-bridge-write-helper.test.ts b/src/agents/sandbox/fs-bridge-write-helper.test.ts new file mode 100644 index 00000000000..75da7046573 --- /dev/null +++ b/src/agents/sandbox/fs-bridge-write-helper.test.ts @@ -0,0 +1,86 @@ +import { spawnSync } from "node:child_process"; +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { SANDBOX_PINNED_WRITE_PYTHON } from "./fs-bridge-write-helper.js"; + +async function withTempRoot(prefix: string, run: (root: string) => Promise): Promise { + const root = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + try { + return await run(root); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } +} + +function runPinnedWrite(params: { + mountRoot: string; + relativeParentPath: string; + basename: string; + mkdir: boolean; + input: string; +}) { + return spawnSync( + "python3", + [ + "-c", + SANDBOX_PINNED_WRITE_PYTHON, + params.mountRoot, + params.relativeParentPath, + params.basename, + params.mkdir ? "1" : "0", + ], + { + input: params.input, + encoding: "utf8", + stdio: ["pipe", "pipe", "pipe"], + }, + ); +} + +describe("sandbox pinned write helper", () => { + it("creates missing parents and writes through a pinned directory fd", async () => { + await withTempRoot("openclaw-write-helper-", async (root) => { + const workspace = path.join(root, "workspace"); + await fs.mkdir(workspace, { recursive: true }); + + const result = runPinnedWrite({ + mountRoot: workspace, + relativeParentPath: "nested/deeper", + basename: "note.txt", + mkdir: true, + input: "hello", + }); + + expect(result.status).toBe(0); + await expect( + fs.readFile(path.join(workspace, "nested", "deeper", "note.txt"), "utf8"), + ).resolves.toBe("hello"); + }); + }); + + it.runIf(process.platform !== "win32")( + "rejects symlink-parent writes instead of materializing a temp file outside the mount", + async () => { + await withTempRoot("openclaw-write-helper-", async (root) => { + const workspace = path.join(root, "workspace"); + const outside = path.join(root, "outside"); + await fs.mkdir(workspace, { recursive: true }); + await fs.mkdir(outside, { recursive: true }); + await fs.symlink(outside, path.join(workspace, "alias")); + + const result = runPinnedWrite({ + mountRoot: workspace, + relativeParentPath: "alias", + basename: "escape.txt", + mkdir: false, + input: "owned", + }); + + expect(result.status).not.toBe(0); + await expect(fs.readFile(path.join(outside, "escape.txt"), "utf8")).rejects.toThrow(); + }); + }, + ); +}); diff --git a/src/agents/sandbox/fs-bridge-write-helper.ts b/src/agents/sandbox/fs-bridge-write-helper.ts new file mode 100644 index 00000000000..a8a30388799 --- /dev/null +++ b/src/agents/sandbox/fs-bridge-write-helper.ts @@ -0,0 +1,109 @@ +import type { PathSafetyCheck, PinnedSandboxWriteEntry } from "./fs-bridge-path-safety.js"; +import type { SandboxFsCommandPlan } from "./fs-bridge-shell-command-plans.js"; + +export const SANDBOX_PINNED_WRITE_PYTHON = [ + "import errno", + "import os", + "import secrets", + "import sys", + "", + "mount_root = sys.argv[1]", + "relative_parent = sys.argv[2]", + "basename = sys.argv[3]", + 'mkdir_enabled = sys.argv[4] == "1"', + "", + "DIR_FLAGS = os.O_RDONLY", + "if hasattr(os, 'O_DIRECTORY'):", + " DIR_FLAGS |= os.O_DIRECTORY", + "if hasattr(os, 'O_NOFOLLOW'):", + " DIR_FLAGS |= os.O_NOFOLLOW", + "", + "WRITE_FLAGS = os.O_WRONLY | os.O_CREAT | os.O_EXCL", + "if hasattr(os, 'O_NOFOLLOW'):", + " WRITE_FLAGS |= os.O_NOFOLLOW", + "", + "def open_dir(path, dir_fd=None):", + " return os.open(path, DIR_FLAGS, dir_fd=dir_fd)", + "", + "def walk_parent(root_fd, rel_parent, mkdir_enabled):", + " current_fd = os.dup(root_fd)", + " try:", + " segments = [segment for segment in rel_parent.split('/') if segment and segment != '.']", + " for segment in segments:", + " if segment == '..':", + " raise OSError(errno.EPERM, 'path traversal is not allowed', segment)", + " try:", + " next_fd = open_dir(segment, dir_fd=current_fd)", + " except FileNotFoundError:", + " if not mkdir_enabled:", + " raise", + " os.mkdir(segment, 0o777, dir_fd=current_fd)", + " next_fd = open_dir(segment, dir_fd=current_fd)", + " os.close(current_fd)", + " current_fd = next_fd", + " return current_fd", + " except Exception:", + " os.close(current_fd)", + " raise", + "", + "def create_temp_file(parent_fd, basename):", + " prefix = '.openclaw-write-' + basename + '.'", + " for _ in range(128):", + " candidate = prefix + secrets.token_hex(6)", + " try:", + " fd = os.open(candidate, WRITE_FLAGS, 0o600, dir_fd=parent_fd)", + " return candidate, fd", + " except FileExistsError:", + " continue", + " raise RuntimeError('failed to allocate sandbox temp file')", + "", + "root_fd = open_dir(mount_root)", + "parent_fd = None", + "temp_fd = None", + "temp_name = None", + "try:", + " parent_fd = walk_parent(root_fd, relative_parent, mkdir_enabled)", + " temp_name, temp_fd = create_temp_file(parent_fd, basename)", + " while True:", + " chunk = sys.stdin.buffer.read(65536)", + " if not chunk:", + " break", + " os.write(temp_fd, chunk)", + " os.fsync(temp_fd)", + " os.close(temp_fd)", + " temp_fd = None", + " os.replace(temp_name, basename, src_dir_fd=parent_fd, dst_dir_fd=parent_fd)", + " os.fsync(parent_fd)", + "except Exception:", + " if temp_fd is not None:", + " os.close(temp_fd)", + " temp_fd = None", + " if temp_name is not None and parent_fd is not None:", + " try:", + " os.unlink(temp_name, dir_fd=parent_fd)", + " except FileNotFoundError:", + " pass", + " raise", + "finally:", + " if parent_fd is not None:", + " os.close(parent_fd)", + " os.close(root_fd)", +].join("\n"); + +export function buildPinnedWritePlan(params: { + check: PathSafetyCheck; + pinned: PinnedSandboxWriteEntry; + mkdir: boolean; +}): SandboxFsCommandPlan & { stdin?: Buffer | string } { + return { + checks: [params.check], + recheckBeforeCommand: true, + script: ["set -eu", "python3 - \"$@\" <<'PY'", SANDBOX_PINNED_WRITE_PYTHON, "PY"].join("\n"), + args: [ + params.pinned.mountRootPath, + params.pinned.relativeParentPath, + params.pinned.basename, + params.mkdir ? "1" : "0", + ], + }; +} diff --git a/src/agents/sandbox/fs-bridge.shell.test.ts b/src/agents/sandbox/fs-bridge.shell.test.ts index d8b29c0f5d5..5b27f210333 100644 --- a/src/agents/sandbox/fs-bridge.shell.test.ts +++ b/src/agents/sandbox/fs-bridge.shell.test.ts @@ -130,11 +130,11 @@ describe("sandbox fs bridge shell compatibility", () => { const scripts = getScriptsFromCalls(); expect(scripts.some((script) => script.includes('cat >"$1"'))).toBe(false); - expect(scripts.some((script) => script.includes('cat >"$tmp"'))).toBe(true); - expect(scripts.some((script) => script.includes('mv -f -- "$1" "$2"'))).toBe(true); + expect(scripts.some((script) => script.includes('cat >"$tmp"'))).toBe(false); + expect(scripts.some((script) => script.includes("os.replace("))).toBe(true); }); - it("re-validates target before final rename and cleans temp file on failure", async () => { + it("re-validates target before the pinned write helper runs", async () => { const { mockedOpenBoundaryFile } = await import("./fs-bridge.test-helpers.js"); mockedOpenBoundaryFile .mockImplementationOnce(async () => ({ ok: false, reason: "path" })) @@ -150,8 +150,6 @@ describe("sandbox fs bridge shell compatibility", () => { ); const scripts = getScriptsFromCalls(); - expect(scripts.some((script) => script.includes("mktemp"))).toBe(true); - expect(scripts.some((script) => script.includes('mv -f -- "$1" "$2"'))).toBe(false); - expect(scripts.some((script) => script.includes('rm -f -- "$1"'))).toBe(true); + expect(scripts.some((script) => script.includes("os.replace("))).toBe(false); }); }); diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index f937ad2c702..67fedf3b515 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -6,15 +6,14 @@ import { buildRemovePlan, buildRenamePlan, buildStatPlan, - buildWriteCommitPlan, type SandboxFsCommandPlan, } from "./fs-bridge-shell-command-plans.js"; +import { buildPinnedWritePlan } from "./fs-bridge-write-helper.js"; import { buildSandboxFsMounts, resolveSandboxFsPathWithMounts, type SandboxResolvedFsPath, } from "./fs-paths.js"; -import { normalizeContainerPath } from "./path-utils.js"; import type { SandboxContext, SandboxWorkspaceAccess } from "./types.js"; type RunCommandOptions = { @@ -112,26 +111,24 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { }): Promise { const target = this.resolveResolvedPath(params); this.ensureWriteAccess(target, "write files"); - await this.pathGuard.assertPathSafety(target, { action: "write files", requireWritable: true }); + const writeCheck = { + target, + options: { action: "write files", requireWritable: true } as const, + }; + await this.pathGuard.assertPathSafety(target, writeCheck.options); const buffer = Buffer.isBuffer(params.data) ? params.data : Buffer.from(params.data, params.encoding ?? "utf8"); - const tempPath = await this.writeFileToTempPath({ - targetContainerPath: target.containerPath, - mkdir: params.mkdir !== false, - data: buffer, + const pinnedWriteTarget = this.pathGuard.resolvePinnedWriteEntry(target, "write files"); + await this.runCheckedCommand({ + ...buildPinnedWritePlan({ + check: writeCheck, + pinned: pinnedWriteTarget, + mkdir: params.mkdir !== false, + }), + stdin: buffer, signal: params.signal, }); - - try { - await this.runCheckedCommand({ - ...buildWriteCommitPlan(target, tempPath), - signal: params.signal, - }); - } catch (error) { - await this.cleanupTempPath(tempPath, params.signal); - throw error; - } } async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { @@ -265,58 +262,6 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { return await this.runCheckedCommand({ ...plan, signal }); } - private async writeFileToTempPath(params: { - targetContainerPath: string; - mkdir: boolean; - data: Buffer; - signal?: AbortSignal; - }): Promise { - const script = params.mkdir - ? [ - "set -eu", - 'target="$1"', - 'dir=$(dirname -- "$target")', - 'if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi', - 'base=$(basename -- "$target")', - 'tmp=$(mktemp "$dir/.openclaw-write-$base.XXXXXX")', - 'cat >"$tmp"', - 'printf "%s\\n" "$tmp"', - ].join("\n") - : [ - "set -eu", - 'target="$1"', - 'dir=$(dirname -- "$target")', - 'base=$(basename -- "$target")', - 'tmp=$(mktemp "$dir/.openclaw-write-$base.XXXXXX")', - 'cat >"$tmp"', - 'printf "%s\\n" "$tmp"', - ].join("\n"); - const result = await this.runCommand(script, { - args: [params.targetContainerPath], - stdin: params.data, - signal: params.signal, - }); - const tempPath = result.stdout.toString("utf8").trim().split(/\r?\n/).at(-1)?.trim(); - if (!tempPath || !tempPath.startsWith("/")) { - throw new Error( - `Failed to create temporary sandbox write path for ${params.targetContainerPath}`, - ); - } - return normalizeContainerPath(tempPath); - } - - private async cleanupTempPath(tempPath: string, signal?: AbortSignal): Promise { - try { - await this.runCommand('set -eu; rm -f -- "$1"', { - args: [tempPath], - signal, - allowFailure: true, - }); - } catch { - // Best-effort cleanup only. - } - } - private ensureWriteAccess(target: SandboxResolvedFsPath, action: string) { if (!allowsWrites(this.sandbox.workspaceAccess) || !target.writable) { throw new Error(`Sandbox path is read-only; cannot ${action}: ${target.containerPath}`);