Skip to content

fix(browser): migrate snapshot/vision private-URL checks onto shared helper - #482

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56583
Open

fix(browser): migrate snapshot/vision private-URL checks onto shared helper#482
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56583

Conversation

@hashbender

Copy link
Copy Markdown
Owner

Closes NousResearch#56579

Problem

browser_snapshot and browser_vision used a weaker SSRF floor than the shared _current_page_private_url helper. Their inline checks only tested:

if _current_url and not _is_safe_url(_current_url):

This misses the _is_always_blocked_url() predicate, which covers cloud-metadata addresses (169.254.169.254, 169.254.169.253, 100.100.100.200, metadata.google.internal). If _is_safe_url were to return True for one of those addresses, the guard would silently pass and expose private page content via snapshot or vision.

The shared _current_page_private_url helper — already used correctly by browser_back, browser_console, and browser_get_images — applies both predicates:

_is_always_blocked_url(current_url) or not _is_safe_url(current_url)

Fix

Add _is_always_blocked_url(_current_url) to the condition in both inline guards so they match the shared helper:

# browser_snapshot (~L2973) and browser_vision (~L3901)
if _current_url and (
    _is_always_blocked_url(_current_url) or not _is_safe_url(_current_url)
):

Two lines changed in browser_tool.py.

Tests

Added TestSnapshotBlocksCloudMetadataViaAlwaysBlocked and TestVisionBlocksCloudMetadataViaAlwaysBlocked in tests/tools/test_browser_snapshot_ssrf.py. Each test patches _is_safe_url to return True (simulating the gap) while _is_always_blocked_url correctly identifies 169.254.169.254 — and asserts both functions still block the request.


Mirror-of: NousResearch#56583
NousResearch#56583

@tenki-reviewer

tenki-reviewer Bot commented Jul 1, 2026

Copy link
Copy Markdown

Review Complete
No issues found!

Risk: 🟠 High (72/100) — no findings · 102 LOC across 2 files


The _is_always_blocked_url SSRF floor is conditionally gated in browser_snapshot and browser_vision, creating a bypass on cloud VMs with local Chromium backends.

Files Reviewed (2 files)
tests/tools/test_browser_snapshot_ssrf.py
tools/browser_tool.py

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

browser_snapshot/browser_vision use a weaker private-page floor than the shared _current_page_private_url helper

1 participant