From dd9d9c1c609dcb4579f9e57bd7b5c879d0146b53 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 02:17:06 +0000 Subject: [PATCH] fix(security): enforce workspaceOnly for sandbox image tool --- CHANGELOG.md | 1 + src/agents/openclaw-tools.ts | 2 + src/agents/pi-tools.ts | 1 + src/agents/tools/image-tool.test.ts | 104 +++++++++++++++++++++++++++- src/agents/tools/image-tool.ts | 23 +++++- 5 files changed, 129 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d34e9405779..dcb1e9f6209 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai - Security/Exec: harden `safeBins` long-option validation by rejecting unknown/ambiguous GNU long-option abbreviations and denying sort filesystem-dependent flags (`--random-source`, `--temporary-directory`, `-T`), closing safe-bin denylist bypasses. Thanks @jiseoung. - Security/Channels: unify dangerous name-matching policy checks (`dangerouslyAllowNameMatching`) across core and extension channels, share mutable-allowlist detectors between `openclaw doctor` and `openclaw security audit`, and scan all configured accounts (not only the default account) in channel security audit findings. - Security/Exec approvals: enforce canonical wrapper execution plans across allowlist analysis and runtime execution (node host + gateway host), fail closed on semantic `env` wrapper usage, and reject unknown short safe-bin flags to prevent `env -S/--split-string` interpretation-mismatch bypasses. This ships in the next npm release. Thanks @jiseoung for reporting. +- Security/Image tool: enforce `tools.fs.workspaceOnly` for sandboxed `image` path resolution so mounted out-of-workspace paths are blocked before media bytes are loaded/sent to vision providers. This ships in the next npm release. Thanks @tdjackey for reporting. ## 2026.2.23 (Unreleased) diff --git a/src/agents/openclaw-tools.ts b/src/agents/openclaw-tools.ts index 0cc577bb42b..e6ba62650be 100644 --- a/src/agents/openclaw-tools.ts +++ b/src/agents/openclaw-tools.ts @@ -41,6 +41,7 @@ export function createOpenClawTools(options?: { agentDir?: string; sandboxRoot?: string; sandboxFsBridge?: SandboxFsBridge; + workspaceOnly?: boolean; workspaceDir?: string; sandboxed?: boolean; config?: OpenClawConfig; @@ -78,6 +79,7 @@ export function createOpenClawTools(options?: { options?.sandboxRoot && options?.sandboxFsBridge ? { root: options.sandboxRoot, bridge: options.sandboxFsBridge } : undefined, + workspaceOnly: options?.workspaceOnly, modelHasVision: options?.modelHasVision, }) : null; diff --git a/src/agents/pi-tools.ts b/src/agents/pi-tools.ts index 4b09d9ad993..6383ae2c815 100644 --- a/src/agents/pi-tools.ts +++ b/src/agents/pi-tools.ts @@ -458,6 +458,7 @@ export function createOpenClawCodingTools(options?: { agentDir: options?.agentDir, sandboxRoot, sandboxFsBridge, + workspaceOnly, workspaceDir: workspaceRoot, sandboxed: !!sandbox, config: options?.config, diff --git a/src/agents/tools/image-tool.test.ts b/src/agents/tools/image-tool.test.ts index 1a87e4e2af6..71b2f375b1f 100644 --- a/src/agents/tools/image-tool.test.ts +++ b/src/agents/tools/image-tool.test.ts @@ -6,7 +6,13 @@ 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 { createHostSandboxFsBridge } from "../test-helpers/host-sandbox-fs-bridge.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 { __testing, createImageTool, resolveImageModelConfigForTool } from "./image-tool.js"; async function writeAuthProfiles(agentDir: string, profiles: unknown) { @@ -46,6 +52,58 @@ 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, @@ -503,6 +561,50 @@ describe("image tool implicit imageModel config", () => { ); }); + it("applies tools.fs.workspaceOnly to image paths in sandbox mode", async () => { + const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-image-sandbox-")); + const agentDir = path.join(stateDir, "agent"); + const sandboxRoot = path.join(stateDir, "sandbox"); + await fs.mkdir(agentDir, { recursive: true }); + 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 fetch = stubMinimaxOkFetch(); + const cfg: OpenClawConfig = { + ...createMinimaxImageConfig(), + tools: { fs: { workspaceOnly: true } }, + }; + + try { + const tools = createOpenClawCodingTools({ + config: cfg, + agentDir, + sandbox, + workspaceDir: sandboxRoot, + }); + const readTool = tools.find((candidate) => candidate.name === "read"); + if (!readTool) { + throw new Error("expected read tool"); + } + const imageTool = requireImageTool(tools.find((candidate) => candidate.name === "image")); + + await expect(readTool.execute("t1", { path: "/agent/secret.png" })).rejects.toThrow( + /Path escapes sandbox root/i, + ); + await expect( + imageTool.execute("t2", { + prompt: "Describe the image.", + image: "/agent/secret.png", + }), + ).rejects.toThrow(/Path escapes sandbox root/i); + expect(fetch).not.toHaveBeenCalled(); + } finally { + await fs.rm(stateDir, { recursive: true, force: true }); + } + }); + it("rewrites inbound absolute paths into sandbox media/inbound", async () => { const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-image-sandbox-")); const agentDir = path.join(stateDir, "agent"); diff --git a/src/agents/tools/image-tool.ts b/src/agents/tools/image-tool.ts index f27f9bdaaaf..872c95f8653 100644 --- a/src/agents/tools/image-tool.ts +++ b/src/agents/tools/image-tool.ts @@ -12,6 +12,7 @@ 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 type { SandboxFsBridge } from "../sandbox/fs-bridge.js"; import { normalizeWorkspaceDir } from "../workspace-dir.js"; import type { AnyAgentTool } from "./common.js"; @@ -207,6 +208,7 @@ function buildImageContext( type ImageSandboxConfig = { root: string; bridge: SandboxFsBridge; + workspaceOnly?: boolean; }; async function resolveSandboxedImagePath(params: { @@ -220,6 +222,13 @@ async function resolveSandboxedImagePath(params: { 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); @@ -239,6 +248,13 @@ async function resolveSandboxedImagePath(params: { 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 }; } } @@ -336,6 +352,7 @@ export function createImageTool(options?: { agentDir?: string; workspaceDir?: string; sandbox?: ImageSandboxConfig; + workspaceOnly?: boolean; /** If true, the model has native vision capability and images in the prompt are auto-injected */ modelHasVision?: boolean; }): AnyAgentTool | null { @@ -444,7 +461,11 @@ export function createImageTool(options?: { const sandboxConfig = options?.sandbox && options?.sandbox.root.trim() - ? { root: options.sandbox.root.trim(), bridge: options.sandbox.bridge } + ? { + root: options.sandbox.root.trim(), + bridge: options.sandbox.bridge, + workspaceOnly: options.workspaceOnly === true, + } : null; // MARK: - Load and resolve each image