fix(security): apply tools.fs.workspaceOnly to sandbox file tools
This commit is contained in:
206
src/agents/pi-tools.sandbox-mounted-paths.workspace-only.test.ts
Normal file
206
src/agents/pi-tools.sandbox-mounted-paths.workspace-only.test.ts
Normal file
@@ -0,0 +1,206 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { OpenClawConfig } from "../config/config.js";
|
||||
import type { SandboxContext } from "./sandbox.js";
|
||||
import type { SandboxFsBridge, SandboxFsStat, SandboxResolvedPath } from "./sandbox/fs-bridge.js";
|
||||
import { createOpenClawCodingTools } from "./pi-tools.js";
|
||||
|
||||
vi.mock("../infra/shell-env.js", async (importOriginal) => {
|
||||
const mod = await importOriginal<typeof import("../infra/shell-env.js")>();
|
||||
return { ...mod, getShellPathFromLoginShell: () => null };
|
||||
});
|
||||
|
||||
function getTextContent(result?: { content?: Array<{ type: string; text?: string }> }) {
|
||||
const textBlock = result?.content?.find((block) => block.type === "text");
|
||||
return textBlock?.text ?? "";
|
||||
}
|
||||
|
||||
function createUnsafeMountedBridge(params: {
|
||||
root: string;
|
||||
agentHostRoot: string;
|
||||
workspaceContainerRoot?: string;
|
||||
}): SandboxFsBridge {
|
||||
const root = path.resolve(params.root);
|
||||
const agentHostRoot = path.resolve(params.agentHostRoot);
|
||||
const workspaceContainerRoot = params.workspaceContainerRoot ?? "/workspace";
|
||||
|
||||
const resolvePath = (filePath: string, cwd?: string): SandboxResolvedPath => {
|
||||
// Intentionally unsafe: simulate a sandbox FS bridge that maps /agent/* into a host path
|
||||
// outside the workspace root (e.g. an operator-configured bind mount).
|
||||
const hostPath =
|
||||
filePath === "/agent" || filePath === "/agent/" || filePath.startsWith("/agent/")
|
||||
? path.join(
|
||||
agentHostRoot,
|
||||
filePath === "/agent" || filePath === "/agent/" ? "" : filePath.slice("/agent/".length),
|
||||
)
|
||||
: path.isAbsolute(filePath)
|
||||
? filePath
|
||||
: path.resolve(cwd ?? root, filePath);
|
||||
|
||||
const relFromRoot = path.relative(root, hostPath);
|
||||
const relativePath =
|
||||
relFromRoot && !relFromRoot.startsWith("..") && !path.isAbsolute(relFromRoot)
|
||||
? relFromRoot.split(path.sep).filter(Boolean).join(path.posix.sep)
|
||||
: filePath.replace(/\\/g, "/");
|
||||
|
||||
const containerPath = filePath.startsWith("/")
|
||||
? filePath.replace(/\\/g, "/")
|
||||
: relativePath
|
||||
? path.posix.join(workspaceContainerRoot, relativePath)
|
||||
: workspaceContainerRoot;
|
||||
|
||||
return { hostPath, relativePath, containerPath };
|
||||
};
|
||||
|
||||
return {
|
||||
resolvePath: ({ filePath, cwd }) => resolvePath(filePath, cwd),
|
||||
readFile: async ({ filePath, cwd }) => {
|
||||
const target = resolvePath(filePath, cwd);
|
||||
return fs.readFile(target.hostPath);
|
||||
},
|
||||
writeFile: async ({ filePath, cwd, data, mkdir = true }) => {
|
||||
const target = resolvePath(filePath, cwd);
|
||||
if (mkdir) {
|
||||
await fs.mkdir(path.dirname(target.hostPath), { recursive: true });
|
||||
}
|
||||
const buffer = Buffer.isBuffer(data) ? data : Buffer.from(data);
|
||||
await fs.writeFile(target.hostPath, buffer);
|
||||
},
|
||||
mkdirp: async ({ filePath, cwd }) => {
|
||||
const target = resolvePath(filePath, cwd);
|
||||
await fs.mkdir(target.hostPath, { recursive: true });
|
||||
},
|
||||
remove: async ({ filePath, cwd, recursive, force }) => {
|
||||
const target = resolvePath(filePath, cwd);
|
||||
await fs.rm(target.hostPath, {
|
||||
recursive: recursive ?? false,
|
||||
force: force ?? false,
|
||||
});
|
||||
},
|
||||
rename: async ({ from, to, cwd }) => {
|
||||
const source = resolvePath(from, cwd);
|
||||
const target = resolvePath(to, cwd);
|
||||
await fs.mkdir(path.dirname(target.hostPath), { recursive: true });
|
||||
await fs.rename(source.hostPath, target.hostPath);
|
||||
},
|
||||
stat: async ({ filePath, cwd }) => {
|
||||
try {
|
||||
const target = resolvePath(filePath, cwd);
|
||||
const stats = await fs.stat(target.hostPath);
|
||||
return {
|
||||
type: stats.isDirectory() ? "directory" : stats.isFile() ? "file" : "other",
|
||||
size: stats.size,
|
||||
mtimeMs: stats.mtimeMs,
|
||||
} satisfies SandboxFsStat;
|
||||
} catch (error) {
|
||||
if ((error as NodeJS.ErrnoException).code === "ENOENT") {
|
||||
return null;
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function createSandbox(params: {
|
||||
sandboxRoot: string;
|
||||
agentRoot: string;
|
||||
fsBridge: SandboxFsBridge;
|
||||
}): SandboxContext {
|
||||
return {
|
||||
enabled: true,
|
||||
sessionKey: "sandbox:test",
|
||||
workspaceDir: params.sandboxRoot,
|
||||
agentWorkspaceDir: params.agentRoot,
|
||||
workspaceAccess: "rw",
|
||||
containerName: "openclaw-sbx-test",
|
||||
containerWorkdir: "/workspace",
|
||||
fsBridge: params.fsBridge,
|
||||
docker: {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
workdir: "/workspace",
|
||||
readOnlyRoot: true,
|
||||
tmpfs: [],
|
||||
network: "none",
|
||||
user: "1000:1000",
|
||||
capDrop: ["ALL"],
|
||||
env: { LANG: "C.UTF-8" },
|
||||
},
|
||||
tools: { allow: [], deny: [] },
|
||||
browserAllowHostControl: false,
|
||||
};
|
||||
}
|
||||
|
||||
describe("tools.fs.workspaceOnly", () => {
|
||||
it("defaults to allowing sandbox mounts outside the workspace root", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sbx-mounts-"));
|
||||
const sandboxRoot = path.join(stateDir, "sandbox");
|
||||
const agentRoot = path.join(stateDir, "agent");
|
||||
await fs.mkdir(sandboxRoot, { recursive: true });
|
||||
await fs.mkdir(agentRoot, { recursive: true });
|
||||
try {
|
||||
await fs.writeFile(path.join(agentRoot, "secret.txt"), "shh", "utf8");
|
||||
|
||||
const bridge = createUnsafeMountedBridge({ root: sandboxRoot, agentHostRoot: agentRoot });
|
||||
const sandbox = createSandbox({ sandboxRoot, agentRoot, fsBridge: bridge });
|
||||
|
||||
const tools = createOpenClawCodingTools({ sandbox, workspaceDir: sandboxRoot });
|
||||
const readTool = tools.find((tool) => tool.name === "read");
|
||||
const writeTool = tools.find((tool) => tool.name === "write");
|
||||
expect(readTool).toBeDefined();
|
||||
expect(writeTool).toBeDefined();
|
||||
|
||||
const readResult = await readTool?.execute("t1", { path: "/agent/secret.txt" });
|
||||
expect(getTextContent(readResult)).toContain("shh");
|
||||
|
||||
await writeTool?.execute("t2", { path: "/agent/owned.txt", content: "x" });
|
||||
expect(await fs.readFile(path.join(agentRoot, "owned.txt"), "utf8")).toBe("x");
|
||||
} finally {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects sandbox mounts outside the workspace root when enabled", async () => {
|
||||
const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sbx-mounts-"));
|
||||
const sandboxRoot = path.join(stateDir, "sandbox");
|
||||
const agentRoot = path.join(stateDir, "agent");
|
||||
await fs.mkdir(sandboxRoot, { recursive: true });
|
||||
await fs.mkdir(agentRoot, { recursive: true });
|
||||
try {
|
||||
await fs.writeFile(path.join(agentRoot, "secret.txt"), "shh", "utf8");
|
||||
|
||||
const bridge = createUnsafeMountedBridge({ root: sandboxRoot, agentHostRoot: agentRoot });
|
||||
const sandbox = createSandbox({ sandboxRoot, agentRoot, fsBridge: bridge });
|
||||
|
||||
const cfg = { tools: { fs: { workspaceOnly: true } } } as unknown as OpenClawConfig;
|
||||
const tools = createOpenClawCodingTools({ sandbox, workspaceDir: sandboxRoot, config: cfg });
|
||||
const readTool = tools.find((tool) => tool.name === "read");
|
||||
const writeTool = tools.find((tool) => tool.name === "write");
|
||||
const editTool = tools.find((tool) => tool.name === "edit");
|
||||
expect(readTool).toBeDefined();
|
||||
expect(writeTool).toBeDefined();
|
||||
expect(editTool).toBeDefined();
|
||||
|
||||
await expect(readTool?.execute("t1", { path: "/agent/secret.txt" })).rejects.toThrow(
|
||||
/Path escapes sandbox root/i,
|
||||
);
|
||||
|
||||
await expect(
|
||||
writeTool?.execute("t2", { path: "/agent/owned.txt", content: "x" }),
|
||||
).rejects.toThrow(/Path escapes sandbox root/i);
|
||||
await expect(fs.stat(path.join(agentRoot, "owned.txt"))).rejects.toMatchObject({
|
||||
code: "ENOENT",
|
||||
});
|
||||
|
||||
await expect(
|
||||
editTool?.execute("t3", { path: "/agent/secret.txt", oldText: "shh", newText: "nope" }),
|
||||
).rejects.toThrow(/Path escapes sandbox root/i);
|
||||
expect(await fs.readFile(path.join(agentRoot, "secret.txt"), "utf8")).toBe("shh");
|
||||
} finally {
|
||||
await fs.rm(stateDir, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -276,12 +276,11 @@ export function createOpenClawCodingTools(options?: {
|
||||
const base = (codingTools as unknown as AnyAgentTool[]).flatMap((tool) => {
|
||||
if (tool.name === readTool.name) {
|
||||
if (sandboxRoot) {
|
||||
return [
|
||||
createSandboxedReadTool({
|
||||
root: sandboxRoot,
|
||||
bridge: sandboxFsBridge!,
|
||||
}),
|
||||
];
|
||||
const sandboxed = createSandboxedReadTool({
|
||||
root: sandboxRoot,
|
||||
bridge: sandboxFsBridge!,
|
||||
});
|
||||
return [workspaceOnly ? wrapToolWorkspaceRootGuard(sandboxed, sandboxRoot) : sandboxed];
|
||||
}
|
||||
const freshReadTool = createReadTool(workspaceRoot);
|
||||
const wrapped = createOpenClawReadTool(freshReadTool);
|
||||
@@ -365,8 +364,18 @@ export function createOpenClawCodingTools(options?: {
|
||||
...(sandboxRoot
|
||||
? allowWorkspaceWrites
|
||||
? [
|
||||
createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
workspaceOnly
|
||||
? wrapToolWorkspaceRootGuard(
|
||||
createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
sandboxRoot,
|
||||
)
|
||||
: createSandboxedEditTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
workspaceOnly
|
||||
? wrapToolWorkspaceRootGuard(
|
||||
createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
sandboxRoot,
|
||||
)
|
||||
: createSandboxedWriteTool({ root: sandboxRoot, bridge: sandboxFsBridge! }),
|
||||
]
|
||||
: []
|
||||
: []),
|
||||
|
||||
Reference in New Issue
Block a user