From 0055d2a79740b0f66373c7bee02bdc5193c2b4a9 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 11:43:25 -0400 Subject: [PATCH 01/30] feat(browser): attach recorder to existing sessions --- README.md | 15 + claims.yaml | 17 + docs/BROWSER_RECORDING.md | 163 ++++ docs/LIMITS.md | 2 +- docs/PRODUCT_STATUS.md | 2 +- docs/SURFACES.md | 13 + docs/VERIFICATION.md | 2 + docs/verification.json | 16 +- openadapt_flow/__main__.py | 66 +- openadapt_flow/backends/playwright_backend.py | 42 +- openadapt_flow/compiler/compile.py | 97 +++ openadapt_flow/interactive_recorder.py | 757 ++++++++++++++++-- openadapt_flow/recorder.py | 19 +- tests/test_browser_attach.py | 588 ++++++++++++++ tests/test_compiler.py | 127 +++ tests/test_recorder.py | 2 + 16 files changed, 1831 insertions(+), 97 deletions(-) create mode 100644 docs/BROWSER_RECORDING.md create mode 100644 tests/test_browser_attach.py diff --git a/README.md b/README.md index e7071317..83e414f7 100644 --- a/README.md +++ b/README.md @@ -187,6 +187,10 @@ openadapt-flow record --backend web --url https://your.app --out rec openadapt-flow compile rec --out bundle --name my-task openadapt-flow replay bundle --backend web --url https://your.app +# Browser with an existing local SSO/2FA session: attach one open tab. +openadapt-flow record --backend web --url https://your.app \ + --browser-cdp-endpoint http://127.0.0.1:9222 --out rec + # Native Windows: Capture records the local target window. WAA drives replay. openadapt-flow record --backend windows --window "Target App" \ --task "add a patient note" --out rec @@ -241,6 +245,17 @@ ignoring them; pass them to `replay` or `run`. Drive a real deployment with effects, actuation, durable, and policy sections from one config. Recorded parameter values are the defaults, and `--param` overrides them at replay. +The browser recorder can launch a clean Playwright browser or attach to one +existing local Chromium tab. Attach mode preserves a browser profile that has +already completed sign-in, SSO, or 2FA. It refuses remote CDP endpoints and +ambiguous same-origin tabs. It does not navigate or close the attached browser. +You can resize the tab or move its window between monitors. Flow waits for a +stable CSS-pixel frame and binds the next event to the new viewport. It refuses +an action only if that action overlaps the coordinate-space transition. +See the [browser recording guide](docs/BROWSER_RECORDING.md) for setup, exact +tab selection, secret handling, and the boundary with the Capture Chrome +extension prototype. + **You don't have to name parameters up front.** The recorder passively captures each typed field's label (DOM/accessibility, or nearby OCR on pixel paths), and `compile` proposes a parameter named from it (`"Insurance No."` diff --git a/claims.yaml b/claims.yaml index f1874dd9..d89ada32 100644 --- a/claims.yaml +++ b/claims.yaml @@ -65,6 +65,18 @@ claims: proves: >- The deterministic replayer resolves steps, substitutes parameters, enforces postconditions and the risk gate — no model in the loop. + - path: tests/test_browser_attach.py + node: test_live_cdp_attach_records_compiles_and_leaves_browser_running_three_trials + proves: >- + Three real Chromium CDP-attach trials record and compile the same + synthetic workflow, exclude password values before persistence, + preserve CSS-pixel frame/coordinate alignment, and detach without + closing the external browser. A live case records and compiles actions + across viewport and monitor-scale changes. It binds each event to its + exact frame dimensions. A separate live case refuses an action that + overlaps the transition. Unit cases refuse remote endpoints, + cross-origin selectors and navigation, iframe events, invalid viewport + evidence, and ambiguous same-origin tabs. caveats: - >- "Supported" is scoped to the reference headless-browser backend in this @@ -73,6 +85,11 @@ claims: - >- The full record->compile->replay browser suite runs in the required e2e-browser PR gate and repeats in the weekly compatibility matrix. + - >- + Existing-session attachment is Chromium-only and loopback-only. It + requires a dedicated browser process started with remote debugging. + It does not claim support for the Capture Chrome extension prototype + or direct extension replay. # -------------------------------------------------- deterministic $0 replay - id: deterministic-zero-model-replay diff --git a/docs/BROWSER_RECORDING.md b/docs/BROWSER_RECORDING.md new file mode 100644 index 00000000..92aa08cd --- /dev/null +++ b/docs/BROWSER_RECORDING.md @@ -0,0 +1,163 @@ +# Browser recording + +OpenAdapt has one supported browser recording contract. `openadapt-flow` +uses a Playwright page to retain ordered input events, DOM identity, field +geometry, exact before/after frames, and source-time secret redaction. The +result is the same compile-ready recording for both browser entry modes: + +- **Launch mode:** Flow starts a new Chromium browser and opens `--url`. +- **Attach mode:** Flow connects to an existing local Chromium browser and + binds one open tab. This mode keeps a browser session that already completed + sign-in, SSO, or 2FA. + +Both modes are part of the Browser / Playwright Beta surface. Attach mode uses +Chromium DevTools Protocol only as the local connection transport. The +recorder, schema, compiler, secret handling, and governed replay path do not +change. + +## Launch a new recording browser + +```bash +openadapt-flow record --backend web \ + --url https://your.app \ + --out recording +``` + +Flow opens the URL. Perform the workflow. Then press Ctrl-C in the terminal or +close the recording window. + +## Attach an existing signed-in browser + +Start Chromium with a dedicated debugging profile. Reuse this profile for +later recordings if it must retain its signed-in session. Do not enable remote +debugging on a sensitive general-purpose browser profile. + +macOS with Google Chrome: + +```bash +"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \ + --remote-debugging-address=127.0.0.1 \ + --remote-debugging-port=9222 \ + --user-data-dir="./.openadapt-chrome-profile" +``` + +Linux with Google Chrome or Chromium: + +```bash +google-chrome \ + --remote-debugging-address=127.0.0.1 \ + --remote-debugging-port=9222 \ + --user-data-dir="./.openadapt-chrome-profile" +``` + +Windows PowerShell with Google Chrome: + +```powershell +& "$env:ProgramFiles\Google\Chrome\Application\chrome.exe" ` + --remote-debugging-address=127.0.0.1 ` + --remote-debugging-port=9222 ` + --user-data-dir="$PWD\.openadapt-chrome-profile" +``` + +Open and sign in to the application in that browser. Then attach the recorder: + +```bash +openadapt-flow record --backend web \ + --url https://your.app \ + --browser-cdp-endpoint http://127.0.0.1:9222 \ + --out recording +``` + +Flow selects the sole open HTTP or HTTPS tab on the `--url` origin. It does not +navigate the tab. It also does not close the tab or browser when recording +finishes. + +If two or more open tabs have that origin, Flow refuses to guess. Supply the +exact current URL: + +```bash +openadapt-flow record --backend web \ + --url https://your.app \ + --browser-cdp-endpoint http://127.0.0.1:9222 \ + --browser-page-url 'https://your.app/work/items?view=open' \ + --out recording +``` + +Diagnostic messages omit URL query and fragment values. The exact selector is +used only to bind the requested tab and is not stored as separate attachment +metadata. The CDP endpoint is not stored. The normal recording evidence does +retain the declared app URL and each observed page URL before and after an +action. Those URLs can contain query or fragment values. Treat them as +sensitive recording data. + +## Safety and privacy contract + +- The CDP endpoint must use `localhost` or a loopback IP address and must have + an explicit port. Flow refuses a remote endpoint, URL credentials, query, or + fragment. +- The selected tab must have the same origin as `--url`. Zero matches and + ambiguous matches are refusals. +- The selected tab must stay on that origin for the full recording. A + cross-origin navigation stops the recording and does not produce complete + metadata. +- Attach mode does not combine with `--headless`. The external browser owns + its display mode. +- Input event payloads carry a unique recording-session binding and have a + 1 MB limit. Flow removes the current document listeners when it detaches. +- Attached screenshots use CSS pixels and the actual live viewport. Thus, DOM + coordinates and retained frame coordinates stay aligned on high-density + displays. +- You can resize the tab or move its window between monitors while no action is + in progress. Flow observes viewport and device-scale changes, waits for a + stable CSS-pixel frame, and then starts a new per-event coordinate baseline. + `meta.json` retains the viewport history. Each frame-backed event retains its + exact `viewport_before` and `viewport_after`. +- Flow refuses only an action that overlaps a resize or monitor-scale change. + In that case, no exact pre-action frame exists in the new coordinate space, + so the recording stops without complete metadata. Stop interacting for a + moment after a resize. Recording then continues automatically. +- `input[type=password]` and fields declared with `--secret FIELD` never send + their values to Python. Flow redacts their field rectangle in the retained + evidence frames. Other typed values and visible page content are recording + evidence and can contain sensitive data. Keep raw recordings inside the + approved local boundary. + +## Why the Capture Chrome extension is not this path + +The `openadapt-capture` repository contains a custom Chrome extension +prototype. It proved useful DOM event capture, but its current direct +WebSocket and replay design does not implement the supported contract above. +It does not yet bind messages to an authenticated recording session, one tab, +one document, and an ordered acknowledged event stream. It also does not +provide the compiler's exact before/after frame binding and source-time secret +redaction. Its direct DOM replay can dispatch actions without the governed +runtime's identity, policy, fresh-frame, and effect checks. + +The prototype should remain available for development. It can become a +supported acquisition transport after it does all of the following: + +1. Use the shared Flow event and evidence schema. Do not create a second + compiler or replay format. +2. Redact secret values before they cross the extension boundary. +3. Bind and authenticate the browser profile, tab, document, run, session, and + monotonically increasing event sequence. A reconnect must acknowledge or + safely resume events instead of dropping them. +4. Retain exact frame-to-event evidence and bind each event to its current + viewport coordinate system. +5. Send recordings to the existing compiler. Do not perform direct replay. +6. Pass the same three-trial record, compile, secret, ambiguity-refusal, and + browser-lifecycle tests as the Playwright attach mode. + +Until that contract exists, the extension is a prototype component. This label +does not apply to `openadapt-capture` as a whole. Capture is the canonical +native recorder; the browser recorder stays Playwright-native because browser +DOM identity and source-time secret handling are load-bearing. + +## Current boundary + +Attach mode supports local Chromium-family browsers that expose a CDP +endpoint. It requires a browser process started with remote debugging and a +separate user-data directory. It does not claim Firefox, WebKit, arbitrary +Chrome extensions, an ordinary browser process that was not started for local +debugging, cross-origin tab selection, separately qualified cross-frame/iframe +recording, or direct extension replay. diff --git a/docs/LIMITS.md b/docs/LIMITS.md index 78cd50b8..f6ba95e5 100644 --- a/docs/LIMITS.md +++ b/docs/LIMITS.md @@ -18,7 +18,7 @@ For the current evidence behind each maturity claim, see | Capability | Maturity | What the claim means | What it does not mean | | --- | --- | --- | --- | -| Browser record, compile, and replay | **Beta** | The reference browser path runs end to end in automated and clean-environment tests. | It is not evidence for every site, browser extension, authentication flow, or long-running production workload. | +| Browser record, compile, and replay | **Beta** | The launched-browser path runs end to end in automated and clean-environment tests. The attached-browser path reuses the same recorder and passes 3 real Chromium record-and-compile trials with secret exclusion and external-browser survival checks. A live resize case also compiles actions from two viewport and device-scale baselines. | It is not evidence for every site, browser extension, authentication flow, or long-running production workload. Attach mode requires a loopback CDP endpoint and one same-origin Chromium tab. An action that overlaps a resize is refused because its pre-action coordinate evidence is not exact. The custom Capture extension remains a prototype and is not a direct replay path. | | Healthy replay with zero model calls | **Beta** | A run that resolves from retained evidence can execute without a language or vision model; the run report counts model calls. | Zero model calls does not mean zero network traffic. The target application, hosted control plane, remote backend, or effect verifier may still use the network. | | Deterministic re-resolution | **Beta** | Bounded visual or structural drift can be resolved through non-model evidence and recorded as a reviewable change. | It is not general adaptation to a redesigned workflow, changed business rules, or missing evidence. | | AI-assisted repair | **Experimental** | An explicitly enabled model can propose a target or interpret a changed screen. Existing runtime checks still apply. | A model proposal is not authorization, proof of identity, or proof that a business transaction succeeded. | diff --git a/docs/PRODUCT_STATUS.md b/docs/PRODUCT_STATUS.md index 955f88f3..92db2399 100644 --- a/docs/PRODUCT_STATUS.md +++ b/docs/PRODUCT_STATUS.md @@ -23,7 +23,7 @@ and its generated view is [`VERIFICATION.md`](VERIFICATION.md). | Surface | Status | What is proven | Boundary that remains | | --- | --- | --- | --- | | Demonstration compiler and bundle | **Beta** | Browser recording compiles into a parameterized, inspectable bundle in CI. | One demonstration can under-specify intent; production policies and effect bindings still require operator work. | -| Browser / Playwright recording and replay | **Beta** | Record, compile, replay, deterministic drift repair, reports, and refusal all run end to end against MockMed; a bounded OpenEMR result is published separately. | The reference path is not evidence for arbitrary sites, long-term drift, or production reliability. | +| Browser / Playwright recording and replay | **Beta** | Record, compile, replay, deterministic drift repair, reports, and refusal all run end to end against MockMed; a bounded OpenEMR result is published separately. The required browser suite also performs 3 real Chromium CDP-attach record-and-compile trials, checks source-time password exclusion, proves that recorder shutdown leaves the external browser running, and compiles actions across a live viewport and device-scale change. | The reference path is not evidence for arbitrary sites, long-term drift, or production reliability. Attach mode is Chromium-only, loopback-only, and requires a browser started with remote debugging. It refuses an action that overlaps a resize transition. It does not promote the Capture Chrome extension prototype or direct extension replay. | | Healthy zero-model replay | **Beta** | Repeated CI runs use the deterministic ladder with zero model calls. | Optional model grounding is a separate opt-in fallback; a changed app can still halt. | | Deterministic re-resolution | **Beta** | Theme, moved-control, and renamed-control fixtures resolve through non-model rungs and emit reviewable patches. | It covers bounded evidence-preserving drift, not arbitrary workflow or business-logic change. | | AI-assisted repair | **Experimental** | Local/remote VLM contracts, egress gates, refusal behavior, and retention boundaries are tested. | Off by default; model accuracy is not a safety guarantee and real deployment quality is unmeasured. | diff --git a/docs/SURFACES.md b/docs/SURFACES.md index 06de164d..d2f2f4ea 100644 --- a/docs/SURFACES.md +++ b/docs/SURFACES.md @@ -58,6 +58,10 @@ openadapt-flow record --backend web --url https://your.app --out rec openadapt-flow compile rec --out bundle --name my-task openadapt-flow replay bundle --url https://your.app +# Attach the same recorder to one existing signed-in local Chromium tab. +openadapt-flow record --backend web --url https://your.app \ + --browser-cdp-endpoint http://127.0.0.1:9222 --out rec + # Windows: Capture records the local window; the in-guest WAA agent replays it. openadapt-flow record --backend windows --window "Target App" --out rec openadapt-flow compile rec --out bundle --name my-task @@ -100,6 +104,15 @@ session cannot control them, so `record` refuses them instead of accepting an unused flag. Pass them to `replay`/`run`; `run ... --config deploy.yaml --profile standard|regulated` wires the same selection for a real deployment. +The browser attach mode keeps the Playwright-native recording contract. It +binds one same-origin tab and reuses the same event schema, DOM evidence, +before/after frames, secret redaction, compiler, and governed replay path as a +browser that Flow launches. The endpoint is local-loopback only. Flow refuses +ambiguous tabs and does not navigate or close the attached browser. It +rebaselines exact event/frame coordinates after an idle resize or +monitor-scale change. It refuses an action that overlaps that transition. See +[`BROWSER_RECORDING.md`](BROWSER_RECORDING.md). + ## The two remote execution modes Remote systems (a Windows guest, a virtual desktop, a published app) can be diff --git a/docs/VERIFICATION.md b/docs/VERIFICATION.md index 8cca61fc..e1154dbb 100644 --- a/docs/VERIFICATION.md +++ b/docs/VERIFICATION.md @@ -30,11 +30,13 @@ | `tests/e2e/test_record_compile_replay.py` | test | ci (required PR gate (e2e-browser)) | supported | Records the MockMed browser demo once, compiles it, and replays it under baseline + theme/move/rename drift and parameter substitution through the headless-browser Backend. | | `tests/test_mockmed.py` | test | ci (required PR gate (test)) | supported | The reference browser demo app and its drift screens render deterministically (no CSS transitions), so replay is repeatable. | | `tests/test_replayer.py` | test | ci (required PR gate (test)) | supported | The deterministic replayer resolves steps, substitutes parameters, enforces postconditions and the risk gate — no model in the loop. | +| `tests/test_browser_attach.py` | test | ci (required PR gate (test)) | supported | Three real Chromium CDP-attach trials record and compile the same synthetic workflow, exclude password values before persistence, preserve CSS-pixel frame/coordinate alignment, and detach without closing the external browser. A live case records and compiles actions across viewport and monitor-scale changes. It binds each event to its exact frame dimensions. A separate live case refuses an action that overlaps the transition. Unit cases refuse remote endpoints, cross-origin selectors and navigation, iframe events, invalid viewport evidence, and ambiguous same-origin tabs. | **Caveats (honest limits):** - "Supported" is scoped to the reference headless-browser backend in this registry. Desktop and remote-display workflows use the separately scoped acceptance and code-qualified claims below. - The full record->compile->replay browser suite runs in the required e2e-browser PR gate and repeats in the weekly compatibility matrix. +- Existing-session attachment is Chromium-only and loopback-only. It requires a dedicated browser process started with remote debugging. It does not claim support for the Capture Chrome extension prototype or direct extension replay. ### `deterministic-zero-model-replay` — supported — bound to required CI pass evidence diff --git a/docs/verification.json b/docs/verification.json index 04e3726c..63cb370b 100644 --- a/docs/verification.json +++ b/docs/verification.json @@ -1,5 +1,5 @@ { - "generated_at": "committed registry state (regenerate: scripts/validate_claims.py --report)", + "generated_at": "2026-08-15T00:03:37+02:00 (git HEAD commit date)", "green_check_run": false, "green_check_job": null, "green_check_scope": [], @@ -19,7 +19,8 @@ "strongest_evidence": "supported", "caveats": [ "\"Supported\" is scoped to the reference headless-browser backend in this registry. Desktop and remote-display workflows use the separately scoped acceptance and code-qualified claims below.", - "The full record->compile->replay browser suite runs in the required e2e-browser PR gate and repeats in the weekly compatibility matrix." + "The full record->compile->replay browser suite runs in the required e2e-browser PR gate and repeats in the weekly compatibility matrix.", + "Existing-session attachment is Chromium-only and loopback-only. It requires a dedicated browser process started with remote debugging. It does not claim support for the Capture Chrome extension prototype or direct extension replay." ], "evidence": [ { @@ -57,6 +58,17 @@ "ci_job": "test", "junit_status": null, "proves": "The deterministic replayer resolves steps, substitutes parameters, enforces postconditions and the risk gate \u2014 no model in the loop." + }, + { + "path": "tests/test_browser_attach.py", + "kind": "test", + "exists": true, + "strength": "supported", + "gating": "ci (required PR gate (test))", + "node": "test_live_cdp_attach_records_compiles_and_leaves_browser_running_three_trials", + "node_found": true, + "junit_status": null, + "proves": "Three real Chromium CDP-attach trials record and compile the same synthetic workflow, exclude password values before persistence, preserve CSS-pixel frame/coordinate alignment, and detach without closing the external browser. A live case records and compiles actions across viewport and monitor-scale changes. It binds each event to its exact frame dimensions. A separate live case refuses an action that overlaps the transition. Unit cases refuse remote endpoints, cross-origin selectors and navigation, iframe events, invalid viewport evidence, and ambiguous same-origin tabs." } ], "errors": [] diff --git a/openadapt_flow/__main__.py b/openadapt_flow/__main__.py index 67a445d8..ccae9b26 100644 --- a/openadapt_flow/__main__.py +++ b/openadapt_flow/__main__.py @@ -895,7 +895,16 @@ def _cmd_record(args: argparse.Namespace) -> int: print(demo_default_notice(backend, from_last_used=last is not None)) elif profile == "demo": store_last_surface(_report_backend_kind(backend)) + browser_attach_requested = bool( + getattr(args, "browser_cdp_endpoint", None) + or getattr(args, "browser_page_url", None) + ) if backend in ("windows", "macos", "linux", "rdp", "citrix"): + if browser_attach_requested: + raise SystemExit( + "record: --browser-cdp-endpoint and --browser-page-url apply " + "only to --backend web" + ) return _cmd_record_desktop(args, backend) if ( @@ -920,17 +929,35 @@ def _cmd_record(args: argparse.Namespace) -> int: raise SystemExit( "record --backend web requires --url (the app to record against)." ) + if getattr(args, "browser_page_url", None) and not getattr( + args, "browser_cdp_endpoint", None + ): + raise SystemExit("record: --browser-page-url requires --browser-cdp-endpoint") + if getattr(args, "browser_cdp_endpoint", None) and args.headless: + raise SystemExit( + "record: --headless cannot be combined with " + "--browser-cdp-endpoint; the attached browser controls its own " + "display mode" + ) - from openadapt_flow.interactive_recorder import record_interactive - - out = record_interactive( - args.url, - Path(args.out), - secret_fields=tuple(args.secret or ()), - param_fields=tuple(args.param or ()), - identifier_fields=tuple(getattr(args, "identifier", None) or ()), - headless=args.headless, + from openadapt_flow.interactive_recorder import ( + BrowserAttachError, + record_interactive, ) + + try: + out = record_interactive( + args.url, + Path(args.out), + secret_fields=tuple(args.secret or ()), + param_fields=tuple(args.param or ()), + identifier_fields=tuple(getattr(args, "identifier", None) or ()), + headless=args.headless, + cdp_endpoint=getattr(args, "browser_cdp_endpoint", None), + browser_page_url=getattr(args, "browser_page_url", None), + ) + except BrowserAttachError as exc: + raise SystemExit(f"record: browser attachment refused: {exc}") from exc _stamp_recording_surface(out, "web") print(f"Recording written to {out}") secrets = sorted(args.secret or ()) @@ -4491,6 +4518,27 @@ def build_parser() -> argparse.ArgumentParser: default=None, help="URL of the app to record against (required for --backend web)", ) + p.add_argument( + "--browser-cdp-endpoint", + default=None, + metavar="URL", + help=( + "Attach the web recorder to an already-running local Chromium " + "browser through its loopback DevTools endpoint (for example, " + "http://127.0.0.1:9222). The recorder selects a tab on the " + "--url origin and does not launch, navigate, or close the browser." + ), + ) + p.add_argument( + "--browser-page-url", + default=None, + metavar="URL", + help=( + "Exact current URL of the existing tab to record. Use this with " + "--browser-cdp-endpoint when more than one open tab has the " + "--url origin." + ), + ) p.add_argument("--out", required=True, help="Recording output directory") p.add_argument( "--secret", diff --git a/openadapt_flow/backends/playwright_backend.py b/openadapt_flow/backends/playwright_backend.py index 13f1cd9f..8823732d 100644 --- a/openadapt_flow/backends/playwright_backend.py +++ b/openadapt_flow/backends/playwright_backend.py @@ -15,7 +15,7 @@ import uuid from dataclasses import dataclass, field from datetime import datetime, timezone -from typing import TYPE_CHECKING, Any, Callable, Optional +from typing import TYPE_CHECKING, Any, Callable, Literal, Optional from urllib.parse import urlsplit if TYPE_CHECKING: # pragma: no cover @@ -703,13 +703,24 @@ class PlaywrightBackend: such as the demo driver may use locators; replay never does). """ - def __init__(self, page: "Page") -> None: + def __init__( + self, + page: "Page", + *, + screenshot_scale: Literal["css", "device"] = "device", + ) -> None: """Wrap an existing Playwright page. Args: - page: A page created with viewport 1280x800, deviceScaleFactor=1. + page: A Playwright page. + screenshot_scale: Pixel scale for retained screenshots. The + ordinary launched-browser path uses Playwright's ``device`` + default. A browser attached through CDP uses ``css`` so DOM + event coordinates and retained frame pixels stay in the same + coordinate system even on a high-density display. """ self.page = page + self._screenshot_scale = screenshot_scale # Opaque per-backend key keeps the WeakMap private from ordinary page # code. Python retains only token material keyed by the public # SHA-256 fingerprint; target/row text stays page-local and ephemeral. @@ -726,7 +737,21 @@ def __init__(self, page: "Page") -> None: def viewport(self) -> tuple[int, int]: """(width, height) of the page viewport in pixels.""" size = self.page.viewport_size - if size is None: # pragma: no cover - viewport always set by launch() + if size is None: + # A Chromium page reached through ``connect_over_cdp`` normally + # has no Playwright viewport emulation. Reading the live CSS + # viewport avoids both the old fixed 1280x800 fallback and any + # mutation of the operator's browser window. + try: + live = self.page.evaluate( + "() => ({width: window.innerWidth, height: window.innerHeight})" + ) + width = int(live["width"]) + height = int(live["height"]) + if width > 0 and height > 0: + return (width, height) + except Exception: + pass return VIEWPORT return (size["width"], size["height"]) @@ -2091,11 +2116,15 @@ def guarded_keyboard_frame(self) -> bytes: """ def capture() -> bytes: + options: dict[str, Any] = {} + if self._screenshot_scale == "css": + options["scale"] = "css" return self.page.screenshot( type="png", full_page=False, caret="initial", style="* { caret-color: transparent !important; }", + **options, ) previous = capture() @@ -2197,7 +2226,10 @@ def type_text_guarded( def screenshot(self) -> bytes: """Return the current full-viewport frame as PNG bytes.""" - return self.page.screenshot(type="png", full_page=False) + options: dict[str, Any] = {} + if self._screenshot_scale == "css": + options["scale"] = "css" + return self.page.screenshot(type="png", full_page=False, **options) def click(self, x: int, y: int, *, double: bool = False) -> None: """Click (or double-click) at pixel coordinates via the mouse.""" diff --git a/openadapt_flow/compiler/compile.py b/openadapt_flow/compiler/compile.py index 6987ddac..80217c98 100644 --- a/openadapt_flow/compiler/compile.py +++ b/openadapt_flow/compiler/compile.py @@ -214,6 +214,78 @@ def _read_png(path: Path) -> Optional[bytes]: return path.read_bytes() if path.exists() else None +def _png_viewport(png: bytes, *, source: str) -> tuple[int, int]: + """Return a PNG's exact ``(width, height)`` or reject invalid evidence.""" + + frame = cv2.imdecode(np.frombuffer(png, dtype=np.uint8), cv2.IMREAD_COLOR) + if frame is None: + raise ValueError(f"could not decode {source} PNG") + return (int(frame.shape[1]), int(frame.shape[0])) + + +def _validated_event_viewport( + event: dict, + *, + key: str, + png: Optional[bytes], + event_index: int, +) -> Optional[tuple[int, int]]: + """Bind an event viewport declaration to its exact retained PNG. + + Older recordings do not carry per-event viewport fields. They remain + valid and use the PNG dimensions as the source of truth. A new recording + that does carry the field must match the retained evidence exactly. + """ + + declared = event.get(key) + actual = ( + _png_viewport(png, source=f"event {event_index} {key}") + if png is not None + else None + ) + if declared is None: + return actual + if ( + not isinstance(declared, (list, tuple)) + or len(declared) != 2 + or any( + isinstance(value, bool) or not isinstance(value, int) for value in declared + ) + or int(declared[0]) <= 0 + or int(declared[1]) <= 0 + ): + raise ValueError( + f"events.jsonl event {event_index} {key} must be two positive integers" + ) + if actual is None: + raise ValueError( + f"events.jsonl event {event_index} declares {key} without its PNG" + ) + normalized = (int(declared[0]), int(declared[1])) + if normalized != actual: + raise ValueError( + f"events.jsonl event {event_index} {key} {normalized} does not match " + f"the retained PNG {actual}" + ) + return actual + + +def _validate_point_in_viewport( + point: Point, + viewport: tuple[int, int], + *, + event_index: int, + label: str, +) -> None: + """Reject pointer evidence outside its declared coordinate space.""" + + if not (0 <= point[0] < viewport[0] and 0 <= point[1] < viewport[1]): + raise ValueError( + f"events.jsonl event {event_index} {label} {point} is outside " + f"viewport {viewport}" + ) + + def _clamped_crop_region( click: Point, frame_w: int, @@ -1715,6 +1787,18 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: ) before_png = _read_png(before_path) after_png = _read_png(after_path) + before_viewport = _validated_event_viewport( + event, + key="viewport_before", + png=before_png, + event_index=i, + ) + _validated_event_viewport( + event, + key="viewport_after", + png=after_png, + event_index=i, + ) if kind in ("click", "double_click", "right_click", "drag"): if before_png is None: @@ -1722,6 +1806,13 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: f"missing before frame for {kind} event {i} in {recording}" ) click: Point = (int(event["x"]), int(event["y"])) + assert before_viewport is not None + _validate_point_in_viewport( + click, + before_viewport, + event_index=i, + label="pointer", + ) # ``before_png`` is a captured frame we already hold; the decode is # known-valid. cv2's stub types imdecode as Optional, hence the cast. frame = cast( @@ -1874,6 +1965,12 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: drag_end_anchor: Optional[Anchor] = None if kind == "drag": drag_end: Point = (int(event["end_x"]), int(event["end_y"])) + _validate_point_in_viewport( + drag_end, + before_viewport, + event_index=i, + label="drag destination", + ) end_crop_region = _discriminative_crop_region(frame, drag_end) end_template_rel = f"templates/{step_id}_drag_end.png" (bundle / end_template_rel).write_bytes( diff --git a/openadapt_flow/interactive_recorder.py b/openadapt_flow/interactive_recorder.py index 4179f02b..d690c3f6 100644 --- a/openadapt_flow/interactive_recorder.py +++ b/openadapt_flow/interactive_recorder.py @@ -1,11 +1,13 @@ """Interactive recorder: capture a demonstration the USER drives live. ``openadapt-flow record --url `` opens a real (headed) Playwright browser -pointed at the user's OWN app and simply watches: it listens to the user's -real clicks, typing, key presses and scrolls via in-page capture-phase DOM -listeners (the same technique ``playwright codegen`` uses) and writes the -EXACT recording format the compiler already consumes (``meta.json`` + -``events.jsonl`` + ``frames/{i:04d}_before.png`` / ``_after.png``). +pointed at the user's OWN app. With ``--browser-cdp-endpoint`` it instead +attaches to one explicitly bound tab in an already-running local Chromium +browser, which preserves an existing SSO or 2FA session. Both modes listen to +the user's real clicks, typing, key presses and scrolls via in-page +capture-phase DOM listeners and write the EXACT recording format the compiler +already consumes (``meta.json`` + ``events.jsonl`` + +``frames/{i:04d}_before.png`` / ``_after.png``). record --url … → compile → replay @@ -38,9 +40,15 @@ from __future__ import annotations +import io +import ipaddress import json +import uuid from pathlib import Path from typing import Any, Callable, Optional +from urllib.parse import urlsplit + +from PIL import Image from openadapt_flow.backends.playwright_backend import PlaywrightBackend from openadapt_flow.recorder import Recorder @@ -62,16 +70,192 @@ "End", ) + +class BrowserAttachError(RuntimeError): + """A safe browser-attachment precondition was not met.""" + + +def _http_origin(url: str, *, label: str) -> tuple[str, str, int]: + """Return a normalized HTTP origin or refuse an unsafe attach target.""" + + try: + parsed = urlsplit(url) + port = parsed.port + except ValueError as exc: + raise BrowserAttachError(f"{label} is not a valid URL") from exc + scheme = parsed.scheme.lower() + host = (parsed.hostname or "").lower() + if scheme not in {"http", "https"} or not host: + raise BrowserAttachError(f"{label} must be an http:// or https:// URL") + return (scheme, host, port or (443 if scheme == "https" else 80)) + + +def _origin_label(origin: tuple[str, str, int]) -> str: + scheme, host, port = origin + default_port = 443 if scheme == "https" else 80 + rendered_host = f"[{host}]" if ":" in host else host + suffix = "" if port == default_port else f":{port}" + return f"{scheme}://{rendered_host}{suffix}" + + +def _safe_page_label(url: str) -> str: + """Describe a tab without exposing URL credentials, query, or fragment.""" + + try: + parsed = urlsplit(url) + origin = _http_origin(url, label="browser tab URL") + except BrowserAttachError: + return "" + path = parsed.path or "/" + if len(path) > 120: + path = path[:117] + "..." + return _origin_label(origin) + path + + +def validate_browser_cdp_endpoint(endpoint: str) -> str: + """Require an explicit loopback CDP endpoint. + + Browser attachment is local-only. A remote CDP endpoint is effectively a + remote-control credential and can also expose every page in that browser. + The supported recorder does not accept that boundary implicitly. + """ + + try: + parsed = urlsplit(endpoint) + port = parsed.port + except ValueError as exc: + raise BrowserAttachError("the browser CDP endpoint is not a valid URL") from exc + if parsed.scheme.lower() not in {"http", "https", "ws", "wss"}: + raise BrowserAttachError( + "the browser CDP endpoint must use http, https, ws, or wss" + ) + if parsed.username or parsed.password or parsed.query or parsed.fragment: + raise BrowserAttachError( + "the browser CDP endpoint must not contain credentials, a query, " + "or a fragment" + ) + host = (parsed.hostname or "").lower() + is_loopback = host == "localhost" + if not is_loopback: + try: + is_loopback = ipaddress.ip_address(host).is_loopback + except ValueError: + is_loopback = False + if not is_loopback: + raise BrowserAttachError( + "the browser CDP endpoint must use localhost or a loopback IP address" + ) + if port is None: + raise BrowserAttachError("the browser CDP endpoint must include a port") + return endpoint + + +def select_attached_page( + browser: Any, + *, + app_url: str, + page_url: Optional[str] = None, +) -> Any: + """Bind one existing same-origin web tab without guessing. + + A sole tab on the declared application origin is unambiguous. If two or + more tabs use that origin, the operator must give the exact current URL. + Query and fragment values are never included in an error message. + """ + + app_origin = _http_origin(app_url, label="the declared app URL") + if ( + page_url is not None + and _http_origin(page_url, label="the selected browser page URL") != app_origin + ): + raise BrowserAttachError( + "the selected browser page URL must have the same origin as " + f"the declared app URL ({_origin_label(app_origin)})" + ) + + matches: list[tuple[Any, str]] = [] + for context in browser.contexts: + for page in context.pages: + try: + current_url = str(page.url) + current_origin = _http_origin(current_url, label="browser tab URL") + except Exception: + continue + if current_origin == app_origin: + matches.append((page, current_url)) + + if page_url is not None: + exact = [page for page, current_url in matches if current_url == page_url] + if len(exact) == 1: + return exact[0] + if not exact: + raise BrowserAttachError( + "no open tab has the exact --browser-page-url on the declared " + f"app origin ({_origin_label(app_origin)})" + ) + raise BrowserAttachError( + "more than one open tab has the exact --browser-page-url; close " + "the duplicate tabs and retry" + ) + + if len(matches) == 1: + return matches[0][0] + if not matches: + raise BrowserAttachError( + "no open tab matches the declared app origin " + f"({_origin_label(app_origin)}); open the app in the attached " + "browser and retry" + ) + labels = sorted({_safe_page_label(current_url) for _, current_url in matches}) + rendered = ", ".join(labels[:5]) + if len(labels) > 5: + rendered += f", and {len(labels) - 5} more" + raise BrowserAttachError( + f"{len(matches)} open tabs match the declared app origin; supply the " + "exact current URL with --browser-page-url. Candidate paths " + f"(query and fragment hidden): {rendered}" + ) + + # In-page recorder script. Installed via add_init_script so it re-arms on every # document (navigations). Emits raw events to the Python side via the # __oaflow_emit binding. __SECRET_NAMES__ / __SPECIAL_KEYS__ are substituted in. _INIT_JS = r""" (() => { - if (window.__oaflowInstalled) return; - window.__oaflowInstalled = true; + const SESSION_ID = __SESSION_ID__; + const BINDING_NAME = __BINDING_NAME__; + const GLOBAL_KEY = '__oaflowRecorder'; + const previous = window[GLOBAL_KEY]; + if (previous && previous.sessionId === SESSION_ID) return; + if (previous && typeof previous.cleanup === 'function') { + try { previous.cleanup(); } catch (e) {} + } const SECRET_NAMES = __SECRET_NAMES__; const IDENT_NAMES = __IDENT_NAMES__; const SPECIAL = __SPECIAL_KEYS__; + const listeners = []; + let resizeTimer = null; + + function listenOn(target, type, handler) { + target.addEventListener(type, handler, true); + listeners.push([target, type, handler]); + } + + function listen(type, handler) { + listenOn(document, type, handler); + } + + function cleanup() { + if (resizeTimer !== null) clearTimeout(resizeTimer); + for (const [target, type, handler] of listeners) { + try { target.removeEventListener(type, handler, true); } catch (e) {} + } + const current = window[GLOBAL_KEY]; + if (current && current.sessionId === SESSION_ID) { + try { delete window[GLOBAL_KEY]; } catch (e) { window[GLOBAL_KEY] = null; } + } + } + window[GLOBAL_KEY] = {sessionId: SESSION_ID, cleanup}; function identifierRect() { // Bounding rect of the operator-marked record-identifying field @@ -256,11 +440,28 @@ return null; } - function emit(o) { try { window.__oaflow_emit(o); } catch (e) {} } + function emit(o) { + try { + o.__oaflow_session = SESSION_ID; + o.__oaflow_top_level = window === window.top; + o.__oaflow_viewport = [Math.round(window.innerWidth), + Math.round(window.innerHeight)]; + o.__oaflow_dpr = Number(window.devicePixelRatio || 1); + const binding = window[BINDING_NAME]; + if (typeof binding === 'function') binding(o); + } catch (e) {} + } let pointerDown = null; let suppressClick = false; - document.addEventListener('pointerdown', (e) => { + listenOn(window, 'resize', () => { + if (resizeTimer !== null) clearTimeout(resizeTimer); + resizeTimer = setTimeout(() => { + resizeTimer = null; + emit({kind: 'viewport', url: location.href, title: document.title}); + }, 100); + }); + listen('pointerdown', (e) => { if (e.button !== 0) return; pointerDown = { x: Math.round(e.clientX), y: Math.round(e.clientY), @@ -268,9 +469,9 @@ structural: structuralTarget(e.clientX, e.clientY), idr: identifierRect(), }; - }, true); + }); - document.addEventListener('pointerup', (e) => { + listen('pointerup', (e) => { if (e.button !== 0 || !pointerDown) return; const start = pointerDown; pointerDown = null; @@ -284,9 +485,9 @@ end_structural: structuralTarget(endX, endY), idr: start.idr, url: location.href, title: document.title, }); - }, true); + }); - document.addEventListener('click', (e) => { + listen('click', (e) => { if (e.button !== 0) return; if (suppressClick) { suppressClick = false; return; } emit({ @@ -297,9 +498,9 @@ idr: identifierRect(), url: location.href, title: document.title, }); - }, true); + }); - document.addEventListener('contextmenu', (e) => { + listen('contextmenu', (e) => { emit({ kind: 'right_click', x: Math.round(e.clientX), y: Math.round(e.clientY), @@ -308,9 +509,9 @@ idr: identifierRect(), url: location.href, title: document.title, }); - }, true); + }); - document.addEventListener('input', (e) => { + listen('input', (e) => { const el = e.target; const secret = isSecretEl(el); const r = (el.getBoundingClientRect && el.getBoundingClientRect()) @@ -327,9 +528,9 @@ // The literal value of a SECRET field is never read or transmitted. if (!secret) o.value = (el.value != null ? String(el.value) : ''); emit(o); - }, true); + }); - document.addEventListener('keydown', (e) => { + listen('keydown', (e) => { const modifiers = []; if (e.ctrlKey) modifiers.push('ctrl'); if (e.altKey) modifiers.push('alt'); @@ -347,15 +548,15 @@ } if (SPECIAL.indexOf(e.key) < 0) return; emit({ kind: 'key', key: e.key, url: location.href, title: document.title }); - }, true); + }); - document.addEventListener('wheel', (e) => { + listen('wheel', (e) => { emit({ kind: 'scroll', dx: Math.round(e.deltaX), dy: Math.round(e.deltaY), url: location.href, title: document.title, }); - }, true); + }); })(); """ @@ -377,6 +578,8 @@ def __init__( param_fields: tuple[str, ...] = (), identifier_fields: tuple[str, ...] = (), headless: bool = False, + cdp_endpoint: Optional[str] = None, + browser_page_url: Optional[str] = None, poll_ms: int = 60, settle_timeout_s: float = 5.0, settle_stable_frames: int = 2, @@ -393,6 +596,24 @@ def __init__( self._param_fields = set(param_fields) self._identifier_fields = set(identifier_fields) self._headless = headless + if browser_page_url and not cdp_endpoint: + raise BrowserAttachError("browser_page_url requires a browser CDP endpoint") + if cdp_endpoint and headless: + raise BrowserAttachError( + "headless mode cannot be combined with an attached browser" + ) + self._cdp_endpoint = ( + validate_browser_cdp_endpoint(cdp_endpoint) if cdp_endpoint else None + ) + self._attached_origin = ( + _http_origin(url, label="the declared app URL") + if self._cdp_endpoint + else None + ) + self._browser_page_url = browser_page_url + self._owns_browser = self._cdp_endpoint is None + self._session_id = uuid.uuid4().hex + self._binding_name = f"__oaflow_emit_{self._session_id}" self._poll_ms = poll_ms self._viewport = viewport # Recording-only, read-only observation. This does not add an effect @@ -409,6 +630,7 @@ def __init__( self._pyq: list[dict[str, Any]] = [] self._pending_type: Optional[dict[str, Any]] = None self._pending_scroll: Optional[dict[str, Any]] = None + self._listener_error: Optional[BrowserAttachError] = None self.done = False # Set on start(). @@ -419,61 +641,123 @@ def __init__( self.recorder: Optional[Recorder] = None self._last_frame: bytes = b"" self._last_structural: dict[str, Any] = {} + self._attached_geometry: Optional[tuple[int, int, float]] = None + self._initial_attached_viewport: Optional[tuple[int, int]] = None + self._viewport_dirty = False + self._viewport_history: list[dict[str, Any]] = [] # -- lifecycle ----------------------------------------------------------- def start(self) -> None: - """Launch the browser, install the in-page listeners, capture the - initial settled frame.""" - from openadapt_flow._browser_setup import ensure_chromium_installed + """Launch or attach, install listeners, and capture the first frame.""" + + if self._owns_browser: + from openadapt_flow._browser_setup import ensure_chromium_installed - ensure_chromium_installed() + ensure_chromium_installed() from playwright.sync_api import sync_playwright self._pw = sync_playwright().start() try: - self._browser = self._pw.chromium.launch(headless=self._headless) - except Exception: - self._pw.stop() - raise - self.page = self._browser.new_page( - viewport={"width": self._viewport[0], "height": self._viewport[1]}, - device_scale_factor=1, - ) - self.page.on("close", lambda _=None: setattr(self, "done", True)) - self.page.expose_binding( - "__oaflow_emit", - lambda source, detail: self._pyq.append(detail), - ) - init_js = ( - _INIT_JS.replace( - "__SECRET_NAMES__", json.dumps(sorted(self._secret_fields)) + if self._owns_browser: + self._browser = self._pw.chromium.launch(headless=self._headless) + self.page = self._browser.new_page( + viewport={ + "width": self._viewport[0], + "height": self._viewport[1], + }, + device_scale_factor=1, + ) + else: + try: + self._browser = self._pw.chromium.connect_over_cdp( + self._cdp_endpoint + ) + except Exception as exc: + raise BrowserAttachError( + "could not connect to the local Chromium CDP endpoint; " + "confirm that the browser was started with remote " + "debugging and that the endpoint is ready" + ) from exc + self.page = select_attached_page( + self._browser, + app_url=self._url, + page_url=self._browser_page_url, + ) + + self.page.on("close", lambda _=None: setattr(self, "done", True)) + self.page.expose_binding( + self._binding_name, + lambda source, detail: self._enqueue_browser_event( + detail, + source=source, + ), ) - .replace("__IDENT_NAMES__", json.dumps(sorted(self._identifier_fields))) - .replace("__SPECIAL_KEYS__", json.dumps(list(_SPECIAL_KEYS))) - ) - self.page.add_init_script(init_js) - self.page.goto(self._url) - try: - self.page.wait_for_load_state("load") + init_js = ( + _INIT_JS.replace("__SESSION_ID__", json.dumps(self._session_id)) + .replace("__BINDING_NAME__", json.dumps(self._binding_name)) + .replace("__SECRET_NAMES__", json.dumps(sorted(self._secret_fields))) + .replace( + "__IDENT_NAMES__", + json.dumps(sorted(self._identifier_fields)), + ) + .replace("__SPECIAL_KEYS__", json.dumps(list(_SPECIAL_KEYS))) + ) + self.page.add_init_script(init_js) + if self._owns_browser: + self.page.goto(self._url) + try: + self.page.wait_for_load_state("load") + except Exception: + pass + else: + # add_init_script applies after the next navigation. Install + # the same session in every already-open document now. This + # includes child frames, whose local coordinates cannot yet be + # bound to top-level evidence and therefore emit an explicit + # refusal instead of disappearing from the recording. + for frame in list(self.page.frames): + try: + frame.evaluate(init_js) + except Exception as exc: + try: + detached = frame.is_detached() + except Exception: + detached = False + if detached: + continue + raise BrowserAttachError( + "could not install the recording listener in every " + "existing browser frame; recording was refused" + ) from exc + + self.backend = PlaywrightBackend( + self.page, + screenshot_scale="device" if self._owns_browser else "css", + ) + self.recorder = Recorder( + self.backend, + self._out_dir, + app_url=self._url, + system_of_record_reader=self._system_of_record_reader, + **self._settle, + ) + if self._owns_browser: + self._last_frame = self.recorder._wait_settled() + self._last_structural = self._structural_state() + else: + self._rebaseline_attached_viewport() except Exception: - pass - self.backend = PlaywrightBackend(self.page) - self.recorder = Recorder( - self.backend, - self._out_dir, - app_url=self._url, - system_of_record_reader=self._system_of_record_reader, - **self._settle, - ) - self._last_frame = self.recorder._wait_settled() - self._last_structural = self._structural_state() + self._stop_browser_connection() + raise def run(self) -> Path: """Pump until completion, an operator stop, or a closed window.""" if self._stop_when is None: finish_instruction = ( - "Press Ctrl-C here (or close the browser window) to finish." + "Press Ctrl-C here (or close the selected browser tab) to finish." + if not self._owns_browser + else "Press Ctrl-C here (or close the browser window) to finish." ) else: finish_instruction = ( @@ -482,8 +766,12 @@ def run(self) -> Path: ) print( f"Recording {self._url}\n" - " Perform your workflow in the browser window.\n" - f" {finish_instruction}" + + ( + " Perform your workflow in the selected existing browser tab.\n" + if not self._owns_browser + else " Perform your workflow in the browser window.\n" + ) + + f" {finish_instruction}" ) try: while not self.done: @@ -491,31 +779,239 @@ def run(self) -> Path: break except KeyboardInterrupt: print("\n[record] stopping…") + except Exception: + self.abort() + raise return self.finish() def run_script(self, script: Callable[[Any, Callable[[], None]], None]) -> Path: """Scripted loop (tests): run ``script(page, pump)`` — which performs synthetic input and calls ``pump()`` to let the recorder drain — then flush and finish.""" - script(self.page, self.pump) + try: + script(self.page, self.pump) + except Exception: + self.abort() + raise return self.finish() def finish(self) -> Path: - """Flush trailing input, write meta.json, tear the browser down.""" + """Flush input, write metadata, and close or detach as appropriate.""" try: + self._cleanup_page_listeners() + self._drain_event_queue() self._flush_type() self._flush_scroll() - finally: - assert self.recorder is not None + except Exception: + self.abort() + raise + assert self.recorder is not None + try: out = self.recorder.finish() - try: - if self._browser is not None: - self._browser.close() - finally: - if self._pw is not None: - self._pw.stop() + meta_path = out / "meta.json" + meta = json.loads(meta_path.read_text()) + meta["source"] = ( + "openadapt-flow-playwright" + if self._owns_browser + else "openadapt-flow-playwright-cdp" + ) + if not self._owns_browser: + assert self._initial_attached_viewport is not None + meta["viewport"] = list(self._initial_attached_viewport) + meta["viewport_mode"] = "per-event" + meta["viewport_history"] = list(self._viewport_history) + meta_path.write_text(json.dumps(meta, indent=2)) + finally: + self._stop_browser_connection() return out + def abort(self) -> None: + """Detach after a refused or failed recording without writing meta.json.""" + + self.done = True + self._cleanup_page_listeners() + self._pyq.clear() + self._stop_browser_connection() + + def _enqueue_browser_event( + self, + detail: Any, + *, + source: Optional[dict[str, Any]] = None, + ) -> None: + """Accept only a bounded event from this recorder session.""" + + if not isinstance(detail, dict): + return + event = dict(detail) + if event.pop("__oaflow_session", None) != self._session_id: + return + kind = event.get("kind") + reported_top_level = bool(event.pop("__oaflow_top_level", True)) + source_is_selected_top_level = reported_top_level + if source is not None: + try: + source_is_selected_top_level = ( + source.get("page") is self.page + and source.get("frame") is self.page.main_frame + ) + except Exception: + source_is_selected_top_level = False + if not source_is_selected_top_level and kind == "viewport": + return + if not source_is_selected_top_level: + self._listener_error = BrowserAttachError( + "an event came from an iframe; cross-frame recording is not " + "qualified, so recording stopped before accepting the event" + ) + self.done = True + return + if kind not in { + "click", + "right_click", + "drag", + "input", + "key", + "hotkey", + "scroll", + "viewport", + }: + return + if not self._owns_browser: + try: + event_origin = _http_origin( + str(event["url"]), + label="the browser event URL", + ) + except Exception: + event_origin = None + if event_origin != self._attached_origin: + self._listener_error = BrowserAttachError( + "a browser event came from outside the declared application " + "origin; recording stopped before accepting the event" + ) + self.done = True + return + raw_viewport = event.pop("__oaflow_viewport", None) + raw_dpr = event.pop("__oaflow_dpr", None) + try: + event_geometry = ( + int(raw_viewport[0]), + int(raw_viewport[1]), + round(float(raw_dpr), 6), + ) + except (IndexError, TypeError, ValueError): + event_geometry = (0, 0, 0.0) + if ( + event_geometry[0] <= 0 + or event_geometry[1] <= 0 + or not 0.1 <= event_geometry[2] <= 16.0 + ): + self._listener_error = BrowserAttachError( + "the browser emitted invalid viewport evidence; recording " + "stopped before accepting the event" + ) + self.done = True + return + event["_oaflow_geometry"] = event_geometry + if kind == "viewport": + self._viewport_dirty = True + return + else: + event.pop("__oaflow_viewport", None) + event.pop("__oaflow_dpr", None) + try: + encoded_size = len(json.dumps(event).encode("utf-8")) + except (TypeError, ValueError): + return + if encoded_size > 1_000_000: + self._listener_error = BrowserAttachError( + "the browser emitted an event larger than 1 MB; recording " + "stopped without accepting the event" + ) + self.done = True + return + self._pyq.append(event) + + def _cleanup_page_listeners(self) -> None: + """Remove this session's current-document listeners before detach.""" + + if self.page is None: + return + try: + frames = list(self.page.frames) + except Exception: + frames = [] + for frame in frames: + try: + frame.evaluate( + """sessionId => { + const current = window.__oaflowRecorder; + if (current && current.sessionId === sessionId + && typeof current.cleanup === 'function') { + current.cleanup(); + } + }""", + self._session_id, + ) + except Exception: + continue + + def _stop_browser_connection(self) -> None: + """Close an owned browser or detach without closing an external one.""" + + self._cleanup_page_listeners() + browser, self._browser = self._browser, None + playwright, self._pw = self._pw, None + try: + if self._owns_browser and browser is not None: + browser.close() + finally: + if playwright is not None: + playwright.stop() + + def _drain_event_queue(self) -> bool: + """Process all events already delivered by the page binding.""" + + if self._listener_error is not None: + raise self._listener_error + batch = self._pyq[:] + del self._pyq[:] + rebased = False + if not self._owns_browser: + current_geometry = self._read_attached_geometry() + if self._viewport_dirty or current_geometry != self._attached_geometry: + if batch: + raise BrowserAttachError( + "an action overlapped a browser resize or monitor-scale " + "change; recording stopped because no exact pre-action " + "frame exists in the new coordinate space" + ) + self._rebaseline_attached_viewport() + rebased = True + for event in batch: + if not self._owns_browser: + event_geometry = event.pop("_oaflow_geometry", None) + if event_geometry != self._attached_geometry: + raise BrowserAttachError( + "an action overlapped a browser resize or monitor-scale " + "change; recording stopped because no exact pre-action " + "frame exists in the new coordinate space" + ) + self._process(event) + if not self._owns_browser and ( + self._viewport_dirty + or self._read_attached_geometry() != self._attached_geometry + ): + raise BrowserAttachError( + "the browser resized or changed monitor scale while an " + "action was being retained; recording stopped without " + "complete metadata" + ) + if self._listener_error is not None: + raise self._listener_error + return bool(batch) or rebased + # -- event pump ---------------------------------------------------------- def pump(self) -> bool: @@ -524,6 +1020,8 @@ def pump(self) -> bool: return self._pump() def _pump(self) -> bool: + if self._listener_error is not None: + raise self._listener_error if self.done: return False try: @@ -531,17 +1029,13 @@ def _pump(self) -> bool: except Exception: self.done = True return False - batch = self._pyq[:] - del self._pyq[:] - if not batch: + if not self._drain_event_queue(): # Distinct scroll gestures are separated by pauses; flush a # completed scroll on idle so each becomes its own step. A type run # is NOT idle-flushed (a mid-word pause must not split it) — it # flushes on the next boundary event or at finish(). self._flush_scroll() return not self._stop_condition_reached() - for ev in batch: - self._process(ev) return not self._stop_condition_reached() def _process(self, ev: dict[str, Any]) -> None: @@ -724,6 +1218,102 @@ def _record_key(self, ev: dict[str, Any]) -> None: # -- internals ----------------------------------------------------------- + def _read_attached_geometry(self) -> tuple[int, int, float]: + """Read the selected tab's origin, CSS viewport, and monitor scale.""" + + assert not self._owns_browser + assert self.page is not None + try: + current_origin = _http_origin( + str(self.page.url), + label="the selected browser tab URL", + ) + except Exception as exc: + raise BrowserAttachError( + "the selected browser tab left the declared application origin; " + "recording was refused" + ) from exc + if current_origin != self._attached_origin: + raise BrowserAttachError( + "the selected browser tab left the declared application origin; " + "recording was refused" + ) + try: + raw = self.page.evaluate( + """() => ({ + width: window.innerWidth, + height: window.innerHeight, + dpr: window.devicePixelRatio || 1, + })""" + ) + geometry = ( + int(raw["width"]), + int(raw["height"]), + round(float(raw["dpr"]), 6), + ) + except Exception as exc: + raise BrowserAttachError( + "the attached tab geometry could not be read; recording was refused" + ) from exc + if geometry[0] <= 0 or geometry[1] <= 0 or not 0.1 <= geometry[2] <= 16.0: + raise BrowserAttachError( + "the attached tab reported invalid viewport or monitor-scale " + "geometry; recording was refused" + ) + return geometry + + def _rebaseline_attached_viewport(self) -> None: + """Resume after an idle resize with a fresh exact CSS-pixel baseline.""" + + assert not self._owns_browser + assert self.recorder is not None + # A deferred input or scroll already has its exact old-space after + # frame. Persist it before the new coordinate space becomes current. + self._flush_type() + self._flush_scroll() + for _attempt in range(3): + before = self._read_attached_geometry() + frame = self.recorder._wait_settled() + after = self._read_attached_geometry() + if self._pyq or self._listener_error is not None: + raise BrowserAttachError( + "an action occurred before the resized browser viewport was " + "rebound to a fresh frame; recording stopped without " + "complete metadata" + ) + with Image.open(io.BytesIO(frame)) as image: + frame_size = image.size + if before == after and frame_size == after[:2]: + self._attached_geometry = after + self._last_frame = frame + self._last_structural = self._structural_state() + self._viewport_dirty = False + viewport = after[:2] + if self._initial_attached_viewport is None: + self._initial_attached_viewport = viewport + entry = { + "before_event": self.recorder.event_count, + "viewport": list(viewport), + "device_scale_factor": after[2], + } + if ( + self._viewport_history + and self._viewport_history[-1]["before_event"] + == entry["before_event"] + ): + self._viewport_history[-1] = entry + elif not self._viewport_history or ( + self._viewport_history[-1]["viewport"] != entry["viewport"] + or self._viewport_history[-1]["device_scale_factor"] + != entry["device_scale_factor"] + ): + self._viewport_history.append(entry) + return + raise BrowserAttachError( + "the attached browser viewport did not settle long enough to bind " + "a new exact frame and coordinate space" + ) + def _advance(self) -> None: """After an IMMEDIATE step (click/key), the current settled frame becomes the next step's BEFORE frame.""" @@ -788,6 +1378,8 @@ def record_interactive( param_fields: tuple[str, ...] = (), identifier_fields: tuple[str, ...] = (), headless: bool = False, + cdp_endpoint: Optional[str] = None, + browser_page_url: Optional[str] = None, script: Optional[Callable[[Any, Callable[[], None]], None]] = None, system_of_record_reader: Optional[ Callable[[], Optional[list[dict[str, Any]]]] @@ -817,6 +1409,13 @@ def record_interactive( remote-display/pixel substrate (Citrix/RDP). headless: Run the browser headless (used by scripted/CI recording; a human recording is headed). + cdp_endpoint: Optional local-loopback Chromium DevTools endpoint. When + set, the recorder attaches to an existing browser and never + launches, navigates, or closes it. + browser_page_url: Exact current tab URL used to disambiguate two or + more open tabs on the declared app origin. Requires + ``cdp_endpoint``. Query and fragment values are not written to + recorder diagnostics. script: Test hook — ``script(page, pump)`` drives synthetic input and pumps the loop; when given, the human wait loop is skipped. system_of_record_reader: Optional read-only observation of the @@ -837,6 +1436,8 @@ def record_interactive( param_fields=param_fields, identifier_fields=identifier_fields, headless=headless, + cdp_endpoint=cdp_endpoint, + browser_page_url=browser_page_url, system_of_record_reader=system_of_record_reader, stop_when=stop_when, **kwargs, diff --git a/openadapt_flow/recorder.py b/openadapt_flow/recorder.py index 0c95b707..ca699f69 100644 --- a/openadapt_flow/recorder.py +++ b/openadapt_flow/recorder.py @@ -22,6 +22,8 @@ # structural observations (StructuralBackend), and # sor_before/sor_after (a system-of-record snapshot) # when a recorder-only observer is configured. + # Every frame-backed event also carries + # viewport_before/viewport_after from the actual PNGs. frames/{i:04d}_before.png frames/{i:04d}_after.png # captured after the action settled @@ -208,6 +210,12 @@ def finish(self) -> Path: (self._dir / "meta.json").write_text(json.dumps(meta, indent=2)) return self._dir + @property + def event_count(self) -> int: + """Number of complete events already persisted.""" + + return self._i + # -- internals ----------------------------------------------------------- def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None: @@ -346,7 +354,16 @@ def _commit( after_png = self._redact(after_png, redact_region) (self._frames_dir / f"{i:04d}_before.png").write_bytes(before_png) (self._frames_dir / f"{i:04d}_after.png").write_bytes(after_png) - line: dict[str, Any] = {"i": i, **event} + with Image.open(io.BytesIO(before_png)) as before_image: + before_viewport = [int(before_image.width), int(before_image.height)] + with Image.open(io.BytesIO(after_png)) as after_image: + after_viewport = [int(after_image.width), int(after_image.height)] + line: dict[str, Any] = { + "i": i, + **event, + "viewport_before": before_viewport, + "viewport_after": after_viewport, + } for key, value in structural_before.items(): line[f"{key}_before"] = value if structural_after is None: diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py new file mode 100644 index 00000000..411b1a52 --- /dev/null +++ b/tests/test_browser_attach.py @@ -0,0 +1,588 @@ +"""Safe attachment of the Playwright recorder to an existing Chromium tab.""" + +from __future__ import annotations + +import json +import os +import shutil +import subprocess +import threading +import time +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from pathlib import Path +from types import SimpleNamespace +from urllib.request import urlopen + +import pytest + +from openadapt_flow.__main__ import main +from openadapt_flow.backends.playwright_backend import PlaywrightBackend +from openadapt_flow.compiler import compile_recording +from openadapt_flow.interactive_recorder import ( + BrowserAttachError, + InteractiveRecorder, + record_interactive, + select_attached_page, + validate_browser_cdp_endpoint, +) + + +class _Page: + def __init__(self, url: str) -> None: + self.url = url + + +def _browser(*urls: str): + return SimpleNamespace( + contexts=[SimpleNamespace(pages=[_Page(url) for url in urls])] + ) + + +@pytest.mark.parametrize( + "endpoint", + [ + "http://localhost:9222", + "http://127.0.0.1:9222", + "http://127.9.8.7:9222", + "ws://[::1]:9222/devtools/browser/abc", + ], +) +def test_cdp_endpoint_accepts_only_explicit_loopback(endpoint: str) -> None: + assert validate_browser_cdp_endpoint(endpoint) == endpoint + + +@pytest.mark.parametrize( + ("endpoint", "message"), + [ + ("http://example.com:9222", "loopback"), + ("http://localhost", "include a port"), + ("ftp://localhost:9222", "must use http"), + ("http://user:pass@localhost:9222", "must not contain credentials"), + ("http://localhost:9222?token=secret", "must not contain credentials"), + ], +) +def test_cdp_endpoint_refuses_unsafe_boundaries(endpoint: str, message: str) -> None: + with pytest.raises(BrowserAttachError, match=message): + validate_browser_cdp_endpoint(endpoint) + + +def test_attach_selects_the_only_tab_on_the_declared_origin() -> None: + wanted = _Page("https://app.example.test/work/12?record=private") + browser = SimpleNamespace( + contexts=[ + SimpleNamespace( + pages=[ + _Page("chrome://settings/"), + _Page("https://unrelated.example.test/"), + wanted, + ] + ) + ] + ) + assert select_attached_page(browser, app_url="https://app.example.test/") is wanted + + +def test_attach_requires_exact_url_when_origin_has_multiple_tabs() -> None: + browser = _browser( + "https://app.example.test/one?patient=SECRET_ONE#private", + "https://app.example.test/two?patient=SECRET_TWO", + ) + with pytest.raises(BrowserAttachError) as caught: + select_attached_page(browser, app_url="https://app.example.test/") + message = str(caught.value) + assert "--browser-page-url" in message + assert "/one" in message and "/two" in message + assert "SECRET_ONE" not in message and "SECRET_TWO" not in message + + +def test_attach_exact_url_selects_one_tab_without_navigation() -> None: + selected = "https://app.example.test/two?record=42" + browser = _browser("https://app.example.test/one", selected) + page = select_attached_page( + browser, + app_url="https://app.example.test/", + page_url=selected, + ) + assert page.url == selected + + +def test_attach_refuses_cross_origin_page_selector_without_echoing_it() -> None: + private_url = "https://other.example.test/path?token=DO_NOT_PRINT" + with pytest.raises(BrowserAttachError) as caught: + select_attached_page( + _browser("https://app.example.test/"), + app_url="https://app.example.test/", + page_url=private_url, + ) + assert "same origin" in str(caught.value) + assert private_url not in str(caught.value) + assert "DO_NOT_PRINT" not in str(caught.value) + + +def test_attached_backend_uses_live_css_viewport_and_css_screenshot() -> None: + class Page: + viewport_size = None + + def __init__(self) -> None: + self.screenshot_options = None + + def evaluate(self, _script): + return {"width": 1440, "height": 900} + + def screenshot(self, **kwargs): + self.screenshot_options = kwargs + return b"png" + + page = Page() + backend = PlaywrightBackend(page, screenshot_scale="css") # type: ignore[arg-type] + assert backend.viewport == (1440, 900) + assert backend.screenshot() == b"png" + assert page.screenshot_options["scale"] == "css" + + +def test_attached_recorder_api_refuses_incompatible_options(tmp_path: Path) -> None: + with pytest.raises(BrowserAttachError, match="requires a browser CDP"): + InteractiveRecorder( + "https://app.example.test/", + tmp_path / "recording", + browser_page_url="https://app.example.test/work", + ) + with pytest.raises(BrowserAttachError, match="headless"): + InteractiveRecorder( + "https://app.example.test/", + tmp_path / "recording", + cdp_endpoint="http://127.0.0.1:9222", + headless=True, + ) + + +def test_attached_recorder_reads_geometry_and_refuses_origin_drift( + tmp_path: Path, +) -> None: + session = InteractiveRecorder( + "https://app.example.test/", + tmp_path / "recording", + cdp_endpoint="http://127.0.0.1:9222", + ) + session.page = SimpleNamespace( + url="https://app.example.test/work", + evaluate=lambda _script: {"width": 1280, "height": 800, "dpr": 2}, + ) + assert session._read_attached_geometry() == (1280, 800, 2.0) + + session.page.url = "https://other.example.test/?token=DO_NOT_PRINT" + with pytest.raises(BrowserAttachError) as caught: + session._read_attached_geometry() + assert "left the declared application origin" in str(caught.value) + assert "DO_NOT_PRINT" not in str(caught.value) + + +def test_attached_recorder_refuses_iframe_events(tmp_path: Path) -> None: + session = InteractiveRecorder( + "https://app.example.test/", + tmp_path / "recording", + cdp_endpoint="http://127.0.0.1:9222", + ) + session._enqueue_browser_event( + { + "__oaflow_session": session._session_id, + "__oaflow_top_level": False, + "kind": "click", + "x": 10, + "y": 20, + } + ) + assert session.done is True + assert session._pyq == [] + assert session._listener_error is not None + assert "iframe" in str(session._listener_error) + + session.done = False + session._listener_error = None + selected_frame = object() + session.page = SimpleNamespace(main_frame=selected_frame) + session._enqueue_browser_event( + { + "__oaflow_session": session._session_id, + "__oaflow_top_level": True, + "kind": "click", + "x": 10, + "y": 20, + }, + source={"page": session.page, "frame": object()}, + ) + assert session.done is True + assert session._listener_error is not None + assert "iframe" in str(session._listener_error) + + +def test_attached_recorder_refuses_invalid_viewport_evidence(tmp_path: Path) -> None: + session = InteractiveRecorder( + "https://app.example.test/", + tmp_path / "recording", + cdp_endpoint="http://127.0.0.1:9222", + ) + selected_frame = object() + session.page = SimpleNamespace(main_frame=selected_frame) + session._enqueue_browser_event( + { + "__oaflow_session": session._session_id, + "__oaflow_top_level": True, + "__oaflow_viewport": [0, 800], + "__oaflow_dpr": 2, + "kind": "click", + "url": "https://app.example.test/work", + "x": 10, + "y": 20, + }, + source={"page": session.page, "frame": selected_frame}, + ) + assert session.done is True + assert session._pyq == [] + assert session._listener_error is not None + assert "invalid viewport evidence" in str(session._listener_error) + + +def test_cli_threads_attach_contract_to_the_recorder( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + captured: dict = {} + + def fake_record(url, out_dir, **kwargs): + captured["url"] = url + captured.update(kwargs) + out_dir.mkdir(parents=True) + (out_dir / "meta.json").write_text(json.dumps({"source": "fake"})) + return out_dir + + monkeypatch.setattr( + "openadapt_flow.interactive_recorder.record_interactive", fake_record + ) + selected = "https://app.example.test/work?record=42" + rc = main( + [ + "record", + "--backend", + "web", + "--url", + "https://app.example.test/", + "--browser-cdp-endpoint", + "http://127.0.0.1:9222", + "--browser-page-url", + selected, + "--out", + str(tmp_path / "recording"), + ] + ) + assert rc == 0 + assert captured["url"] == "https://app.example.test/" + assert captured["cdp_endpoint"] == "http://127.0.0.1:9222" + assert captured["browser_page_url"] == selected + + +def test_cli_refuses_attach_flags_on_the_wrong_surface(tmp_path: Path) -> None: + with pytest.raises(SystemExit, match="apply only to --backend web"): + main( + [ + "record", + "--backend", + "windows", + "--browser-cdp-endpoint", + "http://127.0.0.1:9222", + "--out", + str(tmp_path / "recording"), + ] + ) + + +def test_cli_refuses_page_selector_without_endpoint(tmp_path: Path) -> None: + with pytest.raises(SystemExit, match="requires --browser-cdp-endpoint"): + main( + [ + "record", + "--backend", + "web", + "--url", + "https://app.example.test/", + "--browser-page-url", + "https://app.example.test/work", + "--out", + str(tmp_path / "recording"), + ] + ) + + +def test_cli_refuses_headless_attachment(tmp_path: Path) -> None: + with pytest.raises(SystemExit, match="--headless cannot be combined"): + main( + [ + "record", + "--backend", + "web", + "--url", + "https://app.example.test/", + "--browser-cdp-endpoint", + "http://127.0.0.1:9222", + "--headless", + "--out", + str(tmp_path / "recording"), + ] + ) + + +_ATTACH_HTML = b""" +Attach recorder test + + + + + + +""" + + +@pytest.fixture(scope="module") +def attach_app_url() -> str: + class Handler(BaseHTTPRequestHandler): + def do_GET(self): # noqa: N802 - stdlib callback name + self.send_response(200) + self.send_header("Content-Type", "text/html; charset=utf-8") + self.send_header("Content-Length", str(len(_ATTACH_HTML))) + self.end_headers() + self.wfile.write(_ATTACH_HTML) + + def log_message(self, _format, *args): + return + + server = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield f"http://127.0.0.1:{server.server_address[1]}/" + finally: + server.shutdown() + server.server_close() + thread.join(timeout=5) + + +def _chromium_executable() -> Path | None: + configured = os.environ.get("OPENADAPT_TEST_CHROMIUM_EXECUTABLE") + candidates = [Path(configured)] if configured else [] + try: + from playwright.sync_api import sync_playwright + + with sync_playwright() as playwright: + candidates.append(Path(playwright.chromium.executable_path)) + except Exception: + pass + for command in ("google-chrome", "google-chrome-stable", "chromium"): + found = shutil.which(command) + if found: + candidates.append(Path(found)) + candidates.append( + Path("/Applications/Google Chrome.app/Contents/MacOS/Google Chrome") + ) + return next((candidate for candidate in candidates if candidate.is_file()), None) + + +@pytest.mark.timeout(180) +def test_live_cdp_attach_records_compiles_and_leaves_browser_running_three_trials( + attach_app_url: str, + tmp_path: Path, +) -> None: + """Three real Chromium trials cover attach, secrets, frames, and detach.""" + + executable = _chromium_executable() + if executable is None: + pytest.skip("no Chromium executable is installed") + profile = tmp_path / "chrome-profile" + profile.mkdir() + process = subprocess.Popen( + [ + str(executable), + "--headless=new", + "--no-sandbox", + "--no-first-run", + "--no-default-browser-check", + "--disable-background-networking", + "--remote-debugging-address=127.0.0.1", + "--remote-debugging-port=0", + f"--user-data-dir={profile}", + "--window-size=1280,800", + attach_app_url, + ], + stdout=subprocess.DEVNULL, + stderr=subprocess.DEVNULL, + ) + try: + port_file = profile / "DevToolsActivePort" + deadline = time.monotonic() + 20 + while not port_file.is_file() and time.monotonic() < deadline: + if process.poll() is not None: + pytest.fail("Chromium exited before its CDP endpoint was ready") + time.sleep(0.05) + assert port_file.is_file(), "Chromium CDP endpoint did not become ready" + port = int(port_file.read_text().splitlines()[0]) + endpoint = f"http://127.0.0.1:{port}" + + for trial in range(3): + secret = f"ATTACH-SECRET-{trial}-NEVER-PERSIST" + + def drive(page, pump, *, secret_value=secret, trial_number=trial): + page.evaluate( + """() => { + document.querySelector('#note').value = ''; + document.querySelector('#password').value = ''; + delete document.body.dataset.saved; + }""" + ) + page.click("#note") + page.keyboard.type(f"trial-{trial_number}") + pump() + pump() + page.click("#password") + page.keyboard.type(secret_value) + pump() + pump() + page.click("#save") + pump() + pump() + assert page.get_attribute("body", "data-saved") == "yes" + + recording = record_interactive( + attach_app_url, + tmp_path / f"recording-{trial}", + param_fields=("note",), + cdp_endpoint=endpoint, + script=drive, + ) + meta = json.loads((recording / "meta.json").read_text()) + events_text = (recording / "events.jsonl").read_text() + assert meta["source"] == "openadapt-flow-playwright-cdp" + assert meta["secret_params"] == ["password"] + assert secret not in json.dumps(meta) + assert secret not in events_text + assert meta["viewport"][0] > 0 and meta["viewport"][1] > 0 + + bundle = tmp_path / f"bundle-{trial}" + workflow = compile_recording( + recording, + bundle, + name=f"attached-browser-{trial}", + ) + assert workflow.steps + assert workflow.secret_params == ["password"] + + # Finishing a recording detaches Playwright. It does not close the + # operator's external browser or its selected tab. + assert process.poll() is None + with urlopen(f"{endpoint}/json/version", timeout=2) as response: + assert response.status == 200 + + iframe_recording = tmp_path / "recording-iframe-refusal" + + def click_inside_existing_iframe(page, pump): + page.frame_locator("#child").locator("#inside").click() + pump() + + with pytest.raises(BrowserAttachError, match="iframe"): + record_interactive( + attach_app_url, + iframe_recording, + cdp_endpoint=endpoint, + script=click_inside_existing_iframe, + ) + assert not (iframe_recording / "meta.json").exists() + assert process.poll() is None + + overlap_recording = tmp_path / "recording-resize-overlap-refusal" + + def resize_during_action(page, pump): + cdp = page.context.new_cdp_session(page) + cdp.send( + "Emulation.setDeviceMetricsOverride", + { + "width": 1000, + "height": 650, + "deviceScaleFactor": 2, + "mobile": False, + }, + ) + page.click("#note") + pump() + + with pytest.raises(BrowserAttachError, match="overlapped"): + record_interactive( + attach_app_url, + overlap_recording, + cdp_endpoint=endpoint, + script=resize_during_action, + ) + assert not (overlap_recording / "meta.json").exists() + assert process.poll() is None + + resized_recording = tmp_path / "recording-resized" + + def resize_then_record(page, pump): + page.evaluate("document.querySelector('#note').value = ''") + page.click("#note") + pump() + page.keyboard.type("before-resize") + pump() + pump() + cdp = page.context.new_cdp_session(page) + cdp.send( + "Emulation.setDeviceMetricsOverride", + { + "width": 900, + "height": 600, + "deviceScaleFactor": 1, + "mobile": False, + }, + ) + pump() + page.click("#note") + pump() + page.keyboard.type("after-resize") + pump() + pump() + + recording = record_interactive( + attach_app_url, + resized_recording, + cdp_endpoint=endpoint, + script=resize_then_record, + ) + meta = json.loads((recording / "meta.json").read_text()) + events = [ + json.loads(line) + for line in (recording / "events.jsonl").read_text().splitlines() + ] + assert meta["viewport_mode"] == "per-event" + assert meta["viewport_history"][-1]["viewport"] == [900, 600] + assert len(meta["viewport_history"]) >= 2 + assert events + assert {tuple(event["viewport_before"]) for event in events} == { + (1000, 650), + (900, 600), + } + assert all( + event["viewport_before"] == event["viewport_after"] for event in events + ) + resized_bundle = tmp_path / "bundle-resized" + resized_workflow = compile_recording( + recording, + resized_bundle, + name="attached-browser-resized", + ) + assert resized_workflow.steps + assert process.poll() is None + with urlopen(f"{endpoint}/json/version", timeout=2) as response: + assert response.status == 200 + finally: + process.terminate() + try: + process.wait(timeout=10) + except subprocess.TimeoutExpired: + process.kill() + process.wait(timeout=10) diff --git a/tests/test_compiler.py b/tests/test_compiler.py index 4e8d219c..ea82a88f 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -142,6 +142,133 @@ def compiled(tmp_path_factory: pytest.TempPathFactory): class TestCompileRecording: + def test_per_event_viewports_can_change_between_steps(self, tmp_path: Path) -> None: + recording = tmp_path / "recording" + (recording / "frames").mkdir(parents=True) + first = blank() + second = np.full((600, 900, 3), 245, dtype=np.uint8) + frames = {0: (first, first), 1: (second, second)} + for i, (before, after) in frames.items(): + write_frame(recording, i, "before", before) + write_frame(recording, i, "after", after) + events = [ + { + "i": 0, + "kind": "key", + "key": "Tab", + "t": 1.0, + "viewport_before": [1280, 800], + "viewport_after": [1280, 800], + }, + { + "i": 1, + "kind": "key", + "key": "Enter", + "t": 2.0, + "viewport_before": [900, 600], + "viewport_after": [900, 600], + }, + ] + (recording / "events.jsonl").write_text( + "\n".join(json.dumps(event) for event in events) + "\n" + ) + (recording / "meta.json").write_text( + json.dumps( + { + "id": "resized-recording", + "created_at": "2026-08-18T00:00:00+00:00", + "viewport": [1280, 800], + "viewport_mode": "per-event", + "viewport_history": [ + { + "before_event": 0, + "viewport": [1280, 800], + "device_scale_factor": 2, + }, + { + "before_event": 1, + "viewport": [900, 600], + "device_scale_factor": 1, + }, + ], + "params": {}, + } + ) + ) + + workflow = compile_recording( + recording, + tmp_path / "bundle", + name="resized-recording", + ) + assert len(workflow.steps) == 2 + assert workflow.viewport == VIEWPORT + + def test_per_event_viewport_must_match_retained_png(self, tmp_path: Path) -> None: + recording = tmp_path / "recording" + (recording / "frames").mkdir(parents=True) + write_frame(recording, 0, "before", blank()) + write_frame(recording, 0, "after", blank()) + (recording / "events.jsonl").write_text( + json.dumps( + { + "i": 0, + "kind": "key", + "key": "Enter", + "t": 1.0, + "viewport_before": [900, 600], + "viewport_after": [1280, 800], + } + ) + + "\n" + ) + (recording / "meta.json").write_text( + json.dumps( + { + "id": "mismatched-viewport", + "created_at": "2026-08-18T00:00:00+00:00", + "viewport": list(VIEWPORT), + "params": {}, + } + ) + ) + + with pytest.raises(ValueError, match="does not match the retained PNG"): + compile_recording(recording, tmp_path / "bundle", name="mismatch") + + def test_pointer_must_be_inside_event_viewport(self, tmp_path: Path) -> None: + recording = tmp_path / "recording" + (recording / "frames").mkdir(parents=True) + write_frame(recording, 0, "before", blank()) + write_frame(recording, 0, "after", blank()) + (recording / "events.jsonl").write_text( + json.dumps( + { + "i": 0, + "kind": "click", + "x": 1280, + "y": 50, + "t": 1.0, + "viewport_before": [1280, 800], + "viewport_after": [1280, 800], + } + ) + + "\n" + ) + (recording / "meta.json").write_text( + json.dumps( + { + "id": "outside-viewport", + "created_at": "2026-08-18T00:00:00+00:00", + "viewport": list(VIEWPORT), + "params": {}, + } + ) + ) + + with pytest.raises(ValueError, match="is outside viewport"): + compile_recording(recording, tmp_path / "bundle", name="outside") + def test_incomplete_frame_pair_fails_loud(self, tmp_path: Path) -> None: recording = tmp_path / "recording" (recording / "frames").mkdir(parents=True) diff --git a/tests/test_recorder.py b/tests/test_recorder.py index 89fbf993..2f0e0426 100644 --- a/tests/test_recorder.py +++ b/tests/test_recorder.py @@ -95,6 +95,8 @@ def test_recorder_writes_recording_format(tmp_path: Path) -> None: assert events[3]["text"] == "world" assert events[3]["param"] == "note" assert events[4]["key"] == "Enter" + assert all(e["viewport_before"] == [1280, 800] for e in events) + assert all(e["viewport_after"] == [1280, 800] for e in events) times = [e["t"] for e in events] assert times == sorted(times) assert all(t >= 0 for t in times) From 67f38b519fd815c2ecd7881ff92cff9a113102d0 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 11:47:38 -0400 Subject: [PATCH 02/30] docs: align generated verification timestamp --- docs/verification.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/verification.json b/docs/verification.json index 63cb370b..c5b851f2 100644 --- a/docs/verification.json +++ b/docs/verification.json @@ -1,5 +1,5 @@ { - "generated_at": "2026-08-15T00:03:37+02:00 (git HEAD commit date)", + "generated_at": "committed registry state (regenerate: scripts/validate_claims.py --report)", "green_check_run": false, "green_check_job": null, "green_check_scope": [], From 64285e7452841ad74fc7450af94fe0d699246901 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 12:35:59 -0400 Subject: [PATCH 03/30] fix(browser): harden attached recording boundaries --- .gitignore | 2 + docs/BROWSER_RECORDING.md | 19 +++- openadapt_flow/compiler/compile.py | 12 +- openadapt_flow/interactive_recorder.py | 146 +++++++++++++++++++++---- pyproject.toml | 4 + scripts/check_release_consistency.py | 28 +++-- tests/test_browser_attach.py | 96 ++++++++++++++++ tests/test_compiler.py | 40 +++++++ tests/test_release_contract.py | 35 ++++++ 9 files changed, 348 insertions(+), 34 deletions(-) diff --git a/.gitignore b/.gitignore index e0b754be..64bfcee4 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,8 @@ build/ .DS_Store /runs/ /recordings/ +/.openadapt-chrome-profile/ +/.openadapt-recording-partial-*/ dist/ benchmark/openemr/finals/ benchmark/openemr/rows.jsonl diff --git a/docs/BROWSER_RECORDING.md b/docs/BROWSER_RECORDING.md index 92aa08cd..021c65df 100644 --- a/docs/BROWSER_RECORDING.md +++ b/docs/BROWSER_RECORDING.md @@ -20,7 +20,7 @@ change. ```bash openadapt-flow record --backend web \ --url https://your.app \ - --out recording + --out recordings/browser-session ``` Flow opens the URL. Perform the workflow. Then press Ctrl-C in the terminal or @@ -38,7 +38,7 @@ macOS with Google Chrome: "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \ --remote-debugging-address=127.0.0.1 \ --remote-debugging-port=9222 \ - --user-data-dir="./.openadapt-chrome-profile" + --user-data-dir="$HOME/Library/Application Support/OpenAdapt/ChromeRecorderProfile" ``` Linux with Google Chrome or Chromium: @@ -47,7 +47,7 @@ Linux with Google Chrome or Chromium: google-chrome \ --remote-debugging-address=127.0.0.1 \ --remote-debugging-port=9222 \ - --user-data-dir="./.openadapt-chrome-profile" + --user-data-dir="${XDG_DATA_HOME:-$HOME/.local/share}/openadapt/chrome-recorder-profile" ``` Windows PowerShell with Google Chrome: @@ -56,7 +56,7 @@ Windows PowerShell with Google Chrome: & "$env:ProgramFiles\Google\Chrome\Application\chrome.exe" ` --remote-debugging-address=127.0.0.1 ` --remote-debugging-port=9222 ` - --user-data-dir="$PWD\.openadapt-chrome-profile" + --user-data-dir="$env:LOCALAPPDATA\OpenAdapt\ChromeRecorderProfile" ``` Open and sign in to the application in that browser. Then attach the recorder: @@ -65,9 +65,13 @@ Open and sign in to the application in that browser. Then attach the recorder: openadapt-flow record --backend web \ --url https://your.app \ --browser-cdp-endpoint http://127.0.0.1:9222 \ - --out recording + --out recordings/browser-session ``` +Keep the selected tab open during recording. Press `Ctrl-C` in the terminal to +finish. Flow retains the final evidence and then confirms the output path. It +refuses a closed tab and does not publish incomplete metadata. + Flow selects the sole open HTTP or HTTPS tab on the `--url` origin. It does not navigate the tab. It also does not close the tab or browser when recording finishes. @@ -80,7 +84,7 @@ openadapt-flow record --backend web \ --url https://your.app \ --browser-cdp-endpoint http://127.0.0.1:9222 \ --browser-page-url 'https://your.app/work/items?view=open' \ - --out recording + --out recordings/browser-session-selected ``` Diagnostic messages omit URL query and fragment values. The exact selector is @@ -102,6 +106,9 @@ sensitive recording data. metadata. - Attach mode does not combine with `--headless`. The external browser owns its display mode. +- The `--out` path must not exist. Flow writes to a new temporary sibling and + publishes that directory only after it writes the final metadata. A refusal + removes the temporary output and does not change an existing recording. - Input event payloads carry a unique recording-session binding and have a 1 MB limit. Flow removes the current document listeners when it detaches. - Attached screenshots use CSS pixels and the actual live viewport. Thus, DOM diff --git a/openadapt_flow/compiler/compile.py b/openadapt_flow/compiler/compile.py index 80217c98..8a6059dd 100644 --- a/openadapt_flow/compiler/compile.py +++ b/openadapt_flow/compiler/compile.py @@ -1793,12 +1793,22 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: png=before_png, event_index=i, ) - _validated_event_viewport( + after_viewport = _validated_event_viewport( event, key="viewport_after", png=after_png, event_index=i, ) + if ( + before_viewport is not None + and after_viewport is not None + and before_viewport != after_viewport + ): + raise ValueError( + f"viewport changed during event {i}: before " + f"{before_viewport[0]}x{before_viewport[1]}, after " + f"{after_viewport[0]}x{after_viewport[1]}" + ) if kind in ("click", "double_click", "right_click", "drag"): if before_png is None: diff --git a/openadapt_flow/interactive_recorder.py b/openadapt_flow/interactive_recorder.py index d690c3f6..9d20b725 100644 --- a/openadapt_flow/interactive_recorder.py +++ b/openadapt_flow/interactive_recorder.py @@ -43,6 +43,9 @@ import io import ipaddress import json +import os +import shutil +import tempfile import uuid from pathlib import Path from typing import Any, Callable, Optional @@ -75,6 +78,9 @@ class BrowserAttachError(RuntimeError): """A safe browser-attachment precondition was not met.""" +_PARTIAL_RECORDING_PREFIX = ".openadapt-recording-partial-" + + def _http_origin(url: str, *, label: str) -> tuple[str, str, int]: """Return a normalized HTTP origin or refuse an unsafe attach target.""" @@ -634,9 +640,13 @@ def __init__( self.done = False # Set on start(). + self._recording_dir: Optional[Path] = None self._pw = None self._browser = None self.page = None + self._page_close_listener = self._handle_page_close + self._frame_navigation_listener = self._handle_frame_navigation + self._page_lifecycle_listeners_installed = False self.backend: Optional[PlaywrightBackend] = None self.recorder: Optional[Recorder] = None self._last_frame: bytes = b"" @@ -651,14 +661,15 @@ def __init__( def start(self) -> None: """Launch or attach, install listeners, and capture the first frame.""" - if self._owns_browser: - from openadapt_flow._browser_setup import ensure_chromium_installed + self._prepare_recording_dir() + try: + if self._owns_browser: + from openadapt_flow._browser_setup import ensure_chromium_installed - ensure_chromium_installed() - from playwright.sync_api import sync_playwright + ensure_chromium_installed() + from playwright.sync_api import sync_playwright - self._pw = sync_playwright().start() - try: + self._pw = sync_playwright().start() if self._owns_browser: self._browser = self._pw.chromium.launch(headless=self._headless) self.page = self._browser.new_page( @@ -685,7 +696,9 @@ def start(self) -> None: page_url=self._browser_page_url, ) - self.page.on("close", lambda _=None: setattr(self, "done", True)) + self.page.on("close", self._page_close_listener) + self.page.on("framenavigated", self._frame_navigation_listener) + self._page_lifecycle_listeners_installed = True self.page.expose_binding( self._binding_name, lambda source, detail: self._enqueue_browser_event( @@ -735,9 +748,10 @@ def start(self) -> None: self.page, screenshot_scale="device" if self._owns_browser else "css", ) + assert self._recording_dir is not None self.recorder = Recorder( self.backend, - self._out_dir, + self._recording_dir, app_url=self._url, system_of_record_reader=self._system_of_record_reader, **self._settle, @@ -748,14 +762,15 @@ def start(self) -> None: else: self._rebaseline_attached_viewport() except Exception: - self._stop_browser_connection() + self.abort() raise def run(self) -> Path: """Pump until completion, an operator stop, or a closed window.""" if self._stop_when is None: finish_instruction = ( - "Press Ctrl-C here (or close the selected browser tab) to finish." + "Press Ctrl-C here to finish. Keep the selected browser tab open " + "until Flow confirms the recording." if not self._owns_browser else "Press Ctrl-C here (or close the browser window) to finish." ) @@ -802,11 +817,7 @@ def finish(self) -> Path: self._drain_event_queue() self._flush_type() self._flush_scroll() - except Exception: - self.abort() - raise - assert self.recorder is not None - try: + assert self.recorder is not None out = self.recorder.finish() meta_path = out / "meta.json" meta = json.loads(meta_path.read_text()) @@ -821,17 +832,102 @@ def finish(self) -> Path: meta["viewport_mode"] = "per-event" meta["viewport_history"] = list(self._viewport_history) meta_path.write_text(json.dumps(meta, indent=2)) - finally: self._stop_browser_connection() - return out + return self._promote_recording() + except Exception: + self.abort() + raise def abort(self) -> None: - """Detach after a refused or failed recording without writing meta.json.""" + """Detach and remove only this session's unpublished temporary output.""" self.done = True - self._cleanup_page_listeners() self._pyq.clear() - self._stop_browser_connection() + try: + self._cleanup_page_listeners() + finally: + try: + self._stop_browser_connection() + finally: + self._discard_recording_dir() + + def _prepare_recording_dir(self) -> None: + """Reserve a fresh sibling directory without changing the final path.""" + + if self._recording_dir is not None: + return + if os.path.lexists(self._out_dir): + raise BrowserAttachError( + "the recording output already exists; choose a new --out directory" + ) + try: + self._out_dir.parent.mkdir(parents=True, exist_ok=True) + temporary = tempfile.mkdtemp( + prefix=f"{_PARTIAL_RECORDING_PREFIX}{self._out_dir.name}-", + dir=self._out_dir.parent, + ) + except OSError as exc: + raise BrowserAttachError( + "the temporary recording output could not be created" + ) from exc + self._recording_dir = Path(temporary) + + def _promote_recording(self) -> Path: + """Atomically publish the complete recording at the requested path.""" + + assert self._recording_dir is not None + if os.path.lexists(self._out_dir): + raise BrowserAttachError( + "the recording output appeared during capture; Flow refused to " + "replace it" + ) + try: + self._recording_dir.rename(self._out_dir) + except OSError as exc: + raise BrowserAttachError( + "the complete recording could not be published atomically" + ) from exc + self._recording_dir = None + return self._out_dir + + def _discard_recording_dir(self) -> None: + """Delete only the temporary directory that this session created.""" + + recording_dir, self._recording_dir = self._recording_dir, None + if recording_dir is None or not recording_dir.exists(): + return + shutil.rmtree(recording_dir) + + def _handle_page_close(self, _page: Any = None) -> None: + """Retain a refusal when an attached tab closes before finalization.""" + + self.done = True + if not self._owns_browser and self._listener_error is None: + self._listener_error = BrowserAttachError( + "the selected browser tab closed before Flow could retain the " + "final evidence; recording stopped without complete metadata" + ) + + def _handle_frame_navigation(self, frame: Any) -> None: + """Retain the first selected-main-frame origin violation.""" + + if self._owns_browser or self.page is None or self._listener_error is not None: + return + try: + if frame is not self.page.main_frame: + return + current_origin = _http_origin( + str(frame.url), + label="the selected browser tab URL", + ) + except Exception: + current_origin = None + if current_origin != self._attached_origin: + self._listener_error = BrowserAttachError( + "the selected browser tab left the declared application origin; " + "recording was refused" + ) + self.done = True def _enqueue_browser_event( self, @@ -961,6 +1057,16 @@ def _stop_browser_connection(self) -> None: """Close an owned browser or detach without closing an external one.""" self._cleanup_page_listeners() + if self.page is not None and self._page_lifecycle_listeners_installed: + for event, listener in ( + ("close", self._page_close_listener), + ("framenavigated", self._frame_navigation_listener), + ): + try: + self.page.remove_listener(event, listener) + except Exception: + pass + self._page_lifecycle_listeners_installed = False browser, self._browser = self._browser, None playwright, self._pw = self._pw, None try: diff --git a/pyproject.toml b/pyproject.toml index c4a8cfcc..41507fe5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -196,6 +196,8 @@ packages = ["openadapt_flow"] # list in lockstep with the sdist target and the archive validator below. exclude = [ "/.hypothesis", + "/.openadapt-chrome-profile", + "/.openadapt-recording-partial-*", "/benchmark/**/api-delta-probe-*", "/benchmark/**/bundle-live*", "/benchmark/**/out", @@ -249,6 +251,8 @@ exclude = [ # fake-patient synthetic fixtures, and bounded aggregate evidence remain. exclude = [ "/.hypothesis", + "/.openadapt-chrome-profile", + "/.openadapt-recording-partial-*", "/benchmark/**/api-delta-probe-*", "/benchmark/**/bundle-live*", "/benchmark/**/out", diff --git a/scripts/check_release_consistency.py b/scripts/check_release_consistency.py index 219fb5a4..af619ab6 100644 --- a/scripts/check_release_consistency.py +++ b/scripts/check_release_consistency.py @@ -226,6 +226,8 @@ def load_source_policy(path: Path = SOURCE_POLICY_PATH) -> SourcePolicy: } ) RECORDING_METADATA_BASENAMES = frozenset({"events.jsonl", "meta.json"}) +LOCAL_SENSITIVE_ARCHIVE_SEGMENTS = frozenset({".openadapt-chrome-profile"}) +LOCAL_SENSITIVE_ARCHIVE_PREFIXES = (".openadapt-recording-partial-",) RECORDING_AUTHORITY_TOKENS = ( b"access_token", b"api_key", @@ -494,17 +496,27 @@ def _repository_only_evaluation_hits(members: set[str]) -> set[str]: def _generated_or_raw_archive_hits( members: set[str], payloads: dict[str, bytes] ) -> set[str]: - """Return generated sessions, raw media/results, or retained authority. + """Return local secrets, generated sessions, or retained authority. Synthetic fixture source and reviewed aggregate results remain eligible for - publication. What cannot ship is a local run directory, raw video/per-run - result, or recording metadata that retains session authority. + publication. What cannot ship is a browser profile, an unpublished partial + recording, a local run directory, raw video/per-run result, or recording + metadata that retains session authority. """ hits: set[str] = set() for member in members: path = PurePosixPath(member) lower_parts = tuple(part.lower() for part in path.parts) basename = path.name.lower() + if any( + part in LOCAL_SENSITIVE_ARCHIVE_SEGMENTS + or any( + part.startswith(prefix) for prefix in LOCAL_SENSITIVE_ARCHIVE_PREFIXES + ) + for part in lower_parts + ): + hits.add(member) + continue if path.suffix.lower() in RAW_VIDEO_SUFFIXES: hits.add(member) continue @@ -1323,8 +1335,9 @@ def validate_sdist_license_boundary( generated_or_raw = _generated_or_raw_archive_hits(members, payloads) if generated_or_raw: raise ValueError( - "source distribution contains generated/raw benchmark output, " - "recording authority, or per-run evidence: " + "source distribution contains a browser profile, unpublished " + "recording, generated/raw benchmark output, recording authority, " + "or per-run evidence: " f"{sorted(generated_or_raw)}" ) missing = REQUIRED_SDIST_PATHS - members @@ -1437,8 +1450,9 @@ def validate_wheel_license_boundary( generated_or_raw = _generated_or_raw_archive_hits(members, payloads) if generated_or_raw: raise ValueError( - "wheel contains generated/raw benchmark output, recording authority, " - f"or per-run evidence: {sorted(generated_or_raw)}" + "wheel contains a browser profile, unpublished recording, " + "generated/raw benchmark output, recording authority, or per-run " + f"evidence: {sorted(generated_or_raw)}" ) forbidden = { member diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index 411b1a52..5aa8a380 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -177,6 +177,84 @@ def test_attached_recorder_reads_geometry_and_refuses_origin_drift( assert "DO_NOT_PRINT" not in str(caught.value) +def test_attached_recorder_retains_main_frame_origin_violation( + tmp_path: Path, +) -> None: + out = tmp_path / "recording" + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + session._prepare_recording_dir() + frame = SimpleNamespace(url="https://other.example.test/temporary") + session.page = SimpleNamespace(main_frame=frame) + + session._handle_frame_navigation(frame) + frame.url = "https://app.example.test/returned" + session._handle_frame_navigation(frame) + + assert session.done is True + assert session._listener_error is not None + assert "left the declared application origin" in str(session._listener_error) + with pytest.raises(BrowserAttachError, match="left the declared"): + session.finish() + assert not out.exists() + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + +def test_attached_recorder_refuses_existing_output_without_changing_it( + tmp_path: Path, +) -> None: + out = tmp_path / "recording" + frames = out / "frames" + frames.mkdir(parents=True) + (out / "meta.json").write_text('{"id":"complete-existing"}\n') + (out / "events.jsonl").write_text('{"i":0,"kind":"key"}\n') + (frames / "0000_before.png").write_bytes(b"SENSITIVE-FRAME") + before = { + path.relative_to(out): path.read_bytes() + for path in out.rglob("*") + if path.is_file() + } + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + + with pytest.raises(BrowserAttachError, match="output already exists"): + session._prepare_recording_dir() + session.abort() + + after = { + path.relative_to(out): path.read_bytes() + for path in out.rglob("*") + if path.is_file() + } + assert after == before + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + +def test_attached_tab_close_discards_partial_output(tmp_path: Path) -> None: + out = tmp_path / "recording" + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + session._prepare_recording_dir() + assert session._recording_dir is not None + (session._recording_dir / "meta.json").write_text('{"id":"not-final"}\n') + + session._handle_page_close() + + with pytest.raises(BrowserAttachError, match="selected browser tab closed"): + session.finish() + assert not out.exists() + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + def test_attached_recorder_refuses_iframe_events(tmp_path: Path) -> None: session = InteractiveRecorder( "https://app.example.test/", @@ -495,6 +573,24 @@ def click_inside_existing_iframe(page, pump): assert not (iframe_recording / "meta.json").exists() assert process.poll() is None + origin_bounce_recording = tmp_path / "recording-origin-bounce-refusal" + other_origin = attach_app_url.replace("127.0.0.1", "localhost") + + def leave_origin_and_return(page, pump): + page.goto(other_origin) + page.goto(attach_app_url) + pump() + + with pytest.raises(BrowserAttachError, match="left the declared"): + record_interactive( + attach_app_url, + origin_bounce_recording, + cdp_endpoint=endpoint, + script=leave_origin_and_return, + ) + assert not origin_bounce_recording.exists() + assert process.poll() is None + overlap_recording = tmp_path / "recording-resize-overlap-refusal" def resize_during_action(page, pump): diff --git a/tests/test_compiler.py b/tests/test_compiler.py index ea82a88f..bacc88a5 100644 --- a/tests/test_compiler.py +++ b/tests/test_compiler.py @@ -204,6 +204,46 @@ def test_per_event_viewports_can_change_between_steps(self, tmp_path: Path) -> N assert len(workflow.steps) == 2 assert workflow.viewport == VIEWPORT + def test_per_event_viewport_cannot_change_within_one_step( + self, tmp_path: Path + ) -> None: + recording = tmp_path / "recording" + (recording / "frames").mkdir(parents=True) + write_frame(recording, 0, "before", blank()) + write_frame( + recording, + 0, + "after", + np.full((600, 900, 3), 245, dtype=np.uint8), + ) + (recording / "events.jsonl").write_text( + json.dumps( + { + "i": 0, + "kind": "key", + "key": "Enter", + "t": 1.0, + "viewport_before": [1280, 800], + "viewport_after": [900, 600], + } + ) + + "\n" + ) + (recording / "meta.json").write_text( + json.dumps( + { + "id": "mid-event-resize", + "created_at": "2026-08-18T00:00:00+00:00", + "viewport": list(VIEWPORT), + "viewport_mode": "per-event", + "params": {}, + } + ) + ) + + with pytest.raises(ValueError, match="viewport changed during event 0"): + compile_recording(recording, tmp_path / "bundle", name="mid-resize") + def test_per_event_viewport_must_match_retained_png(self, tmp_path: Path) -> None: recording = tmp_path / "recording" (recording / "frames").mkdir(parents=True) diff --git a/tests/test_release_contract.py b/tests/test_release_contract.py index bc1d855c..9b28f334 100644 --- a/tests/test_release_contract.py +++ b/tests/test_release_contract.py @@ -70,6 +70,10 @@ "/tests/test_reliability.py", } EPHEMERAL_BUILD_EXCLUDES = {"/.hypothesis"} +LOCAL_SENSITIVE_EXCLUDES = { + "/.openadapt-chrome-profile", + "/.openadapt-recording-partial-*", +} GENERATED_BENCHMARK_EXCLUDES = { "/benchmark/**/api-delta-probe-*", "/benchmark/**/bundle-live*", @@ -139,11 +143,18 @@ def test_required_wheel_job_builds_and_validates_actual_sdist() -> None: def test_wheel_and_sdist_exclude_repository_only_evidence() -> None: pyproject = tomllib.loads((ROOT / "pyproject.toml").read_text()) targets = pyproject["tool"]["hatch"]["build"]["targets"] + gitignore = set((ROOT / ".gitignore").read_text().splitlines()) assert SOURCE_BOUNDARY_EXCLUDES <= set(targets["wheel"]["exclude"]) assert SOURCE_BOUNDARY_EXCLUDES <= set(targets["sdist"]["exclude"]) assert EPHEMERAL_BUILD_EXCLUDES <= set(targets["wheel"]["exclude"]) assert EPHEMERAL_BUILD_EXCLUDES <= set(targets["sdist"]["exclude"]) + assert LOCAL_SENSITIVE_EXCLUDES <= set(targets["wheel"]["exclude"]) + assert LOCAL_SENSITIVE_EXCLUDES <= set(targets["sdist"]["exclude"]) + assert { + "/.openadapt-chrome-profile/", + "/.openadapt-recording-partial-*/", + } <= gitignore assert GENERATED_BENCHMARK_EXCLUDES <= set(targets["wheel"]["exclude"]) assert GENERATED_BENCHMARK_EXCLUDES <= set(targets["sdist"]["exclude"]) @@ -573,6 +584,30 @@ def test_sdist_requires_mit_license_and_excludes_openimis_surface( validate_sdist_license_boundary(mixed) +def test_release_artifacts_refuse_local_browser_and_partial_recordings( + tmp_path: Path, +) -> None: + sdist_base = {*REQUIRED_SDIST_PATHS, "PKG-INFO"} + wheel_base = { + "openadapt_flow-1.0.dist-info/licenses/LICENSE", + "openadapt_flow-1.0.dist-info/METADATA", + } + sensitive_paths = ( + ".openadapt-chrome-profile/Cookies", + ".openadapt-recording-partial-session/events.jsonl", + ) + for index, sensitive in enumerate(sensitive_paths): + sdist = tmp_path / f"sensitive-{index}.tar.gz" + _write_sdist(sdist, {*sdist_base, sensitive}) + with pytest.raises(ValueError, match="browser profile, unpublished"): + validate_sdist_license_boundary(sdist) + + wheel = tmp_path / f"sensitive-{index}.whl" + _write_wheel(wheel, {*wheel_base, sensitive}) + with pytest.raises(ValueError, match="browser profile, unpublished"): + validate_wheel_license_boundary(wheel) + + def test_wheel_refuses_private_corpus_material(tmp_path: Path) -> None: """The wheel must reject private source-policy material. From b5621bd01ba5fe31d54af141d40823c45ad99fe9 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 12:59:08 -0400 Subject: [PATCH 04/30] fix(browser): harden attached recording finalization --- .gitignore | 4 +- openadapt_flow/backends/playwright_backend.py | 11 ++ openadapt_flow/interactive_recorder.py | 101 ++++++++++++- pyproject.toml | 4 + tests/test_browser_attach.py | 133 +++++++++++++++++- tests/test_release_contract.py | 21 ++- 6 files changed, 263 insertions(+), 11 deletions(-) diff --git a/.gitignore b/.gitignore index 64bfcee4..baaa4842 100644 --- a/.gitignore +++ b/.gitignore @@ -13,8 +13,8 @@ build/ .DS_Store /runs/ /recordings/ -/.openadapt-chrome-profile/ -/.openadapt-recording-partial-*/ +.openadapt-chrome-profile/ +.openadapt-recording-partial-*/ dist/ benchmark/openemr/finals/ benchmark/openemr/rows.jsonl diff --git a/openadapt_flow/backends/playwright_backend.py b/openadapt_flow/backends/playwright_backend.py index 8823732d..8b140ff5 100644 --- a/openadapt_flow/backends/playwright_backend.py +++ b/openadapt_flow/backends/playwright_backend.py @@ -708,6 +708,7 @@ def __init__( page: "Page", *, screenshot_scale: Literal["css", "device"] = "device", + screenshot_mask_selectors: tuple[str, ...] = (), ) -> None: """Wrap an existing Playwright page. @@ -718,9 +719,16 @@ def __init__( default. A browser attached through CDP uses ``css`` so DOM event coordinates and retained frame pixels stay in the same coordinate system even on a high-density display. + screenshot_mask_selectors: CSS selectors whose matching elements + are blacked out by Chromium before screenshot bytes reach + Python. The interactive recorder uses this for password and + declared-secret fields on every retained frame. """ self.page = page self._screenshot_scale = screenshot_scale + self._screenshot_masks = [ + page.locator(selector) for selector in screenshot_mask_selectors + ] # Opaque per-backend key keeps the WeakMap private from ordinary page # code. Python retains only token material keyed by the public # SHA-256 fingerprint; target/row text stays page-local and ephemeral. @@ -2229,6 +2237,9 @@ def screenshot(self) -> bytes: options: dict[str, Any] = {} if self._screenshot_scale == "css": options["scale"] = "css" + if self._screenshot_masks: + options["mask"] = self._screenshot_masks + options["mask_color"] = "#000000" return self.page.screenshot(type="png", full_page=False, **options) def click(self, x: int, y: int, *, double: bool = False) -> None: diff --git a/openadapt_flow/interactive_recorder.py b/openadapt_flow/interactive_recorder.py index 9d20b725..6ea21ebe 100644 --- a/openadapt_flow/interactive_recorder.py +++ b/openadapt_flow/interactive_recorder.py @@ -40,11 +40,14 @@ from __future__ import annotations +import ctypes +import errno import io import ipaddress import json import os import shutil +import sys import tempfile import uuid from pathlib import Path @@ -81,6 +84,83 @@ class BrowserAttachError(RuntimeError): _PARTIAL_RECORDING_PREFIX = ".openadapt-recording-partial-" +def _rename_directory_noreplace(source: Path, destination: Path) -> None: + """Atomically rename a directory without replacing any destination.""" + + if os.name == "nt": + os.rename(source, destination) + return + + libc = ctypes.CDLL(None, use_errno=True) + source_bytes = os.fsencode(source) + destination_bytes = os.fsencode(destination) + result: int + if sys.platform == "linux": + try: + renameat2 = libc.renameat2 + except AttributeError as exc: + raise OSError( + errno.ENOTSUP, + "atomic no-replace directory promotion is unavailable", + destination, + ) from exc + renameat2.argtypes = [ + ctypes.c_int, + ctypes.c_char_p, + ctypes.c_int, + ctypes.c_char_p, + ctypes.c_uint, + ] + renameat2.restype = ctypes.c_int + result = int( + renameat2( + -100, # AT_FDCWD + source_bytes, + -100, # AT_FDCWD + destination_bytes, + 1, # RENAME_NOREPLACE + ) + ) + elif sys.platform == "darwin": + try: + renamex_np = libc.renamex_np + except AttributeError as exc: + raise OSError( + errno.ENOTSUP, + "atomic no-replace directory promotion is unavailable", + destination, + ) from exc + renamex_np.argtypes = [ctypes.c_char_p, ctypes.c_char_p, ctypes.c_uint] + renamex_np.restype = ctypes.c_int + result = int( + renamex_np( + source_bytes, + destination_bytes, + 0x00000004, # RENAME_EXCL + ) + ) + else: + raise OSError( + errno.ENOTSUP, + "atomic no-replace directory promotion is unavailable", + destination, + ) + + if result != 0: + error_number = ctypes.get_errno() + raise OSError(error_number, os.strerror(error_number), destination) + + +def _secret_screenshot_selectors(secret_fields: set[str]) -> tuple[str, ...]: + """Return selectors that mask password and declared-secret fields.""" + + selectors = ["input[type='password']"] + for field in sorted(secret_fields): + encoded = json.dumps(field) + selectors.append(f"[name={encoded}], [id={encoded}]") + return tuple(selectors) + + def _http_origin(url: str, *, label: str) -> tuple[str, str, int]: """Return a normalized HTTP origin or refuse an unsafe attach target.""" @@ -747,6 +827,9 @@ def start(self) -> None: self.backend = PlaywrightBackend( self.page, screenshot_scale="device" if self._owns_browser else "css", + screenshot_mask_selectors=_secret_screenshot_selectors( + self._secret_fields + ), ) assert self._recording_dir is not None self.recorder = Recorder( @@ -819,6 +902,8 @@ def finish(self) -> Path: self._flush_scroll() assert self.recorder is not None out = self.recorder.finish() + if self._listener_error is not None: + raise self._listener_error meta_path = out / "meta.json" meta = json.loads(meta_path.read_text()) meta["source"] = ( @@ -832,7 +917,11 @@ def finish(self) -> Path: meta["viewport_mode"] = "per-event" meta["viewport_history"] = list(self._viewport_history) meta_path.write_text(json.dumps(meta, indent=2)) + if self._listener_error is not None: + raise self._listener_error self._stop_browser_connection() + if self._listener_error is not None: + raise self._listener_error return self._promote_recording() except Exception: self.abort() @@ -876,14 +965,14 @@ def _promote_recording(self) -> Path: """Atomically publish the complete recording at the requested path.""" assert self._recording_dir is not None - if os.path.lexists(self._out_dir): - raise BrowserAttachError( - "the recording output appeared during capture; Flow refused to " - "replace it" - ) try: - self._recording_dir.rename(self._out_dir) + _rename_directory_noreplace(self._recording_dir, self._out_dir) except OSError as exc: + if exc.errno in {errno.EEXIST, errno.ENOTEMPTY}: + raise BrowserAttachError( + "the recording output appeared during capture; Flow refused to " + "replace it" + ) from exc raise BrowserAttachError( "the complete recording could not be published atomically" ) from exc diff --git a/pyproject.toml b/pyproject.toml index 41507fe5..e66eb613 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -198,6 +198,8 @@ exclude = [ "/.hypothesis", "/.openadapt-chrome-profile", "/.openadapt-recording-partial-*", + "/**/.openadapt-chrome-profile", + "/**/.openadapt-recording-partial-*", "/benchmark/**/api-delta-probe-*", "/benchmark/**/bundle-live*", "/benchmark/**/out", @@ -253,6 +255,8 @@ exclude = [ "/.hypothesis", "/.openadapt-chrome-profile", "/.openadapt-recording-partial-*", + "/**/.openadapt-chrome-profile", + "/**/.openadapt-recording-partial-*", "/benchmark/**/api-delta-probe-*", "/benchmark/**/bundle-live*", "/benchmark/**/out", diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index 5aa8a380..3767f6f0 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -14,6 +14,7 @@ from urllib.request import urlopen import pytest +from PIL import Image from openadapt_flow.__main__ import main from openadapt_flow.backends.playwright_backend import PlaywrightBackend @@ -140,6 +141,38 @@ def screenshot(self, **kwargs): assert page.screenshot_options["scale"] == "css" +def test_backend_masks_password_and_declared_secret_fields_on_every_frame() -> None: + class Page: + viewport_size = {"width": 1280, "height": 800} + + def __init__(self) -> None: + self.screenshot_options: list[dict] = [] + + def locator(self, selector): + return f"locator:{selector}" + + def screenshot(self, **kwargs): + self.screenshot_options.append(kwargs) + return b"png" + + selectors = ( + "input[type='password']", + '[name="token"], [id="token"]', + ) + page = Page() + backend = PlaywrightBackend( # type: ignore[arg-type] + page, + screenshot_mask_selectors=selectors, + ) + + assert backend.screenshot() == b"png" + assert backend.screenshot() == b"png" + assert len(page.screenshot_options) == 2 + for options in page.screenshot_options: + assert options["mask"] == [f"locator:{selector}" for selector in selectors] + assert options["mask_color"] == "#000000" + + def test_attached_recorder_api_refuses_incompatible_options(tmp_path: Path) -> None: with pytest.raises(BrowserAttachError, match="requires a browser CDP"): InteractiveRecorder( @@ -236,6 +269,34 @@ def test_attached_recorder_refuses_existing_output_without_changing_it( assert not list(tmp_path.glob(".openadapt-recording-partial-*")) +def test_attached_recorder_refuses_output_created_during_promotion( + tmp_path: Path, +) -> None: + out = tmp_path / "recording" + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + session._prepare_recording_dir() + assert session._recording_dir is not None + (session._recording_dir / "complete.txt").write_text("new recording") + + out.mkdir() + original_identity = out.stat() + with pytest.raises(BrowserAttachError, match="appeared during capture"): + session._promote_recording() + + current_identity = out.stat() + assert (current_identity.st_dev, current_identity.st_ino) == ( + original_identity.st_dev, + original_identity.st_ino, + ) + session.abort() + assert out.is_dir() + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + def test_attached_tab_close_discards_partial_output(tmp_path: Path) -> None: out = tmp_path / "recording" session = InteractiveRecorder( @@ -255,6 +316,52 @@ def test_attached_tab_close_discards_partial_output(tmp_path: Path) -> None: assert not list(tmp_path.glob(".openadapt-recording-partial-*")) +def test_attached_tab_close_during_finalization_discards_metadata( + tmp_path: Path, +) -> None: + out = tmp_path / "recording" + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + session._prepare_recording_dir() + + class ClosingPage: + url = "https://app.example.test/" + frames: list = [] + + def __init__(self) -> None: + self.main_frame = SimpleNamespace(url=self.url) + + def evaluate(self, _script): + return {"width": 1280, "height": 800, "dpr": 1} + + def remove_listener(self, event, listener): + if event == "close": + listener() + + session.page = ClosingPage() + session._page_lifecycle_listeners_installed = True + session._attached_geometry = (1280, 800, 1.0) + session._initial_attached_viewport = (1280, 800) + + class ClosingRecorder: + def finish(self): + assert session._recording_dir is not None + (session._recording_dir / "meta.json").write_text( + json.dumps({"viewport": [1280, 800]}) + ) + return session._recording_dir + + session.recorder = ClosingRecorder() # type: ignore[assignment] + + with pytest.raises(BrowserAttachError, match="selected browser tab closed"): + session.finish() + assert not out.exists() + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + def test_attached_recorder_refuses_iframe_events(tmp_path: Path) -> None: session = InteractiveRecorder( "https://app.example.test/", @@ -413,7 +520,7 @@ def test_cli_refuses_headless_attachment(tmp_path: Path) -> None: - + """ @@ -505,6 +612,7 @@ def test_live_cdp_attach_records_compiles_and_leaves_browser_running_three_trial for trial in range(3): secret = f"ATTACH-SECRET-{trial}-NEVER-PERSIST" + secret_rect: dict[str, int] = {} def drive(page, pump, *, secret_value=secret, trial_number=trial): page.evaluate( @@ -519,6 +627,14 @@ def drive(page, pump, *, secret_value=secret, trial_number=trial): pump() pump() page.click("#password") + box = page.locator("#password").bounding_box() + assert box is not None + secret_rect.update( + x=round(box["x"]), + y=round(box["y"]), + width=round(box["width"]), + height=round(box["height"]), + ) page.keyboard.type(secret_value) pump() pump() @@ -530,6 +646,7 @@ def drive(page, pump, *, secret_value=secret, trial_number=trial): recording = record_interactive( attach_app_url, tmp_path / f"recording-{trial}", + secret_fields=("password",), param_fields=("note",), cdp_endpoint=endpoint, script=drive, @@ -541,6 +658,20 @@ def drive(page, pump, *, secret_value=secret, trial_number=trial): assert secret not in json.dumps(meta) assert secret not in events_text assert meta["viewport"][0] > 0 and meta["viewport"][1] > 0 + events = [json.loads(line) for line in events_text.splitlines()] + secret_event = next(event for event in events if event.get("secret")) + next_before = Image.open( + recording / "frames" / f"{int(secret_event['i']) + 1:04d}_before.png" + ).convert("RGB") + crop = next_before.crop( + ( + secret_rect["x"], + secret_rect["y"], + secret_rect["x"] + secret_rect["width"], + secret_rect["y"] + secret_rect["height"], + ) + ) + assert all(extrema == (0, 0) for extrema in crop.getextrema()) bundle = tmp_path / f"bundle-{trial}" workflow = compile_recording( diff --git a/tests/test_release_contract.py b/tests/test_release_contract.py index 9b28f334..39547299 100644 --- a/tests/test_release_contract.py +++ b/tests/test_release_contract.py @@ -73,6 +73,8 @@ LOCAL_SENSITIVE_EXCLUDES = { "/.openadapt-chrome-profile", "/.openadapt-recording-partial-*", + "/**/.openadapt-chrome-profile", + "/**/.openadapt-recording-partial-*", } GENERATED_BENCHMARK_EXCLUDES = { "/benchmark/**/api-delta-probe-*", @@ -152,8 +154,8 @@ def test_wheel_and_sdist_exclude_repository_only_evidence() -> None: assert LOCAL_SENSITIVE_EXCLUDES <= set(targets["wheel"]["exclude"]) assert LOCAL_SENSITIVE_EXCLUDES <= set(targets["sdist"]["exclude"]) assert { - "/.openadapt-chrome-profile/", - "/.openadapt-recording-partial-*/", + ".openadapt-chrome-profile/", + ".openadapt-recording-partial-*/", } <= gitignore assert GENERATED_BENCHMARK_EXCLUDES <= set(targets["wheel"]["exclude"]) assert GENERATED_BENCHMARK_EXCLUDES <= set(targets["sdist"]["exclude"]) @@ -176,6 +178,21 @@ def test_wheel_and_sdist_exclude_repository_only_evidence() -> None: assert "tests/test_reliability.py" in REPOSITORY_ONLY_EVALUATION_EXACT_PATHS +def test_local_sensitive_exclusions_apply_at_nested_paths() -> None: + pyproject = tomllib.loads((ROOT / "pyproject.toml").read_text()) + targets = pyproject["tool"]["hatch"]["build"]["targets"] + gitignore = set((ROOT / ".gitignore").read_text().splitlines()) + recursive = { + "/**/.openadapt-chrome-profile", + "/**/.openadapt-recording-partial-*", + } + + assert recursive <= set(targets["wheel"]["exclude"]) + assert recursive <= set(targets["sdist"]["exclude"]) + assert ".openadapt-chrome-profile/" in gitignore + assert ".openadapt-recording-partial-*/" in gitignore + + def test_public_source_tree_excludes_private_data_and_recipes(tmp_path: Path) -> None: allowed = tmp_path / "benchmark/reliability" allowed.mkdir(parents=True) From 66dad30e23c30cea3c2627e5a81ba81f1b701fb7 Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 13:19:46 -0400 Subject: [PATCH 05/30] fix(browser): close attach evidence gaps --- docs/BROWSER_RECORDING.md | 6 +- openadapt_flow/backends/playwright_backend.py | 22 +- openadapt_flow/interactive_recorder.py | 75 +++++- tests/test_browser_attach.py | 246 +++++++++++++++++- 4 files changed, 336 insertions(+), 13 deletions(-) diff --git a/docs/BROWSER_RECORDING.md b/docs/BROWSER_RECORDING.md index 021c65df..aa1c803d 100644 --- a/docs/BROWSER_RECORDING.md +++ b/docs/BROWSER_RECORDING.md @@ -104,6 +104,10 @@ sensitive recording data. - The selected tab must stay on that origin for the full recording. A cross-origin navigation stops the recording and does not produce complete metadata. +- Do not open a popup or a new tab in the selected tab's browser context while + recording. Flow currently binds one page. It refuses a new page instead of + omitting actions performed there. A refusal leaves the external browser and + its tabs open. - Attach mode does not combine with `--headless`. The external browser owns its display mode. - The `--out` path must not exist. Flow writes to a new temporary sibling and @@ -167,4 +171,4 @@ endpoint. It requires a browser process started with remote debugging and a separate user-data directory. It does not claim Firefox, WebKit, arbitrary Chrome extensions, an ordinary browser process that was not started for local debugging, cross-origin tab selection, separately qualified cross-frame/iframe -recording, or direct extension replay. +recording, multi-page/popup recording, or direct extension replay. diff --git a/openadapt_flow/backends/playwright_backend.py b/openadapt_flow/backends/playwright_backend.py index 8b140ff5..f22673cf 100644 --- a/openadapt_flow/backends/playwright_backend.py +++ b/openadapt_flow/backends/playwright_backend.py @@ -722,13 +722,14 @@ def __init__( screenshot_mask_selectors: CSS selectors whose matching elements are blacked out by Chromium before screenshot bytes reach Python. The interactive recorder uses this for password and - declared-secret fields on every retained frame. + declared-secret fields on every retained frame. Locators are + rebuilt from every current document frame for each screenshot + so a child-frame field or a frame added after startup cannot + bypass the mask. """ self.page = page self._screenshot_scale = screenshot_scale - self._screenshot_masks = [ - page.locator(selector) for selector in screenshot_mask_selectors - ] + self._screenshot_mask_selectors = screenshot_mask_selectors # Opaque per-backend key keeps the WeakMap private from ordinary page # code. Python retains only token material keyed by the public # SHA-256 fingerprint; target/row text stays page-local and ephemeral. @@ -2237,8 +2238,17 @@ def screenshot(self) -> bytes: options: dict[str, Any] = {} if self._screenshot_scale == "css": options["scale"] = "css" - if self._screenshot_masks: - options["mask"] = self._screenshot_masks + if self._screenshot_mask_selectors: + # A Page locator does not cross an iframe boundary. Rebuild the + # masks from the live frame inventory for every capture so both + # existing and newly attached frames use the same secret contract. + # If a frame detaches while Playwright resolves these locators, the + # screenshot fails closed instead of retaining an unmasked frame. + options["mask"] = [ + frame.locator(selector) + for frame in list(self.page.frames) + for selector in self._screenshot_mask_selectors + ] options["mask_color"] = "#000000" return self.page.screenshot(type="png", full_page=False, **options) diff --git a/openadapt_flow/interactive_recorder.py b/openadapt_flow/interactive_recorder.py index 6ea21ebe..37bc031a 100644 --- a/openadapt_flow/interactive_recorder.py +++ b/openadapt_flow/interactive_recorder.py @@ -156,11 +156,44 @@ def _secret_screenshot_selectors(secret_fields: set[str]) -> tuple[str, ...]: selectors = ["input[type='password']"] for field in sorted(secret_fields): - encoded = json.dumps(field) + encoded = _css_string_literal(field) selectors.append(f"[name={encoded}], [id={encoded}]") return tuple(selectors) +def _css_string_literal(value: str) -> str: + """Serialize an exact value as a valid double-quoted CSS string. + + JSON ``\\u`` escapes are not CSS Unicode escapes. Using ``json.dumps`` + therefore made a declared field such as ``päss`` select a different name + and left its later frames unmasked. CSS strings accept Unicode directly; + quotes, backslashes, and control characters need CSS-specific escapes. + """ + + escaped: list[str] = ['"'] + for character in value: + codepoint = ord(character) + if character in {'"', "\\"}: + escaped.append("\\" + character) + elif codepoint == 0: + raise BrowserAttachError( + "a declared secret field name contains a null character and " + "cannot be bound to a safe browser mask" + ) + elif 0xD800 <= codepoint <= 0xDFFF: + raise BrowserAttachError( + "a declared secret field name contains an invalid Unicode " + "surrogate and cannot be bound to a safe browser mask" + ) + elif codepoint < 0x20 or codepoint == 0x7F: + # The trailing space terminates the variable-width CSS hex escape. + escaped.append(f"\\{codepoint:x} ") + else: + escaped.append(character) + escaped.append('"') + return "".join(escaped) + + def _http_origin(url: str, *, label: str) -> tuple[str, str, int]: """Return a normalized HTTP origin or refuse an unsafe attach target.""" @@ -726,7 +759,9 @@ def __init__( self.page = None self._page_close_listener = self._handle_page_close self._frame_navigation_listener = self._handle_frame_navigation + self._popup_listener = self._handle_popup self._page_lifecycle_listeners_installed = False + self._context_pages_at_start: tuple[Any, ...] = () self.backend: Optional[PlaywrightBackend] = None self.recorder: Optional[Recorder] = None self._last_frame: bytes = b"" @@ -776,8 +811,10 @@ def start(self) -> None: page_url=self._browser_page_url, ) + self._context_pages_at_start = tuple(self.page.context.pages) self.page.on("close", self._page_close_listener) self.page.on("framenavigated", self._frame_navigation_listener) + self.page.on("popup", self._popup_listener) self._page_lifecycle_listeners_installed = True self.page.expose_binding( self._binding_name, @@ -919,6 +956,7 @@ def finish(self) -> Path: meta_path.write_text(json.dumps(meta, indent=2)) if self._listener_error is not None: raise self._listener_error + self._assert_no_new_pages() self._stop_browser_connection() if self._listener_error is not None: raise self._listener_error @@ -1018,6 +1056,38 @@ def _handle_frame_navigation(self, frame: Any) -> None: ) self.done = True + def _handle_popup(self, _popup: Any = None) -> None: + """Refuse a second page that the selected recording tab opens.""" + + self.done = True + if self._listener_error is None: + self._listener_error = BrowserAttachError( + "the selected browser tab opened a popup or new tab; this " + "recording is bound to one tab, so Flow stopped before " + "publishing incomplete metadata" + ) + + def _assert_no_new_pages(self) -> None: + """Retain a refusal if this recording context gained another page.""" + + if self.page is None or not self._context_pages_at_start: + return + try: + current_pages = tuple(self.page.context.pages) + except Exception as exc: + raise BrowserAttachError( + "the selected browser tab page inventory could not be read; " + "recording was refused" + ) from exc + for candidate in current_pages: + if not any( + candidate is existing for existing in self._context_pages_at_start + ): + self._handle_popup(candidate) + break + if self._listener_error is not None: + raise self._listener_error + def _enqueue_browser_event( self, detail: Any, @@ -1150,6 +1220,7 @@ def _stop_browser_connection(self) -> None: for event, listener in ( ("close", self._page_close_listener), ("framenavigated", self._frame_navigation_listener), + ("popup", self._popup_listener), ): try: self.page.remove_listener(event, listener) @@ -1170,6 +1241,7 @@ def _drain_event_queue(self) -> bool: if self._listener_error is not None: raise self._listener_error + self._assert_no_new_pages() batch = self._pyq[:] del self._pyq[:] rebased = False @@ -1203,6 +1275,7 @@ def _drain_event_queue(self) -> bool: "action was being retained; recording stopped without " "complete metadata" ) + self._assert_no_new_pages() if self._listener_error is not None: raise self._listener_error return bool(batch) or rebased diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index 3767f6f0..3a121885 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -22,6 +22,7 @@ from openadapt_flow.interactive_recorder import ( BrowserAttachError, InteractiveRecorder, + _secret_screenshot_selectors, record_interactive, select_attached_page, validate_browser_cdp_endpoint, @@ -142,14 +143,19 @@ def screenshot(self, **kwargs): def test_backend_masks_password_and_declared_secret_fields_on_every_frame() -> None: + class Frame: + def __init__(self, name: str) -> None: + self.name = name + + def locator(self, selector): + return f"locator:{self.name}:{selector}" + class Page: viewport_size = {"width": 1280, "height": 800} def __init__(self) -> None: self.screenshot_options: list[dict] = [] - - def locator(self, selector): - return f"locator:{selector}" + self.frames = [Frame("main"), Frame("child")] def screenshot(self, **kwargs): self.screenshot_options.append(kwargs) @@ -166,13 +172,36 @@ def screenshot(self, **kwargs): ) assert backend.screenshot() == b"png" + page.frames.append(Frame("late-child")) assert backend.screenshot() == b"png" assert len(page.screenshot_options) == 2 + assert page.screenshot_options[0]["mask"] == [ + f"locator:{frame}:{selector}" + for frame in ("main", "child") + for selector in selectors + ] + assert page.screenshot_options[1]["mask"] == [ + f"locator:{frame}:{selector}" + for frame in ("main", "child", "late-child") + for selector in selectors + ] for options in page.screenshot_options: - assert options["mask"] == [f"locator:{selector}" for selector in selectors] assert options["mask_color"] == "#000000" +def test_declared_secret_selectors_use_css_string_escaping() -> None: + selectors = _secret_screenshot_selectors({"päss", 'quote"\\line\nend'}) + + assert '[name="päss"], [id="päss"]' in selectors + assert ( + '[name="quote\\"\\\\line\\a end"], [id="quote\\"\\\\line\\a end"]' in selectors + ) + with pytest.raises(BrowserAttachError, match="null character"): + _secret_screenshot_selectors({"unsafe\x00field"}) + with pytest.raises(BrowserAttachError, match="Unicode surrogate"): + _secret_screenshot_selectors({"unsafe\ud800field"}) + + def test_attached_recorder_api_refuses_incompatible_options(tmp_path: Path) -> None: with pytest.raises(BrowserAttachError, match="requires a browser CDP"): InteractiveRecorder( @@ -362,6 +391,71 @@ def finish(self): assert not list(tmp_path.glob(".openadapt-recording-partial-*")) +def test_attached_popup_during_finalization_discards_metadata( + tmp_path: Path, +) -> None: + out = tmp_path / "recording" + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + session._prepare_recording_dir() + + class PopupPage: + url = "https://app.example.test/" + frames: list = [] + + def __init__(self) -> None: + self.main_frame = SimpleNamespace(url=self.url) + + def evaluate(self, _script): + return {"width": 1280, "height": 800, "dpr": 1} + + def remove_listener(self, event, listener): + if event == "popup": + listener(SimpleNamespace(url="about:blank")) + + session.page = PopupPage() + session._page_lifecycle_listeners_installed = True + session._attached_geometry = (1280, 800, 1.0) + session._initial_attached_viewport = (1280, 800) + + class FinalizingRecorder: + def finish(self): + assert session._recording_dir is not None + (session._recording_dir / "meta.json").write_text( + json.dumps({"viewport": [1280, 800]}) + ) + return session._recording_dir + + session.recorder = FinalizingRecorder() # type: ignore[assignment] + + with pytest.raises(BrowserAttachError, match="popup or new tab"): + session.finish() + assert not out.exists() + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + +def test_attached_recorder_refuses_a_new_context_page(tmp_path: Path) -> None: + session = InteractiveRecorder( + "https://app.example.test/", + tmp_path / "recording", + cdp_endpoint="http://127.0.0.1:9222", + ) + selected = SimpleNamespace() + context = SimpleNamespace(pages=[selected]) + selected.context = context + session.page = selected + session._context_pages_at_start = (selected,) + + context.pages.append(SimpleNamespace(context=context)) + + with pytest.raises(BrowserAttachError, match="popup or new tab"): + session._assert_no_new_pages() + assert session.done is True + + def test_attached_recorder_refuses_iframe_events(tmp_path: Path) -> None: session = InteractiveRecorder( "https://app.example.test/", @@ -521,8 +615,17 @@ def test_cli_refuses_headless_attachment(tmp_path: Path) -> None: + + - + + """ @@ -688,6 +791,139 @@ def drive(page, pump, *, secret_value=secret, trial_number=trial): with urlopen(f"{endpoint}/json/version", timeout=2) as response: assert response.status == 200 + unicode_secret = "INTERNATIONAL-SECRET-NEVER-PERSIST" + unicode_secret_rect: dict[str, int] = {} + + def record_unicode_secret(page, pump): + field = page.locator('[name="päss"]') + field.fill("") + box = field.bounding_box() + assert box is not None + unicode_secret_rect.update( + x=round(box["x"]), + y=round(box["y"]), + width=round(box["width"]), + height=round(box["height"]), + ) + field.click() + page.keyboard.type(unicode_secret) + pump() + pump() + page.click("#save") + pump() + pump() + + unicode_recording = record_interactive( + attach_app_url, + tmp_path / "recording-unicode-secret", + secret_fields=("päss",), + cdp_endpoint=endpoint, + script=record_unicode_secret, + ) + unicode_events_text = (unicode_recording / "events.jsonl").read_text() + unicode_meta_text = (unicode_recording / "meta.json").read_text() + assert unicode_secret not in unicode_events_text + assert unicode_secret not in unicode_meta_text + unicode_events = [json.loads(line) for line in unicode_events_text.splitlines()] + unicode_event = next(event for event in unicode_events if event.get("secret")) + unicode_next_before = Image.open( + unicode_recording + / "frames" + / f"{int(unicode_event['i']) + 1:04d}_before.png" + ).convert("RGB") + unicode_crop = unicode_next_before.crop( + ( + unicode_secret_rect["x"], + unicode_secret_rect["y"], + unicode_secret_rect["x"] + unicode_secret_rect["width"], + unicode_secret_rect["y"] + unicode_secret_rect["height"], + ) + ) + assert all(extrema == (0, 0) for extrema in unicode_crop.getextrema()) + assert process.poll() is None + + child_secret_rect: dict[str, int] = {} + + def retain_top_level_action_with_child_secret(page, pump): + child_secret = page.frame_locator("#child").locator("#frame-password") + box = child_secret.bounding_box() + assert box is not None + child_secret_rect.update( + x=round(box["x"]), + y=round(box["y"]), + width=round(box["width"]), + height=round(box["height"]), + ) + page.click("#note") + pump() + pump() + + child_secret_recording = record_interactive( + attach_app_url, + tmp_path / "recording-child-frame-secret", + cdp_endpoint=endpoint, + script=retain_top_level_action_with_child_secret, + ) + child_before = Image.open( + child_secret_recording / "frames" / "0000_before.png" + ).convert("RGB") + child_crop = child_before.crop( + ( + child_secret_rect["x"], + child_secret_rect["y"], + child_secret_rect["x"] + child_secret_rect["width"], + child_secret_rect["y"] + child_secret_rect["height"], + ) + ) + assert all(extrema == (0, 0) for extrema in child_crop.getextrema()) + assert process.poll() is None + + popup_recording = tmp_path / "recording-popup-refusal" + + def open_popup(page, pump): + with page.expect_popup() as popup_info: + page.click("#open-popup") + popup_info.value.wait_for_load_state() + pump() + + with pytest.raises(BrowserAttachError, match="popup or new tab"): + record_interactive( + attach_app_url, + popup_recording, + cdp_endpoint=endpoint, + script=open_popup, + ) + assert not (popup_recording / "meta.json").exists() + assert process.poll() is None + with urlopen(f"{endpoint}/json/list", timeout=2) as response: + popup_targets = json.load(response) + assert any(target.get("url") == "about:blank" for target in popup_targets) + + popup_activity_recording = tmp_path / "recording-popup-activity-refusal" + + def act_inside_popup(page, pump): + with page.expect_popup() as popup_info: + page.click("#open-popup") + popup = popup_info.value + popup.set_content( + "" + ) + popup.fill("#popup-note", "activity-that-must-not-disappear") + popup.click("#popup-save") + pump() + + with pytest.raises(BrowserAttachError, match="popup or new tab"): + record_interactive( + attach_app_url, + popup_activity_recording, + cdp_endpoint=endpoint, + script=act_inside_popup, + ) + assert not (popup_activity_recording / "meta.json").exists() + assert process.poll() is None + with urlopen(f"{endpoint}/json/version", timeout=2) as response: + assert response.status == 200 + iframe_recording = tmp_path / "recording-iframe-refusal" def click_inside_existing_iframe(page, pump): From b2931e8c9408ae43c46fce51f65de526fd4ab16c Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 13:59:50 -0400 Subject: [PATCH 06/30] fix(browser): latch attached recording races --- docs/BROWSER_RECORDING.md | 32 +- openadapt_flow/backends/playwright_backend.py | 79 ++- openadapt_flow/interactive_recorder.py | 291 +++++++++-- tests/test_browser_attach.py | 458 +++++++++++++++++- 4 files changed, 804 insertions(+), 56 deletions(-) diff --git a/docs/BROWSER_RECORDING.md b/docs/BROWSER_RECORDING.md index aa1c803d..3d937278 100644 --- a/docs/BROWSER_RECORDING.md +++ b/docs/BROWSER_RECORDING.md @@ -105,9 +105,11 @@ sensitive recording data. cross-origin navigation stops the recording and does not produce complete metadata. - Do not open a popup or a new tab in the selected tab's browser context while - recording. Flow currently binds one page. It refuses a new page instead of - omitting actions performed there. A refusal leaves the external browser and - its tabs open. + recording. Flow currently binds one page. A context-level listener records + every new-page signal, including a tab that acts and closes between recorder + polls. Flow keeps this refusal active through the final Playwright detach so + a late tab cannot produce complete metadata. A refusal leaves the external + browser and its tabs open. - Attach mode does not combine with `--headless`. The external browser owns its display mode. - The `--out` path must not exist. Flow writes to a new temporary sibling and @@ -115,6 +117,11 @@ sensitive recording data. removes the temporary output and does not change an existing recording. - Input event payloads carry a unique recording-session binding and have a 1 MB limit. Flow removes the current document listeners when it detaches. +- Flow retains one exact frame boundary per logical action. It coalesces only + consecutive input changes from the same bound field session or consecutive + scroll deltas from one observed gesture. If two distinct actions arrive in + one recorder poll, Flow refuses the recording before it captures a shared, + incorrect after-frame. - Attached screenshots use CSS pixels and the actual live viewport. Thus, DOM coordinates and retained frame coordinates stay aligned on high-density displays. @@ -128,10 +135,21 @@ sensitive recording data. so the recording stops without complete metadata. Stop interacting for a moment after a resize. Recording then continues automatically. - `input[type=password]` and fields declared with `--secret FIELD` never send - their values to Python. Flow redacts their field rectangle in the retained - evidence frames. Other typed values and visible page content are recording - evidence and can contain sensitive data. Keep raw recordings inside the - approved local boundary. + their values to Python. Flow binds a private input-session identity and a + temporary screenshot-mask marker when a declared secret field receives + focus. The identity remains secret if application code changes the field name + or ID, or replaces the active input element during the same input session. + Flow removes the marker when it detaches. Other typed values and visible page + content are recording evidence and can contain sensitive data. Keep raw + recordings inside the approved local boundary. +- Secret masks cover every frame in the selected page. Before each masked + screenshot, Flow snapshots the frame inventory and its lifecycle generation. + It accepts the in-memory image only when the inventory stays unchanged + through capture. It retries a bounded number of times and refuses persistent + frame churn. A discarded unstable image never reaches disk or metadata. +- The close, new-page, origin, and frame lifecycle guards stay active until + Flow detaches from the external browser. Flow promotes the temporary output + only after detach succeeds and every guard remains clear. ## Why the Capture Chrome extension is not this path diff --git a/openadapt_flow/backends/playwright_backend.py b/openadapt_flow/backends/playwright_backend.py index f22673cf..2ece800a 100644 --- a/openadapt_flow/backends/playwright_backend.py +++ b/openadapt_flow/backends/playwright_backend.py @@ -33,6 +33,7 @@ ) VIEWPORT: tuple[int, int] = (1280, 800) +_MASKED_SCREENSHOT_ATTEMPTS = 3 _MODIFIER_ALIASES = { "meta": "Meta", @@ -88,6 +89,10 @@ ) +class ScreenshotMaskStabilityError(RuntimeError): + """The browser frame tree changed across every masked screenshot attempt.""" + + @dataclass class _StructuralGuard: """Private one-shot binding retaining only token, scope, and frame selectors.""" @@ -730,6 +735,13 @@ def __init__( self.page = page self._screenshot_scale = screenshot_scale self._screenshot_mask_selectors = screenshot_mask_selectors + self._screenshot_frame_generation = 0 + self._screenshot_frame_listener = self._handle_screenshot_frame_lifecycle + self._screenshot_frame_tracking = False + if self._screenshot_mask_selectors: + for event in ("frameattached", "framedetached", "framenavigated"): + self.page.on(event, self._screenshot_frame_listener) + self._screenshot_frame_tracking = True # Opaque per-backend key keeps the WeakMap private from ordinary page # code. Python retains only token material keyed by the public # SHA-256 fingerprint; target/row text stays page-local and ephemeral. @@ -2233,24 +2245,69 @@ def type_text_guarded( deliver=lambda locator: locator.press_sequentially(text, timeout=1000), ) + def _handle_screenshot_frame_lifecycle(self, _frame: Any = None) -> None: + """Advance the irreversible frame-tree generation.""" + + self._screenshot_frame_generation += 1 + + def stop_screenshot_mask_tracking(self) -> None: + """Remove recording-only frame listeners from an external page.""" + + if not self._screenshot_frame_tracking: + return + for event in ("frameattached", "framedetached", "framenavigated"): + try: + self.page.remove_listener(event, self._screenshot_frame_listener) + except Exception: + pass + self._screenshot_frame_tracking = False + + @staticmethod + def _same_frames(left: tuple[Any, ...], right: tuple[Any, ...]) -> bool: + return len(left) == len(right) and all( + before is after for before, after in zip(left, right) + ) + def screenshot(self) -> bytes: - """Return the current full-viewport frame as PNG bytes.""" - options: dict[str, Any] = {} + """Return a stable current full-viewport frame as PNG bytes.""" + base_options: dict[str, Any] = {} if self._screenshot_scale == "css": - options["scale"] = "css" - if self._screenshot_mask_selectors: - # A Page locator does not cross an iframe boundary. Rebuild the - # masks from the live frame inventory for every capture so both - # existing and newly attached frames use the same secret contract. - # If a frame detaches while Playwright resolves these locators, the - # screenshot fails closed instead of retaining an unmasked frame. + base_options["scale"] = "css" + if not self._screenshot_mask_selectors: + return self.page.screenshot(type="png", full_page=False, **base_options) + + for _attempt in range(_MASKED_SCREENSHOT_ATTEMPTS): + generation = self._screenshot_frame_generation + frames = tuple(self.page.frames) + if generation != self._screenshot_frame_generation: + continue + options = dict(base_options) options["mask"] = [ frame.locator(selector) - for frame in list(self.page.frames) + for frame in frames for selector in self._screenshot_mask_selectors ] options["mask_color"] = "#000000" - return self.page.screenshot(type="png", full_page=False, **options) + try: + png = self.page.screenshot(type="png", full_page=False, **options) + # Flush lifecycle events that Chromium sent with or before the + # screenshot response before accepting the in-memory bytes. + self.page.evaluate("() => null") + except Exception: + if generation != self._screenshot_frame_generation: + continue + raise + current_frames = tuple(self.page.frames) + if generation == self._screenshot_frame_generation and self._same_frames( + frames, current_frames + ): + return png + # ``png`` is intentionally discarded here. It never reaches the + # recorder, disk, or a compiled bundle. + raise ScreenshotMaskStabilityError( + "the browser frame tree changed during every secret-masked " + "screenshot attempt; recording was refused" + ) def click(self, x: int, y: int, *, double: bool = False) -> None: """Click (or double-click) at pixel coordinates via the mouse.""" diff --git a/openadapt_flow/interactive_recorder.py b/openadapt_flow/interactive_recorder.py index 37bc031a..fa828cf1 100644 --- a/openadapt_flow/interactive_recorder.py +++ b/openadapt_flow/interactive_recorder.py @@ -46,6 +46,7 @@ import ipaddress import json import os +import re import shutil import sys import tempfile @@ -151,13 +152,21 @@ def _rename_directory_noreplace(source: Path, destination: Path) -> None: raise OSError(error_number, os.strerror(error_number), destination) -def _secret_screenshot_selectors(secret_fields: set[str]) -> tuple[str, ...]: +def _secret_screenshot_selectors( + secret_fields: set[str], + *, + marker_attribute: Optional[str] = None, +) -> tuple[str, ...]: """Return selectors that mask password and declared-secret fields.""" selectors = ["input[type='password']"] for field in sorted(secret_fields): encoded = _css_string_literal(field) selectors.append(f"[name={encoded}], [id={encoded}]") + if marker_attribute is not None: + if not re.fullmatch(r"data-oaflow-secret-[0-9a-f]{32}", marker_attribute): + raise BrowserAttachError("the browser secret marker is invalid") + selectors.append(f"[{marker_attribute}]") return tuple(selectors) @@ -350,10 +359,28 @@ def select_attached_page( try { previous.cleanup(); } catch (e) {} } const SECRET_NAMES = __SECRET_NAMES__; + const SECRET_MARKER = __SECRET_MARKER__; const IDENT_NAMES = __IDENT_NAMES__; const SPECIAL = __SPECIAL_KEYS__; const listeners = []; + const secretStates = new WeakMap(); + const inputSessions = new WeakMap(); + const stickySecretElements = new Set(); + let nextInputSession = 0; + let activeSecretElement = null; + let activeSecretState = null; let resizeTimer = null; + const secretObserver = new MutationObserver(() => { + for (const el of stickySecretElements) { + if (!el.isConnected) continue; + try { + if (!el.hasAttribute(SECRET_MARKER)) el.setAttribute(SECRET_MARKER, ''); + } catch (e) {} + } + }); + secretObserver.observe(document.documentElement || document, { + attributes: true, childList: true, subtree: true, + }); function listenOn(target, type, handler) { target.addEventListener(type, handler, true); @@ -366,6 +393,7 @@ def select_attached_page( function cleanup() { if (resizeTimer !== null) clearTimeout(resizeTimer); + secretObserver.disconnect(); for (const [target, type, handler] of listeners) { try { target.removeEventListener(type, handler, true); } catch (e) {} } @@ -499,11 +527,57 @@ def select_attached_page( } catch (e) { return null; } } - function isSecretEl(el) { - if (!el) return false; - if ((el.type || '').toLowerCase() === 'password') return true; + function isTextEntry(el) { + try { + return !!el && !!el.matches && el.matches( + 'input, textarea, [contenteditable=""], [contenteditable="true"],' + + ' [role="textbox"]' + ); + } catch (e) { return false; } + } + + function bindSecretState(el, state) { + secretStates.set(el, state); + stickySecretElements.add(el); + activeSecretElement = el; + activeSecretState = state; + try { + el.setAttribute(SECRET_MARKER, ''); + return el.hasAttribute(SECRET_MARKER); + } catch (e) { return false; } + } + + function inputSessionFor(el) { + let session = inputSessions.get(el) || null; + if (!session) { + nextInputSession += 1; + session = SESSION_ID + ':input:' + String(nextInputSession); + inputSessions.set(el, session); + } + return session; + } + + function declaredSecretState(el) { + if (!el) return null; const n = el.name || '', i = el.id || ''; - return SECRET_NAMES.indexOf(n) >= 0 || SECRET_NAMES.indexOf(i) >= 0; + if ((el.type || '').toLowerCase() !== 'password' + && SECRET_NAMES.indexOf(n) < 0 && SECRET_NAMES.indexOf(i) < 0) { + return null; + } + return {field: n || i || null, inputSession: inputSessionFor(el)}; + } + + function secretStateForInput(el) { + let state = secretStates.get(el) || null; + if (!state) state = declaredSecretState(el); + if (!state && activeSecretState && activeSecretElement + && !activeSecretElement.isConnected && isTextEntry(el)) { + // Some controlled inputs replace their DOM element after each change. + // Programmatic focus transfer is still the same input session. + state = activeSecretState; + } + if (!state) return null; + return {state, maskBound: bindSecretState(el, state)}; } function fieldLabel(el) { @@ -580,8 +654,36 @@ def select_attached_page( emit({kind: 'viewport', url: location.href, title: document.title}); }, 100); }); + listen('focusin', (e) => { + const el = e.target; + const declared = declaredSecretState(el); + if (declared) { + // Bind before the first key event. Application code can remove a + // declared name/id during keydown, beforeinput, or input dispatch. + bindSecretState(el, declared); + return; + } + if (!activeSecretState || el === activeSecretElement) return; + const retained = secretStates.get(el) || null; + if (retained) { + bindSecretState(el, retained); + } else if (activeSecretElement && !activeSecretElement.isConnected + && isTextEntry(el)) { + bindSecretState(el, activeSecretState); + } else { + activeSecretElement = null; + activeSecretState = null; + } + }); listen('pointerdown', (e) => { if (e.button !== 0) return; + if (activeSecretState && e.target !== activeSecretElement + && !secretStates.has(e.target)) { + // An explicit operator action on another target ends the sticky input + // session. Programmatic replacement without a pointer action does not. + activeSecretElement = null; + activeSecretState = null; + } pointerDown = { x: Math.round(e.clientX), y: Math.round(e.clientY), sid: structuredIdentity(e.clientX, e.clientY), @@ -632,20 +734,27 @@ def select_attached_page( listen('input', (e) => { const el = e.target; - const secret = isSecretEl(el); + const secretBinding = secretStateForInput(el); + const secret = secretBinding !== null; const r = (el.getBoundingClientRect && el.getBoundingClientRect()) || { left: 0, top: 0, width: 0, height: 0 }; const o = { kind: 'input', - field: el.name || el.id || null, + field: secret ? secretBinding.state.field : (el.name || el.id || null), label: fieldLabel(el), secret: secret, + __oaflow_input_session: secret + ? secretBinding.state.inputSession : inputSessionFor(el), rect: [Math.round(r.left), Math.round(r.top), Math.round(r.width), Math.round(r.height)], url: location.href, title: document.title, }; // The literal value of a SECRET field is never read or transmitted. - if (!secret) o.value = (el.value != null ? String(el.value) : ''); + if (secret) { + o.__oaflow_secret_mask_bound = secretBinding.maskBound; + } else { + o.value = (el.value != null ? String(el.value) : ''); + } emit(o); }); @@ -733,6 +842,7 @@ def __init__( self._owns_browser = self._cdp_endpoint is None self._session_id = uuid.uuid4().hex self._binding_name = f"__oaflow_emit_{self._session_id}" + self._secret_marker_attribute = f"data-oaflow-secret-{self._session_id}" self._poll_ms = poll_ms self._viewport = viewport # Recording-only, read-only observation. This does not add an effect @@ -759,9 +869,15 @@ def __init__( self.page = None self._page_close_listener = self._handle_page_close self._frame_navigation_listener = self._handle_frame_navigation + self._frame_attached_listener = self._handle_frame_tree_change + self._frame_detached_listener = self._handle_frame_tree_change self._popup_listener = self._handle_popup + self._context_page_listener = self._handle_context_page self._page_lifecycle_listeners_installed = False + self._context_page_listener_installed = False + self._context = None self._context_pages_at_start: tuple[Any, ...] = () + self._finalizing = False self.backend: Optional[PlaywrightBackend] = None self.recorder: Optional[Recorder] = None self._last_frame: bytes = b"" @@ -811,9 +927,16 @@ def start(self) -> None: page_url=self._browser_page_url, ) - self._context_pages_at_start = tuple(self.page.context.pages) + self._context = self.page.context + self._context.on("page", self._context_page_listener) + self._context_page_listener_installed = True + self._context_pages_at_start = tuple(self._context.pages) + if self._listener_error is not None: + raise self._listener_error self.page.on("close", self._page_close_listener) self.page.on("framenavigated", self._frame_navigation_listener) + self.page.on("frameattached", self._frame_attached_listener) + self.page.on("framedetached", self._frame_detached_listener) self.page.on("popup", self._popup_listener) self._page_lifecycle_listeners_installed = True self.page.expose_binding( @@ -827,6 +950,10 @@ def start(self) -> None: _INIT_JS.replace("__SESSION_ID__", json.dumps(self._session_id)) .replace("__BINDING_NAME__", json.dumps(self._binding_name)) .replace("__SECRET_NAMES__", json.dumps(sorted(self._secret_fields))) + .replace( + "__SECRET_MARKER__", + json.dumps(self._secret_marker_attribute), + ) .replace( "__IDENT_NAMES__", json.dumps(sorted(self._identifier_fields)), @@ -865,7 +992,8 @@ def start(self) -> None: self.page, screenshot_scale="device" if self._owns_browser else "css", screenshot_mask_selectors=_secret_screenshot_selectors( - self._secret_fields + self._secret_fields, + marker_attribute=self._secret_marker_attribute, ), ) assert self._recording_dir is not None @@ -935,6 +1063,7 @@ def finish(self) -> Path: try: self._cleanup_page_listeners() self._drain_event_queue() + self._finalizing = True self._flush_type() self._flush_scroll() assert self.recorder is not None @@ -1042,6 +1171,8 @@ def _handle_frame_navigation(self, frame: Any) -> None: return try: if frame is not self.page.main_frame: + if self._finalizing: + self._retain_late_frame_error() return current_origin = _http_origin( str(frame.url), @@ -1049,13 +1180,32 @@ def _handle_frame_navigation(self, frame: Any) -> None: ) except Exception: current_origin = None - if current_origin != self._attached_origin: + if self._finalizing or current_origin != self._attached_origin: self._listener_error = BrowserAttachError( - "the selected browser tab left the declared application origin; " - "recording was refused" + "the selected browser tab changed frame state after Flow " + "retained its final evidence; recording was refused" + if self._finalizing + else "the selected browser tab left the declared application " + "origin; recording was refused" ) self.done = True + def _retain_late_frame_error(self) -> None: + """Retain one refusal for a post-snapshot frame-tree change.""" + + if self._listener_error is None: + self._listener_error = BrowserAttachError( + "the selected browser tab changed frame state after Flow " + "retained its final evidence; recording was refused" + ) + self.done = True + + def _handle_frame_tree_change(self, _frame: Any = None) -> None: + """Refuse a frame attach/detach after the final evidence snapshot.""" + + if not self._owns_browser and self._finalizing: + self._retain_late_frame_error() + def _handle_popup(self, _popup: Any = None) -> None: """Refuse a second page that the selected recording tab opens.""" @@ -1067,6 +1217,17 @@ def _handle_popup(self, _popup: Any = None) -> None: "publishing incomplete metadata" ) + def _handle_context_page(self, _page: Any = None) -> None: + """Irreversibly refuse any page created after context binding.""" + + self.done = True + if self._listener_error is None: + self._listener_error = BrowserAttachError( + "the selected browser context opened a popup or new tab; this " + "recording is bound to its accepted page baseline, so Flow " + "stopped before publishing incomplete metadata" + ) + def _assert_no_new_pages(self) -> None: """Retain a refusal if this recording context gained another page.""" @@ -1102,6 +1263,33 @@ def _enqueue_browser_event( if event.pop("__oaflow_session", None) != self._session_id: return kind = event.get("kind") + secret_mask_bound = event.pop("__oaflow_secret_mask_bound", None) + if ( + kind == "input" + and bool(event.get("secret")) + and secret_mask_bound is not True + ): + self._listener_error = BrowserAttachError( + "a secret input could not retain its screenshot mask identity; " + "recording stopped before accepting the event" + ) + self.done = True + return + raw_input_session = event.pop("__oaflow_input_session", None) + if kind == "input": + expected_prefix = f"{self._session_id}:input:" + if ( + not isinstance(raw_input_session, str) + or not raw_input_session.startswith(expected_prefix) + or not raw_input_session.removeprefix(expected_prefix).isdigit() + ): + self._listener_error = BrowserAttachError( + "the browser emitted an input without a valid bound field " + "session; recording stopped before accepting the event" + ) + self.done = True + return + event["_oaflow_input_session"] = raw_input_session reported_top_level = bool(event.pop("__oaflow_top_level", True)) source_is_selected_top_level = reported_top_level if source is not None: @@ -1212,29 +1400,53 @@ def _cleanup_page_listeners(self) -> None: except Exception: continue + def _cleanup_secret_markers(self) -> None: + """Remove this session's temporary secret-mask attributes.""" + + if self.page is None: + return + try: + frames = list(self.page.frames) + except Exception: + frames = [] + for frame in frames: + try: + frame.evaluate( + """marker => { + for (const element of document.querySelectorAll('*')) { + if (element.hasAttribute(marker)) { + element.removeAttribute(marker); + } + } + }""", + self._secret_marker_attribute, + ) + except Exception: + continue + def _stop_browser_connection(self) -> None: """Close an owned browser or detach without closing an external one.""" self._cleanup_page_listeners() - if self.page is not None and self._page_lifecycle_listeners_installed: - for event, listener in ( - ("close", self._page_close_listener), - ("framenavigated", self._frame_navigation_listener), - ("popup", self._popup_listener), - ): - try: - self.page.remove_listener(event, listener) - except Exception: - pass - self._page_lifecycle_listeners_installed = False + self._cleanup_secret_markers() browser, self._browser = self._browser, None playwright, self._pw = self._pw, None try: if self._owns_browser and browser is not None: browser.close() finally: - if playwright is not None: - playwright.stop() + try: + if playwright is not None: + playwright.stop() + finally: + # Keep all local lifecycle latches active until the external + # Playwright connection has detached. They do not remain in + # Chromium after the connection closes. + if self.backend is not None: + self.backend.stop_screenshot_mask_tracking() + self._page_lifecycle_listeners_installed = False + self._context_page_listener_installed = False + self._context = None def _drain_event_queue(self) -> bool: """Process all events already delivered by the page binding.""" @@ -1244,6 +1456,7 @@ def _drain_event_queue(self) -> bool: self._assert_no_new_pages() batch = self._pyq[:] del self._pyq[:] + self._validate_event_batch(batch) rebased = False if not self._owns_browser: current_geometry = self._read_attached_geometry() @@ -1280,6 +1493,25 @@ def _drain_event_queue(self) -> bool: raise self._listener_error return bool(batch) or rebased + @staticmethod + def _validate_event_batch(batch: list[dict[str, Any]]) -> None: + """Refuse a batch that lacks an exact frame between logical actions.""" + + if len(batch) <= 1: + return + kinds = {event.get("kind") for event in batch} + if kinds == {"scroll"}: + return + if kinds == {"input"}: + sessions = {event.get("_oaflow_input_session") for event in batch} + fields = {event.get("field") for event in batch} + if len(sessions) == 1 and None not in sessions and len(fields) == 1: + return + raise BrowserAttachError( + "more than one logical browser action arrived before Flow could " + "retain an exact intermediate frame; recording was refused" + ) + # -- event pump ---------------------------------------------------------- def pump(self) -> bool: @@ -1327,11 +1559,16 @@ def _process(self, ev: dict[str, Any]) -> None: def _accumulate_input(self, ev: dict[str, Any]) -> None: field = ev.get("field") - if self._pending_type is not None and self._pending_type.get("field") != field: + input_session = ev.get("_oaflow_input_session") + if self._pending_type is not None and ( + self._pending_type.get("field") != field + or self._pending_type.get("input_session") != input_session + ): self._flush_type() # focus moved to a different field if self._pending_type is None: self._pending_type = { "field": field, + "input_session": input_session, "label": ev.get("label"), "secret": bool(ev.get("secret")), "value": "", diff --git a/tests/test_browser_attach.py b/tests/test_browser_attach.py index 3a121885..914f9e3c 100644 --- a/tests/test_browser_attach.py +++ b/tests/test_browser_attach.py @@ -9,6 +9,7 @@ import threading import time from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer +from io import BytesIO from pathlib import Path from types import SimpleNamespace from urllib.request import urlopen @@ -17,7 +18,10 @@ from PIL import Image from openadapt_flow.__main__ import main -from openadapt_flow.backends.playwright_backend import PlaywrightBackend +from openadapt_flow.backends.playwright_backend import ( + PlaywrightBackend, + ScreenshotMaskStabilityError, +) from openadapt_flow.compiler import compile_recording from openadapt_flow.interactive_recorder import ( BrowserAttachError, @@ -156,9 +160,26 @@ class Page: def __init__(self) -> None: self.screenshot_options: list[dict] = [] self.frames = [Frame("main"), Frame("child")] + self.listeners: dict[str, list] = {} + self.attach_on_next_screenshot = True + + def on(self, event, listener): + self.listeners.setdefault(event, []).append(listener) + + def remove_listener(self, event, listener): + self.listeners[event].remove(listener) + + def evaluate(self, _script): + return None def screenshot(self, **kwargs): self.screenshot_options.append(kwargs) + if self.attach_on_next_screenshot: + self.attach_on_next_screenshot = False + frame = Frame("late-child") + self.frames.append(frame) + for listener in self.listeners.get("frameattached", []): + listener(frame) return b"png" selectors = ( @@ -172,9 +193,8 @@ def screenshot(self, **kwargs): ) assert backend.screenshot() == b"png" - page.frames.append(Frame("late-child")) assert backend.screenshot() == b"png" - assert len(page.screenshot_options) == 2 + assert len(page.screenshot_options) == 3 assert page.screenshot_options[0]["mask"] == [ f"locator:{frame}:{selector}" for frame in ("main", "child") @@ -185,8 +205,11 @@ def screenshot(self, **kwargs): for frame in ("main", "child", "late-child") for selector in selectors ] + assert page.screenshot_options[2]["mask"] == page.screenshot_options[1]["mask"] for options in page.screenshot_options: assert options["mask_color"] == "#000000" + backend.stop_screenshot_mask_tracking() + assert not any(page.listeners.values()) def test_declared_secret_selectors_use_css_string_escaping() -> None: @@ -366,10 +389,6 @@ def __init__(self) -> None: def evaluate(self, _script): return {"width": 1280, "height": 800, "dpr": 1} - def remove_listener(self, event, listener): - if event == "close": - listener() - session.page = ClosingPage() session._page_lifecycle_listeners_installed = True session._attached_geometry = (1280, 800, 1.0) @@ -384,6 +403,7 @@ def finish(self): return session._recording_dir session.recorder = ClosingRecorder() # type: ignore[assignment] + session._pw = SimpleNamespace(stop=lambda: session._handle_page_close()) with pytest.raises(BrowserAttachError, match="selected browser tab closed"): session.finish() @@ -412,10 +432,6 @@ def __init__(self) -> None: def evaluate(self, _script): return {"width": 1280, "height": 800, "dpr": 1} - def remove_listener(self, event, listener): - if event == "popup": - listener(SimpleNamespace(url="about:blank")) - session.page = PopupPage() session._page_lifecycle_listeners_installed = True session._attached_geometry = (1280, 800, 1.0) @@ -430,6 +446,9 @@ def finish(self): return session._recording_dir session.recorder = FinalizingRecorder() # type: ignore[assignment] + session._pw = SimpleNamespace( + stop=lambda: session._handle_popup(SimpleNamespace(url="about:blank")) + ) with pytest.raises(BrowserAttachError, match="popup or new tab"): session.finish() @@ -437,6 +456,61 @@ def finish(self): assert not list(tmp_path.glob(".openadapt-recording-partial-*")) +@pytest.mark.parametrize("late_event", ["context_page", "origin", "frame"]) +def test_attached_late_lifecycle_event_discards_metadata( + tmp_path: Path, + late_event: str, +) -> None: + out = tmp_path / f"recording-{late_event}" + session = InteractiveRecorder( + "https://app.example.test/", + out, + cdp_endpoint="http://127.0.0.1:9222", + ) + session._prepare_recording_dir() + + class LifecyclePage: + url = "https://app.example.test/" + frames: list = [] + + def __init__(self) -> None: + self.main_frame = SimpleNamespace(url=self.url) + + def evaluate(self, _script): + return {"width": 1280, "height": 800, "dpr": 1} + + session.page = LifecyclePage() + session._page_lifecycle_listeners_installed = True + session._attached_geometry = (1280, 800, 1.0) + session._initial_attached_viewport = (1280, 800) + + class FinalizingRecorder: + def finish(self): + assert session._recording_dir is not None + (session._recording_dir / "meta.json").write_text( + json.dumps({"viewport": [1280, 800]}) + ) + return session._recording_dir + + session.recorder = FinalizingRecorder() # type: ignore[assignment] + + def emit_late_event() -> None: + if late_event == "context_page": + session._handle_context_page(SimpleNamespace(url="about:blank")) + elif late_event == "origin": + session.page.main_frame.url = "https://other.example.test/" + session._handle_frame_navigation(session.page.main_frame) + else: + session._handle_frame_tree_change(SimpleNamespace()) + + session._pw = SimpleNamespace(stop=emit_late_event) + + with pytest.raises(BrowserAttachError): + session.finish() + assert not out.exists() + assert not list(tmp_path.glob(".openadapt-recording-partial-*")) + + def test_attached_recorder_refuses_a_new_context_page(tmp_path: Path) -> None: session = InteractiveRecorder( "https://app.example.test/", @@ -522,6 +596,48 @@ def test_attached_recorder_refuses_invalid_viewport_evidence(tmp_path: Path) -> assert "invalid viewport evidence" in str(session._listener_error) +@pytest.mark.parametrize( + "batch", + [ + [{"kind": "click"}, {"kind": "click"}], + [ + {"kind": "input", "field": "note", "_oaflow_input_session": "a"}, + {"kind": "click"}, + ], + [ + {"kind": "input", "field": "note", "_oaflow_input_session": "a"}, + {"kind": "key", "key": "Enter"}, + ], + [{"kind": "scroll", "dy": 10}, {"kind": "click"}], + [ + {"kind": "input", "field": "note", "_oaflow_input_session": "a"}, + {"kind": "input", "field": "note", "_oaflow_input_session": "b"}, + ], + ], +) +def test_browser_event_batch_refuses_multiple_logical_actions( + batch: list[dict], +) -> None: + with pytest.raises(BrowserAttachError, match="more than one logical"): + InteractiveRecorder._validate_event_batch(batch) + + +@pytest.mark.parametrize( + "batch", + [ + [ + {"kind": "input", "field": "note", "_oaflow_input_session": "a"}, + {"kind": "input", "field": "note", "_oaflow_input_session": "a"}, + ], + [{"kind": "scroll", "dy": 10}, {"kind": "scroll", "dy": 20}], + ], +) +def test_browser_event_batch_preserves_one_coalescible_action( + batch: list[dict], +) -> None: + InteractiveRecorder._validate_event_batch(batch) + + def test_cli_threads_attach_contract_to_the_recorder( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: @@ -617,6 +733,19 @@ def test_cli_refuses_headless_attachment(tmp_path: Path) -> None: + + + + " + ) + temporary.fill("#short-note", "activity-that-must-not-disappear") + temporary.click("#short-save") + temporary.close() + assert len(page.context.pages) >= 1 + pump() + + with pytest.raises(BrowserAttachError, match="popup or new tab"): + record_interactive( + attach_app_url, + short_page_recording, + cdp_endpoint=endpoint, + script=act_in_short_lived_context_page, + ) + assert not (short_page_recording / "meta.json").exists() + assert process.poll() is None + with urlopen(f"{endpoint}/json/version", timeout=2) as response: + assert response.status == 200 + + late_page_recording = tmp_path / "recording-late-page-refusal" + late_page_session = InteractiveRecorder( + attach_app_url, + late_page_recording, + cdp_endpoint=endpoint, + ) + late_page_session.start() + assert late_page_session.page is not None + assert late_page_session._pw is not None + original_playwright_stop = late_page_session._pw.stop + + def stop_after_short_lived_page() -> None: + temporary = late_page_session.page.context.new_page() + temporary.set_content( + "" + ) + temporary.fill("#late-note", "late-activity-must-not-disappear") + temporary.click("#late-save") + temporary.close() + original_playwright_stop() + + monkeypatch.setattr(late_page_session._pw, "stop", stop_after_short_lived_page) + with pytest.raises(BrowserAttachError, match="popup or new tab"): + late_page_session.finish() + assert not (late_page_recording / "meta.json").exists() + assert process.poll() is None + with urlopen(f"{endpoint}/json/version", timeout=2) as response: + assert response.status == 200 + iframe_recording = tmp_path / "recording-iframe-refusal" def click_inside_existing_iframe(page, pump): From 63bfc999b122702c17a523b3b5be675bf88074ea Mon Sep 17 00:00:00 2001 From: Richard Abrich Date: Tue, 18 Aug 2026 14:15:26 -0400 Subject: [PATCH 07/30] fix(browser): close attached finalization gaps --- docs/BROWSER_RECORDING.md | 26 ++-- openadapt_flow/interactive_recorder.py | 180 ++++++++++++++++++++++--- tests/test_browser_attach.py | 140 ++++++++++++++++++- 3 files changed, 314 insertions(+), 32 deletions(-) diff --git a/docs/BROWSER_RECORDING.md b/docs/BROWSER_RECORDING.md index 3d937278..07e9e47f 100644 --- a/docs/BROWSER_RECORDING.md +++ b/docs/BROWSER_RECORDING.md @@ -105,11 +105,13 @@ sensitive recording data. cross-origin navigation stops the recording and does not produce complete metadata. - Do not open a popup or a new tab in the selected tab's browser context while - recording. Flow currently binds one page. A context-level listener records - every new-page signal, including a tab that acts and closes between recorder - polls. Flow keeps this refusal active through the final Playwright detach so - a late tab cannot produce complete metadata. A refusal leaves the external - browser and its tabs open. + recording. Flow currently binds one page. It snapshots the existing tabs and + installs a new-page latch on every candidate context before it selects the + recording tab. The selected-context latch records every later new-page signal, + including a tab that acts and closes between recorder polls. Flow keeps this + refusal active through the final Playwright detach so a late tab cannot + produce complete metadata. A refusal leaves the external browser and its tabs + open. - Attach mode does not combine with `--headless`. The external browser owns its display mode. - The `--out` path must not exist. Flow writes to a new temporary sibling and @@ -136,12 +138,14 @@ sensitive recording data. moment after a resize. Recording then continues automatically. - `input[type=password]` and fields declared with `--secret FIELD` never send their values to Python. Flow binds a private input-session identity and a - temporary screenshot-mask marker when a declared secret field receives - focus. The identity remains secret if application code changes the field name - or ID, or replaces the active input element during the same input session. - Flow removes the marker when it detaches. Other typed values and visible page - content are recording evidence and can contain sensitive data. Keep raw - recordings inside the approved local boundary. + temporary screenshot-mask marker when the field appears in the document. The + identity remains secret if application code removes the field name or ID + before focus, changes either attribute during input, or replaces the active + input element during the same input session. Flow removes the marker when it + detaches, including from a page-owned element that is no longer in the DOM. + Other typed values and visible page content are recording evidence and can + contain sensitive data. Keep raw recordings inside the approved local + boundary. - Secret masks cover every frame in the selected page. Before each masked screenshot, Flow snapshots the frame inventory and its lifecycle generation. It accepts the in-memory image only when the inventory stays unchanged diff --git a/openadapt_flow/interactive_recorder.py b/openadapt_flow/interactive_recorder.py index fa828cf1..66e925d2 100644 --- a/openadapt_flow/interactive_recorder.py +++ b/openadapt_flow/interactive_recorder.py @@ -370,17 +370,7 @@ def select_attached_page( let activeSecretElement = null; let activeSecretState = null; let resizeTimer = null; - const secretObserver = new MutationObserver(() => { - for (const el of stickySecretElements) { - if (!el.isConnected) continue; - try { - if (!el.hasAttribute(SECRET_MARKER)) el.setAttribute(SECRET_MARKER, ''); - } catch (e) {} - } - }); - secretObserver.observe(document.documentElement || document, { - attributes: true, childList: true, subtree: true, - }); + let secretObserver = null; function listenOn(target, type, handler) { target.addEventListener(type, handler, true); @@ -393,10 +383,19 @@ def select_attached_page( function cleanup() { if (resizeTimer !== null) clearTimeout(resizeTimer); - secretObserver.disconnect(); + if (secretObserver !== null) secretObserver.disconnect(); for (const [target, type, handler] of listeners) { try { target.removeEventListener(type, handler, true); } catch (e) {} } + // The Set retains disconnected elements for this exact cleanup. Remove + // the temporary marker before releasing those references so reinserting a + // page-owned node after Flow detaches cannot expose recorder metadata. + for (const el of stickySecretElements) { + try { el.removeAttribute(SECRET_MARKER); } catch (e) {} + } + stickySecretElements.clear(); + activeSecretElement = null; + activeSecretState = null; const current = window[GLOBAL_KEY]; if (current && current.sessionId === SESSION_ID) { try { delete window[GLOBAL_KEY]; } catch (e) { window[GLOBAL_KEY] = null; } @@ -536,11 +535,13 @@ def select_attached_page( } catch (e) { return false; } } - function bindSecretState(el, state) { + function bindSecretState(el, state, activate = true) { secretStates.set(el, state); stickySecretElements.add(el); - activeSecretElement = el; - activeSecretState = state; + if (activate) { + activeSecretElement = el; + activeSecretState = state; + } try { el.setAttribute(SECRET_MARKER, ''); return el.hasAttribute(SECRET_MARKER); @@ -580,6 +581,65 @@ def select_attached_page( return {state, maskBound: bindSecretState(el, state)}; } + function discoverDeclaredSecrets(root) { + const candidates = []; + try { + if (root && root.nodeType === 1 && isTextEntry(root)) candidates.push(root); + if (root && root.querySelectorAll) { + candidates.push(...root.querySelectorAll( + 'input, textarea, [contenteditable=""], [contenteditable="true"],' + + ' [role="textbox"]' + )); + } + } catch (e) {} + for (const el of candidates) { + if (secretStates.has(el)) continue; + const state = declaredSecretState(el); + if (state) bindSecretState(el, state, false); + } + } + + function stateFromPriorDeclaration(mutation) { + const el = mutation.target; + if (!isTextEntry(el) || typeof mutation.oldValue !== 'string') return null; + const oldValue = mutation.oldValue; + const oldDeclaredName = (mutation.attributeName === 'name' + || mutation.attributeName === 'id') + && SECRET_NAMES.indexOf(oldValue) >= 0; + const oldPasswordType = mutation.attributeName === 'type' + && oldValue.toLowerCase() === 'password'; + if (!oldDeclaredName && !oldPasswordType) return null; + const currentField = el.name || el.id || null; + return { + field: oldDeclaredName ? oldValue : currentField, + inputSession: inputSessionFor(el), + }; + } + + secretObserver = new MutationObserver((mutations) => { + for (const mutation of mutations) { + if (mutation.type === 'attributes') { + if (!secretStates.has(mutation.target)) { + const priorState = stateFromPriorDeclaration(mutation); + if (priorState) bindSecretState(mutation.target, priorState, false); + } + discoverDeclaredSecrets(mutation.target); + } else { + for (const node of mutation.addedNodes) discoverDeclaredSecrets(node); + } + } + for (const el of stickySecretElements) { + if (!el.isConnected) continue; + try { + if (!el.hasAttribute(SECRET_MARKER)) el.setAttribute(SECRET_MARKER, ''); + } catch (e) {} + } + }); + secretObserver.observe(document.documentElement || document, { + attributes: true, attributeOldValue: true, childList: true, subtree: true, + }); + discoverDeclaredSecrets(document); + function fieldLabel(el) { // Best available human label for the receiving field, best-first: // associated