From 67b3a4949e26fdf70b5d3c6f83ef32a526901830 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 02:26:50 +0200 Subject: [PATCH 01/24] fix: address PR 2 secret retention and preflight retry findings Reject environment-backed controls in recovery allowlists during normal/reconstruction task validation. Guard live selector aliases before capture in both Playwright and managed Jev. Preserve unrelated allowlisted fields without storing the protected values. Retry the same request ID after preflight failure only when the journal proves execution never started. Keep completed, partial, uncertain, and unknown-event requests deduplicated. Add unit/browser regressions including failed preflight followed by exactly one Save. Build on the merged Chrome readiness fix without replacing it. Allow 30 seconds for cold startup in the shared Node fixture. No production timeout, dependency, skill, documentation, or Action pin changes. --- .github/workflows/ci.yml | 3 ++ package.json | 2 +- src/page-worker.ts | 34 ++++++++++++- src/task.ts | 5 ++ src/workflows.ts | 6 ++- test/review-followups.smoke.mjs | 80 +++++++++++++++++++++++++++++ test/review-followups.test.mjs | 90 +++++++++++++++++++++++++++++++++ test/support/chrome.mjs | 2 +- worker/jev_runner.py | 10 +++- worker/tests/managed_smoke.py | 17 ++++++- 10 files changed, 242 insertions(+), 7 deletions(-) create mode 100644 test/review-followups.smoke.mjs create mode 100644 test/review-followups.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0f9cade..68a9fac 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,3 +46,6 @@ jobs: - name: Actual Midscene with controlled provider if: always() && !cancelled() run: timeout 90s node test/vision.smoke.mjs + - name: Review regressions for retention and preflight + if: always() && !cancelled() + run: timeout 90s node test/review-followups.smoke.mjs diff --git a/package.json b/package.json index 58cb053..0f22e5a 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "test:python": "python3 -m unittest discover -s worker/tests -v", "check": "npm run typecheck && npm test && npm run test:python", "prepack": "npm run build", - "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs" + "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs" }, "dependencies": { "@effect/platform-node": "4.0.0-rc.116", diff --git a/src/page-worker.ts b/src/page-worker.ts index 2bccb64..0b95b64 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -2,7 +2,7 @@ import * as fs from "node:fs"; import path from "node:path"; import { pathToFileURL } from "node:url"; import { randomUUID } from "node:crypto"; -import type { Browser, Page, Locator, FrameLocator } from "playwright-core"; +import type { Browser, Page, Locator, FrameLocator, ElementHandle, Frame } from "playwright-core"; import { BrowserError, type Check } from "./contracts.js"; import { Journal, remaining, digest, atomic, type Data } from "./journal.js"; import { Cdp, checkpoint, liveStateExpression } from "./cdp.js"; @@ -75,7 +75,38 @@ export async function pollChecks(page: Page, checks: readonly Check[], timeoutMs await new Promise(resolve => setTimeout(resolve, 100)); } } +async function assertRecoveryTargets(page: Page, journal: Journal): Promise { + const task = journal.meta().task; + const fields = task.recovery?.fields ?? []; + const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] + .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); + if (!fields.length || !protectedSteps.length) return; + const handles: ElementHandle[] = []; + try { + const protectedNodes: { node: ElementHandle; frame: Frame | null }[] = []; + for (const step of protectedSteps) { + const { locator } = await scoped(page, step); + const nodes = await locator.elementHandles(); + handles.push(...nodes); + for (const node of nodes) protectedNodes.push({ node, frame: await node.ownerFrame() }); + } + for (const field of fields) { + const { locator } = await scoped(page, field); + const nodes = await locator.elementHandles(); handles.push(...nodes); + for (const node of nodes) { + // Different CSS/frame paths can resolve to the same control. Compare + // DOM identity, without reading either value or persisting a handle. + const frame = await node.ownerFrame(); + const candidates = protectedNodes.filter(p => p.frame === frame).map(p => p.node); + if (candidates.length && await node.evaluate((element, others) => others.includes(element), candidates)) { + throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); + } + } + } + } finally { await Promise.allSettled(handles.map(handle => handle.dispose())); } +} async function captureFields(page: Page, journal: Journal): Promise { + await assertRecoveryTargets(page, journal); for (const f of journal.meta().task.recovery?.fields ?? []) { const { locator } = await scoped(page, f); if (await locator.count() !== 1) continue; @@ -116,6 +147,7 @@ async function runSteps(page: Page, journal: Journal, job: PageJob, snap: () => throw new BrowserError({ code: "ambiguous_target", reason: "Action requires one matching control. Inspect the scoped page; no action was dispatched." }); } if (!rebuilding) await captureFields(page, journal); + else if (s.valueFromEnv) await assertRecoveryTargets(page, journal); if (s.kind === "fill") for (const f of journal.meta().task.recovery?.fields ?? []) { if (f.selector === s.selector && JSON.stringify(f.frames) === JSON.stringify(s.frames) && f.shadow === s.shadow && !s.valueFromEnv) { const result = await locator.evaluate(readElement, { kind: "value" }); diff --git a/src/task.ts b/src/task.ts index 336b08f..86ff935 100644 --- a/src/task.ts +++ b/src/task.ts @@ -75,6 +75,8 @@ export function validateTask(task: Task, testing: boolean): void { if (x.schema && JSON.stringify(x.schema).length > 20_000) invalid("Extraction schema too large."); } if (task.inputs && (Object.keys(task.inputs).length > 30 || Object.values(task.inputs).some(v => v.length > 20_000))) invalid("Inputs exceed their storage budget."); + const targetKey = (s: { selector?: string | undefined; frames?: readonly string[] | undefined; shadow?: string | undefined }) => + JSON.stringify([s.selector?.trim(), (s.frames ?? []).map(f => f.trim()), s.shadow ?? "none"]); const fieldKeys = new Set(); for (const f of task.recovery?.fields ?? []) { scope(f); @@ -99,6 +101,9 @@ export function validateTask(task: Task, testing: boolean): void { if (["navigate", "fill", "press", "select", "check", "scroll"].includes(s.kind) && values.length !== 1) invalid(`${s.kind} requires exactly one value source.`); if (s.valueFromInput && (!/^[a-zA-Z][\w-]{0,63}$/.test(s.valueFromInput) || task.inputs?.[s.valueFromInput] === undefined)) invalid("A named step input is missing."); if (s.valueFromEnv && !/^[A-Z_][A-Z_0-9]{0,127}$/.test(s.valueFromEnv)) invalid("Invalid environment reference."); + if (s.valueFromEnv && s.selector && task.recovery?.fields?.some(f => targetKey(f) === targetKey(s))) { + invalid("An environment-backed control cannot be a recovery field. Remove it from recovery.fields; use its environment reference for reconstruction."); + } if (s.valueFromRecovery && !fieldKeys.has(s.valueFromRecovery)) invalid("Recovery key is not allowlisted."); if (s.kind === "goal" && !s.goal?.trim()) invalid("goal step requires a goal."); if (s.kind === "checkpoint" && !s.checks?.length) invalid("checkpoint requires assertions."); diff --git a/src/workflows.ts b/src/workflows.ts index 7bc6bb0..532df81 100644 --- a/src/workflows.ts +++ b/src/workflows.ts @@ -249,7 +249,11 @@ export function runTask(input: Task, testing = false) { const config = yield* io(host.configuration); const profile = yield* io(() => fs.promises.realpath(config.profileDir)); const { journal, duplicate } = yield* sync(() => createRun(host.home(), task, testing, profile)); - if (duplicate) return { ...summary(journal.state()), duplicate: true }; + // Only preflight failures are retryable through the idempotency key. Once + // execution started (or an unknown event exists), recovery must be explicit. + if (duplicate && !journal.events().every(e => e.type === "created" || e.type === "preflight.failed")) { + return { ...summary(journal.state()), duplicate: true }; + } return yield* io(host.connect).pipe(Effect.flatMap(session => attempt(journal, session, "new")), Effect.tap(result => sync(() => { if (task.recipe && ["precondition_failed", "ambiguous_target", "assertion_failed"].includes(journal.state().reasonCode)) quarantineRecipe(host.home(), task.recipe, journal.state().reasonCode); })), Effect.catchTag("BrowserError", e => sync(() => { diff --git a/test/review-followups.smoke.mjs b/test/review-followups.smoke.mjs new file mode 100644 index 0000000..2c99fc5 --- /dev/null +++ b/test/review-followups.smoke.mjs @@ -0,0 +1,80 @@ +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import path from 'node:path'; +import { Effect } from 'effect'; +import { fixture } from './support/chrome.mjs'; +import { runTask, closeRun } from '../dist/workflows.js'; +import { Journal } from '../dist/journal.js'; + +const f = await fixture(), execute = task => Effect.runPromise(runTask(task, true)); +const key = 'FE2E_REVIEW_PRIVATE_INPUT', previous = process.env[key]; +const secret = 'secret-sentinel-never-retain'; +process.env[key] = secret; +const passed = []; +try { + for (const [source, field] of [ + [{ selector: 'input[name=displayName]' }, { selector: '#name' }], + [{ selector: '#frameName', frames: ['iframe#remote'] }, { selector: '#frameName', frames: ['#remote'] }], + [{ selector: 'open-host input', shadow: 'open' }, { selector: '#shadowInput', shadow: 'open' }], + ]) { + const result = await execute({ name: 'Environment alias cannot be retained', url: f.url, goal: 'Fill an environment-backed field', keepTab: true, + steps: [{ kind: 'fill', ...source, valueFromEnv: key }], + checks: [{ kind: 'text', selector: 'h1', value: 'Settings' }], + recovery: { fields: [{ key: 'private', ...field }] }, + }); + assert.equal(result.status, 'blocked', JSON.stringify(result)); + assert.equal(result.execution.reasonCode, 'retention_forbidden'); + const j = new Journal(f.directory, result.runId); + assert.deepEqual(j.recoveryFields(), {}); + const files = fs.readdirSync(j.dir, { recursive: true }).filter(file => fs.statSync(path.join(j.dir, file)).isFile()); + for (const file of files) assert.equal(fs.readFileSync(path.join(j.dir, file), 'utf8').includes(secret), false, file); + assert.equal(j.events().some(e => e.type === 'action.start' && e.data.kind === 'fill'), false); + await Effect.runPromise(closeRun(result.runId)); + } + passed.push('alias selectors, frame aliases, and open-root aliases cannot retain environment values'); + + const unrelated = await execute({ name: 'Independent recovery stays usable', url: f.url, goal: 'Fill independent controls', keepTab: true, + steps: [{ kind: 'fill', selector: '#other', valueFromEnv: key }, { kind: 'fill', selector: '#name', value: 'Recoverable' }], + checks: [{ kind: 'value', selector: '#name', value: 'Recoverable' }], + recovery: { fields: [{ key: 'name', selector: '#name' }] }, + }); + assert.equal(unrelated.status, 'passed', JSON.stringify(unrelated)); + assert.equal(new Journal(f.directory, unrelated.runId).recoveryFields().name.value, 'Recoverable'); + passed.push('unrelated allowlisted fields still capture correctly'); + + const input = { name: 'Retry preflight without replaying Save', url: f.url, goal: 'Save once', requestId: 'recover-preflight', keepTab: true, + steps: [{ kind: 'fill', selector: '#name', value: 'Preflight recovered' }, { kind: 'click', selector: '#save' }], + checks: [{ kind: 'text', selector: '#status', value: 'Saved: Preflight recovered' }], + }; + const activeFile = path.join(f.profile, 'DevToolsActivePort'), active = fs.readFileSync(activeFile); + fs.rmSync(activeFile); + let first; + try { + first = await execute(input); + assert.equal(first.status, 'blocked'); + assert.equal(first.targetId, ''); + assert.equal(first.attempts.length, 0); + const second = await execute(input); + assert.equal(second.runId, first.runId); + assert.ok(second.revision > first.revision, 'failed preflight must be retried, not cached'); + } finally { fs.writeFileSync(activeFile, active); } + const writes = f.writes; + const success = await execute(input); + assert.equal(success.runId, first.runId); + assert.equal(success.status, 'passed', JSON.stringify(success)); + assert.equal(success.attempts.length, 1); + assert.equal(f.writes, writes + 1); + // A cached success needs neither the live endpoint nor another Save. + fs.rmSync(activeFile); + try { + const duplicate = await execute(input); + assert.equal(duplicate.duplicate, true); + assert.equal(duplicate.revision, success.revision); + assert.equal(f.writes, writes + 1); + } finally { fs.writeFileSync(activeFile, active); } + passed.push('same request survives failed preflight, then dispatches Save exactly once'); + console.log(JSON.stringify({ suite: 'PR review regressions', passed, paidModelCalls: 0 })); +} finally { + if (previous === undefined) delete process.env[key]; else process.env[key] = previous; + await f.cleanup(); +} diff --git a/test/review-followups.test.mjs b/test/review-followups.test.mjs new file mode 100644 index 0000000..f52487b --- /dev/null +++ b/test/review-followups.test.mjs @@ -0,0 +1,90 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import * as fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { Effect } from 'effect'; +import { runTask } from '../dist/workflows.js'; +import { Journal } from '../dist/journal.js'; +import { validateTask } from '../dist/task.js'; + +const base = { url: 'https://example.com/settings', goal: 'Use a local credential', + steps: [{ kind: 'fill', selector: '#token', valueFromEnv: 'FE2E_PRIVATE_INPUT' }] }; + +for (const scope of [{}, { frames: ['#frame'] }, { frames: ['#frame', '#nested'], shadow: 'open' }]) { + test(`environment input and recovery cannot overlap: ${JSON.stringify(scope)}`, () => { + const task = { ...base, steps: [{ ...base.steps[0], ...scope }], + recovery: { fields: [{ key: 'token', selector: '#token', ...scope }] } }; + assert.throws(() => validateTask(task, false), /environment-backed/i); + }); +} +test('scope defaults and selector whitespace do not bypass retention validation', () => { + assert.throws(() => validateTask({ ...base, + recovery: { fields: [{ key: 'token', selector: ' #token ', frames: [], shadow: 'none' }] }, + }, false), /environment-backed/i); +}); +test('reconstruction environment inputs obey the same retention rule', () => { + assert.throws(() => validateTask({ ...base, steps: undefined, recovery: { + fields: [{ key: 'token', selector: '#token' }], reconstruct: [ + { kind: 'navigate', value: base.url, safeToRepeat: true }, + { ...base.steps[0], safeToRepeat: true }, + ], + } }, false), /environment-backed/i); +}); +test('unrelated recovery fields and distinct frame scopes remain usable', () => { + for (const field of [{ key: 'name', selector: '#name' }, { key: 'token', selector: '#token', frames: ['#frame'] }]) { + validateTask({ ...base, recovery: { fields: [field] } }, false); + } +}); + +function isolatedHome(t) { + const previous = process.env.FASTEST_E2E_HOME; + const root = fs.mkdtempSync(path.join(os.tmpdir(), 'fe2e-preflight-')); + const profile = path.join(root, 'profile'); fs.mkdirSync(profile); + fs.writeFileSync(path.join(root, 'config.json'), JSON.stringify({ version: 1, profileDir: profile, chromeExecutable: process.execPath })); + process.env.FASTEST_E2E_HOME = root; + t.after(() => { + if (previous === undefined) delete process.env.FASTEST_E2E_HOME; + else process.env.FASTEST_E2E_HOME = previous; + fs.rmSync(root, { recursive: true, force: true }); + }); + return root; +} +const execute = input => Effect.runPromise(runTask(input)); + +test('duplicate request retries failed preflight in the same never-started run', async t => { + const root = isolatedHome(t); + const input = { ...base, requestId: 'preflight-once' }; + const first = await execute(input), j = new Journal(root, first.runId); + assert.equal(first.status, 'blocked'); + assert.deepEqual(j.events().map(e => e.type), ['created', 'preflight.failed']); + for (let n = 2; n <= 3; n++) { + const retry = await execute(input); + assert.equal(retry.runId, first.runId); + assert.equal(j.events().filter(e => e.type === 'preflight.failed').length, n); + assert.deepEqual(j.state().attempts, []); + assert.equal(j.state().actionsUsed, 0); + assert.equal(j.state().modelCallsUsed, 0); + } + await assert.rejects(execute({ ...input, goal: 'Conflicting request' }), /different inputs/); +}); + +test('duplicates cannot retry once any execution boundary or unknown event exists', async t => { + const root = isolatedHome(t); + for (const event of [ + ['attempt.start', { mode: 'new', engine: 'jev', allocatedMs: 1000 }], + ['target', { targetId: 'owned', browserId: 'original' }], + ['action.start', { id: 'save', safeToRepeat: false }], + ['call.start', { engine: 'jev' }], + ['future.execution.event', {}], + ]) { + const input = { ...base, requestId: event[0] }; + const first = await execute(input), j = new Journal(root, first.runId); + j.append(...event); + const revision = j.state().revision; + const duplicate = await execute(input); + assert.equal(duplicate.duplicate, true); + assert.equal(duplicate.runId, first.runId); + assert.equal(j.state().revision, revision, `${event[0]} must prevent redispatch`); + } +}); diff --git a/test/support/chrome.mjs b/test/support/chrome.mjs index 276ba31..5e67e78 100644 --- a/test/support/chrome.mjs +++ b/test/support/chrome.mjs @@ -52,7 +52,7 @@ export async function fixture() { child = spawn(chromePath, [`--user-data-dir=${profile}`, '--remote-debugging-port=0', '--remote-debugging-address=127.0.0.1', '--headless=new', '--no-first-run', '--no-default-browser-check', '--site-per-process', '--window-size=1280,900', ...(process.getuid?.()===0 ? ['--no-sandbox'] : []), 'about:blank'], { stdio: ['ignore', 'ignore', 'pipe'], detached: true }); child.stderr.on('data', chunk => { diagnostic = (diagnostic + chunk.toString()).slice(-8_192); }); child.once('error', error => { spawnError = error; }); - const deadline = Date.now() + 15_000; + const deadline = Date.now() + 30_000; let last; while (Date.now() < deadline && alive() && !spawnError) { try { return await connect(); } catch (error) { last = error; } diff --git a/worker/jev_runner.py b/worker/jev_runner.py index 6a0d6e4..2fd9c6a 100644 --- a/worker/jev_runner.py +++ b/worker/jev_runner.py @@ -70,17 +70,23 @@ def ids(t): journal.append("checkpoint", documentId=document_id, fingerprint=hashlib.sha256(live.encode()).hexdigest(), url=json.loads(live)["url"]) def capture(browser, journal, action=None, text=None): - fields = journal.meta["task"].get("recovery", {}).get("fields", []) + task = journal.meta["task"] + fields = task.get("recovery", {}).get("fields", []) + # Capture also runs after a scoped/environment-backed step hands off to Jev. + # Protect aliases in the top-level document without reading secret values. + protected = [s["selector"] for s in [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])] + if s.get("valueFromEnv") is not None and s.get("selector") and not s.get("frames")] for field in fields: if field.get("frames") or field.get("shadow") == "open": continue # Jev cannot type into these; the scoped adapter captures them. result = browser.evaluate("""(x=>{ const nodes=[...document.querySelectorAll(x.selector)]; if(nodes.length!==1)return null; const e=nodes[0]; + if(x.protected.some(selector=>e.matches(selector)))return {forbidden:true}; if(e.matches('input[type=password],input[type=file],[autocomplete=one-time-code]'))return {forbidden:true}; if(!('value' in e))return {forbidden:true}; return {value:String(e.value),matches:x.node!=null&&window.__jevFast?.nodes.get(x.node)===e}; - })(""" + json.dumps(dict(selector=field["selector"], node=action.get("node") if action else None)) + ")") + })(""" + json.dumps(dict(selector=field["selector"], protected=protected, node=action.get("node") if action else None)) + ")") if not result: continue if result.get("forbidden"): raise BridgeError("retention_forbidden", "A recovery field targets sensitive or unsupported input.") intended = action and action.get("kind") == "fill" and result.get("matches") and text is not None diff --git a/worker/tests/managed_smoke.py b/worker/tests/managed_smoke.py index 28b4ea3..6b5b98a 100644 --- a/worker/tests/managed_smoke.py +++ b/worker/tests/managed_smoke.py @@ -80,6 +80,21 @@ def make_run(**extra): saved = next(e["data"] for e in reversed(events) if e["type"] == "checkpoint") assert observed["documentId"] == saved["documentId"], (observed, saved) assert observed["fingerprint"] == saved["fingerprint"], (observed, saved) + # Saved v0.2.0 tasks and selector aliases must not bypass capture protection. + secret = "managed-secret-sentinel" + page.evaluate("document.querySelector('#name').value=" + json.dumps(secret)) + for location in ("steps", "reconstruct"): + protected_step = {"kind": "fill", "selector": "input", "valueFromEnv": "PRIVATE_INPUT"} + recovery = {"fields": [{"key": "name", "selector": "#name"}]} + extra = {"steps": [protected_step]} if location == "steps" else {} + if location == "reconstruct": recovery["reconstruct"] = [protected_step] + protected_journal, _ = make_run(recovery=recovery, **extra) + try: jev_runner.capture(page, protected_journal) + except bridge.BridgeError as error: assert error.code == "retention_forbidden", error.code + else: raise AssertionError("Environment-backed selector alias was captured") + assert not (protected_journal.directory / "recovery.json").exists() + assert secret not in protected_journal.file.read_text() + page.evaluate("document.querySelector('#name').value='CI value'") # Upstream retries StalePage, but the wrapper must never retry it after dispatch. second, req = make_run() original_act = Browser.act @@ -101,7 +116,7 @@ def choose_save(page, _goal, _history): except bridge.BridgeError as error: assert error.code == "budget_exhausted", error.code else: raise AssertionError("Call budget ignored") page.close() - print(json.dumps({"suite": "managed Jev", "paidModelCalls": 0, "checks": ["actual upstream loop", "same owned tab", "durable dispatch receipts", "allowlisted fields", "cross-language checkpoints", "no post-dispatch stale retry", "shared call budget"]})) + print(json.dumps({"suite": "managed Jev", "paidModelCalls": 0, "checks": ["actual upstream loop", "same owned tab", "durable dispatch receipts", "allowlisted fields", "cross-language checkpoints", "no post-dispatch stale retry", "shared call budget", "environment-backed capture aliases rejected"]})) finally: with contextlib.suppress(Exception): from browser_harness.admin import restart_daemon From dd8461ae21aef511c8f67b8829a2d3369e79be00 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 11:44:02 +0200 Subject: [PATCH 02/24] fix: resolve protected Jev selectors in document scope Address PR #4 review comment 4058589871. Resolve protected selectors with document.querySelectorAll and compare actual DOM node identity instead of Element.matches, whose :scope root differs. Extend the managed-Jev browser regression across normal/reconstruction plans, :scope descendants and child selectors, selector lists, and multiple matches with a decoy first. Assert protected values are never read or retained and unrelated recovery fields still work. Locally reproduced disk retention with the original capture function in Chromium, then passed all 10 protected cases and the unrelated-field control with the fix. Python syntax checks pass. Full upstream and repository validation remains in CI. No new docs, dependencies, or workflow changes. --- worker/jev_runner.py | 3 ++- worker/tests/managed_smoke.py | 51 ++++++++++++++++++++++++++--------- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/worker/jev_runner.py b/worker/jev_runner.py index 2fd9c6a..c38cc44 100644 --- a/worker/jev_runner.py +++ b/worker/jev_runner.py @@ -82,7 +82,8 @@ def capture(browser, journal, action=None, text=None): result = browser.evaluate("""(x=>{ const nodes=[...document.querySelectorAll(x.selector)]; if(nodes.length!==1)return null; const e=nodes[0]; - if(x.protected.some(selector=>e.matches(selector)))return {forbidden:true}; + // Keep document scope: Element.matches() would rebind :scope to e. + if(x.protected.some(selector=>[...document.querySelectorAll(selector)].includes(e)))return {forbidden:true}; if(e.matches('input[type=password],input[type=file],[autocomplete=one-time-code]'))return {forbidden:true}; if(!('value' in e))return {forbidden:true}; return {value:String(e.value),matches:x.node!=null&&window.__jevFast?.nodes.get(x.node)===e}; diff --git a/worker/tests/managed_smoke.py b/worker/tests/managed_smoke.py index 6b5b98a..98c260f 100644 --- a/worker/tests/managed_smoke.py +++ b/worker/tests/managed_smoke.py @@ -80,20 +80,47 @@ def make_run(**extra): saved = next(e["data"] for e in reversed(events) if e["type"] == "checkpoint") assert observed["documentId"] == saved["documentId"], (observed, saved) assert observed["fingerprint"] == saved["fingerprint"], (observed, saved) - # Saved v0.2.0 tasks and selector aliases must not bypass capture protection. + # Prefilled saved runs must protect document-scoped aliases before reading values. secret = "managed-secret-sentinel" page.evaluate("document.querySelector('#name').value=" + json.dumps(secret)) + page.evaluate("""(() => { + const decoy = document.createElement('input'); + decoy.id = 'decoy'; decoy.value = 'Public value'; + const control = document.querySelector('#name'); + control.before(decoy); + window.__protectedValueReads = 0; + const descriptor = Object.getOwnPropertyDescriptor(HTMLInputElement.prototype, 'value'); + Object.defineProperty(control, 'value', {configurable: true, + get() { window.__protectedValueReads++; return descriptor.get.call(this); }, + set(value) { descriptor.set.call(this, value); } + }); + })()""") + # :scope input matches the decoy first; all results must be checked by identity. for location in ("steps", "reconstruct"): - protected_step = {"kind": "fill", "selector": "input", "valueFromEnv": "PRIVATE_INPUT"} - recovery = {"fields": [{"key": "name", "selector": "#name"}]} - extra = {"steps": [protected_step]} if location == "steps" else {} - if location == "reconstruct": recovery["reconstruct"] = [protected_step] - protected_journal, _ = make_run(recovery=recovery, **extra) - try: jev_runner.capture(page, protected_journal) - except bridge.BridgeError as error: assert error.code == "retention_forbidden", error.code - else: raise AssertionError("Environment-backed selector alias was captured") - assert not (protected_journal.directory / "recovery.json").exists() - assert secret not in protected_journal.file.read_text() + for selector in ("input", ":scope #name", ":scope > body > #name", + "#missing, :scope #name", ":scope input"): + protected_step = {"kind": "fill", "selector": selector, "valueFromEnv": "PRIVATE_INPUT"} + recovery = {"fields": [{"key": "name", "selector": "#name"}]} + extra = {"steps": [protected_step]} if location == "steps" else {} + if location == "reconstruct": recovery["reconstruct"] = [protected_step] + protected_journal, _ = make_run(recovery=recovery, **extra) + try: jev_runner.capture(page, protected_journal) + except bridge.BridgeError as error: assert error.code == "retention_forbidden", error.code + else: raise AssertionError(f"Environment-backed alias was captured: {location}, {selector}") + assert not (protected_journal.directory / "recovery.json").exists() + assert secret not in protected_journal.file.read_text() + assert page.evaluate("window.__protectedValueReads") == 0 + # Protecting another control must not disable ordinary recovery capture. + unrelated, _ = make_run( + steps=[{"kind": "fill", "selector": ":scope #name", "valueFromEnv": "PRIVATE_INPUT"}], + recovery={"fields": [{"key": "public", "selector": "#decoy"}]}, + ) + jev_runner.capture(page, unrelated) + recovered = json.loads((unrelated.directory / "recovery.json").read_text()) + assert recovered["public"]["value"] == "Public value" + assert secret not in (unrelated.directory / "recovery.json").read_text() + assert page.evaluate("window.__protectedValueReads") == 0 + page.evaluate("delete document.querySelector('#name').value; document.querySelector('#decoy').remove()") page.evaluate("document.querySelector('#name').value='CI value'") # Upstream retries StalePage, but the wrapper must never retry it after dispatch. second, req = make_run() @@ -116,7 +143,7 @@ def choose_save(page, _goal, _history): except bridge.BridgeError as error: assert error.code == "budget_exhausted", error.code else: raise AssertionError("Call budget ignored") page.close() - print(json.dumps({"suite": "managed Jev", "paidModelCalls": 0, "checks": ["actual upstream loop", "same owned tab", "durable dispatch receipts", "allowlisted fields", "cross-language checkpoints", "no post-dispatch stale retry", "shared call budget", "environment-backed capture aliases rejected"]})) + print(json.dumps({"suite": "managed Jev", "paidModelCalls": 0, "checks": ["actual upstream loop", "same owned tab", "durable dispatch receipts", "allowlisted fields", "cross-language checkpoints", "no post-dispatch stale retry", "shared call budget", "document-scoped protected aliases rejected before value reads", "unrelated recovery remains usable"]})) finally: with contextlib.suppress(Exception): from browser_harness.admin import restart_daemon From 0a084222365a6d3f918966ce2d4deccacc5f518c Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:50:10 +0200 Subject: [PATCH 03/24] Make recovery guard atomic with capture --- src/page-worker.ts | 52 +++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 24 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index 0b95b64..bdeb3e2 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -75,12 +75,11 @@ export async function pollChecks(page: Page, checks: readonly Check[], timeoutMs await new Promise(resolve => setTimeout(resolve, 100)); } } -async function assertRecoveryTargets(page: Page, journal: Journal): Promise { +async function captureFields(page: Page, journal: Journal): Promise { const task = journal.meta().task; const fields = task.recovery?.fields ?? []; const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); - if (!fields.length || !protectedSteps.length) return; const handles: ElementHandle[] = []; try { const protectedNodes: { node: ElementHandle; frame: Frame | null }[] = []; @@ -90,32 +89,37 @@ async function assertRecoveryTargets(page: Page, journal: Journal): Promise p.frame === frame).map(p => p.node); - if (candidates.length && await node.evaluate((element, others) => others.includes(element), candidates)) { - throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); - } + for (const f of fields) { + const { locator } = await scoped(page, f); + const nodes = await locator.elementHandles(); + handles.push(...nodes); + if (nodes.length !== 1) continue; + const node = nodes[0]!; + const frame = await node.ownerFrame(); + const candidates = protectedNodes.filter(p => p.frame === frame).map(p => p.node); + // Pin the recovery target to one DOM node, then compare identity and read + // its value in the same page evaluation. A rerender cannot swap the + // recovery selector to an environment-backed control between the guard + // and the read. + const result = await node.evaluate((element, others) => { + if (others.includes(element)) return { actual: "", protected: true }; + const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + const rect = element.getBoundingClientRect(); + const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); + if (!visible) return { actual: "", available: false }; + if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; + if (!("value" in element)) return { actual: "", error: "Target is not a value control." }; + return { actual: String((element as HTMLInputElement).value) }; + }, candidates); + if (result.protected) { + throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } + if (result.error) throw new BrowserError({ code: "retention_forbidden", reason: "A recovery allowlist includes a sensitive or unsupported control. Remove it; no value was saved." }); + if (result.available === false) continue; + journal.saveField(f.key, result.actual, "observed"); } } finally { await Promise.allSettled(handles.map(handle => handle.dispose())); } } -async function captureFields(page: Page, journal: Journal): Promise { - await assertRecoveryTargets(page, journal); - for (const f of journal.meta().task.recovery?.fields ?? []) { - const { locator } = await scoped(page, f); - if (await locator.count() !== 1) continue; - const result = await locator.evaluate(readElement, { kind: "value" }); - if (result.error) throw new BrowserError({ code: "retention_forbidden", reason: "A recovery allowlist includes a sensitive or unsupported control. Remove it; no value was saved." }); - if (result.available === false) continue; - journal.saveField(f.key, result.actual, "observed"); - } -} function valueFor(s: Step, journal: Journal, retained = journal.recoveryFields()): string { if (s.valueFromInput) { const value = journal.meta().task.inputs?.[s.valueFromInput]; From 5e7ab19d14543b9e4a1ec51bcdbdf2277e9bc30e Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:50:58 +0200 Subject: [PATCH 04/24] Add recovery selector race fixture --- test/support/chrome.mjs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/support/chrome.mjs b/test/support/chrome.mjs index 5e67e78..d9947ce 100644 --- a/test/support/chrome.mjs +++ b/test/support/chrome.mjs @@ -16,6 +16,19 @@ export async function fixture() { const route = (req,res) => { res.setHeader('Content-Type','text/html; charset=utf-8'); if (req.url.startsWith('/save')) { writes++; res.end('saved'); return; } + if (req.url.startsWith('/recovery-race')) { res.end(` +

Recovery race

+ `); return; } if (req.url.startsWith('/nested')) { res.end('

Nested

'); return; } if (req.url.startsWith('/frame')) { res.end(`

Remote frame

`); return; } res.end(`Fixture From 345266fd9b6e497abdeba1874511d33ccce928ca Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:51:01 +0200 Subject: [PATCH 05/24] Cover recovery selector swap race --- test/review-followups.smoke.mjs | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/review-followups.smoke.mjs b/test/review-followups.smoke.mjs index 2c99fc5..a406d9e 100644 --- a/test/review-followups.smoke.mjs +++ b/test/review-followups.smoke.mjs @@ -33,6 +33,19 @@ try { } passed.push('alias selectors, frame aliases, and open-root aliases cannot retain environment values'); + const raced = await execute({ name: 'Recovery selector cannot swap after identity guard', url: f.url + '/recovery-race', goal: 'Fill an environment-backed field', keepTab: true, + steps: [{ kind: 'fill', selector: '#token', valueFromEnv: key }], + checks: [{ kind: 'text', selector: 'h1', value: 'Recovery race' }], + recovery: { fields: [{ key: 'active', selector: '.active-input' }] }, + }); + assert.equal(raced.status, 'passed', JSON.stringify(raced)); + const racedJournal = new Journal(f.directory, raced.runId); + assert.equal(racedJournal.recoveryFields().active.value, 'public-sentinel'); + const racedFiles = fs.readdirSync(racedJournal.dir, { recursive: true }).filter(file => fs.statSync(path.join(racedJournal.dir, file)).isFile()); + for (const file of racedFiles) assert.equal(fs.readFileSync(path.join(racedJournal.dir, file), 'utf8').includes(secret), false, file); + await Effect.runPromise(closeRun(raced.runId)); + passed.push('recovery target identity is pinned atomically with its value read'); + const unrelated = await execute({ name: 'Independent recovery stays usable', url: f.url, goal: 'Fill independent controls', keepTab: true, steps: [{ kind: 'fill', selector: '#other', valueFromEnv: key }, { kind: 'fill', selector: '#name', value: 'Recoverable' }], checks: [{ kind: 'value', selector: '#name', value: 'Recoverable' }], From e02af9206b4d8c8754c2630614cae04ce8eb091c Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:52:56 +0200 Subject: [PATCH 06/24] Fix atomic recovery capture typecheck --- src/page-worker.ts | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index bdeb3e2..4b61139 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -75,7 +75,7 @@ export async function pollChecks(page: Page, checks: readonly Check[], timeoutMs await new Promise(resolve => setTimeout(resolve, 100)); } } -async function captureFields(page: Page, journal: Journal): Promise { +async function captureFields(page: Page, journal: Journal, save = true): Promise { const task = journal.meta().task; const fields = task.recovery?.fields ?? []; const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] @@ -102,18 +102,21 @@ async function captureFields(page: Page, journal: Journal): Promise { // recovery selector to an environment-backed control between the guard // and the read. const result = await node.evaluate((element, others) => { + const control = element as Element; if (others.includes(element)) return { actual: "", protected: true }; - const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); - const rect = element.getBoundingClientRect(); - const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); + if (!save) return { actual: "" }; + const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + const rect = control.getBoundingClientRect(); + const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); if (!visible) return { actual: "", available: false }; if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!("value" in element)) return { actual: "", error: "Target is not a value control." }; - return { actual: String((element as HTMLInputElement).value) }; + if (!("value" in control)) return { actual: "", error: "Target is not a value control." }; + return { actual: String((control as HTMLInputElement).value) }; }, candidates); if (result.protected) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } + if (!save) continue; if (result.error) throw new BrowserError({ code: "retention_forbidden", reason: "A recovery allowlist includes a sensitive or unsupported control. Remove it; no value was saved." }); if (result.available === false) continue; journal.saveField(f.key, result.actual, "observed"); @@ -151,7 +154,7 @@ async function runSteps(page: Page, journal: Journal, job: PageJob, snap: () => throw new BrowserError({ code: "ambiguous_target", reason: "Action requires one matching control. Inspect the scoped page; no action was dispatched." }); } if (!rebuilding) await captureFields(page, journal); - else if (s.valueFromEnv) await assertRecoveryTargets(page, journal); + else if (s.valueFromEnv) await captureFields(page, journal, false); if (s.kind === "fill") for (const f of journal.meta().task.recovery?.fields ?? []) { if (f.selector === s.selector && JSON.stringify(f.frames) === JSON.stringify(s.frames) && f.shadow === s.shadow && !s.valueFromEnv) { const result = await locator.evaluate(readElement, { kind: "value" }); From 30e5bf02f2ffa79f211f849a0bfaa90c41ed83a0 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:53:13 +0200 Subject: [PATCH 07/24] Serialize recovery capture mode explicitly --- src/page-worker.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index 4b61139..94a97a0 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -101,10 +101,10 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise // its value in the same page evaluation. A rerender cannot swap the // recovery selector to an environment-backed control between the guard // and the read. - const result = await node.evaluate((element, others) => { + const result = await node.evaluate((element, input) => { const control = element as Element; - if (others.includes(element)) return { actual: "", protected: true }; - if (!save) return { actual: "" }; + if (input.others.includes(element)) return { actual: "", protected: true }; + if (!input.save) return { actual: "" }; const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); const rect = control.getBoundingClientRect(); const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); @@ -112,7 +112,7 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; if (!("value" in control)) return { actual: "", error: "Target is not a value control." }; return { actual: String((control as HTMLInputElement).value) }; - }, candidates); + }, { others: candidates, save }); if (result.protected) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } From 8389c91165dff66d52ac24ae0fe988467dc418d7 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:58:05 +0200 Subject: [PATCH 08/24] Resolve protected recovery targets at read time --- src/page-worker.ts | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index 94a97a0..4dee013 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -82,13 +82,17 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); const handles: ElementHandle[] = []; try { - const protectedNodes: { node: ElementHandle; frame: Frame | null }[] = []; + const protectedScopes: { selector: string; frame: Frame | null }[] = []; for (const step of protectedSteps) { - const { locator } = await scoped(page, step); - const nodes = await locator.elementHandles(); - handles.push(...nodes); - for (const node of nodes) protectedNodes.push({ node, frame: await node.ownerFrame() }); + const { root } = await scoped(page, { frames: step.frames, shadow: step.shadow }); + const documentElement = await root.locator("html").elementHandle(); + if (!documentElement) continue; + handles.push(documentElement); + protectedScopes.push({ selector: step.selector!, frame: await documentElement.ownerFrame() }); } + const protectedValues = protectedSteps + .map(step => process.env[step.valueFromEnv!]) + .filter((value): value is string => value !== undefined && value.length > 0); for (const f of fields) { const { locator } = await scoped(page, f); const nodes = await locator.elementHandles(); @@ -96,23 +100,29 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (nodes.length !== 1) continue; const node = nodes[0]!; const frame = await node.ownerFrame(); - const candidates = protectedNodes.filter(p => p.frame === frame).map(p => p.node); - // Pin the recovery target to one DOM node, then compare identity and read - // its value in the same page evaluation. A rerender cannot swap the - // recovery selector to an environment-backed control between the guard - // and the read. + const selectors = protectedScopes.filter(p => p.frame === frame).map(p => p.selector); + // Pin the recovery target once. Inside that same browser evaluation, + // resolve current protected selectors against the pinned node and compare + // its value against environment-backed values before anything crosses + // the protocol boundary or is persisted. const result = await node.evaluate((element, input) => { const control = element as Element; - if (input.others.includes(element)) return { actual: "", protected: true }; + const selectorProtected = input.selectors.some(selector => { + try { return Array.from(control.ownerDocument.querySelectorAll(selector)).includes(control); } + catch { return false; } + }); + const hasValue = "value" in control; + const rawValue = hasValue ? String((control as HTMLInputElement).value) : ""; + if (selectorProtected || input.protectedValues.includes(rawValue)) return { actual: "", protected: true }; if (!input.save) return { actual: "" }; const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); const rect = control.getBoundingClientRect(); const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); if (!visible) return { actual: "", available: false }; if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!("value" in control)) return { actual: "", error: "Target is not a value control." }; - return { actual: String((control as HTMLInputElement).value) }; - }, { others: candidates, save }); + if (!hasValue) return { actual: "", error: "Target is not a value control." }; + return { actual: rawValue }; + }, { selectors, protectedValues, save }); if (result.protected) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } From db700b45510a3ff99c276a1e099b2d643bc85d67 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:58:23 +0200 Subject: [PATCH 09/24] Add protected replacement race fixture --- test/support/chrome.mjs | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/support/chrome.mjs b/test/support/chrome.mjs index d9947ce..6b47c19 100644 --- a/test/support/chrome.mjs +++ b/test/support/chrome.mjs @@ -29,6 +29,21 @@ export async function fixture() { }}; patch(Document.prototype);patch(Element.prototype);patch(ShadowRoot.prototype); `); return; } + if (req.url.startsWith('/protected-replacement-race')) { res.end(` +

Protected replacement race

+ `); return; } if (req.url.startsWith('/nested')) { res.end('

Nested

'); return; } if (req.url.startsWith('/frame')) { res.end(`

Remote frame

`); return; } res.end(`Fixture From 4aa5d52c6fd6170ecfbd98597617c501f345f9de Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 12:58:25 +0200 Subject: [PATCH 10/24] Cover protected node replacement race --- test/review-followups.smoke.mjs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/review-followups.smoke.mjs b/test/review-followups.smoke.mjs index a406d9e..da9129d 100644 --- a/test/review-followups.smoke.mjs +++ b/test/review-followups.smoke.mjs @@ -46,6 +46,20 @@ try { await Effect.runPromise(closeRun(raced.runId)); passed.push('recovery target identity is pinned atomically with its value read'); + const replaced = await execute({ name: 'Protected target replacement cannot leak recovery', url: f.url + '/protected-replacement-race', goal: 'Fill an environment-backed field', keepTab: true, + steps: [{ kind: 'fill', selector: '#token', valueFromEnv: key }], + checks: [{ kind: 'text', selector: 'h1', value: 'Protected replacement race' }], + recovery: { fields: [{ key: 'active', selector: '.active-input' }] }, + }); + assert.equal(replaced.status, 'blocked', JSON.stringify(replaced)); + assert.equal(replaced.execution.reasonCode, 'retention_forbidden'); + const replacedJournal = new Journal(f.directory, replaced.runId); + assert.notEqual(replacedJournal.recoveryFields().active?.value, secret); + const replacedFiles = fs.readdirSync(replacedJournal.dir, { recursive: true }).filter(file => fs.statSync(path.join(replacedJournal.dir, file)).isFile()); + for (const file of replacedFiles) assert.equal(fs.readFileSync(path.join(replacedJournal.dir, file), 'utf8').includes(secret), false, file); + await Effect.runPromise(closeRun(replaced.runId)); + passed.push('replaced protected nodes cannot leak their environment-backed value'); + const unrelated = await execute({ name: 'Independent recovery stays usable', url: f.url, goal: 'Fill independent controls', keepTab: true, steps: [{ kind: 'fill', selector: '#other', valueFromEnv: key }, { kind: 'fill', selector: '#name', value: 'Recoverable' }], checks: [{ kind: 'value', selector: '#name', value: 'Recoverable' }], From b92b1622c6c7b992828b5920b1f4b12fd24ce4fc Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:02:40 +0200 Subject: [PATCH 11/24] Preserve scoped alias preflight while keeping capture atomic --- src/page-worker.ts | 26 ++++++++++++++++++++++---- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index 4dee013..dc00cb7 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -82,8 +82,20 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); const handles: ElementHandle[] = []; try { + // Preflight runs before any environment-backed action, so resolve aliases + // with Playwright's full scoping semantics (including open shadow roots). + // Capture after the action does not trust this snapshot. + const preflightNodes: { node: ElementHandle; frame: Frame | null }[] = []; + if (!save) { + for (const step of protectedSteps) { + const { locator } = await scoped(page, step); + const nodes = await locator.elementHandles(); + handles.push(...nodes); + for (const node of nodes) preflightNodes.push({ node, frame: await node.ownerFrame() }); + } + } const protectedScopes: { selector: string; frame: Frame | null }[] = []; - for (const step of protectedSteps) { + if (save) for (const step of protectedSteps) { const { root } = await scoped(page, { frames: step.frames, shadow: step.shadow }); const documentElement = await root.locator("html").elementHandle(); if (!documentElement) continue; @@ -100,6 +112,14 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (nodes.length !== 1) continue; const node = nodes[0]!; const frame = await node.ownerFrame(); + if (!save) { + const candidates = preflightNodes.filter(p => p.frame === frame).map(p => p.node); + const protectedAlias = await node.evaluate((element, others) => others.includes(element), candidates); + if (protectedAlias) { + throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); + } + continue; + } const selectors = protectedScopes.filter(p => p.frame === frame).map(p => p.selector); // Pin the recovery target once. Inside that same browser evaluation, // resolve current protected selectors against the pinned node and compare @@ -114,7 +134,6 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise const hasValue = "value" in control; const rawValue = hasValue ? String((control as HTMLInputElement).value) : ""; if (selectorProtected || input.protectedValues.includes(rawValue)) return { actual: "", protected: true }; - if (!input.save) return { actual: "" }; const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); const rect = control.getBoundingClientRect(); const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); @@ -122,11 +141,10 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; if (!hasValue) return { actual: "", error: "Target is not a value control." }; return { actual: rawValue }; - }, { selectors, protectedValues, save }); + }, { selectors, protectedValues }); if (result.protected) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } - if (!save) continue; if (result.error) throw new BrowserError({ code: "retention_forbidden", reason: "A recovery allowlist includes a sensitive or unsupported control. Remove it; no value was saved." }); if (result.available === false) continue; journal.saveField(f.key, result.actual, "observed"); From fc917505cd67c2797d0f9333c556aaeb221de928 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:04:13 +0200 Subject: [PATCH 12/24] Use shadow-aware locators for atomic recovery guard --- src/page-worker.ts | 91 +++++++++++++++++++++++++++------------------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index dc00cb7..cb724de 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -82,25 +82,13 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); const handles: ElementHandle[] = []; try { - // Preflight runs before any environment-backed action, so resolve aliases - // with Playwright's full scoping semantics (including open shadow roots). - // Capture after the action does not trust this snapshot. - const preflightNodes: { node: ElementHandle; frame: Frame | null }[] = []; - if (!save) { - for (const step of protectedSteps) { - const { locator } = await scoped(page, step); - const nodes = await locator.elementHandles(); - handles.push(...nodes); - for (const node of nodes) preflightNodes.push({ node, frame: await node.ownerFrame() }); - } - } - const protectedScopes: { selector: string; frame: Frame | null }[] = []; - if (save) for (const step of protectedSteps) { - const { root } = await scoped(page, { frames: step.frames, shadow: step.shadow }); + const protectedTargets: { locator: Locator; frame: Frame | null }[] = []; + for (const step of protectedSteps) { + const { root, locator } = await scoped(page, step); const documentElement = await root.locator("html").elementHandle(); if (!documentElement) continue; handles.push(documentElement); - protectedScopes.push({ selector: step.selector!, frame: await documentElement.ownerFrame() }); + protectedTargets.push({ locator, frame: await documentElement.ownerFrame() }); } const protectedValues = protectedSteps .map(step => process.env[step.valueFromEnv!]) @@ -112,36 +100,63 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (nodes.length !== 1) continue; const node = nodes[0]!; const frame = await node.ownerFrame(); + const current = protectedTargets.filter(p => p.frame === frame).map(p => p.locator); + if (!save) { - const candidates = preflightNodes.filter(p => p.frame === frame).map(p => p.node); - const protectedAlias = await node.evaluate((element, others) => others.includes(element), candidates); - if (protectedAlias) { - throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); + for (const protectedLocator of current) { + const overlaps = await protectedLocator.evaluateAll((elements, target) => elements.includes(target), node); + if (overlaps) { + throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); + } } continue; } - const selectors = protectedScopes.filter(p => p.frame === frame).map(p => p.selector); - // Pin the recovery target once. Inside that same browser evaluation, - // resolve current protected selectors against the pinned node and compare - // its value against environment-backed values before anything crosses - // the protocol boundary or is persisted. - const result = await node.evaluate((element, input) => { - const control = element as Element; - const selectorProtected = input.selectors.some(selector => { - try { return Array.from(control.ownerDocument.querySelectorAll(selector)).includes(control); } - catch { return false; } - }); - const hasValue = "value" in control; - const rawValue = hasValue ? String((control as HTMLInputElement).value) : ""; - if (selectorProtected || input.protectedValues.includes(rawValue)) return { actual: "", protected: true }; - const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); - const rect = control.getBoundingClientRect(); - const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); + + // Resolve every protected selector with Playwright's current, shadow-aware + // locator semantics, compare against the already-pinned recovery node, + // and read that same node in the very same browser evaluation. + let protectedLocator = current[0]; + for (const next of current.slice(1)) protectedLocator = protectedLocator!.or(next); + const inspect = (element: Element, values: string[], protectedBySelector: boolean) => { + const hasValue = "value" in element; + const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; + if (protectedBySelector || values.includes(rawValue)) return { actual: "", protected: true }; + const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + const rect = element.getBoundingClientRect(); + const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); if (!visible) return { actual: "", available: false }; if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; if (!hasValue) return { actual: "", error: "Target is not a value control." }; return { actual: rawValue }; - }, { selectors, protectedValues }); + }; + const result = protectedLocator + ? await protectedLocator.evaluateAll((elements, input) => { + const element = input.target as unknown as Element; + const hasValue = "value" in element; + const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; + if (elements.includes(element) || input.values.includes(rawValue)) return { actual: "", protected: true }; + const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + const rect = element.getBoundingClientRect(); + const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); + if (!visible) return { actual: "", available: false }; + if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; + if (!hasValue) return { actual: "", error: "Target is not a value control." }; + return { actual: rawValue }; + }, { target: node, values: protectedValues }) + : await node.evaluate((element, values) => { + const hasValue = "value" in element; + const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; + if (values.includes(rawValue)) return { actual: "", protected: true }; + const control = element as Element; + const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + const rect = control.getBoundingClientRect(); + const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); + if (!visible) return { actual: "", available: false }; + if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; + if (!hasValue) return { actual: "", error: "Target is not a value control." }; + return { actual: rawValue }; + }, protectedValues); + void inspect; if (result.protected) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } From ebd44e9b96480ed12608499821b461556c219638 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:04:25 +0200 Subject: [PATCH 13/24] Remove redundant recovery helper --- src/page-worker.ts | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index cb724de..9c728ce 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -117,18 +117,6 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise // and read that same node in the very same browser evaluation. let protectedLocator = current[0]; for (const next of current.slice(1)) protectedLocator = protectedLocator!.or(next); - const inspect = (element: Element, values: string[], protectedBySelector: boolean) => { - const hasValue = "value" in element; - const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; - if (protectedBySelector || values.includes(rawValue)) return { actual: "", protected: true }; - const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); - const rect = element.getBoundingClientRect(); - const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); - if (!visible) return { actual: "", available: false }; - if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!hasValue) return { actual: "", error: "Target is not a value control." }; - return { actual: rawValue }; - }; const result = protectedLocator ? await protectedLocator.evaluateAll((elements, input) => { const element = input.target as unknown as Element; @@ -156,7 +144,6 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (!hasValue) return { actual: "", error: "Target is not a value control." }; return { actual: rawValue }; }, protectedValues); - void inspect; if (result.protected) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } From 5cb911535373e68c360891ffe1454e28e20cd3cd Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:06:06 +0200 Subject: [PATCH 14/24] Fix locator identity callback types --- src/page-worker.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index 9c728ce..49955b9 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -104,7 +104,7 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise if (!save) { for (const protectedLocator of current) { - const overlaps = await protectedLocator.evaluateAll((elements, target) => elements.includes(target), node); + const overlaps = await protectedLocator.evaluateAll((elements, target) => elements.includes(target as HTMLElement | SVGElement), node); if (overlaps) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } @@ -122,7 +122,7 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise const element = input.target as unknown as Element; const hasValue = "value" in element; const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; - if (elements.includes(element) || input.values.includes(rawValue)) return { actual: "", protected: true }; + if (elements.includes(element as HTMLElement | SVGElement) || input.values.includes(rawValue)) return { actual: "", protected: true }; const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); const rect = element.getBoundingClientRect(); const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); From 1a7a48907f8f892ad1ada0e6a408022b1d95cef0 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Mon, 21 Sep 2026 13:09:22 +0200 Subject: [PATCH 15/24] Make protected replacement regression selector-engine independent --- test/support/chrome.mjs | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/test/support/chrome.mjs b/test/support/chrome.mjs index 6b47c19..7e28e15 100644 --- a/test/support/chrome.mjs +++ b/test/support/chrome.mjs @@ -32,17 +32,10 @@ export async function fixture() { if (req.url.startsWith('/protected-replacement-race')) { res.end(`

Protected replacement race

`); return; } if (req.url.startsWith('/nested')) { res.end('

Nested

'); return; } if (req.url.startsWith('/frame')) { res.end(`

Remote frame

`); return; } From 709bb24e474b4a032e3c6e24b7d24581da4b24eb Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:34:13 +0200 Subject: [PATCH 16/24] test: reproduce open recovery capture review findings Exercise environment-array exposure in page callbacks, future and nested iframe scopes, sensitive value getters, and existing fail-closed expectations through the real page worker. Use only local fixtures and synthetic environment values. --- .github/workflows/ci.yml | 3 + package.json | 2 +- test/recovery-capture.smoke.mjs | 178 ++++++++++++++++++++++++++++++++ 3 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 test/recovery-capture.smoke.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68a9fac..1a3f706 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,3 +49,6 @@ jobs: - name: Review regressions for retention and preflight if: always() && !cancelled() run: timeout 90s node test/review-followups.smoke.mjs + - name: Recovery capture isolation and late frames + if: always() && !cancelled() + run: timeout 120s node test/recovery-capture.smoke.mjs diff --git a/package.json b/package.json index 0f22e5a..b5efe18 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "test:python": "python3 -m unittest discover -s worker/tests -v", "check": "npm run typecheck && npm test && npm run test:python", "prepack": "npm run build", - "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs" + "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs" }, "dependencies": { "@effect/platform-node": "4.0.0-rc.116", diff --git a/test/recovery-capture.smoke.mjs b/test/recovery-capture.smoke.mjs new file mode 100644 index 0000000..02110c4 --- /dev/null +++ b/test/recovery-capture.smoke.mjs @@ -0,0 +1,178 @@ +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import path from 'node:path'; +import { fixture } from './support/chrome.mjs'; +import { Cdp } from '../dist/cdp.js'; +import { createRun, atomic } from '../dist/journal.js'; +import { attachPage, pageJob, checkOne } from '../dist/page-worker.js'; + +// All secrets and hostile pages in this suite are synthetic and local. +const key = 'FE2E_CAPTURE_PRIVATE_INPUT', previous = process.env[key]; +const secret = 'capture-secret-not-for-other-documents'; +process.env[key] = secret; +const f = await fixture(), raw = await Cdp.open(f.session); +const passed = [], failures = []; +const retentionError = error => error?.code === 'retention_forbidden'; + +function assertNotRetained(journal, value = secret) { + for (const file of fs.readdirSync(journal.dir, { recursive: true })) { + const full = path.join(journal.dir, file); + if (fs.statSync(full).isFile()) assert.equal(fs.readFileSync(full, 'utf8').includes(value), false, file); + } +} +async function scenario(name, task, markup, check) { + const { targetId } = await raw.send('Target.createTarget', { url: 'about:blank' }); + const connection = await attachPage(f.session, targetId); + try { + const page = connection.page; + page.setDefaultTimeout(5_000); + await page.goto(f.url, { waitUntil: 'load' }); + const remoteUrl = await page.locator('#remote').getAttribute('src'); + await page.setContent(typeof markup === 'function' ? markup(remoteUrl) : markup); + const input = { url: f.url, goal: name, ...task }; + const { journal } = createRun(f.directory, input, false, f.profile); + journal.append('target', { targetId, browserId: f.session.browserId }); + atomic(path.join(f.directory, 'targets', f.session.namespace, `${targetId}.json`), { targetId, browserId: f.session.browserId }); + const invoke = (operation = 'capture', extra = {}) => pageJob({ + root: f.directory, runId: journal.runId, session: f.session, targetId, + operation, steps: input.steps, timeoutMs: 20_000, ...extra, + }, new AbortController().signal); + await check({ page, journal, invoke }); + passed.push(name); + } catch (error) { + failures.push(name); + console.error(`${name}: ${error.stack ?? error}`); + } finally { + await connection.browser.close(); + await raw.send('Target.closeTarget', { targetId }); + } +} +async function watchEvaluationArrays(page) { + await page.evaluate(() => { + globalThis.evaluatedStrings = []; + const original = Array.prototype.includes; + Array.prototype.includes = function (...args) { + for (let i = 0; i < this.length; i++) if (typeof this[i] === 'string') globalThis.evaluatedStrings.push(this[i]); + return Reflect.apply(original, this, args); + }; + }); +} +const futurePage = remoteUrl => `

Before frame

+ `; +try { + for (const scope of ['same document', 'other origin']) { + await scenario(`No environment values enter recovery evaluations: ${scope}`, { + steps: [{ kind: 'fill', selector: '#token', valueFromEnv: key, ...(scope === 'other origin' ? { frames: ['#remote'] } : {}) }], + recovery: { fields: [{ key: 'public', selector: '#public' }] }, + }, remote => ``, async ({ page, journal, invoke }) => { + await watchEvaluationArrays(page); + await invoke(); + assert.equal((await page.evaluate(() => globalThis.evaluatedStrings)).includes(secret), false, 'Unrelated page learned an environment secret'); + assert.equal(journal.recoveryFields().public.value, 'public value'); + assertNotRetained(journal); + }); + } + await scenario('Host-side comparison rejects a reflected environment value', { + steps: [{ kind: 'fill', selector: '#frameName', frames: ['#remote'], valueFromEnv: key }], + recovery: { fields: [{ key: 'public', selector: '#public' }] }, + }, remote => ``, async ({ page, journal, invoke }) => { + // Simulate a value the site already knows. It must not be persisted as public input. + await page.locator('#public').fill(secret); + await watchEvaluationArrays(page); + await assert.rejects(() => invoke(), retentionError); + assert.equal((await page.evaluate(() => globalThis.evaluatedStrings)).includes(secret), false); + assert.deepEqual(journal.recoveryFields(), {}); + assertNotRetained(journal); + }); + + for (const retain of [false, true]) for (const nested of [false, true]) { + const frames = nested ? ['#late', '#nested'] : ['#late']; + const selector = nested ? '#nestedName' : '#frameName'; + await scenario(`Future ${nested ? 'nested ' : ''}frame does not block its creation; recovery=${retain}`, { + steps: [{ kind: 'click', selector: '#reveal' }, { kind: 'fill', selector, frames, valueFromEnv: key }], + ...(retain ? { recovery: { fields: [{ key: 'public', selector: '#public' }] } } : {}), + }, futurePage, async ({ page, journal, invoke }) => { + await invoke('steps'); + let scope = page; + for (const frame of frames) scope = scope.frameLocator(frame); + assert.equal(await scope.locator(selector).inputValue(), secret); + assert.equal(journal.events().filter(e => e.type === 'action.start').length, 2); + if (retain) assert.equal(journal.recoveryFields().public.value, 'keep me'); + assertNotRetained(journal); + }); + } + for (const reconstruct of [false, true]) { + const steps = [{ kind: 'click', selector: '#reveal' }, { kind: 'fill', selector: '#frameName', frames: ['iframe#late'], valueFromEnv: key }]; + await scenario(`Late-frame aliases are rejected before secret dispatch; reconstruct=${reconstruct}`, { + steps: reconstruct ? [] : steps, + recovery: { fields: [{ key: 'private', selector: 'input#frameName', frames: ['#late'] }], ...(reconstruct ? { reconstruct: steps } : {}) }, + }, futurePage, async ({ journal, invoke }) => { + await assert.rejects(() => invoke('steps', { steps, ...(reconstruct ? { saveProgress: false } : {}) }), retentionError); + assert.equal(journal.events().some(e => e.type === 'action.start' && e.data.kind === 'click'), true, 'Frame creation should be allowed'); + assert.equal(journal.events().some(e => e.type === 'action.start' && e.data.kind === 'fill'), false); + assert.deepEqual(journal.recoveryFields(), {}); + assertNotRetained(journal); + }); + } + await scenario('A future reconstruction-only scope does not block navigation', { + steps: [{ kind: 'click', selector: '#reveal' }], + recovery: { fields: [{ key: 'public', selector: '#public' }], reconstruct: [{ kind: 'fill', selector: '#frameName', frames: ['#late'], valueFromEnv: key }] }, + }, futurePage, async ({ journal, invoke }) => { + await invoke('steps'); + assert.equal(journal.recoveryFields().public.value, 'keep me'); + assertNotRetained(journal); + }); + + for (const type of ['password', 'file', 'otp']) for (const protectedTarget of [false, true]) { + const getterSecret = `synthetic-${type}-getter-value`; + await scenario(`Sensitive ${type} getter is never invoked; protected locator=${protectedTarget}`, { + steps: protectedTarget ? [{ kind: 'fill', selector: '#other', valueFromEnv: key }] : [], + recovery: { fields: [{ key: 'private', selector: '#private' }] }, + }, ``, async ({ page, journal, invoke }) => { + await page.evaluate(value => { + globalThis.valueReads = 0; + const node = document.querySelector('#private'); + Object.defineProperty(node, 'value', { configurable: true, get() { + globalThis.valueReads++; node.removeAttribute('type'); node.removeAttribute('autocomplete'); return value; + } }); + }, getterSecret); + let error; + try { await invoke(); } catch (caught) { error = caught; } + assert.equal(await page.evaluate(() => globalThis.valueReads), 0, 'Classification must happen before the value getter'); + assert.equal(error?.code, 'retention_forbidden'); + assert.deepEqual(journal.recoveryFields(), {}); + assertNotRetained(journal, getterSecret); + }); + } + await scenario('An environment-protected getter is not read', { + steps: [{ kind: 'fill', selector: 'input#token', valueFromEnv: key }], + recovery: { fields: [{ key: 'private', selector: '#token' }] }, + }, '', async ({ page, journal, invoke }) => { + await page.evaluate(() => { globalThis.valueReads = 0; Object.defineProperty(document.querySelector('#token'), 'value', { get() { globalThis.valueReads++; return 'must-not-read'; } }); }); + await assert.rejects(() => invoke(), retentionError); + assert.equal(await page.evaluate(() => globalThis.valueReads), 0); + assert.deepEqual(journal.recoveryFields(), {}); + }); + + for (const [label, frames, markup] of [ + ['ambiguous', ['.ambiguous'], ''], + ['non-frame', ['#notAFrame'], '
'], + ['invalid selector', ['['], ''], + ]) { + await scenario(`Invalid protected scopes still fail closed: ${label}`, { + steps: [{ kind: 'fill', selector: '#token', frames, valueFromEnv: key }], + recovery: { fields: [{ key: 'public', selector: '#public' }] }, + }, markup, async ({ journal, invoke }) => { + await assert.rejects(() => invoke()); + assert.deepEqual(journal.recoveryFields(), {}); + }); + } + await scenario('Required assertion scopes remain strict', { steps: [] }, '

No iframe

', async ({ page }) => { + await assert.rejects(() => checkOne(page, { kind: 'count', frames: ['#missing'], selector: 'input', value: '0' }), error => error.code === 'unsupported_scope'); + }); +} finally { + await raw.close(); await f.cleanup(); + if (previous === undefined) delete process.env[key]; else process.env[key] = previous; +} +console.log(JSON.stringify({ suite: 'recovery capture review regressions', passed, failures, paidModelCalls: 0 })); +assert.equal(failures.length, 0, `${failures.length} recovery-capture regressions failed`); From ea20aa7a796ccf5b82e3c79ef8b2f709a78a13dd Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:38:22 +0200 Subject: [PATCH 17/24] fix: isolate recovery capture secrets and handle future frame scopes Keep environment-value comparisons in Node instead of passing secrets into page-realm evaluations. Reject protected identities and sensitive controls before reading value getters, retaining Playwright's atomic shadow-aware protection. Resolve only currently present recovery/protected frame scopes without waits; preserve strict malformed/ambiguous/action/check handling. Skip capture entirely without recovery fields, and re-evaluate protection when later frames appear. --- src/page-worker.ts | 98 ++++++++++++++++++++++++++-------------------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index 49955b9..fb4cfe6 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -75,36 +75,61 @@ export async function pollChecks(page: Page, checks: readonly Check[], timeoutMs await new Promise(resolve => setTimeout(resolve, 100)); } } +// Recovery is a passive snapshot: a later step's frame may not exist yet. +// Pin each existing frame without waiting or relaxing required action/check scopes. +async function captureScope(page: Page, s: Scoped): Promise<{ frame: Frame; locator: Locator } | undefined> { + if (s.shadow === "closed") throw new BrowserError({ code: "unsupported_scope", reason: "Closed shadow-root DOM access is unsupported. Use explicitly requested visual evidence." }); + let frame = page.mainFrame(); + for (const selector of s.frames ?? []) { + const nodes = await frame.locator(css(selector, s.shadow)).elementHandles(); + try { + if (nodes.length === 0) return undefined; + if (nodes.length !== 1 || !(await nodes[0]!.evaluate(e => /^(IFRAME|FRAME)$/.test(e.tagName)))) { + throw new BrowserError({ code: "unsupported_scope", reason: "The frame path did not identify exactly one frame. Recovery capture cannot establish its scope." }); + } + const child = await nodes[0]!.contentFrame(); + if (!child) return undefined; + frame = child; + } finally { await Promise.allSettled(nodes.map(node => node.dispose())); } + } + return { frame, locator: frame.locator(css(s.selector ?? "body", s.shadow)) }; +} async function captureFields(page: Page, journal: Journal, save = true): Promise { const task = journal.meta().task; const fields = task.recovery?.fields ?? []; + if (fields.length === 0) return; const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); + // These values never enter browser evaluation arguments. A page may redefine + // its prototypes; equality against environment secrets belongs in this process. + const protectedValues = new Set(protectedSteps + .map(step => process.env[step.valueFromEnv!]) + .filter((value): value is string => value !== undefined && value.length > 0)); const handles: ElementHandle[] = []; try { - const protectedTargets: { locator: Locator; frame: Frame | null }[] = []; - for (const step of protectedSteps) { - const { root, locator } = await scoped(page, step); - const documentElement = await root.locator("html").elementHandle(); - if (!documentElement) continue; - handles.push(documentElement); - protectedTargets.push({ locator, frame: await documentElement.ownerFrame() }); - } - const protectedValues = protectedSteps - .map(step => process.env[step.valueFromEnv!]) - .filter((value): value is string => value !== undefined && value.length > 0); for (const f of fields) { - const { locator } = await scoped(page, f); - const nodes = await locator.elementHandles(); + const scope = await captureScope(page, f); + if (!scope) continue; + const nodes = await scope.locator.elementHandles(); handles.push(...nodes); if (nodes.length !== 1) continue; const node = nodes[0]!; const frame = await node.ownerFrame(); - const current = protectedTargets.filter(p => p.frame === frame).map(p => p.locator); + if (!frame) continue; + const current: Locator[] = []; + // Resolve protection after pinning the candidate. Missing future frames + // are skipped, but malformed or ambiguous scopes still fail closed. + for (const step of protectedSteps) { + const protectedScope = await captureScope(page, step); + if (protectedScope?.frame === frame) current.push(protectedScope.locator); + } if (!save) { for (const protectedLocator of current) { - const overlaps = await protectedLocator.evaluateAll((elements, target) => elements.includes(target as HTMLElement | SVGElement), node); + const overlaps = await protectedLocator.evaluateAll((elements, target) => { + for (let i = 0; i < elements.length; i++) if (elements[i] === target) return true; + return false; + }, node); if (overlaps) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } @@ -112,39 +137,26 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise continue; } - // Resolve every protected selector with Playwright's current, shadow-aware - // locator semantics, compare against the already-pinned recovery node, - // and read that same node in the very same browser evaluation. + // Resolve protected selectors with Playwright's shadow-aware semantics, + // reject identity/sensitivity, then read that exact pinned node atomically. let protectedLocator = current[0]; for (const next of current.slice(1)) protectedLocator = protectedLocator!.or(next); - const result = protectedLocator - ? await protectedLocator.evaluateAll((elements, input) => { - const element = input.target as unknown as Element; - const hasValue = "value" in element; - const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; - if (elements.includes(element as HTMLElement | SVGElement) || input.values.includes(rawValue)) return { actual: "", protected: true }; + const result: { actual: string; protected?: boolean; error?: string; available?: boolean } = protectedLocator + ? await protectedLocator.evaluateAll((elements, target) => { + const element = target as unknown as Element; + for (let i = 0; i < elements.length; i++) if (elements[i] === element) return { actual: "", protected: true }; + // A page-controlled value getter can change these attributes. Check + // them before any value access, including environment comparisons. const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; + if (!("value" in element)) return { actual: "", error: "Target is not a value control." }; const rect = element.getBoundingClientRect(); const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); if (!visible) return { actual: "", available: false }; - if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!hasValue) return { actual: "", error: "Target is not a value control." }; - return { actual: rawValue }; - }, { target: node, values: protectedValues }) - : await node.evaluate((element, values) => { - const hasValue = "value" in element; - const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; - if (values.includes(rawValue)) return { actual: "", protected: true }; - const control = element as Element; - const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); - const rect = control.getBoundingClientRect(); - const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); - if (!visible) return { actual: "", available: false }; - if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!hasValue) return { actual: "", error: "Target is not a value control." }; - return { actual: rawValue }; - }, protectedValues); - if (result.protected) { + return { actual: String((element as HTMLInputElement).value) }; + }, node) + : await node.evaluate(readElement, { kind: "value" }); + if (result.protected || protectedValues.has(result.actual)) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } if (result.error) throw new BrowserError({ code: "retention_forbidden", reason: "A recovery allowlist includes a sensitive or unsupported control. Remove it; no value was saved." }); @@ -416,4 +428,4 @@ async function main(): Promise { catch (e) { process.stdout.write(`${JSON.stringify({ ok: false, error: { code: controller.signal.aborted ? "cancelled" : e instanceof BrowserError ? e.code : "adapter", reason: e instanceof BrowserError ? e.reason : "Browser adapter could not finish. Inspect the run before any retry." } })}\n`); } finally { clearTimeout(timer); } } -if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) void main().catch(() => { process.exitCode = 2; }); +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) void main().catch(() => { process.exitCode = 2; }); \ No newline at end of file From 5f1a0f4b46adfb450bb42b7376ba1341a3ce3edd Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:46:01 +0200 Subject: [PATCH 18/24] fix: narrow locator element handles at the recovery boundary --- src/page-worker.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index fb4cfe6..e76a12a 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -84,7 +84,7 @@ async function captureScope(page: Page, s: Scoped): Promise<{ frame: Frame; loca const nodes = await frame.locator(css(selector, s.shadow)).elementHandles(); try { if (nodes.length === 0) return undefined; - if (nodes.length !== 1 || !(await nodes[0]!.evaluate(e => /^(IFRAME|FRAME)$/.test(e.tagName)))) { + if (nodes.length !== 1 || !(await nodes[0]!.evaluate(e => e.nodeType === 1 && /^(IFRAME|FRAME)$/.test((e as Element).tagName)))) { throw new BrowserError({ code: "unsupported_scope", reason: "The frame path did not identify exactly one frame. Recovery capture cannot establish its scope." }); } const child = await nodes[0]!.contentFrame(); @@ -113,7 +113,8 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise const nodes = await scope.locator.elementHandles(); handles.push(...nodes); if (nodes.length !== 1) continue; - const node = nodes[0]!; + // Locator matches are Elements; Playwright declares their handles as Node. + const node = nodes[0]! as ElementHandle; const frame = await node.ownerFrame(); if (!frame) continue; const current: Locator[] = []; @@ -428,4 +429,4 @@ async function main(): Promise { catch (e) { process.stdout.write(`${JSON.stringify({ ok: false, error: { code: controller.signal.aborted ? "cancelled" : e instanceof BrowserError ? e.code : "adapter", reason: e instanceof BrowserError ? e.reason : "Browser adapter could not finish. Inspect the run before any retry." } })}\n`); } finally { clearTimeout(timer); } } -if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) void main().catch(() => { process.exitCode = 2; }); \ No newline at end of file +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) void main().catch(() => { process.exitCode = 2; }); From 50889f2bb8bb872caca8d54a4c497375efe4d826 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:56:48 +0200 Subject: [PATCH 19/24] perf: share protected frame traversal within each recovery snapshot --- src/page-worker.ts | 31 +++++++++++++++++++++---------- 1 file changed, 21 insertions(+), 10 deletions(-) diff --git a/src/page-worker.ts b/src/page-worker.ts index e76a12a..ca77c95 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -57,7 +57,6 @@ export async function checkOne(page: Page, c: Check): Promise { else { const count = await locator.count(); if (c.kind === "count") actual = String(count); - else if (c.kind === "visible" && count === 0) actual = "false"; else if (count !== 1) { actual = `<${count} matching elements>`; available = false; } else { const value = await locator.evaluate(readElement, c); @@ -107,8 +106,9 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise .filter((value): value is string => value !== undefined && value.length > 0)); const handles: ElementHandle[] = []; try { - for (const f of fields) { - const scope = await captureScope(page, f); + const candidates: Array<{ field: (typeof fields)[number]; node: ElementHandle; frame: Frame }> = []; + for (const field of fields) { + const scope = await captureScope(page, field); if (!scope) continue; const nodes = await scope.locator.elementHandles(); handles.push(...nodes); @@ -116,15 +116,26 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise // Locator matches are Elements; Playwright declares their handles as Node. const node = nodes[0]! as ElementHandle; const frame = await node.ownerFrame(); + if (frame) candidates.push({ field, node, frame }); + } + if (candidates.length === 0) return; + + // Pin recovery nodes first, then resolve each protected frame path once. + // Cache only this snapshot's scopes, never protected node identities/values. + const protectedFrames = new Map(); + const protectedLocators = new Map(); + for (const step of protectedSteps) { + const key = JSON.stringify([step.frames ?? [], step.shadow ?? "none"]); + if (!protectedFrames.has(key)) protectedFrames.set(key, (await captureScope(page, step))?.frame); + const frame = protectedFrames.get(key); if (!frame) continue; - const current: Locator[] = []; - // Resolve protection after pinning the candidate. Missing future frames - // are skipped, but malformed or ambiguous scopes still fail closed. - for (const step of protectedSteps) { - const protectedScope = await captureScope(page, step); - if (protectedScope?.frame === frame) current.push(protectedScope.locator); - } + const locators = protectedLocators.get(frame) ?? []; + locators.push(frame.locator(css(step.selector!, step.shadow))); + protectedLocators.set(frame, locators); + } + for (const { field: f, node, frame } of candidates) { + const current = protectedLocators.get(frame) ?? []; if (!save) { for (const protectedLocator of current) { const overlaps = await protectedLocator.evaluateAll((elements, target) => { From b410efc03bcc4db9c5e743c7d80604e91728b736 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 15:59:46 +0200 Subject: [PATCH 20/24] fix: address all open Codex recovery capture findings Apply the recovery capture fixes and regression coverage from stacked PR #5 (source commit 50889f2bb8bb872caca8d54a4c497375efe4d826) directly to PR #4. - Keep environment-secret comparisons in Node, outside page evaluations. - Skip absent future frame scopes during passive capture and bypass empty recovery allowlists without relaxing required action or assertion scopes. - Reject protected identities and sensitive controls before reading values, preserving pinned-node and shadow-aware checks. - Cache protected frame traversal within each recovery snapshot. - Include 21 real-Chrome regressions in the browser command and CI. Addresses Codex review comments 4061550418, 4061550427, and 4061578096. --- .github/workflows/ci.yml | 3 + package.json | 2 +- src/page-worker.ts | 114 ++++++++++++-------- test/recovery-capture.smoke.mjs | 178 ++++++++++++++++++++++++++++++++ 4 files changed, 251 insertions(+), 46 deletions(-) create mode 100644 test/recovery-capture.smoke.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 68a9fac..1a3f706 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,3 +49,6 @@ jobs: - name: Review regressions for retention and preflight if: always() && !cancelled() run: timeout 90s node test/review-followups.smoke.mjs + - name: Recovery capture isolation and late frames + if: always() && !cancelled() + run: timeout 120s node test/recovery-capture.smoke.mjs diff --git a/package.json b/package.json index 0f22e5a..b5efe18 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "test:python": "python3 -m unittest discover -s worker/tests -v", "check": "npm run typecheck && npm test && npm run test:python", "prepack": "npm run build", - "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs" + "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs" }, "dependencies": { "@effect/platform-node": "4.0.0-rc.116", diff --git a/src/page-worker.ts b/src/page-worker.ts index 49955b9..ca77c95 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -57,7 +57,6 @@ export async function checkOne(page: Page, c: Check): Promise { else { const count = await locator.count(); if (c.kind === "count") actual = String(count); - else if (c.kind === "visible" && count === 0) actual = "false"; else if (count !== 1) { actual = `<${count} matching elements>`; available = false; } else { const value = await locator.evaluate(readElement, c); @@ -75,36 +74,74 @@ export async function pollChecks(page: Page, checks: readonly Check[], timeoutMs await new Promise(resolve => setTimeout(resolve, 100)); } } +// Recovery is a passive snapshot: a later step's frame may not exist yet. +// Pin each existing frame without waiting or relaxing required action/check scopes. +async function captureScope(page: Page, s: Scoped): Promise<{ frame: Frame; locator: Locator } | undefined> { + if (s.shadow === "closed") throw new BrowserError({ code: "unsupported_scope", reason: "Closed shadow-root DOM access is unsupported. Use explicitly requested visual evidence." }); + let frame = page.mainFrame(); + for (const selector of s.frames ?? []) { + const nodes = await frame.locator(css(selector, s.shadow)).elementHandles(); + try { + if (nodes.length === 0) return undefined; + if (nodes.length !== 1 || !(await nodes[0]!.evaluate(e => e.nodeType === 1 && /^(IFRAME|FRAME)$/.test((e as Element).tagName)))) { + throw new BrowserError({ code: "unsupported_scope", reason: "The frame path did not identify exactly one frame. Recovery capture cannot establish its scope." }); + } + const child = await nodes[0]!.contentFrame(); + if (!child) return undefined; + frame = child; + } finally { await Promise.allSettled(nodes.map(node => node.dispose())); } + } + return { frame, locator: frame.locator(css(s.selector ?? "body", s.shadow)) }; +} async function captureFields(page: Page, journal: Journal, save = true): Promise { const task = journal.meta().task; const fields = task.recovery?.fields ?? []; + if (fields.length === 0) return; const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); + // These values never enter browser evaluation arguments. A page may redefine + // its prototypes; equality against environment secrets belongs in this process. + const protectedValues = new Set(protectedSteps + .map(step => process.env[step.valueFromEnv!]) + .filter((value): value is string => value !== undefined && value.length > 0)); const handles: ElementHandle[] = []; try { - const protectedTargets: { locator: Locator; frame: Frame | null }[] = []; - for (const step of protectedSteps) { - const { root, locator } = await scoped(page, step); - const documentElement = await root.locator("html").elementHandle(); - if (!documentElement) continue; - handles.push(documentElement); - protectedTargets.push({ locator, frame: await documentElement.ownerFrame() }); - } - const protectedValues = protectedSteps - .map(step => process.env[step.valueFromEnv!]) - .filter((value): value is string => value !== undefined && value.length > 0); - for (const f of fields) { - const { locator } = await scoped(page, f); - const nodes = await locator.elementHandles(); + const candidates: Array<{ field: (typeof fields)[number]; node: ElementHandle; frame: Frame }> = []; + for (const field of fields) { + const scope = await captureScope(page, field); + if (!scope) continue; + const nodes = await scope.locator.elementHandles(); handles.push(...nodes); if (nodes.length !== 1) continue; - const node = nodes[0]!; + // Locator matches are Elements; Playwright declares their handles as Node. + const node = nodes[0]! as ElementHandle; const frame = await node.ownerFrame(); - const current = protectedTargets.filter(p => p.frame === frame).map(p => p.locator); + if (frame) candidates.push({ field, node, frame }); + } + if (candidates.length === 0) return; + + // Pin recovery nodes first, then resolve each protected frame path once. + // Cache only this snapshot's scopes, never protected node identities/values. + const protectedFrames = new Map(); + const protectedLocators = new Map(); + for (const step of protectedSteps) { + const key = JSON.stringify([step.frames ?? [], step.shadow ?? "none"]); + if (!protectedFrames.has(key)) protectedFrames.set(key, (await captureScope(page, step))?.frame); + const frame = protectedFrames.get(key); + if (!frame) continue; + const locators = protectedLocators.get(frame) ?? []; + locators.push(frame.locator(css(step.selector!, step.shadow))); + protectedLocators.set(frame, locators); + } + for (const { field: f, node, frame } of candidates) { + const current = protectedLocators.get(frame) ?? []; if (!save) { for (const protectedLocator of current) { - const overlaps = await protectedLocator.evaluateAll((elements, target) => elements.includes(target as HTMLElement | SVGElement), node); + const overlaps = await protectedLocator.evaluateAll((elements, target) => { + for (let i = 0; i < elements.length; i++) if (elements[i] === target) return true; + return false; + }, node); if (overlaps) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } @@ -112,39 +149,26 @@ async function captureFields(page: Page, journal: Journal, save = true): Promise continue; } - // Resolve every protected selector with Playwright's current, shadow-aware - // locator semantics, compare against the already-pinned recovery node, - // and read that same node in the very same browser evaluation. + // Resolve protected selectors with Playwright's shadow-aware semantics, + // reject identity/sensitivity, then read that exact pinned node atomically. let protectedLocator = current[0]; for (const next of current.slice(1)) protectedLocator = protectedLocator!.or(next); - const result = protectedLocator - ? await protectedLocator.evaluateAll((elements, input) => { - const element = input.target as unknown as Element; - const hasValue = "value" in element; - const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; - if (elements.includes(element as HTMLElement | SVGElement) || input.values.includes(rawValue)) return { actual: "", protected: true }; + const result: { actual: string; protected?: boolean; error?: string; available?: boolean } = protectedLocator + ? await protectedLocator.evaluateAll((elements, target) => { + const element = target as unknown as Element; + for (let i = 0; i < elements.length; i++) if (elements[i] === element) return { actual: "", protected: true }; + // A page-controlled value getter can change these attributes. Check + // them before any value access, including environment comparisons. const sensitive = element.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); + if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; + if (!("value" in element)) return { actual: "", error: "Target is not a value control." }; const rect = element.getBoundingClientRect(); const visible = !!rect.width && !!rect.height && element.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); if (!visible) return { actual: "", available: false }; - if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!hasValue) return { actual: "", error: "Target is not a value control." }; - return { actual: rawValue }; - }, { target: node, values: protectedValues }) - : await node.evaluate((element, values) => { - const hasValue = "value" in element; - const rawValue = hasValue ? String((element as HTMLInputElement).value) : ""; - if (values.includes(rawValue)) return { actual: "", protected: true }; - const control = element as Element; - const sensitive = control.matches('input[type=password], input[type=file], [autocomplete="one-time-code"]'); - const rect = control.getBoundingClientRect(); - const visible = !!rect.width && !!rect.height && control.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }); - if (!visible) return { actual: "", available: false }; - if (sensitive) return { actual: "", error: "Sensitive control values cannot be collected." }; - if (!hasValue) return { actual: "", error: "Target is not a value control." }; - return { actual: rawValue }; - }, protectedValues); - if (result.protected) { + return { actual: String((element as HTMLInputElement).value) }; + }, node) + : await node.evaluate(readElement, { kind: "value" }); + if (result.protected || protectedValues.has(result.actual)) { throw new BrowserError({ code: "retention_forbidden", reason: "An environment-backed control overlaps the recovery allowlist. Remove it and use its environment reference for reconstruction." }); } if (result.error) throw new BrowserError({ code: "retention_forbidden", reason: "A recovery allowlist includes a sensitive or unsupported control. Remove it; no value was saved." }); diff --git a/test/recovery-capture.smoke.mjs b/test/recovery-capture.smoke.mjs new file mode 100644 index 0000000..02110c4 --- /dev/null +++ b/test/recovery-capture.smoke.mjs @@ -0,0 +1,178 @@ +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import path from 'node:path'; +import { fixture } from './support/chrome.mjs'; +import { Cdp } from '../dist/cdp.js'; +import { createRun, atomic } from '../dist/journal.js'; +import { attachPage, pageJob, checkOne } from '../dist/page-worker.js'; + +// All secrets and hostile pages in this suite are synthetic and local. +const key = 'FE2E_CAPTURE_PRIVATE_INPUT', previous = process.env[key]; +const secret = 'capture-secret-not-for-other-documents'; +process.env[key] = secret; +const f = await fixture(), raw = await Cdp.open(f.session); +const passed = [], failures = []; +const retentionError = error => error?.code === 'retention_forbidden'; + +function assertNotRetained(journal, value = secret) { + for (const file of fs.readdirSync(journal.dir, { recursive: true })) { + const full = path.join(journal.dir, file); + if (fs.statSync(full).isFile()) assert.equal(fs.readFileSync(full, 'utf8').includes(value), false, file); + } +} +async function scenario(name, task, markup, check) { + const { targetId } = await raw.send('Target.createTarget', { url: 'about:blank' }); + const connection = await attachPage(f.session, targetId); + try { + const page = connection.page; + page.setDefaultTimeout(5_000); + await page.goto(f.url, { waitUntil: 'load' }); + const remoteUrl = await page.locator('#remote').getAttribute('src'); + await page.setContent(typeof markup === 'function' ? markup(remoteUrl) : markup); + const input = { url: f.url, goal: name, ...task }; + const { journal } = createRun(f.directory, input, false, f.profile); + journal.append('target', { targetId, browserId: f.session.browserId }); + atomic(path.join(f.directory, 'targets', f.session.namespace, `${targetId}.json`), { targetId, browserId: f.session.browserId }); + const invoke = (operation = 'capture', extra = {}) => pageJob({ + root: f.directory, runId: journal.runId, session: f.session, targetId, + operation, steps: input.steps, timeoutMs: 20_000, ...extra, + }, new AbortController().signal); + await check({ page, journal, invoke }); + passed.push(name); + } catch (error) { + failures.push(name); + console.error(`${name}: ${error.stack ?? error}`); + } finally { + await connection.browser.close(); + await raw.send('Target.closeTarget', { targetId }); + } +} +async function watchEvaluationArrays(page) { + await page.evaluate(() => { + globalThis.evaluatedStrings = []; + const original = Array.prototype.includes; + Array.prototype.includes = function (...args) { + for (let i = 0; i < this.length; i++) if (typeof this[i] === 'string') globalThis.evaluatedStrings.push(this[i]); + return Reflect.apply(original, this, args); + }; + }); +} +const futurePage = remoteUrl => `

Before frame

+ `; +try { + for (const scope of ['same document', 'other origin']) { + await scenario(`No environment values enter recovery evaluations: ${scope}`, { + steps: [{ kind: 'fill', selector: '#token', valueFromEnv: key, ...(scope === 'other origin' ? { frames: ['#remote'] } : {}) }], + recovery: { fields: [{ key: 'public', selector: '#public' }] }, + }, remote => ``, async ({ page, journal, invoke }) => { + await watchEvaluationArrays(page); + await invoke(); + assert.equal((await page.evaluate(() => globalThis.evaluatedStrings)).includes(secret), false, 'Unrelated page learned an environment secret'); + assert.equal(journal.recoveryFields().public.value, 'public value'); + assertNotRetained(journal); + }); + } + await scenario('Host-side comparison rejects a reflected environment value', { + steps: [{ kind: 'fill', selector: '#frameName', frames: ['#remote'], valueFromEnv: key }], + recovery: { fields: [{ key: 'public', selector: '#public' }] }, + }, remote => ``, async ({ page, journal, invoke }) => { + // Simulate a value the site already knows. It must not be persisted as public input. + await page.locator('#public').fill(secret); + await watchEvaluationArrays(page); + await assert.rejects(() => invoke(), retentionError); + assert.equal((await page.evaluate(() => globalThis.evaluatedStrings)).includes(secret), false); + assert.deepEqual(journal.recoveryFields(), {}); + assertNotRetained(journal); + }); + + for (const retain of [false, true]) for (const nested of [false, true]) { + const frames = nested ? ['#late', '#nested'] : ['#late']; + const selector = nested ? '#nestedName' : '#frameName'; + await scenario(`Future ${nested ? 'nested ' : ''}frame does not block its creation; recovery=${retain}`, { + steps: [{ kind: 'click', selector: '#reveal' }, { kind: 'fill', selector, frames, valueFromEnv: key }], + ...(retain ? { recovery: { fields: [{ key: 'public', selector: '#public' }] } } : {}), + }, futurePage, async ({ page, journal, invoke }) => { + await invoke('steps'); + let scope = page; + for (const frame of frames) scope = scope.frameLocator(frame); + assert.equal(await scope.locator(selector).inputValue(), secret); + assert.equal(journal.events().filter(e => e.type === 'action.start').length, 2); + if (retain) assert.equal(journal.recoveryFields().public.value, 'keep me'); + assertNotRetained(journal); + }); + } + for (const reconstruct of [false, true]) { + const steps = [{ kind: 'click', selector: '#reveal' }, { kind: 'fill', selector: '#frameName', frames: ['iframe#late'], valueFromEnv: key }]; + await scenario(`Late-frame aliases are rejected before secret dispatch; reconstruct=${reconstruct}`, { + steps: reconstruct ? [] : steps, + recovery: { fields: [{ key: 'private', selector: 'input#frameName', frames: ['#late'] }], ...(reconstruct ? { reconstruct: steps } : {}) }, + }, futurePage, async ({ journal, invoke }) => { + await assert.rejects(() => invoke('steps', { steps, ...(reconstruct ? { saveProgress: false } : {}) }), retentionError); + assert.equal(journal.events().some(e => e.type === 'action.start' && e.data.kind === 'click'), true, 'Frame creation should be allowed'); + assert.equal(journal.events().some(e => e.type === 'action.start' && e.data.kind === 'fill'), false); + assert.deepEqual(journal.recoveryFields(), {}); + assertNotRetained(journal); + }); + } + await scenario('A future reconstruction-only scope does not block navigation', { + steps: [{ kind: 'click', selector: '#reveal' }], + recovery: { fields: [{ key: 'public', selector: '#public' }], reconstruct: [{ kind: 'fill', selector: '#frameName', frames: ['#late'], valueFromEnv: key }] }, + }, futurePage, async ({ journal, invoke }) => { + await invoke('steps'); + assert.equal(journal.recoveryFields().public.value, 'keep me'); + assertNotRetained(journal); + }); + + for (const type of ['password', 'file', 'otp']) for (const protectedTarget of [false, true]) { + const getterSecret = `synthetic-${type}-getter-value`; + await scenario(`Sensitive ${type} getter is never invoked; protected locator=${protectedTarget}`, { + steps: protectedTarget ? [{ kind: 'fill', selector: '#other', valueFromEnv: key }] : [], + recovery: { fields: [{ key: 'private', selector: '#private' }] }, + }, ``, async ({ page, journal, invoke }) => { + await page.evaluate(value => { + globalThis.valueReads = 0; + const node = document.querySelector('#private'); + Object.defineProperty(node, 'value', { configurable: true, get() { + globalThis.valueReads++; node.removeAttribute('type'); node.removeAttribute('autocomplete'); return value; + } }); + }, getterSecret); + let error; + try { await invoke(); } catch (caught) { error = caught; } + assert.equal(await page.evaluate(() => globalThis.valueReads), 0, 'Classification must happen before the value getter'); + assert.equal(error?.code, 'retention_forbidden'); + assert.deepEqual(journal.recoveryFields(), {}); + assertNotRetained(journal, getterSecret); + }); + } + await scenario('An environment-protected getter is not read', { + steps: [{ kind: 'fill', selector: 'input#token', valueFromEnv: key }], + recovery: { fields: [{ key: 'private', selector: '#token' }] }, + }, '', async ({ page, journal, invoke }) => { + await page.evaluate(() => { globalThis.valueReads = 0; Object.defineProperty(document.querySelector('#token'), 'value', { get() { globalThis.valueReads++; return 'must-not-read'; } }); }); + await assert.rejects(() => invoke(), retentionError); + assert.equal(await page.evaluate(() => globalThis.valueReads), 0); + assert.deepEqual(journal.recoveryFields(), {}); + }); + + for (const [label, frames, markup] of [ + ['ambiguous', ['.ambiguous'], ''], + ['non-frame', ['#notAFrame'], '
'], + ['invalid selector', ['['], ''], + ]) { + await scenario(`Invalid protected scopes still fail closed: ${label}`, { + steps: [{ kind: 'fill', selector: '#token', frames, valueFromEnv: key }], + recovery: { fields: [{ key: 'public', selector: '#public' }] }, + }, markup, async ({ journal, invoke }) => { + await assert.rejects(() => invoke()); + assert.deepEqual(journal.recoveryFields(), {}); + }); + } + await scenario('Required assertion scopes remain strict', { steps: [] }, '

No iframe

', async ({ page }) => { + await assert.rejects(() => checkOne(page, { kind: 'count', frames: ['#missing'], selector: 'input', value: '0' }), error => error.code === 'unsupported_scope'); + }); +} finally { + await raw.close(); await f.cleanup(); + if (previous === undefined) delete process.env[key]; else process.env[key] = previous; +} +console.log(JSON.stringify({ suite: 'recovery capture review regressions', passed, failures, paidModelCalls: 0 })); +assert.equal(failures.length, 0, `${failures.length} recovery-capture regressions failed`); From b3cc23ed4eee84d31f395d1c26edf5fb133a8700 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 16:01:40 +0200 Subject: [PATCH 21/24] test: bound recovery frame lookups and retain absent-control semantics Assert that eight recovery fields and twenty protected steps over two frame paths require only three path lookups per snapshot, and that the next snapshot resolves afresh. Keep the legacy visible=false result for an absent top-level control and cover it alongside strict missing-frame checks. --- src/page-worker.ts | 1 + test/recovery-capture.smoke.mjs | 26 ++++++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/page-worker.ts b/src/page-worker.ts index ca77c95..ea080e4 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -57,6 +57,7 @@ export async function checkOne(page: Page, c: Check): Promise { else { const count = await locator.count(); if (c.kind === "count") actual = String(count); + else if (c.kind === "visible" && count === 0) actual = "false"; else if (count !== 1) { actual = `<${count} matching elements>`; available = false; } else { const value = await locator.evaluate(readElement, c); diff --git a/test/recovery-capture.smoke.mjs b/test/recovery-capture.smoke.mjs index 02110c4..eb3380d 100644 --- a/test/recovery-capture.smoke.mjs +++ b/test/recovery-capture.smoke.mjs @@ -169,6 +169,32 @@ try { } await scenario('Required assertion scopes remain strict', { steps: [] }, '

No iframe

', async ({ page }) => { await assert.rejects(() => checkOne(page, { kind: 'count', frames: ['#missing'], selector: 'input', value: '0' }), error => error.code === 'unsupported_scope'); + assert.equal((await checkOne(page, { kind: 'visible', selector: '#missing', value: 'false' })).passed, true); + }); + + const publicFields = Array.from({ length: 8 }, (_, i) => ({ key: `public${i}`, selector: `#public${i}` })); + await scenario('Protected frame traversal is shared within, not across, snapshots', { + steps: Array.from({ length: 20 }, (_, i) => ({ kind: 'fill', valueFromEnv: key, + selector: i % 2 ? '#nestedName' : '#frameName', frames: i % 2 ? ['#remote', '#nested'] : ['#remote'] })), + recovery: { fields: publicFields }, + }, remote => publicFields.map((_, i) => ``).join('') + ``, async ({ page, journal, invoke }) => { + // Count real frame-path lookups, not timing: added fields/duplicate steps + // must not multiply browser round trips. The second snapshot must be fresh. + const prototype = Object.getPrototypeOf(page.mainFrame()), original = prototype.locator; + let queries = 0; + prototype.locator = function (selector, ...options) { + if (selector === 'css:light=#remote' || selector === 'css:light=#nested') queries++; + return Reflect.apply(original, this, [selector, ...options]); + }; + try { + for (let attempt = 0; attempt < 2; attempt++) { + queries = 0; + await invoke(); + assert.equal(queries, 3, 'Resolve the two distinct paths once each per snapshot'); + assert.equal(Object.keys(journal.recoveryFields()).length, 8); + } + } finally { prototype.locator = original; } + assertNotRetained(journal); }); } finally { await raw.close(); await f.cleanup(); From 1438b9c9d9a8b97e73aec8782867597703135875 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 16:23:54 +0200 Subject: [PATCH 22/24] fix: carry recovery sensitivity across environment dispatch Persist a recovery-value capture suspension before an environment-backed UI dispatch. Current selectors and exact value equality cannot prove a transformed or rerendered credential became public. Preserve prior snapshots and explicit non-secret input intents, while preventing later automatic observations from replacing them. Honor the same durable boundary in managed Jev, including across reattachment, reconstruction, missing environment values and legacy Playwright receipts without provenance. Keep protected-alias preflight checks active in the scoped adapter. Add eight real-Chrome formatter/mirror/reconstruction tests, cross-language handoff assertions, and four Python policy tests. No new dependencies or documentation. Addresses PR #4 comment 4072551621. --- .github/workflows/ci.yml | 3 + package.json | 2 +- src/page-worker.ts | 26 +++++- test/environment-retention.smoke.mjs | 117 +++++++++++++++++++++++++++ worker/jev_runner.py | 16 +++- worker/tests/test_jev_retention.py | 66 +++++++++++++++ 6 files changed, 225 insertions(+), 5 deletions(-) create mode 100644 test/environment-retention.smoke.mjs create mode 100644 worker/tests/test_jev_retention.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1a3f706..544ac54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,3 +52,6 @@ jobs: - name: Recovery capture isolation and late frames if: always() && !cancelled() run: timeout 120s node test/recovery-capture.smoke.mjs + - name: Environment retention across dispatch and handoff + if: always() && !cancelled() + run: timeout 90s node test/environment-retention.smoke.mjs diff --git a/package.json b/package.json index b5efe18..3107f04 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "test:python": "python3 -m unittest discover -s worker/tests -v", "check": "npm run typecheck && npm test && npm run test:python", "prepack": "npm run build", - "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs" + "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs && node test/environment-retention.smoke.mjs" }, "dependencies": { "@effect/platform-node": "4.0.0-rc.116", diff --git a/src/page-worker.ts b/src/page-worker.ts index ea080e4..e7020c3 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -94,12 +94,28 @@ async function captureScope(page: Page, s: Scoped): Promise<{ frame: Frame; loca } return { frame, locator: frame.locator(css(s.selector ?? "body", s.shadow)) }; } +function suspendRecoveryCapture(journal: Journal): void { + if (!journal.events().some(e => e.type === "recovery.capture_suspended")) { + journal.append("recovery.capture_suspended", { + reason: "Environment-backed input may be moved or transformed by the page. Automatic recovery-value capture is disabled for this run; prior snapshots and explicit non-secret input intents remain available.", + }); + } +} async function captureFields(page: Page, journal: Journal, save = true): Promise { const task = journal.meta().task; const fields = task.recovery?.fields ?? []; if (fields.length === 0) return; - const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] - .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); + const steps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])]; + const events = journal.events(); + const suspended = events.some(e => e.type === "recovery.capture_suspended"); + // Old receipts cannot prove which environment-input boundary was crossed. + const legacy = steps.some(s => s.valueFromEnv !== undefined) && events.some(e => + e.type === "action.start" && e.data.engine === "playwright" && e.data.recoveryGuarded !== true); + if (suspended || legacy) { + if (!suspended) suspendRecoveryCapture(journal); + save = false; // Still reject known protected aliases, but never read new page values. + } + const protectedSteps = steps.filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); // These values never enter browser evaluation arguments. A page may redefine // its prototypes; equality against environment secrets belongs in this process. const protectedValues = new Set(protectedSteps @@ -224,8 +240,12 @@ async function runSteps(page: Page, journal: Journal, job: PageJob, snap: () => let actionId: string | undefined; if (s.kind !== "checkpoint") { if (remaining(journal.state()).actions <= 0) throw new BrowserError({ code: "budget_exhausted", reason: "Action budget exhausted." }); + // Selectors, node identities and exact-value matching cannot follow an + // arbitrary formatter/rerender. Persist this boundary before dispatch so + // resume, reconstruction and a Jev/Midscene handoff cannot forget it. + if (s.valueFromEnv) suspendRecoveryCapture(journal); actionId = randomUUID(); - journal.append("action.start", { id: actionId, kind: s.kind, safeToRepeat: s.safeToRepeat === true, step: index, selector: s.selector, frames: s.frames, engine: "playwright" }); + journal.append("action.start", { id: actionId, kind: s.kind, safeToRepeat: s.safeToRepeat === true, step: index, selector: s.selector, frames: s.frames, engine: "playwright", recoveryGuarded: true }); } switch (s.kind) { case "navigate": { diff --git a/test/environment-retention.smoke.mjs b/test/environment-retention.smoke.mjs new file mode 100644 index 0000000..1627041 --- /dev/null +++ b/test/environment-retention.smoke.mjs @@ -0,0 +1,117 @@ +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { execFileSync } from 'node:child_process'; +import { fixture } from './support/chrome.mjs'; +import { Cdp } from '../dist/cdp.js'; +import { createRun, atomic } from '../dist/journal.js'; +import { attachPage, pageJob } from '../dist/page-worker.js'; + +const key = 'FE2E_RERENDER_SECRET', secret = 'synthetic-private-123'; +const formatted = [...secret].join(' '), previous = process.env[key]; +process.env[key] = secret; +const f = await fixture(), raw = await Cdp.open(f.session), passed = []; +const worker = fileURLToPath(new URL('../worker', import.meta.url)); +function assertPrivate(journal) { + for (const entry of fs.readdirSync(journal.dir, { recursive: true })) { + const file = path.join(journal.dir, entry); + if (fs.statSync(file).isFile()) { + const text = fs.readFileSync(file, 'utf8'); + assert.equal(text.includes(secret), false, entry); + assert.equal(text.includes(formatted), false, entry); + } + } +} +function checkJevHandoff(journal) { + // A fresh Python adapter must honor the durable Node suspension even without + // the original environment. Any capture evaluation would fail this test. + const env = { ...process.env }; delete env[key]; + execFileSync('python3', ['-c', `import sys +sys.path.insert(0, sys.argv[1]) +from jev_runner import Journal, capture +class Page: + def evaluate(self, expression): + raise AssertionError('Suspended Jev recovery must not evaluate the page') +capture(Page(), Journal(sys.argv[2], sys.argv[3])) +`, worker, f.directory, journal.runId], { env, timeout: 10_000 }); +} +async function scenario(name, variant, reconstruct, check) { + const { targetId } = await raw.send('Target.createTarget', { url: 'about:blank' }); + const connection = await attachPage(f.session, targetId); + try { + const page = connection.page; + await page.goto(f.url, { waitUntil: 'load' }); + await page.setContent(` + `); + const step = { kind: 'fill', selector: '#token', valueFromEnv: key }; + const task = { url: f.url, goal: name, steps: [step], recovery: { + fields: [{ key: 'public', selector: '.active-input' }, { key: 'safe', selector: '#safe' }], + ...(reconstruct ? { reconstruct: [{ kind: 'navigate', value: f.url, safeToRepeat: true }, { ...step, safeToRepeat: true }] } : {}), + } }; + const { journal } = createRun(f.directory, task, false, f.profile); + journal.append('target', { targetId, browserId: f.session.browserId }); + atomic(path.join(f.directory, 'targets', f.session.namespace, `${targetId}.json`), { targetId, browserId: f.session.browserId }); + const invoke = (operation = 'capture', extra = {}) => pageJob({ + root: f.directory, runId: journal.runId, session: f.session, targetId, + operation, steps: [step], timeoutMs: 20_000, ...extra, + }, new AbortController().signal); + await invoke(); + await check({ page, journal, step, invoke }); + assertPrivate(journal); checkJevHandoff(journal); + passed.push(name); + } finally { + await connection.browser.close(); await raw.send('Target.closeTarget', { targetId }); + } +} +try { + for (const variant of ['retag', 'replace', 'mirror']) for (const reconstruct of [false, true]) { + await scenario(`Formatted ${variant} value is not retained; reconstruct=${reconstruct}`, variant, reconstruct, + async ({ page, journal, invoke }) => { + await invoke('steps', reconstruct ? { saveProgress: false } : {}); + assert.equal(await page.locator('.active-input').inputValue(), formatted); + const retained = journal.recoveryFields(); + assert.equal(retained.public.value, 'safe-before'); + assert.equal(retained.public.source, 'observed'); + const events = journal.events(); + const boundary = events.find(e => e.type === 'recovery.capture_suspended'); + assert.ok(boundary); + assert.ok(boundary.seq < events.find(e => e.type === 'action.start').seq); + // Reattachment, a later capture and loss of the env value cannot undo suspension. + delete process.env[key]; + try { await invoke(); await invoke(); } finally { process.env[key] = secret; } + assert.deepEqual(journal.recoveryFields(), retained); + assert.equal(journal.events().filter(e => e.type === 'recovery.capture_suspended').length, 1); + }); + } + await scenario('Explicit public inputs remain recoverable after secret dispatch', 'mirror', false, + async ({ journal, step, invoke }) => { + await invoke('steps', { steps: [step, { kind: 'fill', selector: '#safe', value: 'user-provided-public' }] }); + assert.equal(journal.recoveryFields().safe.value, 'user-provided-public'); + assert.equal(journal.recoveryFields().safe.source, 'intent'); + assert.equal(journal.recoveryFields().public.value, 'safe-before'); + }); + await scenario('Legacy dispatch without provenance suspends capture on resume', 'mirror', false, + async ({ page, journal, invoke }) => { + journal.append('action.start', { id: 'legacy', kind: 'fill', selector: '#token', step: 0, engine: 'playwright' }); + await page.locator('#token').fill(secret); + journal.append('action.end', { id: 'legacy' }); + delete process.env[key]; + try { await invoke(); } finally { process.env[key] = secret; } + assert.equal(journal.recoveryFields().public.value, 'safe-before'); + assert.ok(journal.events().some(e => e.type === 'recovery.capture_suspended')); + }); +} finally { + await raw.close(); await f.cleanup(); + if (previous === undefined) delete process.env[key]; else process.env[key] = previous; +} +console.log(JSON.stringify({ suite: 'environment retention across dispatch and handoff', passed, paidModelCalls: 0 })); diff --git a/worker/jev_runner.py b/worker/jev_runner.py index c38cc44..6f2634c 100644 --- a/worker/jev_runner.py +++ b/worker/jev_runner.py @@ -72,9 +72,23 @@ def ids(t): def capture(browser, journal, action=None, text=None): task = journal.meta["task"] fields = task.get("recovery", {}).get("fields", []) + if not fields: + return + steps = [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])] + events = journal.events() + # A scoped secret fill can hand off to Jev after the page has formatted or + # moved its value. Never infer that it became public from current selectors. + if any(e["type"] == "recovery.capture_suspended" for e in events): + return + if any(s.get("valueFromEnv") is not None for s in steps) and any( + e["type"] == "action.start" and e["data"].get("engine") == "playwright" + and e["data"].get("recoveryGuarded") is not True for e in events + ): + journal.append("recovery.capture_suspended", reason="Legacy dispatch cannot establish the environment-input boundary. Keep existing recovery inputs; do not collect new page values.") + return # Capture also runs after a scoped/environment-backed step hands off to Jev. # Protect aliases in the top-level document without reading secret values. - protected = [s["selector"] for s in [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])] + protected = [s["selector"] for s in steps if s.get("valueFromEnv") is not None and s.get("selector") and not s.get("frames")] for field in fields: if field.get("frames") or field.get("shadow") == "open": diff --git a/worker/tests/test_jev_retention.py b/worker/tests/test_jev_retention.py new file mode 100644 index 0000000..7f222e6 --- /dev/null +++ b/worker/tests/test_jev_retention.py @@ -0,0 +1,66 @@ +"""A fresh managed-Jev process must honor the shared capture boundary.""" +import json +from pathlib import Path +import sys +import tempfile +import time +import unittest +from unittest.mock import Mock + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from jev_runner import Journal, capture + + +class RecoveryBoundaryTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.run_id = 'r_' + 'a' * 32 + self.directory = Path(self.temp.name) / 'runs' / self.run_id + self.directory.mkdir(parents=True) + self.task = { + 'steps': [{'kind': 'fill', 'selector': '#token', 'valueFromEnv': 'SYNTHETIC_INPUT'}], + 'recovery': {'fields': [{'key': 'public', 'selector': '#public'}]}, + } + self.write_meta() + (self.directory / 'events.jsonl').write_text('') + self.journal = Journal(self.temp.name, self.run_id) + self.page = Mock() + self.page.evaluate.return_value = {'value': 'public', 'matches': False} + + def write_meta(self): + (self.directory / 'task.json').write_text(json.dumps({ + 'version': 2, 'expiresAt': time.time() * 1000 + 60_000, 'task': self.task, + })) + + def test_suspension_survives_new_journal_and_skips_model_generated_intents(self): + self.journal.append('recovery.capture_suspended', reason='environment boundary') + fresh = Journal(self.temp.name, self.run_id) + capture(self.page, fresh) + capture(self.page, fresh, {'kind': 'fill', 'node': 1}, 'model-generated') + self.page.evaluate.assert_not_called() + self.assertFalse((self.directory / 'recovery.json').exists()) + + def test_legacy_playwright_receipt_is_not_assumed_public(self): + self.journal.append('action.start', engine='playwright', id='old') + capture(self.page, self.journal) + capture(self.page, Journal(self.temp.name, self.run_id)) + self.page.evaluate.assert_not_called() + self.assertEqual(sum(e['type'] == 'recovery.capture_suspended' for e in self.journal.events()), 1) + + def test_guarded_public_dispatch_does_not_disable_capture(self): + self.journal.append('action.start', engine='playwright', id='public', recoveryGuarded=True) + capture(self.page, self.journal) + self.page.evaluate.assert_called_once() + self.assertEqual(json.loads((self.directory / 'recovery.json').read_text())['public']['value'], 'public') + + def test_legacy_run_without_environment_steps_still_captures(self): + self.task['steps'] = [{'kind': 'click', 'selector': '#open'}] + self.write_meta() + self.journal.append('action.start', engine='playwright', id='old-public') + capture(self.page, Journal(self.temp.name, self.run_id)) + self.page.evaluate.assert_called_once() + + +if __name__ == '__main__': + unittest.main() From cb060b287aa0095b42338cd6a1375be74fccc1d0 Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 16:31:27 +0200 Subject: [PATCH 23/24] fix: restore negative visibility checks and document recovery limits Restore the zero-match visible=false case without changing ambiguity or unsupported-scope handling. Extend the existing browser suite with scoped absence, legacy parity, hidden/ambiguous controls and disappearance polling. Defer transformed-secret tracking across replacement controls as a known prototype limitation. Document the risk and require recovery allowlists to remain non-sensitive; do not add taint tracking or blanket capture shutdown. Addresses review comment 4072551608; documents the scope decision for 4072551621. No new dependencies or runtime abstractions. --- docs/design.md | 2 ++ src/page-worker.ts | 1 + test/adapter.smoke.mjs | 19 ++++++++++++++++++- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/docs/design.md b/docs/design.md index 429e100..4ebda4c 100644 --- a/docs/design.md +++ b/docs/design.md @@ -24,6 +24,8 @@ A run journal persists work, not browser memory. It flushes intent before dispat [recover-form.test.json](../examples/recover-form.test.json) shows a half-filled form. Only use `safeToRepeat` where the application's behavior justifies it. Typing and navigation may autosave or submit changes. Recovery freezes retained inputs before navigation, so an empty replacement form cannot erase them. Secrets, files, one-time codes, and opaque editor state are not recoverable payloads; re-entry may be necessary. +Recovery allowlists must remain non-sensitive throughout the task. The prototype does not track secret-derived values across application formatting and replacement controls: if the original protected selector disappears, a transformed secret exposed through an allowlisted alias can be retained. Exclude those fields; when their contents cannot be guaranteed non-sensitive, omit `recovery.fields` and use local environment references for reconstruction. This is a known limitation, not a guarantee of secret redaction. + Declare `recovery.reconcile` and `reconcileOutcome` before a possibly uncertain effect. `reconcile --run ID` evaluates that UI evidence. For a completed deterministic dispatch, matching evidence advances its cursor once instead of resubmitting. For a completed autonomous dispatch without a cursor, continuation stays blocked rather than guessing. After a browser crash, a new observation task can inspect application state in the same profile; `reconcile --run ORIGINAL --from-run OBSERVER` uses its live page with the original expectations. This does not retarget or restore the original run. Evidence of absence must be strong enough for the application; a missing toast is not proof that Save did nothing. diff --git a/src/page-worker.ts b/src/page-worker.ts index ca77c95..ea080e4 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -57,6 +57,7 @@ export async function checkOne(page: Page, c: Check): Promise { else { const count = await locator.count(); if (c.kind === "count") actual = String(count); + else if (c.kind === "visible" && count === 0) actual = "false"; else if (count !== 1) { actual = `<${count} matching elements>`; available = false; } else { const value = await locator.evaluate(readElement, c); diff --git a/test/adapter.smoke.mjs b/test/adapter.smoke.mjs index a70ecdc..edab1a4 100644 --- a/test/adapter.smoke.mjs +++ b/test/adapter.smoke.mjs @@ -4,7 +4,7 @@ import {fixture} from './support/chrome.mjs'; import {mcpClient} from './support/mcp-client.mjs'; import {Cdp,checkpoint} from '../dist/cdp.js'; import {createRun,atomic} from '../dist/journal.js'; -import {attachPage,pageJob,checkOne} from '../dist/page-worker.js'; +import {attachPage,pageJob,checkOne,pollChecks} from '../dist/page-worker.js'; import {legacyExpression} from '../dist/dom.js'; import {screenshotRun} from '../dist/workflows.js'; import path from 'node:path'; @@ -28,6 +28,23 @@ try { await assert.rejects(()=>checkOne(connected.page,{kind:'count',frames:['#missing'],selector:'button',value:'0'}),/frame path/); assert.equal((await checkOne(connected.page,{kind:'text',selector:'#hidden',value:''})).passed,false); assert.equal((await raw.evaluate(sid,legacyExpression([{kind:'text',selector:'#missing',value:'matching elements'}])))[0].passed,false);checks.push('unsupported scopes and diagnostic sentinels cannot pass'); + // Missing elements are not visible; missing scopes and ambiguous matches remain unknown. + for(const scope of [{},{frames:['#frame']},{frames:['#frame','#nested']},{shadow:'open'}]) for(const value of ['false','true']) { + const c={kind:'visible',selector:'#absent',value,...scope}, result=await checkOne(connected.page,c); + assert.equal(result.actual,'false');assert.equal(result.passed,value==='false'); + if(!scope.frames && !scope.shadow)assert.deepEqual(result,(await raw.evaluate(sid,legacyExpression([c])))[0]); + } + assert.equal((await checkOne(connected.page,{kind:'visible',selector:'#hidden',value:'false'})).passed,true); + for(const value of ['false','true'])assert.equal((await checkOne(connected.page,{kind:'visible',selector:'h1, #name',value})).passed,false); + for(const kind of ['text','value','checked'])assert.equal((await checkOne(connected.page,{kind,selector:'#absent',value:'<0 matching elements>'})).passed,false); + assert.equal((await checkOne(connected.page,{kind:'count',selector:'#absent',value:'0'})).passed,true); + await assert.rejects(()=>checkOne(connected.page,{kind:'visible',frames:['#missing'],selector:'button',value:'false'}),/frame path/); + await assert.rejects(()=>checkOne(connected.page,{kind:'visible',shadow:'closed',selector:'button',value:'false'}),/Closed shadow/); + await connected.page.evaluate(()=>{const e=document.createElement('span');e.id='transient';e.textContent='Busy';document.body.append(e)}); + const disappears={kind:'visible',selector:'#transient',value:'false'}; + assert.equal((await checkOne(connected.page,disappears)).passed,false); + const waiting=pollChecks(connected.page,[disappears],3000);await connected.page.locator('#transient').evaluate(e=>e.remove()); + assert.equal((await waiting)[0].passed,true);checks.push('negative visibility, legacy parity, and disappearance polling'); const extracted=await pageJob({root:f.directory,runId:journal.runId,session:f.session,targetId,operation:'extract',extract:[{name:'good',kind:'value',selector:'#inside',shadow:'open'},{name:'missing',kind:'value',selector:'#absent'}],timeoutMs:10000},new AbortController().signal);assert.equal(extracted.fields.good.value,'Shadow');assert.ok(extracted.fields.missing.error);assert.equal(extracted.complete,false);checks.push('partial extraction preserves valid fields'); const first=await Effect.runPromise(screenshotRun({runId:journal.runId}));assert.ok(first.width<=1800);assert.equal(first.scale.x,first.width/first.css.width); await raw.send('Emulation.setDeviceMetricsOverride',{width:1280,height:800,deviceScaleFactor:2,mobile:false},sid); From afa92390d94091e2b3f0f5e2656f38ed345e6f4e Mon Sep 17 00:00:00 2001 From: Petr Glaser <12586960+BleedingDev@users.noreply.github.com> Date: Tue, 22 Sep 2026 16:35:34 +0200 Subject: [PATCH 24/24] fix: suspend observed recovery values before secret dispatch Adopt the existing, CI-validated conservative safeguard from PR #5 head 1438b9c9d9a8b97e73aec8782867597703135875, preserving this branch's added visibility regressions and documenting the recovery tradeoff. Persist a capture-suspended boundary before environment-backed dispatch; honor it across resume, reconstruction, Jev and Midscene. Prior snapshots and explicit non-secret Playwright fill intents remain usable. Do not try to track transformed secrets through node replacements or page formatters. This supersedes the initial deferral in cb060b2: the concurrent PR supplies a small fail-closed policy rather than a taint-tracking subsystem. Include its browser/Python regressions and keep dependencies unchanged. Addresses Codex review comment 4072551621. --- .github/workflows/ci.yml | 3 + docs/design.md | 2 +- package.json | 2 +- src/page-worker.ts | 26 +++++- test/environment-retention.smoke.mjs | 117 +++++++++++++++++++++++++++ test/recovery-capture.smoke.mjs | 26 ++++++ worker/jev_runner.py | 16 +++- worker/tests/test_jev_retention.py | 66 +++++++++++++++ 8 files changed, 252 insertions(+), 6 deletions(-) create mode 100644 test/environment-retention.smoke.mjs create mode 100644 worker/tests/test_jev_retention.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1a3f706..544ac54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -52,3 +52,6 @@ jobs: - name: Recovery capture isolation and late frames if: always() && !cancelled() run: timeout 120s node test/recovery-capture.smoke.mjs + - name: Environment retention across dispatch and handoff + if: always() && !cancelled() + run: timeout 90s node test/environment-retention.smoke.mjs diff --git a/docs/design.md b/docs/design.md index 4ebda4c..d363bcf 100644 --- a/docs/design.md +++ b/docs/design.md @@ -24,7 +24,7 @@ A run journal persists work, not browser memory. It flushes intent before dispat [recover-form.test.json](../examples/recover-form.test.json) shows a half-filled form. Only use `safeToRepeat` where the application's behavior justifies it. Typing and navigation may autosave or submit changes. Recovery freezes retained inputs before navigation, so an empty replacement form cannot erase them. Secrets, files, one-time codes, and opaque editor state are not recoverable payloads; re-entry may be necessary. -Recovery allowlists must remain non-sensitive throughout the task. The prototype does not track secret-derived values across application formatting and replacement controls: if the original protected selector disappears, a transformed secret exposed through an allowlisted alias can be retained. Exclude those fields; when their contents cannot be guaranteed non-sensitive, omit `recovery.fields` and use local environment references for reconstruction. This is a known limitation, not a guarantee of secret redaction. +Before an environment-backed UI dispatch, the run records `recovery.capture_suspended`. Automatic recovery-value observations then remain disabled for that run, including resume, reconstruction, and executor handoff; selectors and exact-value matching cannot reliably identify transformed or replaced secrets. Existing snapshots and explicit non-secret Playwright fill intents remain usable, but later page-derived changes are not captured. Legacy Playwright dispatch receipts without the guard marker suspend capture conservatively. Keep recovery allowlists non-sensitive; this does not scrub old files or promise complete redaction of screenshots/page evidence. Declare `recovery.reconcile` and `reconcileOutcome` before a possibly uncertain effect. `reconcile --run ID` evaluates that UI evidence. For a completed deterministic dispatch, matching evidence advances its cursor once instead of resubmitting. For a completed autonomous dispatch without a cursor, continuation stays blocked rather than guessing. diff --git a/package.json b/package.json index b5efe18..3107f04 100644 --- a/package.json +++ b/package.json @@ -26,7 +26,7 @@ "test:python": "python3 -m unittest discover -s worker/tests -v", "check": "npm run typecheck && npm test && npm run test:python", "prepack": "npm run build", - "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs" + "test:browser": "node test/adapter.smoke.mjs && node test/workflows.smoke.mjs && node test/vision.smoke.mjs && node test/review-followups.smoke.mjs && node test/recovery-capture.smoke.mjs && node test/environment-retention.smoke.mjs" }, "dependencies": { "@effect/platform-node": "4.0.0-rc.116", diff --git a/src/page-worker.ts b/src/page-worker.ts index ea080e4..e7020c3 100644 --- a/src/page-worker.ts +++ b/src/page-worker.ts @@ -94,12 +94,28 @@ async function captureScope(page: Page, s: Scoped): Promise<{ frame: Frame; loca } return { frame, locator: frame.locator(css(s.selector ?? "body", s.shadow)) }; } +function suspendRecoveryCapture(journal: Journal): void { + if (!journal.events().some(e => e.type === "recovery.capture_suspended")) { + journal.append("recovery.capture_suspended", { + reason: "Environment-backed input may be moved or transformed by the page. Automatic recovery-value capture is disabled for this run; prior snapshots and explicit non-secret input intents remain available.", + }); + } +} async function captureFields(page: Page, journal: Journal, save = true): Promise { const task = journal.meta().task; const fields = task.recovery?.fields ?? []; if (fields.length === 0) return; - const protectedSteps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])] - .filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); + const steps = [...(task.steps ?? []), ...(task.recovery?.reconstruct ?? [])]; + const events = journal.events(); + const suspended = events.some(e => e.type === "recovery.capture_suspended"); + // Old receipts cannot prove which environment-input boundary was crossed. + const legacy = steps.some(s => s.valueFromEnv !== undefined) && events.some(e => + e.type === "action.start" && e.data.engine === "playwright" && e.data.recoveryGuarded !== true); + if (suspended || legacy) { + if (!suspended) suspendRecoveryCapture(journal); + save = false; // Still reject known protected aliases, but never read new page values. + } + const protectedSteps = steps.filter(s => s.valueFromEnv !== undefined && s.selector !== undefined); // These values never enter browser evaluation arguments. A page may redefine // its prototypes; equality against environment secrets belongs in this process. const protectedValues = new Set(protectedSteps @@ -224,8 +240,12 @@ async function runSteps(page: Page, journal: Journal, job: PageJob, snap: () => let actionId: string | undefined; if (s.kind !== "checkpoint") { if (remaining(journal.state()).actions <= 0) throw new BrowserError({ code: "budget_exhausted", reason: "Action budget exhausted." }); + // Selectors, node identities and exact-value matching cannot follow an + // arbitrary formatter/rerender. Persist this boundary before dispatch so + // resume, reconstruction and a Jev/Midscene handoff cannot forget it. + if (s.valueFromEnv) suspendRecoveryCapture(journal); actionId = randomUUID(); - journal.append("action.start", { id: actionId, kind: s.kind, safeToRepeat: s.safeToRepeat === true, step: index, selector: s.selector, frames: s.frames, engine: "playwright" }); + journal.append("action.start", { id: actionId, kind: s.kind, safeToRepeat: s.safeToRepeat === true, step: index, selector: s.selector, frames: s.frames, engine: "playwright", recoveryGuarded: true }); } switch (s.kind) { case "navigate": { diff --git a/test/environment-retention.smoke.mjs b/test/environment-retention.smoke.mjs new file mode 100644 index 0000000..1627041 --- /dev/null +++ b/test/environment-retention.smoke.mjs @@ -0,0 +1,117 @@ +import assert from 'node:assert/strict'; +import * as fs from 'node:fs'; +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { execFileSync } from 'node:child_process'; +import { fixture } from './support/chrome.mjs'; +import { Cdp } from '../dist/cdp.js'; +import { createRun, atomic } from '../dist/journal.js'; +import { attachPage, pageJob } from '../dist/page-worker.js'; + +const key = 'FE2E_RERENDER_SECRET', secret = 'synthetic-private-123'; +const formatted = [...secret].join(' '), previous = process.env[key]; +process.env[key] = secret; +const f = await fixture(), raw = await Cdp.open(f.session), passed = []; +const worker = fileURLToPath(new URL('../worker', import.meta.url)); +function assertPrivate(journal) { + for (const entry of fs.readdirSync(journal.dir, { recursive: true })) { + const file = path.join(journal.dir, entry); + if (fs.statSync(file).isFile()) { + const text = fs.readFileSync(file, 'utf8'); + assert.equal(text.includes(secret), false, entry); + assert.equal(text.includes(formatted), false, entry); + } + } +} +function checkJevHandoff(journal) { + // A fresh Python adapter must honor the durable Node suspension even without + // the original environment. Any capture evaluation would fail this test. + const env = { ...process.env }; delete env[key]; + execFileSync('python3', ['-c', `import sys +sys.path.insert(0, sys.argv[1]) +from jev_runner import Journal, capture +class Page: + def evaluate(self, expression): + raise AssertionError('Suspended Jev recovery must not evaluate the page') +capture(Page(), Journal(sys.argv[2], sys.argv[3])) +`, worker, f.directory, journal.runId], { env, timeout: 10_000 }); +} +async function scenario(name, variant, reconstruct, check) { + const { targetId } = await raw.send('Target.createTarget', { url: 'about:blank' }); + const connection = await attachPage(f.session, targetId); + try { + const page = connection.page; + await page.goto(f.url, { waitUntil: 'load' }); + await page.setContent(` + `); + const step = { kind: 'fill', selector: '#token', valueFromEnv: key }; + const task = { url: f.url, goal: name, steps: [step], recovery: { + fields: [{ key: 'public', selector: '.active-input' }, { key: 'safe', selector: '#safe' }], + ...(reconstruct ? { reconstruct: [{ kind: 'navigate', value: f.url, safeToRepeat: true }, { ...step, safeToRepeat: true }] } : {}), + } }; + const { journal } = createRun(f.directory, task, false, f.profile); + journal.append('target', { targetId, browserId: f.session.browserId }); + atomic(path.join(f.directory, 'targets', f.session.namespace, `${targetId}.json`), { targetId, browserId: f.session.browserId }); + const invoke = (operation = 'capture', extra = {}) => pageJob({ + root: f.directory, runId: journal.runId, session: f.session, targetId, + operation, steps: [step], timeoutMs: 20_000, ...extra, + }, new AbortController().signal); + await invoke(); + await check({ page, journal, step, invoke }); + assertPrivate(journal); checkJevHandoff(journal); + passed.push(name); + } finally { + await connection.browser.close(); await raw.send('Target.closeTarget', { targetId }); + } +} +try { + for (const variant of ['retag', 'replace', 'mirror']) for (const reconstruct of [false, true]) { + await scenario(`Formatted ${variant} value is not retained; reconstruct=${reconstruct}`, variant, reconstruct, + async ({ page, journal, invoke }) => { + await invoke('steps', reconstruct ? { saveProgress: false } : {}); + assert.equal(await page.locator('.active-input').inputValue(), formatted); + const retained = journal.recoveryFields(); + assert.equal(retained.public.value, 'safe-before'); + assert.equal(retained.public.source, 'observed'); + const events = journal.events(); + const boundary = events.find(e => e.type === 'recovery.capture_suspended'); + assert.ok(boundary); + assert.ok(boundary.seq < events.find(e => e.type === 'action.start').seq); + // Reattachment, a later capture and loss of the env value cannot undo suspension. + delete process.env[key]; + try { await invoke(); await invoke(); } finally { process.env[key] = secret; } + assert.deepEqual(journal.recoveryFields(), retained); + assert.equal(journal.events().filter(e => e.type === 'recovery.capture_suspended').length, 1); + }); + } + await scenario('Explicit public inputs remain recoverable after secret dispatch', 'mirror', false, + async ({ journal, step, invoke }) => { + await invoke('steps', { steps: [step, { kind: 'fill', selector: '#safe', value: 'user-provided-public' }] }); + assert.equal(journal.recoveryFields().safe.value, 'user-provided-public'); + assert.equal(journal.recoveryFields().safe.source, 'intent'); + assert.equal(journal.recoveryFields().public.value, 'safe-before'); + }); + await scenario('Legacy dispatch without provenance suspends capture on resume', 'mirror', false, + async ({ page, journal, invoke }) => { + journal.append('action.start', { id: 'legacy', kind: 'fill', selector: '#token', step: 0, engine: 'playwright' }); + await page.locator('#token').fill(secret); + journal.append('action.end', { id: 'legacy' }); + delete process.env[key]; + try { await invoke(); } finally { process.env[key] = secret; } + assert.equal(journal.recoveryFields().public.value, 'safe-before'); + assert.ok(journal.events().some(e => e.type === 'recovery.capture_suspended')); + }); +} finally { + await raw.close(); await f.cleanup(); + if (previous === undefined) delete process.env[key]; else process.env[key] = previous; +} +console.log(JSON.stringify({ suite: 'environment retention across dispatch and handoff', passed, paidModelCalls: 0 })); diff --git a/test/recovery-capture.smoke.mjs b/test/recovery-capture.smoke.mjs index 02110c4..eb3380d 100644 --- a/test/recovery-capture.smoke.mjs +++ b/test/recovery-capture.smoke.mjs @@ -169,6 +169,32 @@ try { } await scenario('Required assertion scopes remain strict', { steps: [] }, '

No iframe

', async ({ page }) => { await assert.rejects(() => checkOne(page, { kind: 'count', frames: ['#missing'], selector: 'input', value: '0' }), error => error.code === 'unsupported_scope'); + assert.equal((await checkOne(page, { kind: 'visible', selector: '#missing', value: 'false' })).passed, true); + }); + + const publicFields = Array.from({ length: 8 }, (_, i) => ({ key: `public${i}`, selector: `#public${i}` })); + await scenario('Protected frame traversal is shared within, not across, snapshots', { + steps: Array.from({ length: 20 }, (_, i) => ({ kind: 'fill', valueFromEnv: key, + selector: i % 2 ? '#nestedName' : '#frameName', frames: i % 2 ? ['#remote', '#nested'] : ['#remote'] })), + recovery: { fields: publicFields }, + }, remote => publicFields.map((_, i) => ``).join('') + ``, async ({ page, journal, invoke }) => { + // Count real frame-path lookups, not timing: added fields/duplicate steps + // must not multiply browser round trips. The second snapshot must be fresh. + const prototype = Object.getPrototypeOf(page.mainFrame()), original = prototype.locator; + let queries = 0; + prototype.locator = function (selector, ...options) { + if (selector === 'css:light=#remote' || selector === 'css:light=#nested') queries++; + return Reflect.apply(original, this, [selector, ...options]); + }; + try { + for (let attempt = 0; attempt < 2; attempt++) { + queries = 0; + await invoke(); + assert.equal(queries, 3, 'Resolve the two distinct paths once each per snapshot'); + assert.equal(Object.keys(journal.recoveryFields()).length, 8); + } + } finally { prototype.locator = original; } + assertNotRetained(journal); }); } finally { await raw.close(); await f.cleanup(); diff --git a/worker/jev_runner.py b/worker/jev_runner.py index c38cc44..6f2634c 100644 --- a/worker/jev_runner.py +++ b/worker/jev_runner.py @@ -72,9 +72,23 @@ def ids(t): def capture(browser, journal, action=None, text=None): task = journal.meta["task"] fields = task.get("recovery", {}).get("fields", []) + if not fields: + return + steps = [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])] + events = journal.events() + # A scoped secret fill can hand off to Jev after the page has formatted or + # moved its value. Never infer that it became public from current selectors. + if any(e["type"] == "recovery.capture_suspended" for e in events): + return + if any(s.get("valueFromEnv") is not None for s in steps) and any( + e["type"] == "action.start" and e["data"].get("engine") == "playwright" + and e["data"].get("recoveryGuarded") is not True for e in events + ): + journal.append("recovery.capture_suspended", reason="Legacy dispatch cannot establish the environment-input boundary. Keep existing recovery inputs; do not collect new page values.") + return # Capture also runs after a scoped/environment-backed step hands off to Jev. # Protect aliases in the top-level document without reading secret values. - protected = [s["selector"] for s in [*task.get("steps", []), *task.get("recovery", {}).get("reconstruct", [])] + protected = [s["selector"] for s in steps if s.get("valueFromEnv") is not None and s.get("selector") and not s.get("frames")] for field in fields: if field.get("frames") or field.get("shadow") == "open": diff --git a/worker/tests/test_jev_retention.py b/worker/tests/test_jev_retention.py new file mode 100644 index 0000000..7f222e6 --- /dev/null +++ b/worker/tests/test_jev_retention.py @@ -0,0 +1,66 @@ +"""A fresh managed-Jev process must honor the shared capture boundary.""" +import json +from pathlib import Path +import sys +import tempfile +import time +import unittest +from unittest.mock import Mock + +sys.path.insert(0, str(Path(__file__).resolve().parents[1])) +from jev_runner import Journal, capture + + +class RecoveryBoundaryTests(unittest.TestCase): + def setUp(self): + self.temp = tempfile.TemporaryDirectory() + self.addCleanup(self.temp.cleanup) + self.run_id = 'r_' + 'a' * 32 + self.directory = Path(self.temp.name) / 'runs' / self.run_id + self.directory.mkdir(parents=True) + self.task = { + 'steps': [{'kind': 'fill', 'selector': '#token', 'valueFromEnv': 'SYNTHETIC_INPUT'}], + 'recovery': {'fields': [{'key': 'public', 'selector': '#public'}]}, + } + self.write_meta() + (self.directory / 'events.jsonl').write_text('') + self.journal = Journal(self.temp.name, self.run_id) + self.page = Mock() + self.page.evaluate.return_value = {'value': 'public', 'matches': False} + + def write_meta(self): + (self.directory / 'task.json').write_text(json.dumps({ + 'version': 2, 'expiresAt': time.time() * 1000 + 60_000, 'task': self.task, + })) + + def test_suspension_survives_new_journal_and_skips_model_generated_intents(self): + self.journal.append('recovery.capture_suspended', reason='environment boundary') + fresh = Journal(self.temp.name, self.run_id) + capture(self.page, fresh) + capture(self.page, fresh, {'kind': 'fill', 'node': 1}, 'model-generated') + self.page.evaluate.assert_not_called() + self.assertFalse((self.directory / 'recovery.json').exists()) + + def test_legacy_playwright_receipt_is_not_assumed_public(self): + self.journal.append('action.start', engine='playwright', id='old') + capture(self.page, self.journal) + capture(self.page, Journal(self.temp.name, self.run_id)) + self.page.evaluate.assert_not_called() + self.assertEqual(sum(e['type'] == 'recovery.capture_suspended' for e in self.journal.events()), 1) + + def test_guarded_public_dispatch_does_not_disable_capture(self): + self.journal.append('action.start', engine='playwright', id='public', recoveryGuarded=True) + capture(self.page, self.journal) + self.page.evaluate.assert_called_once() + self.assertEqual(json.loads((self.directory / 'recovery.json').read_text())['public']['value'], 'public') + + def test_legacy_run_without_environment_steps_still_captures(self): + self.task['steps'] = [{'kind': 'click', 'selector': '#open'}] + self.write_meta() + self.journal.append('action.start', engine='playwright', id='old-public') + capture(self.page, Journal(self.temp.name, self.run_id)) + self.page.evaluate.assert_called_once() + + +if __name__ == '__main__': + unittest.main()