diff --git a/CHANGELOG.md b/CHANGELOG.md index d975feb0abc..f20ab8d6cf4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,7 +58,7 @@ Docs: https://docs.openclaw.ai - Security/Dependencies: bump transitive `hono` usage to `4.11.10` to incorporate timing-safe authentication comparison hardening for `basicAuth`/`bearerAuth` (`GHSA-gq3j-xvxp-8hrf`). Thanks @vincentkoc. - Security/Gateway: parse `X-Forwarded-For` with trust-preserving semantics when requests come from configured trusted proxies, preventing proxy-chain spoofing from influencing client IP classification and rate-limit identity. Thanks @AnthonyDiSanti and @vincentkoc. - Security/Sandbox: remove default `--no-sandbox` for the browser container entrypoint, add explicit opt-in via `OPENCLAW_BROWSER_NO_SANDBOX` / `CLAWDBOT_BROWSER_NO_SANDBOX`, and add security-audit checks for stale/missing sandbox browser Docker hash labels. This ships in the next npm release. Thanks @TerminalsandCoffee and @vincentkoc. -- Security/Sandbox Browser: require VNC password auth for noVNC observer sessions in the sandbox browser entrypoint, plumb per-container noVNC passwords from runtime, and emit password-bearing auto-connect observer URLs while keeping loopback-only host port publishing. This ships in the next npm release. Thanks @TerminalsandCoffee for reporting. +- Security/Sandbox Browser: require VNC password auth for noVNC observer sessions in the sandbox browser entrypoint, plumb per-container noVNC passwords from runtime, and emit short-lived noVNC observer token URLs while keeping loopback-only host port publishing. This ships in the next npm release. Thanks @TerminalsandCoffee for reporting. - Auto-reply/Tools: forward `senderIsOwner` through embedded queued/followup runner params so owner-only tools remain available for authorized senders. (#22296) thanks @hcoj. - Discord: restore model picker back navigation when a provider is missing and document the Discord picker flow. (#21458) Thanks @pejmanjohn and @thewilloftheshadow. - Memory/QMD: respect per-agent `memorySearch.enabled=false` during gateway QMD startup initialization, split multi-collection QMD searches into per-collection queries (`search`/`vsearch`/`query`) to avoid sparse-term drops, prefer collection-hinted doc resolution to avoid stale-hash collisions, retry boot updates on transient lock/timeout failures, skip `qmd embed` in BM25-only `search` mode (including `memory index --force`), and serialize embed runs globally with failure backoff to prevent CPU storms on multi-agent hosts. (#20581, #21590, #20513, #20001, #21266, #21583, #20346, #19493) Thanks @danielrevivo, @zanderkrause, @sunyan034-cmd, @tilleulenspiegel, @dae-oss, @adamlongcreativellc, @jonathanadams96, and @kiliansitel. diff --git a/docs/gateway/configuration-reference.md b/docs/gateway/configuration-reference.md index 860d7a2e33b..e470fbc6255 100644 --- a/docs/gateway/configuration-reference.md +++ b/docs/gateway/configuration-reference.md @@ -992,7 +992,7 @@ Optional **Docker sandboxing** for the embedded agent. See [Sandboxing](/gateway **`docker.binds`** mounts additional host directories; global and per-agent binds are merged. **Sandboxed browser** (`sandbox.browser.enabled`): Chromium + CDP in a container. noVNC URL injected into system prompt. Does not require `browser.enabled` in main config. -noVNC observer access uses VNC auth by default and the generated URL includes the password query parameter automatically. +noVNC observer access uses VNC auth by default and OpenClaw emits a short-lived token URL (instead of exposing the password in the shared URL). - `allowHostControl: false` (default) blocks sandboxed sessions from targeting the host browser. - `sandbox.browser.binds` mounts additional host directories into the sandbox browser container only. When set (including `[]`), it replaces `docker.binds` for the browser container. diff --git a/docs/gateway/sandboxing.md b/docs/gateway/sandboxing.md index d07b85617fd..25c5efa6793 100644 --- a/docs/gateway/sandboxing.md +++ b/docs/gateway/sandboxing.md @@ -22,7 +22,7 @@ and process access when the model does something dumb. - Optional sandboxed browser (`agents.defaults.sandbox.browser`). - By default, the sandbox browser auto-starts (ensures CDP is reachable) when the browser tool needs it. Configure via `agents.defaults.sandbox.browser.autoStart` and `agents.defaults.sandbox.browser.autoStartTimeoutMs`. - - noVNC observer access is password-protected by default; OpenClaw emits an auto-connect URL with password query parameter. + - noVNC observer access is password-protected by default; OpenClaw emits a short-lived token URL that resolves to the observer session. - `agents.defaults.sandbox.browser.allowHostControl` lets sandboxed sessions target the host browser explicitly. - Optional allowlists gate `target: "custom"`: `allowedControlUrls`, `allowedControlHosts`, `allowedControlPorts`. diff --git a/docs/install/docker.md b/docs/install/docker.md index 37e44fd1e65..92fe1fc4751 100644 --- a/docs/install/docker.md +++ b/docs/install/docker.md @@ -495,7 +495,7 @@ Notes: - Headful (Xvfb) reduces bot blocking vs headless. - Headless can still be used by setting `agents.defaults.sandbox.browser.headless=true`. - No full desktop environment (GNOME) is needed; Xvfb provides the display. -- noVNC observer access is password-protected by default; OpenClaw provides an auto-connect URL with the password query parameter. +- noVNC observer access is password-protected by default; OpenClaw provides a short-lived observer token URL instead of sharing the raw password in the URL. Use config: diff --git a/src/agents/sandbox/browser.create.test.ts b/src/agents/sandbox/browser.create.test.ts new file mode 100644 index 00000000000..6dabb00e699 --- /dev/null +++ b/src/agents/sandbox/browser.create.test.ts @@ -0,0 +1,185 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { BROWSER_BRIDGES } from "./browser-bridges.js"; +import { ensureSandboxBrowser } from "./browser.js"; +import { resetNoVncObserverTokensForTests } from "./novnc-auth.js"; +import type { SandboxConfig } from "./types.js"; + +const dockerMocks = vi.hoisted(() => ({ + dockerContainerState: vi.fn(), + execDocker: vi.fn(), + readDockerContainerEnvVar: vi.fn(), + readDockerContainerLabel: vi.fn(), + readDockerPort: vi.fn(), +})); + +const registryMocks = vi.hoisted(() => ({ + readBrowserRegistry: vi.fn(), + updateBrowserRegistry: vi.fn(), +})); + +const bridgeMocks = vi.hoisted(() => ({ + startBrowserBridgeServer: vi.fn(), + stopBrowserBridgeServer: vi.fn(), +})); + +vi.mock("./docker.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + dockerContainerState: dockerMocks.dockerContainerState, + execDocker: dockerMocks.execDocker, + readDockerContainerEnvVar: dockerMocks.readDockerContainerEnvVar, + readDockerContainerLabel: dockerMocks.readDockerContainerLabel, + readDockerPort: dockerMocks.readDockerPort, + }; +}); + +vi.mock("./registry.js", () => ({ + readBrowserRegistry: registryMocks.readBrowserRegistry, + updateBrowserRegistry: registryMocks.updateBrowserRegistry, +})); + +vi.mock("../../browser/bridge-server.js", () => ({ + startBrowserBridgeServer: bridgeMocks.startBrowserBridgeServer, + stopBrowserBridgeServer: bridgeMocks.stopBrowserBridgeServer, +})); + +function buildConfig(enableNoVnc: boolean): SandboxConfig { + return { + mode: "all", + scope: "session", + workspaceAccess: "none", + workspaceRoot: "/tmp/openclaw-sandboxes", + docker: { + image: "openclaw-sandbox:bookworm-slim", + containerPrefix: "openclaw-sbx-", + workdir: "/workspace", + readOnlyRoot: true, + tmpfs: ["/tmp", "/var/tmp", "/run"], + network: "none", + capDrop: ["ALL"], + env: { LANG: "C.UTF-8" }, + }, + browser: { + enabled: true, + image: "openclaw-sandbox-browser:bookworm-slim", + containerPrefix: "openclaw-sbx-browser-", + cdpPort: 9222, + vncPort: 5900, + noVncPort: 6080, + headless: false, + enableNoVnc, + allowHostControl: false, + autoStart: true, + autoStartTimeoutMs: 12_000, + }, + tools: { + allow: ["browser"], + deny: [], + }, + prune: { + idleHours: 24, + maxAgeDays: 7, + }, + }; +} + +function envEntriesFromDockerArgs(args: string[]): string[] { + const values: string[] = []; + for (let i = 0; i < args.length; i += 1) { + if (args[i] === "-e" && typeof args[i + 1] === "string") { + values.push(args[i + 1]); + } + } + return values; +} + +describe("ensureSandboxBrowser create args", () => { + beforeEach(() => { + BROWSER_BRIDGES.clear(); + resetNoVncObserverTokensForTests(); + dockerMocks.dockerContainerState.mockReset(); + dockerMocks.execDocker.mockReset(); + dockerMocks.readDockerContainerEnvVar.mockReset(); + dockerMocks.readDockerContainerLabel.mockReset(); + dockerMocks.readDockerPort.mockReset(); + registryMocks.readBrowserRegistry.mockReset(); + registryMocks.updateBrowserRegistry.mockReset(); + bridgeMocks.startBrowserBridgeServer.mockReset(); + bridgeMocks.stopBrowserBridgeServer.mockReset(); + + dockerMocks.dockerContainerState.mockResolvedValue({ exists: false, running: false }); + dockerMocks.execDocker.mockImplementation(async (args: string[]) => { + if (args[0] === "image" && args[1] === "inspect") { + return { stdout: "[]", stderr: "", code: 0 }; + } + return { stdout: "", stderr: "", code: 0 }; + }); + dockerMocks.readDockerContainerLabel.mockResolvedValue(null); + dockerMocks.readDockerContainerEnvVar.mockResolvedValue(null); + dockerMocks.readDockerPort.mockImplementation(async (_containerName: string, port: number) => { + if (port === 9222) { + return 49100; + } + if (port === 6080) { + return 49101; + } + return null; + }); + registryMocks.readBrowserRegistry.mockResolvedValue({ entries: [] }); + registryMocks.updateBrowserRegistry.mockResolvedValue(undefined); + bridgeMocks.startBrowserBridgeServer.mockResolvedValue({ + server: {} as never, + port: 19000, + baseUrl: "http://127.0.0.1:19000", + state: { + server: null, + port: 19000, + resolved: { profiles: {} }, + profiles: new Map(), + }, + }); + bridgeMocks.stopBrowserBridgeServer.mockResolvedValue(undefined); + }); + + it("publishes noVNC on loopback and injects noVNC password env", async () => { + const result = await ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(true), + }); + + const createArgs = dockerMocks.execDocker.mock.calls.find( + (call: unknown[]) => Array.isArray(call[0]) && call[0][0] === "create", + )?.[0] as string[] | undefined; + + expect(createArgs).toBeDefined(); + expect(createArgs).toContain("127.0.0.1::6080"); + const envEntries = envEntriesFromDockerArgs(createArgs ?? []); + const passwordEntry = envEntries.find((entry) => + entry.startsWith("OPENCLAW_BROWSER_NOVNC_PASSWORD="), + ); + expect(passwordEntry).toMatch(/^OPENCLAW_BROWSER_NOVNC_PASSWORD=[a-f0-9]{8}$/); + expect(result?.noVncUrl).toMatch(/^http:\/\/127\.0\.0\.1:19000\/sandbox\/novnc\?token=/); + expect(result?.noVncUrl).not.toContain("password="); + }); + + it("does not inject noVNC password env when noVNC is disabled", async () => { + const result = await ensureSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + }); + + const createArgs = dockerMocks.execDocker.mock.calls.find( + (call: unknown[]) => Array.isArray(call[0]) && call[0][0] === "create", + )?.[0] as string[] | undefined; + const envEntries = envEntriesFromDockerArgs(createArgs ?? []); + expect(envEntries.some((entry) => entry.startsWith("OPENCLAW_BROWSER_NOVNC_PASSWORD="))).toBe( + false, + ); + expect(result?.noVncUrl).toBeUndefined(); + }); +}); diff --git a/src/agents/sandbox/browser.novnc-url.test.ts b/src/agents/sandbox/browser.novnc-url.test.ts index b542f6b9496..2020af869db 100644 --- a/src/agents/sandbox/browser.novnc-url.test.ts +++ b/src/agents/sandbox/browser.novnc-url.test.ts @@ -1,16 +1,46 @@ import { describe, expect, it } from "vitest"; -import { buildNoVncObserverUrl } from "./browser.js"; +import { + buildNoVncDirectUrl, + buildNoVncObserverTokenUrl, + consumeNoVncObserverToken, + issueNoVncObserverToken, + resetNoVncObserverTokensForTests, +} from "./novnc-auth.js"; -describe("buildNoVncObserverUrl", () => { +describe("noVNC auth helpers", () => { it("builds the default observer URL without password", () => { - expect(buildNoVncObserverUrl(45678)).toBe( + expect(buildNoVncDirectUrl(45678)).toBe( "http://127.0.0.1:45678/vnc.html?autoconnect=1&resize=remote", ); }); it("adds an encoded password query parameter when provided", () => { - expect(buildNoVncObserverUrl(45678, "a+b c&d")).toBe( + expect(buildNoVncDirectUrl(45678, "a+b c&d")).toBe( "http://127.0.0.1:45678/vnc.html?autoconnect=1&resize=remote&password=a%2Bb+c%26d", ); }); + + it("issues one-time short-lived observer tokens", () => { + resetNoVncObserverTokensForTests(); + const token = issueNoVncObserverToken({ + url: "http://127.0.0.1:50123/vnc.html?autoconnect=1&resize=remote&password=abcd1234", + nowMs: 1000, + ttlMs: 100, + }); + expect(buildNoVncObserverTokenUrl("http://127.0.0.1:19999", token)).toBe( + `http://127.0.0.1:19999/sandbox/novnc?token=${token}`, + ); + expect(consumeNoVncObserverToken(token, 1050)).toContain("/vnc.html?"); + expect(consumeNoVncObserverToken(token, 1050)).toBeNull(); + }); + + it("expires observer tokens", () => { + resetNoVncObserverTokensForTests(); + const token = issueNoVncObserverToken({ + url: "http://127.0.0.1:50123/vnc.html?autoconnect=1&resize=remote&password=abcd1234", + nowMs: 1000, + ttlMs: 100, + }); + expect(consumeNoVncObserverToken(token, 1200)).toBeNull(); + }); }); diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index 125e9bbdf4a..0b0e4de0534 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -23,29 +23,21 @@ import { readDockerContainerLabel, readDockerPort, } from "./docker.js"; +import { + buildNoVncDirectUrl, + buildNoVncObserverTokenUrl, + consumeNoVncObserverToken, + generateNoVncPassword, + isNoVncEnabled, + NOVNC_PASSWORD_ENV_KEY, + issueNoVncObserverToken, +} from "./novnc-auth.js"; import { readBrowserRegistry, updateBrowserRegistry } from "./registry.js"; import { resolveSandboxAgentId, slugifySessionKey } from "./shared.js"; import { isToolAllowed } from "./tool-policy.js"; import type { SandboxBrowserContext, SandboxConfig } from "./types.js"; const HOT_BROWSER_WINDOW_MS = 5 * 60 * 1000; -const NOVNC_PASSWORD_ENV_KEY = "OPENCLAW_BROWSER_NOVNC_PASSWORD"; - -function generateNoVncPassword() { - // VNC auth uses an 8-char password max. - return crypto.randomBytes(4).toString("hex"); -} - -export function buildNoVncObserverUrl(port: number, password?: string) { - const query = new URLSearchParams({ - autoconnect: "1", - resize: "remote", - }); - if (password?.trim()) { - query.set("password", password); - } - return `http://127.0.0.1:${port}/vnc.html?${query.toString()}`; -} async function waitForSandboxCdp(params: { cdpPort: number; timeoutMs: number }): Promise { const deadline = Date.now() + Math.max(0, params.timeoutMs); @@ -158,7 +150,7 @@ export async function ensureSandboxBrowser(params: { let running = state.running; let currentHash: string | null = null; let hashMismatch = false; - const noVncEnabled = params.cfg.browser.enableNoVnc && !params.cfg.browser.headless; + const noVncEnabled = isNoVncEnabled(params.cfg.browser); let noVncPassword: string | undefined; if (hasContainer) { @@ -331,6 +323,7 @@ export async function ensureSandboxBrowser(params: { authToken: desiredAuthToken, authPassword: desiredAuthPassword, onEnsureAttachTarget, + resolveSandboxNoVncToken: consumeNoVncObserverToken, }); }; @@ -356,7 +349,13 @@ export async function ensureSandboxBrowser(params: { }); const noVncUrl = - mappedNoVnc && noVncEnabled ? buildNoVncObserverUrl(mappedNoVnc, noVncPassword) : undefined; + mappedNoVnc && noVncEnabled + ? (() => { + const directUrl = buildNoVncDirectUrl(mappedNoVnc, noVncPassword); + const token = issueNoVncObserverToken({ url: directUrl }); + return buildNoVncObserverTokenUrl(resolvedBridge.baseUrl, token); + })() + : undefined; return { bridgeUrl: resolvedBridge.baseUrl, diff --git a/src/agents/sandbox/novnc-auth.ts b/src/agents/sandbox/novnc-auth.ts new file mode 100644 index 00000000000..b176479c111 --- /dev/null +++ b/src/agents/sandbox/novnc-auth.ts @@ -0,0 +1,81 @@ +import crypto from "node:crypto"; + +export const NOVNC_PASSWORD_ENV_KEY = "OPENCLAW_BROWSER_NOVNC_PASSWORD"; +const NOVNC_TOKEN_TTL_MS = 5 * 60 * 1000; + +type NoVncObserverTokenEntry = { + url: string; + expiresAt: number; +}; + +const NO_VNC_OBSERVER_TOKENS = new Map(); + +function pruneExpiredNoVncObserverTokens(now: number) { + for (const [token, entry] of NO_VNC_OBSERVER_TOKENS) { + if (entry.expiresAt <= now) { + NO_VNC_OBSERVER_TOKENS.delete(token); + } + } +} + +export function isNoVncEnabled(params: { enableNoVnc: boolean; headless: boolean }) { + return params.enableNoVnc && !params.headless; +} + +export function generateNoVncPassword() { + // VNC auth uses an 8-char password max. + return crypto.randomBytes(4).toString("hex"); +} + +export function buildNoVncDirectUrl(port: number, password?: string) { + const query = new URLSearchParams({ + autoconnect: "1", + resize: "remote", + }); + if (password?.trim()) { + query.set("password", password); + } + return `http://127.0.0.1:${port}/vnc.html?${query.toString()}`; +} + +export function issueNoVncObserverToken(params: { + url: string; + ttlMs?: number; + nowMs?: number; +}): string { + const now = params.nowMs ?? Date.now(); + pruneExpiredNoVncObserverTokens(now); + const token = crypto.randomBytes(24).toString("hex"); + NO_VNC_OBSERVER_TOKENS.set(token, { + url: params.url, + expiresAt: now + Math.max(1, params.ttlMs ?? NOVNC_TOKEN_TTL_MS), + }); + return token; +} + +export function consumeNoVncObserverToken(token: string, nowMs?: number): string | null { + const now = nowMs ?? Date.now(); + pruneExpiredNoVncObserverTokens(now); + const normalized = token.trim(); + if (!normalized) { + return null; + } + const entry = NO_VNC_OBSERVER_TOKENS.get(normalized); + if (!entry) { + return null; + } + NO_VNC_OBSERVER_TOKENS.delete(normalized); + if (entry.expiresAt <= now) { + return null; + } + return entry.url; +} + +export function buildNoVncObserverTokenUrl(baseUrl: string, token: string) { + const query = new URLSearchParams({ token }); + return `${baseUrl}/sandbox/novnc?${query.toString()}`; +} + +export function resetNoVncObserverTokensForTests() { + NO_VNC_OBSERVER_TOKENS.clear(); +} diff --git a/src/browser/bridge-server.ts b/src/browser/bridge-server.ts index d98f878d713..1640f9642f1 100644 --- a/src/browser/bridge-server.ts +++ b/src/browser/bridge-server.ts @@ -30,6 +30,7 @@ export async function startBrowserBridgeServer(params: { authToken?: string; authPassword?: string; onEnsureAttachTarget?: (profile: ProfileContext["profile"]) => Promise; + resolveSandboxNoVncToken?: (token: string) => string | null; }): Promise { const host = params.host ?? "127.0.0.1"; if (!isLoopbackHost(host)) { @@ -40,6 +41,23 @@ export async function startBrowserBridgeServer(params: { const app = express(); installBrowserCommonMiddleware(app); + if (params.resolveSandboxNoVncToken) { + app.get("/sandbox/novnc", (req, res) => { + const rawToken = typeof req.query?.token === "string" ? req.query.token.trim() : ""; + if (!rawToken) { + res.status(400).send("Missing token"); + return; + } + const redirectUrl = params.resolveSandboxNoVncToken?.(rawToken); + if (!redirectUrl) { + res.status(404).send("Invalid or expired token"); + return; + } + res.setHeader("Cache-Control", "no-store"); + res.redirect(302, redirectUrl); + }); + } + const authToken = params.authToken?.trim() || undefined; const authPassword = params.authPassword?.trim() || undefined; if (!authToken && !authPassword) { diff --git a/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts index ef83ab9f4e4..8fecfdd039d 100644 --- a/src/security/audit-extra.async.ts +++ b/src/security/audit-extra.async.ts @@ -306,6 +306,49 @@ async function readSandboxBrowserHashLabels(params: { } } +function parsePublishedHostFromDockerPortLine(line: string): string | null { + const trimmed = line.trim(); + const rhs = trimmed.includes("->") ? (trimmed.split("->").at(-1)?.trim() ?? "") : trimmed; + if (!rhs) { + return null; + } + const bracketHost = rhs.match(/^\[([^\]]+)\]:\d+$/); + if (bracketHost?.[1]) { + return bracketHost[1]; + } + const hostPort = rhs.match(/^([^:]+):\d+$/); + if (hostPort?.[1]) { + return hostPort[1]; + } + return null; +} + +function isLoopbackPublishHost(host: string): boolean { + const normalized = host.trim().toLowerCase(); + return normalized === "127.0.0.1" || normalized === "::1" || normalized === "localhost"; +} + +async function readSandboxBrowserPortMappings(params: { + containerName: string; + execDockerRawFn: ExecDockerRawFn; +}): Promise { + try { + const result = await params.execDockerRawFn(["port", params.containerName], { + allowFailure: true, + }); + if (result.code !== 0) { + return null; + } + return result.stdout + .toString("utf8") + .split(/\r?\n/) + .map((entry) => entry.trim()) + .filter(Boolean); + } catch { + return null; + } +} + export async function collectSandboxBrowserHashLabelFindings(params?: { execDockerRawFn?: ExecDockerRawFn; }): Promise { @@ -318,6 +361,7 @@ export async function collectSandboxBrowserHashLabelFindings(params?: { const missingHash: string[] = []; const staleEpoch: string[] = []; + const nonLoopbackPublished: string[] = []; for (const containerName of containers) { const labels = await readSandboxBrowserHashLabels({ containerName, execDockerRawFn: execFn }); @@ -330,6 +374,20 @@ export async function collectSandboxBrowserHashLabelFindings(params?: { if (labels.epoch !== SANDBOX_BROWSER_SECURITY_HASH_EPOCH) { staleEpoch.push(containerName); } + const portMappings = await readSandboxBrowserPortMappings({ + containerName, + execDockerRawFn: execFn, + }); + if (!portMappings?.length) { + continue; + } + const exposedMappings = portMappings.filter((line) => { + const host = parsePublishedHostFromDockerPortLine(line); + return Boolean(host && !isLoopbackPublishHost(host)); + }); + if (exposedMappings.length > 0) { + nonLoopbackPublished.push(`${containerName} (${exposedMappings.join("; ")})`); + } } if (missingHash.length > 0) { @@ -356,6 +414,20 @@ export async function collectSandboxBrowserHashLabelFindings(params?: { }); } + if (nonLoopbackPublished.length > 0) { + findings.push({ + checkId: "sandbox.browser_container.non_loopback_publish", + severity: "critical", + title: "Sandbox browser container publishes ports on non-loopback interfaces", + detail: + `Containers: ${nonLoopbackPublished.join(", ")}. ` + + "Sandbox browser observer/control ports should stay loopback-only to avoid unintended remote access.", + remediation: + `${formatCliCommand("openclaw sandbox recreate --browser --all")} (add --force to skip prompt), ` + + "then verify published ports are bound to 127.0.0.1.", + }); + } + return findings; } diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts index bcef4baf9ca..65f71b7edcd 100644 --- a/src/security/audit.test.ts +++ b/src/security/audit.test.ts @@ -498,6 +498,57 @@ describe("security audit", () => { expect(hasFinding(res, "sandbox.browser_container.hash_epoch_stale")).toBe(false); }); + it("flags sandbox browser containers with non-loopback published ports", async () => { + const tmp = await makeTmpDir("browser-non-loopback-publish"); + const stateDir = path.join(tmp, "state"); + await fs.mkdir(stateDir, { recursive: true, mode: 0o700 }); + const configPath = path.join(stateDir, "openclaw.json"); + await fs.writeFile(configPath, "{}\n", "utf-8"); + await fs.chmod(configPath, 0o600); + + const execDockerRawFn = (async (args: string[]) => { + if (args[0] === "ps") { + return { + stdout: Buffer.from("openclaw-sbx-browser-exposed\n"), + stderr: Buffer.alloc(0), + code: 0, + }; + } + if (args[0] === "inspect" && args.at(-1) === "openclaw-sbx-browser-exposed") { + return { + stdout: Buffer.from("hash123\t2026-02-21-novnc-auth-default\n"), + stderr: Buffer.alloc(0), + code: 0, + }; + } + if (args[0] === "port" && args.at(-1) === "openclaw-sbx-browser-exposed") { + return { + stdout: Buffer.from("6080/tcp -> 0.0.0.0:49101\n9222/tcp -> 127.0.0.1:49100\n"), + stderr: Buffer.alloc(0), + code: 0, + }; + } + return { + stdout: Buffer.alloc(0), + stderr: Buffer.from("not found"), + code: 1, + }; + }) as NonNullable; + + const res = await runSecurityAudit({ + config: {}, + includeFilesystem: true, + includeChannelSecurity: false, + stateDir, + configPath, + execDockerRawFn, + }); + + expect(hasFinding(res, "sandbox.browser_container.non_loopback_publish", "critical")).toBe( + true, + ); + }); + it("uses symlink target permissions for config checks", async () => { if (isWindows) { return;