From 78f0bc3ec09742d487c7c30e794fa393ab9c4980 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Tue, 27 Jan 2026 05:00:07 +0000 Subject: [PATCH] fix(browser): gate evaluate behind config flag --- docs/gateway/configuration.md | 2 + docs/gateway/security.md | 3 ++ docs/tools/browser.md | 3 ++ src/agents/clawdbot-gateway-tool.test.ts | 7 +++ src/agents/sandbox/browser.ts | 9 +++- src/agents/sandbox/context.ts | 4 ++ src/agents/skills/config.ts | 1 + src/browser/client-fetch.ts | 10 +++- src/browser/config.ts | 4 ++ src/browser/constants.ts | 1 + src/browser/routes/agent.act.ts | 21 ++++++++ ...-contract-form-layout-act-commands.test.ts | 27 +++++++++++ ...overs-additional-endpoint-branches.test.ts | 7 +++ ...s-open-profile-unknown-returns-404.test.ts | 7 +++ src/cli/browser-cli-inspect.test.ts | 48 +++++++++++++++---- src/config/schema.ts | 1 + src/config/types.browser.ts | 2 + src/config/zod-schema.ts | 1 + src/hooks/config.ts | 1 + src/pairing/pairing-messages.test.ts | 17 ++++++- 20 files changed, 162 insertions(+), 14 deletions(-) diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 3473c1ade..cea4d786d 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -2768,6 +2768,7 @@ scheme/host for profiles that only set `cdpPort`. Defaults: - enabled: `true` +- evaluateEnabled: `true` (set `false` to disable `act:evaluate` and `wait --fn`) - control service: loopback only (port derived from `gateway.port`, default `18791`) - CDP URL: `http://127.0.0.1:18792` (control service + 1, legacy single-profile) - profile color: `#FF4500` (lobster-orange) @@ -2778,6 +2779,7 @@ Defaults: { browser: { enabled: true, + evaluateEnabled: true, // cdpUrl: "http://127.0.0.1:18792", // legacy single-profile override defaultProfile: "chrome", profiles: { diff --git a/docs/gateway/security.md b/docs/gateway/security.md index f83ea3eb6..5f5990b9e 100644 --- a/docs/gateway/security.md +++ b/docs/gateway/security.md @@ -572,6 +572,9 @@ If that browser profile already contains logged-in sessions, the model can access those accounts and data. Treat browser profiles as **sensitive state**: - Prefer a dedicated profile for the agent (the default `clawd` profile). - Avoid pointing the agent at your personal daily-driver profile. +- `act:evaluate` and `wait --fn` run arbitrary JavaScript in the page context. + Prompt injection can steer the model into calling them. If you do not need + them, set `browser.evaluateEnabled=false` (see [Configuration](/gateway/configuration#browser-clawd-managed-browser)). - Keep host browser control disabled for sandboxed agents unless you trust them. - Treat browser downloads as untrusted input; prefer an isolated downloads directory. - Disable browser sync/password managers in the agent profile if possible (reduces blast radius). diff --git a/docs/tools/browser.md b/docs/tools/browser.md index 4b8b7eb00..db63ab93c 100644 --- a/docs/tools/browser.md +++ b/docs/tools/browser.md @@ -505,6 +505,9 @@ These are useful for “make the site behave like X” workflows: ## Security & privacy - The clawd browser profile may contain logged-in sessions; treat it as sensitive. +- `browser act kind=evaluate` / `clawdbot browser evaluate` and `wait --fn` + execute arbitrary JavaScript in the page context. Prompt injection can steer + this. Disable it with `browser.evaluateEnabled=false` if you do not need it. - For logins and anti-bot notes (X/Twitter, etc.), see [Browser login + X/Twitter posting](/tools/browser-login). - Keep the Gateway/node host private (loopback or tailnet-only). - Remote CDP endpoints are powerful; tunnel and protect them. diff --git a/src/agents/clawdbot-gateway-tool.test.ts b/src/agents/clawdbot-gateway-tool.test.ts index b452e9379..76d8ff8e4 100644 --- a/src/agents/clawdbot-gateway-tool.test.ts +++ b/src/agents/clawdbot-gateway-tool.test.ts @@ -20,8 +20,10 @@ describe("gateway tool", () => { vi.useFakeTimers(); const kill = vi.spyOn(process, "kill").mockImplementation(() => true); const previousStateDir = process.env.CLAWDBOT_STATE_DIR; + const previousProfile = process.env.CLAWDBOT_PROFILE; const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "clawdbot-test-")); process.env.CLAWDBOT_STATE_DIR = stateDir; + process.env.CLAWDBOT_PROFILE = "isolated"; try { const tool = createClawdbotTools({ @@ -62,6 +64,11 @@ describe("gateway tool", () => { } else { process.env.CLAWDBOT_STATE_DIR = previousStateDir; } + if (previousProfile === undefined) { + delete process.env.CLAWDBOT_PROFILE; + } else { + process.env.CLAWDBOT_PROFILE = previousProfile; + } } }); diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index cf552e157..4951d9ce5 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -1,6 +1,9 @@ import { startBrowserBridgeServer, stopBrowserBridgeServer } from "../../browser/bridge-server.js"; import { type ResolvedBrowserConfig, resolveProfile } from "../../browser/config.js"; -import { DEFAULT_CLAWD_BROWSER_COLOR } from "../../browser/constants.js"; +import { + DEFAULT_BROWSER_EVALUATE_ENABLED, + DEFAULT_CLAWD_BROWSER_COLOR, +} from "../../browser/constants.js"; import { BROWSER_BRIDGES } from "./browser-bridges.js"; import { DEFAULT_SANDBOX_BROWSER_IMAGE, SANDBOX_AGENT_WORKSPACE_MOUNT } from "./constants.js"; import { @@ -39,10 +42,12 @@ function buildSandboxBrowserResolvedConfig(params: { controlPort: number; cdpPort: number; headless: boolean; + evaluateEnabled: boolean; }): ResolvedBrowserConfig { const cdpHost = "127.0.0.1"; return { enabled: true, + evaluateEnabled: params.evaluateEnabled, controlPort: params.controlPort, cdpProtocol: "http", cdpHost, @@ -76,6 +81,7 @@ export async function ensureSandboxBrowser(params: { workspaceDir: string; agentWorkspaceDir: string; cfg: SandboxConfig; + evaluateEnabled?: boolean; }): Promise { if (!params.cfg.browser.enabled) return null; if (!isToolAllowed(params.cfg.tools, "browser")) return null; @@ -170,6 +176,7 @@ export async function ensureSandboxBrowser(params: { controlPort: 0, cdpPort: mappedCdp, headless: params.cfg.browser.headless, + evaluateEnabled: params.evaluateEnabled ?? DEFAULT_BROWSER_EVALUATE_ENABLED, }), onEnsureAttachTarget, }); diff --git a/src/agents/sandbox/context.ts b/src/agents/sandbox/context.ts index 8e0c004b5..c983ca6df 100644 --- a/src/agents/sandbox/context.ts +++ b/src/agents/sandbox/context.ts @@ -3,6 +3,7 @@ import fs from "node:fs/promises"; import type { ClawdbotConfig } from "../../config/config.js"; import { defaultRuntime } from "../../runtime.js"; import { resolveUserPath } from "../../utils.js"; +import { DEFAULT_BROWSER_EVALUATE_ENABLED } from "../../browser/constants.js"; import { syncSkillsToWorkspace } from "../skills.js"; import { DEFAULT_AGENT_WORKSPACE_DIR } from "../workspace.js"; import { ensureSandboxBrowser } from "./browser.js"; @@ -69,11 +70,14 @@ export async function resolveSandboxContext(params: { cfg, }); + const evaluateEnabled = + params.config?.browser?.evaluateEnabled ?? DEFAULT_BROWSER_EVALUATE_ENABLED; const browser = await ensureSandboxBrowser({ scopeKey, workspaceDir, agentWorkspaceDir, cfg, + evaluateEnabled, }); return { diff --git a/src/agents/skills/config.ts b/src/agents/skills/config.ts index a0dab4125..71b3130a4 100644 --- a/src/agents/skills/config.ts +++ b/src/agents/skills/config.ts @@ -6,6 +6,7 @@ import type { SkillEligibilityContext, SkillEntry } from "./types.js"; const DEFAULT_CONFIG_VALUES: Record = { "browser.enabled": true, + "browser.evaluateEnabled": true, }; function isTruthy(value: unknown): boolean { diff --git a/src/browser/client-fetch.ts b/src/browser/client-fetch.ts index 06facc416..a1efb8f86 100644 --- a/src/browser/client-fetch.ts +++ b/src/browser/client-fetch.ts @@ -14,7 +14,13 @@ function enhanceBrowserFetchError(url: string, err: unknown, timeoutMs: number): ? "If this is a sandboxed session, ensure the sandbox browser is running and try again." : `Start (or restart) the Clawdbot gateway (Clawdbot.app menubar, or \`${formatCliCommand("clawdbot gateway")}\`) and try again.`; const msg = String(err); - if (msg.toLowerCase().includes("timed out") || msg.toLowerCase().includes("timeout")) { + const msgLower = msg.toLowerCase(); + const looksLikeTimeout = + msgLower.includes("timed out") || + msgLower.includes("timeout") || + msgLower.includes("aborted") || + msgLower.includes("abort"); + if (looksLikeTimeout) { return new Error( `Can't reach the clawd browser control service (timed out after ${timeoutMs}ms). ${hint}`, ); @@ -48,7 +54,7 @@ export async function fetchBrowserJson( const timeoutMs = init?.timeoutMs ?? 5000; try { if (isAbsoluteHttp(url)) { - return await fetchHttpJson(url, init ? { ...init, timeoutMs } : { timeoutMs }); + return await fetchHttpJson(url, { ...init, timeoutMs }); } const started = await startBrowserControlServiceFromConfig(); if (!started) { diff --git a/src/browser/config.ts b/src/browser/config.ts index 4f862db51..02ab0e0a1 100644 --- a/src/browser/config.ts +++ b/src/browser/config.ts @@ -8,6 +8,7 @@ import { resolveGatewayPort } from "../config/paths.js"; import { DEFAULT_CLAWD_BROWSER_COLOR, DEFAULT_CLAWD_BROWSER_ENABLED, + DEFAULT_BROWSER_EVALUATE_ENABLED, DEFAULT_BROWSER_DEFAULT_PROFILE_NAME, DEFAULT_CLAWD_BROWSER_PROFILE_NAME, } from "./constants.js"; @@ -15,6 +16,7 @@ import { CDP_PORT_RANGE_START, getUsedPorts } from "./profiles.js"; export type ResolvedBrowserConfig = { enabled: boolean; + evaluateEnabled: boolean; controlPort: number; cdpProtocol: "http" | "https"; cdpHost: string; @@ -140,6 +142,7 @@ export function resolveBrowserConfig( rootConfig?: ClawdbotConfig, ): ResolvedBrowserConfig { const enabled = cfg?.enabled ?? DEFAULT_CLAWD_BROWSER_ENABLED; + const evaluateEnabled = cfg?.evaluateEnabled ?? DEFAULT_BROWSER_EVALUATE_ENABLED; const gatewayPort = resolveGatewayPort(rootConfig); const controlPort = deriveDefaultBrowserControlPort(gatewayPort ?? DEFAULT_BROWSER_CONTROL_PORT); const defaultColor = normalizeHexColor(cfg?.color); @@ -197,6 +200,7 @@ export function resolveBrowserConfig( return { enabled, + evaluateEnabled, controlPort, cdpProtocol, cdpHost: cdpInfo.parsed.hostname, diff --git a/src/browser/constants.ts b/src/browser/constants.ts index 88642a6c5..e06a7dff8 100644 --- a/src/browser/constants.ts +++ b/src/browser/constants.ts @@ -1,4 +1,5 @@ export const DEFAULT_CLAWD_BROWSER_ENABLED = true; +export const DEFAULT_BROWSER_EVALUATE_ENABLED = true; export const DEFAULT_CLAWD_BROWSER_COLOR = "#FF4500"; export const DEFAULT_CLAWD_BROWSER_PROFILE_NAME = "clawd"; export const DEFAULT_BROWSER_DEFAULT_PROFILE_NAME = "chrome"; diff --git a/src/browser/routes/agent.act.ts b/src/browser/routes/agent.act.ts index ae8d274b7..ee1242a4a 100644 --- a/src/browser/routes/agent.act.ts +++ b/src/browser/routes/agent.act.ts @@ -39,6 +39,7 @@ export function registerBrowserAgentActRoutes( const cdpUrl = profileCtx.profile.cdpUrl; const pw = await requirePwAi(res, `act:${kind}`); if (!pw) return; + const evaluateEnabled = ctx.state().resolved.evaluateEnabled; switch (kind) { case "click": { @@ -210,6 +211,16 @@ export function registerBrowserAgentActRoutes( : undefined; const fn = toStringOrEmpty(body.fn) || undefined; const timeoutMs = toNumber(body.timeoutMs) ?? undefined; + if (fn && !evaluateEnabled) { + return jsonError( + res, + 403, + [ + "wait --fn is disabled by config (browser.evaluateEnabled=false).", + "Docs: /gateway/configuration#browser-clawd-managed-browser", + ].join("\n"), + ); + } if ( timeMs === undefined && !text && @@ -240,6 +251,16 @@ export function registerBrowserAgentActRoutes( return res.json({ ok: true, targetId: tab.targetId }); } case "evaluate": { + if (!evaluateEnabled) { + return jsonError( + res, + 403, + [ + "act:evaluate is disabled by config (browser.evaluateEnabled=false).", + "Docs: /gateway/configuration#browser-clawd-managed-browser", + ].join("\n"), + ); + } const fn = toStringOrEmpty(body.fn); if (!fn) return jsonError(res, 400, "fn is required"); const ref = toStringOrEmpty(body.ref) || undefined; diff --git a/src/browser/server.agent-contract-form-layout-act-commands.test.ts b/src/browser/server.agent-contract-form-layout-act-commands.test.ts index 87293a9a3..9276a5280 100644 --- a/src/browser/server.agent-contract-form-layout-act-commands.test.ts +++ b/src/browser/server.agent-contract-form-layout-act-commands.test.ts @@ -7,6 +7,7 @@ let testPort = 0; let cdpBaseUrl = ""; let reachable = false; let cfgAttachOnly = false; +let cfgEvaluateEnabled = true; let createTargetId: string | null = null; let prevGatewayPort: string | undefined; @@ -89,6 +90,7 @@ vi.mock("../config/config.js", async (importOriginal) => { loadConfig: () => ({ browser: { enabled: true, + evaluateEnabled: cfgEvaluateEnabled, color: "#FF4500", attachOnly: cfgAttachOnly, headless: true, @@ -185,6 +187,7 @@ describe("browser control server", () => { beforeEach(async () => { reachable = false; cfgAttachOnly = false; + cfgEvaluateEnabled = true; createTargetId = null; cdpMocks.createTargetViaCdp.mockImplementation(async () => { @@ -349,6 +352,30 @@ describe("browser control server", () => { slowTimeoutMs, ); + it( + "blocks act:evaluate when browser.evaluateEnabled=false", + async () => { + cfgEvaluateEnabled = false; + const base = await startServerAndBase(); + + const waitRes = (await postJson(`${base}/act`, { + kind: "wait", + fn: "() => window.ready === true", + })) as { error?: string }; + expect(waitRes.error).toContain("browser.evaluateEnabled=false"); + expect(pwMocks.waitForViaPlaywright).not.toHaveBeenCalled(); + + const res = (await postJson(`${base}/act`, { + kind: "evaluate", + fn: "() => 1", + })) as { error?: string }; + + expect(res.error).toContain("browser.evaluateEnabled=false"); + expect(pwMocks.evaluateViaPlaywright).not.toHaveBeenCalled(); + }, + slowTimeoutMs, + ); + it("agent contract: hooks + response + downloads + screenshot", async () => { const base = await startServerAndBase(); diff --git a/src/browser/server.covers-additional-endpoint-branches.test.ts b/src/browser/server.covers-additional-endpoint-branches.test.ts index ee5463ab5..948b5984f 100644 --- a/src/browser/server.covers-additional-endpoint-branches.test.ts +++ b/src/browser/server.covers-additional-endpoint-branches.test.ts @@ -308,6 +308,8 @@ describe("backward compatibility (profile parameter)", () => { testPort = await getFreePort(); _cdpBaseUrl = `http://127.0.0.1:${testPort + 1}`; + prevGatewayPort = process.env.CLAWDBOT_GATEWAY_PORT; + process.env.CLAWDBOT_GATEWAY_PORT = String(testPort - 2); vi.stubGlobal( "fetch", @@ -344,6 +346,11 @@ describe("backward compatibility (profile parameter)", () => { afterEach(async () => { vi.unstubAllGlobals(); vi.restoreAllMocks(); + if (prevGatewayPort === undefined) { + delete process.env.CLAWDBOT_GATEWAY_PORT; + } else { + process.env.CLAWDBOT_GATEWAY_PORT = prevGatewayPort; + } const { stopBrowserControlServer } = await import("./server.js"); await stopBrowserControlServer(); }); diff --git a/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts b/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts index f178858a7..1ba1de28c 100644 --- a/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts +++ b/src/browser/server.post-tabs-open-profile-unknown-returns-404.test.ts @@ -285,6 +285,8 @@ describe("profile CRUD endpoints", () => { testPort = await getFreePort(); _cdpBaseUrl = `http://127.0.0.1:${testPort + 1}`; + prevGatewayPort = process.env.CLAWDBOT_GATEWAY_PORT; + process.env.CLAWDBOT_GATEWAY_PORT = String(testPort - 2); vi.stubGlobal( "fetch", @@ -299,6 +301,11 @@ describe("profile CRUD endpoints", () => { afterEach(async () => { vi.unstubAllGlobals(); vi.restoreAllMocks(); + if (prevGatewayPort === undefined) { + delete process.env.CLAWDBOT_GATEWAY_PORT; + } else { + process.env.CLAWDBOT_GATEWAY_PORT = prevGatewayPort; + } const { stopBrowserControlServer } = await import("./server.js"); await stopBrowserControlServer(); }); diff --git a/src/cli/browser-cli-inspect.test.ts b/src/cli/browser-cli-inspect.test.ts index 30111e9e5..3e68de49f 100644 --- a/src/cli/browser-cli-inspect.test.ts +++ b/src/cli/browser-cli-inspect.test.ts @@ -18,6 +18,33 @@ const configMocks = vi.hoisted(() => ({ })); vi.mock("../config/config.js", () => configMocks); +const sharedMocks = vi.hoisted(() => ({ + callBrowserRequest: vi.fn( + async (_opts: unknown, params: { path?: string; query?: Record }) => { + const format = params.query?.format === "aria" ? "aria" : "ai"; + if (format === "aria") { + return { + ok: true, + format: "aria", + targetId: "t1", + url: "https://example.com", + nodes: [], + }; + } + return { + ok: true, + format: "ai", + targetId: "t1", + url: "https://example.com", + snapshot: "ok", + }; + }, + ), +})); +vi.mock("./browser-cli-shared.js", () => ({ + callBrowserRequest: sharedMocks.callBrowserRequest, +})); + const runtime = { log: vi.fn(), error: vi.fn(), @@ -44,13 +71,13 @@ describe("browser cli snapshot defaults", () => { await program.parseAsync(["browser", "snapshot"], { from: "user" }); - expect(clientMocks.browserSnapshot).toHaveBeenCalledWith( - "http://127.0.0.1:18791", - expect.objectContaining({ - format: "ai", - mode: "efficient", - }), - ); + expect(sharedMocks.callBrowserRequest).toHaveBeenCalled(); + const [, params] = sharedMocks.callBrowserRequest.mock.calls.at(-1) ?? []; + expect(params?.path).toBe("/snapshot"); + expect(params?.query).toMatchObject({ + format: "ai", + mode: "efficient", + }); }); it("does not apply config snapshot defaults to aria snapshots", async () => { @@ -71,8 +98,9 @@ describe("browser cli snapshot defaults", () => { await program.parseAsync(["browser", "snapshot", "--format", "aria"], { from: "user" }); - expect(clientMocks.browserSnapshot).toHaveBeenCalled(); - const [, opts] = clientMocks.browserSnapshot.mock.calls.at(-1) ?? []; - expect(opts?.mode).toBeUndefined(); + expect(sharedMocks.callBrowserRequest).toHaveBeenCalled(); + const [, params] = sharedMocks.callBrowserRequest.mock.calls.at(-1) ?? []; + expect(params?.path).toBe("/snapshot"); + expect((params?.query as { mode?: unknown } | undefined)?.mode).toBeUndefined(); }); }); diff --git a/src/config/schema.ts b/src/config/schema.ts index f05ac277c..69846236e 100644 --- a/src/config/schema.ts +++ b/src/config/schema.ts @@ -279,6 +279,7 @@ const FIELD_LABELS: Record = { "ui.seamColor": "Accent Color", "ui.assistant.name": "Assistant Name", "ui.assistant.avatar": "Assistant Avatar", + "browser.evaluateEnabled": "Browser Evaluate Enabled", "browser.snapshotDefaults": "Browser Snapshot Defaults", "browser.snapshotDefaults.mode": "Browser Snapshot Mode", "browser.remoteCdpTimeoutMs": "Remote CDP Timeout (ms)", diff --git a/src/config/types.browser.ts b/src/config/types.browser.ts index cbd006589..d8678b80b 100644 --- a/src/config/types.browser.ts +++ b/src/config/types.browser.ts @@ -14,6 +14,8 @@ export type BrowserSnapshotDefaults = { }; export type BrowserConfig = { enabled?: boolean; + /** If false, disable browser act:evaluate (arbitrary JS). Default: true */ + evaluateEnabled?: boolean; /** Base URL of the CDP endpoint (for remote browsers). Default: loopback CDP on the derived port. */ cdpUrl?: string; /** Remote CDP HTTP timeout (ms). Default: 1500. */ diff --git a/src/config/zod-schema.ts b/src/config/zod-schema.ts index 75f438c1e..3c83d2c09 100644 --- a/src/config/zod-schema.ts +++ b/src/config/zod-schema.ts @@ -134,6 +134,7 @@ export const ClawdbotSchema = z browser: z .object({ enabled: z.boolean().optional(), + evaluateEnabled: z.boolean().optional(), cdpUrl: z.string().optional(), remoteCdpTimeoutMs: z.number().int().nonnegative().optional(), remoteCdpHandshakeTimeoutMs: z.number().int().nonnegative().optional(), diff --git a/src/hooks/config.ts b/src/hooks/config.ts index e290e6b5d..ee2acc07e 100644 --- a/src/hooks/config.ts +++ b/src/hooks/config.ts @@ -6,6 +6,7 @@ import type { HookEligibilityContext, HookEntry } from "./types.js"; const DEFAULT_CONFIG_VALUES: Record = { "browser.enabled": true, + "browser.evaluateEnabled": true, "workspace.dir": true, }; diff --git a/src/pairing/pairing-messages.test.ts b/src/pairing/pairing-messages.test.ts index 581d405d3..416a6ab19 100644 --- a/src/pairing/pairing-messages.test.ts +++ b/src/pairing/pairing-messages.test.ts @@ -1,8 +1,23 @@ -import { describe, expect, it } from "vitest"; +import { afterEach, beforeEach, describe, expect, it } from "vitest"; import { buildPairingReply } from "./pairing-messages.js"; describe("buildPairingReply", () => { + let previousProfile: string | undefined; + + beforeEach(() => { + previousProfile = process.env.CLAWDBOT_PROFILE; + process.env.CLAWDBOT_PROFILE = "isolated"; + }); + + afterEach(() => { + if (previousProfile === undefined) { + delete process.env.CLAWDBOT_PROFILE; + return; + } + process.env.CLAWDBOT_PROFILE = previousProfile; + }); + const cases = [ { channel: "discord",