mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-06 13:00:44 +00:00
fix: honor sandbox browser SSRF policy
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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";
|
||||
|
||||
@@ -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<SandboxBrowserContext | null> {
|
||||
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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user