fix(browser): per-snapshot byte cap

jkyberneees · jkyberneees · commit 489d7fb24c5b · 2026-07-18T21:35:56.000+02:00
The browser capped history at 50 snapshots and elements at 500 per page,
but a single snapshot could retain ~10 MB of extracted text. A hostile
page could still push ~500 MB into memory.

- Add maxBrowserSnapshotBytes (1 MiB) and truncate extracted content that
  exceeds it, appending a truncation marker.
- Add TestBrowser_SnapshotByteCap regression test.
- Document the cap in SECURITY.md and AGENTS.md.

Closes L-5 in sec_findings.md.
diff --git a/AGENTS.md b/AGENTS.md
@@ -166,6 +166,7 @@ Layered prompt-injection / approval-fatigue defenses. Full reference: [docs/SECU
 - **`batch_patch` trusted-class propagation** (`cmd/odek/perf_tools.go`) — `batch_patch` now passes its cached `trustedClasses` to `CheckOperation`, matching `write_file` and `patch`. A trusted `local_write` class is honored across all patches in the batch instead of re-prompting per patch.
 - **Browser link URL wrapping** (`cmd/odek/browser_tool.go`) — interactive element text was already wrapped as untrusted, but link URLs in `clickableRef.URL` were returned raw. They are now wrapped too, while an unexported `rawURL` is kept for internal click resolution.
 - **Browser post-redirect URL attribution** (`cmd/odek/browser_tool.go`) — `browser_navigate` now uses `resp.Request.URL` (the final post-redirect URL) for the snapshot URL, the untrusted-content source, and relative-link click resolution, instead of attributing content to the original requested URL.
+- **Browser per-snapshot byte cap** (`cmd/odek/browser_tool.go`) — each browser snapshot truncates extracted content to 1 MiB, so the 50-snapshot history cap cannot be bypassed by a small number of huge pages.
 - **Telegram message length by UTF-16 code units** (`internal/telegram/handler.go`) — `MaxMsgLength` is enforced using UTF-16 code-unit counting, matching Telegram's own limits. Multi-byte UTF-8 characters (e.g. emoji) no longer pass the local check while being rejected by Telegram.
 - **Telegram restart marker permissions** (`cmd/odek/telegram.go`) — `~/.odek/restart.json` is now written with `0600` instead of `0644`, preventing local users from reading the list of active chat IDs.
 - **Telegram singleton flock lock** (`cmd/odek/telegram.go` + `internal/flock`) — the Telegram bot now uses an advisory `flock` on `~/.odek/telegram.lock` instead of a PID file probed with signals. This removes the non-Linux path where a planted PID could cause odek to kill an arbitrary process.
diff --git a/cmd/odek/browser_tool.go b/cmd/odek/browser_tool.go
@@ -56,6 +56,10 @@ const maxBrowserHistory = 50
 // or buttons.
 const maxBrowserElements = 500
 
+// maxBrowserSnapshotBytes caps the extracted text retained per snapshot so the
+// history limit cannot be bypassed by a small number of huge pages.
+const maxBrowserSnapshotBytes = 1 * 1024 * 1024
+
 // browserState holds the shared state for one browser session.
 type browserState struct {
 	mu      sync.Mutex
@@ -467,6 +471,10 @@ func parseHTML(ctx context.Context, html, pageURL string, status int) browserSna
 	}
 
 	snap.Content = strings.Join(contentParts, "\n")
+	if len(snap.Content) > maxBrowserSnapshotBytes {
+		snap.Content = snap.Content[:maxBrowserSnapshotBytes] +
+			"\n[content truncated: exceeds per-snapshot byte cap]"
+	}
 	snap.Elements = elements
 
 	// Title, element text, and link URLs come from the page — wrap them as
diff --git a/cmd/odek/next_security_vulnerabilities_test.go b/cmd/odek/next_security_vulnerabilities_test.go
@@ -37,6 +37,32 @@ func TestBrowser_HistoryCap(t *testing.T) {
 	}
 }
 
+func TestBrowser_SnapshotByteCap(t *testing.T) {
+	huge := strings.Repeat("<p>word</p>", 300000) // ~1.5 MB of extracted text
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		fmt.Fprintf(w, "<html><body>%s</body></html>", huge)
+	}))
+	defer srv.Close()
+
+	tool := newTestBrowserTool()
+	result := callJSON(t, tool, fmt.Sprintf(`{"action":"navigate","url":%q}`, srv.URL))
+	var r struct {
+		Content string `json:"content"`
+		Error   string `json:"error,omitempty"`
+	}
+	mustUnmarshal(t, result, &r)
+	if r.Error != "" {
+		t.Fatalf("navigate error: %s", r.Error)
+	}
+	body := unwrapUntrusted(r.Content)
+	if len(body) > maxBrowserSnapshotBytes+200 {
+		t.Fatalf("snapshot content = %d bytes, expected cap near %d", len(body), maxBrowserSnapshotBytes)
+	}
+	if !strings.Contains(body, "truncated") {
+		t.Fatalf("expected truncation marker in capped content")
+	}
+}
+
 // ── 2. search_files / multi_grep must cap limit and result size ──────────
 
 func TestSearchFiles_LimitCap(t *testing.T) {
diff --git a/docs/SECURITY.md b/docs/SECURITY.md
@@ -540,6 +540,10 @@ In addition, a friction counter tracks approvals per class. After 3 approvals of
 
 `browser_navigate` previously attributed the fetched content to the URL originally requested, even when the HTTP client followed redirects. An attacker could point a reputable-looking URL at a redirector and have the resulting page content labeled with the reputable domain, and relative links on the landing page resolved against the wrong origin. `browser_navigate` now uses `resp.Request.URL` (the final post-redirect URL) for the snapshot URL, the untrusted-content wrapper source, and click resolution.
 
+### 36b. Browser per-snapshot byte cap
+
+The browser tool already capped navigation history at 50 snapshots and interactive elements at 500 per page, but a single snapshot could retain ~10 MB of extracted text (the HTTP body read is capped at 10 MB). A hostile page or a small number of huge pages could still consume ~500 MB. Each snapshot now truncates extracted content to 1 MiB, so the worst-case retained history is bounded to ~50 MiB.
+
 ### 37. Telegram message length by UTF-16 code units
 
 Telegram's message and caption limits are defined in UTF-16 code units, but `internal/telegram/handler.go` was using `len(msg.Text)` and `len(msg.Caption)`, which count UTF-8 bytes. Emoji and other supplementary-plane characters consume 4 UTF-8 bytes but 2 UTF-16 code units, so emoji-heavy messages could pass the local check and then be rejected by Telegram. The handler now counts UTF-16 code units via `utf16Len`.
