fix(security): pin staged writes and fs mutations

This commit is contained in:
Peter Steinberger
2026-03-11 02:18:11 +00:00
parent daaf211e20
commit a0d5462571
11 changed files with 1072 additions and 388 deletions

View File

@@ -3,7 +3,7 @@ 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_FS_MUTATION_PYTHON } from "./fs-bridge-mutation-helper.js";
import { SANDBOX_PINNED_MUTATION_PYTHON } from "./fs-bridge-mutation-helper.js";
async function withTempRoot<T>(prefix: string, run: (root: string) => Promise<T>): Promise<T> {
const root = await fs.mkdtemp(path.join(os.tmpdir(), prefix));
@@ -14,33 +14,21 @@ async function withTempRoot<T>(prefix: string, run: (root: string) => Promise<T>
}
}
function runPinnedMutation(params: {
op: "write" | "mkdirp" | "remove" | "rename";
args: string[];
input?: string;
}) {
return spawnSync(
"python3",
["-c", SANDBOX_PINNED_FS_MUTATION_PYTHON, params.op, ...params.args],
{
input: params.input,
encoding: "utf8",
stdio: ["pipe", "pipe", "pipe"],
},
);
function runMutation(args: string[], input?: string) {
return spawnSync("python3", ["-c", SANDBOX_PINNED_MUTATION_PYTHON, ...args], {
input,
encoding: "utf8",
stdio: ["pipe", "pipe", "pipe"],
});
}
describe("sandbox pinned mutation helper", () => {
it("creates missing parents and writes through a pinned directory fd", async () => {
await withTempRoot("openclaw-write-helper-", async (root) => {
it("writes through a pinned directory fd", async () => {
await withTempRoot("openclaw-mutation-helper-", async (root) => {
const workspace = path.join(root, "workspace");
await fs.mkdir(workspace, { recursive: true });
const result = runPinnedMutation({
op: "write",
args: [workspace, "nested/deeper", "note.txt", "1"],
input: "hello",
});
const result = runMutation(["write", workspace, "nested/deeper", "note.txt", "1"], "hello");
expect(result.status).toBe(0);
await expect(
@@ -52,22 +40,104 @@ describe("sandbox pinned mutation helper", () => {
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) => {
await withTempRoot("openclaw-mutation-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 = runPinnedMutation({
op: "write",
args: [workspace, "alias", "escape.txt", "0"],
input: "owned",
});
const result = runMutation(["write", workspace, "alias", "escape.txt", "0"], "owned");
expect(result.status).not.toBe(0);
await expect(fs.readFile(path.join(outside, "escape.txt"), "utf8")).rejects.toThrow();
});
},
);
it.runIf(process.platform !== "win32")("rejects symlink segments during mkdirp", async () => {
await withTempRoot("openclaw-mutation-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 = runMutation(["mkdirp", workspace, "alias/nested"]);
expect(result.status).not.toBe(0);
await expect(fs.readFile(path.join(outside, "nested"), "utf8")).rejects.toThrow();
});
});
it.runIf(process.platform !== "win32")("remove unlinks the symlink itself", async () => {
await withTempRoot("openclaw-mutation-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.writeFile(path.join(outside, "secret.txt"), "classified", "utf8");
await fs.symlink(path.join(outside, "secret.txt"), path.join(workspace, "link.txt"));
const result = runMutation(["remove", workspace, "", "link.txt", "0", "0"]);
expect(result.status).toBe(0);
await expect(fs.readlink(path.join(workspace, "link.txt"))).rejects.toThrow();
await expect(fs.readFile(path.join(outside, "secret.txt"), "utf8")).resolves.toBe(
"classified",
);
});
});
it.runIf(process.platform !== "win32")(
"rejects symlink destination parents during rename",
async () => {
await withTempRoot("openclaw-mutation-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.writeFile(path.join(workspace, "from.txt"), "payload", "utf8");
await fs.symlink(outside, path.join(workspace, "alias"));
const result = runMutation([
"rename",
workspace,
"",
"from.txt",
workspace,
"alias",
"escape.txt",
"1",
]);
expect(result.status).not.toBe(0);
await expect(fs.readFile(path.join(workspace, "from.txt"), "utf8")).resolves.toBe(
"payload",
);
await expect(fs.readFile(path.join(outside, "escape.txt"), "utf8")).rejects.toThrow();
});
},
);
it.runIf(process.platform !== "win32")(
"copies directories across different mount roots during rename fallback",
async () => {
await withTempRoot("openclaw-mutation-helper-", async (root) => {
const sourceRoot = path.join(root, "source");
const destRoot = path.join(root, "dest");
await fs.mkdir(path.join(sourceRoot, "dir", "nested"), { recursive: true });
await fs.mkdir(destRoot, { recursive: true });
await fs.writeFile(path.join(sourceRoot, "dir", "nested", "file.txt"), "payload", "utf8");
const result = runMutation(["rename", sourceRoot, "", "dir", destRoot, "", "moved", "1"]);
expect(result.status).toBe(0);
await expect(
fs.readFile(path.join(destRoot, "moved", "nested", "file.txt"), "utf8"),
).resolves.toBe("payload");
await expect(fs.stat(path.join(sourceRoot, "dir"))).rejects.toThrow();
});
},
);
});

View File

@@ -1,38 +1,274 @@
import { PATH_ALIAS_POLICIES } from "../../infra/path-alias-guards.js";
import { SANDBOX_PINNED_FS_MUTATION_PYTHON } from "./fs-bridge-mutation-python-source.js";
import type { PinnedSandboxEntry } from "./fs-bridge-path-safety.js";
import type {
PathSafetyCheck,
PinnedSandboxDirectoryEntry,
PinnedSandboxEntry,
} from "./fs-bridge-path-safety.js";
import type { SandboxFsCommandPlan } from "./fs-bridge-shell-command-plans.js";
import type { SandboxResolvedFsPath } from "./fs-paths.js";
export const SANDBOX_PINNED_MUTATION_PYTHON = [
"import errno",
"import os",
"import secrets",
"import stat",
"import sys",
"",
"operation = sys.argv[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",
"",
"READ_FLAGS = os.O_RDONLY",
"if hasattr(os, 'O_NOFOLLOW'):",
" READ_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 split_relative(path_value):",
" segments = []",
" for segment in path_value.split('/'):",
" if not segment or segment == '.':",
" continue",
" if segment == '..':",
" raise OSError(errno.EPERM, 'path traversal is not allowed', segment)",
" segments.append(segment)",
" return segments",
"",
"def open_dir(path_value, dir_fd=None):",
" return os.open(path_value, DIR_FLAGS, dir_fd=dir_fd)",
"",
"def walk_dir(root_fd, rel_path, mkdir_enabled):",
" current_fd = os.dup(root_fd)",
" try:",
" for segment in split_relative(rel_path):",
" 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')",
"",
"def create_temp_dir(parent_fd, basename, mode):",
" prefix = '.openclaw-move-' + basename + '.'",
" for _ in range(128):",
" candidate = prefix + secrets.token_hex(6)",
" try:",
" os.mkdir(candidate, mode, dir_fd=parent_fd)",
" return candidate",
" except FileExistsError:",
" continue",
" raise RuntimeError('failed to allocate sandbox temp directory')",
"",
"def write_atomic(parent_fd, basename, stdin_buffer):",
" temp_fd = None",
" temp_name = None",
" try:",
" temp_name, temp_fd = create_temp_file(parent_fd, basename)",
" while True:",
" chunk = 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)",
" temp_name = None",
" os.fsync(parent_fd)",
" finally:",
" if temp_fd is not None:",
" os.close(temp_fd)",
" if temp_name is not None:",
" try:",
" os.unlink(temp_name, dir_fd=parent_fd)",
" except FileNotFoundError:",
" pass",
"",
"def remove_tree(parent_fd, basename):",
" entry_stat = os.lstat(basename, dir_fd=parent_fd)",
" if not stat.S_ISDIR(entry_stat.st_mode) or stat.S_ISLNK(entry_stat.st_mode):",
" os.unlink(basename, dir_fd=parent_fd)",
" return",
" dir_fd = open_dir(basename, dir_fd=parent_fd)",
" try:",
" for child in os.listdir(dir_fd):",
" remove_tree(dir_fd, child)",
" finally:",
" os.close(dir_fd)",
" os.rmdir(basename, dir_fd=parent_fd)",
"",
"def move_entry(src_parent_fd, src_basename, dst_parent_fd, dst_basename):",
" try:",
" os.rename(src_basename, dst_basename, src_dir_fd=src_parent_fd, dst_dir_fd=dst_parent_fd)",
" os.fsync(dst_parent_fd)",
" os.fsync(src_parent_fd)",
" return",
" except OSError as err:",
" if err.errno != errno.EXDEV:",
" raise",
" src_stat = os.lstat(src_basename, dir_fd=src_parent_fd)",
" if stat.S_ISDIR(src_stat.st_mode) and not stat.S_ISLNK(src_stat.st_mode):",
" temp_dir_name = create_temp_dir(dst_parent_fd, dst_basename, stat.S_IMODE(src_stat.st_mode) or 0o755)",
" temp_dir_fd = open_dir(temp_dir_name, dir_fd=dst_parent_fd)",
" src_dir_fd = open_dir(src_basename, dir_fd=src_parent_fd)",
" try:",
" for child in os.listdir(src_dir_fd):",
" move_entry(src_dir_fd, child, temp_dir_fd, child)",
" finally:",
" os.close(src_dir_fd)",
" os.close(temp_dir_fd)",
" os.rename(temp_dir_name, dst_basename, src_dir_fd=dst_parent_fd, dst_dir_fd=dst_parent_fd)",
" os.rmdir(src_basename, dir_fd=src_parent_fd)",
" os.fsync(dst_parent_fd)",
" os.fsync(src_parent_fd)",
" return",
" if stat.S_ISLNK(src_stat.st_mode):",
" link_target = os.readlink(src_basename, dir_fd=src_parent_fd)",
" try:",
" os.unlink(dst_basename, dir_fd=dst_parent_fd)",
" except FileNotFoundError:",
" pass",
" os.symlink(link_target, dst_basename, dir_fd=dst_parent_fd)",
" os.unlink(src_basename, dir_fd=src_parent_fd)",
" os.fsync(dst_parent_fd)",
" os.fsync(src_parent_fd)",
" return",
" src_fd = os.open(src_basename, READ_FLAGS, dir_fd=src_parent_fd)",
" temp_fd = None",
" temp_name = None",
" try:",
" temp_name, temp_fd = create_temp_file(dst_parent_fd, dst_basename)",
" while True:",
" chunk = os.read(src_fd, 65536)",
" if not chunk:",
" break",
" os.write(temp_fd, chunk)",
" try:",
" os.fchmod(temp_fd, stat.S_IMODE(src_stat.st_mode))",
" except AttributeError:",
" pass",
" os.fsync(temp_fd)",
" os.close(temp_fd)",
" temp_fd = None",
" os.replace(temp_name, dst_basename, src_dir_fd=dst_parent_fd, dst_dir_fd=dst_parent_fd)",
" temp_name = None",
" os.unlink(src_basename, dir_fd=src_parent_fd)",
" os.fsync(dst_parent_fd)",
" os.fsync(src_parent_fd)",
" finally:",
" if temp_fd is not None:",
" os.close(temp_fd)",
" if temp_name is not None:",
" try:",
" os.unlink(temp_name, dir_fd=dst_parent_fd)",
" except FileNotFoundError:",
" pass",
" os.close(src_fd)",
"",
"if operation == 'write':",
" root_fd = open_dir(sys.argv[2])",
" parent_fd = None",
" try:",
" parent_fd = walk_dir(root_fd, sys.argv[3], sys.argv[5] == '1')",
" write_atomic(parent_fd, sys.argv[4], sys.stdin.buffer)",
" finally:",
" if parent_fd is not None:",
" os.close(parent_fd)",
" os.close(root_fd)",
"elif operation == 'mkdirp':",
" root_fd = open_dir(sys.argv[2])",
" target_fd = None",
" try:",
" target_fd = walk_dir(root_fd, sys.argv[3], True)",
" os.fsync(target_fd)",
" finally:",
" if target_fd is not None:",
" os.close(target_fd)",
" os.close(root_fd)",
"elif operation == 'remove':",
" root_fd = open_dir(sys.argv[2])",
" parent_fd = None",
" try:",
" parent_fd = walk_dir(root_fd, sys.argv[3], False)",
" try:",
" if sys.argv[5] == '1':",
" remove_tree(parent_fd, sys.argv[4])",
" else:",
" entry_stat = os.lstat(sys.argv[4], dir_fd=parent_fd)",
" if stat.S_ISDIR(entry_stat.st_mode) and not stat.S_ISLNK(entry_stat.st_mode):",
" os.rmdir(sys.argv[4], dir_fd=parent_fd)",
" else:",
" os.unlink(sys.argv[4], dir_fd=parent_fd)",
" os.fsync(parent_fd)",
" except FileNotFoundError:",
" if sys.argv[6] != '1':",
" raise",
" finally:",
" if parent_fd is not None:",
" os.close(parent_fd)",
" os.close(root_fd)",
"elif operation == 'rename':",
" src_root_fd = open_dir(sys.argv[2])",
" dst_root_fd = open_dir(sys.argv[5])",
" src_parent_fd = None",
" dst_parent_fd = None",
" try:",
" src_parent_fd = walk_dir(src_root_fd, sys.argv[3], False)",
" dst_parent_fd = walk_dir(dst_root_fd, sys.argv[6], sys.argv[8] == '1')",
" move_entry(src_parent_fd, sys.argv[4], dst_parent_fd, sys.argv[7])",
" finally:",
" if src_parent_fd is not None:",
" os.close(src_parent_fd)",
" if dst_parent_fd is not None:",
" os.close(dst_parent_fd)",
" os.close(src_root_fd)",
" os.close(dst_root_fd)",
"else:",
" raise RuntimeError('unknown sandbox mutation operation: ' + operation)",
].join("\n");
function buildPinnedMutationPlan(params: {
checks: SandboxFsCommandPlan["checks"];
args: string[];
stdin?: Buffer | string;
checks: PathSafetyCheck[];
}): SandboxFsCommandPlan {
return {
checks: params.checks,
recheckBeforeCommand: true,
script: ["set -eu", "python3 - \"$@\" <<'PY'", SANDBOX_PINNED_FS_MUTATION_PYTHON, "PY"].join(
"\n",
),
script: ["set -eu", "python3 - \"$@\" <<'PY'", SANDBOX_PINNED_MUTATION_PYTHON, "PY"].join("\n"),
args: params.args,
stdin: params.stdin,
};
}
export function buildPinnedWritePlan(params: {
target: SandboxResolvedFsPath;
check: PathSafetyCheck;
pinned: PinnedSandboxEntry;
mkdir: boolean;
stdin: Buffer | string;
}): SandboxFsCommandPlan {
return buildPinnedMutationPlan({
checks: [
{
target: params.target,
options: { action: "write files", requireWritable: true },
},
],
checks: [params.check],
args: [
"write",
params.pinned.mountRootPath,
@@ -40,36 +276,21 @@ export function buildPinnedWritePlan(params: {
params.pinned.basename,
params.mkdir ? "1" : "0",
],
stdin: params.stdin,
});
}
export function buildPinnedMkdirpPlan(params: {
target: SandboxResolvedFsPath;
pinned: PinnedSandboxEntry;
check: PathSafetyCheck;
pinned: PinnedSandboxDirectoryEntry;
}): SandboxFsCommandPlan {
return buildPinnedMutationPlan({
checks: [
{
target: params.target,
options: {
action: "create directories",
requireWritable: true,
allowedType: "directory",
},
},
],
args: [
"mkdirp",
params.pinned.mountRootPath,
params.pinned.relativeParentPath,
params.pinned.basename,
],
checks: [params.check],
args: ["mkdirp", params.pinned.mountRootPath, params.pinned.relativePath],
});
}
export function buildPinnedRemovePlan(params: {
target: SandboxResolvedFsPath;
check: PathSafetyCheck;
pinned: PinnedSandboxEntry;
recursive?: boolean;
force?: boolean;
@@ -77,10 +298,9 @@ export function buildPinnedRemovePlan(params: {
return buildPinnedMutationPlan({
checks: [
{
target: params.target,
target: params.check.target,
options: {
action: "remove files",
requireWritable: true,
...params.check.options,
aliasPolicy: PATH_ALIAS_POLICIES.unlinkTarget,
},
},
@@ -97,39 +317,31 @@ export function buildPinnedRemovePlan(params: {
}
export function buildPinnedRenamePlan(params: {
from: SandboxResolvedFsPath;
to: SandboxResolvedFsPath;
pinnedFrom: PinnedSandboxEntry;
pinnedTo: PinnedSandboxEntry;
fromCheck: PathSafetyCheck;
toCheck: PathSafetyCheck;
from: PinnedSandboxEntry;
to: PinnedSandboxEntry;
}): SandboxFsCommandPlan {
return buildPinnedMutationPlan({
checks: [
{
target: params.from,
target: params.fromCheck.target,
options: {
action: "rename files",
requireWritable: true,
...params.fromCheck.options,
aliasPolicy: PATH_ALIAS_POLICIES.unlinkTarget,
},
},
{
target: params.to,
options: {
action: "rename files",
requireWritable: true,
},
},
params.toCheck,
],
args: [
"rename",
params.pinnedFrom.mountRootPath,
params.pinnedFrom.relativeParentPath,
params.pinnedFrom.basename,
params.pinnedTo.mountRootPath,
params.pinnedTo.relativeParentPath,
params.pinnedTo.basename,
params.from.mountRootPath,
params.from.relativeParentPath,
params.from.basename,
params.to.mountRootPath,
params.to.relativeParentPath,
params.to.basename,
"1",
],
});
}
export { SANDBOX_PINNED_FS_MUTATION_PYTHON };

View File

@@ -18,17 +18,17 @@ export type PathSafetyCheck = {
options: PathSafetyOptions;
};
export type AnchoredSandboxEntry = {
canonicalParentPath: string;
basename: string;
};
export type PinnedSandboxEntry = {
mountRootPath: string;
relativeParentPath: string;
basename: string;
};
export type PinnedSandboxDirectoryEntry = {
mountRootPath: string;
relativePath: string;
};
type RunCommand = (
script: string,
options?: {
@@ -134,22 +134,14 @@ export class SandboxFsPathGuard {
return guarded;
}
async resolveAnchoredSandboxEntry(target: SandboxResolvedFsPath): Promise<AnchoredSandboxEntry> {
const splitTarget = this.splitSandboxEntryTarget(target);
const canonicalParentPath = await this.resolveCanonicalContainerPath({
containerPath: splitTarget.parentPath,
allowFinalSymlinkForUnlink: false,
});
return {
canonicalParentPath,
basename: splitTarget.basename,
};
}
resolvePinnedMutationEntry(target: SandboxResolvedFsPath, action: string): PinnedSandboxEntry {
const splitTarget = this.splitSandboxEntryTarget(target);
const mount = this.resolveRequiredMount(splitTarget.parentPath, action);
const relativeParentPath = path.posix.relative(mount.containerRoot, splitTarget.parentPath);
resolvePinnedEntry(target: SandboxResolvedFsPath, action: string): PinnedSandboxEntry {
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}`,
@@ -158,21 +150,24 @@ export class SandboxFsPathGuard {
return {
mountRootPath: mount.containerRoot,
relativeParentPath: relativeParentPath === "." ? "" : relativeParentPath,
basename: splitTarget.basename,
basename,
};
}
private splitSandboxEntryTarget(target: SandboxResolvedFsPath): {
basename: string;
parentPath: string;
} {
const basename = path.posix.basename(target.containerPath);
if (!basename || basename === "." || basename === "/") {
throw new Error(`Invalid sandbox entry target: ${target.containerPath}`);
resolvePinnedDirectoryEntry(
target: SandboxResolvedFsPath,
action: string,
): PinnedSandboxDirectoryEntry {
const mount = this.resolveRequiredMount(target.containerPath, action);
const relativePath = path.posix.relative(mount.containerRoot, target.containerPath);
if (relativePath.startsWith("..") || path.posix.isAbsolute(relativePath)) {
throw new Error(
`Sandbox path escapes allowed mounts; cannot ${action}: ${target.containerPath}`,
);
}
return {
parentPath: normalizeContainerPath(path.posix.dirname(target.containerPath)),
basename,
mountRootPath: mount.containerRoot,
relativePath: relativePath === "." ? "" : relativePath,
};
}

View File

@@ -4,7 +4,6 @@ import { describe, expect, it } from "vitest";
import {
createSandbox,
createSandboxFsBridge,
findCallByDockerArg,
getDockerArg,
installFsBridgeTestHarness,
mockedExecDockerRaw,
@@ -66,46 +65,60 @@ describe("sandbox fs bridge anchored ops", () => {
});
});
const anchoredCases = [
const pinnedCases = [
{
name: "mkdirp pins mount root + relative parent + basename",
name: "mkdirp pins mount root + relative path",
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
bridge.mkdirp({ filePath: "nested/leaf" }),
op: "mkdirp",
expectedArgs: ["/workspace", "nested", "leaf"],
forbiddenArgs: ["/workspace/nested/leaf", "/workspace/nested"],
expectedArgs: ["mkdirp", "/workspace", "nested/leaf"],
forbiddenArgs: ["/workspace/nested/leaf"],
},
{
name: "remove pins mount root + relative parent + basename",
name: "remove pins mount root + parent/basename",
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
bridge.remove({ filePath: "nested/file.txt" }),
op: "remove",
expectedArgs: ["/workspace", "nested", "file.txt", "0", "1"],
forbiddenArgs: ["/workspace/nested/file.txt", "/workspace/nested"],
expectedArgs: ["remove", "/workspace", "nested", "file.txt", "0", "1"],
forbiddenArgs: ["/workspace/nested/file.txt"],
},
{
name: "rename pins both parents + basenames",
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
bridge.rename({ from: "from.txt", to: "nested/to.txt" }),
op: "rename",
expectedArgs: ["/workspace", "", "from.txt", "/workspace", "nested", "to.txt"],
forbiddenArgs: ["/workspace/from.txt", "/workspace/nested/to.txt", "/workspace/nested"],
expectedArgs: ["rename", "/workspace", "", "from.txt", "/workspace", "nested", "to.txt", "1"],
forbiddenArgs: ["/workspace/from.txt", "/workspace/nested/to.txt"],
},
] as const;
it.each(anchoredCases)("$name", async (testCase) => {
const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
it.each(pinnedCases)("$name", async (testCase) => {
await withTempDir("openclaw-fs-bridge-contract-write-", async (stateDir) => {
const workspaceDir = path.join(stateDir, "workspace");
await fs.mkdir(path.join(workspaceDir, "nested"), { recursive: true });
await fs.writeFile(path.join(workspaceDir, "from.txt"), "hello", "utf8");
await fs.writeFile(path.join(workspaceDir, "nested", "file.txt"), "bye", "utf8");
await testCase.invoke(bridge);
const bridge = createSandboxFsBridge({
sandbox: createSandbox({
workspaceDir,
agentWorkspaceDir: workspaceDir,
}),
});
const opCall = findCallByDockerArg(1, testCase.op);
expect(opCall).toBeDefined();
const args = opCall?.[0] ?? [];
testCase.expectedArgs.forEach((value, index) => {
expect(getDockerArg(args, index + 2)).toBe(value);
});
testCase.forbiddenArgs.forEach((value) => {
expect(args).not.toContain(value);
await testCase.invoke(bridge);
const opCall = mockedExecDockerRaw.mock.calls.find(
([args]) =>
typeof args[5] === "string" &&
args[5].includes("python3 - \"$@\" <<'PY'") &&
getDockerArg(args, 1) === testCase.expectedArgs[0],
);
expect(opCall).toBeDefined();
const args = opCall?.[0] ?? [];
testCase.expectedArgs.forEach((value, index) => {
expect(getDockerArg(args, index + 1)).toBe(value);
});
testCase.forbiddenArgs.forEach((value) => {
expect(args).not.toContain(value);
});
});
});
});

View File

@@ -45,10 +45,10 @@ describe("sandbox fs bridge shell compatibility", () => {
});
});
it("resolveCanonicalContainerPath script is valid POSIX sh (no do; token)", async () => {
it("path canonicalization recheck script is valid POSIX sh", async () => {
const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
await bridge.mkdirp({ filePath: "nested" });
await bridge.writeFile({ filePath: "b.txt", data: "hello" });
const scripts = getScriptsFromCalls();
const canonicalScript = scripts.find((script) => script.includes("allow_final"));
@@ -134,6 +134,32 @@ describe("sandbox fs bridge shell compatibility", () => {
expect(scripts.some((script) => script.includes("os.replace("))).toBe(true);
});
it("routes mkdirp, remove, and rename through the pinned mutation helper", async () => {
await withTempDir("openclaw-fs-bridge-shell-write-", async (stateDir) => {
const workspaceDir = path.join(stateDir, "workspace");
await fs.mkdir(path.join(workspaceDir, "nested"), { recursive: true });
await fs.writeFile(path.join(workspaceDir, "a.txt"), "hello", "utf8");
await fs.writeFile(path.join(workspaceDir, "nested", "file.txt"), "bye", "utf8");
const bridge = createSandboxFsBridge({
sandbox: createSandbox({
workspaceDir,
agentWorkspaceDir: workspaceDir,
}),
});
await bridge.mkdirp({ filePath: "nested" });
await bridge.remove({ filePath: "nested/file.txt" });
await bridge.rename({ from: "a.txt", to: "nested/b.txt" });
const scripts = getScriptsFromCalls();
expect(scripts.filter((script) => script.includes("operation = sys.argv[1]")).length).toBe(3);
expect(scripts.some((script) => script.includes('mkdir -p -- "$2"'))).toBe(false);
expect(scripts.some((script) => script.includes('rm -f -- "$2"'))).toBe(false);
expect(scripts.some((script) => script.includes('mv -- "$3" "$2/$4"'))).toBe(false);
});
});
it("re-validates target before the pinned write helper runs", async () => {
const { mockedOpenBoundaryFile } = await import("./fs-bridge.test-helpers.js");
mockedOpenBoundaryFile

View File

@@ -54,10 +54,6 @@ export function findCallsByScriptFragment(fragment: string) {
);
}
export function findCallByDockerArg(position: number, value: string) {
return mockedExecDockerRaw.mock.calls.find(([args]) => getDockerArg(args, position) === value);
}
export function dockerExecResult(stdout: string) {
return {
stdout: Buffer.from(stdout),
@@ -146,14 +142,16 @@ export async function expectMkdirpAllowsExistingDirectory(params?: {
await expect(bridge.mkdirp({ filePath: "memory/kemik" })).resolves.toBeUndefined();
const mkdirCall = findCallByDockerArg(1, "mkdirp");
const mkdirCall = mockedExecDockerRaw.mock.calls.find(
([args]) =>
getDockerScript(args).includes("operation = sys.argv[1]") &&
getDockerArg(args, 1) === "mkdirp",
);
expect(mkdirCall).toBeDefined();
const mkdirRoot = mkdirCall ? getDockerArg(mkdirCall[0], 2) : "";
const mkdirParent = mkdirCall ? getDockerArg(mkdirCall[0], 3) : "";
const mkdirBase = mkdirCall ? getDockerArg(mkdirCall[0], 4) : "";
expect(mkdirRoot).toBe("/workspace");
expect(mkdirParent).toBe("memory");
expect(mkdirBase).toBe("kemik");
const mountRoot = mkdirCall ? getDockerArg(mkdirCall[0], 2) : "";
const relativePath = mkdirCall ? getDockerArg(mkdirCall[0], 3) : "";
expect(mountRoot).toBe("/workspace");
expect(relativePath).toBe("memory/kemik");
});
}

View File

@@ -110,29 +110,44 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
}): Promise<void> {
const target = this.resolveResolvedPath(params);
this.ensureWriteAccess(target, "write files");
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 pinnedWriteTarget = this.pathGuard.resolvePinnedMutationEntry(target, "write files");
await this.runCheckedCommand(
buildPinnedWritePlan({
target,
const pinnedWriteTarget = this.pathGuard.resolvePinnedEntry(target, "write files");
await this.runCheckedCommand({
...buildPinnedWritePlan({
check: writeCheck,
pinned: pinnedWriteTarget,
mkdir: params.mkdir !== false,
stdin: buffer,
}),
params.signal,
);
stdin: buffer,
signal: params.signal,
});
}
async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise<void> {
const target = this.resolveResolvedPath(params);
this.ensureWriteAccess(target, "create directories");
const pinnedTarget = this.pathGuard.resolvePinnedMutationEntry(target, "create directories");
await this.runCheckedCommand(
buildPinnedMkdirpPlan({ target, pinned: pinnedTarget }),
params.signal,
);
const mkdirCheck = {
target,
options: {
action: "create directories",
requireWritable: true,
allowedType: "directory",
} as const,
};
await this.runCheckedCommand({
...buildPinnedMkdirpPlan({
check: mkdirCheck,
pinned: this.pathGuard.resolvePinnedDirectoryEntry(target, "create directories"),
}),
signal: params.signal,
});
}
async remove(params: {
@@ -144,16 +159,22 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
}): Promise<void> {
const target = this.resolveResolvedPath(params);
this.ensureWriteAccess(target, "remove files");
const pinnedTarget = this.pathGuard.resolvePinnedMutationEntry(target, "remove files");
await this.runCheckedCommand(
buildPinnedRemovePlan({
target,
pinned: pinnedTarget,
const removeCheck = {
target,
options: {
action: "remove files",
requireWritable: true,
} as const,
};
await this.runCheckedCommand({
...buildPinnedRemovePlan({
check: removeCheck,
pinned: this.pathGuard.resolvePinnedEntry(target, "remove files"),
recursive: params.recursive,
force: params.force,
}),
params.signal,
);
signal: params.signal,
});
}
async rename(params: {
@@ -166,17 +187,29 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
const to = this.resolveResolvedPath({ filePath: params.to, cwd: params.cwd });
this.ensureWriteAccess(from, "rename files");
this.ensureWriteAccess(to, "rename files");
const pinnedFrom = this.pathGuard.resolvePinnedMutationEntry(from, "rename files");
const pinnedTo = this.pathGuard.resolvePinnedMutationEntry(to, "rename files");
await this.runCheckedCommand(
buildPinnedRenamePlan({
from,
to,
pinnedFrom,
pinnedTo,
const fromCheck = {
target: from,
options: {
action: "rename files",
requireWritable: true,
} as const,
};
const toCheck = {
target: to,
options: {
action: "rename files",
requireWritable: true,
} as const,
};
await this.runCheckedCommand({
...buildPinnedRenamePlan({
fromCheck,
toCheck,
from: this.pathGuard.resolvePinnedEntry(from, "rename files"),
to: this.pathGuard.resolvePinnedEntry(to, "rename files"),
}),
params.signal,
);
signal: params.signal,
});
}
async stat(params: {
@@ -185,7 +218,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
signal?: AbortSignal;
}): Promise<SandboxFsStat | null> {
const target = this.resolveResolvedPath(params);
const result = await this.runCheckedCommand(buildStatPlan(target), params.signal);
const result = await this.runPlannedCommand(buildStatPlan(target), params.signal);
if (result.code !== 0) {
const stderr = result.stderr.toString("utf8");
if (stderr.includes("No such file or directory")) {
@@ -238,8 +271,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
}
private async runCheckedCommand(
plan: SandboxFsCommandPlan,
signal?: AbortSignal,
plan: SandboxFsCommandPlan & { stdin?: Buffer | string; signal?: AbortSignal },
): Promise<ExecDockerRawResult> {
await this.pathGuard.assertPathChecks(plan.checks);
if (plan.recheckBeforeCommand) {
@@ -249,10 +281,17 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
args: plan.args,
stdin: plan.stdin,
allowFailure: plan.allowFailure,
signal,
signal: plan.signal,
});
}
private async runPlannedCommand(
plan: SandboxFsCommandPlan,
signal?: AbortSignal,
): Promise<ExecDockerRawResult> {
return await this.runCheckedCommand({ ...plan, signal });
}
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}`);

View File

@@ -0,0 +1,86 @@
import fs from "node:fs/promises";
import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import { createTrackedTempDirs } from "../test-utils/tracked-temp-dirs.js";
import { runPinnedWriteHelper } from "./fs-pinned-write-helper.js";
const tempDirs = createTrackedTempDirs();
afterEach(async () => {
await tempDirs.cleanup();
});
describe("fs pinned write helper", () => {
it.runIf(process.platform !== "win32")("writes through a pinned parent directory", async () => {
const root = await tempDirs.make("openclaw-fs-pinned-root-");
const identity = await runPinnedWriteHelper({
rootPath: root,
relativeParentPath: "nested/deeper",
basename: "note.txt",
mkdir: true,
mode: 0o600,
input: {
kind: "buffer",
data: "hello",
},
});
await expect(
fs.readFile(path.join(root, "nested", "deeper", "note.txt"), "utf8"),
).resolves.toBe("hello");
expect(identity.dev).toBeGreaterThanOrEqual(0);
expect(identity.ino).toBeGreaterThan(0);
});
it.runIf(process.platform !== "win32")(
"rejects symlink-parent writes instead of creating a temp file outside root",
async () => {
const root = await tempDirs.make("openclaw-fs-pinned-root-");
const outside = await tempDirs.make("openclaw-fs-pinned-outside-");
await fs.symlink(outside, path.join(root, "alias"));
await expect(
runPinnedWriteHelper({
rootPath: root,
relativeParentPath: "alias",
basename: "escape.txt",
mkdir: false,
mode: 0o600,
input: {
kind: "buffer",
data: "owned",
},
}),
).rejects.toThrow();
await expect(fs.stat(path.join(outside, "escape.txt"))).rejects.toThrow();
const outsideFiles = await fs.readdir(outside);
expect(outsideFiles).toEqual([]);
},
);
it.runIf(process.platform !== "win32")("accepts streamed input", async () => {
const root = await tempDirs.make("openclaw-fs-pinned-root-");
const sourcePath = path.join(await tempDirs.make("openclaw-fs-pinned-src-"), "source.txt");
await fs.writeFile(sourcePath, "streamed", "utf8");
const sourceHandle = await fs.open(sourcePath, "r");
try {
await runPinnedWriteHelper({
rootPath: root,
relativeParentPath: "",
basename: "stream.txt",
mkdir: true,
mode: 0o600,
input: {
kind: "stream",
stream: sourceHandle.createReadStream(),
},
});
} finally {
await sourceHandle.close();
}
await expect(fs.readFile(path.join(root, "stream.txt"), "utf8")).resolves.toBe("streamed");
});
});

View File

@@ -0,0 +1,230 @@
import { spawn } from "node:child_process";
import { once } from "node:events";
import fs from "node:fs/promises";
import path from "node:path";
import type { Readable } from "node:stream";
import { pipeline } from "node:stream/promises";
import type { FileIdentityStat } from "./file-identity.js";
export type PinnedWriteInput =
| { kind: "buffer"; data: string | Buffer; encoding?: BufferEncoding }
| { kind: "stream"; stream: Readable };
const LOCAL_PINNED_WRITE_PYTHON = [
"import errno",
"import os",
"import secrets",
"import stat",
"import sys",
"",
"root_path = sys.argv[1]",
"relative_parent = sys.argv[2]",
"basename = sys.argv[3]",
'mkdir_enabled = sys.argv[4] == "1"',
"file_mode = int(sys.argv[5], 8)",
"",
"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_value, dir_fd=None):",
" return os.open(path_value, DIR_FLAGS, dir_fd=dir_fd)",
"",
"def walk_parent(root_fd, rel_parent, mkdir_enabled):",
" current_fd = os.dup(root_fd)",
" try:",
" for segment in [part for part in rel_parent.split('/') if part and part != '.']:",
" 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, mode):",
" prefix = '.' + basename + '.'",
" for _ in range(128):",
" candidate = prefix + secrets.token_hex(6) + '.tmp'",
" try:",
" fd = os.open(candidate, WRITE_FLAGS, mode, dir_fd=parent_fd)",
" return candidate, fd",
" except FileExistsError:",
" continue",
" raise RuntimeError('failed to allocate pinned temp file')",
"",
"root_fd = open_dir(root_path)",
"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, file_mode)",
" 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)",
" temp_name = None",
" os.fsync(parent_fd)",
" result_stat = os.stat(basename, dir_fd=parent_fd, follow_symlinks=False)",
" print(f'{result_stat.st_dev}|{result_stat.st_ino}')",
"finally:",
" if temp_fd is not None:",
" os.close(temp_fd)",
" if temp_name is not None and parent_fd is not None:",
" try:",
" os.unlink(temp_name, dir_fd=parent_fd)",
" except FileNotFoundError:",
" pass",
" if parent_fd is not None:",
" os.close(parent_fd)",
" os.close(root_fd)",
].join("\n");
function parsePinnedIdentity(stdout: string): FileIdentityStat {
const line = stdout
.trim()
.split(/\r?\n/)
.map((value) => value.trim())
.filter(Boolean)
.at(-1);
if (!line) {
throw new Error("Pinned write helper returned no identity");
}
const [devRaw, inoRaw] = line.split("|");
const dev = Number.parseInt(devRaw ?? "", 10);
const ino = Number.parseInt(inoRaw ?? "", 10);
if (!Number.isFinite(dev) || !Number.isFinite(ino)) {
throw new Error(`Pinned write helper returned invalid identity: ${line}`);
}
return { dev, ino };
}
export async function runPinnedWriteHelper(params: {
rootPath: string;
relativeParentPath: string;
basename: string;
mkdir: boolean;
mode: number;
input: PinnedWriteInput;
}): Promise<FileIdentityStat> {
const child = spawn(
"python3",
[
"-c",
LOCAL_PINNED_WRITE_PYTHON,
params.rootPath,
params.relativeParentPath,
params.basename,
params.mkdir ? "1" : "0",
(params.mode || 0o600).toString(8),
],
{
stdio: ["pipe", "pipe", "pipe"],
},
);
let stdout = "";
let stderr = "";
child.stdout.setEncoding?.("utf8");
child.stderr.setEncoding?.("utf8");
child.stdout.on("data", (chunk: string) => {
stdout += chunk;
});
child.stderr.on("data", (chunk: string) => {
stderr += chunk;
});
const exitPromise = once(child, "close") as Promise<[number | null, NodeJS.Signals | null]>;
try {
if (!child.stdin) {
const identity = await runPinnedWriteFallback(params);
await exitPromise.catch(() => {});
return identity;
}
if (params.input.kind === "buffer") {
const input = params.input;
await new Promise<void>((resolve, reject) => {
child.stdin.once("error", reject);
if (typeof input.data === "string") {
child.stdin.end(input.data, input.encoding ?? "utf8", () => resolve());
return;
}
child.stdin.end(input.data, () => resolve());
});
} else {
await pipeline(params.input.stream, child.stdin);
}
const [code, signal] = await exitPromise;
if (code !== 0) {
throw new Error(
stderr.trim() ||
`Pinned write helper failed with code ${code ?? "null"} (${signal ?? "?"})`,
);
}
return parsePinnedIdentity(stdout);
} catch (error) {
child.kill("SIGKILL");
await exitPromise.catch(() => {});
throw error;
}
}
async function runPinnedWriteFallback(params: {
rootPath: string;
relativeParentPath: string;
basename: string;
mkdir: boolean;
mode: number;
input: PinnedWriteInput;
}): Promise<FileIdentityStat> {
const parentPath = params.relativeParentPath
? path.join(params.rootPath, ...params.relativeParentPath.split("/"))
: params.rootPath;
if (params.mkdir) {
await fs.mkdir(parentPath, { recursive: true });
}
const targetPath = path.join(parentPath, params.basename);
const tempPath = path.join(parentPath, `.${params.basename}.fallback.tmp`);
if (params.input.kind === "buffer") {
if (typeof params.input.data === "string") {
await fs.writeFile(tempPath, params.input.data, {
encoding: params.input.encoding ?? "utf8",
mode: params.mode,
});
} else {
await fs.writeFile(tempPath, params.input.data, { mode: params.mode });
}
} else {
const handle = await fs.open(tempPath, "w", params.mode);
try {
await pipeline(params.input.stream, handle.createWriteStream());
} finally {
await handle.close().catch(() => {});
}
}
await fs.rename(tempPath, targetPath);
const stat = await fs.stat(targetPath);
return { dev: stat.dev, ino: stat.ino };
}

View File

@@ -1,6 +1,6 @@
import fs from "node:fs/promises";
import path from "node:path";
import { afterEach, describe, expect, it, vi } from "vitest";
import { afterEach, describe, expect, it } from "vitest";
import {
createRebindableDirectoryAlias,
withRealpathSymlinkRebindRace,
@@ -36,7 +36,9 @@ async function expectWriteOpenRaceIsBlocked(params: {
symlinkTarget: params.outsideDir,
timing: "before-realpath",
run: async () => {
await expect(params.runWrite()).rejects.toMatchObject({ code: "outside-workspace" });
await expect(params.runWrite()).rejects.toMatchObject({
code: expect.stringMatching(/outside-workspace|invalid-path/),
});
},
});
}
@@ -263,120 +265,6 @@ describe("fs-safe", () => {
await expect(fs.readFile(targetPath, "utf8")).resolves.toBe("seed\nnext");
});
it("does not truncate existing target when atomic rename fails", async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const targetPath = path.join(root, "nested", "out.txt");
await fs.mkdir(path.dirname(targetPath), { recursive: true });
await fs.writeFile(targetPath, "existing-content");
const renameSpy = vi
.spyOn(fs, "rename")
.mockRejectedValue(Object.assign(new Error("rename blocked"), { code: "EACCES" }));
try {
await expect(
writeFileWithinRoot({
rootDir: root,
relativePath: "nested/out.txt",
data: "new-content",
}),
).rejects.toMatchObject({ code: "EACCES" });
} finally {
renameSpy.mockRestore();
}
await expect(fs.readFile(targetPath, "utf8")).resolves.toBe("existing-content");
});
it.runIf(process.platform !== "win32")(
"rejects when a hardlink appears after atomic write rename",
async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const targetPath = path.join(root, "nested", "out.txt");
const aliasPath = path.join(root, "nested", "alias.txt");
await fs.mkdir(path.dirname(targetPath), { recursive: true });
await fs.writeFile(targetPath, "existing-content");
const realRename = fs.rename.bind(fs);
let linked = false;
const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (...args) => {
await realRename(...args);
if (!linked) {
linked = true;
await fs.link(String(args[1]), aliasPath);
}
});
try {
await expect(
writeFileWithinRoot({
rootDir: root,
relativePath: "nested/out.txt",
data: "new-content",
}),
).rejects.toMatchObject({ code: "invalid-path" });
} finally {
renameSpy.mockRestore();
}
await expect(fs.readFile(aliasPath, "utf8")).resolves.toBe("new-content");
},
);
it("does not truncate existing target when atomic copy rename fails", async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const sourceDir = await tempDirs.make("openclaw-fs-safe-source-");
const sourcePath = path.join(sourceDir, "in.txt");
const targetPath = path.join(root, "nested", "copied.txt");
await fs.mkdir(path.dirname(targetPath), { recursive: true });
await fs.writeFile(sourcePath, "copy-new");
await fs.writeFile(targetPath, "copy-existing");
const renameSpy = vi
.spyOn(fs, "rename")
.mockRejectedValue(Object.assign(new Error("rename blocked"), { code: "EACCES" }));
try {
await expect(
copyFileWithinRoot({
sourcePath,
rootDir: root,
relativePath: "nested/copied.txt",
}),
).rejects.toMatchObject({ code: "EACCES" });
} finally {
renameSpy.mockRestore();
}
await expect(fs.readFile(targetPath, "utf8")).resolves.toBe("copy-existing");
});
it.runIf(process.platform !== "win32")(
"rejects when a hardlink appears after atomic copy rename",
async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const sourceDir = await tempDirs.make("openclaw-fs-safe-source-");
const sourcePath = path.join(sourceDir, "copy-source.txt");
const targetPath = path.join(root, "nested", "copied.txt");
const aliasPath = path.join(root, "nested", "alias.txt");
await fs.mkdir(path.dirname(targetPath), { recursive: true });
await fs.writeFile(sourcePath, "copy-new");
await fs.writeFile(targetPath, "copy-existing");
const realRename = fs.rename.bind(fs);
let linked = false;
const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (...args) => {
await realRename(...args);
if (!linked) {
linked = true;
await fs.link(String(args[1]), aliasPath);
}
});
try {
await expect(
copyFileWithinRoot({
sourcePath,
rootDir: root,
relativePath: "nested/copied.txt",
}),
).rejects.toMatchObject({ code: "invalid-path" });
} finally {
renameSpy.mockRestore();
}
await expect(fs.readFile(aliasPath, "utf8")).resolves.toBe("copy-new");
},
);
it("copies a file within root safely", async () => {
const root = await tempDirs.make("openclaw-fs-safe-root-");
const sourceDir = await tempDirs.make("openclaw-fs-safe-source-");
@@ -537,47 +425,6 @@ describe("fs-safe", () => {
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-");
const missing = path.join(dir, "missing.txt");

View File

@@ -5,9 +5,9 @@ import type { FileHandle } from "node:fs/promises";
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import { pipeline } from "node:stream/promises";
import { logWarn } from "../logger.js";
import { sameFileIdentity } from "./file-identity.js";
import { runPinnedWriteHelper } from "./fs-pinned-write-helper.js";
import { expandHomePrefix } from "./home-dir.js";
import { assertNoPathAliasEscape } from "./path-alias-guards.js";
import {
@@ -332,13 +332,13 @@ async function writeTempFileForAtomicReplace(params: {
async function verifyAtomicWriteResult(params: {
rootDir: string;
targetPath: string;
expectedStat: Stats;
expectedIdentity: { dev: number | bigint; ino: number | bigint };
}): Promise<void> {
const rootReal = await fs.realpath(params.rootDir);
const rootWithSep = ensureTrailingSep(rootReal);
const opened = await openVerifiedLocalFile(params.targetPath, { rejectHardlinks: true });
try {
if (!sameFileIdentity(opened.stat, params.expectedStat)) {
if (!sameFileIdentity(opened.stat, params.expectedIdentity)) {
throw new SafeOpenError("path-mismatch", "path changed during write");
}
if (!isPathInside(rootWithSep, opened.realPath)) {
@@ -550,6 +550,195 @@ export async function writeFileWithinRoot(params: {
data: string | Buffer;
encoding?: BufferEncoding;
mkdir?: boolean;
}): Promise<void> {
if (process.platform === "win32") {
await writeFileWithinRootLegacy(params);
return;
}
const pinned = await resolvePinnedWriteTargetWithinRoot({
rootDir: params.rootDir,
relativePath: params.relativePath,
});
const identity = await runPinnedWriteHelper({
rootPath: pinned.rootReal,
relativeParentPath: pinned.relativeParentPath,
basename: pinned.basename,
mkdir: params.mkdir !== false,
mode: pinned.mode,
input: {
kind: "buffer",
data: params.data,
encoding: params.encoding,
},
}).catch((error) => {
throw normalizePinnedWriteError(error);
});
try {
await verifyAtomicWriteResult({
rootDir: params.rootDir,
targetPath: pinned.targetPath,
expectedIdentity: identity,
});
} catch (err) {
emitWriteBoundaryWarning(`post-write verification failed: ${String(err)}`);
throw err;
}
}
export async function copyFileWithinRoot(params: {
sourcePath: string;
rootDir: string;
relativePath: string;
maxBytes?: number;
mkdir?: boolean;
rejectSourceHardlinks?: boolean;
}): Promise<void> {
const source = await openVerifiedLocalFile(params.sourcePath, {
rejectHardlinks: params.rejectSourceHardlinks,
});
if (params.maxBytes !== undefined && source.stat.size > params.maxBytes) {
await source.handle.close().catch(() => {});
throw new SafeOpenError(
"too-large",
`file exceeds limit of ${params.maxBytes} bytes (got ${source.stat.size})`,
);
}
try {
if (process.platform === "win32") {
await copyFileWithinRootLegacy(params, source);
return;
}
const pinned = await resolvePinnedWriteTargetWithinRoot({
rootDir: params.rootDir,
relativePath: params.relativePath,
});
const sourceStream = source.handle.createReadStream();
const identity = await runPinnedWriteHelper({
rootPath: pinned.rootReal,
relativeParentPath: pinned.relativeParentPath,
basename: pinned.basename,
mkdir: params.mkdir !== false,
mode: pinned.mode,
input: {
kind: "stream",
stream: sourceStream,
},
}).catch((error) => {
throw normalizePinnedWriteError(error);
});
try {
await verifyAtomicWriteResult({
rootDir: params.rootDir,
targetPath: pinned.targetPath,
expectedIdentity: identity,
});
} catch (err) {
emitWriteBoundaryWarning(`post-copy verification failed: ${String(err)}`);
throw err;
}
} finally {
await source.handle.close().catch(() => {});
}
}
export async function writeFileFromPathWithinRoot(params: {
rootDir: string;
relativePath: string;
sourcePath: string;
mkdir?: boolean;
}): Promise<void> {
await copyFileWithinRoot({
sourcePath: params.sourcePath,
rootDir: params.rootDir,
relativePath: params.relativePath,
mkdir: params.mkdir,
rejectSourceHardlinks: true,
});
}
async function resolvePinnedWriteTargetWithinRoot(params: {
rootDir: string;
relativePath: string;
}): Promise<{
rootReal: string;
targetPath: string;
relativeParentPath: string;
basename: string;
mode: number;
}> {
const { rootReal, rootWithSep, resolved } = await resolvePathWithinRoot(params);
try {
await assertNoPathAliasEscape({
absolutePath: resolved,
rootPath: rootReal,
boundaryLabel: "root",
});
} catch (err) {
throw new SafeOpenError("invalid-path", "path alias escape blocked", { cause: err });
}
const relativeResolved = path.relative(rootReal, resolved);
if (relativeResolved.startsWith("..") || path.isAbsolute(relativeResolved)) {
throw new SafeOpenError("outside-workspace", "file is outside workspace root");
}
const relativePosix = relativeResolved
? relativeResolved.split(path.sep).join(path.posix.sep)
: "";
const basename = path.posix.basename(relativePosix);
if (!basename || basename === "." || basename === "/") {
throw new SafeOpenError("invalid-path", "invalid target path");
}
let mode = 0o600;
try {
const opened = await openFileWithinRoot({
rootDir: params.rootDir,
relativePath: params.relativePath,
rejectHardlinks: true,
});
try {
mode = opened.stat.mode & 0o777;
if (!isPathInside(rootWithSep, opened.realPath)) {
throw new SafeOpenError("outside-workspace", "file is outside workspace root");
}
} finally {
await opened.handle.close().catch(() => {});
}
} catch (err) {
if (!(err instanceof SafeOpenError) || err.code !== "not-found") {
throw err;
}
}
return {
rootReal,
targetPath: resolved,
relativeParentPath:
path.posix.dirname(relativePosix) === "." ? "" : path.posix.dirname(relativePosix),
basename,
mode: mode || 0o600,
};
}
function normalizePinnedWriteError(error: unknown): Error {
if (error instanceof SafeOpenError) {
return error;
}
return new SafeOpenError("invalid-path", "path is not a regular file under root", {
cause: error instanceof Error ? error : undefined,
});
}
async function writeFileWithinRootLegacy(params: {
rootDir: string;
relativePath: string;
data: string | Buffer;
encoding?: BufferEncoding;
mkdir?: boolean;
}): Promise<void> {
const target = await openWritableFileWithinRoot({
rootDir: params.rootDir,
@@ -575,7 +764,7 @@ export async function writeFileWithinRoot(params: {
await verifyAtomicWriteResult({
rootDir: params.rootDir,
targetPath: destinationPath,
expectedStat: writtenStat,
expectedIdentity: writtenStat,
});
} catch (err) {
emitWriteBoundaryWarning(`post-write verification failed: ${String(err)}`);
@@ -588,25 +777,17 @@ export async function writeFileWithinRoot(params: {
}
}
export async function copyFileWithinRoot(params: {
sourcePath: string;
rootDir: string;
relativePath: string;
maxBytes?: number;
mkdir?: boolean;
rejectSourceHardlinks?: boolean;
}): Promise<void> {
const source = await openVerifiedLocalFile(params.sourcePath, {
rejectHardlinks: params.rejectSourceHardlinks,
});
if (params.maxBytes !== undefined && source.stat.size > params.maxBytes) {
await source.handle.close().catch(() => {});
throw new SafeOpenError(
"too-large",
`file exceeds limit of ${params.maxBytes} bytes (got ${source.stat.size})`,
);
}
async function copyFileWithinRootLegacy(
params: {
sourcePath: string;
rootDir: string;
relativePath: string;
maxBytes?: number;
mkdir?: boolean;
rejectSourceHardlinks?: boolean;
},
source: SafeOpenResult,
): Promise<void> {
let target: SafeWritableOpenResult | null = null;
let sourceClosedByStream = false;
let targetClosedByUs = false;
@@ -635,7 +816,9 @@ export async function copyFileWithinRoot(params: {
targetStream.once("close", () => {
tempClosedByStream = true;
});
await pipeline(sourceStream, targetStream);
await import("node:stream/promises").then(({ pipeline }) =>
pipeline(sourceStream, targetStream),
);
const writtenStat = await fs.stat(tempPath);
if (!tempClosedByStream) {
await tempHandle.close().catch(() => {});
@@ -648,7 +831,7 @@ export async function copyFileWithinRoot(params: {
await verifyAtomicWriteResult({
rootDir: params.rootDir,
targetPath: destinationPath,
expectedStat: writtenStat,
expectedIdentity: writtenStat,
});
} catch (err) {
emitWriteBoundaryWarning(`post-copy verification failed: ${String(err)}`);
@@ -674,18 +857,3 @@ export async function copyFileWithinRoot(params: {
}
}
}
export async function writeFileFromPathWithinRoot(params: {
rootDir: string;
relativePath: string;
sourcePath: string;
mkdir?: boolean;
}): Promise<void> {
await copyFileWithinRoot({
sourcePath: params.sourcePath,
rootDir: params.rootDir,
relativePath: params.relativePath,
mkdir: params.mkdir,
rejectSourceHardlinks: true,
});
}