From 48f5c42599edcbcf1f5a0c146dfe8d47f3d9e7b2 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sat, 13 Jun 2026 01:24:45 +0800 Subject: [PATCH] fix(browser): extend private-network guard to browser_vision MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SSRF bypass in #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. --- tests/tools/test_browser_snapshot_ssrf.py | 148 ++++++++++++++++++++++ tools/browser_tool.py | 30 +++++ 2 files changed, 178 insertions(+) diff --git a/tests/tools/test_browser_snapshot_ssrf.py b/tests/tools/test_browser_snapshot_ssrf.py index 8f500d9973b..9cb29d9c498 100644 --- a/tests/tools/test_browser_snapshot_ssrf.py +++ b/tests/tools/test_browser_snapshot_ssrf.py @@ -246,3 +246,151 @@ class TestBrowserSnapshotPrivateNetworkGuard: def browser_browser_snapshot(**kwargs): from tools.browser_tool import browser_snapshot return browser_snapshot(**kwargs) + + +def browser_browser_vision(**kwargs): + from tools.browser_tool import browser_vision + return browser_vision(**kwargs) + + +def _make_screenshot_result(path="/tmp/test_screenshot.png"): + """Return a mock successful screenshot result.""" + return {"success": True, "data": {"path": path}} + + +# --------------------------------------------------------------------------- +# browser_vision: private-network guard after eval navigation +# --------------------------------------------------------------------------- + + +class TestBrowserVisionPrivateNetworkGuard: + """browser_vision must block screenshots from private pages navigated via eval.""" + + PRIVATE_URL = "http://127.0.0.1:8080/secret" + PUBLIC_URL = "https://example.com/page" + + @pytest.fixture(autouse=True) + def _setup(self, monkeypatch): + """Common patches for vision SSRF tests.""" + monkeypatch.setattr(browser_tool, "_is_camofox_mode", lambda: False) + + def test_blocks_private_url_after_eval_navigation(self, monkeypatch): + """Vision must block when current page URL is private.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: False) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "eval": + return _make_eval_result(self.PRIVATE_URL) + return {"success": False, "error": "should not reach screenshot"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + + result = json.loads(browser_browser_vision(question="what do you see", task_id="test")) + assert result["success"] is False + assert "private or internal address" in result["error"] + assert self.PRIVATE_URL in result["error"] + + def test_allows_public_url_after_eval_navigation(self, monkeypatch): + """Vision must proceed when current page URL is public.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + monkeypatch.setattr(browser_tool, "_is_safe_url", lambda url: True) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "eval": + return _make_eval_result(self.PUBLIC_URL) + elif command == "screenshot": + return _make_screenshot_result() + return {"success": False, "error": "unknown"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + # Screenshot file won't exist — that's fine, function returns error + # but the important thing is the guard didn't block it. + + result_raw = browser_browser_vision(question="what do you see", task_id="test") + result = json.loads(result_raw) + # Guard passed; function continues to screenshot path. + # Since screenshot file doesn't exist, it returns a file-not-found error, + # NOT the "private or internal address" error. + assert "private or internal address" not in result.get("error", "") + + def test_skips_check_in_local_backend_mode(self, monkeypatch): + """Local backend mode skips SSRF check entirely.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: True) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "screenshot": + return _make_screenshot_result() + return {"success": False, "error": "should not be called"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + + result_raw = browser_browser_vision(question="what", task_id="test") + result = json.loads(result_raw) + assert "private or internal address" not in result.get("error", "") + + def test_skips_check_when_private_urls_allowed(self, monkeypatch): + """When allow_private_urls is enabled, SSRF check is skipped.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: True) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "screenshot": + return _make_screenshot_result() + return {"success": False, "error": "should not be called"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + + result_raw = browser_browser_vision(question="what", task_id="test") + result = json.loads(result_raw) + assert "private or internal address" not in result.get("error", "") + + def test_handles_eval_failure_gracefully(self, monkeypatch): + """If URL eval fails, vision should still proceed (fail-open).""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "eval": + return {"success": False, "error": "eval failed"} + elif command == "screenshot": + return _make_screenshot_result() + return {"success": False, "error": "unknown"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + + result_raw = browser_browser_vision(question="what", task_id="test") + result = json.loads(result_raw) + assert "private or internal address" not in result.get("error", "") + + def test_handles_eval_exception(self, monkeypatch): + """If URL eval raises an exception, vision should still proceed.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "eval": + raise RuntimeError("CDP connection lost") + elif command == "screenshot": + return _make_screenshot_result() + return {"success": False, "error": "unknown"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + + result_raw = browser_browser_vision(question="what", task_id="test") + result = json.loads(result_raw) + assert "private or internal address" not in result.get("error", "") diff --git a/tools/browser_tool.py b/tools/browser_tool.py index 9a584c77088..f4e952d060c 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -3314,6 +3314,36 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str] screenshot_path = screenshots_dir / f"browser_screenshot_{uuid_mod.uuid4().hex}.png" effective_task_id = _last_session_key(task_id or "default") + # ── Private-network guard: block vision from eval-navigated private pages ── + # After any eval (browser_console) that may have changed location.href to a + # private/internal address, the screenshot would expose private page content + # to the vision model. Re-check the current URL before capturing anything. + if ( + not _is_local_backend() + and not _allow_private_urls() + ): + try: + _url_result = _run_browser_command( + effective_task_id, "eval", ["window.location.href"], + timeout=5, _engine_override="auto", + ) + if _url_result.get("success"): + _current_url = ( + _url_result.get("data", {}).get("result", "") + .strip().strip('"').strip("'") + ) + if _current_url and not _is_safe_url(_current_url): + return json.dumps({ + "success": False, + "error": ( + "Blocked: page URL targets a private or internal address " + f"({_current_url}). This may have been caused by a " + "JavaScript navigation via browser_console." + ), + }, ensure_ascii=False) + except Exception as _url_exc: + logger.debug("browser_vision: URL safety check failed (%s)", _url_exc) + # Lightpanda has no graphical renderer — pre-route screenshots to Chrome # via the fallback helper instead of letting the normal path fail with a # CDP error or return a placeholder PNG. The normal analysis path below