Skip to content

fix(browser): block private-page interactions after eval navigation - #55949

Closed
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/browser-private-page-action-guard
Closed

fix(browser): block private-page interactions after eval navigation#55949
necoweb3 wants to merge 1 commit into
NousResearch:mainfrom
necoweb3:fix/browser-private-page-action-guard

Conversation

@necoweb3

Copy link
Copy Markdown
Contributor

Summary

This blocks state-changing browser interactions when a cloud browser page has landed on a private/internal URL after JavaScript-driven navigation.

Why

Recent browser SSRF hardening covers content-returning paths such as browser_snapshot, browser_vision, browser_console, and browser_get_images. Those paths re-check the current page URL after eval-driven navigation and refuse to return private-network content.

However, interaction tools did not have the same sibling guard. If a page was navigated to a private/internal URL through an eval-driven path, browser_click, browser_type, and browser_press would still send input to that page. In cloud browser mode, that crosses the same private-network boundary, but as a side-effect primitive rather than a read primitive.

Changes

  • Add a shared private-page action guard in tools/browser_tool.py.
  • Refuse browser_click, browser_type, and browser_press when the current cloud browser page is private/internal.
  • Reuse the existing _eval_ssrf_guard_active() and _current_page_private_url() checks.
  • Leave local/Camofox behavior unchanged.
  • Leave non-state-changing navigation helpers such as back and scroll unchanged.

Tests

python -m pytest tests/tools/test_browser_private_page_action_guard.py -q --timeout-method=thread
4 passed

python -m pytest tests/tools/test_browser_eval_ssrf.py tests/tools/test_browser_get_images_ssrf.py tests/tools/test_browser_snapshot_ssrf.py -q --timeout-method=thread
34 passed

@alt-glitch alt-glitch added type/security Security vulnerability or hardening tool/browser Browser automation (CDP, Playwright) sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data P1 High — major feature broken, no workaround labels Jun 30, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #54132 (merged cluster anchor: snapshot/vision/eval guards for #44731), #54435 (merged: get_images sink), #54477 (open: console-output sink), #54187 (closed predecessor). This PR seals the remaining STATE-CHANGING interaction sinks (browser_click/browser_type/browser_press) that still sent input to an eval-navigated private page in cloud-browser mode. Distinct sink (side-effect primitives, not read-back), so related to the cluster rather than a duplicate.

@egilewski

Copy link
Copy Markdown
Contributor

looks mergeable

Security evidence:

  • trust boundary: this is the cloud browser/private-network boundary after eval-driven navigation, where the page can land on a private/internal URL without going through browser_navigate().
  • source/sink/invariant: browser_click, browser_type, and browser_press are side-effecting sinks and must not send input to a cloud browser page whose current URL is private/internal.
  • current-main reproduction: on current main, I reproduced that synthetic private-page state still lets click, fill, and press reach _run_browser_command; the type path also returned the typed sentinel text.
  • PR-head or patch-replay validation: on the PR head, the same private-page state blocks click, type, and press before _run_browser_command, and the typed sentinel is absent from the result.
  • positive/negative cases: the new focused tests cover all three blocked actions and the public-page click case, and the existing eval, snapshot, and get_images SSRF tests still pass.
  • residual bypass search: I checked the sibling browser tools; the remaining active-session commands are either intentionally non-state-changing navigation helpers (scroll, back) or already have read/eval SSRF guards on current main.
  • reviewer validation: CodeRabbit completed with no findings in the clean-pass flow.

The patch is focused, merges cleanly into current main, preserves the existing local/Camofox/private-URL opt-out behavior through _eval_ssrf_guard_active(), and adds regression coverage for the side-effecting action paths.

Signed: GPT-5.5-xhigh in Codex

@kshitijk4poor

Copy link
Copy Markdown
Collaborator

Superseded by #56173, which carries your fix commit verbatim (authorship preserved) rebased onto current main (this PR was 67 commits behind), plus a small review follow-up:

  • added a guard-inactive negative test (local backend / allow_private_urls must proceed without probing the page URL) — the branch most likely to silently regress; a mutation check confirms it catches an inverted condition
  • added a camofox short-circuit test
  • PEP8 blank-line fix before the helper

Full credit to you for the original fix. Closing in favor of #56173 — feel free to push back if you'd prefer to carry the follow-up on this branch instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

P1 High — major feature broken, no workaround sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data tool/browser Browser automation (CDP, Playwright) type/security Security vulnerability or hardening

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants