mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-08 02:21:25 +00:00
fix(security): harden sandbox docker config validation
This commit is contained in:
@@ -94,7 +94,7 @@ describe("buildSandboxCreateArgs", () => {
|
||||
);
|
||||
});
|
||||
|
||||
it("emits -v flags for custom binds", () => {
|
||||
it("emits -v flags for safe custom binds", () => {
|
||||
const cfg: SandboxDockerConfig = {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
@@ -103,7 +103,7 @@ describe("buildSandboxCreateArgs", () => {
|
||||
tmpfs: [],
|
||||
network: "none",
|
||||
capDrop: [],
|
||||
binds: ["/home/user/source:/source:rw", "/var/run/docker.sock:/var/run/docker.sock"],
|
||||
binds: ["/home/user/source:/source:rw", "/var/data/myapp:/data:ro"],
|
||||
};
|
||||
|
||||
const args = buildSandboxCreateArgs({
|
||||
@@ -124,7 +124,116 @@ describe("buildSandboxCreateArgs", () => {
|
||||
}
|
||||
}
|
||||
expect(vFlags).toContain("/home/user/source:/source:rw");
|
||||
expect(vFlags).toContain("/var/run/docker.sock:/var/run/docker.sock");
|
||||
expect(vFlags).toContain("/var/data/myapp:/data:ro");
|
||||
});
|
||||
|
||||
it("throws on dangerous bind mounts (Docker socket)", () => {
|
||||
const cfg: SandboxDockerConfig = {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
workdir: "/workspace",
|
||||
readOnlyRoot: false,
|
||||
tmpfs: [],
|
||||
network: "none",
|
||||
capDrop: [],
|
||||
binds: ["/var/run/docker.sock:/var/run/docker.sock"],
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
buildSandboxCreateArgs({
|
||||
name: "openclaw-sbx-dangerous",
|
||||
cfg,
|
||||
scopeKey: "main",
|
||||
createdAtMs: 1700000000000,
|
||||
}),
|
||||
).toThrow(/blocked path/);
|
||||
});
|
||||
|
||||
it("throws on dangerous bind mounts (parent path)", () => {
|
||||
const cfg: SandboxDockerConfig = {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
workdir: "/workspace",
|
||||
readOnlyRoot: false,
|
||||
tmpfs: [],
|
||||
network: "none",
|
||||
capDrop: [],
|
||||
binds: ["/run:/run"],
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
buildSandboxCreateArgs({
|
||||
name: "openclaw-sbx-dangerous-parent",
|
||||
cfg,
|
||||
scopeKey: "main",
|
||||
createdAtMs: 1700000000000,
|
||||
}),
|
||||
).toThrow(/blocked path/);
|
||||
});
|
||||
|
||||
it("throws on network host mode", () => {
|
||||
const cfg: SandboxDockerConfig = {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
workdir: "/workspace",
|
||||
readOnlyRoot: false,
|
||||
tmpfs: [],
|
||||
network: "host",
|
||||
capDrop: [],
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
buildSandboxCreateArgs({
|
||||
name: "openclaw-sbx-host",
|
||||
cfg,
|
||||
scopeKey: "main",
|
||||
createdAtMs: 1700000000000,
|
||||
}),
|
||||
).toThrow(/network mode "host" is blocked/);
|
||||
});
|
||||
|
||||
it("throws on seccomp unconfined", () => {
|
||||
const cfg: SandboxDockerConfig = {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
workdir: "/workspace",
|
||||
readOnlyRoot: false,
|
||||
tmpfs: [],
|
||||
network: "none",
|
||||
capDrop: [],
|
||||
seccompProfile: "unconfined",
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
buildSandboxCreateArgs({
|
||||
name: "openclaw-sbx-seccomp",
|
||||
cfg,
|
||||
scopeKey: "main",
|
||||
createdAtMs: 1700000000000,
|
||||
}),
|
||||
).toThrow(/seccomp profile "unconfined" is blocked/);
|
||||
});
|
||||
|
||||
it("throws on apparmor unconfined", () => {
|
||||
const cfg: SandboxDockerConfig = {
|
||||
image: "openclaw-sandbox:bookworm-slim",
|
||||
containerPrefix: "openclaw-sbx-",
|
||||
workdir: "/workspace",
|
||||
readOnlyRoot: false,
|
||||
tmpfs: [],
|
||||
network: "none",
|
||||
capDrop: [],
|
||||
apparmorProfile: "unconfined",
|
||||
};
|
||||
|
||||
expect(() =>
|
||||
buildSandboxCreateArgs({
|
||||
name: "openclaw-sbx-apparmor",
|
||||
cfg,
|
||||
scopeKey: "main",
|
||||
createdAtMs: 1700000000000,
|
||||
}),
|
||||
).toThrow(/apparmor profile "unconfined" is blocked/);
|
||||
});
|
||||
|
||||
it("omits -v flags when binds is empty or undefined", () => {
|
||||
|
||||
@@ -111,6 +111,7 @@ import { computeSandboxConfigHash } from "./config-hash.js";
|
||||
import { DEFAULT_SANDBOX_IMAGE, SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js";
|
||||
import { readRegistry, updateRegistry } from "./registry.js";
|
||||
import { resolveSandboxAgentId, resolveSandboxScopeKey, slugifySessionKey } from "./shared.js";
|
||||
import { validateSandboxSecurity } from "./validate-sandbox-security.js";
|
||||
|
||||
const HOT_CONTAINER_WINDOW_MS = 5 * 60 * 1000;
|
||||
|
||||
@@ -240,6 +241,9 @@ export function buildSandboxCreateArgs(params: {
|
||||
labels?: Record<string, string>;
|
||||
configHash?: string;
|
||||
}) {
|
||||
// Runtime security validation: blocks dangerous bind mounts, network modes, and profiles.
|
||||
validateSandboxSecurity(params.cfg);
|
||||
|
||||
const createdAtMs = params.createdAtMs ?? Date.now();
|
||||
const args = ["create", "--name", params.name];
|
||||
args.push("--label", "openclaw.sandbox=1");
|
||||
|
||||
146
src/agents/sandbox/validate-sandbox-security.test.ts
Normal file
146
src/agents/sandbox/validate-sandbox-security.test.ts
Normal file
@@ -0,0 +1,146 @@
|
||||
import { mkdtempSync, symlinkSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { join } from "node:path";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import {
|
||||
getBlockedBindReasonStringOnly,
|
||||
validateBindMounts,
|
||||
validateNetworkMode,
|
||||
validateSeccompProfile,
|
||||
validateApparmorProfile,
|
||||
validateSandboxSecurity,
|
||||
} from "./validate-sandbox-security.js";
|
||||
|
||||
describe("getBlockedBindReasonStringOnly", () => {
|
||||
it("blocks ancestor mounts that would expose the Docker socket", () => {
|
||||
expect(getBlockedBindReasonStringOnly("/run:/run")).toEqual(
|
||||
expect.objectContaining({ kind: "covers" }),
|
||||
);
|
||||
expect(getBlockedBindReasonStringOnly("/var/run:/var/run:ro")).toEqual(
|
||||
expect.objectContaining({ kind: "covers" }),
|
||||
);
|
||||
expect(getBlockedBindReasonStringOnly("/var:/var")).toEqual(
|
||||
expect.objectContaining({ kind: "covers" }),
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateBindMounts", () => {
|
||||
it("allows legitimate project directory mounts", () => {
|
||||
expect(() =>
|
||||
validateBindMounts([
|
||||
"/home/user/source:/source:rw",
|
||||
"/home/user/projects:/projects:ro",
|
||||
"/var/data/myapp:/data",
|
||||
"/opt/myapp/config:/config:ro",
|
||||
]),
|
||||
).not.toThrow();
|
||||
});
|
||||
|
||||
it("allows undefined or empty binds", () => {
|
||||
expect(() => validateBindMounts(undefined)).not.toThrow();
|
||||
expect(() => validateBindMounts([])).not.toThrow();
|
||||
});
|
||||
|
||||
it("blocks /etc mount", () => {
|
||||
expect(() => validateBindMounts(["/etc/passwd:/mnt/passwd:ro"])).toThrow(
|
||||
/blocked path "\/etc"/,
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks /proc mount", () => {
|
||||
expect(() => validateBindMounts(["/proc:/proc:ro"])).toThrow(/blocked path "\/proc"/);
|
||||
});
|
||||
|
||||
it("blocks Docker socket mounts (/var/run + /run)", () => {
|
||||
expect(() => validateBindMounts(["/var/run/docker.sock:/var/run/docker.sock"])).toThrow(
|
||||
/docker\.sock/,
|
||||
);
|
||||
expect(() => validateBindMounts(["/run/docker.sock:/run/docker.sock"])).toThrow(/docker\.sock/);
|
||||
});
|
||||
|
||||
it("blocks parent mounts that would expose the Docker socket", () => {
|
||||
expect(() => validateBindMounts(["/run:/run"])).toThrow(/blocked path/);
|
||||
expect(() => validateBindMounts(["/var/run:/var/run"])).toThrow(/blocked path/);
|
||||
expect(() => validateBindMounts(["/var:/var"])).toThrow(/blocked path/);
|
||||
});
|
||||
|
||||
it("blocks paths with .. traversal to dangerous directories", () => {
|
||||
expect(() => validateBindMounts(["/home/user/../../etc/shadow:/mnt/shadow"])).toThrow(
|
||||
/blocked path "\/etc"/,
|
||||
);
|
||||
});
|
||||
|
||||
it("blocks paths with double slashes normalizing to dangerous dirs", () => {
|
||||
expect(() => validateBindMounts(["//etc//passwd:/mnt/passwd"])).toThrow(/blocked path "\/etc"/);
|
||||
});
|
||||
|
||||
it("blocks symlink escapes into blocked directories", () => {
|
||||
const dir = mkdtempSync(join(tmpdir(), "openclaw-sbx-"));
|
||||
const link = join(dir, "etc-link");
|
||||
symlinkSync("/etc", link);
|
||||
expect(() => validateBindMounts([`${link}/passwd:/mnt/passwd:ro`])).toThrow(/blocked path/);
|
||||
});
|
||||
|
||||
it("rejects non-absolute source paths (relative or named volumes)", () => {
|
||||
expect(() => validateBindMounts(["../etc/passwd:/mnt/passwd"])).toThrow(/non-absolute/);
|
||||
expect(() => validateBindMounts(["etc/passwd:/mnt/passwd"])).toThrow(/non-absolute/);
|
||||
expect(() => validateBindMounts(["myvol:/mnt"])).toThrow(/non-absolute/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateNetworkMode", () => {
|
||||
it("allows bridge/none/custom/undefined", () => {
|
||||
expect(() => validateNetworkMode("bridge")).not.toThrow();
|
||||
expect(() => validateNetworkMode("none")).not.toThrow();
|
||||
expect(() => validateNetworkMode("my-custom-network")).not.toThrow();
|
||||
expect(() => validateNetworkMode(undefined)).not.toThrow();
|
||||
});
|
||||
|
||||
it("blocks host mode (case-insensitive)", () => {
|
||||
expect(() => validateNetworkMode("host")).toThrow(/network mode "host" is blocked/);
|
||||
expect(() => validateNetworkMode("HOST")).toThrow(/network mode "HOST" is blocked/);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateSeccompProfile", () => {
|
||||
it("allows custom profile paths/undefined", () => {
|
||||
expect(() => validateSeccompProfile("/tmp/seccomp.json")).not.toThrow();
|
||||
expect(() => validateSeccompProfile(undefined)).not.toThrow();
|
||||
});
|
||||
|
||||
it("blocks unconfined (case-insensitive)", () => {
|
||||
expect(() => validateSeccompProfile("unconfined")).toThrow(
|
||||
/seccomp profile "unconfined" is blocked/,
|
||||
);
|
||||
expect(() => validateSeccompProfile("Unconfined")).toThrow(
|
||||
/seccomp profile "Unconfined" is blocked/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateApparmorProfile", () => {
|
||||
it("allows named profile/undefined", () => {
|
||||
expect(() => validateApparmorProfile("openclaw-sandbox")).not.toThrow();
|
||||
expect(() => validateApparmorProfile(undefined)).not.toThrow();
|
||||
});
|
||||
|
||||
it("blocks unconfined (case-insensitive)", () => {
|
||||
expect(() => validateApparmorProfile("unconfined")).toThrow(
|
||||
/apparmor profile "unconfined" is blocked/,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
describe("validateSandboxSecurity", () => {
|
||||
it("passes with safe config", () => {
|
||||
expect(() =>
|
||||
validateSandboxSecurity({
|
||||
binds: ["/home/user/src:/src:rw"],
|
||||
network: "none",
|
||||
seccompProfile: "/tmp/seccomp.json",
|
||||
apparmorProfile: "openclaw-sandbox",
|
||||
}),
|
||||
).not.toThrow();
|
||||
});
|
||||
});
|
||||
208
src/agents/sandbox/validate-sandbox-security.ts
Normal file
208
src/agents/sandbox/validate-sandbox-security.ts
Normal file
@@ -0,0 +1,208 @@
|
||||
/**
|
||||
* Sandbox security validation — blocks dangerous Docker configurations.
|
||||
*
|
||||
* Threat model: local-trusted config, but protect against foot-guns and config injection.
|
||||
* Enforced at runtime when creating sandbox containers.
|
||||
*/
|
||||
|
||||
import { existsSync, realpathSync } from "node:fs";
|
||||
import { posix } from "node:path";
|
||||
|
||||
// Targeted denylist: host paths that should never be exposed inside sandbox containers.
|
||||
// Exported for reuse in security audit collectors.
|
||||
export const BLOCKED_HOST_PATHS = [
|
||||
"/etc",
|
||||
"/private/etc",
|
||||
"/proc",
|
||||
"/sys",
|
||||
"/dev",
|
||||
"/root",
|
||||
"/boot",
|
||||
"/var/run/docker.sock",
|
||||
"/private/var/run/docker.sock",
|
||||
"/run/docker.sock",
|
||||
];
|
||||
|
||||
const BLOCKED_NETWORK_MODES = new Set(["host"]);
|
||||
const BLOCKED_SECCOMP_PROFILES = new Set(["unconfined"]);
|
||||
const BLOCKED_APPARMOR_PROFILES = new Set(["unconfined"]);
|
||||
|
||||
export type BlockedBindReason =
|
||||
| { kind: "targets"; blockedPath: string }
|
||||
| { kind: "covers"; blockedPath: string }
|
||||
| { kind: "non_absolute"; sourcePath: string };
|
||||
|
||||
/**
|
||||
* 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);
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalize a POSIX path: resolve `.`, `..`, collapse `//`, strip trailing `/`.
|
||||
*/
|
||||
export function normalizeHostPath(raw: string): string {
|
||||
const trimmed = raw.trim();
|
||||
return posix.normalize(trimmed).replace(/\/+$/, "") || "/";
|
||||
}
|
||||
|
||||
/**
|
||||
* String-only blocked-path check (no filesystem I/O).
|
||||
* Blocks:
|
||||
* - binds that target blocked paths (equal or under)
|
||||
* - binds that cover blocked paths (ancestor mounts like /run or /var)
|
||||
* - non-absolute source paths (relative / volume names) because they are hard to validate safely
|
||||
*/
|
||||
export function getBlockedBindReasonStringOnly(bind: string): BlockedBindReason | null {
|
||||
const sourceRaw = parseBindSourcePath(bind);
|
||||
if (!sourceRaw.startsWith("/")) {
|
||||
return { kind: "non_absolute", sourcePath: sourceRaw };
|
||||
}
|
||||
|
||||
const normalized = normalizeHostPath(sourceRaw);
|
||||
|
||||
for (const blocked of BLOCKED_HOST_PATHS) {
|
||||
if (normalized === blocked || normalized.startsWith(blocked + "/")) {
|
||||
return { kind: "targets", blockedPath: blocked };
|
||||
}
|
||||
// Ancestor mounts: mounting /run exposes /run/docker.sock.
|
||||
if (normalized === "/") {
|
||||
return { kind: "covers", blockedPath: blocked };
|
||||
}
|
||||
if (blocked.startsWith(normalized + "/")) {
|
||||
return { kind: "covers", blockedPath: blocked };
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
function tryRealpathAbsolute(path: string): string {
|
||||
if (!path.startsWith("/")) {
|
||||
return path;
|
||||
}
|
||||
if (!existsSync(path)) {
|
||||
return path;
|
||||
}
|
||||
try {
|
||||
// Use native when available (keeps platform semantics); normalize for prefix checks.
|
||||
return normalizeHostPath(realpathSync.native(path));
|
||||
} catch {
|
||||
return path;
|
||||
}
|
||||
}
|
||||
|
||||
function formatBindBlockedError(params: { bind: string; reason: BlockedBindReason }): Error {
|
||||
if (params.reason.kind === "non_absolute") {
|
||||
return new Error(
|
||||
`Sandbox security: bind mount "${params.bind}" uses a non-absolute source path ` +
|
||||
`"${params.reason.sourcePath}". Only absolute POSIX paths are supported for sandbox binds.`,
|
||||
);
|
||||
}
|
||||
const verb = params.reason.kind === "covers" ? "covers" : "targets";
|
||||
return new Error(
|
||||
`Sandbox security: bind mount "${params.bind}" ${verb} blocked path "${params.reason.blockedPath}". ` +
|
||||
"Mounting system directories (or Docker socket paths) into sandbox containers is not allowed. " +
|
||||
"Use project-specific paths instead (e.g. /home/user/myproject).",
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* 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 {
|
||||
if (!binds?.length) {
|
||||
return;
|
||||
}
|
||||
|
||||
for (const rawBind of binds) {
|
||||
const bind = rawBind.trim();
|
||||
if (!bind) {
|
||||
continue;
|
||||
}
|
||||
|
||||
// Fast string-only check (covers .., //, ancestor/descendant logic).
|
||||
const blocked = getBlockedBindReasonStringOnly(bind);
|
||||
if (blocked) {
|
||||
throw formatBindBlockedError({ bind, reason: blocked });
|
||||
}
|
||||
|
||||
// Symlink escape hardening: resolve existing absolute paths and re-check.
|
||||
const sourceRaw = parseBindSourcePath(bind);
|
||||
const sourceNormalized = normalizeHostPath(sourceRaw);
|
||||
const sourceReal = tryRealpathAbsolute(sourceNormalized);
|
||||
if (sourceReal !== sourceNormalized) {
|
||||
for (const blockedPath of BLOCKED_HOST_PATHS) {
|
||||
if (sourceReal === blockedPath || sourceReal.startsWith(blockedPath + "/")) {
|
||||
throw formatBindBlockedError({
|
||||
bind,
|
||||
reason: { kind: "targets", blockedPath },
|
||||
});
|
||||
}
|
||||
if (sourceReal === "/") {
|
||||
throw formatBindBlockedError({
|
||||
bind,
|
||||
reason: { kind: "covers", blockedPath },
|
||||
});
|
||||
}
|
||||
if (blockedPath.startsWith(sourceReal + "/")) {
|
||||
throw formatBindBlockedError({
|
||||
bind,
|
||||
reason: { kind: "covers", blockedPath },
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
export function validateNetworkMode(network: string | undefined): void {
|
||||
if (network && BLOCKED_NETWORK_MODES.has(network.trim().toLowerCase())) {
|
||||
throw new Error(
|
||||
`Sandbox security: network mode "${network}" is blocked. ` +
|
||||
'Network "host" mode bypasses container network isolation. ' +
|
||||
'Use "bridge" or "none" instead.',
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export function validateSeccompProfile(profile: string | undefined): void {
|
||||
if (profile && BLOCKED_SECCOMP_PROFILES.has(profile.trim().toLowerCase())) {
|
||||
throw new Error(
|
||||
`Sandbox security: seccomp profile "${profile}" is blocked. ` +
|
||||
"Disabling seccomp removes syscall filtering and weakens sandbox isolation. " +
|
||||
"Use a custom seccomp profile file or omit this setting.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export function validateApparmorProfile(profile: string | undefined): void {
|
||||
if (profile && BLOCKED_APPARMOR_PROFILES.has(profile.trim().toLowerCase())) {
|
||||
throw new Error(
|
||||
`Sandbox security: apparmor profile "${profile}" is blocked. ` +
|
||||
"Disabling AppArmor removes mandatory access controls and weakens sandbox isolation. " +
|
||||
"Use a named AppArmor profile or omit this setting.",
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
export function validateSandboxSecurity(cfg: {
|
||||
binds?: string[];
|
||||
network?: string;
|
||||
seccompProfile?: string;
|
||||
apparmorProfile?: string;
|
||||
}): void {
|
||||
validateBindMounts(cfg.binds);
|
||||
validateNetworkMode(cfg.network);
|
||||
validateSeccompProfile(cfg.seccompProfile);
|
||||
validateApparmorProfile(cfg.apparmorProfile);
|
||||
}
|
||||
Reference in New Issue
Block a user