mirror of
https://github.com/moltbot/moltbot.git
synced 2026-04-24 07:01:49 +00:00
fix(browser): re-check interaction-driven navigations (#63226)
* fix(browser): guard interaction-driven navigations * fix(browser): avoid rechecking unchanged interaction urls * fix(browser): guard delayed interaction navigations * fix(browser): guard interaction-driven navigations for full action duration * fix(browser): avoid waiting on interaction grace timer * fix(browser): ignore same-document hash-only URL changes in navigation guard * fix(browser): dedupe interaction nav guards * fix(browser): guard same-URL reloads in interaction navigation listeners * docs(changelog): add interaction navigation guard entry * fix(browser): drop duplicate ssrfPolicy props * fix(browser): tighten interaction navigation guards --------- Co-authored-by: Devin Robison <drobison@nvidia.com>
This commit is contained in:
@@ -30,6 +30,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Gateway/sessions: clear auto-fallback-pinned model overrides on `/reset` and `/new` while still preserving explicit user model selections, including legacy sessions created before override-source tracking existed. (#63155) Thanks @frankekn.
|
||||
- Codex CLI: pass OpenClaw's system prompt through Codex's `model_instructions_file` config override so fresh Codex CLI sessions receive the same prompt guidance as Claude CLI sessions.
|
||||
- Matrix/gateway: wait for Matrix sync readiness before marking startup successful, keep Matrix background handler failures contained, and route fatal Matrix sync stops through channel-level restart handling instead of crashing the whole gateway. (#62779) Thanks @gumadeiras.
|
||||
- Browser/security: re-run blocked-destination safety checks after interaction-driven main-frame navigations from click, evaluate, hook-triggered click, and batched action flows, so browser interactions cannot bypass the SSRF quarantine when they land on forbidden URLs. (#63226) Thanks @eleqtrizit.
|
||||
|
||||
## 2026.4.8
|
||||
|
||||
|
||||
@@ -1,6 +1,9 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
let page: { evaluate: ReturnType<typeof vi.fn> } | null = null;
|
||||
let page: {
|
||||
evaluate: ReturnType<typeof vi.fn>;
|
||||
url: ReturnType<typeof vi.fn>;
|
||||
} | null = null;
|
||||
|
||||
const getPageForTargetId = vi.fn(async () => {
|
||||
if (!page) {
|
||||
@@ -9,6 +12,7 @@ const getPageForTargetId = vi.fn(async () => {
|
||||
return page;
|
||||
});
|
||||
const ensurePageState = vi.fn(() => {});
|
||||
const assertPageNavigationCompletedSafely = vi.fn(async () => {});
|
||||
const forceDisconnectPlaywrightForTarget = vi.fn(async () => {});
|
||||
const refLocator = vi.fn(() => {
|
||||
throw new Error("test: refLocator should not be called");
|
||||
@@ -19,6 +23,7 @@ const closePageViaPlaywright = vi.fn(async () => {});
|
||||
const resizeViewportViaPlaywright = vi.fn(async () => {});
|
||||
|
||||
vi.mock("./pw-session.js", () => ({
|
||||
assertPageNavigationCompletedSafely,
|
||||
ensurePageState,
|
||||
forceDisconnectPlaywrightForTarget,
|
||||
getPageForTargetId,
|
||||
@@ -38,6 +43,7 @@ describe("batchViaPlaywright", () => {
|
||||
vi.clearAllMocks();
|
||||
page = {
|
||||
evaluate: vi.fn(async () => "ok"),
|
||||
url: vi.fn(() => "about:blank"),
|
||||
};
|
||||
});
|
||||
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
let page: { evaluate: ReturnType<typeof vi.fn> } | null = null;
|
||||
let page: { evaluate: ReturnType<typeof vi.fn>; url: ReturnType<typeof vi.fn> } | null = null;
|
||||
let locator: { evaluate: ReturnType<typeof vi.fn> } | null = null;
|
||||
|
||||
const forceDisconnectPlaywrightForTarget = vi.fn(async () => {});
|
||||
@@ -11,6 +11,7 @@ const getPageForTargetId = vi.fn(async () => {
|
||||
return page;
|
||||
});
|
||||
const ensurePageState = vi.fn(() => {});
|
||||
const assertPageNavigationCompletedSafely = vi.fn(async () => {});
|
||||
const restoreRoleRefsForTarget = vi.fn(() => {});
|
||||
const refLocator = vi.fn(() => {
|
||||
if (!locator) {
|
||||
@@ -21,6 +22,7 @@ const refLocator = vi.fn(() => {
|
||||
|
||||
vi.mock("./pw-session.js", () => {
|
||||
return {
|
||||
assertPageNavigationCompletedSafely,
|
||||
ensurePageState,
|
||||
forceDisconnectPlaywrightForTarget,
|
||||
getPageForTargetId,
|
||||
@@ -64,6 +66,7 @@ describe("evaluateViaPlaywright (abort)", () => {
|
||||
}
|
||||
return pendingPromise;
|
||||
}),
|
||||
url: vi.fn(() => "https://example.com/current"),
|
||||
};
|
||||
locator = {
|
||||
evaluate: vi.fn(() => {
|
||||
|
||||
@@ -0,0 +1,527 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
getPwToolsCoreSessionMocks,
|
||||
installPwToolsCoreTestHooks,
|
||||
setPwToolsCoreCurrentPage,
|
||||
setPwToolsCoreCurrentRefLocator,
|
||||
} from "./pw-tools-core.test-harness.js";
|
||||
|
||||
installPwToolsCoreTestHooks();
|
||||
const mod = await import("./pw-tools-core.js");
|
||||
|
||||
describe("pw-tools-core interaction navigation guard", () => {
|
||||
it("does not wait for the grace window after a successful non-navigating click", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const listeners = new Set<() => void>();
|
||||
const click = vi.fn(async () => {});
|
||||
const page = {
|
||||
on: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => "http://127.0.0.1:9222/json/version"),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const completion = vi.fn();
|
||||
const task = mod
|
||||
.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
})
|
||||
.then(completion);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(completion).toHaveBeenCalledTimes(1);
|
||||
expect(listeners.size).toBe(1);
|
||||
expect(
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely,
|
||||
).not.toHaveBeenCalled();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(250);
|
||||
expect(listeners.size).toBe(0);
|
||||
await task;
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("runs the post-click navigation guard when navigation starts shortly after the click resolves", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const listeners = new Set<() => void>();
|
||||
let currentUrl = "http://127.0.0.1:9222/json/version";
|
||||
const click = vi.fn(async () => {
|
||||
setTimeout(() => {
|
||||
currentUrl = "http://127.0.0.1:9222/json/list";
|
||||
for (const listener of listeners) {
|
||||
listener();
|
||||
}
|
||||
}, 10);
|
||||
});
|
||||
const page = {
|
||||
on: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => currentUrl),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const completion = vi.fn();
|
||||
const task = mod
|
||||
.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
})
|
||||
.then(completion);
|
||||
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
expect(completion).toHaveBeenCalledTimes(1);
|
||||
expect(
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely,
|
||||
).not.toHaveBeenCalled();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
await task;
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith(
|
||||
{
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
},
|
||||
);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("ignores subframe framenavigated events before the main frame navigates", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const listeners = new Set<(frame: object) => void>();
|
||||
const mainFrame = {};
|
||||
const subframe = {};
|
||||
let currentUrl = "http://127.0.0.1:9222/json/version";
|
||||
const click = vi.fn(async () => {
|
||||
setTimeout(() => {
|
||||
for (const listener of listeners) {
|
||||
listener(subframe);
|
||||
}
|
||||
}, 10);
|
||||
setTimeout(() => {
|
||||
currentUrl = "http://127.0.0.1:9222/json/list";
|
||||
for (const listener of listeners) {
|
||||
listener(mainFrame);
|
||||
}
|
||||
}, 20);
|
||||
});
|
||||
const page = {
|
||||
mainFrame: vi.fn(() => mainFrame),
|
||||
on: vi.fn((event: string, listener: (frame: object) => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: (frame: object) => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => currentUrl),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const task = mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
expect(listeners.size).toBe(1);
|
||||
expect(
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely,
|
||||
).not.toHaveBeenCalled();
|
||||
|
||||
await vi.advanceTimersByTimeAsync(10);
|
||||
await task;
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith(
|
||||
{
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
},
|
||||
);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("deduplicates delayed navigation guards across repeated successful interactions", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const listeners = new Set<() => void>();
|
||||
let currentUrl = "http://127.0.0.1:9222/json/version";
|
||||
const click = vi.fn(async () => {});
|
||||
const page = {
|
||||
on: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => currentUrl),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
await mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
expect(listeners.size).toBe(1);
|
||||
|
||||
await mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
expect(listeners.size).toBe(1);
|
||||
|
||||
currentUrl = "http://127.0.0.1:9222/json/list";
|
||||
for (const listener of Array.from(listeners)) {
|
||||
listener();
|
||||
}
|
||||
await vi.advanceTimersByTimeAsync(0);
|
||||
|
||||
expect(
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely,
|
||||
).toHaveBeenCalledTimes(1);
|
||||
expect(listeners.size).toBe(0);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("runs the post-click navigation guard with the resolved SSRF policy", async () => {
|
||||
const click = vi.fn(async () => {});
|
||||
const page = {
|
||||
url: vi
|
||||
.fn()
|
||||
.mockReturnValueOnce("http://127.0.0.1:9222/json/version")
|
||||
.mockReturnValue("http://127.0.0.1:9222/json/list"),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const blocked = new Error("blocked interaction navigation");
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely.mockRejectedValueOnce(blocked);
|
||||
|
||||
await expect(
|
||||
mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
}),
|
||||
).rejects.toThrow("blocked interaction navigation");
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
});
|
||||
});
|
||||
|
||||
it("skips interaction navigation guards when no explicit SSRF policy is provided", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const listeners = new Set<(frame: object) => void>();
|
||||
const mainFrame = {};
|
||||
let currentUrl = "http://127.0.0.1:9222/json/version";
|
||||
const click = vi.fn(async () => {
|
||||
currentUrl = "http://127.0.0.1:9222/json/list";
|
||||
for (const listener of listeners) {
|
||||
listener(mainFrame);
|
||||
}
|
||||
});
|
||||
const page = {
|
||||
mainFrame: vi.fn(() => mainFrame),
|
||||
on: vi.fn((event: string, listener: (frame: object) => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: (frame: object) => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => currentUrl),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
await mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
});
|
||||
await vi.runAllTimersAsync();
|
||||
|
||||
expect(page.on).not.toHaveBeenCalled();
|
||||
expect(page.off).not.toHaveBeenCalled();
|
||||
expect(
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely,
|
||||
).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
|
||||
it("runs the post-evaluate navigation guard after page evaluation", async () => {
|
||||
const page = {
|
||||
evaluate: vi.fn(async () => "ok"),
|
||||
url: vi
|
||||
.fn()
|
||||
.mockReturnValueOnce("http://127.0.0.1:9222/json/version")
|
||||
.mockReturnValue("http://127.0.0.1:9222/json/list"),
|
||||
};
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const result = await mod.evaluateViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
fn: "() => location.href = 'http://127.0.0.1:9222/json/version'",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
|
||||
expect(result).toBe("ok");
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not run the post-click navigation guard when the url is unchanged", async () => {
|
||||
const click = vi.fn(async () => {});
|
||||
const page = { url: vi.fn(() => "http://127.0.0.1:9222/json/version") };
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
await mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not run the navigation guard when only the URL hash changes (same-document navigation)", async () => {
|
||||
const click = vi.fn(async () => {});
|
||||
const page = {
|
||||
url: vi
|
||||
.fn()
|
||||
.mockReturnValueOnce("https://example.com/page")
|
||||
.mockReturnValue("https://example.com/page#section"),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
await mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("runs the navigation guard when a same-URL reload fires framenavigated during a click", async () => {
|
||||
// A page reload (form submit, location.reload()) keeps the URL identical but
|
||||
// fires framenavigated. Prior to the isHashOnlyNavigation fix, didCrossDocumentUrlChange
|
||||
// would treat currentUrl === previousUrl as "no navigation" and skip the SSRF guard.
|
||||
const listeners = new Set<() => void>();
|
||||
const sameUrl = "http://192.168.1.1/admin";
|
||||
const click = vi.fn(async () => {
|
||||
// Simulate reload: URL stays the same but framenavigated fires during the click
|
||||
for (const listener of listeners) {
|
||||
listener();
|
||||
}
|
||||
});
|
||||
const page = {
|
||||
on: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => sameUrl),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
await mod.clickViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ref: "1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
});
|
||||
});
|
||||
|
||||
it("does not run the post-evaluate navigation guard when the url is unchanged", async () => {
|
||||
const page = {
|
||||
evaluate: vi.fn(async () => "ok"),
|
||||
url: vi.fn(() => "http://127.0.0.1:9222/json/version"),
|
||||
};
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const result = await mod.evaluateViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
fn: "() => 1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
|
||||
expect(result).toBe("ok");
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("propagates the SSRF policy through batch interaction actions", async () => {
|
||||
const click = vi.fn(async () => {});
|
||||
const page = {
|
||||
url: vi.fn().mockReturnValueOnce("about:blank").mockReturnValue("https://example.com/after"),
|
||||
};
|
||||
setPwToolsCoreCurrentRefLocator({ click });
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
await mod.batchViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
actions: [{ kind: "click", ref: "1" }],
|
||||
});
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
});
|
||||
});
|
||||
|
||||
it("runs the post-evaluate navigation guard when evaluate rejects after triggering navigation", async () => {
|
||||
vi.useFakeTimers();
|
||||
try {
|
||||
const listeners = new Set<() => void>();
|
||||
let currentUrl = "http://127.0.0.1:9222/json/version";
|
||||
const page = {
|
||||
evaluate: vi.fn(async () => {
|
||||
setTimeout(() => {
|
||||
currentUrl = "http://127.0.0.1:9222/json/list";
|
||||
for (const listener of listeners) {
|
||||
listener();
|
||||
}
|
||||
}, 0);
|
||||
throw new Error("evaluate failed after scheduling navigation");
|
||||
}),
|
||||
on: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.add(listener);
|
||||
}
|
||||
}),
|
||||
off: vi.fn((event: string, listener: () => void) => {
|
||||
if (event === "framenavigated") {
|
||||
listeners.delete(listener);
|
||||
}
|
||||
}),
|
||||
url: vi.fn(() => currentUrl),
|
||||
};
|
||||
setPwToolsCoreCurrentPage(page);
|
||||
|
||||
const blocked = new Error("blocked interaction navigation");
|
||||
getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely.mockRejectedValueOnce(
|
||||
blocked,
|
||||
);
|
||||
|
||||
const task = mod.evaluateViaPlaywright({
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
targetId: "T1",
|
||||
fn: "() => location.href = 'http://127.0.0.1:9222/json/list'",
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
});
|
||||
const expectation = expect(task).rejects.toThrow("blocked interaction navigation");
|
||||
|
||||
await vi.runAllTimersAsync();
|
||||
await expectation;
|
||||
|
||||
expect(getPwToolsCoreSessionMocks().assertPageNavigationCompletedSafely).toHaveBeenCalledWith(
|
||||
{
|
||||
cdpUrl: "http://127.0.0.1:18792",
|
||||
page,
|
||||
response: null,
|
||||
ssrfPolicy: { allowPrivateNetwork: false },
|
||||
targetId: "T1",
|
||||
},
|
||||
);
|
||||
} finally {
|
||||
vi.useRealTimers();
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -1,5 +1,6 @@
|
||||
import { normalizeOptionalString } from "openclaw/plugin-sdk/text-runtime";
|
||||
import { formatErrorMessage } from "../infra/errors.js";
|
||||
import type { Frame, Page } from "playwright-core";
|
||||
import type { SsrFPolicy } from "../infra/net/ssrf.js";
|
||||
import type { BrowserActRequest, BrowserFormField } from "./client-actions-core.js";
|
||||
import { DEFAULT_FILL_FIELD_TYPE } from "./form-fields.js";
|
||||
@@ -28,6 +29,15 @@ type TargetOpts = {
|
||||
const MAX_CLICK_DELAY_MS = 5_000;
|
||||
const MAX_WAIT_TIME_MS = 30_000;
|
||||
const MAX_BATCH_ACTIONS = 100;
|
||||
const INTERACTION_NAVIGATION_GRACE_MS = 250;
|
||||
|
||||
type NavigationObservablePage = Pick<Page, "url"> & {
|
||||
mainFrame?: () => Frame;
|
||||
on?: (event: "framenavigated", listener: (frame: Frame) => void) => unknown;
|
||||
off?: (event: "framenavigated", listener: (frame: Frame) => void) => unknown;
|
||||
};
|
||||
|
||||
const pendingInteractionNavigationGuardCleanup = new WeakMap<Page, () => void>();
|
||||
|
||||
function resolveBoundedDelayMs(value: number | undefined, label: string, maxMs: number): number {
|
||||
const normalized = Math.floor(value ?? 0);
|
||||
@@ -51,6 +61,254 @@ function resolveInteractionTimeoutMs(timeoutMs?: number): number {
|
||||
return Math.max(500, Math.min(60_000, Math.floor(timeoutMs ?? 8000)));
|
||||
}
|
||||
|
||||
// Returns true only when the URL change indicates a cross-document navigation
|
||||
// (i.e., a real network fetch occurred). Same-document hash-only mutations —
|
||||
// anchor clicks and history.pushState/replaceState that change only the
|
||||
// fragment — do not cause a network request and must not trigger SSRF checks.
|
||||
function didCrossDocumentUrlChange(page: { url(): string }, previousUrl: string): boolean {
|
||||
const currentUrl = page.url();
|
||||
if (currentUrl === previousUrl) {
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const prev = new URL(previousUrl);
|
||||
const curr = new URL(currentUrl);
|
||||
if (
|
||||
prev.origin === curr.origin &&
|
||||
prev.pathname === curr.pathname &&
|
||||
prev.search === curr.search
|
||||
) {
|
||||
// Only the fragment changed — same-document navigation, no fetch.
|
||||
return false;
|
||||
}
|
||||
} catch {
|
||||
// Non-parseable URL; fall through to string comparison.
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
// Returns true when a framenavigated event represents only a hash-only
|
||||
// same-document mutation (no network request). Used in event-driven checks
|
||||
// where the event itself is the navigation signal — unlike URL polling, we
|
||||
// cannot use identical URLs as a "no navigation" sentinel because same-URL
|
||||
// reloads and form submits also fire framenavigated with an unchanged URL.
|
||||
function isHashOnlyNavigation(currentUrl: string, previousUrl: string): boolean {
|
||||
if (currentUrl === previousUrl) {
|
||||
// Exact same URL + framenavigated firing = reload or form submit, not a
|
||||
// fragment hop. Must run SSRF checks.
|
||||
return false;
|
||||
}
|
||||
try {
|
||||
const prev = new URL(previousUrl);
|
||||
const curr = new URL(currentUrl);
|
||||
return (
|
||||
prev.origin === curr.origin && prev.pathname === curr.pathname && prev.search === curr.search
|
||||
);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
function isMainFrameNavigation(page: NavigationObservablePage, frame: Frame): boolean {
|
||||
if (typeof page.mainFrame !== "function") {
|
||||
return true;
|
||||
}
|
||||
return frame === page.mainFrame();
|
||||
}
|
||||
|
||||
function observeDelayedInteractionNavigation(
|
||||
page: NavigationObservablePage,
|
||||
previousUrl: string,
|
||||
): Promise<boolean> {
|
||||
if (didCrossDocumentUrlChange(page, previousUrl)) {
|
||||
return Promise.resolve(true);
|
||||
}
|
||||
if (typeof page.on !== "function" || typeof page.off !== "function") {
|
||||
return Promise.resolve(false);
|
||||
}
|
||||
|
||||
return new Promise<boolean>((resolve) => {
|
||||
const onFrameNavigated = (frame: Frame) => {
|
||||
if (!isMainFrameNavigation(page, frame)) {
|
||||
return;
|
||||
}
|
||||
// Use isHashOnlyNavigation rather than !didCrossDocumentUrlChange: the
|
||||
// event firing is itself the navigation signal, so a same-URL reload must
|
||||
// not be treated as "no navigation" the way URL polling would.
|
||||
if (isHashOnlyNavigation(page.url(), previousUrl)) {
|
||||
return;
|
||||
}
|
||||
cleanup();
|
||||
resolve(true);
|
||||
};
|
||||
const timeout = setTimeout(() => {
|
||||
cleanup();
|
||||
resolve(didCrossDocumentUrlChange(page, previousUrl));
|
||||
}, INTERACTION_NAVIGATION_GRACE_MS);
|
||||
const cleanup = () => {
|
||||
clearTimeout(timeout);
|
||||
// Call off directly on page (not via a cached reference) to preserve
|
||||
// Playwright's EventEmitter `this` binding.
|
||||
page.off!("framenavigated", onFrameNavigated);
|
||||
};
|
||||
|
||||
// Call on directly on page (not via a cached reference) to preserve
|
||||
// Playwright's EventEmitter `this` binding.
|
||||
page.on!("framenavigated", onFrameNavigated);
|
||||
});
|
||||
}
|
||||
|
||||
function scheduleDelayedInteractionNavigationGuard(opts: {
|
||||
cdpUrl: string;
|
||||
page: Page;
|
||||
previousUrl: string;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
targetId?: string;
|
||||
}): void {
|
||||
if (!opts.ssrfPolicy) {
|
||||
return;
|
||||
}
|
||||
const page = opts.page as unknown as NavigationObservablePage;
|
||||
if (didCrossDocumentUrlChange(page, opts.previousUrl)) {
|
||||
void assertPageNavigationCompletedSafely({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page: opts.page,
|
||||
response: null,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
}).catch(() => {});
|
||||
return;
|
||||
}
|
||||
if (typeof page.on !== "function" || typeof page.off !== "function") {
|
||||
return;
|
||||
}
|
||||
|
||||
pendingInteractionNavigationGuardCleanup.get(opts.page)?.();
|
||||
|
||||
const onFrameNavigated = (frame: Frame) => {
|
||||
if (!isMainFrameNavigation(page, frame)) {
|
||||
return;
|
||||
}
|
||||
// Use isHashOnlyNavigation rather than !didCrossDocumentUrlChange: the
|
||||
// event firing is itself the navigation signal, so a same-URL reload must
|
||||
// not be treated as "no navigation" the way URL polling would.
|
||||
if (isHashOnlyNavigation(page.url(), opts.previousUrl)) {
|
||||
return;
|
||||
}
|
||||
cleanup();
|
||||
void assertPageNavigationCompletedSafely({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page: opts.page,
|
||||
response: null,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
}).catch(() => {});
|
||||
};
|
||||
const timeout = setTimeout(() => {
|
||||
cleanup();
|
||||
}, INTERACTION_NAVIGATION_GRACE_MS);
|
||||
const cleanup = () => {
|
||||
clearTimeout(timeout);
|
||||
page.off!("framenavigated", onFrameNavigated);
|
||||
if (pendingInteractionNavigationGuardCleanup.get(opts.page) === cleanup) {
|
||||
pendingInteractionNavigationGuardCleanup.delete(opts.page);
|
||||
}
|
||||
};
|
||||
|
||||
pendingInteractionNavigationGuardCleanup.set(opts.page, cleanup);
|
||||
page.on("framenavigated", onFrameNavigated);
|
||||
}
|
||||
|
||||
async function assertInteractionNavigationCompletedSafely<T>(opts: {
|
||||
action: () => Promise<T>;
|
||||
cdpUrl: string;
|
||||
page: Page;
|
||||
previousUrl: string;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
targetId?: string;
|
||||
}): Promise<T> {
|
||||
if (!opts.ssrfPolicy) {
|
||||
return await opts.action();
|
||||
}
|
||||
// Phase 1: keep a framenavigated listener alive for the entire duration of the
|
||||
// action so navigations triggered mid-click or mid-evaluate are not missed.
|
||||
// Using a fixed pre-action timer would expire before the action finishes for
|
||||
// slow interactions, silently bypassing the SSRF guard.
|
||||
const navPage = opts.page as unknown as NavigationObservablePage;
|
||||
let navigatedDuringAction = false;
|
||||
const onFrameNavigated = (frame: Frame) => {
|
||||
if (!isMainFrameNavigation(navPage, frame)) {
|
||||
return;
|
||||
}
|
||||
// Use isHashOnlyNavigation rather than didCrossDocumentUrlChange: the event
|
||||
// firing is the navigation signal, so a same-URL reload must not be skipped
|
||||
// the way it would be by URL-equality polling.
|
||||
if (!isHashOnlyNavigation(opts.page.url(), opts.previousUrl)) {
|
||||
navigatedDuringAction = true;
|
||||
}
|
||||
};
|
||||
if (typeof navPage.on === "function") {
|
||||
navPage.on("framenavigated", onFrameNavigated);
|
||||
}
|
||||
|
||||
let result: T | undefined;
|
||||
let actionError: unknown = null;
|
||||
try {
|
||||
result = await opts.action();
|
||||
} catch (err) {
|
||||
actionError = err;
|
||||
} finally {
|
||||
if (typeof navPage.off === "function") {
|
||||
navPage.off("framenavigated", onFrameNavigated);
|
||||
}
|
||||
}
|
||||
|
||||
const navigationObserved =
|
||||
navigatedDuringAction || didCrossDocumentUrlChange(opts.page, opts.previousUrl);
|
||||
|
||||
if (navigationObserved) {
|
||||
await assertPageNavigationCompletedSafely({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page: opts.page,
|
||||
response: null,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
} else if (actionError) {
|
||||
// Preserve the action-error path semantics: if a rejected click/evaluate still
|
||||
// triggers a delayed navigation, the SSRF block must win over the original
|
||||
// action error instead of surfacing a stale interaction failure.
|
||||
const delayedNavigationObserved = await observeDelayedInteractionNavigation(
|
||||
opts.page,
|
||||
opts.previousUrl,
|
||||
);
|
||||
if (delayedNavigationObserved) {
|
||||
await assertPageNavigationCompletedSafely({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page: opts.page,
|
||||
response: null,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
}
|
||||
} else {
|
||||
// Successful non-navigating interactions should not wait out the grace window,
|
||||
// but we still keep a short-lived listener alive to quarantine late SSRF hops.
|
||||
scheduleDelayedInteractionNavigationGuard({
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page: opts.page,
|
||||
previousUrl: opts.previousUrl,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
}
|
||||
|
||||
if (actionError) {
|
||||
throw actionError;
|
||||
}
|
||||
return result as T;
|
||||
}
|
||||
|
||||
async function awaitEvalWithAbort<T>(
|
||||
evalPromise: Promise<T>,
|
||||
abortPromise?: Promise<never>,
|
||||
@@ -118,28 +376,32 @@ export async function clickViaPlaywright(opts: {
|
||||
? refLocator(page, requireRef(resolved.ref))
|
||||
: page.locator(resolved.selector!);
|
||||
const timeout = resolveInteractionTimeoutMs(opts.timeoutMs);
|
||||
const previousUrl = page.url();
|
||||
try {
|
||||
const delayMs = resolveBoundedDelayMs(opts.delayMs, "click delayMs", MAX_CLICK_DELAY_MS);
|
||||
if (delayMs > 0) {
|
||||
await locator.hover({ timeout });
|
||||
await new Promise((resolve) => setTimeout(resolve, delayMs));
|
||||
}
|
||||
if (opts.doubleClick) {
|
||||
await locator.dblclick({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
});
|
||||
} else {
|
||||
await locator.click({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
});
|
||||
}
|
||||
await assertPostInteractionNavigationSafe({
|
||||
await assertInteractionNavigationCompletedSafely({
|
||||
action: async () => {
|
||||
const delayMs = resolveBoundedDelayMs(opts.delayMs, "click delayMs", MAX_CLICK_DELAY_MS);
|
||||
if (delayMs > 0) {
|
||||
await locator.hover({ timeout });
|
||||
await new Promise((resolve) => setTimeout(resolve, delayMs));
|
||||
}
|
||||
if (opts.doubleClick) {
|
||||
await locator.dblclick({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
});
|
||||
return;
|
||||
}
|
||||
await locator.click({
|
||||
timeout,
|
||||
button: opts.button,
|
||||
modifiers: opts.modifiers,
|
||||
});
|
||||
},
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page,
|
||||
previousUrl,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
@@ -332,6 +594,7 @@ export async function fillFormViaPlaywright(opts: {
|
||||
export async function evaluateViaPlaywright(opts: {
|
||||
cdpUrl: string;
|
||||
targetId?: string;
|
||||
ssrfPolicy?: SsrFPolicy;
|
||||
fn: string;
|
||||
ref?: string;
|
||||
timeoutMs?: number;
|
||||
@@ -393,6 +656,7 @@ export async function evaluateViaPlaywright(opts: {
|
||||
try {
|
||||
if (opts.ref) {
|
||||
const locator = refLocator(page, opts.ref);
|
||||
const previousUrl = page.url();
|
||||
// eslint-disable-next-line @typescript-eslint/no-implied-eval -- required for browser-context eval
|
||||
const elementEvaluator = new Function(
|
||||
"el",
|
||||
@@ -421,9 +685,18 @@ export async function evaluateViaPlaywright(opts: {
|
||||
fnBody: fnText,
|
||||
timeoutMs: evaluateTimeout,
|
||||
});
|
||||
return await awaitEvalWithAbort(evalPromise, abortPromise);
|
||||
const result = await assertInteractionNavigationCompletedSafely({
|
||||
action: () => awaitEvalWithAbort(evalPromise, abortPromise),
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page,
|
||||
previousUrl,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
return result;
|
||||
}
|
||||
|
||||
const previousUrl = page.url();
|
||||
// eslint-disable-next-line @typescript-eslint/no-implied-eval -- required for browser-context eval
|
||||
const browserEvaluator = new Function(
|
||||
"args",
|
||||
@@ -451,7 +724,15 @@ export async function evaluateViaPlaywright(opts: {
|
||||
fnBody: fnText,
|
||||
timeoutMs: evaluateTimeout,
|
||||
});
|
||||
return await awaitEvalWithAbort(evalPromise, abortPromise);
|
||||
const result = await assertInteractionNavigationCompletedSafely({
|
||||
action: () => awaitEvalWithAbort(evalPromise, abortPromise),
|
||||
cdpUrl: opts.cdpUrl,
|
||||
page,
|
||||
previousUrl,
|
||||
ssrfPolicy: opts.ssrfPolicy,
|
||||
targetId: opts.targetId,
|
||||
});
|
||||
return result;
|
||||
} finally {
|
||||
if (signal && abortListener) {
|
||||
signal.removeEventListener("abort", abortListener);
|
||||
@@ -880,6 +1161,7 @@ async function executeSingleAction(
|
||||
await evaluateViaPlaywright({
|
||||
cdpUrl,
|
||||
targetId: effectiveTargetId,
|
||||
ssrfPolicy,
|
||||
fn: action.fn,
|
||||
ref: action.ref,
|
||||
timeoutMs: action.timeoutMs,
|
||||
@@ -895,10 +1177,10 @@ async function executeSingleAction(
|
||||
await batchViaPlaywright({
|
||||
cdpUrl,
|
||||
targetId: effectiveTargetId,
|
||||
ssrfPolicy,
|
||||
actions: action.actions,
|
||||
stopOnError: action.stopOnError,
|
||||
evaluateEnabled,
|
||||
ssrfPolicy,
|
||||
depth: depth + 1,
|
||||
});
|
||||
break;
|
||||
|
||||
@@ -100,8 +100,8 @@ export function registerBrowserAgentActHookRoutes(
|
||||
await pw.clickViaPlaywright({
|
||||
cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
ref,
|
||||
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
|
||||
ref,
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
@@ -539,8 +539,8 @@ export function registerBrowserAgentActRoutes(
|
||||
const clickRequest: Parameters<typeof pw.clickViaPlaywright>[0] = {
|
||||
cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
|
||||
doubleClick,
|
||||
ssrfPolicy,
|
||||
};
|
||||
if (ref) {
|
||||
clickRequest.ref = ref;
|
||||
@@ -1047,6 +1047,7 @@ export function registerBrowserAgentActRoutes(
|
||||
const evalRequest: Parameters<typeof pw.evaluateViaPlaywright>[0] = {
|
||||
cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
|
||||
fn,
|
||||
ref,
|
||||
signal: req.signal,
|
||||
@@ -1106,10 +1107,10 @@ export function registerBrowserAgentActRoutes(
|
||||
const result = await pw.batchViaPlaywright({
|
||||
cdpUrl,
|
||||
targetId: tab.targetId,
|
||||
ssrfPolicy: ctx.state().resolved.ssrfPolicy,
|
||||
actions,
|
||||
stopOnError,
|
||||
evaluateEnabled,
|
||||
ssrfPolicy,
|
||||
});
|
||||
return res.json({ ok: true, targetId: tab.targetId, results: result.results });
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user