refactor(agents): centralize sandbox media and fs policy helpers

This commit is contained in:
Peter Steinberger
2026-02-24 02:30:45 +00:00
parent 207ec7cfae
commit ce02ad9643
8 changed files with 178 additions and 198 deletions

View File

@@ -3,6 +3,7 @@ import { resolvePluginTools } from "../plugins/tools.js";
import type { GatewayMessageChannel } from "../utils/message-channel.js";
import { resolveSessionAgentId } from "./agent-scope.js";
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
import type { ToolFsPolicy } from "./tool-fs-policy.js";
import { createAgentsListTool } from "./tools/agents-list-tool.js";
import { createBrowserTool } from "./tools/browser-tool.js";
import { createCanvasTool } from "./tools/canvas-tool.js";
@@ -41,7 +42,7 @@ export function createOpenClawTools(options?: {
agentDir?: string;
sandboxRoot?: string;
sandboxFsBridge?: SandboxFsBridge;
workspaceOnly?: boolean;
fsPolicy?: ToolFsPolicy;
workspaceDir?: string;
sandboxed?: boolean;
config?: OpenClawConfig;
@@ -79,7 +80,7 @@ export function createOpenClawTools(options?: {
options?.sandboxRoot && options?.sandboxFsBridge
? { root: options.sandboxRoot, bridge: options.sandboxFsBridge }
: undefined,
workspaceOnly: options?.workspaceOnly,
fsPolicy: options?.fsPolicy,
modelHasVision: options?.modelHasVision,
})
: null;

View File

@@ -1,99 +1,23 @@
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 { createOpenClawCodingTools } from "./pi-tools.js";
import type { SandboxContext } from "./sandbox.js";
import type { SandboxFsBridge, SandboxResolvedPath } from "./sandbox/fs-bridge.js";
import { createSandboxFsBridgeFromResolver } from "./test-helpers/host-sandbox-fs-bridge.js";
import {
expectReadWriteEditTools,
expectReadWriteTools,
getTextContent,
} from "./test-helpers/pi-tools-fs-helpers.js";
import { createPiToolsSandboxContext } from "./test-helpers/pi-tools-sandbox-context.js";
import { withUnsafeMountedSandboxHarness } from "./test-helpers/unsafe-mounted-sandbox.js";
vi.mock("../infra/shell-env.js", async (importOriginal) => {
const mod = await importOriginal<typeof import("../infra/shell-env.js")>();
return { ...mod, getShellPathFromLoginShell: () => null };
});
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 createSandboxFsBridgeFromResolver(resolvePath);
}
function createSandbox(params: {
sandboxRoot: string;
agentRoot: string;
fsBridge: SandboxFsBridge;
}): SandboxContext {
return createPiToolsSandboxContext({
workspaceDir: params.sandboxRoot,
agentWorkspaceDir: params.agentRoot,
workspaceAccess: "rw",
fsBridge: params.fsBridge,
tools: { allow: [], deny: [] },
});
}
type ToolWithExecute = {
execute: (toolCallId: string, args: unknown, signal?: AbortSignal) => Promise<unknown>;
};
async function withUnsafeMountedSandboxHarness(
run: (ctx: { sandboxRoot: string; agentRoot: string; sandbox: SandboxContext }) => Promise<void>,
) {
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 });
const bridge = createUnsafeMountedBridge({ root: sandboxRoot, agentHostRoot: agentRoot });
const sandbox = createSandbox({ sandboxRoot, agentRoot, fsBridge: bridge });
try {
await run({ sandboxRoot, agentRoot, sandbox });
} finally {
await fs.rm(stateDir, { recursive: true, force: true });
}
}
describe("tools.fs.workspaceOnly", () => {
it("defaults to allowing sandbox mounts outside the workspace root", async () => {
await withUnsafeMountedSandboxHarness(async ({ sandboxRoot, agentRoot, sandbox }) => {

View File

@@ -49,6 +49,7 @@ import { cleanToolSchemaForGemini, normalizeToolParameters } from "./pi-tools.sc
import type { AnyAgentTool } from "./pi-tools.types.js";
import type { SandboxContext } from "./sandbox.js";
import { getSubagentDepthFromSessionStore } from "./subagent-depth.js";
import { createToolFsPolicy } from "./tool-fs-policy.js";
import {
applyToolPolicyPipeline,
buildDefaultToolPolicyPipelineSteps,
@@ -291,11 +292,14 @@ export function createOpenClawCodingTools(options?: {
]);
const execConfig = resolveExecConfig({ cfg: options?.config, agentId });
const fsConfig = resolveFsConfig({ cfg: options?.config, agentId });
const fsPolicy = createToolFsPolicy({
workspaceOnly: fsConfig.workspaceOnly,
});
const sandboxRoot = sandbox?.workspaceDir;
const sandboxFsBridge = sandbox?.fsBridge;
const allowWorkspaceWrites = sandbox?.workspaceAccess !== "ro";
const workspaceRoot = resolveWorkspaceRoot(options?.workspaceDir);
const workspaceOnly = fsConfig.workspaceOnly === true;
const workspaceOnly = fsPolicy.workspaceOnly;
const applyPatchConfig = execConfig.applyPatch;
// Secure by default: apply_patch is workspace-contained unless explicitly disabled.
// (tools.fs.workspaceOnly is a separate umbrella flag for read/write/edit/apply_patch.)
@@ -458,7 +462,7 @@ export function createOpenClawCodingTools(options?: {
agentDir: options?.agentDir,
sandboxRoot,
sandboxFsBridge,
workspaceOnly,
fsPolicy,
workspaceDir: workspaceRoot,
sandboxed: !!sandbox,
config: options?.config,

View File

@@ -0,0 +1,63 @@
import path from "node:path";
import { assertSandboxPath } from "./sandbox-paths.js";
import type { SandboxFsBridge } from "./sandbox/fs-bridge.js";
export type SandboxedBridgeMediaPathConfig = {
root: string;
bridge: SandboxFsBridge;
workspaceOnly?: boolean;
};
export async function resolveSandboxedBridgeMediaPath(params: {
sandbox: SandboxedBridgeMediaPathConfig;
mediaPath: string;
inboundFallbackDir?: string;
}): Promise<{ resolved: string; rewrittenFrom?: string }> {
const normalizeFileUrl = (rawPath: string) =>
rawPath.startsWith("file://") ? rawPath.slice("file://".length) : rawPath;
const filePath = normalizeFileUrl(params.mediaPath);
const enforceWorkspaceBoundary = async (hostPath: string) => {
if (!params.sandbox.workspaceOnly) {
return;
}
await assertSandboxPath({
filePath: hostPath,
cwd: params.sandbox.root,
root: params.sandbox.root,
});
};
const resolveDirect = () =>
params.sandbox.bridge.resolvePath({
filePath,
cwd: params.sandbox.root,
});
try {
const resolved = resolveDirect();
await enforceWorkspaceBoundary(resolved.hostPath);
return { resolved: resolved.hostPath };
} catch (err) {
const fallbackDir = params.inboundFallbackDir?.trim();
if (!fallbackDir) {
throw err;
}
const fallbackPath = path.join(fallbackDir, path.basename(filePath));
try {
const stat = await params.sandbox.bridge.stat({
filePath: fallbackPath,
cwd: params.sandbox.root,
});
if (!stat) {
throw err;
}
} catch {
throw err;
}
const resolvedFallback = params.sandbox.bridge.resolvePath({
filePath: fallbackPath,
cwd: params.sandbox.root,
});
await enforceWorkspaceBoundary(resolvedFallback.hostPath);
return { resolved: resolvedFallback.hostPath, rewrittenFrom: filePath };
}
}

View File

@@ -0,0 +1,82 @@
import fs from "node:fs/promises";
import os from "node:os";
import path from "node:path";
import type { SandboxContext } from "../sandbox.js";
import type { SandboxFsBridge, SandboxResolvedPath } from "../sandbox/fs-bridge.js";
import { createSandboxFsBridgeFromResolver } from "./host-sandbox-fs-bridge.js";
import { createPiToolsSandboxContext } from "./pi-tools-sandbox-context.js";
export 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 createSandboxFsBridgeFromResolver(resolvePath);
}
export function createUnsafeMountedSandbox(params: {
sandboxRoot: string;
agentRoot: string;
workspaceContainerRoot?: string;
}): SandboxContext {
const bridge = createUnsafeMountedBridge({
root: params.sandboxRoot,
agentHostRoot: params.agentRoot,
workspaceContainerRoot: params.workspaceContainerRoot,
});
return createPiToolsSandboxContext({
workspaceDir: params.sandboxRoot,
agentWorkspaceDir: params.agentRoot,
workspaceAccess: "rw",
fsBridge: bridge,
tools: { allow: [], deny: [] },
});
}
export async function withUnsafeMountedSandboxHarness(
run: (ctx: { sandboxRoot: string; agentRoot: string; sandbox: SandboxContext }) => Promise<void>,
) {
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 });
const sandbox = createUnsafeMountedSandbox({ sandboxRoot, agentRoot });
try {
await run({ sandboxRoot, agentRoot, sandbox });
} finally {
await fs.rm(stateDir, { recursive: true, force: true });
}
}

View File

@@ -0,0 +1,9 @@
export type ToolFsPolicy = {
workspaceOnly: boolean;
};
export function createToolFsPolicy(params: { workspaceOnly?: boolean }): ToolFsPolicy {
return {
workspaceOnly: params.workspaceOnly === true,
};
}

View File

@@ -6,13 +6,8 @@ import type { OpenClawConfig } from "../../config/config.js";
import type { ModelDefinitionConfig } from "../../config/types.models.js";
import { withFetchPreconnect } from "../../test-utils/fetch-mock.js";
import { createOpenClawCodingTools } from "../pi-tools.js";
import type { SandboxContext } from "../sandbox.js";
import type { SandboxFsBridge, SandboxResolvedPath } from "../sandbox/fs-bridge.js";
import {
createHostSandboxFsBridge,
createSandboxFsBridgeFromResolver,
} from "../test-helpers/host-sandbox-fs-bridge.js";
import { createPiToolsSandboxContext } from "../test-helpers/pi-tools-sandbox-context.js";
import { createHostSandboxFsBridge } from "../test-helpers/host-sandbox-fs-bridge.js";
import { createUnsafeMountedSandbox } from "../test-helpers/unsafe-mounted-sandbox.js";
import { __testing, createImageTool, resolveImageModelConfigForTool } from "./image-tool.js";
async function writeAuthProfiles(agentDir: string, profiles: unknown) {
@@ -52,58 +47,6 @@ async function withTempWorkspacePng(
}
}
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 => {
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 createSandboxFsBridgeFromResolver(resolvePath);
}
function createSandbox(params: {
sandboxRoot: string;
agentRoot: string;
fsBridge: SandboxFsBridge;
}): SandboxContext {
return createPiToolsSandboxContext({
workspaceDir: params.sandboxRoot,
agentWorkspaceDir: params.agentRoot,
workspaceAccess: "rw",
fsBridge: params.fsBridge,
tools: { allow: [], deny: [] },
});
}
function stubMinimaxOkFetch() {
const fetch = vi.fn().mockResolvedValue({
ok: true,
@@ -569,8 +512,7 @@ describe("image tool implicit imageModel config", () => {
await fs.mkdir(sandboxRoot, { recursive: true });
await fs.writeFile(path.join(agentDir, "secret.png"), Buffer.from(ONE_PIXEL_PNG_B64, "base64"));
const bridge = createUnsafeMountedBridge({ root: sandboxRoot, agentHostRoot: agentDir });
const sandbox = createSandbox({ sandboxRoot, agentRoot: agentDir, fsBridge: bridge });
const sandbox = createUnsafeMountedSandbox({ sandboxRoot, agentRoot: agentDir });
const fetch = stubMinimaxOkFetch();
const cfg: OpenClawConfig = {
...createMinimaxImageConfig(),

View File

@@ -1,4 +1,3 @@
import path from "node:path";
import { type Api, type Context, complete, type Model } from "@mariozechner/pi-ai";
import { Type } from "@sinclair/typebox";
import type { OpenClawConfig } from "../../config/config.js";
@@ -12,8 +11,12 @@ import { runWithImageModelFallback } from "../model-fallback.js";
import { resolveConfiguredModelRef } from "../model-selection.js";
import { ensureOpenClawModelsJson } from "../models-config.js";
import { discoverAuthStorage, discoverModels } from "../pi-model-discovery.js";
import { assertSandboxPath } from "../sandbox-paths.js";
import {
resolveSandboxedBridgeMediaPath,
type SandboxedBridgeMediaPathConfig,
} from "../sandbox-media-paths.js";
import type { SandboxFsBridge } from "../sandbox/fs-bridge.js";
import type { ToolFsPolicy } from "../tool-fs-policy.js";
import { normalizeWorkspaceDir } from "../workspace-dir.js";
import type { AnyAgentTool } from "./common.js";
import {
@@ -208,57 +211,8 @@ function buildImageContext(
type ImageSandboxConfig = {
root: string;
bridge: SandboxFsBridge;
workspaceOnly?: boolean;
};
async function resolveSandboxedImagePath(params: {
sandbox: ImageSandboxConfig;
imagePath: string;
}): Promise<{ resolved: string; rewrittenFrom?: string }> {
const normalize = (p: string) => (p.startsWith("file://") ? p.slice("file://".length) : p);
const filePath = normalize(params.imagePath);
try {
const resolved = params.sandbox.bridge.resolvePath({
filePath,
cwd: params.sandbox.root,
});
if (params.sandbox.workspaceOnly) {
await assertSandboxPath({
filePath: resolved.hostPath,
cwd: params.sandbox.root,
root: params.sandbox.root,
});
}
return { resolved: resolved.hostPath };
} catch (err) {
const name = path.basename(filePath);
const candidateRel = path.join("media", "inbound", name);
try {
const stat = await params.sandbox.bridge.stat({
filePath: candidateRel,
cwd: params.sandbox.root,
});
if (!stat) {
throw err;
}
} catch {
throw err;
}
const out = params.sandbox.bridge.resolvePath({
filePath: candidateRel,
cwd: params.sandbox.root,
});
if (params.sandbox.workspaceOnly) {
await assertSandboxPath({
filePath: out.hostPath,
cwd: params.sandbox.root,
root: params.sandbox.root,
});
}
return { resolved: out.hostPath, rewrittenFrom: filePath };
}
}
async function runImagePrompt(params: {
cfg?: OpenClawConfig;
agentDir: string;
@@ -352,7 +306,7 @@ export function createImageTool(options?: {
agentDir?: string;
workspaceDir?: string;
sandbox?: ImageSandboxConfig;
workspaceOnly?: boolean;
fsPolicy?: ToolFsPolicy;
/** If true, the model has native vision capability and images in the prompt are auto-injected */
modelHasVision?: boolean;
}): AnyAgentTool | null {
@@ -459,12 +413,12 @@ export function createImageTool(options?: {
const maxBytesMb = typeof record.maxBytesMb === "number" ? record.maxBytesMb : undefined;
const maxBytes = pickMaxBytes(options?.config, maxBytesMb);
const sandboxConfig =
const sandboxConfig: SandboxedBridgeMediaPathConfig | null =
options?.sandbox && options?.sandbox.root.trim()
? {
root: options.sandbox.root.trim(),
bridge: options.sandbox.bridge,
workspaceOnly: options.workspaceOnly === true,
workspaceOnly: options.fsPolicy?.workspaceOnly === true,
}
: null;
@@ -524,9 +478,10 @@ export function createImageTool(options?: {
const resolvedPathInfo: { resolved: string; rewrittenFrom?: string } = isDataUrl
? { resolved: "" }
: sandboxConfig
? await resolveSandboxedImagePath({
? await resolveSandboxedBridgeMediaPath({
sandbox: sandboxConfig,
imagePath: resolvedImage,
mediaPath: resolvedImage,
inboundFallbackDir: "media/inbound",
})
: {
resolved: resolvedImage.startsWith("file://")