diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0f9cade..544ac54 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -46,3 +46,12 @@ 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 + - 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 429e100..d363bcf 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. +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. 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/package.json b/package.json index 58cb053..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" + "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 2bccb64..e7020c3 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,15 +75,124 @@ 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 { - 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"); +// 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)) }; +} +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 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 + .map(step => process.env[step.valueFromEnv!]) + .filter((value): value is string => value !== undefined && value.length > 0)); + const handles: ElementHandle[] = []; + try { + 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; + // 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 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) => { + 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." }); + } + } + continue; + } + + // 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: { 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 }; + 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." }); + if (result.available === false) continue; + journal.saveField(f.key, result.actual, "observed"); + } + } finally { await Promise.allSettled(handles.map(handle => handle.dispose())); } } function valueFor(s: Step, journal: Journal, retained = journal.recoveryFields()): string { if (s.valueFromInput) { @@ -116,6 +225,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 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" }); @@ -130,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/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/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); 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 new file mode 100644 index 0000000..eb3380d --- /dev/null +++ b/test/recovery-capture.smoke.mjs @@ -0,0 +1,204 @@ +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'); + 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(); + 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`); diff --git a/test/review-followups.smoke.mjs b/test/review-followups.smoke.mjs new file mode 100644 index 0000000..da9129d --- /dev/null +++ b/test/review-followups.smoke.mjs @@ -0,0 +1,107 @@ +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 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 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' }], + 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..7e28e15 100644 --- a/test/support/chrome.mjs +++ b/test/support/chrome.mjs @@ -16,6 +16,27 @@ 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('/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 @@ -52,7 +73,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..6f2634c 100644 --- a/worker/jev_runner.py +++ b/worker/jev_runner.py @@ -70,17 +70,38 @@ 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", []) + 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 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": 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]; + // 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}; - })(""" + 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..98c260f 100644 --- a/worker/tests/managed_smoke.py +++ b/worker/tests/managed_smoke.py @@ -80,6 +80,48 @@ 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) + # 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"): + 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() original_act = Browser.act @@ -101,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"]})) + 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 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()