From 14a779ee8db90c5db82c3c087ac8e6fac3448269 Mon Sep 17 00:00:00 2001 From: Jacob Tomlinson Date: Wed, 1 Apr 2026 06:01:05 -0700 Subject: [PATCH] revert: sandbox: block sensitive external bind sources (#59016) This reverts commit 8db20c196598b12043886ddc1c3ec0fd7695b9b4. --- .../sandbox/validate-sandbox-security.test.ts | 110 +----------------- .../sandbox/validate-sandbox-security.ts | 77 +----------- 2 files changed, 5 insertions(+), 182 deletions(-) diff --git a/src/agents/sandbox/validate-sandbox-security.test.ts b/src/agents/sandbox/validate-sandbox-security.test.ts index 47924b5380b..3f06b1daa45 100644 --- a/src/agents/sandbox/validate-sandbox-security.test.ts +++ b/src/agents/sandbox/validate-sandbox-security.test.ts @@ -1,7 +1,7 @@ -import { mkdirSync, mkdtempSync, symlinkSync, unlinkSync } from "node:fs"; +import { mkdirSync, mkdtempSync, symlinkSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { describe, expect, it } from "vitest"; import { getBlockedBindReason, validateBindMounts, @@ -16,10 +16,6 @@ function expectBindMountsToThrow(binds: string[], expected: RegExp, label: strin } describe("getBlockedBindReason", () => { - afterEach(() => { - vi.unstubAllEnvs(); - }); - it("blocks common Docker socket directories", () => { expect(getBlockedBindReason("/run:/run")).toEqual(expect.objectContaining({ kind: "targets" })); expect(getBlockedBindReason("/var/run:/var/run:ro")).toEqual( @@ -30,61 +26,9 @@ describe("getBlockedBindReason", () => { it("does not block /var by default", () => { expect(getBlockedBindReason("/var:/var")).toBeNull(); }); - - it("blocks sensitive home subdirectories", () => { - vi.stubEnv("HOME", "/home/tester"); - expect(getBlockedBindReason("/home/tester/.openclaw:/mnt/state:ro")).toEqual( - expect.objectContaining({ - kind: "targets", - blockedPath: "/home/tester/.openclaw", - }), - ); - expect(getBlockedBindReason("/home/tester/.ssh:/mnt/ssh:ro")).toEqual( - expect.objectContaining({ - kind: "targets", - blockedPath: "/home/tester/.ssh", - }), - ); - }); - - it("blocks sensitive home subdirectories under OPENCLAW_HOME", () => { - vi.stubEnv("HOME", "/home/tester"); - vi.stubEnv("OPENCLAW_HOME", "/srv/openclaw-home"); - expect(getBlockedBindReason("/srv/openclaw-home/.ssh:/mnt/ssh:ro")).toEqual( - expect.objectContaining({ - kind: "targets", - blockedPath: "/srv/openclaw-home/.ssh", - }), - ); - }); - - it("still blocks OS-home sensitive paths when OPENCLAW_HOME is overridden", () => { - vi.stubEnv("HOME", "/home/tester"); - vi.stubEnv("OPENCLAW_HOME", "/srv/openclaw-home"); - expect(getBlockedBindReason("/home/tester/.aws:/mnt/aws:ro")).toEqual( - expect.objectContaining({ - kind: "targets", - blockedPath: "/home/tester/.aws", - }), - ); - }); - - it("blocks the resolved OpenClaw state directory override", () => { - vi.stubEnv("OPENCLAW_STATE_DIR", "/srv/openclaw-state"); - expect(getBlockedBindReason("/srv/openclaw-state/credentials:/mnt/state:ro")).toEqual( - expect.objectContaining({ - kind: "targets", - blockedPath: "/srv/openclaw-state", - }), - ); - }); }); describe("validateBindMounts", () => { - afterEach(() => { - vi.unstubAllEnvs(); - }); - it("allows legitimate project directory mounts", () => { expect(() => validateBindMounts([ @@ -178,56 +122,6 @@ describe("validateBindMounts", () => { expect(run).toThrow(/blocked path/); }); - it("blocks canonicalized sensitive paths derived from OPENCLAW_HOME", () => { - if (process.platform === "win32") { - return; - } - - const dir = mkdtempSync(join(tmpdir(), "openclaw-home-")); - const realHome = join(dir, "real-home"); - const linkedHome = join(dir, "linked-home"); - mkdirSync(join(realHome, ".ssh"), { recursive: true }); - symlinkSync(realHome, linkedHome); - vi.stubEnv("OPENCLAW_HOME", linkedHome); - - expect(() => validateBindMounts([`${join(realHome, ".ssh")}:/mnt/ssh:ro`])).toThrow( - /blocked path/, - ); - }); - - it("refreshes canonical blocked aliases when OPENCLAW_HOME symlinks retarget", () => { - if (process.platform === "win32") { - return; - } - - const dir = mkdtempSync(join(tmpdir(), "openclaw-home-")); - const firstHome = join(dir, "home-a"); - const secondHome = join(dir, "home-b"); - const linkedHome = join(dir, "linked-home"); - mkdirSync(join(firstHome, ".ssh"), { recursive: true }); - mkdirSync(join(secondHome, ".ssh"), { recursive: true }); - symlinkSync(firstHome, linkedHome); - vi.stubEnv("OPENCLAW_HOME", linkedHome); - - expect(() => validateBindMounts([`${join(firstHome, ".ssh")}:/mnt/ssh:ro`])).toThrow( - /blocked path/, - ); - - unlinkSync(linkedHome); - symlinkSync(secondHome, linkedHome); - - expect(() => validateBindMounts([`${join(secondHome, ".ssh")}:/mnt/ssh:ro`])).toThrow( - /blocked path/, - ); - }); - - it("blocks OS-home sensitive paths when OPENCLAW_HOME points elsewhere", () => { - vi.stubEnv("HOME", "/home/tester"); - vi.stubEnv("OPENCLAW_HOME", "/srv/openclaw-home"); - - expect(() => validateBindMounts(["/home/tester/.ssh:/mnt/ssh:ro"])).toThrow(/blocked path/); - }); - it("blocks symlink-parent escapes with non-existent leaf outside allowed roots", () => { if (process.platform === "win32") { // Windows source paths (e.g. C:\\...) are intentionally rejected as non-POSIX. diff --git a/src/agents/sandbox/validate-sandbox-security.ts b/src/agents/sandbox/validate-sandbox-security.ts index fdde0c5a479..097f883f988 100644 --- a/src/agents/sandbox/validate-sandbox-security.ts +++ b/src/agents/sandbox/validate-sandbox-security.ts @@ -5,10 +5,6 @@ * Enforced at runtime when creating sandbox containers. */ -import os from "node:os"; -import path from "node:path"; -import { resolveStateDir } from "../../config/paths.js"; -import { resolveRequiredHomeDir, resolveRequiredOsHomeDir } from "../../infra/home-dir.js"; import { splitSandboxBindSpec } from "./bind-spec.js"; import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js"; import { @@ -17,9 +13,8 @@ import { } from "./host-paths.js"; import { getBlockedNetworkModeReason } from "./network-mode.js"; -// Static portion of the targeted denylist: host paths that should never be exposed -// inside sandbox containers. The full runtime set also includes sensitive home -// subdirectories and the resolved OpenClaw state directory. +// 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", @@ -35,25 +30,8 @@ export const BLOCKED_HOST_PATHS = [ "/var/run/docker.sock", "/private/var/run/docker.sock", "/run/docker.sock", - "/var/lib/docker", - "/private/var/lib/docker", - "/var/log", - "/private/var/log", ]; -const BLOCKED_HOME_SUBPATHS = [".aws", ".config", ".kube", ".openclaw", ".ssh"] as const; -let cachedBlockedHostPaths: - | { - key: string; - paths: string[]; - } - | undefined; - -type BlockedHostPathAlias = { - lexical: string; - canonical: string; -}; - const BLOCKED_SECCOMP_PROFILES = new Set(["unconfined"]); const BLOCKED_APPARMOR_PROFILES = new Set(["unconfined"]); const RESERVED_CONTAINER_TARGET_PATHS = ["/workspace", SANDBOX_AGENT_WORKSPACE_MOUNT]; @@ -129,7 +107,7 @@ export function getBlockedReasonForSourcePath(sourceNormalized: string): Blocked if (sourceNormalized === "/") { return { kind: "covers", blockedPath: "/" }; } - for (const blocked of getBlockedHostPaths()) { + for (const blocked of BLOCKED_HOST_PATHS) { if (sourceNormalized === blocked || sourceNormalized.startsWith(blocked + "/")) { return { kind: "targets", blockedPath: blocked }; } @@ -138,55 +116,6 @@ export function getBlockedReasonForSourcePath(sourceNormalized: string): Blocked return null; } -export function getBlockedHostPaths(): string[] { - const effectiveHome = normalizeHostPath(resolveRequiredHomeDir(process.env, os.homedir)); - const osHome = normalizeHostPath(resolveRequiredOsHomeDir(process.env, os.homedir)); - const stateDir = normalizeHostPath(resolveStateDir()); - const aliases: BlockedHostPathAlias[] = []; - for (const candidate of BLOCKED_HOST_PATHS) { - aliases.push(resolveBlockedHostPathAlias(candidate)); - } - for (const home of new Set([effectiveHome, osHome])) { - if (home === "/") { - continue; - } - for (const suffix of BLOCKED_HOME_SUBPATHS) { - aliases.push(resolveBlockedHostPathAlias(path.posix.join(home, suffix))); - } - } - aliases.push(resolveBlockedHostPathAlias(stateDir)); - - const cacheKey = aliases.flatMap(({ lexical, canonical }) => [lexical, canonical]).join("\u0000"); - if (cachedBlockedHostPaths?.key === cacheKey) { - return cachedBlockedHostPaths.paths; - } - - const blocked = new Set(); - for (const alias of aliases) { - addBlockedHostPath(blocked, alias); - } - - const paths = [...blocked]; - cachedBlockedHostPaths = { key: cacheKey, paths }; - return paths; -} - -function resolveBlockedHostPathAlias(candidate: string): BlockedHostPathAlias { - const lexical = normalizeHostPath(candidate); - return { - lexical, - canonical: resolveSandboxHostPathViaExistingAncestor(lexical), - }; -} - -function addBlockedHostPath(blocked: Set, alias: BlockedHostPathAlias): void { - blocked.add(alias.lexical); - - if (alias.canonical !== alias.lexical) { - blocked.add(alias.canonical); - } -} - function normalizeAllowedRoots(roots: string[] | undefined): string[] { if (!roots?.length) { return [];