mirror of https://github.com/openclaw/openclaw.git
fix: harden remote cdp probes
This commit is contained in:
parent
53462b990d
commit
a472f988d8
|
|
@ -26,6 +26,7 @@ Docs: https://docs.openclaw.ai
|
|||
- Models/OpenRouter runtime capabilities: fetch uncatalogued OpenRouter model metadata on first use so newly added vision models keep image input instead of silently degrading to text-only, with top-level capability field fallbacks for `/api/v1/models`. (#45824) Thanks @DJjjjhao.
|
||||
- Z.AI/onboarding: add `glm-5-turbo` to the default Z.AI provider catalog so onboarding-generated configs expose the new model alongside the existing GLM defaults. (#46670) Thanks @tomsun28.
|
||||
- Zalo Personal/group gating: stop reapplying `dmPolicy.allowFrom` as a sender gate for already-allowlisted groups when `groupAllowFrom` is unset, so any member of an allowed group can trigger replies while DMs stay restricted. (#40146)
|
||||
- Browser/remote CDP: honor strict browser SSRF policy during remote CDP reachability and `/json/version` discovery checks, redact sensitive `cdpUrl` tokens from status output, and warn when remote CDP targets private/internal hosts.
|
||||
- Plugins/install precedence: keep bundled plugins ahead of auto-discovered globals by default, but let an explicitly installed plugin record win its own duplicate-id tie so installed channel plugins load from `~/.openclaw/extensions` after `openclaw plugins install`.
|
||||
- macOS/canvas actions: keep unattended local agent actions on trusted in-app canvas surfaces only, and stop exposing the deep-link fallback key to arbitrary page scripts. Thanks @vincentkoc.
|
||||
- Agents/compaction: extend the enclosing run deadline once while compaction is actively in flight, and abort the underlying SDK compaction on timeout/cancel so large-session compactions stop freezing mid-run. (#46889) Thanks @asyncjason.
|
||||
|
|
|
|||
|
|
@ -2370,6 +2370,7 @@ See [Plugins](/tools/plugin).
|
|||
- `evaluateEnabled: false` disables `act:evaluate` and `wait --fn`.
|
||||
- `ssrfPolicy.dangerouslyAllowPrivateNetwork` defaults to `true` when unset (trusted-network model).
|
||||
- Set `ssrfPolicy.dangerouslyAllowPrivateNetwork: false` for strict public-only browser navigation.
|
||||
- In strict mode, remote CDP profile endpoints (`profiles.*.cdpUrl`) are subject to the same private-network blocking during reachability/discovery checks.
|
||||
- `ssrfPolicy.allowPrivateNetwork` remains supported as a legacy alias.
|
||||
- In strict mode, use `ssrfPolicy.hostnameAllowlist` and `ssrfPolicy.allowedHostnames` for explicit exceptions.
|
||||
- Remote profiles are attach-only (start/stop/reset disabled).
|
||||
|
|
|
|||
|
|
@ -114,6 +114,7 @@ Notes:
|
|||
- `remoteCdpTimeoutMs` applies to remote (non-loopback) CDP reachability checks.
|
||||
- `remoteCdpHandshakeTimeoutMs` applies to remote CDP WebSocket reachability checks.
|
||||
- Browser navigation/open-tab is SSRF-guarded before navigation and best-effort re-checked on final `http(s)` URL after navigation.
|
||||
- In strict SSRF mode, remote CDP endpoint discovery/probes (`cdpUrl`, including `/json/version` lookups) are checked too.
|
||||
- `browser.ssrfPolicy.dangerouslyAllowPrivateNetwork` defaults to `true` (trusted-network model). Set it to `false` for strict public-only browsing.
|
||||
- `browser.ssrfPolicy.allowPrivateNetwork` remains supported as a legacy alias for compatibility.
|
||||
- `attachOnly: true` means “never launch a local browser; only attach if it is already running.”
|
||||
|
|
|
|||
|
|
@ -1,6 +1,8 @@
|
|||
import WebSocket from "ws";
|
||||
import { isLoopbackHost } from "../gateway/net.js";
|
||||
import { type SsrFPolicy, resolvePinnedHostnameWithPolicy } from "../infra/net/ssrf.js";
|
||||
import { rawDataToString } from "../infra/ws.js";
|
||||
import { redactSensitiveText } from "../logging/redact.js";
|
||||
import { getDirectAgentForCdp, withNoProxyForCdpUrl } from "./cdp-proxy-bypass.js";
|
||||
import { CDP_HTTP_REQUEST_TIMEOUT_MS, CDP_WS_HANDSHAKE_TIMEOUT_MS } from "./cdp-timeouts.js";
|
||||
import { resolveBrowserRateLimitMessage } from "./client-fetch.js";
|
||||
|
|
@ -22,6 +24,40 @@ export function isWebSocketUrl(url: string): boolean {
|
|||
}
|
||||
}
|
||||
|
||||
export async function assertCdpEndpointAllowed(
|
||||
cdpUrl: string,
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): Promise<void> {
|
||||
if (!ssrfPolicy) {
|
||||
return;
|
||||
}
|
||||
const parsed = new URL(cdpUrl);
|
||||
if (!["http:", "https:", "ws:", "wss:"].includes(parsed.protocol)) {
|
||||
throw new Error(`Invalid CDP URL protocol: ${parsed.protocol.replace(":", "")}`);
|
||||
}
|
||||
await resolvePinnedHostnameWithPolicy(parsed.hostname, {
|
||||
policy: ssrfPolicy,
|
||||
});
|
||||
}
|
||||
|
||||
export function redactCdpUrl(cdpUrl: string | null | undefined): string | null | undefined {
|
||||
if (typeof cdpUrl !== "string") {
|
||||
return cdpUrl;
|
||||
}
|
||||
const trimmed = cdpUrl.trim();
|
||||
if (!trimmed) {
|
||||
return trimmed;
|
||||
}
|
||||
try {
|
||||
const parsed = new URL(trimmed);
|
||||
parsed.username = "";
|
||||
parsed.password = "";
|
||||
return redactSensitiveText(parsed.toString().replace(/\/$/, ""));
|
||||
} catch {
|
||||
return redactSensitiveText(trimmed);
|
||||
}
|
||||
}
|
||||
|
||||
type CdpResponse = {
|
||||
id: number;
|
||||
result?: unknown;
|
||||
|
|
|
|||
|
|
@ -302,6 +302,24 @@ describe("browser chrome helpers", () => {
|
|||
await expect(isChromeReachable("http://127.0.0.1:12345", 50)).resolves.toBe(false);
|
||||
});
|
||||
|
||||
it("blocks private CDP probes when strict SSRF policy is enabled", async () => {
|
||||
const fetchSpy = vi.fn().mockRejectedValue(new Error("should not be called"));
|
||||
vi.stubGlobal("fetch", fetchSpy);
|
||||
|
||||
await expect(
|
||||
isChromeReachable("http://127.0.0.1:12345", 50, {
|
||||
dangerouslyAllowPrivateNetwork: false,
|
||||
}),
|
||||
).resolves.toBe(false);
|
||||
await expect(
|
||||
isChromeReachable("ws://127.0.0.1:19999", 50, {
|
||||
dangerouslyAllowPrivateNetwork: false,
|
||||
}),
|
||||
).resolves.toBe(false);
|
||||
|
||||
expect(fetchSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("reports cdpReady only when Browser.getVersion command succeeds", async () => {
|
||||
await withMockChromeCdpServer({
|
||||
wsPath: "/devtools/browser/health",
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { type ChildProcessWithoutNullStreams, spawn } from "node:child_process";
|
|||
import fs from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import { ensurePortAvailable } from "../infra/ports.js";
|
||||
import { rawDataToString } from "../infra/ws.js";
|
||||
import { createSubsystemLogger } from "../logging/subsystem.js";
|
||||
|
|
@ -17,7 +18,13 @@ import {
|
|||
CHROME_STOP_TIMEOUT_MS,
|
||||
CHROME_WS_READY_TIMEOUT_MS,
|
||||
} from "./cdp-timeouts.js";
|
||||
import { appendCdpPath, fetchCdpChecked, isWebSocketUrl, openCdpWebSocket } from "./cdp.helpers.js";
|
||||
import {
|
||||
appendCdpPath,
|
||||
assertCdpEndpointAllowed,
|
||||
fetchCdpChecked,
|
||||
isWebSocketUrl,
|
||||
openCdpWebSocket,
|
||||
} from "./cdp.helpers.js";
|
||||
import { normalizeCdpWsUrl } from "./cdp.js";
|
||||
import {
|
||||
type BrowserExecutable,
|
||||
|
|
@ -96,13 +103,19 @@ async function canOpenWebSocket(url: string, timeoutMs: number): Promise<boolean
|
|||
export async function isChromeReachable(
|
||||
cdpUrl: string,
|
||||
timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS,
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): Promise<boolean> {
|
||||
if (isWebSocketUrl(cdpUrl)) {
|
||||
// Direct WebSocket endpoint — probe via WS handshake.
|
||||
return await canOpenWebSocket(cdpUrl, timeoutMs);
|
||||
try {
|
||||
await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy);
|
||||
if (isWebSocketUrl(cdpUrl)) {
|
||||
// Direct WebSocket endpoint — probe via WS handshake.
|
||||
return await canOpenWebSocket(cdpUrl, timeoutMs);
|
||||
}
|
||||
const version = await fetchChromeVersion(cdpUrl, timeoutMs, ssrfPolicy);
|
||||
return Boolean(version);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
const version = await fetchChromeVersion(cdpUrl, timeoutMs);
|
||||
return Boolean(version);
|
||||
}
|
||||
|
||||
type ChromeVersion = {
|
||||
|
|
@ -114,10 +127,12 @@ type ChromeVersion = {
|
|||
async function fetchChromeVersion(
|
||||
cdpUrl: string,
|
||||
timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS,
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): Promise<ChromeVersion | null> {
|
||||
const ctrl = new AbortController();
|
||||
const t = setTimeout(ctrl.abort.bind(ctrl), timeoutMs);
|
||||
try {
|
||||
await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy);
|
||||
const versionUrl = appendCdpPath(cdpUrl, "/json/version");
|
||||
const res = await fetchCdpChecked(versionUrl, timeoutMs, { signal: ctrl.signal });
|
||||
const data = (await res.json()) as ChromeVersion;
|
||||
|
|
@ -135,12 +150,14 @@ async function fetchChromeVersion(
|
|||
export async function getChromeWebSocketUrl(
|
||||
cdpUrl: string,
|
||||
timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS,
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): Promise<string | null> {
|
||||
await assertCdpEndpointAllowed(cdpUrl, ssrfPolicy);
|
||||
if (isWebSocketUrl(cdpUrl)) {
|
||||
// Direct WebSocket endpoint — the cdpUrl is already the WebSocket URL.
|
||||
return cdpUrl;
|
||||
}
|
||||
const version = await fetchChromeVersion(cdpUrl, timeoutMs);
|
||||
const version = await fetchChromeVersion(cdpUrl, timeoutMs, ssrfPolicy);
|
||||
const wsUrl = String(version?.webSocketDebuggerUrl ?? "").trim();
|
||||
if (!wsUrl) {
|
||||
return null;
|
||||
|
|
@ -227,8 +244,9 @@ export async function isChromeCdpReady(
|
|||
cdpUrl: string,
|
||||
timeoutMs = CHROME_REACHABILITY_TIMEOUT_MS,
|
||||
handshakeTimeoutMs = CHROME_WS_READY_TIMEOUT_MS,
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): Promise<boolean> {
|
||||
const wsUrl = await getChromeWebSocketUrl(cdpUrl, timeoutMs);
|
||||
const wsUrl = await getChromeWebSocketUrl(cdpUrl, timeoutMs, ssrfPolicy).catch(() => null);
|
||||
if (!wsUrl) {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -71,7 +71,12 @@ export function createProfileAvailability({
|
|||
return true;
|
||||
}
|
||||
const { httpTimeoutMs, wsTimeoutMs } = resolveTimeouts(timeoutMs);
|
||||
return await isChromeCdpReady(profile.cdpUrl, httpTimeoutMs, wsTimeoutMs);
|
||||
return await isChromeCdpReady(
|
||||
profile.cdpUrl,
|
||||
httpTimeoutMs,
|
||||
wsTimeoutMs,
|
||||
state().resolved.ssrfPolicy,
|
||||
);
|
||||
};
|
||||
|
||||
const isHttpReachable = async (timeoutMs?: number) => {
|
||||
|
|
@ -79,7 +84,7 @@ export function createProfileAvailability({
|
|||
return await isReachable(timeoutMs);
|
||||
}
|
||||
const { httpTimeoutMs } = resolveTimeouts(timeoutMs);
|
||||
return await isChromeReachable(profile.cdpUrl, httpTimeoutMs);
|
||||
return await isChromeReachable(profile.cdpUrl, httpTimeoutMs, state().resolved.ssrfPolicy);
|
||||
};
|
||||
|
||||
const attachRunning = (running: NonNullable<ProfileRuntimeState["running"]>) => {
|
||||
|
|
|
|||
|
|
@ -187,7 +187,11 @@ export function createBrowserRouteContext(opts: ContextOptions): BrowserRouteCon
|
|||
} else {
|
||||
// Check if something is listening on the port
|
||||
try {
|
||||
const reachable = await isChromeReachable(profile.cdpUrl, 200);
|
||||
const reachable = await isChromeReachable(
|
||||
profile.cdpUrl,
|
||||
200,
|
||||
current.resolved.ssrfPolicy,
|
||||
);
|
||||
if (reachable) {
|
||||
running = true;
|
||||
const tabs = await profileCtx.listTabs().catch(() => []);
|
||||
|
|
|
|||
|
|
@ -148,4 +148,42 @@ describe("browser manage output", () => {
|
|||
expect(output).toContain("transport: chrome-mcp");
|
||||
expect(output).not.toContain("port: 0");
|
||||
});
|
||||
|
||||
it("redacts sensitive remote cdpUrl details in status output", async () => {
|
||||
mocks.callBrowserRequest.mockImplementation(async (_opts: unknown, req: { path?: string }) =>
|
||||
req.path === "/"
|
||||
? {
|
||||
enabled: true,
|
||||
profile: "remote",
|
||||
driver: "openclaw",
|
||||
transport: "cdp",
|
||||
running: true,
|
||||
cdpReady: true,
|
||||
cdpHttp: true,
|
||||
pid: null,
|
||||
cdpPort: 9222,
|
||||
cdpUrl:
|
||||
"https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890",
|
||||
chosenBrowser: null,
|
||||
userDataDir: null,
|
||||
color: "#00AA00",
|
||||
headless: false,
|
||||
noSandbox: false,
|
||||
executablePath: null,
|
||||
attachOnly: true,
|
||||
}
|
||||
: {},
|
||||
);
|
||||
|
||||
const program = createProgram();
|
||||
await program.parseAsync(["browser", "--browser-profile", "remote", "status"], {
|
||||
from: "user",
|
||||
});
|
||||
|
||||
const output = mocks.runtimeLog.mock.calls.at(-1)?.[0] as string;
|
||||
expect(output).toContain("cdpUrl: https://example.com/chrome?token=supers…7890");
|
||||
expect(output).not.toContain("alice");
|
||||
expect(output).not.toContain("supersecretpasswordvalue1234");
|
||||
expect(output).not.toContain("supersecrettokenvalue1234567890");
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import type { Command } from "commander";
|
||||
import { redactCdpUrl } from "../browser/cdp.helpers.js";
|
||||
import type {
|
||||
BrowserTransport,
|
||||
BrowserCreateProfileResult,
|
||||
|
|
@ -152,7 +153,7 @@ export function registerBrowserManageCommands(
|
|||
...(!usesChromeMcpTransport(status)
|
||||
? [
|
||||
`cdpPort: ${status.cdpPort ?? "(unset)"}`,
|
||||
`cdpUrl: ${status.cdpUrl ?? `http://127.0.0.1:${status.cdpPort}`}`,
|
||||
`cdpUrl: ${redactCdpUrl(status.cdpUrl ?? `http://127.0.0.1:${status.cdpPort}`)}`,
|
||||
]
|
||||
: []),
|
||||
`browser: ${status.chosenBrowser ?? "unknown"}`,
|
||||
|
|
|
|||
|
|
@ -109,6 +109,36 @@ describe("runBrowserProxyCommand", () => {
|
|||
);
|
||||
});
|
||||
|
||||
it("redacts sensitive cdpUrl details in timeout diagnostics", async () => {
|
||||
dispatcherMocks.dispatch
|
||||
.mockImplementationOnce(async () => {
|
||||
await new Promise(() => {});
|
||||
})
|
||||
.mockResolvedValueOnce({
|
||||
status: 200,
|
||||
body: {
|
||||
running: true,
|
||||
cdpHttp: true,
|
||||
cdpReady: false,
|
||||
cdpUrl:
|
||||
"https://alice:supersecretpasswordvalue1234@example.com/chrome?token=supersecrettokenvalue1234567890",
|
||||
},
|
||||
});
|
||||
|
||||
await expect(
|
||||
runBrowserProxyCommand(
|
||||
JSON.stringify({
|
||||
method: "GET",
|
||||
path: "/snapshot",
|
||||
profile: "remote",
|
||||
timeoutMs: 5,
|
||||
}),
|
||||
),
|
||||
).rejects.toThrow(
|
||||
/status\(running=true, cdpHttp=true, cdpReady=false, cdpUrl=https:\/\/example\.com\/chrome\?token=supers…7890\)/,
|
||||
);
|
||||
});
|
||||
|
||||
it("keeps non-timeout browser errors intact", async () => {
|
||||
dispatcherMocks.dispatch.mockResolvedValue({
|
||||
status: 500,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,5 @@
|
|||
import fsPromises from "node:fs/promises";
|
||||
import { redactCdpUrl } from "../browser/cdp.helpers.js";
|
||||
import { resolveBrowserConfig } from "../browser/config.js";
|
||||
import {
|
||||
createBrowserControlContext,
|
||||
|
|
@ -199,7 +200,7 @@ function formatBrowserProxyTimeoutMessage(params: {
|
|||
statusParts.push(`transport=${params.status.transport}`);
|
||||
}
|
||||
if (typeof params.status.cdpUrl === "string" && params.status.cdpUrl.trim()) {
|
||||
statusParts.push(`cdpUrl=${params.status.cdpUrl}`);
|
||||
statusParts.push(`cdpUrl=${redactCdpUrl(params.status.cdpUrl)}`);
|
||||
}
|
||||
parts.push(`status(${statusParts.join(", ")})`);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1378,6 +1378,32 @@ description: test skill
|
|||
expectFinding(res, "browser.remote_cdp_http", "warn");
|
||||
});
|
||||
|
||||
it("warns when remote CDP targets a private/internal host", async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
browser: {
|
||||
profiles: {
|
||||
remote: {
|
||||
cdpUrl:
|
||||
"http://169.254.169.254:9222/json/version?token=supersecrettokenvalue1234567890",
|
||||
color: "#0066CC",
|
||||
},
|
||||
},
|
||||
},
|
||||
};
|
||||
|
||||
const res = await audit(cfg);
|
||||
|
||||
expectFinding(res, "browser.remote_cdp_private_host", "warn");
|
||||
expect(res.findings).toEqual(
|
||||
expect.arrayContaining([
|
||||
expect.objectContaining({
|
||||
checkId: "browser.remote_cdp_private_host",
|
||||
detail: expect.stringContaining("token=supers…7890"),
|
||||
}),
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("warns when control UI allows insecure auth", async () => {
|
||||
const cfg: OpenClawConfig = {
|
||||
gateway: {
|
||||
|
|
|
|||
|
|
@ -2,6 +2,7 @@ import { isIP } from "node:net";
|
|||
import path from "node:path";
|
||||
import { resolveSandboxConfigForAgent } from "../agents/sandbox.js";
|
||||
import { execDockerRaw } from "../agents/sandbox/docker.js";
|
||||
import { redactCdpUrl } from "../browser/cdp.helpers.js";
|
||||
import { resolveBrowserConfig, resolveProfile } from "../browser/config.js";
|
||||
import { resolveBrowserControlAuth } from "../browser/control-auth.js";
|
||||
import { listChannelPlugins } from "../channels/plugins/index.js";
|
||||
|
|
@ -18,6 +19,7 @@ import {
|
|||
resolveMergedSafeBinProfileFixtures,
|
||||
} from "../infra/exec-safe-bin-runtime-policy.js";
|
||||
import { normalizeTrustedSafeBinDirs } from "../infra/exec-safe-bin-trust.js";
|
||||
import { isBlockedHostnameOrIp, isPrivateNetworkAllowedByPolicy } from "../infra/net/ssrf.js";
|
||||
import { collectChannelSecurityFindings } from "./audit-channel.js";
|
||||
import {
|
||||
collectAttackSurfaceSummaryFindings,
|
||||
|
|
@ -782,15 +784,31 @@ function collectBrowserControlFindings(
|
|||
} catch {
|
||||
continue;
|
||||
}
|
||||
const redactedCdpUrl = redactCdpUrl(profile.cdpUrl) ?? profile.cdpUrl;
|
||||
if (url.protocol === "http:") {
|
||||
findings.push({
|
||||
checkId: "browser.remote_cdp_http",
|
||||
severity: "warn",
|
||||
title: "Remote CDP uses HTTP",
|
||||
detail: `browser profile "${name}" uses http CDP (${profile.cdpUrl}); this is OK only if it's tailnet-only or behind an encrypted tunnel.`,
|
||||
detail: `browser profile "${name}" uses http CDP (${redactedCdpUrl}); this is OK only if it's tailnet-only or behind an encrypted tunnel.`,
|
||||
remediation: `Prefer HTTPS/TLS or a tailnet-only endpoint for remote CDP.`,
|
||||
});
|
||||
}
|
||||
if (
|
||||
isPrivateNetworkAllowedByPolicy(resolved.ssrfPolicy) &&
|
||||
isBlockedHostnameOrIp(url.hostname)
|
||||
) {
|
||||
findings.push({
|
||||
checkId: "browser.remote_cdp_private_host",
|
||||
severity: "warn",
|
||||
title: "Remote CDP targets a private/internal host",
|
||||
detail:
|
||||
`browser profile "${name}" points at a private/internal CDP host (${redactedCdpUrl}). ` +
|
||||
"This is expected for LAN/tailnet/WSL-style setups, but treat it as a trusted-network endpoint.",
|
||||
remediation:
|
||||
"Prefer a tailnet or tunnel for remote CDP. If you want strict blocking, set browser.ssrfPolicy.dangerouslyAllowPrivateNetwork=false and allow only explicit hosts.",
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
return findings;
|
||||
|
|
|
|||
Loading…
Reference in New Issue