mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
refactor: unify sandbox fs bridge mutations
This commit is contained in:
@@ -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_WRITE_PYTHON } from "./fs-bridge-write-helper.js";
|
||||
import { SANDBOX_PINNED_FS_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,23 +14,14 @@ async function withTempRoot<T>(prefix: string, run: (root: string) => Promise<T>
|
||||
}
|
||||
}
|
||||
|
||||
function runPinnedWrite(params: {
|
||||
mountRoot: string;
|
||||
relativeParentPath: string;
|
||||
basename: string;
|
||||
mkdir: boolean;
|
||||
input: string;
|
||||
function runPinnedMutation(params: {
|
||||
op: "write" | "mkdirp" | "remove" | "rename";
|
||||
args: string[];
|
||||
input?: string;
|
||||
}) {
|
||||
return spawnSync(
|
||||
"python3",
|
||||
[
|
||||
"-c",
|
||||
SANDBOX_PINNED_WRITE_PYTHON,
|
||||
params.mountRoot,
|
||||
params.relativeParentPath,
|
||||
params.basename,
|
||||
params.mkdir ? "1" : "0",
|
||||
],
|
||||
["-c", SANDBOX_PINNED_FS_MUTATION_PYTHON, params.op, ...params.args],
|
||||
{
|
||||
input: params.input,
|
||||
encoding: "utf8",
|
||||
@@ -39,17 +30,15 @@ function runPinnedWrite(params: {
|
||||
);
|
||||
}
|
||||
|
||||
describe("sandbox pinned write helper", () => {
|
||||
describe("sandbox pinned mutation 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,
|
||||
const result = runPinnedMutation({
|
||||
op: "write",
|
||||
args: [workspace, "nested/deeper", "note.txt", "1"],
|
||||
input: "hello",
|
||||
});
|
||||
|
||||
@@ -70,11 +59,9 @@ describe("sandbox pinned write helper", () => {
|
||||
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,
|
||||
const result = runPinnedMutation({
|
||||
op: "write",
|
||||
args: [workspace, "alias", "escape.txt", "0"],
|
||||
input: "owned",
|
||||
});
|
||||
|
||||
135
src/agents/sandbox/fs-bridge-mutation-helper.ts
Normal file
135
src/agents/sandbox/fs-bridge-mutation-helper.ts
Normal file
@@ -0,0 +1,135 @@
|
||||
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 { SandboxFsCommandPlan } from "./fs-bridge-shell-command-plans.js";
|
||||
import type { SandboxResolvedFsPath } from "./fs-paths.js";
|
||||
|
||||
function buildPinnedMutationPlan(params: {
|
||||
checks: SandboxFsCommandPlan["checks"];
|
||||
args: string[];
|
||||
stdin?: Buffer | string;
|
||||
}): SandboxFsCommandPlan {
|
||||
return {
|
||||
checks: params.checks,
|
||||
recheckBeforeCommand: true,
|
||||
script: ["set -eu", "python3 - \"$@\" <<'PY'", SANDBOX_PINNED_FS_MUTATION_PYTHON, "PY"].join(
|
||||
"\n",
|
||||
),
|
||||
args: params.args,
|
||||
stdin: params.stdin,
|
||||
};
|
||||
}
|
||||
|
||||
export function buildPinnedWritePlan(params: {
|
||||
target: SandboxResolvedFsPath;
|
||||
pinned: PinnedSandboxEntry;
|
||||
mkdir: boolean;
|
||||
stdin: Buffer | string;
|
||||
}): SandboxFsCommandPlan {
|
||||
return buildPinnedMutationPlan({
|
||||
checks: [
|
||||
{
|
||||
target: params.target,
|
||||
options: { action: "write files", requireWritable: true },
|
||||
},
|
||||
],
|
||||
args: [
|
||||
"write",
|
||||
params.pinned.mountRootPath,
|
||||
params.pinned.relativeParentPath,
|
||||
params.pinned.basename,
|
||||
params.mkdir ? "1" : "0",
|
||||
],
|
||||
stdin: params.stdin,
|
||||
});
|
||||
}
|
||||
|
||||
export function buildPinnedMkdirpPlan(params: {
|
||||
target: SandboxResolvedFsPath;
|
||||
pinned: PinnedSandboxEntry;
|
||||
}): 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,
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
export function buildPinnedRemovePlan(params: {
|
||||
target: SandboxResolvedFsPath;
|
||||
pinned: PinnedSandboxEntry;
|
||||
recursive?: boolean;
|
||||
force?: boolean;
|
||||
}): SandboxFsCommandPlan {
|
||||
return buildPinnedMutationPlan({
|
||||
checks: [
|
||||
{
|
||||
target: params.target,
|
||||
options: {
|
||||
action: "remove files",
|
||||
requireWritable: true,
|
||||
aliasPolicy: PATH_ALIAS_POLICIES.unlinkTarget,
|
||||
},
|
||||
},
|
||||
],
|
||||
args: [
|
||||
"remove",
|
||||
params.pinned.mountRootPath,
|
||||
params.pinned.relativeParentPath,
|
||||
params.pinned.basename,
|
||||
params.recursive ? "1" : "0",
|
||||
params.force === false ? "0" : "1",
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
export function buildPinnedRenamePlan(params: {
|
||||
from: SandboxResolvedFsPath;
|
||||
to: SandboxResolvedFsPath;
|
||||
pinnedFrom: PinnedSandboxEntry;
|
||||
pinnedTo: PinnedSandboxEntry;
|
||||
}): SandboxFsCommandPlan {
|
||||
return buildPinnedMutationPlan({
|
||||
checks: [
|
||||
{
|
||||
target: params.from,
|
||||
options: {
|
||||
action: "rename files",
|
||||
requireWritable: true,
|
||||
aliasPolicy: PATH_ALIAS_POLICIES.unlinkTarget,
|
||||
},
|
||||
},
|
||||
{
|
||||
target: params.to,
|
||||
options: {
|
||||
action: "rename files",
|
||||
requireWritable: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
args: [
|
||||
"rename",
|
||||
params.pinnedFrom.mountRootPath,
|
||||
params.pinnedFrom.relativeParentPath,
|
||||
params.pinnedFrom.basename,
|
||||
params.pinnedTo.mountRootPath,
|
||||
params.pinnedTo.relativeParentPath,
|
||||
params.pinnedTo.basename,
|
||||
],
|
||||
});
|
||||
}
|
||||
|
||||
export { SANDBOX_PINNED_FS_MUTATION_PYTHON };
|
||||
190
src/agents/sandbox/fs-bridge-mutation-python-source.ts
Normal file
190
src/agents/sandbox/fs-bridge-mutation-python-source.ts
Normal file
@@ -0,0 +1,190 @@
|
||||
// language=python
|
||||
export const SANDBOX_PINNED_FS_MUTATION_PYTHON = String.raw`import os
|
||||
import secrets
|
||||
import subprocess
|
||||
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
|
||||
|
||||
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("path traversal is not allowed")
|
||||
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 fd_path(fd, basename=None):
|
||||
base = f"/proc/self/fd/{fd}"
|
||||
if basename is None:
|
||||
return base
|
||||
return f"{base}/{basename}"
|
||||
|
||||
|
||||
def run_command(argv, pass_fds):
|
||||
subprocess.run(argv, check=True, pass_fds=tuple(pass_fds))
|
||||
|
||||
|
||||
def write_stdin_to_fd(fd):
|
||||
while True:
|
||||
chunk = sys.stdin.buffer.read(65536)
|
||||
if not chunk:
|
||||
break
|
||||
os.write(fd, chunk)
|
||||
|
||||
|
||||
def run_write(args):
|
||||
mount_root, relative_parent, basename, mkdir_enabled_raw = args
|
||||
mkdir_enabled = mkdir_enabled_raw == "1"
|
||||
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)
|
||||
write_stdin_to_fd(temp_fd)
|
||||
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)
|
||||
|
||||
|
||||
def run_mkdirp(args):
|
||||
mount_root, relative_parent, basename = args
|
||||
root_fd = open_dir(mount_root)
|
||||
parent_fd = None
|
||||
try:
|
||||
parent_fd = walk_parent(root_fd, relative_parent, True)
|
||||
run_command(["mkdir", "-p", "--", fd_path(parent_fd, basename)], [parent_fd])
|
||||
os.fsync(parent_fd)
|
||||
finally:
|
||||
if parent_fd is not None:
|
||||
os.close(parent_fd)
|
||||
os.close(root_fd)
|
||||
|
||||
|
||||
def run_remove(args):
|
||||
mount_root, relative_parent, basename, recursive_raw, force_raw = args
|
||||
root_fd = open_dir(mount_root)
|
||||
parent_fd = None
|
||||
try:
|
||||
parent_fd = walk_parent(root_fd, relative_parent, False)
|
||||
argv = ["rm"]
|
||||
if force_raw == "1":
|
||||
argv.append("-f")
|
||||
if recursive_raw == "1":
|
||||
argv.append("-r")
|
||||
argv.extend(["--", fd_path(parent_fd, basename)])
|
||||
run_command(argv, [parent_fd])
|
||||
os.fsync(parent_fd)
|
||||
finally:
|
||||
if parent_fd is not None:
|
||||
os.close(parent_fd)
|
||||
os.close(root_fd)
|
||||
|
||||
|
||||
def run_rename(args):
|
||||
(
|
||||
from_mount_root,
|
||||
from_relative_parent,
|
||||
from_basename,
|
||||
to_mount_root,
|
||||
to_relative_parent,
|
||||
to_basename,
|
||||
) = args
|
||||
from_root_fd = open_dir(from_mount_root)
|
||||
to_root_fd = open_dir(to_mount_root)
|
||||
from_parent_fd = None
|
||||
to_parent_fd = None
|
||||
try:
|
||||
from_parent_fd = walk_parent(from_root_fd, from_relative_parent, False)
|
||||
to_parent_fd = walk_parent(to_root_fd, to_relative_parent, True)
|
||||
run_command(
|
||||
[
|
||||
"mv",
|
||||
"--",
|
||||
fd_path(from_parent_fd, from_basename),
|
||||
fd_path(to_parent_fd, to_basename),
|
||||
],
|
||||
[from_parent_fd, to_parent_fd],
|
||||
)
|
||||
os.fsync(from_parent_fd)
|
||||
if to_parent_fd != from_parent_fd:
|
||||
os.fsync(to_parent_fd)
|
||||
finally:
|
||||
if from_parent_fd is not None:
|
||||
os.close(from_parent_fd)
|
||||
if to_parent_fd is not None:
|
||||
os.close(to_parent_fd)
|
||||
os.close(from_root_fd)
|
||||
os.close(to_root_fd)
|
||||
|
||||
|
||||
OPERATIONS = {
|
||||
"write": run_write,
|
||||
"mkdirp": run_mkdirp,
|
||||
"remove": run_remove,
|
||||
"rename": run_rename,
|
||||
}
|
||||
|
||||
if operation not in OPERATIONS:
|
||||
raise RuntimeError(f"unknown sandbox fs mutation: {operation}")
|
||||
|
||||
OPERATIONS[operation](sys.argv[2:])`;
|
||||
@@ -23,7 +23,7 @@ export type AnchoredSandboxEntry = {
|
||||
basename: string;
|
||||
};
|
||||
|
||||
export type PinnedSandboxWriteEntry = {
|
||||
export type PinnedSandboxEntry = {
|
||||
mountRootPath: string;
|
||||
relativeParentPath: string;
|
||||
basename: string;
|
||||
@@ -135,29 +135,21 @@ export class SandboxFsPathGuard {
|
||||
}
|
||||
|
||||
async resolveAnchoredSandboxEntry(target: SandboxResolvedFsPath): Promise<AnchoredSandboxEntry> {
|
||||
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 splitTarget = this.splitSandboxEntryTarget(target);
|
||||
const canonicalParentPath = await this.resolveCanonicalContainerPath({
|
||||
containerPath: parentPath,
|
||||
containerPath: splitTarget.parentPath,
|
||||
allowFinalSymlinkForUnlink: false,
|
||||
});
|
||||
return {
|
||||
canonicalParentPath,
|
||||
basename,
|
||||
basename: splitTarget.basename,
|
||||
};
|
||||
}
|
||||
|
||||
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);
|
||||
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);
|
||||
if (relativeParentPath.startsWith("..") || path.posix.isAbsolute(relativeParentPath)) {
|
||||
throw new Error(
|
||||
`Sandbox path escapes allowed mounts; cannot ${action}: ${target.containerPath}`,
|
||||
@@ -166,6 +158,20 @@ export class SandboxFsPathGuard {
|
||||
return {
|
||||
mountRootPath: mount.containerRoot,
|
||||
relativeParentPath: relativeParentPath === "." ? "" : relativeParentPath,
|
||||
basename: splitTarget.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}`);
|
||||
}
|
||||
return {
|
||||
parentPath: normalizeContainerPath(path.posix.dirname(target.containerPath)),
|
||||
basename,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1,95 +1,15 @@
|
||||
import { PATH_ALIAS_POLICIES } from "../../infra/path-alias-guards.js";
|
||||
import type { AnchoredSandboxEntry, PathSafetyCheck } from "./fs-bridge-path-safety.js";
|
||||
import type { PathSafetyCheck } from "./fs-bridge-path-safety.js";
|
||||
import type { SandboxResolvedFsPath } from "./fs-paths.js";
|
||||
|
||||
export type SandboxFsCommandPlan = {
|
||||
checks: PathSafetyCheck[];
|
||||
script: string;
|
||||
args?: string[];
|
||||
stdin?: Buffer | string;
|
||||
recheckBeforeCommand?: boolean;
|
||||
allowFailure?: boolean;
|
||||
};
|
||||
|
||||
export function buildMkdirpPlan(
|
||||
target: SandboxResolvedFsPath,
|
||||
anchoredTarget: AnchoredSandboxEntry,
|
||||
): SandboxFsCommandPlan {
|
||||
return {
|
||||
checks: [
|
||||
{
|
||||
target,
|
||||
options: {
|
||||
action: "create directories",
|
||||
requireWritable: true,
|
||||
allowedType: "directory",
|
||||
},
|
||||
},
|
||||
],
|
||||
script: 'set -eu\ncd -- "$1"\nmkdir -p -- "$2"',
|
||||
args: [anchoredTarget.canonicalParentPath, anchoredTarget.basename],
|
||||
};
|
||||
}
|
||||
|
||||
export function buildRemovePlan(params: {
|
||||
target: SandboxResolvedFsPath;
|
||||
anchoredTarget: AnchoredSandboxEntry;
|
||||
recursive?: boolean;
|
||||
force?: boolean;
|
||||
}): SandboxFsCommandPlan {
|
||||
const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter(Boolean);
|
||||
const rmCommand = flags.length > 0 ? `rm ${flags.join(" ")}` : "rm";
|
||||
return {
|
||||
checks: [
|
||||
{
|
||||
target: params.target,
|
||||
options: {
|
||||
action: "remove files",
|
||||
requireWritable: true,
|
||||
aliasPolicy: PATH_ALIAS_POLICIES.unlinkTarget,
|
||||
},
|
||||
},
|
||||
],
|
||||
recheckBeforeCommand: true,
|
||||
script: `set -eu\ncd -- "$1"\n${rmCommand} -- "$2"`,
|
||||
args: [params.anchoredTarget.canonicalParentPath, params.anchoredTarget.basename],
|
||||
};
|
||||
}
|
||||
|
||||
export function buildRenamePlan(params: {
|
||||
from: SandboxResolvedFsPath;
|
||||
to: SandboxResolvedFsPath;
|
||||
anchoredFrom: AnchoredSandboxEntry;
|
||||
anchoredTo: AnchoredSandboxEntry;
|
||||
}): SandboxFsCommandPlan {
|
||||
return {
|
||||
checks: [
|
||||
{
|
||||
target: params.from,
|
||||
options: {
|
||||
action: "rename files",
|
||||
requireWritable: true,
|
||||
aliasPolicy: PATH_ALIAS_POLICIES.unlinkTarget,
|
||||
},
|
||||
},
|
||||
{
|
||||
target: params.to,
|
||||
options: {
|
||||
action: "rename files",
|
||||
requireWritable: true,
|
||||
},
|
||||
},
|
||||
],
|
||||
recheckBeforeCommand: true,
|
||||
script: ["set -eu", 'mkdir -p -- "$2"', 'cd -- "$1"', 'mv -- "$3" "$2/$4"'].join("\n"),
|
||||
args: [
|
||||
params.anchoredFrom.canonicalParentPath,
|
||||
params.anchoredTo.canonicalParentPath,
|
||||
params.anchoredFrom.basename,
|
||||
params.anchoredTo.basename,
|
||||
],
|
||||
};
|
||||
}
|
||||
|
||||
export function buildStatPlan(target: SandboxResolvedFsPath): SandboxFsCommandPlan {
|
||||
return {
|
||||
checks: [{ target, options: { action: "stat files" } }],
|
||||
|
||||
@@ -1,109 +0,0 @@
|
||||
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",
|
||||
],
|
||||
};
|
||||
}
|
||||
@@ -4,8 +4,7 @@ import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
createSandbox,
|
||||
createSandboxFsBridge,
|
||||
findCallByScriptFragment,
|
||||
findCallsByScriptFragment,
|
||||
findCallByDockerArg,
|
||||
getDockerArg,
|
||||
installFsBridgeTestHarness,
|
||||
mockedExecDockerRaw,
|
||||
@@ -69,31 +68,28 @@ describe("sandbox fs bridge anchored ops", () => {
|
||||
|
||||
const anchoredCases = [
|
||||
{
|
||||
name: "mkdirp anchors parent + basename",
|
||||
name: "mkdirp pins mount root + relative parent + basename",
|
||||
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
|
||||
bridge.mkdirp({ filePath: "nested/leaf" }),
|
||||
scriptFragment: 'mkdir -p -- "$2"',
|
||||
expectedArgs: ["/workspace/nested", "leaf"],
|
||||
forbiddenArgs: ["/workspace/nested/leaf"],
|
||||
canonicalProbe: "/workspace/nested",
|
||||
op: "mkdirp",
|
||||
expectedArgs: ["/workspace", "nested", "leaf"],
|
||||
forbiddenArgs: ["/workspace/nested/leaf", "/workspace/nested"],
|
||||
},
|
||||
{
|
||||
name: "remove anchors parent + basename",
|
||||
name: "remove pins mount root + relative parent + basename",
|
||||
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
|
||||
bridge.remove({ filePath: "nested/file.txt" }),
|
||||
scriptFragment: 'rm -f -- "$2"',
|
||||
expectedArgs: ["/workspace/nested", "file.txt"],
|
||||
forbiddenArgs: ["/workspace/nested/file.txt"],
|
||||
canonicalProbe: "/workspace/nested",
|
||||
op: "remove",
|
||||
expectedArgs: ["/workspace", "nested", "file.txt", "0", "1"],
|
||||
forbiddenArgs: ["/workspace/nested/file.txt", "/workspace/nested"],
|
||||
},
|
||||
{
|
||||
name: "rename anchors both parents + basenames",
|
||||
name: "rename pins both parents + basenames",
|
||||
invoke: (bridge: ReturnType<typeof createSandboxFsBridge>) =>
|
||||
bridge.rename({ from: "from.txt", to: "nested/to.txt" }),
|
||||
scriptFragment: 'mv -- "$3" "$2/$4"',
|
||||
expectedArgs: ["/workspace", "/workspace/nested", "from.txt", "to.txt"],
|
||||
forbiddenArgs: ["/workspace/from.txt", "/workspace/nested/to.txt"],
|
||||
canonicalProbe: "/workspace/nested",
|
||||
op: "rename",
|
||||
expectedArgs: ["/workspace", "", "from.txt", "/workspace", "nested", "to.txt"],
|
||||
forbiddenArgs: ["/workspace/from.txt", "/workspace/nested/to.txt", "/workspace/nested"],
|
||||
},
|
||||
] as const;
|
||||
|
||||
@@ -102,19 +98,14 @@ describe("sandbox fs bridge anchored ops", () => {
|
||||
|
||||
await testCase.invoke(bridge);
|
||||
|
||||
const opCall = findCallByScriptFragment(testCase.scriptFragment);
|
||||
const opCall = findCallByDockerArg(1, testCase.op);
|
||||
expect(opCall).toBeDefined();
|
||||
const args = opCall?.[0] ?? [];
|
||||
testCase.expectedArgs.forEach((value, index) => {
|
||||
expect(getDockerArg(args, index + 1)).toBe(value);
|
||||
expect(getDockerArg(args, index + 2)).toBe(value);
|
||||
});
|
||||
testCase.forbiddenArgs.forEach((value) => {
|
||||
expect(args).not.toContain(value);
|
||||
});
|
||||
|
||||
const canonicalCalls = findCallsByScriptFragment('readlink -f -- "$cursor"');
|
||||
expect(
|
||||
canonicalCalls.some(([callArgs]) => getDockerArg(callArgs, 1) === testCase.canonicalProbe),
|
||||
).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
createSandbox,
|
||||
createSandboxFsBridge,
|
||||
expectMkdirpAllowsExistingDirectory,
|
||||
getScriptsFromCalls,
|
||||
findCallByDockerArg,
|
||||
installFsBridgeTestHarness,
|
||||
mockedExecDockerRaw,
|
||||
withTempDir,
|
||||
@@ -55,8 +55,7 @@ describe("sandbox fs bridge boundary validation", () => {
|
||||
await expect(bridge.mkdirp({ filePath: "memory/kemik" })).rejects.toThrow(
|
||||
/cannot create directories/i,
|
||||
);
|
||||
const scripts = getScriptsFromCalls();
|
||||
expect(scripts.some((script) => script.includes('mkdir -p -- "$2"'))).toBe(false);
|
||||
expect(findCallByDockerArg(1, "mkdirp")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -111,7 +110,6 @@ describe("sandbox fs bridge boundary validation", () => {
|
||||
it("rejects missing files before any docker read command runs", async () => {
|
||||
const bridge = createSandboxFsBridge({ sandbox: createSandbox() });
|
||||
await expect(bridge.readFile({ filePath: "a.txt" })).rejects.toThrow(/ENOENT|no such file/i);
|
||||
const scripts = getScriptsFromCalls();
|
||||
expect(scripts.some((script) => script.includes('cat -- "$1"'))).toBe(false);
|
||||
expect(mockedExecDockerRaw).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
89
src/agents/sandbox/fs-bridge.e2e-docker.test.ts
Normal file
89
src/agents/sandbox/fs-bridge.e2e-docker.test.ts
Normal file
@@ -0,0 +1,89 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { DEFAULT_SANDBOX_IMAGE } from "./constants.js";
|
||||
import { buildSandboxCreateArgs, execDocker, execDockerRaw } from "./docker.js";
|
||||
import { createSandboxFsBridge } from "./fs-bridge.js";
|
||||
import { createSandboxTestContext } from "./test-fixtures.js";
|
||||
import { appendWorkspaceMountArgs } from "./workspace-mounts.js";
|
||||
|
||||
async function sandboxImageReady(): Promise<boolean> {
|
||||
try {
|
||||
const dockerVersion = await execDockerRaw(["version"], { allowFailure: true });
|
||||
if (dockerVersion.code !== 0) {
|
||||
return false;
|
||||
}
|
||||
const pythonCheck = await execDockerRaw(
|
||||
["run", "--rm", "--entrypoint", "python3", DEFAULT_SANDBOX_IMAGE, "--version"],
|
||||
{ allowFailure: true },
|
||||
);
|
||||
return pythonCheck.code === 0;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
describe("sandbox fs bridge docker e2e", () => {
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"writes through docker exec using the pinned mutation helper",
|
||||
async () => {
|
||||
if (!(await sandboxImageReady())) {
|
||||
return;
|
||||
}
|
||||
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fsbridge-e2e-"));
|
||||
const workspaceDir = path.join(stateDir, "workspace");
|
||||
await fs.mkdir(workspaceDir, { recursive: true });
|
||||
|
||||
const suffix = `${process.pid}-${Date.now()}`;
|
||||
const containerName = `openclaw-fsbridge-${suffix}`.slice(0, 63);
|
||||
|
||||
try {
|
||||
const sandbox = createSandboxTestContext({
|
||||
overrides: {
|
||||
workspaceDir,
|
||||
agentWorkspaceDir: workspaceDir,
|
||||
containerName,
|
||||
containerWorkdir: "/workspace",
|
||||
},
|
||||
dockerOverrides: {
|
||||
image: DEFAULT_SANDBOX_IMAGE,
|
||||
containerPrefix: "openclaw-fsbridge-",
|
||||
user: "",
|
||||
},
|
||||
});
|
||||
|
||||
const createArgs = buildSandboxCreateArgs({
|
||||
name: containerName,
|
||||
cfg: sandbox.docker,
|
||||
scopeKey: sandbox.sessionKey,
|
||||
includeBinds: false,
|
||||
bindSourceRoots: [workspaceDir],
|
||||
});
|
||||
createArgs.push("--workdir", sandbox.containerWorkdir);
|
||||
appendWorkspaceMountArgs({
|
||||
args: createArgs,
|
||||
workspaceDir,
|
||||
agentWorkspaceDir: workspaceDir,
|
||||
workdir: sandbox.containerWorkdir,
|
||||
workspaceAccess: sandbox.workspaceAccess,
|
||||
});
|
||||
createArgs.push(sandbox.docker.image, "sleep", "infinity");
|
||||
|
||||
await execDocker(createArgs);
|
||||
await execDocker(["start", containerName]);
|
||||
|
||||
const bridge = createSandboxFsBridge({ sandbox });
|
||||
await bridge.writeFile({ filePath: "nested/hello.txt", data: "from-docker" });
|
||||
|
||||
await expect(
|
||||
fs.readFile(path.join(workspaceDir, "nested", "hello.txt"), "utf8"),
|
||||
).resolves.toBe("from-docker");
|
||||
} finally {
|
||||
await execDocker(["rm", "-f", containerName], { allowFailure: true });
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
@@ -54,6 +54,10 @@ 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),
|
||||
@@ -142,11 +146,13 @@ export async function expectMkdirpAllowsExistingDirectory(params?: {
|
||||
|
||||
await expect(bridge.mkdirp({ filePath: "memory/kemik" })).resolves.toBeUndefined();
|
||||
|
||||
const mkdirCall = findCallByScriptFragment('mkdir -p -- "$2"');
|
||||
const mkdirCall = findCallByDockerArg(1, "mkdirp");
|
||||
expect(mkdirCall).toBeDefined();
|
||||
const mkdirParent = mkdirCall ? getDockerArg(mkdirCall[0], 1) : "";
|
||||
const mkdirBase = mkdirCall ? getDockerArg(mkdirCall[0], 2) : "";
|
||||
expect(mkdirParent).toBe("/workspace/memory");
|
||||
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");
|
||||
});
|
||||
}
|
||||
|
||||
@@ -1,14 +1,13 @@
|
||||
import fs from "node:fs";
|
||||
import { execDockerRaw, type ExecDockerRawResult } from "./docker.js";
|
||||
import { SandboxFsPathGuard } from "./fs-bridge-path-safety.js";
|
||||
import {
|
||||
buildMkdirpPlan,
|
||||
buildRemovePlan,
|
||||
buildRenamePlan,
|
||||
buildStatPlan,
|
||||
type SandboxFsCommandPlan,
|
||||
} from "./fs-bridge-shell-command-plans.js";
|
||||
import { buildPinnedWritePlan } from "./fs-bridge-write-helper.js";
|
||||
buildPinnedMkdirpPlan,
|
||||
buildPinnedRemovePlan,
|
||||
buildPinnedRenamePlan,
|
||||
buildPinnedWritePlan,
|
||||
} from "./fs-bridge-mutation-helper.js";
|
||||
import { SandboxFsPathGuard } from "./fs-bridge-path-safety.js";
|
||||
import { buildStatPlan, type SandboxFsCommandPlan } from "./fs-bridge-shell-command-plans.js";
|
||||
import {
|
||||
buildSandboxFsMounts,
|
||||
resolveSandboxFsPathWithMounts,
|
||||
@@ -111,31 +110,29 @@ 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.resolvePinnedWriteEntry(target, "write files");
|
||||
await this.runCheckedCommand({
|
||||
...buildPinnedWritePlan({
|
||||
check: writeCheck,
|
||||
const pinnedWriteTarget = this.pathGuard.resolvePinnedMutationEntry(target, "write files");
|
||||
await this.runCheckedCommand(
|
||||
buildPinnedWritePlan({
|
||||
target,
|
||||
pinned: pinnedWriteTarget,
|
||||
mkdir: params.mkdir !== false,
|
||||
}),
|
||||
stdin: buffer,
|
||||
signal: params.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 anchoredTarget = await this.pathGuard.resolveAnchoredSandboxEntry(target);
|
||||
await this.runPlannedCommand(buildMkdirpPlan(target, anchoredTarget), params.signal);
|
||||
const pinnedTarget = this.pathGuard.resolvePinnedMutationEntry(target, "create directories");
|
||||
await this.runCheckedCommand(
|
||||
buildPinnedMkdirpPlan({ target, pinned: pinnedTarget }),
|
||||
params.signal,
|
||||
);
|
||||
}
|
||||
|
||||
async remove(params: {
|
||||
@@ -147,11 +144,11 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
}): Promise<void> {
|
||||
const target = this.resolveResolvedPath(params);
|
||||
this.ensureWriteAccess(target, "remove files");
|
||||
const anchoredTarget = await this.pathGuard.resolveAnchoredSandboxEntry(target);
|
||||
await this.runPlannedCommand(
|
||||
buildRemovePlan({
|
||||
const pinnedTarget = this.pathGuard.resolvePinnedMutationEntry(target, "remove files");
|
||||
await this.runCheckedCommand(
|
||||
buildPinnedRemovePlan({
|
||||
target,
|
||||
anchoredTarget,
|
||||
pinned: pinnedTarget,
|
||||
recursive: params.recursive,
|
||||
force: params.force,
|
||||
}),
|
||||
@@ -169,14 +166,14 @@ 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 anchoredFrom = await this.pathGuard.resolveAnchoredSandboxEntry(from);
|
||||
const anchoredTo = await this.pathGuard.resolveAnchoredSandboxEntry(to);
|
||||
await this.runPlannedCommand(
|
||||
buildRenamePlan({
|
||||
const pinnedFrom = this.pathGuard.resolvePinnedMutationEntry(from, "rename files");
|
||||
const pinnedTo = this.pathGuard.resolvePinnedMutationEntry(to, "rename files");
|
||||
await this.runCheckedCommand(
|
||||
buildPinnedRenamePlan({
|
||||
from,
|
||||
to,
|
||||
anchoredFrom,
|
||||
anchoredTo,
|
||||
pinnedFrom,
|
||||
pinnedTo,
|
||||
}),
|
||||
params.signal,
|
||||
);
|
||||
@@ -188,7 +185,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
signal?: AbortSignal;
|
||||
}): Promise<SandboxFsStat | null> {
|
||||
const target = this.resolveResolvedPath(params);
|
||||
const result = await this.runPlannedCommand(buildStatPlan(target), params.signal);
|
||||
const result = await this.runCheckedCommand(buildStatPlan(target), params.signal);
|
||||
if (result.code !== 0) {
|
||||
const stderr = result.stderr.toString("utf8");
|
||||
if (stderr.includes("No such file or directory")) {
|
||||
@@ -241,7 +238,8 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
}
|
||||
|
||||
private async runCheckedCommand(
|
||||
plan: SandboxFsCommandPlan & { stdin?: Buffer | string; signal?: AbortSignal },
|
||||
plan: SandboxFsCommandPlan,
|
||||
signal?: AbortSignal,
|
||||
): Promise<ExecDockerRawResult> {
|
||||
await this.pathGuard.assertPathChecks(plan.checks);
|
||||
if (plan.recheckBeforeCommand) {
|
||||
@@ -251,17 +249,10 @@ class SandboxFsBridgeImpl implements SandboxFsBridge {
|
||||
args: plan.args,
|
||||
stdin: plan.stdin,
|
||||
allowFailure: plan.allowFailure,
|
||||
signal: plan.signal,
|
||||
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}`);
|
||||
|
||||
Reference in New Issue
Block a user