diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f2c09f78f7..522ed54aa0d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Browser/sandbox: pass the resolved `browser.ssrfPolicy` into sandbox browser bridges and refresh cached bridges when the effective policy changes, so sandboxed browser navigation honors private-network opt-ins. Fixes #45153 and #57055. Thanks @jzakirov, @zuoanCo, and @kybrcore. - Dashboard/Windows: open Control UI and OAuth URLs through the system URL handler without `cmd.exe` parsing or PATH-based `rundll32` lookup, and reject non-HTTP browser-open inputs. Fixes #71098. Thanks @Sanjays2402. - Providers/OpenAI: separate API-key and Codex sign-in onboarding groups, and avoid replaying stale OpenAI Responses reasoning blocks after a model route switch. - Providers/ElevenLabs: omit the MP3-only `Accept` header for PCM telephony synthesis, so Voice Call requests for `pcm_22050` no longer receive MP3 audio. Fixes #67340. Thanks @marcchabot. diff --git a/src/agents/sandbox.resolveSandboxContext.test.ts b/src/agents/sandbox.resolveSandboxContext.test.ts index 24ddf9236cb..64b318b3729 100644 --- a/src/agents/sandbox.resolveSandboxContext.test.ts +++ b/src/agents/sandbox.resolveSandboxContext.test.ts @@ -8,11 +8,31 @@ import { ensureSandboxWorkspaceForSession, resolveSandboxContext } from "./sandb const updateRegistryMock = vi.hoisted(() => vi.fn()); const syncSkillsToWorkspaceMock = vi.hoisted(() => vi.fn(async () => undefined)); +const ensureSandboxBrowserMock = vi.hoisted(() => vi.fn(async () => null)); +const browserControlAuthMock = vi.hoisted(() => ({ + ensureBrowserControlAuth: vi.fn(async () => ({ auth: { token: "test-browser-token" } })), + resolveBrowserControlAuth: vi.fn(() => ({ token: "test-browser-token" })), +})); +const browserProfilesMock = vi.hoisted(() => ({ + DEFAULT_BROWSER_EVALUATE_ENABLED: true, + resolveBrowserConfig: vi.fn(() => ({ + evaluateEnabled: true, + ssrfPolicy: { dangerouslyAllowPrivateNetwork: true }, + })), +})); vi.mock("./sandbox/registry.js", () => ({ updateRegistry: updateRegistryMock, })); +vi.mock("./sandbox/browser.js", () => ({ + ensureSandboxBrowser: ensureSandboxBrowserMock, +})); + +vi.mock("../plugin-sdk/browser-control-auth.js", () => browserControlAuthMock); + +vi.mock("../plugin-sdk/browser-profiles.js", () => browserProfilesMock); + vi.mock("../infra/skills-remote.js", () => ({ getRemoteSkillEligibility: vi.fn(() => ({ note: "test-remote" })), })); @@ -172,6 +192,60 @@ describe("resolveSandboxContext", () => { } }, 15_000); + it("passes the resolved browser SSRF policy to sandbox browser setup", async () => { + ensureSandboxBrowserMock.mockClear(); + const restore = registerSandboxBackend("test-browser-backend", async () => ({ + id: "test-browser-backend", + runtimeId: "test-browser-runtime", + runtimeLabel: "Test Browser Runtime", + workdir: "/workspace", + capabilities: { browser: true }, + buildExecSpec: async () => ({ + argv: ["test-browser-backend", "exec"], + env: process.env, + stdinMode: "pipe-closed", + }), + runShellCommand: async () => ({ + stdout: Buffer.alloc(0), + stderr: Buffer.alloc(0), + code: 0, + }), + })); + try { + const cfg: OpenClawConfig = { + browser: { + ssrfPolicy: { dangerouslyAllowPrivateNetwork: true }, + }, + agents: { + defaults: { + sandbox: { + mode: "all", + backend: "test-browser-backend", + scope: "session", + workspaceAccess: "rw", + prune: { idleHours: 0, maxAgeDays: 0 }, + browser: { enabled: true }, + }, + }, + }, + }; + + await resolveSandboxContext({ + config: cfg, + sessionKey: "agent:worker:browser", + workspaceDir: "/tmp/openclaw-test", + }); + + expect(ensureSandboxBrowserMock).toHaveBeenCalledWith( + expect.objectContaining({ + ssrfPolicy: { dangerouslyAllowPrivateNetwork: true }, + }), + ); + } finally { + restore(); + } + }, 15_000); + it("requests skill sync for read-only sandbox workspaces", async () => { syncSkillsToWorkspaceMock.mockClear(); const bundledDir = await createSandboxFixtureDir("bundled"); diff --git a/src/agents/sandbox/browser.create.test.ts b/src/agents/sandbox/browser.create.test.ts index 0cf9d9d5881..13129139b24 100644 --- a/src/agents/sandbox/browser.create.test.ts +++ b/src/agents/sandbox/browser.create.test.ts @@ -210,6 +210,88 @@ describe("ensureSandboxBrowser create args", () => { expect(result?.noVncUrl).toBeUndefined(); }); + it("passes the browser SSRF policy to the sandbox bridge", async () => { + await ensureTestSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + ssrfPolicy: { dangerouslyAllowPrivateNetwork: true }, + }); + + expect(bridgeMocks.startBrowserBridgeServer).toHaveBeenCalledWith( + expect.objectContaining({ + resolved: expect.objectContaining({ + ssrfPolicy: { dangerouslyAllowPrivateNetwork: true }, + }), + }), + ); + }); + + it("recreates a cached bridge when the SSRF policy changes", async () => { + const existingBridge = { + server: {} as never, + port: 19000, + baseUrl: "http://127.0.0.1:19000", + state: { + resolved: { + enabled: true, + evaluateEnabled: true, + controlPort: 0, + cdpProtocol: "http", + cdpHost: "127.0.0.1", + cdpIsLoopback: true, + cdpPortRangeStart: 18800, + cdpPortRangeEnd: 18899, + remoteCdpTimeoutMs: 1500, + remoteCdpHandshakeTimeoutMs: 3000, + color: "#FF4500", + headless: false, + noSandbox: false, + attachOnly: true, + defaultProfile: "openclaw", + extraArgs: [], + tabCleanup: { + enabled: true, + idleMinutes: 120, + maxTabsPerSession: 8, + sweepMinutes: 5, + }, + profiles: { + openclaw: { + cdpPort: 49100, + color: "#FF4500", + }, + }, + ssrfPolicy: { dangerouslyAllowPrivateNetwork: true }, + }, + }, + }; + BROWSER_BRIDGES.set("session:test", { + bridge: existingBridge, + containerName: "openclaw-sbx-browser-session-test-0661d10a", + authToken: "test-bridge-token", + }); + dockerMocks.dockerContainerState.mockResolvedValue({ exists: true, running: true }); + + await ensureTestSandboxBrowser({ + scopeKey: "session:test", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + cfg: buildConfig(false), + ssrfPolicy: { allowedHostnames: ["example.com"] }, + }); + + expect(bridgeMocks.stopBrowserBridgeServer).toHaveBeenCalledWith(existingBridge.server); + expect(bridgeMocks.startBrowserBridgeServer).toHaveBeenCalledWith( + expect.objectContaining({ + resolved: expect.objectContaining({ + ssrfPolicy: { allowedHostnames: ["example.com"] }, + }), + }), + ); + }); + it("mounts the main workspace read-only when workspaceAccess is none", async () => { const cfg = buildConfig(false); cfg.workspaceAccess = "none"; diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index 2c21c28c799..4d6b851acc8 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -1,5 +1,6 @@ import crypto from "node:crypto"; import { deriveDefaultBrowserCdpPortRange } from "../../config/port-defaults.js"; +import { isSameSsrFPolicy, type SsrFPolicy } from "../../infra/net/ssrf.js"; import { startBrowserBridgeServer, stopBrowserBridgeServer, @@ -80,6 +81,7 @@ function buildSandboxBrowserResolvedConfig(params: { cdpPort: number; headless: boolean; evaluateEnabled: boolean; + ssrfPolicy?: SsrFPolicy; }): ResolvedBrowserConfig { const cdpHost = "127.0.0.1"; const cdpPortRange = deriveDefaultBrowserCdpPortRange(params.controlPort); @@ -113,6 +115,7 @@ function buildSandboxBrowserResolvedConfig(params: { color: DEFAULT_OPENCLAW_BROWSER_COLOR, }, }, + ssrfPolicy: params.ssrfPolicy, }; } @@ -153,6 +156,7 @@ export async function ensureSandboxBrowser(params: { cfg: SandboxConfig; evaluateEnabled?: boolean; bridgeAuth?: { token?: string; password?: string }; + ssrfPolicy?: SsrFPolicy; }): Promise { if (!params.cfg.browser.enabled) { return null; @@ -354,6 +358,8 @@ export async function ensureSandboxBrowser(params: { const shouldReuse = existing && existing.containerName === containerName && existingProfile?.cdpPort === mappedCdp; + const policyMatches = + !existing || isSameSsrFPolicy(existing.bridge.state.resolved.ssrfPolicy, params.ssrfPolicy); const authMatches = !existing || (existing.authToken === desiredAuthToken && existing.authPassword === desiredAuthPassword); @@ -361,13 +367,13 @@ export async function ensureSandboxBrowser(params: { await stopBrowserBridgeServer(existing.bridge.server).catch(() => undefined); BROWSER_BRIDGES.delete(params.scopeKey); } - if (existing && shouldReuse && !authMatches) { + if (existing && shouldReuse && (!policyMatches || !authMatches)) { await stopBrowserBridgeServer(existing.bridge.server).catch(() => undefined); BROWSER_BRIDGES.delete(params.scopeKey); } const bridge = (() => { - if (shouldReuse && authMatches && existing) { + if (shouldReuse && policyMatches && authMatches && existing) { return existing.bridge; } return null; @@ -403,6 +409,7 @@ export async function ensureSandboxBrowser(params: { cdpPort: mappedCdp, headless: params.cfg.browser.headless, evaluateEnabled: params.evaluateEnabled ?? DEFAULT_BROWSER_EVALUATE_ENABLED, + ssrfPolicy: params.ssrfPolicy, }), authToken: desiredAuthToken, authPassword: desiredAuthPassword, @@ -412,7 +419,7 @@ export async function ensureSandboxBrowser(params: { }; const resolvedBridge = await ensureBridge(); - if (!shouldReuse || !authMatches) { + if (!shouldReuse || !policyMatches || !authMatches) { BROWSER_BRIDGES.set(params.scopeKey, { bridge: resolvedBridge, containerName, diff --git a/src/agents/sandbox/context.ts b/src/agents/sandbox/context.ts index b0eb91792de..43a9003e3af 100644 --- a/src/agents/sandbox/context.ts +++ b/src/agents/sandbox/context.ts @@ -4,7 +4,10 @@ import { ensureBrowserControlAuth, resolveBrowserControlAuth, } from "../../plugin-sdk/browser-control-auth.js"; -import { DEFAULT_BROWSER_EVALUATE_ENABLED } from "../../plugin-sdk/browser-profiles.js"; +import { + DEFAULT_BROWSER_EVALUATE_ENABLED, + resolveBrowserConfig, +} from "../../plugin-sdk/browser-profiles.js"; import { defaultRuntime } from "../../runtime.js"; import { resolveUserPath } from "../../utils.js"; import { DEFAULT_AGENT_WORKSPACE_DIR } from "../workspace.js"; @@ -171,8 +174,11 @@ export async function resolveSandboxContext(params: { configLabelKind: backend.configLabelKind ?? "Image", }); + const resolvedBrowserConfig = resolvedCfg.browser.enabled + ? resolveBrowserConfig(params.config?.browser, params.config) + : undefined; const evaluateEnabled = - params.config?.browser?.evaluateEnabled ?? DEFAULT_BROWSER_EVALUATE_ENABLED; + resolvedBrowserConfig?.evaluateEnabled ?? DEFAULT_BROWSER_EVALUATE_ENABLED; const bridgeAuth = cfg.browser.enabled ? await (async () => { @@ -204,6 +210,7 @@ export async function resolveSandboxContext(params: { cfg: resolvedCfg, evaluateEnabled, bridgeAuth, + ssrfPolicy: resolvedBrowserConfig?.ssrfPolicy, }) : null; diff --git a/src/infra/net/ssrf.test.ts b/src/infra/net/ssrf.test.ts index e5e97ba4a3d..90cf7998c19 100644 --- a/src/infra/net/ssrf.test.ts +++ b/src/infra/net/ssrf.test.ts @@ -3,6 +3,7 @@ import { blockedIpv6MulticastLiterals } from "../../shared/net/ip-test-fixtures. import { isBlockedHostnameOrIp, isPrivateIpAddress, + isSameSsrFPolicy, ssrfPolicyFromHttpBaseUrlAllowedHostname, } from "./ssrf.js"; @@ -167,3 +168,31 @@ describe("isBlockedHostnameOrIp", () => { expect(isBlockedHostnameOrIp(value)).toBe(false); }); }); + +describe("isSameSsrFPolicy", () => { + it("compares policy fields semantically", () => { + expect( + isSameSsrFPolicy( + { + allowPrivateNetwork: true, + allowRfc2544BenchmarkRange: true, + allowedHostnames: ["b.example.com", "A.example.com"], + hostnameAllowlist: ["*.example.com", "api.example.com"], + }, + { + allowPrivateNetwork: true, + allowRfc2544BenchmarkRange: true, + allowedHostnames: ["a.example.com", "B.EXAMPLE.COM"], + hostnameAllowlist: ["api.example.com", "*.example.com"], + }, + ), + ).toBe(true); + + expect( + isSameSsrFPolicy( + { dangerouslyAllowPrivateNetwork: true }, + { dangerouslyAllowPrivateNetwork: true, allowRfc2544BenchmarkRange: true }, + ), + ).toBe(false); + }); +}); diff --git a/src/infra/net/ssrf.ts b/src/infra/net/ssrf.ts index a240adf5b02..33e5d63fb5d 100644 --- a/src/infra/net/ssrf.ts +++ b/src/infra/net/ssrf.ts @@ -44,6 +44,35 @@ export type SsrFPolicy = { hostnameAllowlist?: string[]; }; +function normalizeSsrFPolicyHostnames(values?: string[]): string[] { + if (!values || values.length === 0) { + return []; + } + return Array.from( + new Set(values.map((value) => normalizeHostname(value)).filter(Boolean)), + ).toSorted(); +} + +function normalizeSsrFPolicyForComparison(policy?: SsrFPolicy) { + if (!policy) { + return null; + } + return { + allowPrivateNetwork: policy.allowPrivateNetwork === true, + dangerouslyAllowPrivateNetwork: policy.dangerouslyAllowPrivateNetwork === true, + allowRfc2544BenchmarkRange: policy.allowRfc2544BenchmarkRange === true, + allowedHostnames: normalizeSsrFPolicyHostnames(policy.allowedHostnames), + hostnameAllowlist: [...normalizeHostnameAllowlist(policy.hostnameAllowlist)].toSorted(), + }; +} + +export function isSameSsrFPolicy(a?: SsrFPolicy, b?: SsrFPolicy): boolean { + return ( + JSON.stringify(normalizeSsrFPolicyForComparison(a)) === + JSON.stringify(normalizeSsrFPolicyForComparison(b)) + ); +} + export function ssrfPolicyFromHttpBaseUrlAllowedHostname(baseUrl: string): SsrFPolicy | undefined { const trimmed = baseUrl.trim(); if (!trimmed) {