mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-05 19:40:21 +00:00
fix(browser): harden SSRF redirect guard against non-navigation document hops [AI] (#62355)
* fix: address issue * fix: address PR review feedback * docs(changelog): add browser redirect SSRF entry --------- Co-authored-by: Devin Robison <drobison@nvidia.com> Co-authored-by: Devin Robison <drobison00@users.noreply.github.com>
This commit is contained in:
@@ -179,6 +179,44 @@ describe("pw-session createPageViaPlaywright navigation guard", () => {
|
||||
expect(pageClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("blocks private redirect hops even when Playwright marks hop as non-navigation", async () => {
|
||||
const { pageGoto, pageClose, getRouteHandler, mainFrame } = installBrowserMocks();
|
||||
pageGoto.mockImplementationOnce(async () => {
|
||||
const handler = getRouteHandler();
|
||||
if (!handler) {
|
||||
throw new Error("missing route handler");
|
||||
}
|
||||
await handler(
|
||||
{ continue: vi.fn(async () => {}), abort: vi.fn(async () => {}) },
|
||||
{
|
||||
isNavigationRequest: () => true,
|
||||
frame: () => mainFrame,
|
||||
url: () => "https://93.184.216.34/start",
|
||||
},
|
||||
);
|
||||
await handler(
|
||||
{ continue: vi.fn(async () => {}), abort: vi.fn(async () => {}) },
|
||||
{
|
||||
isNavigationRequest: () => false,
|
||||
frame: () => mainFrame,
|
||||
resourceType: () => "document",
|
||||
url: () => "http://127.0.0.1:18080/internal-hop",
|
||||
},
|
||||
);
|
||||
throw new Error("Navigation aborted");
|
||||
});
|
||||
|
||||
await expect(
|
||||
createPageViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
url: "https://93.184.216.34/start",
|
||||
}),
|
||||
).rejects.toBeInstanceOf(SsrFBlockedError);
|
||||
|
||||
expect(pageGoto).toHaveBeenCalledTimes(1);
|
||||
expect(pageClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("preserves the created tab on ordinary navigation failure", async () => {
|
||||
const { pageGoto, pageClose } = installBrowserMocks();
|
||||
pageGoto.mockRejectedValueOnce(new Error("page.goto: net::ERR_NAME_NOT_RESOLVED"));
|
||||
|
||||
@@ -665,13 +665,29 @@ export async function getPageForTargetId(opts: {
|
||||
}
|
||||
|
||||
function isTopLevelNavigationRequest(page: Page, request: Request): boolean {
|
||||
if (!request.isNavigationRequest()) {
|
||||
let sameMainFrame = false;
|
||||
try {
|
||||
sameMainFrame = request.frame() === page.mainFrame();
|
||||
} catch {
|
||||
// Frame resolution can fail during redirect/renderer churn; fail closed.
|
||||
sameMainFrame = true;
|
||||
}
|
||||
if (!sameMainFrame) {
|
||||
return false;
|
||||
}
|
||||
|
||||
try {
|
||||
return request.frame() === page.mainFrame();
|
||||
if (request.isNavigationRequest()) {
|
||||
return true;
|
||||
}
|
||||
} catch {
|
||||
return true;
|
||||
// Ignore and fall back to resource-type check below.
|
||||
}
|
||||
|
||||
try {
|
||||
return request.resourceType() === "document";
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user