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