From 75e66bbaddf58c3e75e1b7fbbf4b23dae81ac949 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 13 Feb 2026 05:16:43 +0100 Subject: [PATCH] fix(sandbox): recreate browser containers on non-bridge network (#6961) (thanks @seheepeak) --- CHANGELOG.md | 1 + .../browser.ensure-network-bridge.test.ts | 135 ++++++++++++++++++ src/agents/sandbox/browser.ts | 20 ++- src/agents/sandbox/docker.ts | 14 ++ 4 files changed, 167 insertions(+), 3 deletions(-) create mode 100644 src/agents/sandbox/browser.ensure-network-bridge.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c076183a0a..68d76f3eb8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Sandbox/Browser: force browser containers onto Docker `bridge` networking and auto-recreate existing browser containers still on non-bridge network modes. (#6961) Thanks @seheepeak. - Gateway/OpenResponses: harden URL-based `input_file`/`input_image` handling with explicit SSRF deny policy, hostname allowlists (`files.urlAllowlist` / `images.urlAllowlist`), per-request URL input caps (`maxUrlParts`), blocked-fetch audit logging, and regression coverage/docs updates. - Security: fix unauthenticated Nostr profile API remote config tampering. (#13719) Thanks @coygeek. - Security: remove bundled soul-evil hook. (#14757) Thanks @Imccccc. diff --git a/src/agents/sandbox/browser.ensure-network-bridge.test.ts b/src/agents/sandbox/browser.ensure-network-bridge.test.ts new file mode 100644 index 0000000000..b2cdb4534c --- /dev/null +++ b/src/agents/sandbox/browser.ensure-network-bridge.test.ts @@ -0,0 +1,135 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import type { SandboxConfig } from "./types.js"; + +const { + buildSandboxCreateArgs, + dockerContainerState, + execDocker, + readDockerPort, + readDockerNetworkMode, +} = vi.hoisted(() => ({ + buildSandboxCreateArgs: vi.fn(), + dockerContainerState: vi.fn(), + execDocker: vi.fn(), + readDockerPort: vi.fn(), + readDockerNetworkMode: vi.fn(), +})); + +const { startBrowserBridgeServer } = vi.hoisted(() => ({ + startBrowserBridgeServer: vi.fn(), +})); + +const { updateBrowserRegistry } = vi.hoisted(() => ({ + updateBrowserRegistry: vi.fn(), +})); + +vi.mock("./docker.js", () => ({ + buildSandboxCreateArgs, + dockerContainerState, + execDocker, + readDockerPort, + readDockerNetworkMode, +})); + +vi.mock("../../browser/bridge-server.js", () => ({ + startBrowserBridgeServer, + stopBrowserBridgeServer: vi.fn(async () => undefined), +})); + +vi.mock("./registry.js", () => ({ + updateBrowserRegistry, +})); + +vi.mock("./tool-policy.js", () => ({ + isToolAllowed: vi.fn(() => true), +})); + +import { BROWSER_BRIDGES } from "./browser-bridges.js"; +import { ensureSandboxBrowser } from "./browser.js"; + +function makeConfig(): SandboxConfig { + return { + mode: "all", + scope: "shared", + workspaceAccess: "rw", + workspaceRoot: "/tmp", + docker: { + image: "sandbox-image", + containerPrefix: "sandbox-", + workdir: "/workspace", + readOnlyRoot: true, + tmpfs: ["/tmp"], + network: "none", + capDrop: ["ALL"], + env: { LANG: "C.UTF-8" }, + }, + browser: { + enabled: true, + image: "browser-image", + containerPrefix: "openclaw-browser-", + cdpPort: 9222, + vncPort: 5900, + noVncPort: 6080, + headless: true, + enableNoVnc: false, + allowHostControl: false, + autoStart: true, + autoStartTimeoutMs: 1000, + }, + tools: { allow: ["*"], deny: [] }, + prune: { idleHours: 24, maxAgeDays: 7 }, + }; +} + +describe("ensureSandboxBrowser network mode", () => { + beforeEach(() => { + vi.clearAllMocks(); + BROWSER_BRIDGES.clear(); + buildSandboxCreateArgs.mockReturnValue(["create", "--name", "openclaw-browser-shared"]); + execDocker.mockResolvedValue({ stdout: "", stderr: "", code: 0 }); + readDockerPort.mockResolvedValue(41234); + startBrowserBridgeServer.mockResolvedValue({ + baseUrl: "http://127.0.0.1:3000", + server: {}, + state: { resolved: { profiles: {} } }, + }); + updateBrowserRegistry.mockResolvedValue(undefined); + }); + + it("recreates existing browser container when network is not bridge", async () => { + dockerContainerState.mockResolvedValue({ exists: true, running: true }); + readDockerNetworkMode.mockResolvedValue("none"); + + await ensureSandboxBrowser({ + scopeKey: "session-1", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: makeConfig(), + }); + + expect(buildSandboxCreateArgs).toHaveBeenCalledWith( + expect.objectContaining({ + cfg: expect.objectContaining({ network: "bridge" }), + }), + ); + expect(execDocker).toHaveBeenCalledWith(["stop", "openclaw-browser-shared"]); + expect(execDocker).toHaveBeenCalledWith(["rm", "openclaw-browser-shared"]); + expect(execDocker).toHaveBeenCalledWith(["start", "openclaw-browser-shared"]); + }); + + it("keeps existing bridge container and only starts when stopped", async () => { + dockerContainerState.mockResolvedValue({ exists: true, running: false }); + readDockerNetworkMode.mockResolvedValue("bridge"); + + await ensureSandboxBrowser({ + scopeKey: "session-1", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: makeConfig(), + }); + + expect(buildSandboxCreateArgs).not.toHaveBeenCalled(); + expect(execDocker).not.toHaveBeenCalledWith(["rm", "openclaw-browser-shared"]); + expect(execDocker).toHaveBeenCalledWith(["start", "openclaw-browser-shared"]); + }); +}); diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index dec93370aa..0e189f16b1 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -13,6 +13,7 @@ import { dockerContainerState, execDocker, readDockerPort, + readDockerNetworkMode, } from "./docker.js"; import { updateBrowserRegistry } from "./registry.js"; import { slugifySessionKey } from "./shared.js"; @@ -101,8 +102,21 @@ export async function ensureSandboxBrowser(params: { const slug = params.cfg.scope === "shared" ? "shared" : slugifySessionKey(params.scopeKey); const name = `${params.cfg.browser.containerPrefix}${slug}`; const containerName = name.slice(0, 63); - const state = await dockerContainerState(containerName); - if (!state.exists) { + const initialState = await dockerContainerState(containerName); + let shouldCreate = !initialState.exists; + let shouldStart = initialState.exists && !initialState.running; + if (initialState.exists) { + const networkMode = await readDockerNetworkMode(containerName); + if (networkMode !== "bridge") { + if (initialState.running) { + await execDocker(["stop", containerName]); + } + await execDocker(["rm", containerName]); + shouldCreate = true; + shouldStart = false; + } + } + if (shouldCreate) { await ensureSandboxBrowserImage(params.cfg.browser.image ?? DEFAULT_SANDBOX_BROWSER_IMAGE); const args = buildSandboxCreateArgs({ name: containerName, @@ -134,7 +148,7 @@ export async function ensureSandboxBrowser(params: { args.push(params.cfg.browser.image); await execDocker(args); await execDocker(["start", containerName]); - } else if (!state.running) { + } else if (shouldStart) { await execDocker(["start", containerName]); } diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 2392bb5367..194e88fe71 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -86,6 +86,20 @@ export async function dockerContainerState(name: string) { return { exists: true, running: result.stdout.trim() === "true" }; } +export async function readDockerNetworkMode(containerName: string): Promise { + const result = await execDocker(["inspect", "-f", "{{.HostConfig.NetworkMode}}", containerName], { + allowFailure: true, + }); + if (result.code !== 0) { + return null; + } + const mode = result.stdout.trim(); + if (!mode || mode === "") { + return null; + } + return mode; +} + function normalizeDockerLimit(value?: string | number) { if (value === undefined || value === null) { return undefined;