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

[naterchrdsn]

Closes #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.

[alt-glitch]

This was generated by AI during triage.

Related: fixes #56579 (snapshot/vision private-URL floor parity), part of the browser SSRF cluster #54132 / #56173. This is a defense-in-depth consistency fix — is_safe_url() already blocks cloud-metadata IPs unconditionally on main, so the weaker inline guards still blocked IMDS; this closes the redundancy gap. Not a duplicate of the cluster anchors (distinct helper-parity change).

[teknium1]

Thanks for preserving the cloud-metadata floor across the snapshot and vision paths. The live bypass described in the body is not reproducible on current main: is_safe_url() already rejects metadata hostnames and always-blocked IPs regardless of the private-URL toggle (tools/url_safety.py:405-441). This is therefore a useful defense-in-depth consistency change.

Problems

• The patch does not use the shared helper it identifies. The changed guards at tools/browser_tool.py:2973 and tools/browser_tool.py:3941 still duplicate the URL probe and predicate already centralized in _current_page_private_url() at tools/browser_tool.py:3387-3412. Future changes to that helper can still drift from these two paths.

Suggested changes

• Route both guards through _eval_ssrf_guard_active(effective_task_id) and _current_page_private_url(effective_task_id), preserving their current error payloads. The proposed regression tests can remain to verify the helper's always-blocked branch.

Automated hermes-sweeper review.

[teknium1]

This still duplicates _current_page_private_url() rather than migrating to it. Please have both snapshot and vision call the shared helper after _eval_ssrf_guard_active(effective_task_id) so the probe, always-blocked floor, and future changes have one owner.

[naterchrdsn]

Follow-up on the review: rebased onto current main and updated the existing branch at 1da815d67.

• browser_snapshot and browser_vision now delegate through _eval_ssrf_guard_active() and _current_page_private_url() while preserving their prior response payloads and placement.
• Added regressions that assert both shared helpers are invoked for cloud-metadata pages.

Verification: uv run --extra dev pytest tests/tools/test_browser_snapshot_ssrf.py -q — 21 passed; Ruff and git diff --check clean.