From c070be1bc4d0d25f2be1c0d1ff0702fb5fc56efd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 24 Feb 2026 02:21:33 +0000 Subject: [PATCH] fix(sandbox): harden fs bridge path checks and bind mount policy --- src/agents/sandbox-create-args.test.ts | 43 +++++ src/agents/sandbox/browser.ts | 1 + .../docker.config-hash-recreate.test.ts | 2 + src/agents/sandbox/docker.ts | 15 +- src/agents/sandbox/fs-bridge.test.ts | 60 +++++++ src/agents/sandbox/fs-bridge.ts | 167 +++++++++++++++++ .../sandbox/validate-sandbox-security.test.ts | 42 +++++ .../sandbox/validate-sandbox-security.ts | 169 ++++++++++++++++-- src/config/types.sandbox.ts | 10 ++ src/config/zod-schema.agent-runtime.ts | 2 + src/security/audit-extra.sync.ts | 3 + 11 files changed, 496 insertions(+), 18 deletions(-) diff --git a/src/agents/sandbox-create-args.test.ts b/src/agents/sandbox-create-args.test.ts index b11a62eec..2347b88fc 100644 --- a/src/agents/sandbox-create-args.test.ts +++ b/src/agents/sandbox-create-args.test.ts @@ -228,4 +228,47 @@ describe("buildSandboxCreateArgs", () => { } expect(customVFlags).toHaveLength(0); }); + + it("blocks bind sources outside runtime allowlist roots", () => { + const cfg = createSandboxConfig({}, ["/opt/external:/data:rw"]); + expect(() => + buildSandboxCreateArgs({ + name: "openclaw-sbx-outside-roots", + cfg, + scopeKey: "main", + createdAtMs: 1700000000000, + bindSourceRoots: ["/tmp/workspace", "/tmp/agent"], + }), + ).toThrow(/outside allowed roots/); + }); + + it("allows bind sources outside runtime allowlist with explicit override", () => { + const cfg = createSandboxConfig({}, ["/opt/external:/data:rw"]); + const args = buildSandboxCreateArgs({ + name: "openclaw-sbx-outside-roots-override", + cfg, + scopeKey: "main", + createdAtMs: 1700000000000, + bindSourceRoots: ["/tmp/workspace", "/tmp/agent"], + allowSourcesOutsideAllowedRoots: true, + }); + expect(args).toEqual(expect.arrayContaining(["-v", "/opt/external:/data:rw"])); + }); + + it("blocks reserved /workspace target bind mounts by default", () => { + const cfg = createSandboxConfig({}, ["/tmp/override:/workspace:rw"]); + expectBuildToThrow("openclaw-sbx-reserved-target", cfg, /reserved container path/); + }); + + it("allows reserved /workspace target bind mounts with explicit dangerous override", () => { + const cfg = createSandboxConfig({}, ["/tmp/override:/workspace:rw"]); + const args = buildSandboxCreateArgs({ + name: "openclaw-sbx-reserved-target-override", + cfg, + scopeKey: "main", + createdAtMs: 1700000000000, + allowReservedContainerTargets: true, + }); + expect(args).toEqual(expect.arrayContaining(["-v", "/tmp/override:/workspace:rw"])); + }); }); diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index 0c49e6323..f96261bfa 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -228,6 +228,7 @@ export async function ensureSandboxBrowser(params: { }, configHash: expectedHash, includeBinds: false, + bindSourceRoots: [params.workspaceDir, params.agentWorkspaceDir], }); const mainMountSuffix = params.cfg.workspaceAccess === "ro" && params.workspaceDir === params.agentWorkspaceDir diff --git a/src/agents/sandbox/docker.config-hash-recreate.test.ts b/src/agents/sandbox/docker.config-hash-recreate.test.ts index 08155c305..1664cb16a 100644 --- a/src/agents/sandbox/docker.config-hash-recreate.test.ts +++ b/src/agents/sandbox/docker.config-hash-recreate.test.ts @@ -101,6 +101,7 @@ function createSandboxConfig(dns: string[], binds?: string[]): SandboxConfig { dns, extraHosts: ["host.docker.internal:host-gateway"], binds: binds ?? ["/tmp/workspace:/workspace:rw"], + dangerouslyAllowReservedContainerTargets: true, }, browser: { enabled: false, @@ -196,6 +197,7 @@ describe("ensureSandboxContainer config-hash recreation", () => { ["1.1.1.1"], ["/tmp/workspace-shared/USER.md:/workspace/USER.md:ro"], ); + cfg.docker.dangerouslyAllowExternalBindSources = true; const expectedHash = computeSandboxConfigHash({ docker: cfg.docker, workspaceAccess: cfg.workspaceAccess, diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 6f6769fa3..270e8b761 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -264,9 +264,21 @@ export function buildSandboxCreateArgs(params: { labels?: Record; configHash?: string; includeBinds?: boolean; + bindSourceRoots?: string[]; + allowSourcesOutsideAllowedRoots?: boolean; + allowReservedContainerTargets?: boolean; }) { // Runtime security validation: blocks dangerous bind mounts, network modes, and profiles. - validateSandboxSecurity(params.cfg); + validateSandboxSecurity({ + ...params.cfg, + allowedSourceRoots: params.bindSourceRoots, + allowSourcesOutsideAllowedRoots: + params.allowSourcesOutsideAllowedRoots ?? + params.cfg.dangerouslyAllowExternalBindSources === true, + allowReservedContainerTargets: + params.allowReservedContainerTargets ?? + params.cfg.dangerouslyAllowReservedContainerTargets === true, + }); const createdAtMs = params.createdAtMs ?? Date.now(); const args = ["create", "--name", params.name]; @@ -378,6 +390,7 @@ async function createSandboxContainer(params: { scopeKey, configHash: params.configHash, includeBinds: false, + bindSourceRoots: [workspaceDir, params.agentWorkspaceDir], }); args.push("--workdir", cfg.workdir); const mainMountSuffix = diff --git a/src/agents/sandbox/fs-bridge.test.ts b/src/agents/sandbox/fs-bridge.test.ts index 56fbdb8ee..ca4dd9d62 100644 --- a/src/agents/sandbox/fs-bridge.test.ts +++ b/src/agents/sandbox/fs-bridge.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; import { beforeEach, describe, expect, it, vi } from "vitest"; vi.mock("./docker.js", () => ({ @@ -29,6 +32,13 @@ describe("sandbox fs bridge shell compatibility", () => { mockedExecDockerRaw.mockClear(); mockedExecDockerRaw.mockImplementation(async (args) => { const script = args[5] ?? ""; + if (script.includes('readlink -f -- "$cursor"')) { + return { + stdout: Buffer.from(`${String(args.at(-2) ?? "")}\n`), + stderr: Buffer.alloc(0), + code: 0, + }; + } if (script.includes('stat -c "%F|%s|%Y"')) { return { stdout: Buffer.from("regular file|1|2"), @@ -103,4 +113,54 @@ describe("sandbox fs bridge shell compatibility", () => { ).rejects.toThrow(/read-only/); expect(mockedExecDockerRaw).not.toHaveBeenCalled(); }); + + it("rejects pre-existing host symlink escapes before docker exec", async () => { + const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-fs-bridge-")); + const workspaceDir = path.join(stateDir, "workspace"); + const outsideDir = path.join(stateDir, "outside"); + await fs.mkdir(workspaceDir, { recursive: true }); + await fs.mkdir(outsideDir, { recursive: true }); + await fs.symlink(path.join(outsideDir, "secret.txt"), path.join(workspaceDir, "link.txt")); + + const bridge = createSandboxFsBridge({ + sandbox: createSandbox({ + workspaceDir, + agentWorkspaceDir: workspaceDir, + }), + }); + + await expect(bridge.readFile({ filePath: "link.txt" })).rejects.toThrow(/Symlink escapes/); + expect(mockedExecDockerRaw).not.toHaveBeenCalled(); + await fs.rm(stateDir, { recursive: true, force: true }); + }); + + it("rejects container-canonicalized paths outside allowed mounts", async () => { + mockedExecDockerRaw.mockImplementation(async (args) => { + const script = args[5] ?? ""; + if (script.includes('readlink -f -- "$cursor"')) { + return { + stdout: Buffer.from("/etc/passwd\n"), + stderr: Buffer.alloc(0), + code: 0, + }; + } + if (script.includes('cat -- "$1"')) { + return { + stdout: Buffer.from("content"), + stderr: Buffer.alloc(0), + code: 0, + }; + } + return { + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + code: 0, + }; + }); + + const bridge = createSandboxFsBridge({ sandbox: createSandbox() }); + await expect(bridge.readFile({ filePath: "a.txt" })).rejects.toThrow(/escapes allowed mounts/i); + const scripts = mockedExecDockerRaw.mock.calls.map(([args]) => args[5] ?? ""); + expect(scripts.some((script) => script.includes('cat -- "$1"'))).toBe(false); + }); }); diff --git a/src/agents/sandbox/fs-bridge.ts b/src/agents/sandbox/fs-bridge.ts index c9e9a1503..fdcaf0cc4 100644 --- a/src/agents/sandbox/fs-bridge.ts +++ b/src/agents/sandbox/fs-bridge.ts @@ -1,8 +1,12 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { isNotFoundPathError, isPathInside } from "../../infra/path-guards.js"; import { execDockerRaw, type ExecDockerRawResult } from "./docker.js"; import { buildSandboxFsMounts, resolveSandboxFsPathWithMounts, type SandboxResolvedFsPath, + type SandboxFsMount, } from "./fs-paths.js"; import type { SandboxContext, SandboxWorkspaceAccess } from "./types.js"; @@ -13,6 +17,12 @@ type RunCommandOptions = { signal?: AbortSignal; }; +type PathSafetyOptions = { + action: string; + allowFinalSymlink?: boolean; + requireWritable?: boolean; +}; + export type SandboxResolvedPath = { hostPath: string; relativePath: string; @@ -59,10 +69,14 @@ export function createSandboxFsBridge(params: { sandbox: SandboxContext }): Sand class SandboxFsBridgeImpl implements SandboxFsBridge { private readonly sandbox: SandboxContext; private readonly mounts: ReturnType; + private readonly mountsByContainer: ReturnType; constructor(sandbox: SandboxContext) { this.sandbox = sandbox; this.mounts = buildSandboxFsMounts(sandbox); + this.mountsByContainer = [...this.mounts].toSorted( + (a, b) => b.containerRoot.length - a.containerRoot.length, + ); } resolvePath(params: { filePath: string; cwd?: string }): SandboxResolvedPath { @@ -80,6 +94,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveResolvedPath(params); + await this.assertPathSafety(target, { action: "read files" }); const result = await this.runCommand('set -eu; cat -- "$1"', { args: [target.containerPath], signal: params.signal, @@ -97,6 +112,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { }): Promise { const target = this.resolveResolvedPath(params); this.ensureWriteAccess(target, "write files"); + await this.assertPathSafety(target, { action: "write files", requireWritable: true }); const buffer = Buffer.isBuffer(params.data) ? params.data : Buffer.from(params.data, params.encoding ?? "utf8"); @@ -114,6 +130,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { async mkdirp(params: { filePath: string; cwd?: string; signal?: AbortSignal }): Promise { const target = this.resolveResolvedPath(params); this.ensureWriteAccess(target, "create directories"); + await this.assertPathSafety(target, { action: "create directories", requireWritable: true }); await this.runCommand('set -eu; mkdir -p -- "$1"', { args: [target.containerPath], signal: params.signal, @@ -129,6 +146,11 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { }): Promise { const target = this.resolveResolvedPath(params); this.ensureWriteAccess(target, "remove files"); + await this.assertPathSafety(target, { + action: "remove files", + requireWritable: true, + allowFinalSymlink: true, + }); const flags = [params.force === false ? "" : "-f", params.recursive ? "-r" : ""].filter( Boolean, ); @@ -149,6 +171,15 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { const to = this.resolveResolvedPath({ filePath: params.to, cwd: params.cwd }); this.ensureWriteAccess(from, "rename files"); this.ensureWriteAccess(to, "rename files"); + await this.assertPathSafety(from, { + action: "rename files", + requireWritable: true, + allowFinalSymlink: true, + }); + await this.assertPathSafety(to, { + action: "rename files", + requireWritable: true, + }); await this.runCommand( 'set -eu; dir=$(dirname -- "$2"); if [ "$dir" != "." ]; then mkdir -p -- "$dir"; fi; mv -- "$1" "$2"', { @@ -164,6 +195,7 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { signal?: AbortSignal; }): Promise { const target = this.resolveResolvedPath(params); + await this.assertPathSafety(target, { action: "stat files" }); const result = await this.runCommand('set -eu; stat -c "%F|%s|%Y" -- "$1"', { args: [target.containerPath], signal: params.signal, @@ -211,6 +243,79 @@ class SandboxFsBridgeImpl implements SandboxFsBridge { }); } + private async assertPathSafety(target: SandboxResolvedFsPath, options: PathSafetyOptions) { + const lexicalMount = this.resolveMountByContainerPath(target.containerPath); + if (!lexicalMount) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${options.action}: ${target.containerPath}`, + ); + } + + await assertNoHostSymlinkEscape({ + absolutePath: target.hostPath, + rootPath: lexicalMount.hostRoot, + allowFinalSymlink: options.allowFinalSymlink === true, + }); + + const canonicalContainerPath = await this.resolveCanonicalContainerPath({ + containerPath: target.containerPath, + allowFinalSymlink: options.allowFinalSymlink === true, + }); + const canonicalMount = this.resolveMountByContainerPath(canonicalContainerPath); + if (!canonicalMount) { + throw new Error( + `Sandbox path escapes allowed mounts; cannot ${options.action}: ${target.containerPath}`, + ); + } + if (options.requireWritable && !canonicalMount.writable) { + throw new Error( + `Sandbox path is read-only; cannot ${options.action}: ${target.containerPath}`, + ); + } + } + + private resolveMountByContainerPath(containerPath: string): SandboxFsMount | null { + const normalized = normalizeContainerPath(containerPath); + for (const mount of this.mountsByContainer) { + if (isPathInsidePosix(normalizeContainerPath(mount.containerRoot), normalized)) { + return mount; + } + } + return null; + } + + private async resolveCanonicalContainerPath(params: { + containerPath: string; + allowFinalSymlink: boolean; + }): Promise { + const script = [ + "set -eu", + 'target="$1"', + 'allow_final="$2"', + 'suffix=""', + 'probe="$target"', + 'if [ "$allow_final" = "1" ] && [ -L "$target" ]; then probe=$(dirname -- "$target"); fi', + 'cursor="$probe"', + 'while [ ! -e "$cursor" ] && [ ! -L "$cursor" ]; do', + ' parent=$(dirname -- "$cursor")', + ' if [ "$parent" = "$cursor" ]; then break; fi', + ' base=$(basename -- "$cursor")', + ' suffix="/$base$suffix"', + ' cursor="$parent"', + "done", + 'canonical=$(readlink -f -- "$cursor")', + 'printf "%s%s\\n" "$canonical" "$suffix"', + ].join("; "); + const result = await this.runCommand(script, { + args: [params.containerPath, params.allowFinalSymlink ? "1" : "0"], + }); + const canonical = result.stdout.toString("utf8").trim(); + if (!canonical.startsWith("/")) { + throw new Error(`Failed to resolve canonical sandbox path: ${params.containerPath}`); + } + return normalizeContainerPath(canonical); + } + 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}`); @@ -245,3 +350,65 @@ function coerceStatType(typeRaw?: string): "file" | "directory" | "other" { } return "other"; } + +function normalizeContainerPath(value: string): string { + const normalized = path.posix.normalize(value); + return normalized === "." ? "/" : normalized; +} + +function isPathInsidePosix(root: string, target: string): boolean { + if (root === "/") { + return true; + } + return target === root || target.startsWith(`${root}/`); +} + +async function assertNoHostSymlinkEscape(params: { + absolutePath: string; + rootPath: string; + allowFinalSymlink: boolean; +}): Promise { + const root = path.resolve(params.rootPath); + const target = path.resolve(params.absolutePath); + if (!isPathInside(root, target)) { + throw new Error(`Sandbox path escapes mount root (${root}): ${params.absolutePath}`); + } + const relative = path.relative(root, target); + if (!relative) { + return; + } + const rootReal = await tryRealpath(root); + const parts = relative.split(path.sep).filter(Boolean); + let current = root; + for (let idx = 0; idx < parts.length; idx += 1) { + current = path.join(current, parts[idx] ?? ""); + const isLast = idx === parts.length - 1; + try { + const stat = await fs.lstat(current); + if (!stat.isSymbolicLink()) { + continue; + } + if (params.allowFinalSymlink && isLast) { + return; + } + const symlinkTarget = await tryRealpath(current); + if (!isPathInside(rootReal, symlinkTarget)) { + throw new Error(`Symlink escapes sandbox mount root (${rootReal}): ${current}`); + } + current = symlinkTarget; + } catch (error) { + if (isNotFoundPathError(error)) { + return; + } + throw error; + } + } +} + +async function tryRealpath(value: string): Promise { + try { + return await fs.realpath(value); + } catch { + return path.resolve(value); + } +} diff --git a/src/agents/sandbox/validate-sandbox-security.test.ts b/src/agents/sandbox/validate-sandbox-security.test.ts index 03992bd99..fae66cc79 100644 --- a/src/agents/sandbox/validate-sandbox-security.test.ts +++ b/src/agents/sandbox/validate-sandbox-security.test.ts @@ -123,6 +123,48 @@ describe("validateBindMounts", () => { expectBindMountsToThrow([source], /non-absolute/, source); } }); + + it("blocks bind sources outside allowed roots when allowlist is configured", () => { + expect(() => + validateBindMounts(["/opt/external:/data:ro"], { + allowedSourceRoots: ["/home/user/project"], + }), + ).toThrow(/outside allowed roots/); + }); + + it("allows bind sources in allowed roots when allowlist is configured", () => { + expect(() => + validateBindMounts(["/home/user/project/cache:/data:ro"], { + allowedSourceRoots: ["/home/user/project"], + }), + ).not.toThrow(); + }); + + it("allows bind sources outside allowed roots with explicit dangerous override", () => { + expect(() => + validateBindMounts(["/opt/external:/data:ro"], { + allowedSourceRoots: ["/home/user/project"], + allowSourcesOutsideAllowedRoots: true, + }), + ).not.toThrow(); + }); + + it("blocks reserved container target paths by default", () => { + expect(() => + validateBindMounts([ + "/home/user/project:/workspace:rw", + "/home/user/project:/agent/cache:rw", + ]), + ).toThrow(/reserved container path/); + }); + + it("allows reserved container target paths with explicit dangerous override", () => { + expect(() => + validateBindMounts(["/home/user/project:/workspace:rw"], { + allowReservedContainerTargets: true, + }), + ).not.toThrow(); + }); }); describe("validateNetworkMode", () => { diff --git a/src/agents/sandbox/validate-sandbox-security.ts b/src/agents/sandbox/validate-sandbox-security.ts index 2ed84e9c9..a14fd50d0 100644 --- a/src/agents/sandbox/validate-sandbox-security.ts +++ b/src/agents/sandbox/validate-sandbox-security.ts @@ -7,6 +7,7 @@ import { existsSync, realpathSync } from "node:fs"; import { posix } from "node:path"; +import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js"; // Targeted denylist: host paths that should never be exposed inside sandbox containers. // Exported for reuse in security audit collectors. @@ -30,24 +31,54 @@ export const BLOCKED_HOST_PATHS = [ const BLOCKED_NETWORK_MODES = new Set(["host"]); const BLOCKED_SECCOMP_PROFILES = new Set(["unconfined"]); const BLOCKED_APPARMOR_PROFILES = new Set(["unconfined"]); +const RESERVED_CONTAINER_TARGET_PATHS = ["/workspace", SANDBOX_AGENT_WORKSPACE_MOUNT]; + +export type ValidateBindMountsOptions = { + allowedSourceRoots?: string[]; + allowSourcesOutsideAllowedRoots?: boolean; + allowReservedContainerTargets?: boolean; +}; export type BlockedBindReason = | { kind: "targets"; blockedPath: string } | { kind: "covers"; blockedPath: string } - | { kind: "non_absolute"; sourcePath: string }; + | { kind: "non_absolute"; sourcePath: string } + | { kind: "outside_allowed_roots"; sourcePath: string; allowedRoots: string[] } + | { kind: "reserved_target"; targetPath: string; reservedPath: string }; + +type ParsedBindSpec = { + source: string; + target: string; +}; + +function parseBindSpec(bind: string): ParsedBindSpec { + const trimmed = bind.trim(); + const firstColon = trimmed.indexOf(":"); + if (firstColon <= 0) { + return { source: trimmed, target: "" }; + } + const source = trimmed.slice(0, firstColon); + const rest = trimmed.slice(firstColon + 1); + const secondColon = rest.indexOf(":"); + if (secondColon === -1) { + return { source, target: rest }; + } + return { + source, + target: rest.slice(0, secondColon), + }; +} /** * Parse the host/source path from a Docker bind mount string. * Format: `source:target[:mode]` */ export function parseBindSourcePath(bind: string): string { - const trimmed = bind.trim(); - const firstColon = trimmed.indexOf(":"); - if (firstColon <= 0) { - // No colon or starts with colon — treat as source. - return trimmed; - } - return trimmed.slice(0, firstColon); + return parseBindSpec(bind).source.trim(); +} + +export function parseBindTargetPath(bind: string): string { + return parseBindSpec(bind).target.trim(); } /** @@ -103,6 +134,69 @@ function tryRealpathAbsolute(path: string): string { } } +function normalizeAllowedRoots(roots: string[] | undefined): string[] { + if (!roots?.length) { + return []; + } + const normalized = roots + .map((entry) => entry.trim()) + .filter((entry) => entry.startsWith("/")) + .map(normalizeHostPath); + const expanded = new Set(); + for (const root of normalized) { + expanded.add(root); + const real = tryRealpathAbsolute(root); + if (real !== root) { + expanded.add(real); + } + } + return [...expanded]; +} + +function isPathInsidePosix(root: string, target: string): boolean { + if (root === "/") { + return true; + } + return target === root || target.startsWith(`${root}/`); +} + +function getOutsideAllowedRootsReason( + sourceNormalized: string, + allowedRoots: string[], +): BlockedBindReason | null { + if (allowedRoots.length === 0) { + return null; + } + for (const root of allowedRoots) { + if (isPathInsidePosix(root, sourceNormalized)) { + return null; + } + } + return { + kind: "outside_allowed_roots", + sourcePath: sourceNormalized, + allowedRoots, + }; +} + +function getReservedTargetReason(bind: string): BlockedBindReason | null { + const targetRaw = parseBindTargetPath(bind); + if (!targetRaw || !targetRaw.startsWith("/")) { + return null; + } + const target = normalizeHostPath(targetRaw); + for (const reserved of RESERVED_CONTAINER_TARGET_PATHS) { + if (isPathInsidePosix(reserved, target)) { + return { + kind: "reserved_target", + targetPath: target, + reservedPath: reserved, + }; + } + } + return null; +} + function formatBindBlockedError(params: { bind: string; reason: BlockedBindReason }): Error { if (params.reason.kind === "non_absolute") { return new Error( @@ -110,6 +204,19 @@ function formatBindBlockedError(params: { bind: string; reason: BlockedBindReaso `"${params.reason.sourcePath}". Only absolute POSIX paths are supported for sandbox binds.`, ); } + if (params.reason.kind === "outside_allowed_roots") { + return new Error( + `Sandbox security: bind mount "${params.bind}" source "${params.reason.sourcePath}" is outside allowed roots ` + + `(${params.reason.allowedRoots.join(", ")}). Use a dangerous override only when you fully trust this runtime.`, + ); + } + if (params.reason.kind === "reserved_target") { + return new Error( + `Sandbox security: bind mount "${params.bind}" targets reserved container path "${params.reason.reservedPath}" ` + + `(resolved target: "${params.reason.targetPath}"). This can shadow OpenClaw sandbox mounts. ` + + "Use a dangerous override only when you fully trust this runtime.", + ); + } const verb = params.reason.kind === "covers" ? "covers" : "targets"; return new Error( `Sandbox security: bind mount "${params.bind}" ${verb} blocked path "${params.reason.blockedPath}". ` + @@ -122,11 +229,16 @@ function formatBindBlockedError(params: { bind: string; reason: BlockedBindReaso * Validate bind mounts — throws if any source path is dangerous. * Includes a symlink/realpath pass when the source path exists. */ -export function validateBindMounts(binds: string[] | undefined): void { +export function validateBindMounts( + binds: string[] | undefined, + options?: ValidateBindMountsOptions, +): void { if (!binds?.length) { return; } + const allowedRoots = normalizeAllowedRoots(options?.allowedSourceRoots); + for (const rawBind of binds) { const bind = rawBind.trim(); if (!bind) { @@ -139,15 +251,36 @@ export function validateBindMounts(binds: string[] | undefined): void { throw formatBindBlockedError({ bind, reason: blocked }); } - // Symlink escape hardening: resolve existing absolute paths and re-check. + if (!options?.allowReservedContainerTargets) { + const reservedTarget = getReservedTargetReason(bind); + if (reservedTarget) { + throw formatBindBlockedError({ bind, reason: reservedTarget }); + } + } + const sourceRaw = parseBindSourcePath(bind); const sourceNormalized = normalizeHostPath(sourceRaw); + + if (!options?.allowSourcesOutsideAllowedRoots) { + const allowedReason = getOutsideAllowedRootsReason(sourceNormalized, allowedRoots); + if (allowedReason) { + throw formatBindBlockedError({ bind, reason: allowedReason }); + } + } + + // Symlink escape hardening: resolve existing absolute paths and re-check. const sourceReal = tryRealpathAbsolute(sourceNormalized); if (sourceReal !== sourceNormalized) { const reason = getBlockedReasonForSourcePath(sourceReal); if (reason) { throw formatBindBlockedError({ bind, reason }); } + if (!options?.allowSourcesOutsideAllowedRoots) { + const allowedReason = getOutsideAllowedRootsReason(sourceReal, allowedRoots); + if (allowedReason) { + throw formatBindBlockedError({ bind, reason: allowedReason }); + } + } } } } @@ -182,13 +315,15 @@ export function validateApparmorProfile(profile: string | undefined): void { } } -export function validateSandboxSecurity(cfg: { - binds?: string[]; - network?: string; - seccompProfile?: string; - apparmorProfile?: string; -}): void { - validateBindMounts(cfg.binds); +export function validateSandboxSecurity( + cfg: { + binds?: string[]; + network?: string; + seccompProfile?: string; + apparmorProfile?: string; + } & ValidateBindMountsOptions, +): void { + validateBindMounts(cfg.binds, cfg); validateNetworkMode(cfg.network); validateSeccompProfile(cfg.seccompProfile); validateApparmorProfile(cfg.apparmorProfile); diff --git a/src/config/types.sandbox.ts b/src/config/types.sandbox.ts index dc8d3a791..0d7ecfc8a 100644 --- a/src/config/types.sandbox.ts +++ b/src/config/types.sandbox.ts @@ -42,6 +42,16 @@ export type SandboxDockerSettings = { extraHosts?: string[]; /** Additional bind mounts (host:container:mode format, e.g. ["/host/path:/container/path:rw"]). */ binds?: string[]; + /** + * Dangerous override: allow bind mounts that target reserved container paths + * like /workspace or /agent. + */ + dangerouslyAllowReservedContainerTargets?: boolean; + /** + * Dangerous override: allow bind mount sources outside runtime allowlisted roots + * (workspace + agent workspace roots). + */ + dangerouslyAllowExternalBindSources?: boolean; }; export type SandboxBrowserSettings = { diff --git a/src/config/zod-schema.agent-runtime.ts b/src/config/zod-schema.agent-runtime.ts index 0756a2716..5147ba576 100644 --- a/src/config/zod-schema.agent-runtime.ts +++ b/src/config/zod-schema.agent-runtime.ts @@ -124,6 +124,8 @@ export const SandboxDockerSchema = z dns: z.array(z.string()).optional(), extraHosts: z.array(z.string()).optional(), binds: z.array(z.string()).optional(), + dangerouslyAllowReservedContainerTargets: z.boolean().optional(), + dangerouslyAllowExternalBindSources: z.boolean().optional(), }) .strict() .superRefine((data, ctx) => { diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index 6d36341f8..e5417a0f9 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -681,6 +681,9 @@ export function collectSandboxDangerousConfigFindings(cfg: OpenClawConfig): Secu }); continue; } + if (blocked.kind !== "covers" && blocked.kind !== "targets") { + continue; + } const verb = blocked.kind === "covers" ? "covers" : "targets"; findings.push({ checkId: "sandbox.dangerous_bind_mount",