mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-20 17:41:33 -06:00
fix(browser): apply profile proxy policy to tab list redaction (#104715)
* fix(browser): apply profile proxy policy to tab list redaction
* test(browser): cover proxy tab redaction route proof
* test(browser): streamline proxy tab redaction coverage
* fix(browser): scope proxy inference to managed profiles
---------
Co-authored-by: llagy007 <0668001470@xydigit.com>
(cherry picked from commit 5a6d684019)
This commit is contained in:
committed by
Dallin Romney
parent
7e982ae580
commit
faf6c429d2
@@ -35,19 +35,25 @@ describe("browser proxy mode", () => {
|
||||
expect(
|
||||
resolveBrowserNavigationProxyMode({
|
||||
resolved,
|
||||
profile: { driver: "openclaw", cdpIsLoopback: true },
|
||||
profile: { driver: "openclaw", cdpIsLoopback: true, attachOnly: false },
|
||||
}),
|
||||
).toBe("explicit-browser-proxy");
|
||||
expect(
|
||||
resolveBrowserNavigationProxyMode({
|
||||
resolved,
|
||||
profile: { driver: "existing-session", cdpIsLoopback: true },
|
||||
profile: { driver: "existing-session", cdpIsLoopback: true, attachOnly: true },
|
||||
}),
|
||||
).toBe("direct");
|
||||
expect(
|
||||
resolveBrowserNavigationProxyMode({
|
||||
resolved,
|
||||
profile: { driver: "openclaw", cdpIsLoopback: false },
|
||||
profile: { driver: "openclaw", cdpIsLoopback: false, attachOnly: true },
|
||||
}),
|
||||
).toBe("direct");
|
||||
expect(
|
||||
resolveBrowserNavigationProxyMode({
|
||||
resolved,
|
||||
profile: { driver: "openclaw", cdpIsLoopback: true, attachOnly: true },
|
||||
}),
|
||||
).toBe("direct");
|
||||
});
|
||||
|
||||
@@ -52,11 +52,12 @@ export function omitChromeProxyEnv(env: NodeJS.ProcessEnv): NodeJS.ProcessEnv {
|
||||
/** Resolve the navigation proxy mode used by SSRF/navigation guards. */
|
||||
export function resolveBrowserNavigationProxyMode(params: {
|
||||
resolved: Pick<ResolvedBrowserConfig, "extraArgs">;
|
||||
profile: Pick<ResolvedBrowserProfile, "cdpIsLoopback" | "driver">;
|
||||
profile: Pick<ResolvedBrowserProfile, "attachOnly" | "cdpIsLoopback" | "driver">;
|
||||
}): BrowserNavigationProxyMode {
|
||||
if (
|
||||
params.profile.driver === "openclaw" &&
|
||||
params.profile.cdpIsLoopback &&
|
||||
!params.profile.attachOnly &&
|
||||
hasExplicitChromeProxyRoutingArg(params.resolved.extraArgs)
|
||||
) {
|
||||
return "explicit-browser-proxy";
|
||||
|
||||
@@ -5,7 +5,7 @@ import "../server-context.chrome-test-harness.js";
|
||||
import "../../test-support/browser-security.mock.js";
|
||||
import * as chromeModule from "../chrome.js";
|
||||
import { createBrowserRouteContext } from "../server-context.js";
|
||||
import { makeBrowserServerState } from "../server-context.test-harness.js";
|
||||
import { makeBrowserProfile, makeBrowserServerState } from "../server-context.test-harness.js";
|
||||
import { registerBrowserTabRoutes } from "./tabs.js";
|
||||
import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helpers.js";
|
||||
|
||||
@@ -14,20 +14,22 @@ afterEach(() => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
function makeLoopbackProfile(attachOnly: boolean) {
|
||||
return makeBrowserProfile({
|
||||
name: "manual-cdp",
|
||||
cdpUrl: "http://127.0.0.1:9222",
|
||||
cdpHost: "127.0.0.1",
|
||||
cdpIsLoopback: true,
|
||||
cdpPort: 9222,
|
||||
color: "#00AA00",
|
||||
attachOnly,
|
||||
});
|
||||
}
|
||||
|
||||
describe("browser tab routes attachOnly loopback profiles", () => {
|
||||
it("lists tabs for manual loopback CDP profiles under strict SSRF", async () => {
|
||||
const state = makeBrowserServerState({
|
||||
profile: {
|
||||
name: "manual-cdp",
|
||||
cdpUrl: "http://127.0.0.1:9222",
|
||||
cdpHost: "127.0.0.1",
|
||||
cdpIsLoopback: true,
|
||||
cdpPort: 9222,
|
||||
color: "#00AA00",
|
||||
driver: "openclaw",
|
||||
headless: false,
|
||||
attachOnly: true,
|
||||
},
|
||||
profile: makeLoopbackProfile(true),
|
||||
resolvedOverrides: {
|
||||
defaultProfile: "manual-cdp",
|
||||
ssrfPolicy: {},
|
||||
@@ -85,4 +87,76 @@ describe("browser tab routes attachOnly loopback profiles", () => {
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ attachOnly: false, allowPrivateNetwork: false, expectedUrl: "" },
|
||||
{
|
||||
attachOnly: false,
|
||||
allowPrivateNetwork: true,
|
||||
expectedUrl: "http://93.184.216.34/proxy-routed",
|
||||
},
|
||||
{
|
||||
attachOnly: true,
|
||||
allowPrivateNetwork: false,
|
||||
expectedUrl: "http://93.184.216.34/proxy-routed",
|
||||
},
|
||||
])(
|
||||
"applies managed browser proxy policy to tab list URLs (attachOnly=$attachOnly, allowPrivateNetwork=$allowPrivateNetwork)",
|
||||
async ({ attachOnly, allowPrivateNetwork, expectedUrl }) => {
|
||||
const state = makeBrowserServerState({
|
||||
profile: makeLoopbackProfile(attachOnly),
|
||||
resolvedOverrides: {
|
||||
defaultProfile: "manual-cdp",
|
||||
extraArgs: ["--proxy-server=http://proxy.example.test:8080"],
|
||||
ssrfPolicy: { dangerouslyAllowPrivateNetwork: allowPrivateNetwork },
|
||||
},
|
||||
});
|
||||
|
||||
const isChromeCdpReady = vi.mocked(chromeModule.isChromeCdpReady);
|
||||
isChromeCdpReady.mockResolvedValue(true);
|
||||
|
||||
const fetchMock = vi.fn(
|
||||
async () =>
|
||||
new Response(
|
||||
JSON.stringify([
|
||||
{
|
||||
id: "PAGE-1",
|
||||
title: "Proxy routed",
|
||||
url: "http://93.184.216.34/proxy-routed",
|
||||
webSocketDebuggerUrl: "ws://127.0.0.1:9222/devtools/page/PAGE-1",
|
||||
type: "page",
|
||||
},
|
||||
]),
|
||||
{ headers: { "content-type": "application/json" } },
|
||||
),
|
||||
);
|
||||
vi.stubGlobal("fetch", fetchMock);
|
||||
|
||||
const ctx = createBrowserRouteContext({ getState: () => state });
|
||||
const { app, getHandlers, postHandlers } = createBrowserRouteApp();
|
||||
registerBrowserTabRoutes(app, ctx as never);
|
||||
const getTabs = getHandlers.get("/tabs");
|
||||
const postTabsAction = postHandlers.get("/tabs/action");
|
||||
expect(getTabs).toBeTypeOf("function");
|
||||
expect(postTabsAction).toBeTypeOf("function");
|
||||
|
||||
const getResponse = createBrowserRouteResponse();
|
||||
await getTabs?.({ params: {}, query: { profile: "manual-cdp" }, body: {} }, getResponse.res);
|
||||
|
||||
const actionResponse = createBrowserRouteResponse();
|
||||
await postTabsAction?.(
|
||||
{ params: {}, query: { profile: "manual-cdp" }, body: { action: "list" } },
|
||||
actionResponse.res,
|
||||
);
|
||||
|
||||
expect(getResponse.statusCode).toBe(200);
|
||||
expect(actionResponse.statusCode).toBe(200);
|
||||
expect(fetchMock).toHaveBeenCalledTimes(2);
|
||||
for (const response of [getResponse, actionResponse]) {
|
||||
expect(response.body).toMatchObject({
|
||||
tabs: [{ targetId: "PAGE-1", title: "Proxy routed", url: expectedUrl }],
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -122,6 +122,7 @@ function createRouteContext(
|
||||
state: () => ({
|
||||
resolved: {
|
||||
actionTimeoutMs: options?.actionTimeoutMs ?? 45_000,
|
||||
extraArgs: [],
|
||||
ssrfPolicy: options?.ssrfPolicy,
|
||||
},
|
||||
}),
|
||||
|
||||
@@ -13,7 +13,6 @@ import {
|
||||
import {
|
||||
assertBrowserNavigationAllowed,
|
||||
assertBrowserNavigationResultAllowed,
|
||||
withBrowserNavigationPolicy,
|
||||
} from "../navigation-guard.js";
|
||||
import { getBrowserProfileCapabilities } from "../profile-capabilities.js";
|
||||
import type { BrowserRouteContext, ProfileContext } from "../server-context.js";
|
||||
@@ -101,10 +100,9 @@ async function ensureBrowserRunning(
|
||||
|
||||
async function redactBlockedTabUrls(params: {
|
||||
tabs: Awaited<ReturnType<ProfileContext["listTabs"]>>;
|
||||
ssrfPolicy: ReturnType<BrowserRouteContext["state"]>["resolved"]["ssrfPolicy"];
|
||||
navigationPolicy: ReturnType<typeof browserNavigationPolicyForProfile>;
|
||||
}): Promise<Awaited<ReturnType<ProfileContext["listTabs"]>>> {
|
||||
const ssrfPolicyOpts = withBrowserNavigationPolicy(params.ssrfPolicy);
|
||||
if (!ssrfPolicyOpts.ssrfPolicy) {
|
||||
if (!params.navigationPolicy.ssrfPolicy) {
|
||||
return params.tabs;
|
||||
}
|
||||
|
||||
@@ -113,7 +111,7 @@ async function redactBlockedTabUrls(params: {
|
||||
try {
|
||||
await assertBrowserNavigationResultAllowed({
|
||||
url: tab.url,
|
||||
...ssrfPolicyOpts,
|
||||
...params.navigationPolicy,
|
||||
});
|
||||
redactedTabs.push(tab);
|
||||
} catch {
|
||||
@@ -213,7 +211,7 @@ export function registerBrowserTabRoutes(app: BrowserRouteRegistrar, ctx: Browse
|
||||
}
|
||||
const tabs = await redactBlockedTabUrls({
|
||||
tabs: await profileCtx.listTabs(),
|
||||
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
|
||||
navigationPolicy: browserNavigationPolicyForProfile(ctx, profileCtx),
|
||||
});
|
||||
res.json({ running: true, tabs });
|
||||
},
|
||||
@@ -323,7 +321,7 @@ export function registerBrowserTabRoutes(app: BrowserRouteRegistrar, ctx: Browse
|
||||
}
|
||||
const tabs = await redactBlockedTabUrls({
|
||||
tabs: await profileCtx.listTabs(),
|
||||
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
|
||||
navigationPolicy: browserNavigationPolicyForProfile(ctx, profileCtx),
|
||||
});
|
||||
return res.json({ ok: true, tabs });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user