fix(browser): block unsafe code patterns in browser evaluate by yubrew · Pull Request #8818 · openclaw/openclaw

yubrew · 2026-02-04T12:35:38Z

Summary

Adds code validation to evaluateViaPlaywright to block dangerous patterns that could be used for data exfiltration or code injection.

The Problem

The browser automation evaluate function executes user-provided JavaScript code without any validation. An attacker controlling the code (via prompt injection) could use APIs like fetch(), WebSocket, or navigator.sendBeacon() to exfiltrate cookies, credentials, and sensitive data from the browser context.

Reference: CWE-95: Improper Neutralization of Directives in Dynamically Evaluated Code

Changes

src/browser/pw-evaluate-validation.ts: New validation module with blocklist of dangerous patterns
src/browser/pw-tools-core.interactions.ts: Add validation call before code execution
src/browser/pw-evaluate-validation.test.ts: Unit tests for validation logic (29 tests)
src/browser/pw-tools-core.evaluate-blocks-unsafe.test.ts: Integration tests (13 tests)

Blocked patterns:

Data exfiltration: fetch, XMLHttpRequest, WebSocket, sendBeacon
Code execution: eval, new Function, setTimeout/setInterval with strings
Module loading: dynamic import, importScripts

Test Plan

pnpm build && pnpm check && pnpm test passes
29 unit tests validate blocking of unsafe patterns and allowing of safe operations
13 integration tests confirm evaluateViaPlaywright rejects unsafe code
Validation occurs before page access (early rejection)

Greptile Overview

Greptile Summary

Adds a pre-execution validation step for evaluateViaPlaywright to reject browser-evaluated code containing a set of blocked “dangerous” patterns (exfiltration APIs, dynamic code execution, and module loading). This is implemented via a new pw-evaluate-validation.ts module and covered by unit tests for the validator plus integration tests that ensure unsafe fn strings are rejected before interacting with a Playwright page.

Confidence Score: 3/5

