Skip to content

fix(browser): block snapshot from eval-navigated private pages - #44755

Closed
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/browser-eval-private-network-bypass
Closed

fix(browser): block snapshot from eval-navigated private pages#44755
liuhao1024 wants to merge 1 commit into
NousResearch:mainfrom
liuhao1024:fix/browser-eval-private-network-bypass

Conversation

@liuhao1024

@liuhao1024 liuhao1024 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds a private-network URL check in browser_snapshot() to block content exposure when browser_console() has navigated the page to a private/internal address via JavaScript eval.

Related Issue

Fixes #44731

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)

Changes Made

  • tools/browser_tool.py: Added URL safety check in browser_snapshot() that evaluates window.location.href after getting the snapshot and blocks if the URL targets a private/internal address (30 lines)
  • tests/tools/test_browser_snapshot_ssrf.py: Added 9 tests covering the new private-network guard (248 lines)

How to Test

  1. Run the new tests: pytest tests/tools/test_browser_snapshot_ssrf.py -v
  2. Run existing browser SSRF tests: pytest tests/tools/test_browser_ssrf_local.py -v
  3. Run browser eval supervisor tests: pytest tests/tools/test_browser_eval_supervisor_path.py -v
  4. Verify all 46 tests pass

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(scope):, feat(scope):, etc.)
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix/feature (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes (required for bug fixes, strongly encouraged for features)
  • I've tested on my platform: macOS

Documentation & Housekeeping

  • I've updated relevant documentation (README, docs/, docstrings) — or N/A
  • I've updated cli-config.yaml.example if I added/changed config keys — or N/A
  • I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
  • I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
  • I've updated tool descriptions/schemas if I changed tool behavior — or N/A

Code Intelligence

  • Analyzed: tools/browser_tool.py::browser_snapshot (callers: N/A, flows: snapshot → eval → URL check → block/allow)
  • Blast radius: LOW — only affects browser_snapshot() in cloud mode, no changes to navigation or eval behavior
  • Related patterns: Uses existing _is_safe_url(), _allow_private_urls(), _is_local_backend() infrastructure from tools/url_safety.py

@liuhao1024
fix(browser): block snapshot from eval-navigated private pages
36f791d 
browser_snapshot() now checks the current page URL before returning
content. When browser_console() changes location.href to a private or
internal address (e.g., http://127.0.0.1:8080/), the snapshot returns
an error instead of exposing the private page content.

This closes the SSRF bypass where an attacker could:
1. Navigate to a public page
2. Use browser_console to eval location.href = 'http://127.0.0.1:port/'
3. Use browser_snapshot to read the private page content

The fix reuses the existing _is_safe_url() and _allow_private_urls()
infrastructure, and fails open if the URL check itself fails.

Fixes NousResearch#44731
@alt-glitch alt-glitch added type/security Security vulnerability or hardening P1 High — major feature broken, no workaround comp/tools Tool registry, model_tools, toolsets tool/browser Browser automation (CDP, Playwright) labels Jun 12, 2026
@liuhao1024 liuhao1024 mentioned this pull request Jun 12, 2026
Closed
egilewski
egilewski suggested changes Jun 12, 2026
View reviewed changes

@egilewski egilewski left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Recommendation: request changes

I reviewed this in security mode against current GitHub main 46d758bb3e0709bef51b7e3416cfb25da95d2335, PR base 906bee9cf7917326bc41d2df559647ec14c4ee7d, and PR head 36f791de89281e95cf693b33d7c54c78d175bfd5.

Validation:

  • git rev-list --left-right --count upstream/main...refs/remotes/upstream/pr/44755: 30 1.
  • git merge-tree --write-tree upstream/main refs/remotes/upstream/pr/44755: passed, tree 5c10b0836897a120824fbcf1e3e8e84bdd480952.
  • git diff --check upstream/main...refs/remotes/upstream/pr/44755: passed.
  • python -B -m pytest -q tests/tools/test_browser_snapshot_ssrf.py tests/tools/test_browser_ssrf_local.py tests/tools/test_browser_eval_supervisor_path.py -p no:cacheprovider: passed, 46 passed.
  • python -B -m py_compile tools/browser_tool.py tests/tools/test_browser_snapshot_ssrf.py: passed.
  • Synthetic current-main probe: mocked an active browser page at http://127.0.0.1:8080/secret; browser_snapshot() returned {"success": true, "snapshot": "PRIVATE_SECRET_FROM_127001"}.
  • Same synthetic PR-head probe: browser_snapshot() returned {"success": false, "error": "Blocked: page URL targets a private or internal address ..."}.

Finding:
The fix is still incomplete because it protects only browser_snapshot(). After the same eval-driven private-page navigation, browser_vision() remains a browser content sink: it calls _run_browser_command(..., "screenshot", ...), reads the screenshot, and either attaches it to the active model or sends it to the vision model without re-checking window.location.href first (tools/browser_tool.py around the screenshot and call_llm path). A PR-head synthetic probe with _is_local_backend() == False, _allow_private_urls() == False, and the current page URL mocked as http://127.0.0.1:8080/secret executed only screenshot and returned {"success": true, "analysis": "PRIVATE_SECRET_FROM_127001", ...}. That leaves the same eval-navigation private-network policy bypass available by calling browser_vision() instead of browser_snapshot(), so the security invariant is not fully restored.

Please add the same current-page private/internal URL guard before any browser_vision() screenshot is captured, encoded, attached, or sent to an auxiliary vision model, including the Lightpanda Chrome fallback path.

Signed: GPT-5.5-xhigh in Codex

@liuhao1024 liuhao1024 mentioned this pull request Jun 12, 2026
Closed
13 tasks
@liuhao1024

liuhao1024 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor Author

Superseded by #45101, which adds the same private-network guard to browser_vision() as requested in the CHANGES_REQUESTED review. The new PR includes the original browser_snapshot() fix plus the browser_vision() extension with 7 additional tests.

@liuhao1024 liuhao1024 closed this Jun 12, 2026
@teknium1 teknium1 mentioned this pull request Jun 28, 2026
Merged
@teknium1

teknium1 commented Jun 28, 2026

Copy link
Copy Markdown
Contributor

Superseded by #45133, which was merged into current main via PR #54132 (your authorship preserved). The final fix also closes the eval return-value path. Thanks!

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

Reviewers

1 more reviewer
@egilewski egilewski egilewski requested changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

comp/tools Tool registry, model_tools, toolsets P1 High — major feature broken, no workaround tool/browser Browser automation (CDP, Playwright) type/security Security vulnerability or hardening

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Security] Hermes browser private-network policy bypass via eval-triggered main-frame navigation

4 participants

@liuhao1024 @teknium1 @egilewski @alt-glitch