mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(browser): enforce redirect-hop SSRF checks
This commit is contained in:
@@ -2,6 +2,12 @@
|
||||
|
||||
Docs: https://docs.openclaw.ai
|
||||
|
||||
## Unreleased
|
||||
|
||||
### Fixes
|
||||
|
||||
- Browser/SSRF: block private-network intermediate redirect hops in strict browser navigation flows and fail closed when remote tab-open paths cannot inspect redirect chains. Thanks @zpbrent.
|
||||
|
||||
## 2026.3.8
|
||||
|
||||
### Changes
|
||||
|
||||
@@ -2,8 +2,10 @@ import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { SsrFBlockedError, type LookupFn } from "../infra/net/ssrf.js";
|
||||
import {
|
||||
assertBrowserNavigationAllowed,
|
||||
assertBrowserNavigationRedirectChainAllowed,
|
||||
assertBrowserNavigationResultAllowed,
|
||||
InvalidBrowserNavigationUrlError,
|
||||
requiresInspectableBrowserNavigationRedirects,
|
||||
} from "./navigation-guard.js";
|
||||
|
||||
function createLookupFn(address: string): LookupFn {
|
||||
@@ -147,4 +149,58 @@ describe("browser navigation guard", () => {
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("blocks private intermediate redirect hops", async () => {
|
||||
const publicLookup = createLookupFn("93.184.216.34");
|
||||
const privateLookup = createLookupFn("127.0.0.1");
|
||||
const finalRequest = {
|
||||
url: () => "https://public.example/final",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "http://private.example/internal",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "https://public.example/start",
|
||||
redirectedFrom: () => null,
|
||||
}),
|
||||
}),
|
||||
};
|
||||
|
||||
await expect(
|
||||
assertBrowserNavigationRedirectChainAllowed({
|
||||
request: finalRequest,
|
||||
lookupFn: vi.fn(async (hostname: string) =>
|
||||
hostname === "private.example"
|
||||
? privateLookup(hostname, { all: true })
|
||||
: publicLookup(hostname, { all: true }),
|
||||
) as unknown as LookupFn,
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SsrFBlockedError);
|
||||
});
|
||||
|
||||
it("allows redirect chains when every hop is public", async () => {
|
||||
const lookupFn = createLookupFn("93.184.216.34");
|
||||
const finalRequest = {
|
||||
url: () => "https://public.example/final",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "https://public.example/middle",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "https://public.example/start",
|
||||
redirectedFrom: () => null,
|
||||
}),
|
||||
}),
|
||||
};
|
||||
|
||||
await expect(
|
||||
assertBrowserNavigationRedirectChainAllowed({
|
||||
request: finalRequest,
|
||||
lookupFn,
|
||||
}),
|
||||
).resolves.toBeUndefined();
|
||||
});
|
||||
|
||||
it("treats default browser SSRF mode as requiring redirect-hop inspection", () => {
|
||||
expect(requiresInspectableBrowserNavigationRedirects()).toBe(true);
|
||||
expect(requiresInspectableBrowserNavigationRedirects({ allowPrivateNetwork: true })).toBe(
|
||||
false,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -25,12 +25,21 @@ export type BrowserNavigationPolicyOptions = {
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
};
|
||||
|
||||
export type BrowserNavigationRequestLike = {
|
||||
url(): string;
|
||||
redirectedFrom(): BrowserNavigationRequestLike | null;
|
||||
};
|
||||
|
||||
export function withBrowserNavigationPolicy(
|
||||
ssrfPolicy?: SsrFPolicy,
|
||||
): BrowserNavigationPolicyOptions {
|
||||
return ssrfPolicy ? { ssrfPolicy } : {};
|
||||
}
|
||||
|
||||
export function requiresInspectableBrowserNavigationRedirects(ssrfPolicy?: SsrFPolicy): boolean {
|
||||
return !isPrivateNetworkAllowedByPolicy(ssrfPolicy);
|
||||
}
|
||||
|
||||
export async function assertBrowserNavigationAllowed(
|
||||
opts: {
|
||||
url: string;
|
||||
@@ -102,3 +111,24 @@ export async function assertBrowserNavigationResultAllowed(
|
||||
await assertBrowserNavigationAllowed(opts);
|
||||
}
|
||||
}
|
||||
|
||||
export async function assertBrowserNavigationRedirectChainAllowed(
|
||||
opts: {
|
||||
request?: BrowserNavigationRequestLike | null;
|
||||
lookupFn?: LookupFn;
|
||||
} & BrowserNavigationPolicyOptions,
|
||||
): Promise<void> {
|
||||
const chain: string[] = [];
|
||||
let current = opts.request ?? null;
|
||||
while (current) {
|
||||
chain.push(current.url());
|
||||
current = current.redirectedFrom();
|
||||
}
|
||||
for (const url of chain.toReversed()) {
|
||||
await assertBrowserNavigationAllowed({
|
||||
url,
|
||||
lookupFn: opts.lookupFn,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { chromium } from "playwright-core";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import { SsrFBlockedError } from "../infra/net/ssrf.js";
|
||||
import * as chromeModule from "./chrome.js";
|
||||
import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
|
||||
import { closePlaywrightBrowserConnection, createPageViaPlaywright } from "./pw-session.js";
|
||||
@@ -9,7 +10,9 @@ const getChromeWebSocketUrlSpy = vi.spyOn(chromeModule, "getChromeWebSocketUrl")
|
||||
|
||||
function installBrowserMocks() {
|
||||
const pageOn = vi.fn();
|
||||
const pageGoto = vi.fn(async () => {});
|
||||
const pageGoto = vi.fn<
|
||||
(...args: unknown[]) => Promise<null | { request: () => Record<string, unknown> }>
|
||||
>(async () => null);
|
||||
const pageTitle = vi.fn(async () => "");
|
||||
const pageUrl = vi.fn(() => "about:blank");
|
||||
const contextOn = vi.fn();
|
||||
@@ -84,4 +87,27 @@ describe("pw-session createPageViaPlaywright navigation guard", () => {
|
||||
expect(created.targetId).toBe("TARGET_1");
|
||||
expect(pageGoto).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("blocks private intermediate redirect hops", async () => {
|
||||
const { pageGoto } = installBrowserMocks();
|
||||
pageGoto.mockResolvedValueOnce({
|
||||
request: () => ({
|
||||
url: () => "https://93.184.216.34/final",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "http://127.0.0.1:18080/internal-hop",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "https://93.184.216.34/start",
|
||||
redirectedFrom: () => null,
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(
|
||||
createPageViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
url: "https://93.184.216.34/start",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SsrFBlockedError);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -22,6 +22,7 @@ import { getChromeWebSocketUrl } from "./chrome.js";
|
||||
import { BrowserTabNotFoundError } from "./errors.js";
|
||||
import {
|
||||
assertBrowserNavigationAllowed,
|
||||
assertBrowserNavigationRedirectChainAllowed,
|
||||
assertBrowserNavigationResultAllowed,
|
||||
withBrowserNavigationPolicy,
|
||||
} from "./navigation-guard.js";
|
||||
@@ -787,8 +788,13 @@ export async function createPageViaPlaywright(opts: {
|
||||
url: targetUrl,
|
||||
...navigationPolicy,
|
||||
});
|
||||
await page.goto(targetUrl, { timeout: 30_000 }).catch(() => {
|
||||
const response = await page.goto(targetUrl, { timeout: 30_000 }).catch(() => {
|
||||
// Navigation might fail for some URLs, but page is still created
|
||||
return null;
|
||||
});
|
||||
await assertBrowserNavigationRedirectChainAllowed({
|
||||
request: response?.request(),
|
||||
...navigationPolicy,
|
||||
});
|
||||
await assertBrowserNavigationResultAllowed({
|
||||
url: page.url(),
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { SsrFBlockedError } from "../infra/net/ssrf.js";
|
||||
import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
|
||||
import {
|
||||
getPwToolsCoreSessionMocks,
|
||||
@@ -75,4 +76,32 @@ describe("pw-tools-core.snapshot navigate guard", () => {
|
||||
expect(goto).toHaveBeenCalledTimes(2);
|
||||
expect(result.url).toBe("https://example.com/recovered");
|
||||
});
|
||||
|
||||
it("blocks private intermediate redirect hops during navigation", async () => {
|
||||
const goto = vi.fn(async () => ({
|
||||
request: () => ({
|
||||
url: () => "https://93.184.216.34/final",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "http://127.0.0.1:18080/internal-hop",
|
||||
redirectedFrom: () => ({
|
||||
url: () => "https://93.184.216.34/start",
|
||||
redirectedFrom: () => null,
|
||||
}),
|
||||
}),
|
||||
}),
|
||||
}));
|
||||
setPwToolsCoreCurrentPage({
|
||||
goto,
|
||||
url: vi.fn(() => "https://93.184.216.34/final"),
|
||||
});
|
||||
|
||||
await expect(
|
||||
mod.navigateViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
url: "https://93.184.216.34/start",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SsrFBlockedError);
|
||||
|
||||
expect(goto).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import { type AriaSnapshotNode, formatAriaSnapshot, type RawAXNode } from "./cdp.js";
|
||||
import {
|
||||
assertBrowserNavigationAllowed,
|
||||
assertBrowserNavigationRedirectChainAllowed,
|
||||
assertBrowserNavigationResultAllowed,
|
||||
withBrowserNavigationPolicy,
|
||||
} from "./navigation-guard.js";
|
||||
@@ -196,8 +197,10 @@ export async function navigateViaPlaywright(opts: {
|
||||
const timeout = Math.max(1000, Math.min(120_000, opts.timeoutMs ?? 20_000));
|
||||
let page = await getPageForTargetId(opts);
|
||||
ensurePageState(page);
|
||||
const navigate = async () => await page.goto(url, { timeout });
|
||||
let response;
|
||||
try {
|
||||
await page.goto(url, { timeout });
|
||||
response = await navigate();
|
||||
} catch (err) {
|
||||
if (!isRetryableNavigateError(err)) {
|
||||
throw err;
|
||||
@@ -211,8 +214,12 @@ export async function navigateViaPlaywright(opts: {
|
||||
}).catch(() => {});
|
||||
page = await getPageForTargetId(opts);
|
||||
ensurePageState(page);
|
||||
await page.goto(url, { timeout });
|
||||
response = await navigate();
|
||||
}
|
||||
await assertBrowserNavigationRedirectChainAllowed({
|
||||
request: response?.request(),
|
||||
...withBrowserNavigationPolicy(opts.ssrfPolicy),
|
||||
});
|
||||
const finalUrl = page.url();
|
||||
await assertBrowserNavigationResultAllowed({
|
||||
url: finalUrl,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import "./server-context.chrome-test-harness.js";
|
||||
import * as chromeModule from "./chrome.js";
|
||||
import { InvalidBrowserNavigationUrlError } from "./navigation-guard.js";
|
||||
import * as pwAiModule from "./pw-ai-module.js";
|
||||
import { createBrowserRouteContext } from "./server-context.js";
|
||||
import {
|
||||
@@ -230,6 +231,17 @@ describe("browser server-context remote profile tab operations", () => {
|
||||
expect(tabs.map((t) => t.targetId)).toEqual(["T1"]);
|
||||
});
|
||||
|
||||
it("fails closed for remote tab opens in strict mode without Playwright", async () => {
|
||||
vi.spyOn(pwAiModule, "getPwAiModule").mockResolvedValue(null);
|
||||
const { state, remote, fetchMock } = createRemoteRouteHarness();
|
||||
state.resolved.ssrfPolicy = {};
|
||||
|
||||
await expect(remote.openTab("https://example.com")).rejects.toBeInstanceOf(
|
||||
InvalidBrowserNavigationUrlError,
|
||||
);
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not enforce managed tab cap for remote openclaw profiles", async () => {
|
||||
const listPagesViaPlaywright = vi
|
||||
.fn()
|
||||
|
||||
@@ -5,6 +5,8 @@ import type { ResolvedBrowserProfile } from "./config.js";
|
||||
import {
|
||||
assertBrowserNavigationAllowed,
|
||||
assertBrowserNavigationResultAllowed,
|
||||
InvalidBrowserNavigationUrlError,
|
||||
requiresInspectableBrowserNavigationRedirects,
|
||||
withBrowserNavigationPolicy,
|
||||
} from "./navigation-guard.js";
|
||||
import { getBrowserProfileCapabilities } from "./profile-capabilities.js";
|
||||
@@ -153,6 +155,12 @@ export function createProfileTabOps({
|
||||
}
|
||||
}
|
||||
|
||||
if (requiresInspectableBrowserNavigationRedirects(state().resolved.ssrfPolicy)) {
|
||||
throw new InvalidBrowserNavigationUrlError(
|
||||
"Navigation blocked: strict browser SSRF policy requires Playwright-backed redirect-hop inspection",
|
||||
);
|
||||
}
|
||||
|
||||
const createdViaCdp = await createTargetViaCdp({
|
||||
cdpUrl: profile.cdpUrl,
|
||||
url,
|
||||
|
||||
Reference in New Issue
Block a user