This PR is directionally correct but has a couple of behavioral and security-footgun edge cases worth addressing before merge.
The validation is a simple regex blocklist and the tests are extensive, but (1) blocking any eval( token is likely to break legitimate evaluate snippets given this function always uses eval internally, and (2) the error message currently echoes user code which can leak sensitive data into logs. No other obvious correctness issues were found in the touched paths.
src/browser/pw-evaluate-validation.ts, src/browser/pw-tools-core.interactions.ts

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-04T12:45:41Z

src/browser/pw-evaluate-validation.ts

+  { pattern: /\bsendBeacon\s*\(/i, name: "sendBeacon" },
+  { pattern: /\bnavigator\s*\.\s*sendBeacon/i, name: "navigator.sendBeacon" },
+


[P1] sendBeacon detection can miss common invocations due to missing \(

In BLOCKED_PATTERNS, the navigator.sendBeacon entry is /\bnavigator\s*\.\s*sendBeacon/i (no \s*\(), so code like navigator.sendBeacon('https://…') won’t match this specific rule. You currently rely on the separate /\bsendBeacon\s*\(/i rule to catch it, but that allows trivial bypasses like navigator['sendBeacon']('…') (won’t hit either rule), and it also means the reported pattern name may be misleading ("sendBeacon" vs "navigator.sendBeacon"). Consider aligning the navigator.sendBeacon rule with the comment’s intent (function call detection) or removing it if redundant.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/browser/pw-evaluate-validation.ts Line: 35:37 Comment: [P1] `sendBeacon` detection can miss common invocations due to missing `\(` In `BLOCKED_PATTERNS`, the `navigator.sendBeacon` entry is `/\bnavigator\s*\.\s*sendBeacon/i` (no `\s*\(`), so code like `navigator.sendBeacon('https://…')` won’t match this specific rule. You currently rely on the separate `/\bsendBeacon\s*\(/i` rule to catch it, but that allows trivial bypasses like `navigator['sendBeacon']('…')` (won’t hit either rule), and it also means the reported pattern name may be misleading ("sendBeacon" vs "navigator.sendBeacon"). Consider aligning the `navigator.sendBeacon` rule with the comment’s intent (function call detection) or removing it if redundant. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-04T12:45:47Z

Additional Comments (1)

src/browser/pw-tools-core.interactions.ts
[P2] New validation rejects any fetch(/eval(/etc. substring, even inside string literals/comments

assertSafeEvaluateCode(fnText) runs on the raw text and the validators intentionally can’t distinguish code vs. strings/comments. This means previously-valid snippets like () => "call fetch('…')" or docs/examples embedded in strings will now hard-fail, which could be a breaking behavior change for callers of act:evaluate (see src/browser/routes/agent.act.ts:293). If that’s intended, it may be worth documenting at the API boundary or adjusting tests to explicitly cover/acknowledge the breaking case.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/browser/pw-tools-core.interactions.ts
Line: 226:232

Comment:
[P2] New validation rejects any `fetch(`/`eval(`/etc. substring, even inside string literals/comments

`assertSafeEvaluateCode(fnText)` runs on the raw text and the validators intentionally can’t distinguish code vs. strings/comments. This means previously-valid snippets like `() => "call fetch('…')"` or docs/examples embedded in strings will now hard-fail, which could be a breaking behavior change for callers of `act:evaluate` (see `src/browser/routes/agent.act.ts:293`). If that’s intended, it may be worth documenting at the API boundary or adjusting tests to explicitly cover/acknowledge the breaking case.

How can I resolve this? If you propose a fix, please make it concise.

greptile-apps

_{2 files reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

greptile-apps · 2026-02-04T12:50:31Z

src/browser/pw-evaluate-validation.ts

+  constructor(pattern: string, code: string) {
+    const truncated = code.length > 50 ? code.slice(0, 50) + "..." : code;
+    super(
+      `Unsafe browser code blocked: Contains "${pattern}" which is not allowed for security reasons. ` +
+        `Code preview: ${truncated}`,
+    );


[P1] Error message includes unescaped user code preview, which can leak secrets and/or enable log injection.

UnsafeEvaluateCodeError includes a Code preview: ${truncated} string built directly from the user-supplied code (pw-evaluate-validation.ts:14-19). If callers log this error (or bubble it to users), the preview can contain credentials/cookies/PII and can include newlines/control characters that make logs misleading. Consider redacting obvious sensitive fields and/or sanitizing control characters before embedding the preview.

Prompt To Fix With AI

This is a comment left during a code review. Path: src/browser/pw-evaluate-validation.ts Line: 14:19 Comment: [P1] Error message includes unescaped user code preview, which can leak secrets and/or enable log injection. `UnsafeEvaluateCodeError` includes a `Code preview: ${truncated}` string built directly from the user-supplied `code` (`pw-evaluate-validation.ts:14-19`). If callers log this error (or bubble it to users), the preview can contain credentials/cookies/PII and can include newlines/control characters that make logs misleading. Consider redacting obvious sensitive fields and/or sanitizing control characters before embedding the preview. How can I resolve this? If you propose a fix, please make it concise.

greptile-apps · 2026-02-04T12:50:35Z

Additional Comments (1)

src/browser/pw-tools-core.interactions.ts
[P0] evaluateViaPlaywright now rejects all uses of eval(, which breaks the intended “function string” input format.

fnText is expected to look like (el) => ... or () => ..., and the implementation always runs eval("(" + fnBody + ")") in the browser context (see pw-tools-core.interactions.ts:246 and :263). With the new validator, any user-provided function body containing the substring eval( (even as a method name, variable, comment, or string literal) will be blocked, which is a behavioral breaking change for legitimate evaluate code (e.g. (el) => el.dataset.eval(...), obj.eval(...), or even string content).

If the goal is to prevent the dangerous runtime APIs (fetch, WebSocket, etc.), consider narrowing the eval pattern to the “direct eval call” in user code only, or explicitly document that any appearance of these tokens anywhere (including strings/comments) is disallowed.

Prompt To Fix With AI

This is a comment left during a code review.
Path: src/browser/pw-tools-core.interactions.ts
Line: 226:231

Comment:
[P0] `evaluateViaPlaywright` now rejects *all* uses of `eval(`, which breaks the intended “function string” input format.

`fnText` is expected to look like `(el) => ...` or `() => ...`, and the implementation always runs `eval("(" + fnBody + ")")` in the browser context (see `pw-tools-core.interactions.ts:246` and `:263`). With the new validator, any user-provided function body containing the substring `eval(` (even as a method name, variable, comment, or string literal) will be blocked, which is a behavioral breaking change for legitimate evaluate code (e.g. `(el) => el.dataset.eval(...)`, `obj.eval(...)`, or even string content).

If the goal is to prevent the *dangerous runtime APIs* (`fetch`, `WebSocket`, etc.), consider narrowing the `eval` pattern to the “direct eval call” in user code only, or explicitly document that any appearance of these tokens anywhere (including strings/comments) is disallowed.

How can I resolve this? If you propose a fix, please make it concise.

Add code validation to `evaluateViaPlaywright` to block dangerous patterns that could be used for data exfiltration or code injection. An attacker controlling the code (via prompt injection) could use APIs like `fetch()`, `WebSocket`, or `navigator.sendBeacon()` to exfiltrate cookies, credentials, and sensitive data from the browser context. Blocked patterns include: - Data exfiltration: fetch, XMLHttpRequest, WebSocket, sendBeacon - Code execution: eval, new Function, setTimeout/setInterval with strings - Module loading: dynamic import, importScripts Reference: CWE-95 (Improper Neutralization of Directives in Dynamically Evaluated Code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

openclaw-barnacle · 2026-02-21T04:41:58Z

This pull request has been automatically marked as stale due to inactivity.
Please add updates or it will be closed.

openclaw-barnacle · 2026-03-06T18:55:54Z

Closing this PR because the author has more than 10 active PRs in this repo. Please reduce the active PR queue and reopen or resubmit once it is back under the limit. You can close your own PRs to get back under the limit.

greptile-apps bot reviewed Feb 4, 2026

View reviewed changes

yubrew force-pushed the fix/vuln-037-unsafe-eval-browser branch from 42532ec to b6c9024 Compare February 4, 2026 13:22

Reapor-Yurnero mentioned this pull request Feb 9, 2026

feat(gateway): support modular guardrails extensions for securing against indirect prompt injections and other agentic threats #6095

Closed

sebslight mentioned this pull request Feb 13, 2026

fix(browser): add validation for browser evaluate code #8788

Closed

4 tasks

thewilloftheshadow force-pushed the main branch from bfc1ccb to f92900f Compare February 15, 2026 18:46

openclaw-barnacle bot added stale Marked as stale due to inactivity and removed stale Marked as stale due to inactivity labels Feb 21, 2026

This comment was marked as spam.

Sign in to view

thewilloftheshadow added the r: too-many-prs label Mar 6, 2026

openclaw-barnacle bot closed this Mar 6, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(browser): block unsafe code patterns in browser evaluate#8818

fix(browser): block unsafe code patterns in browser evaluate#8818
yubrew wants to merge 1 commit intoopenclaw:mainfrom
yubrew:fix/vuln-037-unsafe-eval-browser

yubrew commented Feb 4, 2026 •

edited by greptile-apps bot

Loading

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 4, 2026

Uh oh!

greptile-apps bot commented Feb 4, 2026

Uh oh!

greptile-apps bot left a comment

Uh oh!

greptile-apps bot Feb 4, 2026

Uh oh!

greptile-apps bot commented Feb 4, 2026

Uh oh!

openclaw-barnacle bot commented Feb 21, 2026

Uh oh!

This comment was marked as spam.

openclaw-barnacle bot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		{ pattern: /\bsendBeacon\s*\(/i, name: "sendBeacon" },
		{ pattern: /\bnavigator\s\.\ssendBeacon/i, name: "navigator.sendBeacon" },

Uh oh!

Conversation

yubrew commented Feb 4, 2026 • edited by greptile-apps bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

The Problem

Changes

Test Plan

Related

Greptile Overview

Greptile Summary

Confidence Score: 3/5

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 4, 2026

Uh oh!

greptile-apps bot left a comment

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot Feb 4, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps bot commented Feb 4, 2026

Uh oh!

openclaw-barnacle bot commented Feb 21, 2026

Uh oh!

This comment was marked as spam.

openclaw-barnacle bot commented Mar 6, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yubrew commented Feb 4, 2026 •

edited by greptile-apps bot

Loading