From 0ae6196087c845f84e79238511d04785d2a9ebd6 Mon Sep 17 00:00:00 2001 From: liuhao1024 Date: Sat, 13 Jun 2026 02:59:13 +0800 Subject: [PATCH] fix(browser): allow local sidecar sessions to bypass SSRF guard MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #45101 review feedback from egilewski. --- tests/tools/test_browser_snapshot_ssrf.py | 44 ++++++++++++++++++++++- tools/browser_tool.py | 2 ++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/tests/tools/test_browser_snapshot_ssrf.py b/tests/tools/test_browser_snapshot_ssrf.py index 9cb29d9c498..4ced2897b35 100644 --- a/tests/tools/test_browser_snapshot_ssrf.py +++ b/tests/tools/test_browser_snapshot_ssrf.py @@ -122,6 +122,27 @@ class TestBrowserSnapshotPrivateNetworkGuard: assert result["success"] is True assert "snapshot" in result + + def test_skips_check_for_local_sidecar_session(self, monkeypatch): + """Local sidecar sessions can legitimately access private URLs.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + # Simulate the effective_task_id being a local sidecar key + monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: True) + + def mock_run_browser_command(task_id, command, args=None, **kwargs): + if command == "snapshot": + return _make_snapshot_result() + return {"success": False, "error": "should not be called"} + + monkeypatch.setattr( + browser_tool, "_run_browser_command", mock_run_browser_command + ) + + result = json.loads(browser_browser_snapshot(task_id="test")) + assert result["success"] is True + assert "snapshot" in result + 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) @@ -337,6 +358,27 @@ class TestBrowserVisionPrivateNetworkGuard: result = json.loads(result_raw) assert "private or internal address" not in result.get("error", "") + + def test_skips_check_for_local_sidecar_session(self, monkeypatch): + """Local sidecar sessions can legitimately access private URLs.""" + monkeypatch.setattr(browser_tool, "_is_local_backend", lambda: False) + monkeypatch.setattr(browser_tool, "_allow_private_urls", lambda: False) + # Simulate the effective_task_id being a local sidecar key + monkeypatch.setattr(browser_tool, "_is_local_sidecar_key", lambda key: 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) @@ -393,4 +435,4 @@ class TestBrowserVisionPrivateNetworkGuard: 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", "") + assert "private or internal address" not in result.get("error", "") \ No newline at end of file diff --git a/tools/browser_tool.py b/tools/browser_tool.py index f4e952d060c..ec38a0a1eaa 100644 --- a/tools/browser_tool.py +++ b/tools/browser_tool.py @@ -2701,6 +2701,7 @@ def browser_snapshot( # Re-check the current URL before returning the snapshot. if ( not _is_local_backend() + and not _is_local_sidecar_key(effective_task_id) and not _allow_private_urls() ): try: @@ -3320,6 +3321,7 @@ def browser_vision(question: str, annotate: bool = False, task_id: Optional[str] # to the vision model. Re-check the current URL before capturing anything. if ( not _is_local_backend() + and not _is_local_sidecar_key(effective_task_id) and not _allow_private_urls() ): try: