From cf47a164991bf5588fd2b41ce244f55d900c1167 Mon Sep 17 00:00:00 2001 From: Igor Soarez Date: Tue, 10 Nov 2026 19:17:57 +0200 Subject: extension: snapshot sandboxed pages over CDP, never return empty text On a page served with CSP `sandbox` and no allow-scripts, such as raw.githubusercontent.com, chrome.scripting.executeScript runs nothing and resolves with a null result instead of throwing. The access-denied CDP fallback never ran, page.snapshot returned undefined, and chrome_snapshot answered with {type: "text"} and no text, because JSON.stringify(undefined) is undefined. Pi saved that block in the session, and every later turn failed in token estimation with "Cannot read properties of undefined (reading 'length')", whatever the provider. When scripting resolves without an envelope, snapshot and inspect now redo the injection and the call over CDP Runtime.evaluate, which such pages still allow, and raise an error if that also returns nothing. On the pi side, safeJson always returns a string, and an empty snapshot or inspect result is a tool error rather than a text block without text. sandboxed-snapshot.test.mjs drives page.snapshot and page.inspect through a page that runs no extension script. It fails on b8b63ea. --- .../browser-extension/service_worker.js | 89 ++++++++++------- extensions/chrome-profile-bridge/index.ts | 11 ++- package.json | 2 +- test-suite/unit/sandboxed-snapshot.test.mjs | 108 +++++++++++++++++++++ 4 files changed, 170 insertions(+), 40 deletions(-) create mode 100644 test-suite/unit/sandboxed-snapshot.test.mjs diff --git a/extensions/chrome-profile-bridge/browser-extension/service_worker.js b/extensions/chrome-profile-bridge/browser-extension/service_worker.js index d47b385..9ac247c 100644 --- a/extensions/chrome-profile-bridge/browser-extension/service_worker.js +++ b/extensions/chrome-profile-bridge/browser-extension/service_worker.js @@ -614,27 +614,36 @@ async function executeScriptWithFallback(options, label) { if (!isScriptingAccessDenied(error) || !chrome.debugger) throw error; const tabId = options?.target?.tabId; recordAttachEvent({ kind: "scripting-denied-cdp-fallback", tabId, files: options?.files || null, message: String(error?.message || error).slice(0, 300) }); - if (Array.isArray(options?.files) && options.files.length) { - for (const file of options.files) { - if (!packagedFileSources.has(file)) { - const response = await fetch(chrome.runtime.getURL(file)); - if (!response.ok) throw new Error(`Could not read ${file} for CDP injection (HTTP ${response.status})`); - packagedFileSources.set(file, await response.text()); - } - const injected = await cdpEval(tabId, packagedFileSources.get(file)); - if (injected.exceptionDetails) { - throw new Error(`${label || file}: ${cdpExceptionText(injected.exceptionDetails) || "CDP injection failed"}`); - } + return executeScriptViaCdp(options, label); + } +} + +// The CDP half of executeScriptWithFallback, also used directly when scripting silently runs +// nothing: a document served with CSP `sandbox` (no allow-scripts), such as +// raw.githubusercontent.com, makes chrome.scripting.executeScript resolve with a null result +// instead of throwing, while Runtime.evaluate still works there. +async function executeScriptViaCdp(options, label) { + const tabId = options?.target?.tabId; + if (Array.isArray(options?.files) && options.files.length) { + for (const file of options.files) { + if (!packagedFileSources.has(file)) { + const response = await fetch(chrome.runtime.getURL(file)); + if (!response.ok) throw new Error(`Could not read ${file} for CDP injection (HTTP ${response.status})`); + packagedFileSources.set(file, await response.text()); + } + const injected = await cdpEval(tabId, packagedFileSources.get(file)); + if (injected.exceptionDetails) { + throw new Error(`${label || file}: ${cdpExceptionText(injected.exceptionDetails) || "CDP injection failed"}`); } - return [{ result: undefined }]; - } - const expression = `(${options.func.toString()})(...${JSON.stringify(Array.isArray(options.args) ? options.args : [])})`; - const result = await cdpEval(tabId, expression); - if (result.exceptionDetails) { - throw new Error(`${label || "page script"}: ${cdpExceptionText(result.exceptionDetails) || "evaluation failed"}`); } - return [{ result: result.result?.value }]; + return [{ result: undefined }]; + } + const expression = `(${options.func.toString()})(...${JSON.stringify(Array.isArray(options.args) ? options.args : [])})`; + const result = await cdpEval(tabId, expression); + if (result.exceptionDetails) { + throw new Error(`${label || "page script"}: ${cdpExceptionText(result.exceptionDetails) || "evaluation failed"}`); } + return [{ result: result.result?.value }]; } // Wraps cdpRaw with one auto-recover on detached/closed sessions: @@ -2412,6 +2421,28 @@ async function withOptionalSnapshot(params, actionFn) { // chrome.scripting.executeScript({ files }). That file is free of eval/new Function, so it works // on strict-CSP pages, and it installs globalThis.__piChromeSnapshotPage / __piChromeInspectTarget. // It shares window.__PI_CHROME_STATE__ (same el- uid scheme) with the CDP-injected input helpers. +// Injects snapshot_injected.js and runs `scriptOptions.func` (which must return an +// { ok, value | error } envelope) in the tab's top frame. chrome.scripting is primary; if it +// resolves without an envelope, the page ran no extension script (CSP `sandbox` without +// allow-scripts, e.g. raw.githubusercontent.com), so both steps are redone over CDP. +async function runSnapshotInjected(tab, kind, scriptOptions) { + const inject = { target: { tabId: tab.id, frameIds: [0] }, world: "MAIN", files: ["snapshot_injected.js"] }; + const run = { target: { tabId: tab.id, frameIds: [0] }, world: "MAIN", ...scriptOptions }; + const injectLabel = `inject ${kind} script in tab ${tab.id}`; + const runLabel = `run ${kind} script in tab ${tab.id}`; + await executeScriptWithFallback(inject, injectLabel); + const results = await executeScriptWithFallback(run, runLabel); + const first = results?.[0]; + if (first?.error || first?.result != null || !chrome.debugger) return results; + recordAttachEvent({ kind: "scripting-no-result-cdp-fallback", tabId: tab.id, files: inject.files }); + await executeScriptViaCdp(inject, injectLabel); + const viaCdp = await executeScriptViaCdp(run, runLabel); + if (viaCdp?.[0]?.result == null) { + throw new Error(`Chrome ${kind} script returned no result in tab ${tab.id} (${tab.url || "unknown url"}); the page may block script execution.`); + } + return viaCdp; +} + async function snapshotInTab(params) { const tab = await getTabByParams(params); await bringToFront(tab, params); @@ -2424,14 +2455,7 @@ async function snapshotInTab(params) { params.query ?? null, params.maxTextChars ?? null, ]; - await executeScriptWithFallback({ - target: { tabId: tab.id, frameIds: [0] }, - world: "MAIN", - files: ["snapshot_injected.js"], - }, `inject snapshot script in tab ${tab.id}`); - const results = await executeScriptWithFallback({ - target: { tabId: tab.id, frameIds: [0] }, - world: "MAIN", + const results = await runSnapshotInjected(tab, "snapshot", { func: async (invocationArgs) => { try { const snapshotPage = globalThis.__piChromeSnapshotPage; @@ -2442,7 +2466,7 @@ async function snapshotInTab(params) { } }, args: [args], - }, `run snapshot script in tab ${tab.id}`); + }); const first = results?.[0]; if (first?.error) { const message = typeof first.error === "string" ? first.error : (first.error.message || JSON.stringify(first.error)); @@ -2460,14 +2484,7 @@ async function inspectInTab(params) { const tab = await getTabByParams(params); await bringToFront(tab, params); const args = [params.uid ?? null, params.selector ?? null, params.scrollIntoView === true]; - await executeScriptWithFallback({ - target: { tabId: tab.id, frameIds: [0] }, - world: "MAIN", - files: ["snapshot_injected.js"], - }, `inject inspect script in tab ${tab.id}`); - const results = await executeScriptWithFallback({ - target: { tabId: tab.id, frameIds: [0] }, - world: "MAIN", + const results = await runSnapshotInjected(tab, "inspect", { func: async (invocationArgs) => { try { const inspectTarget = globalThis.__piChromeInspectTarget; @@ -2478,7 +2495,7 @@ async function inspectInTab(params) { } }, args: [args], - }, `run inspect script in tab ${tab.id}`); + }); const first = results?.[0]; if (first?.error) { const message = typeof first.error === "string" ? first.error : (first.error.message || JSON.stringify(first.error)); diff --git a/extensions/chrome-profile-bridge/index.ts b/extensions/chrome-profile-bridge/index.ts index 525de6e..0338c5c 100644 --- a/extensions/chrome-profile-bridge/index.ts +++ b/extensions/chrome-profile-bridge/index.ts @@ -133,8 +133,11 @@ function truncateText(text: string, maxChars = MAX_TEXT_CHARS): string { return `${text.slice(0, maxChars)}\n\n[truncated ${text.length - maxChars} characters]`; } +// JSON.stringify returns undefined (not a string) for undefined, functions, and symbols. A tool +// result block with `text: undefined` is persisted to the session and makes every later turn +// fail with "Cannot read properties of undefined (reading 'length')", so always return a string. function safeJson(value: unknown): string { - return JSON.stringify(value, null, 2); + return JSON.stringify(value, null, 2) ?? String(value); } const snapshotModeValues = ["auto", "interactive", "forms", "pageMap", "text", "changes", "full"] as const; @@ -150,7 +153,8 @@ function rectText(rect: any): string { } function formatChromeSnapshot(snapshot: any): string { - if (!snapshot || typeof snapshot !== "object") return safeJson(snapshot); + if (snapshot == null) throw new Error("Chrome returned no snapshot for this page; it may block extension scripts. Try chrome_evaluate instead."); + if (typeof snapshot !== "object") return safeJson(snapshot); if (snapshot.mode === "full") return truncateText(safeJson(snapshot)); const lines: string[] = []; lines.push(`# Chrome snapshot${snapshot.mode ? ` (${snapshot.mode})` : ""}`); @@ -351,7 +355,8 @@ async function pruneScreenshotDir(dir: string, retentionDays?: number): Promise< // End screenshot pruning. function formatChromeInspect(inspect: any): string { - if (!inspect || typeof inspect !== "object") return safeJson(inspect); + if (inspect == null) throw new Error("Chrome returned no inspect result for this page; it may block extension scripts. Try chrome_evaluate instead."); + if (typeof inspect !== "object") return safeJson(inspect); const t = inspect.target || {}; const lines: string[] = []; lines.push(`# Chrome inspect ${t.uid || ""}`.trim()); diff --git a/package.json b/package.json index cc7ef4c..7f7e204 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "name": "pi-chrome", "version": "0.15.57", "scripts": { - "test": "node test-suite/unit/csp-eval.test.mjs && node test-suite/unit/automation-target.test.mjs && node test-suite/unit/session-cleanup.test.mjs && node test-suite/unit/background-policy.test.mjs && node test-suite/unit/input-reliability.test.mjs && node test-suite/unit/chrome-command.test.mjs && node test-suite/unit/bridge-resilience.test.mjs && node test-suite/unit/cdp-passthrough.test.mjs && node test-suite/unit/type-evidence.test.mjs && node test-suite/unit/hidden-input.test.mjs && node test-suite/unit/console-capture.test.mjs && node test-suite/unit/prompt-section.test.mjs && node test-suite/unit/upload-guard.test.mjs && node test-suite/unit/grant-scope.test.mjs && node test-suite/unit/leave-site-prompt.test.mjs && node test-suite/unit/shared-window.test.mjs && node test-suite/unit/overlay.test.mjs", + "test": "node test-suite/unit/csp-eval.test.mjs && node test-suite/unit/automation-target.test.mjs && node test-suite/unit/session-cleanup.test.mjs && node test-suite/unit/background-policy.test.mjs && node test-suite/unit/input-reliability.test.mjs && node test-suite/unit/chrome-command.test.mjs && node test-suite/unit/bridge-resilience.test.mjs && node test-suite/unit/cdp-passthrough.test.mjs && node test-suite/unit/type-evidence.test.mjs && node test-suite/unit/hidden-input.test.mjs && node test-suite/unit/console-capture.test.mjs && node test-suite/unit/prompt-section.test.mjs && node test-suite/unit/upload-guard.test.mjs && node test-suite/unit/grant-scope.test.mjs && node test-suite/unit/leave-site-prompt.test.mjs && node test-suite/unit/shared-window.test.mjs && node test-suite/unit/overlay.test.mjs && node test-suite/unit/sandboxed-snapshot.test.mjs", "version": "node scripts/sync-manifest-version.js", "prepublishOnly": "node scripts/sync-manifest-version.js" }, diff --git a/test-suite/unit/sandboxed-snapshot.test.mjs b/test-suite/unit/sandboxed-snapshot.test.mjs new file mode 100644 index 0000000..9774f21 --- /dev/null +++ b/test-suite/unit/sandboxed-snapshot.test.mjs @@ -0,0 +1,108 @@ +// chrome_snapshot and chrome_inspect on a page served with CSP `sandbox` (no allow-scripts), such as +// raw.githubusercontent.com. There chrome.scripting.executeScript resolves with a null result +// instead of throwing, so the access-denied CDP fallback never ran and the snapshot came back +// undefined. The worker now redoes the injection over CDP Runtime.evaluate, which such pages allow. +import assert from "node:assert/strict"; +import fs from "node:fs"; +import vm from "node:vm"; +import { test } from "node:test"; + +const workerUrl = new URL("../../extensions/chrome-profile-bridge/browser-extension/service_worker.js", import.meta.url); +const src = fs.readFileSync(workerUrl, "utf8"); + +// Stands in for snapshot_injected.js when the worker reads it from the package for CDP injection. +const INJECTED_STUB = ` +globalThis.__piChromeSnapshotPage = async (...args) => ({ mode: args[4], title: "raw page" }); +globalThis.__piChromeInspectTarget = async (uid) => ({ target: { uid } }); +`; + +function loadWorker({ scriptingRuns, cdpReturnsNothing = false }) { + const pageWorld = vm.createContext({ Promise, Error, String, JSON }); + pageWorld.globalThis = pageWorld; + const calls = { scripting: 0, cdp: 0 }; + const noop = () => {}; + const listener = { addListener: noop, removeListener: noop }; + const sandbox = { + console, JSON, Date, Math, Promise, Array, Object, String, Number, Boolean, + Error, TypeError, Map, Set, Symbol, structuredClone, + setTimeout, clearTimeout, setInterval: () => 0, clearInterval: noop, + fetch: async (url) => { + assert.match(String(url), /snapshot_injected\.js$/); + return { ok: true, text: async () => INJECTED_STUB }; + }, + navigator: { userAgent: "unit-test" }, + WebSocket: function () {}, + chrome: { + runtime: { + id: "unittestextension", getManifest: () => ({ version: "0.0.0" }), + getURL: (file) => `chrome-extension://unittestextension/${file}`, + onInstalled: listener, onStartup: listener, lastError: null, + }, + alarms: { onAlarm: listener, create: noop, clear: noop, clearAll: noop }, + action: { onClicked: listener }, + debugger: { sendCommand: noop, attach: async () => {}, detach: async () => {}, getTargets: (cb) => cb([]) }, + scripting: { + // A sandboxed document runs no extension script and reports a null result. + executeScript: async ({ files, func, args }) => { + calls.scripting++; + if (!scriptingRuns) return [{ frameId: 0, result: null }]; + if (files) { + vm.runInContext(INJECTED_STUB, pageWorld); + return [{ frameId: 0, result: undefined }]; + } + const fn = vm.runInContext(`(${func.toString()})`, pageWorld); + return [{ frameId: 0, result: await fn(...(args || [])) }]; + }, + }, + tabs: { query: async () => [], get: async () => ({}), create: async () => ({}), update: async () => ({}), remove: async () => {} }, + windows: { update: async () => {} }, + webNavigation: { onCommitted: listener }, + }, + }; + sandbox.globalThis = sandbox; + sandbox.self = sandbox; + vm.createContext(sandbox); + vm.runInContext(src, sandbox); + sandbox.attachDebugger = async () => ({}); + sandbox.bringToFront = async () => {}; + sandbox.getTabByParams = async () => ({ id: 7, windowId: 1, url: "https://raw.githubusercontent.com/o/r/main/f.swift" }); + sandbox.cdp = async (_tabId, method, params) => { + assert.equal(method, "Runtime.evaluate"); + calls.cdp++; + if (cdpReturnsNothing) return { result: { type: "undefined" } }; + // returnByValue: JSON-able values come back as `value`; functions and undefined only as a type. + const value = await vm.runInContext(params.expression, pageWorld); + const json = JSON.stringify(value); + return json === undefined + ? { result: { type: typeof value } } + : { result: { type: "object", value: JSON.parse(json) } }; + }; + return { dispatch: sandbox.dispatch, calls }; +} + +test("snapshot of a sandboxed page falls back to CDP", async () => { + const { dispatch, calls } = loadWorker({ scriptingRuns: false }); + const snapshot = await dispatch("page.snapshot", { mode: "text" }); + assert.equal(snapshot?.title, "raw page"); + assert.equal(snapshot?.mode, "text"); + assert.equal(calls.scripting, 2, "scripting is tried first for both the injection and the call"); + assert.equal(calls.cdp, 2, "both are redone over CDP"); +}); + +test("inspect of a sandboxed page falls back to CDP", async () => { + const { dispatch } = loadWorker({ scriptingRuns: false }); + const inspect = await dispatch("page.inspect", { uid: "e12" }); + assert.equal(inspect?.target?.uid, "e12"); +}); + +test("snapshot of an ordinary page does not touch CDP", async () => { + const { dispatch, calls } = loadWorker({ scriptingRuns: true }); + const snapshot = await dispatch("page.snapshot", { mode: "auto" }); + assert.equal(snapshot?.title, "raw page"); + assert.equal(calls.cdp, 0); +}); + +test("a page that runs neither path reports an error instead of an empty snapshot", async () => { + const { dispatch } = loadWorker({ scriptingRuns: false, cdpReturnsNothing: true }); + await assert.rejects(dispatch("page.snapshot", { mode: "text" }), /snapshot script returned no result in tab 7 \(https:\/\/raw\.githubusercontent\.com/); +}); -- cgit v1.2.3