diff --git a/.github/workflows/opengrep-precise.yml b/.github/workflows/opengrep-precise.yml index 25be520c758..e9f5a6f2b0c 100644 --- a/.github/workflows/opengrep-precise.yml +++ b/.github/workflows/opengrep-precise.yml @@ -44,7 +44,7 @@ jobs: uses: actions/checkout@v6 with: ref: ${{ github.sha }} - fetch-depth: 1 + fetch-depth: 0 fetch-tags: false persist-credentials: false submodules: false diff --git a/CHANGELOG.md b/CHANGELOG.md index c115388c838..aeb524aeb9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ Docs: https://docs.openclaw.ai ### Fixes +- Browser/snapshot: validate current tab URLs against the configured SSRF policy before ChromeMCP or direct CDP snapshot reads, closing the local-managed CDP bypass from GHSA-2x93-h3hg-2xfp while preserving existing-session coverage; the PR also rejects existing-session selectors before URL checks, adds focused route coverage, fetches full opengrep CI history, and stabilizes plugin activation normalization tests. Thanks @zsxsoft. + - Crabbox: bootstrap raw AWS macOS JavaScript commands launched through `/usr/bin/env` so native mac runners without preinstalled Node, Corepack, or pnpm can still run wrapped Node and pnpm proof. - macOS: let app packaging fall back to `corepack pnpm` when a fresh native runner has Node/Corepack but no pnpm shim on `PATH`. - E2E: keep package/onboarding/plugin smoke commands bounded on macOS shells that have Node but no GNU `timeout` or `gtimeout` binary. diff --git a/extensions/browser/src/browser/routes/agent.existing-session.test.ts b/extensions/browser/src/browser/routes/agent.existing-session.test.ts index 693e006dd9f..5441d22598a 100644 --- a/extensions/browser/src/browser/routes/agent.existing-session.test.ts +++ b/extensions/browser/src/browser/routes/agent.existing-session.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; +import { EXISTING_SESSION_LIMITS } from "./existing-session-limits.js"; import { createExistingSessionAgentSharedModule, existingSessionRouteState, @@ -215,13 +216,72 @@ describe("existing-session browser routes", () => { it("checks existing-session snapshot URL when SSRF policy is configured", async () => { const handler = getSnapshotGetHandler({ allowPrivateNetwork: false }); const response = createBrowserRouteResponse(); + await handler?.({ params: {}, query: { format: "ai" } }, response.res); expect(response.statusCode).toBe(200); + expect(navigationGuardMocks.assertBrowserNavigationAllowed).not.toHaveBeenCalled(); expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledWith({ url: "https://example.com", ssrfPolicy: { allowPrivateNetwork: false }, }); + expect(chromeMcpMocks.takeChromeMcpSnapshot).toHaveBeenCalled(); + }); + + it("allows existing-session snapshots under the default SSRF policy object", async () => { + const handler = getSnapshotGetHandler({}); + const response = createBrowserRouteResponse(); + + await handler?.({ params: {}, query: { format: "ai" } }, response.res); + + expect(response.statusCode).toBe(200); + expect(navigationGuardMocks.assertBrowserNavigationAllowed).not.toHaveBeenCalled(); + expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledWith({ + url: "https://example.com", + ssrfPolicy: {}, + }); + expect(chromeMcpMocks.takeChromeMcpSnapshot).toHaveBeenCalled(); + }); + + it("blocks existing-session snapshots when the current URL violates browser navigation policy", async () => { + routeState.profileCtx.ensureTabAvailable.mockResolvedValueOnce({ + targetId: "7", + url: "http://127.0.0.1:8080/admin", + }); + navigationGuardMocks.assertBrowserNavigationResultAllowed.mockRejectedValueOnce( + new Error("browser navigation blocked by policy"), + ); + const handler = getSnapshotGetHandler({ allowPrivateNetwork: false }); + const response = createBrowserRouteResponse(); + + await handler?.({ params: {}, query: { format: "ai" } }, response.res); + + expect(response.statusCode).toBe(400); + expect(response.body).toEqual({ error: "browser navigation blocked by policy" }); + expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledWith({ + url: "http://127.0.0.1:8080/admin", + ssrfPolicy: { allowPrivateNetwork: false }, + }); + expect(chromeMcpMocks.takeChromeMcpSnapshot).not.toHaveBeenCalled(); + }); + + it("rejects existing-session snapshot selectors before checking the current URL", async () => { + routeState.profileCtx.ensureTabAvailable.mockResolvedValueOnce({ + targetId: "7", + url: "http://127.0.0.1:8080/admin", + }); + const handler = getSnapshotGetHandler({ allowPrivateNetwork: false }); + const response = createBrowserRouteResponse(); + + await handler?.({ params: {}, query: { format: "ai", selector: "#admin" } }, response.res); + + expect(response.statusCode).toBe(400); + expect(response.body).toEqual({ + error: EXISTING_SESSION_LIMITS.snapshot.snapshotSelector, + }); + expect(navigationGuardMocks.assertBrowserNavigationAllowed).not.toHaveBeenCalled(); + expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).not.toHaveBeenCalled(); + expect(chromeMcpMocks.takeChromeMcpSnapshot).not.toHaveBeenCalled(); }); it("checks existing-session screenshot URL when SSRF policy is configured", async () => { diff --git a/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts b/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts new file mode 100644 index 00000000000..c52229d92b8 --- /dev/null +++ b/extensions/browser/src/browser/routes/agent.snapshot.local-managed.test.ts @@ -0,0 +1,148 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; +import { createBrowserRouteApp, createBrowserRouteResponse } from "./test-helpers.js"; +import type { BrowserRequest } from "./types.js"; + +const routeState = vi.hoisted(() => ({ + profileCtx: { + profile: { + driver: "openclaw" as const, + name: "openclaw", + cdpUrl: "http://127.0.0.1:18800", + cdpIsLoopback: true, + }, + ensureTabAvailable: vi.fn(async () => ({ + targetId: "7", + url: "http://127.0.0.1:8080/admin", + wsUrl: "ws://127.0.0.1/devtools/page/7", + })), + }, +})); + +const cdpMocks = vi.hoisted(() => ({ + snapshotAria: vi.fn(async () => ({ + nodes: [{ ref: "1", role: "link", name: "private", depth: 0 }], + })), + snapshotRoleViaCdp: vi.fn(async () => ({ + snapshot: '- link "private" [ref=e1]', + refs: { e1: { role: "link", name: "private" } }, + stats: { lines: 1, chars: 25, refs: 1, interactive: 1 }, + })), +})); + +const navigationGuardMocks = vi.hoisted(() => ({ + assertBrowserNavigationAllowed: vi.fn(async () => {}), + assertBrowserNavigationResultAllowed: vi.fn(async () => { + throw new Error("browser navigation blocked by policy"); + }), + withBrowserNavigationPolicy: vi.fn((ssrfPolicy?: unknown) => (ssrfPolicy ? { ssrfPolicy } : {})), +})); + +vi.mock("../cdp.js", () => ({ + captureScreenshot: vi.fn(), + snapshotAria: cdpMocks.snapshotAria, + snapshotRoleViaCdp: cdpMocks.snapshotRoleViaCdp, +})); + +vi.mock("../chrome-mcp.js", () => ({ + evaluateChromeMcpScript: vi.fn(), + navigateChromeMcpPage: vi.fn(), + takeChromeMcpScreenshot: vi.fn(), + takeChromeMcpSnapshot: vi.fn(), +})); + +vi.mock("../navigation-guard.js", () => ({ + assertBrowserNavigationAllowed: navigationGuardMocks.assertBrowserNavigationAllowed, + assertBrowserNavigationResultAllowed: navigationGuardMocks.assertBrowserNavigationResultAllowed, + withBrowserNavigationPolicy: navigationGuardMocks.withBrowserNavigationPolicy, +})); + +vi.mock("../screenshot.js", () => ({ + DEFAULT_BROWSER_SCREENSHOT_MAX_BYTES: 128, + DEFAULT_BROWSER_SCREENSHOT_MAX_SIDE: 64, + normalizeBrowserScreenshot: vi.fn(async (buffer: Buffer) => ({ + buffer, + contentType: "image/png", + })), +})); + +vi.mock("../../media/store.js", () => ({ + ensureMediaDir: vi.fn(async () => {}), + saveMediaBuffer: vi.fn(async () => ({ path: "/tmp/fake.png" })), +})); + +vi.mock("./agent.shared.js", () => ({ + getPwAiModule: vi.fn(async () => null), + handleRouteError: vi.fn( + ( + _ctx: unknown, + res: { status: (code: number) => unknown; json: (body: unknown) => void }, + err: unknown, + ) => { + const message = err instanceof Error ? err.message : String(err); + res.status(400); + res.json({ error: message }); + }, + ), + readBody: vi.fn((req: BrowserRequest) => req.body ?? {}), + requirePwAi: vi.fn(async () => null), + resolveProfileContext: vi.fn(() => routeState.profileCtx), + withPlaywrightRouteContext: vi.fn(), + withRouteTabContext: vi.fn(), +})); + +const { registerBrowserAgentSnapshotRoutes } = await import("./agent.snapshot.js"); + +function getSnapshotGetHandler() { + const { app, getHandlers } = createBrowserRouteApp(); + registerBrowserAgentSnapshotRoutes(app, { + state: () => ({ + resolved: { + extraArgs: [], + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }, + }), + } as never); + const handler = getHandlers.get("/snapshot"); + expect(handler).toBeTypeOf("function"); + return handler; +} + +describe("local-managed browser snapshot routes", () => { + beforeEach(() => { + routeState.profileCtx.ensureTabAvailable.mockClear(); + cdpMocks.snapshotAria.mockClear(); + cdpMocks.snapshotRoleViaCdp.mockClear(); + navigationGuardMocks.assertBrowserNavigationResultAllowed.mockClear(); + navigationGuardMocks.withBrowserNavigationPolicy.mockClear(); + }); + + it("blocks ARIA CDP snapshots when the current tab violates browser navigation policy", async () => { + const handler = getSnapshotGetHandler(); + const response = createBrowserRouteResponse(); + + await handler?.({ params: {}, query: { format: "aria" } }, response.res); + + expect(response.statusCode).toBe(400); + expect(response.body).toEqual({ error: "browser navigation blocked by policy" }); + expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledWith({ + url: "http://127.0.0.1:8080/admin", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }); + expect(cdpMocks.snapshotAria).not.toHaveBeenCalled(); + }); + + it("blocks AI CDP role snapshots when the current tab violates browser navigation policy", async () => { + const handler = getSnapshotGetHandler(); + const response = createBrowserRouteResponse(); + + await handler?.({ params: {}, query: { format: "ai", interactive: "true" } }, response.res); + + expect(response.statusCode).toBe(400); + expect(response.body).toEqual({ error: "browser navigation blocked by policy" }); + expect(navigationGuardMocks.assertBrowserNavigationResultAllowed).toHaveBeenCalledWith({ + url: "http://127.0.0.1:8080/admin", + ssrfPolicy: { dangerouslyAllowPrivateNetwork: false }, + }); + expect(cdpMocks.snapshotRoleViaCdp).not.toHaveBeenCalled(); + }); +}); diff --git a/extensions/browser/src/browser/routes/agent.snapshot.ts b/extensions/browser/src/browser/routes/agent.snapshot.ts index 34229358a48..94603a77d76 100644 --- a/extensions/browser/src/browser/routes/agent.snapshot.ts +++ b/extensions/browser/src/browser/routes/agent.snapshot.ts @@ -546,12 +546,20 @@ export function registerBrowserAgentSnapshotRoutes( const tab = await profileCtx.ensureTabAvailable(targetId || undefined); const usesChromeMcp = getBrowserProfileCapabilities(profileCtx.profile).usesChromeMcp; const ssrfPolicyOpts = browserNavigationPolicyForProfile(ctx, profileCtx); - let observedBrowserState: unknown; - if (!usesChromeMcp && pwModule) { + if ((plan.labels || plan.mode === "efficient") && plan.format === "aria") { + return jsonError(res, 400, "labels/mode=efficient require format=ai"); + } + if (usesChromeMcp && (plan.selectorValue || plan.frameSelectorValue)) { + return jsonError(res, 400, EXISTING_SESSION_LIMITS.snapshot.snapshotSelector); + } + if (ssrfPolicyOpts.ssrfPolicy) { await assertBrowserNavigationResultAllowed({ url: tab.url, ...ssrfPolicyOpts, }); + } + let observedBrowserState: unknown; + if (!usesChromeMcp && pwModule) { observedBrowserState = await pwModule .getObservedBrowserStateViaPlaywright({ cdpUrl: profileCtx.profile.cdpUrl, @@ -560,19 +568,7 @@ export function registerBrowserAgentSnapshotRoutes( }) .catch(() => undefined); } - if ((plan.labels || plan.mode === "efficient") && plan.format === "aria") { - return jsonError(res, 400, "labels/mode=efficient require format=ai"); - } if (usesChromeMcp) { - if (plan.selectorValue || plan.frameSelectorValue) { - return jsonError(res, 400, EXISTING_SESSION_LIMITS.snapshot.snapshotSelector); - } - if (ssrfPolicyOpts.ssrfPolicy) { - await assertBrowserNavigationResultAllowed({ - url: tab.url, - ...ssrfPolicyOpts, - }); - } const snapshot = await takeChromeMcpSnapshot({ profileName: profileCtx.profile.name, profile: profileCtx.profile, diff --git a/extensions/browser/src/browser/routes/existing-session.test-support.ts b/extensions/browser/src/browser/routes/existing-session.test-support.ts index 83049920409..bfe22ee278b 100644 --- a/extensions/browser/src/browser/routes/existing-session.test-support.ts +++ b/extensions/browser/src/browser/routes/existing-session.test-support.ts @@ -4,7 +4,7 @@ import { withBrowserNavigationPolicy, } from "../navigation-guard.js"; import type { BrowserRouteContext } from "../server-context.js"; -import type { BrowserRequest } from "./types.js"; +import type { BrowserRequest, BrowserResponse } from "./types.js"; export const existingSessionRouteState = { profileCtx: { @@ -32,7 +32,11 @@ export const existingSessionRouteState = { export function createExistingSessionAgentSharedModule() { return { getPwAiModule: vi.fn(async () => null), - handleRouteError: vi.fn(), + handleRouteError: vi.fn((_ctx: BrowserRouteContext, res: BrowserResponse, err: unknown) => { + const message = err instanceof Error ? err.message : String(err); + res.status(400); + res.json({ error: message }); + }), readBody: vi.fn((req: BrowserRequest) => req.body ?? {}), requirePwAi: vi.fn(async () => { throw new Error("Playwright should not be used for existing-session tests"); diff --git a/src/plugin-activation-boundary.test.ts b/src/plugin-activation-boundary.test.ts index 73a272ecb04..8f371fb9235 100644 --- a/src/plugin-activation-boundary.test.ts +++ b/src/plugin-activation-boundary.test.ts @@ -3,6 +3,22 @@ import { normalizeModelRef } from "./agents/model-selection-normalize.js"; import { isStaticallyChannelConfigured } from "./config/channel-configured-shared.js"; import { parseBrowserMajorVersion } from "./plugin-sdk/browser-host-inspection.js"; +const testModelIdNormalization = { + providers: { + google: { + aliases: { + "gemini-3.1-pro": "gemini-3.1-pro-preview", + "gemini-3-pro-preview": "gemini-3.1-pro-preview", + }, + }, + xai: { + aliases: { + "grok-4-fast-reasoning": "grok-4-fast", + }, + }, + }, +}; + const loadBundledPluginPublicSurfaceModuleSync = vi.hoisted(() => vi.fn((params: { artifactBasename: string }) => { if (params.artifactBasename === "browser-host-inspection.js") { @@ -34,21 +50,7 @@ const loadPluginManifestRegistryForPluginRegistry = vi.hoisted(() => slack: ["SLACK_BOT_TOKEN"], telegram: ["TELEGRAM_BOT_TOKEN"], }, - modelIdNormalization: { - providers: { - google: { - aliases: { - "gemini-3.1-pro": "gemini-3.1-pro-preview", - "gemini-3-pro-preview": "gemini-3.1-pro-preview", - }, - }, - xai: { - aliases: { - "grok-4-fast-reasoning": "grok-4-fast", - }, - }, - }, - }, + modelIdNormalization: testModelIdNormalization, skills: [], hooks: [], origin: "bundled", @@ -137,25 +139,7 @@ describe("plugin activation boundary", () => { expect(isStaticallyChannelConfigured({}, "whatsapp", {})).toBe(false); const staticNormalize = { allowPluginNormalization: false, - manifestPlugins: [ - { - modelIdNormalization: { - providers: { - google: { - aliases: { - "gemini-3.1-pro": "gemini-3.1-pro-preview", - "gemini-3-pro-preview": "gemini-3.1-pro-preview", - }, - }, - xai: { - aliases: { - "grok-4-fast-reasoning": "grok-4-fast", - }, - }, - }, - }, - }, - ], + manifestPlugins: [{ modelIdNormalization: testModelIdNormalization }], }; expect(normalizeModelRef("google", "gemini-3.1-pro", staticNormalize)).toEqual({ provider: "google",