From fbf11ebdb7110632f93926d0ac7b48f04cb44d77 Mon Sep 17 00:00:00 2001 From: Davanum Srinivas Date: Fri, 10 Apr 2026 22:59:25 +0200 Subject: [PATCH] fix(sandbox): enforce CDP source-range restriction by default (#61404) * fix(sandbox): enforce CDP source-range restriction by default Auto-derive CDP_SOURCE_RANGE from Docker network gateway IP when not explicitly configured. The entrypoint script refuses to start the socat CDP relay without a source range (fail-closed). - readDockerNetworkGateway: use Go template println, filter sentinel, prefer IPv4 gateway on dual-stack networks - Reject IPv6-only gateways for auto-derivation (relay binds IPv4) - Remove stale browser_cdp_bridge_unrestricted audit check (runtime auto-derives range for all bridge-like networks) - Bump SANDBOX_BROWSER_SECURITY_HASH_EPOCH to force container recreation * chore(changelog): add sandbox CDP source-range entry * fix(sandbox): gate CDP source-range derivation to bridge-style networks Only auto-derive OPENCLAW_BROWSER_CDP_SOURCE_RANGE from the Docker gateway IP for bridge networks (or when driver is unknown). Non-bridge drivers (macvlan, ipvlan, overlay) may route traffic from different source IPs, so they require explicit cdpSourceRange config. Adds readDockerNetworkDriver helper and a regression test for macvlan. --------- Co-authored-by: Devin Robison --- CHANGELOG.md | 2 + scripts/sandbox-browser-entrypoint.sh | 14 +-- src/agents/sandbox/browser.create.test.ts | 101 +++++++++++++++++++++ src/agents/sandbox/browser.ts | 35 ++++++- src/agents/sandbox/constants.ts | 2 +- src/agents/sandbox/docker.ts | 32 +++++++ src/security/audit-extra.sync.ts | 40 +------- src/security/audit-sandbox-browser.test.ts | 30 +----- 8 files changed, 183 insertions(+), 73 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index feb3e6adba3..476e1284b12 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -140,6 +140,8 @@ Docs: https://docs.openclaw.ai - Logging/security: redact Gmail watcher `--hook-token` values from startup logging and `logs.tail` output. (#62661) Thanks @eleqtrizit. +- Sandbox/security: auto-derive CDP source-range from Docker network gateway and refuse to start the socat relay without one, so peer containers cannot reach CDP unauthenticated. (#61404) Thanks @dims. + ## 2026.4.9 ### Changes diff --git a/scripts/sandbox-browser-entrypoint.sh b/scripts/sandbox-browser-entrypoint.sh index 057d359bb25..258bdac28ef 100755 --- a/scripts/sandbox-browser-entrypoint.sh +++ b/scripts/sandbox-browser-entrypoint.sh @@ -174,15 +174,17 @@ fi echo "[sandbox] CDP ready. Starting socat..." -SOCAT_LISTEN_ADDR="TCP-LISTEN:${CDP_PORT},fork,reuseaddr,bind=0.0.0.0" -if [[ -n "${CDP_SOURCE_RANGE}" ]]; then +if [[ -z "${CDP_SOURCE_RANGE}" ]]; then + echo "[sandbox-browser] WARNING: CDP_SOURCE_RANGE unset; socat CDP relay will not start." >&2 + echo "[sandbox-browser] Set OPENCLAW_BROWSER_CDP_SOURCE_RANGE to an explicit CIDR to enable CDP access." >&2 +else + SOCAT_LISTEN_ADDR="TCP-LISTEN:${CDP_PORT},fork,reuseaddr,bind=0.0.0.0" SOCAT_LISTEN_ADDR="${SOCAT_LISTEN_ADDR},range=${CDP_SOURCE_RANGE}" + socat "${SOCAT_LISTEN_ADDR}" "TCP:127.0.0.1:${CHROME_CDP_PORT}" & + SOCAT_PID=$! + echo "[sandbox] socat started (PID: ${SOCAT_PID})" fi -socat "${SOCAT_LISTEN_ADDR}" "TCP:127.0.0.1:${CHROME_CDP_PORT}" & -SOCAT_PID=$! -echo "[sandbox] socat started (PID: ${SOCAT_PID})" - if [[ "${ENABLE_NOVNC}" == "1" && "${HEADLESS}" != "1" ]]; then if [[ -z "${NOVNC_PASSWORD}" ]]; then NOVNC_PASSWORD="$(< /proc/sys/kernel/random/uuid)" diff --git a/src/agents/sandbox/browser.create.test.ts b/src/agents/sandbox/browser.create.test.ts index 47c15ebdd0c..ddfcff61224 100644 --- a/src/agents/sandbox/browser.create.test.ts +++ b/src/agents/sandbox/browser.create.test.ts @@ -12,6 +12,8 @@ const dockerMocks = vi.hoisted(() => ({ execDocker: vi.fn(), readDockerContainerEnvVar: vi.fn(), readDockerContainerLabel: vi.fn(), + readDockerNetworkDriver: vi.fn(), + readDockerNetworkGateway: vi.fn(), readDockerPort: vi.fn(), })); @@ -33,6 +35,8 @@ vi.mock("./docker.js", async () => { execDocker: dockerMocks.execDocker, readDockerContainerEnvVar: dockerMocks.readDockerContainerEnvVar, readDockerContainerLabel: dockerMocks.readDockerContainerLabel, + readDockerNetworkDriver: dockerMocks.readDockerNetworkDriver, + readDockerNetworkGateway: dockerMocks.readDockerNetworkGateway, readDockerPort: dockerMocks.readDockerPort, }; }); @@ -115,6 +119,8 @@ describe("ensureSandboxBrowser create args", () => { dockerMocks.execDocker.mockClear(); dockerMocks.readDockerContainerEnvVar.mockClear(); dockerMocks.readDockerContainerLabel.mockClear(); + dockerMocks.readDockerNetworkDriver.mockClear(); + dockerMocks.readDockerNetworkGateway.mockClear(); dockerMocks.readDockerPort.mockClear(); registryMocks.readBrowserRegistry.mockClear(); registryMocks.updateBrowserRegistry.mockClear(); @@ -130,6 +136,8 @@ describe("ensureSandboxBrowser create args", () => { }); dockerMocks.readDockerContainerLabel.mockResolvedValue(null); dockerMocks.readDockerContainerEnvVar.mockResolvedValue(null); + dockerMocks.readDockerNetworkDriver.mockResolvedValue("bridge"); + dockerMocks.readDockerNetworkGateway.mockResolvedValue("172.21.0.1"); dockerMocks.readDockerPort.mockImplementation(async (_containerName: string, port: number) => { if (port === 9222) { return 49100; @@ -275,4 +283,97 @@ describe("ensureSandboxBrowser create args", () => { { allowFailure: true }, ); }); + + it("auto-derives CDP source range from Docker network gateway", async () => { + dockerMocks.readDockerNetworkGateway.mockResolvedValue("172.21.0.1"); + + await ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + }); + + const createArgs = findDockerArgsCall(dockerMocks.execDocker.mock.calls, "create"); + const envEntries = collectDockerFlagValues(createArgs ?? [], "-e"); + expect(envEntries).toContain("OPENCLAW_BROWSER_CDP_SOURCE_RANGE=172.21.0.1/32"); + }); + + it("uses explicit cdpSourceRange over auto-derived gateway", async () => { + dockerMocks.readDockerNetworkGateway.mockResolvedValue("172.21.0.1"); + const cfg = buildConfig(false); + cfg.browser.cdpSourceRange = "10.0.0.0/24"; + + await ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg, + }); + + const createArgs = findDockerArgsCall(dockerMocks.execDocker.mock.calls, "create"); + const envEntries = collectDockerFlagValues(createArgs ?? [], "-e"); + expect(envEntries).toContain("OPENCLAW_BROWSER_CDP_SOURCE_RANGE=10.0.0.0/24"); + expect(dockerMocks.readDockerNetworkGateway).not.toHaveBeenCalled(); + }); + + it("rejects IPv6-only gateway (relay binds IPv4)", async () => { + dockerMocks.readDockerNetworkGateway.mockResolvedValue("fd12::1"); + + await expect( + ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + }), + ).rejects.toThrow(/Cannot derive CDP source range/); + }); + + it("throws when CDP source range cannot be derived", async () => { + dockerMocks.readDockerNetworkGateway.mockResolvedValue(null); + + await expect( + ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + }), + ).rejects.toThrow(/Cannot derive CDP source range/); + }); + + it("requires explicit cdpSourceRange for non-bridge network drivers", async () => { + dockerMocks.readDockerNetworkDriver.mockResolvedValue("macvlan"); + dockerMocks.readDockerNetworkGateway.mockResolvedValue("172.21.0.1"); + + await expect( + ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + }), + ).rejects.toThrow(/Cannot derive CDP source range/); + // Gateway helper should not have been called for non-bridge networks. + expect(dockerMocks.readDockerNetworkGateway).not.toHaveBeenCalled(); + }); + + it("uses loopback range for network=none (no IPAM gateway, no peer risk)", async () => { + dockerMocks.readDockerNetworkGateway.mockResolvedValue(null); + const cfg = buildConfig(false); + cfg.browser.network = "none"; + + const result = await ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg, + }); + + expect(result).toBeDefined(); + const createArgs = findDockerArgsCall(dockerMocks.execDocker.mock.calls, "create"); + const envEntries = collectDockerFlagValues(createArgs ?? [], "-e"); + expect(envEntries).toContain("OPENCLAW_BROWSER_CDP_SOURCE_RANGE=127.0.0.1/32"); + }); }); diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index 26a9c007472..035fa3b7d97 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -26,6 +26,8 @@ import { execDocker, readDockerContainerEnvVar, readDockerContainerLabel, + readDockerNetworkDriver, + readDockerNetworkGateway, readDockerPort, } from "./docker.js"; import { @@ -232,6 +234,35 @@ export async function ensureSandboxBrowser(params: { allowContainerNamespaceJoin: browserDockerCfg.dangerouslyAllowContainerNamespaceJoin === true, }); await ensureSandboxBrowserImage(browserImage); + // Derive effective CDP source range: explicit config > Docker network gateway > fail-closed. + // Only IPv4 gateways are usable for auto-derivation because the CDP relay + // binds on 0.0.0.0 (IPv4); an IPv6 CIDR would cause an address-family mismatch. + let effectiveCdpSourceRange = cdpSourceRange; + if (!effectiveCdpSourceRange) { + // Only auto-derive from gateway for bridge-style networks where inbound + // CDP traffic reliably comes from the Docker gateway IP. Non-bridge drivers + // (macvlan, ipvlan, overlay, etc.) may route traffic from other source IPs, + // so they require explicit cdpSourceRange config. + const driver = await readDockerNetworkDriver(browserDockerCfg.network); + const isBridgeLike = !driver || driver === "bridge"; + if (isBridgeLike) { + const gateway = await readDockerNetworkGateway(browserDockerCfg.network); + if (gateway && !gateway.includes(":")) { + effectiveCdpSourceRange = `${gateway}/32`; + } + } + } + // network="none" has no IPAM gateway by design and no peer container risk; + // use loopback range so the socat CDP relay still starts. + if (!effectiveCdpSourceRange && browserDockerCfg.network.trim().toLowerCase() === "none") { + effectiveCdpSourceRange = "127.0.0.1/32"; + } + if (!effectiveCdpSourceRange) { + throw new Error( + `Cannot derive CDP source range for sandbox browser on network "${browserDockerCfg.network}". ` + + `Set agents.defaults.sandbox.browser.cdpSourceRange explicitly.`, + ); + } const args = buildSandboxCreateArgs({ name: containerName, cfg: browserDockerCfg, @@ -267,8 +298,8 @@ export async function ensureSandboxBrowser(params: { "-e", `OPENCLAW_BROWSER_AUTO_START_TIMEOUT_MS=${params.cfg.browser.autoStartTimeoutMs}`, ); - if (cdpSourceRange) { - args.push("-e", `${CDP_SOURCE_RANGE_ENV_KEY}=${cdpSourceRange}`); + if (effectiveCdpSourceRange) { + args.push("-e", `${CDP_SOURCE_RANGE_ENV_KEY}=${effectiveCdpSourceRange}`); } args.push("-e", `OPENCLAW_BROWSER_VNC_PORT=${params.cfg.browser.vncPort}`); args.push("-e", `OPENCLAW_BROWSER_NOVNC_PORT=${params.cfg.browser.noVncPort}`); diff --git a/src/agents/sandbox/constants.ts b/src/agents/sandbox/constants.ts index 80915b3bfce..173acf28d35 100644 --- a/src/agents/sandbox/constants.ts +++ b/src/agents/sandbox/constants.ts @@ -39,7 +39,7 @@ export const DEFAULT_TOOL_DENY = [ export const DEFAULT_SANDBOX_BROWSER_IMAGE = "openclaw-sandbox-browser:bookworm-slim"; export const DEFAULT_SANDBOX_COMMON_IMAGE = "openclaw-sandbox-common:bookworm-slim"; -export const SANDBOX_BROWSER_SECURITY_HASH_EPOCH = "2026-02-28-no-sandbox-env"; +export const SANDBOX_BROWSER_SECURITY_HASH_EPOCH = "2026-04-05-cdp-source-range"; export const DEFAULT_SANDBOX_BROWSER_PREFIX = "openclaw-sbx-browser-"; export const DEFAULT_SANDBOX_BROWSER_NETWORK = "openclaw-sandbox-browser"; diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 6f90bbea94e..a1238ecf86f 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -225,6 +225,38 @@ export async function readDockerContainerEnvVar( return null; } +export async function readDockerNetworkDriver(network: string): Promise { + const result = await execDocker( + ["network", "inspect", "-f", "{{.Driver}}", network], + { allowFailure: true }, + ); + if (result.code !== 0) { + return null; + } + const driver = result.stdout.trim(); + return driver || null; +} + +export async function readDockerNetworkGateway(network: string): Promise { + const result = await execDocker( + ["network", "inspect", "-f", "{{range .IPAM.Config}}{{println .Gateway}}{{end}}", network], + { allowFailure: true }, + ); + if (result.code !== 0) { + return null; + } + // Filter valid, non-empty gateways (handles dual-stack / multi-subnet networks + // and filters Docker's "" sentinel for nil IPAM entries). + const gateways = result.stdout + .split(/\r?\n/) + .map((l) => l.trim()) + .filter((l) => l && l !== ""); + // Prefer IPv4: the CDP relay binds on 0.0.0.0 so an IPv6-only range would + // reject forwarded IPv4 traffic from the bridge gateway. + const gw = gateways.find((g) => !g.includes(":")) ?? gateways[0] ?? ""; + return gw || null; +} + export async function readDockerPort(containerName: string, port: number) { const result = await execDocker(["port", containerName, `${port}/tcp`], { allowFailure: true, diff --git a/src/security/audit-extra.sync.ts b/src/security/audit-extra.sync.ts index fa7ec82ffff..90e6cee1f85 100644 --- a/src/security/audit-extra.sync.ts +++ b/src/security/audit-extra.sync.ts @@ -845,44 +845,8 @@ export function collectSandboxDangerousConfigFindings(cfg: OpenClawConfig): Secu } } - const browserExposurePaths: string[] = []; - const defaultBrowser = resolveSandboxConfigForAgent(cfg).browser; - if ( - defaultBrowser.enabled && - normalizeOptionalLowercaseString(defaultBrowser.network) === "bridge" && - !defaultBrowser.cdpSourceRange?.trim() - ) { - browserExposurePaths.push("agents.defaults.sandbox.browser"); - } - for (const entry of agents) { - if (!entry || typeof entry !== "object" || typeof entry.id !== "string") { - continue; - } - const browser = resolveSandboxConfigForAgent(cfg, entry.id).browser; - if (!browser.enabled) { - continue; - } - if (normalizeOptionalLowercaseString(browser.network) !== "bridge") { - continue; - } - if (browser.cdpSourceRange?.trim()) { - continue; - } - browserExposurePaths.push(`agents.list.${entry.id}.sandbox.browser`); - } - if (browserExposurePaths.length > 0) { - findings.push({ - checkId: "sandbox.browser_cdp_bridge_unrestricted", - severity: "warn", - title: "Sandbox browser CDP may be reachable by peer containers", - detail: - "These sandbox browser configs use Docker bridge networking with no CDP source restriction:\n" + - browserExposurePaths.map((entry) => `- ${entry}`).join("\n"), - remediation: - "Set sandbox.browser.network to a dedicated bridge network (recommended default: openclaw-sandbox-browser), " + - "or set sandbox.browser.cdpSourceRange (for example 172.21.0.1/32) to restrict container-edge CDP ingress.", - }); - } + // CDP source range is now auto-derived at runtime from the Docker network gateway + // for all bridge-like networks, so an unset cdpSourceRange is no longer a security gap. return findings; } diff --git a/src/security/audit-sandbox-browser.test.ts b/src/security/audit-sandbox-browser.test.ts index 5771891d9e6..4cbb0c10d87 100644 --- a/src/security/audit-sandbox-browser.test.ts +++ b/src/security/audit-sandbox-browser.test.ts @@ -7,8 +7,7 @@ function hasFinding( checkId: | "sandbox.browser_container.hash_label_missing" | "sandbox.browser_container.hash_epoch_stale" - | "sandbox.browser_container.non_loopback_publish" - | "sandbox.browser_cdp_bridge_unrestricted", + | "sandbox.browser_container.non_loopback_publish", severity: "warn" | "critical", findings: Array<{ checkId: string; severity: string; detail: string }>, ) { @@ -105,7 +104,7 @@ describe("security audit sandbox browser findings", () => { ); }); - it("warns when bridge network omits cdpSourceRange", () => { + it("does not warn about cdpSourceRange since runtime auto-derives it", () => { const findings = collectSandboxDangerousConfigFindings({ agents: { defaults: { @@ -116,29 +115,8 @@ describe("security audit sandbox browser findings", () => { }, }, } satisfies OpenClawConfig); - const finding = findings.find( - (entry) => entry.checkId === "sandbox.browser_cdp_bridge_unrestricted", + expect(findings.some((f) => f.checkId === "sandbox.browser_cdp_bridge_unrestricted")).toBe( + false, ); - expect(finding?.severity).toBe("warn"); - expect(finding?.detail).toContain("agents.defaults.sandbox.browser"); - }); - - it("does not warn for dedicated default browser network", () => { - expect( - hasFinding( - "sandbox.browser_cdp_bridge_unrestricted", - "warn", - collectSandboxDangerousConfigFindings({ - agents: { - defaults: { - sandbox: { - mode: "all", - browser: { enabled: true }, - }, - }, - }, - } satisfies OpenClawConfig), - ), - ).toBe(false); }); });