fix(browser): allow local sidecar sessions to bypass SSRF guard - #45133
Closed
liuhao1024 wants to merge 3 commits into
Closed
fix(browser): allow local sidecar sessions to bypass SSRF guard#45133liuhao1024 wants to merge 3 commits into
liuhao1024 wants to merge 3 commits into
Conversation
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
The SSRF bypass in NousResearch#44731 was only patched for browser_snapshot(), but browser_vision() exposes the same vulnerability — it takes a screenshot and sends it to the vision model without checking if eval-driven navigation moved the page to a private/internal URL. Add the same current-page URL safety check to browser_vision() before any screenshot is captured, encoded, or forwarded to the vision model. This covers both the normal screenshot path and the Lightpanda Chrome fallback path. 7 new tests: blocks private URL, allows public URL, skips in local backend, skips when private URLs allowed, handles eval failure/empty/exception.
The private-network guard in browser_snapshot() and browser_vision() blocked all private URLs, including those accessed via local sidecar sessions (hybrid routing). Local sidecar sessions intentionally access private URLs — the cloud provider never sees the URL in that case. Add `_is_local_sidecar_key(effective_task_id)` check to both guards, matching the existing pattern in browser_navigate(). Fixes NousResearch#45101 review feedback from egilewski.
13 tasks
alt-glitch
added
type/bug
Contributor
|
Merged via PR #54132 (rebase-merged). Your three commits were cherry-picked onto current main with your authorship preserved in git log (7a6fe9b, 48f5c42, 0ae6196). We added a sibling commit on top that closes the eval return-value path (direct private fetch + navigate-then-read via browser_console), so the SSRF guard now covers navigate/snapshot/vision/eval. Thanks for the snapshot+vision fix and for iterating through the local-sidecar feedback. |
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Extends the private-network guard in
browser_snapshot()andbrowser_vision()to allow local sidecar sessions (hybrid routing) to access private URLs. The previous implementation blocked all private URLs, including those accessed via local sidecar sessions where the cloud provider never sees the URL.Related Issue
Fixes #44731 (original SSRF bypass issue)
Addresses review feedback from egilewski on #45101
Type of Change
Changes Made
tools/browser_tool.py: Add_is_local_sidecar_key(effective_task_id)check to bothbrowser_snapshot()andbrowser_vision()private-network guards, matching the existing pattern inbrowser_navigate()tests/tools/test_browser_snapshot_ssrf.py: Addtest_skips_check_for_local_sidecar_sessiontests for both snapshot and vision guardsHow to Test
pytest tests/tools/test_browser_snapshot_ssrf.py -vpytest tests/tools/test_browser_ssrf_local.py -vhttp://192.168.1.1/admin) without being blockedallow_private_urlsis enabledChecklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/ACode Intelligence
tools/browser_tool.py(browser_snapshot, browser_vision, _is_local_sidecar_key)