From a1c3f975afc0439e56a70ff4f8c4f29960c28a1a Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 10:55:04 -0400 Subject: [PATCH 01/11] feat: arm a create-new form's save by the form the run typed into (parked) Squashed from #1543 onto main. Parked under the feature pause; the re-review's open findings are in the PR description. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/LIMITS.md | 42 ++ .../backends/playwright_backend.py | 235 ++++++++- .../flow/openadapt_flow/compiler/compile.py | 44 +- .../openadapt_flow/compiler/fixup_passes.py | 1 + .../openadapt_flow/interactive_recorder.py | 47 +- packages/flow/openadapt_flow/ir.py | 52 ++ packages/flow/openadapt_flow/policy.py | 3 +- packages/flow/openadapt_flow/recorder.py | 30 +- .../runtime/identity_template.py | 7 + .../flow/openadapt_flow/runtime/replayer.py | 81 ++- .../flow/openadapt_flow/submit_binding.py | 320 +++++++++++- .../flow/tests/test_create_form_identity.py | 485 ++++++++++++++++++ 12 files changed, 1306 insertions(+), 41 deletions(-) create mode 100644 packages/flow/tests/test_create_form_identity.py diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index 6baf20a495..6255559371 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -375,6 +375,48 @@ The binding ties the click to the entry the run made. It doesn't check that the entry is right: the values typed come from the run's parameters, and only a record check can confirm what the application saved. +A labelled save on a create-new form (Trac's **Create ticket**, NetBox's +**Create**) has the same problem: no existing record is on screen next to it, +so it compiled with no identity check, and a policy that refuses unarmed saves +refused it. Its record is the one the run is typing into the form. When the +recording shows the save is the submit button of the one form that owns every +field typed since the last save, the compiler gives it a **form identity** +(`Anchor.form_identity`). Before the click, a run requires (identity mode +`form`): + +- the button is still that form's submit button, and the form owns exactly the + fields this run typed into, each holding exactly what the run typed; +- the form submits to the same origin and path with the same method, and would + send the same set of field names and types, hidden ones included, so an edit + form that looks like the create form, or an address, method or record id a + script changed or added, halts; +- every field the run did not type into (a hidden parent or record id, a + select left on its default, a checked box) holds its demonstrated value, + stored as a salted digest. Hidden tokens are not compared: ones named like a + token (`csrf`, `token`, `nonce` and the like), whose value changes with every + page load, and ones that changed while the demonstration was typed. A + program that should create under a different parent on each run therefore + halts until that parent is checked another way. + +During the click, a submit from any other form, or from the right form without +its save button (`requestSubmit()` from a click handler), is cancelled and +halts the run. A script that submits with `form.submit()` or `fetch` sends no +submit event, so the guard doesn't see it. + +A backend that can't read the page leaves the identity unreadable, which halts +too. Each field must hold exactly what the run typed, so a controlled input +that reformats the value halts: one that reformats on every keystroke stops at +the typed-value read-back, and one that reformats when it loses focus (a +phone-number mask, for example) stops at the form check. A field that a React +keyed remount replaces with a new element, even with the same value, also +halts: the new element isn't one the run typed into. A submit button outside +the `
` element counts when its `form=` attribute names that form. One +with no form owner at all (a React portal that submits from script) gets no +form identity, so its save stays unarmed. + +The form identity ties the click to the entry the run made in the demonstrated +form. It doesn't prove the app saved it correctly: only a record check does. + For consequential or entity-sensitive workflows, require a policy that refuses unarmed actions, review the inferred risk and tags, and test identity against real near-match and same-name records. Pure-pixel substrates should expect more diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 094d975971..b6dd16401d 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -31,6 +31,7 @@ from openadapt_flow.backend import ActionDeliveryUncertain, StructuralResolutionRefused from openadapt_flow.ir import ( ActionDeliveryReceipt, + FormIdentity, StructuralHandle, StructuralLocator, SubmitBinding, @@ -42,7 +43,10 @@ from openadapt_flow.submit_binding import ( OBSERVE_SUBMIT_BINDING_JS, SUBMIT_BINDING_FIELD_SELECTOR, + form_values_from_page, + form_values_record, observation_from_page, + verify_form_identity, verify_submit_binding, ) @@ -456,7 +460,9 @@ class _FramePoint: + OBSERVE_SUBMIT_BINDING_JS + r"""; const fills = window[args.fillKey]; - return observe(control, Array.isArray(fills) ? fills : []); + return observe( + control, Array.isArray(fills) ? fills : [], window[args.fillKey + '_hidden'] + ); }""" ) @@ -474,10 +480,123 @@ class _FramePoint: Object.defineProperty(window, key, {value: fills, configurable: true}); } if (fills.indexOf(el) < 0) fills.push(el); + // Each hidden control's value as typing starts, so a create-new form's + // fingerprint can tell a token that rotates while the form is filled in. + let hidden = window[key + '_hidden']; + if (!(hidden instanceof WeakMap)) { + hidden = new WeakMap(); + Object.defineProperty( + window, key + '_hidden', {value: hidden, configurable: true}); + } + for (const input of el.ownerDocument.querySelectorAll('input[type="hidden"]')) { + if (!hidden.has(input)) hidden.set(input, String(input.value)); + } + return true; +}""" +) + +# Remember what a field holds once the run has finished typing into it, and +# whether that is exactly what the run typed, for a create-new form's identity +# check (openadapt_flow.ir.FormIdentity). A select holds what was typed when +# its chosen option's label is the typed text. A field that reformatted the +# typed text (a controlled input, a mask) does not. The WeakMap lives in the +# field's own document under a per-backend random key and is only compared in +# the page; no value is returned to Python. +_SNAPSHOT_FILLED_VALUE_JS = r"""(el, args) => { + let values = window[args.key]; + if (!(values instanceof WeakMap)) { + values = new WeakMap(); + Object.defineProperty(window, args.key, {value: values, configurable: true}); + } + const clean = (s) => String(s || '').replace(/\s+/g, ' ').trim(); + const value = ('value' in el) ? String(el.value) : String(el.innerText || ''); + let typed; + if (el.localName === 'select') { + const option = el.options[el.selectedIndex]; + typed = Boolean(option) && clean(option.text) === clean(args.typed); + } else if ('value' in el) { + typed = value === String(args.typed); + } else { + typed = clean(value) === clean(args.typed); + } + values.set(el, {value: value, typed: typed}); + return true; +}""" + +# A create-new form's identity at a point: the submit-binding observation of +# the control there against the fields this run typed into, and how many of +# those fields in the control's form no longer hold what the run left in them +# (or were never snapshotted). Counts only: no value leaves the page. +_FORM_IDENTITY_AT_JS = ( + r"""(args) => { + const hit = document.elementFromPoint(args.x, args.y); + if (!hit) return null; + const control = hit.closest(""" + + json.dumps(GUARDED_ACTIONABLE_SELECTOR) + + r"""); + if (!control) return null; + const observe = """ + + OBSERVE_SUBMIT_BINDING_JS + + r"""; + const fills = Array.isArray(window[args.fillKey]) ? window[args.fillKey] : []; + const out = observe(control, fills, window[args.fillKey + '_hidden']); + const values = window[args.valueKey]; + const form = control.form || control.closest('form'); + let changed = 0; + for (const field of fills) { + if (!field || !field.isConnected || !form) continue; + const owner = field.form || field.closest('form'); + if (owner !== form) continue; + const now = ('value' in field) ? String(field.value) + : String(field.innerText || ''); + const left = (values instanceof WeakMap) ? values.get(field) : null; + if (!left || !left.typed || left.value !== now) changed += 1; + } + return {observation: out.observation, changed: changed}; +}""" +) + +# Before a create-new form's save: cancel any submit during the click that is +# not the checked form submitted by the checked button (a click handler that +# submits another form, or the form without its button), and remember why. +# One shot: it removes itself at the first submit or after 30 s. +_ARM_FORM_SUBMIT_GUARD_JS = ( + r"""(args) => { + const hit = document.elementFromPoint(args.x, args.y); + const control = hit ? hit.closest(""" + + json.dumps(GUARDED_ACTIONABLE_SELECTOR) + + r""") : null; + const form = control ? (control.form || control.closest('form')) : null; + if (!form) return false; + const previous = window[args.key]; + if (previous && previous.remove) previous.remove(); + const state = {refused: null}; + const handler = (event) => { + state.remove(); + if (event.target === form && event.submitter === control) return; + event.preventDefault(); + event.stopImmediatePropagation(); + state.refused = event.target === form + ? 'the form was submitted by something other than its save button' + : 'the click submitted a different form'; + }; + state.remove = () => window.removeEventListener('submit', handler, true); + window.addEventListener('submit', handler, true); + setTimeout(state.remove, 30000); + Object.defineProperty( + window, args.key, {value: state, configurable: true, writable: true}); return true; }""" ) +_FORM_SUBMIT_GUARD_RESULT_JS = r"""(key) => { + const state = window[key]; + if (!state) return null; + if (state.remove) state.remove(); + delete window[key]; + return state.refused || null; +}""" + # Page-structure identity of a FORM FIELD that sits in no record row. # # A data-entry form (OpenEMR's new-patient page, most CRUD screens) lays its @@ -1303,6 +1422,9 @@ def __init__( # Fields this backend typed into, kept page-side per document (element # references only) for submit bindings. A new backend starts empty. self._fill_store_key = f"__oaflow_filled_{uuid.uuid4().hex}" + self._fill_value_slot = f"__oaflow_filled_values_{uuid.uuid4().hex}" + self._form_guard_slot = f"__oaflow_form_guard_{uuid.uuid4().hex}" + self._form_guard_scope: Any = None self._structural_tokens: dict[str, _StructuralGuard] = {} self._guarded_coordinate: Optional[_CoordinateGuard] = None # One-shot expectation that the next click closes this page surface @@ -1940,6 +2062,112 @@ def _register_focused_field(self) -> None: _REGISTER_FILLED_FIELD_JS, arg=self._fill_store_key, with_arg=True ) + def _snapshot_focused_field(self, typed: str) -> None: + """Remember what the focused field holds now that ``typed`` is in, + and whether that is exactly ``typed``. + + A create-new form's identity check compares each filled field with + this snapshot before the save. A field with no snapshot, or one that + did not end up holding exactly what was typed, counts as changed, so + a failure here makes that check refuse: the safe direction. + """ + + self._focused_element_call( + _SNAPSHOT_FILLED_VALUE_JS, + arg={"key": self._fill_value_slot, "typed": typed}, + with_arg=True, + ) + + def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[str]: + """Check a create-new form's identity for the control at a point. + + Returns None when the control is still the submit button of the + demonstrated form (same address, method and set of fields), the form + owns exactly the fields this run typed into, each holding exactly what + the run typed, and every other field holds its demonstrated value. + Then it arms a one-shot guard that cancels any other submit during the + click (:meth:`form_submit_guard_refusal` reports it). Otherwise the + reason, in plain words, including when the page can't be read. + """ + + self.cancel_form_submit_guard() + try: + point = self._frame_point(int(x), int(y)) + if point is None: + return "the resolved point has no readable page" + raw = point.scope.evaluate( + _FORM_IDENTITY_AT_JS, + { + "x": point.x, + "y": point.y, + "fillKey": self._fill_store_key, + "valueKey": self._fill_value_slot, + }, + ) + if not isinstance(raw, dict): + return "no control at the resolved point" + observation = observation_from_page(raw.get("observation")) + values = form_values_from_page(raw.get("observation")) + changed = int(raw.get("changed", -1)) + except Exception: + return "could not read the form at the resolved point" + if changed < 0: + return "could not read the form's field values" + reason = verify_form_identity( + identity, observation, changed_fields=changed, values=values + ) + if reason is not None: + return reason + try: + armed = point.scope.evaluate( + _ARM_FORM_SUBMIT_GUARD_JS, + {"x": point.x, "y": point.y, "key": self._form_guard_slot}, + ) + except Exception: + armed = False + if armed is not True: + return "could not guard the form's submission" + self._form_guard_scope = point.scope + return None + + def form_submit_guard_refusal(self) -> Optional[str]: + """After the click: why the guard cancelled a submit, or None. + + Disarms the guard. A page that navigated away (the checked form's own + submit) has nothing to report. + """ + + scope, self._form_guard_scope = self._form_guard_scope, None + if scope is None: + return None + try: + refused = scope.evaluate( + _FORM_SUBMIT_GUARD_RESULT_JS, self._form_guard_slot + ) + except Exception: + return None + return refused if isinstance(refused, str) else None + + def cancel_form_submit_guard(self) -> None: + """Disarm a guard whose click never happened.""" + + self.form_submit_guard_refusal() + + def form_values_at(self, x: int, y: int) -> Optional[dict[str, Any]]: + """RECORD-TIME: the values of the form's fields the demonstration did + not type into, for the control at a point (see + :func:`~openadapt_flow.submit_binding.form_values_record`), or None. + Plaintext for the recording only; the compiler keeps salted digests. + Never raises. + """ + + try: + raw = self._observe_submit_binding(x, y) + values = form_values_from_page(raw.get("observation")) if raw else None + except Exception: + return None + return form_values_record(values) if values is not None else None + def _focused_element_call( self, js: str, *, arg: Any = None, with_arg: bool = False ) -> Any: @@ -3151,13 +3379,15 @@ def type_text_guarded( """Type through the pre-identity focused-element lease.""" self._register_focused_field() - return self._act_guarded_keyboard( + receipt = self._act_guarded_keyboard( expected_frame_sha256=expected_frame_sha256, operation="guarded_dom_type", # Keys go through the leased element itself, never the page # keyboard, so delivery stays bound to the checked target. deliver=lambda locator: locator.press_sequentially(text, timeout=1000), ) + self._snapshot_focused_field(text) + return receipt def _handle_screenshot_frame_lifecycle(self, _frame: Any = None) -> None: """Advance the irreversible frame-tree generation.""" @@ -3732,6 +3962,7 @@ def type_text(self, text: str) -> None: self._assert_qualification_environment_current() self._register_focused_field() self._deliver_text(text, lambda: self.page.keyboard.type(text)) + self._snapshot_focused_field(text) def press(self, key: str) -> None: """Press a key or chord, e.g. ``'Enter'`` or ``'Meta+a'``.""" diff --git a/packages/flow/openadapt_flow/compiler/compile.py b/packages/flow/openadapt_flow/compiler/compile.py index 248839a3b2..6ca6a9cddb 100644 --- a/packages/flow/openadapt_flow/compiler/compile.py +++ b/packages/flow/openadapt_flow/compiler/compile.py @@ -2866,6 +2866,7 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: # Submit-binding observations recorded on click events (unlabeled # controls only), attached once every risk decision is final. binding_observations: dict[str, object] = {} + form_values: dict[str, object] = {} for j, (step, step_before, step_after, event) in enumerate(pending): steps.append(step) if isinstance(event.get("dialog"), dict): @@ -2875,6 +2876,8 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: step.dialog = _dialog_identity(event["dialog"], exclude_texts) if event.get("submit_binding") is not None: binding_observations[step.id] = event["submit_binding"] + if event.get("form_values") is not None: + form_values[step.id] = event["form_values"] if step.action in (ActionKind.SCROLL, ActionKind.READ): continue # see the scroll and read branches in pass 1 if step.action is ActionKind.SWITCH_SURFACE: @@ -3472,7 +3475,7 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: # fields the demonstration filled, or record why it cannot be bound (the # replayer refuses that click). Runs after every risk decision, model # annotation included, so the binding covers exactly the final writes. - _attach_submit_bindings(workflow.steps, binding_observations) + _attach_submit_bindings(workflow.steps, binding_observations, form_values) # PHI-at-rest remediation (audit REM-2): replace the plaintext identity # band (``anchor.context_text``) and structured identity on every anchor @@ -3532,7 +3535,11 @@ def cached_lines(i: int, suffix: str, png: bytes) -> list[OcrLine]: return workflow -def _attach_submit_bindings(steps: list[Step], observations: dict[str, object]) -> None: +def _attach_submit_bindings( + steps: list[Step], + observations: dict[str, object], + form_values: Optional[dict[str, object]] = None, +) -> None: """Attach :class:`~openadapt_flow.ir.SubmitBinding` evidence in place. A binding applies to a consequential CLICK/DOUBLE_CLICK whose target has @@ -3540,12 +3547,20 @@ def _attach_submit_bindings(steps: list[Step], observations: dict[str, object]) carried a submit-binding observation. Each such step gets either a binding (every condition in :mod:`openadapt_flow.submit_binding` held in the demonstration) or a plain-language reason it could not be bound. - Steps without an observation (pixel recordings, labelled controls, - recordings made before this capability) are left unchanged. + + A consequential click on a labelled control that compiled with no + identity evidence, after fields were typed, and whose observation shows + it is a button of the one form owning those fields, gets a + :class:`~openadapt_flow.ir.FormIdentity` instead: a create-new form's + save. Steps without an observation (pixel recordings, recordings made + before this capability) are left unchanged. """ + from openadapt_flow.policy import is_identity_armed from openadapt_flow.submit_binding import ( + decide_form_identity, decide_submit_binding, + form_values_from_record, observation_from_page, ) @@ -3575,6 +3590,27 @@ def _attach_submit_bindings(steps: list[Step], observations: dict[str, object]) ) step.submit_binding = binding step.submit_binding_unbound_reason = reason + elif ( + raw is not None + and step.action in (ActionKind.CLICK, ActionKind.DOUBLE_CLICK) + and step.risk == "irreversible" + and step.anchor is not None + and typed_since_commit + and not is_identity_armed(step) + ): + # A labelled save with no record on screen to check: a create-new + # form. The record is the one the run typed into the form. + try: + form_identity, _ = decide_form_identity( + observation_from_page(raw), + form_values_from_record((form_values or {}).get(step.id)), + ) + except ValueError: + form_identity = None + if form_identity is not None: + step.anchor.form_identity = form_identity + step.identity_armed = True + step.identity_unarmed_reason = None if step.risk == "irreversible": # A consequential step closes the entry it committed. typed_since_commit = [] diff --git a/packages/flow/openadapt_flow/compiler/fixup_passes.py b/packages/flow/openadapt_flow/compiler/fixup_passes.py index 575b104804..e795c63006 100644 --- a/packages/flow/openadapt_flow/compiler/fixup_passes.py +++ b/packages/flow/openadapt_flow/compiler/fixup_passes.py @@ -166,6 +166,7 @@ def _has_identity_evidence(step: Step) -> bool: or anchor.structured_identity or anchor.identity_template or anchor.identifier_crop + or anchor.form_identity ) ) diff --git a/packages/flow/openadapt_flow/interactive_recorder.py b/packages/flow/openadapt_flow/interactive_recorder.py index 43e459af53..49c4dc5660 100644 --- a/packages/flow/openadapt_flow/interactive_recorder.py +++ b/packages/flow/openadapt_flow/interactive_recorder.py @@ -97,6 +97,8 @@ from openadapt_flow.submit_binding import ( OBSERVE_SUBMIT_BINDING_JS, SUBMIT_BINDING_FIELD_SELECTOR, + form_values_from_page, + form_values_record, observation_from_page, ) @@ -441,6 +443,9 @@ def select_attached_page( const SUBMIT_FIELD = __SUBMIT_FIELD__; const observeSubmitBinding = __OBSERVE_SUBMIT_BINDING__; const filledFields = []; + // Each hidden control's value as typing starts: a create-new form's + // fingerprint names the hidden tokens that rotate while it is filled in. + const hiddenBaseline = new WeakMap(); // One identity for this DOCUMENT. The init script builds a fresh closure per // document, so this closure never saw a value an earlier document received. // Python compares this id against the document that received a secret and @@ -1927,18 +1932,22 @@ def select_attached_page( emitInFrameSpace(dragEvent); }); - // How the clicked control relates to the fields typed into so far. Only an - // unlabeled control yields an observation; it is structure only (tags, - // roles, counts, relation words, icon geometry), captured before the page - // reacts to the click. + // How the clicked control relates to the fields typed into so far: for an + // unlabeled control, or a labelled one inside the form those fields belong + // to. It is structure only (tags, roles, counts, relation words, icon + // geometry), captured before the page reacts to the click. function submitBindingEvidence(target) { try { const control = target && target.closest ? target.closest(SUBMIT_ACTIONABLE) : null; if (!control || control.matches( 'canvas, input[type="range"], [role="slider"]')) return null; - const out = observeSubmitBinding(control, filledFields); - if (!out || !out.observation || out.observation.labelled) return null; + const out = observeSubmitBinding(control, filledFields, hiddenBaseline); + if (!out || !out.observation) return null; + // A labelled control matters only as the save of the form the fields + // were typed into (openadapt_flow.ir.FormIdentity). + if (out.observation.labelled + && out.observation.form_owner !== 'shared') return null; return out.observation; } catch (e) { return null; } } @@ -2143,6 +2152,9 @@ def select_attached_page( if (e.isTrusted && el.matches(SUBMIT_FIELD) && filledFields.indexOf(el) < 0) { filledFields.push(el); + for (const input of document.querySelectorAll('input[type="hidden"]')) { + if (!hiddenBaseline.has(input)) hiddenBaseline.set(input, String(input.value)); + } } } catch (err) {} emitInFrameSpace(o); @@ -4139,21 +4151,24 @@ def _record_pointer(self, ev: dict[str, Any]) -> None: event["section"] = { k: section[k] for k in ("kind", "text", "header") if k in section } - if ( - kind in ("click", "double_click") - and ev.get("submit_binding") - and not _locatable_structural(ev.get("structural")) - ): - # An unlabeled control (no usable locator). Structure only: the - # icon geometry becomes its SHA-256 here, and a malformed - # observation is dropped (no binding, so a later consequential - # click on that control is refused). + if kind in ("click", "double_click") and ev.get("submit_binding"): + # An unlabeled control (no usable locator), or a labelled one in + # the form the run typed into (a create-new form's save). + # Structure only: the icon geometry becomes its SHA-256 here, and + # a malformed observation is dropped (no binding or form identity, + # so a later consequential click on that control is refused). try: event["submit_binding"] = observation_from_page( ev["submit_binding"] ).model_dump(mode="json", exclude_none=True) + # A create-new form's other field values, for the compiler + # to keep as salted digests. Tokens are dropped here. + values = form_values_from_page(ev["submit_binding"]) + if values is not None: + event["form_values"] = form_values_record(values) except ValueError: - pass + event.pop("submit_binding", None) + event.pop("form_values", None) # Marked identifier field rect (--identifier FIELD), captured in-page # at click time: the compiler crops these pixels # (anchor.identifier_crop) to arm the pixel identity tier. diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 034a306885..3caaf7e030 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -365,6 +365,49 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: return data +class FormIdentity(BaseModel): + """Record identity of a save on a create-new form: the form it submits is + the demonstrated form, holding exactly what this run typed. + + A form that creates a record shows no existing record next to its save + button: the record is the one the run is entering. So the save's identity + is the form itself, all of it. The recorder observes, right before the + demonstrated click on a labelled submit button, that the button submits + the one HTML form owning every field typed into since the last save (not + in a table row or a single field's own widget), and fingerprints that + form. Before the click at run time, the replayer requires: + + - the button is still that form's submit button, and the form owns + exactly ``filled_fields`` fields this run typed into, each holding + exactly what the run typed; + - the form submits to the same origin and path (``action_sha256``) with + the same ``method``, and would send the same set of control names and + types (``controls_sha256``), hidden ones included; + - every control the run did not type into (a hidden record or parent id, + a select left on its default, a checked box) holds the demonstrated + value: salted digests in ``value_digests``, under ``value_salt`` (the + identity-template scheme). Hidden tokens are not compared (``exempt``): + ones named like a token, and ones that changed while the demonstration + was typed. + + Anything else halts before the click. A submit event from any other form + or control during the click is cancelled and halts the run. + + Counts, tokens, names and digests only; no field text or value. + """ + + model_config = ConfigDict(extra="forbid", frozen=True) + + filled_fields: int = Field(ge=1, le=10_000) + submit_role: Literal["form_submit"] + action_sha256: str = Field(pattern=r"^[0-9a-f]{64}$") + method: Literal["get", "post", "dialog"] + controls_sha256: str = Field(pattern=r"^[0-9a-f]{64}$") + value_salt: str = Field(default="", max_length=128) + value_digests: dict[str, str] = Field(default_factory=dict) + exempt: tuple[str, ...] = () + + class Anchor(BaseModel): """Redundant evidence for locating a step's target on screen. @@ -481,6 +524,14 @@ class Anchor(BaseModel): " tiers compare like-for-like. Set iff `identifier_crop` is set." ), ) + form_identity: Optional[FormIdentity] = Field( + default=None, + description=( + "A labelled save on a create-new form: before the click, its form" + " must hold exactly the fields this run typed, with the values" + " typed. See FormIdentity. None everywhere else." + ), + ) landmarks: list[Landmark] = Field(default_factory=list) search_pad: int = Field( default=80, @@ -3730,6 +3781,7 @@ class IdentityCheck(BaseModel): "vlm", "signal_quorum", "hover", + "form", ] = "context" coverage: float = 0.0 expected: str = "" diff --git a/packages/flow/openadapt_flow/policy.py b/packages/flow/openadapt_flow/policy.py index eb772b288b..34ae419580 100644 --- a/packages/flow/openadapt_flow/policy.py +++ b/packages/flow/openadapt_flow/policy.py @@ -85,7 +85,7 @@ def is_identity_armed(step: Step) -> bool: The compiler-written ``identity_armed`` field is an audit flag, not the identity evidence. An explicit false remains a refusal. Otherwise require the ground truth the replayer uses: structured/context identity, an - identity template, or an identifier crop. + identity template, an identifier crop, or a create-new form's identity. """ if step.identity_armed is False: return False @@ -95,6 +95,7 @@ def is_identity_armed(step: Step) -> bool: or a.structured_identity or a.identity_template or a.identifier_crop + or a.form_identity ) diff --git a/packages/flow/openadapt_flow/recorder.py b/packages/flow/openadapt_flow/recorder.py index 9cf74995c7..4235c80d68 100644 --- a/packages/flow/openadapt_flow/recorder.py +++ b/packages/flow/openadapt_flow/recorder.py @@ -70,7 +70,6 @@ MAX_PAGES_OPENED_PER_STEP, PRIMARY_SURFACE_ID, StructuralLocator, - structural_locator_is_locatable, ) #: Bound on waiting for a newly opened page's first navigation. @@ -702,17 +701,18 @@ def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None: section = self._section_context_at(int(event["x"]), int(event["y"])) if section: event = {**event, "section": section} - # An unlabeled control has no structural locator. Observe how it - # relates to the fields typed into so far (form ownership, submit - # role, position, uniqueness) so the compiler can bind it to the - # form it submits. Before the action: the click may submit it. - if not structural_locator_is_locatable(locator) and event.get("kind") in ( - "click", - "double_click", - ): + # Observe how the control relates to the fields typed into so far + # (form ownership, submit role, position, uniqueness) so the + # compiler can bind an unlabeled control to the form it submits, + # or give a labelled save on a create-new form its form identity. + # Before the action: the click may submit it. + if event.get("kind") in ("click", "double_click"): binding = self._submit_binding_at(int(event["x"]), int(event["y"])) if binding: event = {**event, "submit_binding": binding} + values = self._form_values_at(int(event["x"]), int(event["y"])) + if values: + event = {**event, "form_values": values} dialog_response, self._expected_dialog = self._expected_dialog, None if dialog_response is not None: arm = getattr(self._backend, "arm_dialog", None) @@ -957,6 +957,18 @@ def _submit_binding_at(self, x: int, y: int) -> Optional[dict[str, Any]]: return None return observation if isinstance(observation, dict) else None + def _form_values_at(self, x: int, y: int) -> Optional[dict[str, Any]]: + """A create-new form's untouched field values at (x, y), if any (the + backend's ``form_values_at``). The compiler keeps salted digests.""" + getter = getattr(self._backend, "form_values_at", None) + if getter is None: + return None + try: + values = getter(int(x), int(y)) + except Exception: + return None + return values if isinstance(values, dict) else None + @staticmethod def _redact(png: bytes, region: tuple[int, int, int, int]) -> bytes: """Return ``png`` with ``region`` (x, y, w, h) filled solid black.""" diff --git a/packages/flow/openadapt_flow/runtime/identity_template.py b/packages/flow/openadapt_flow/runtime/identity_template.py index acc0ada0dd..8e60778b10 100644 --- a/packages/flow/openadapt_flow/runtime/identity_template.py +++ b/packages/flow/openadapt_flow/runtime/identity_template.py @@ -97,6 +97,13 @@ def _signal_hash(salt: bytes, text: str) -> str: return hmac.new(salt, text.encode("utf-8"), hashlib.sha256).hexdigest() +def salted_digest(salt_hex: str, text: str) -> str: + """Full salted digest of ``text`` under a stored salt (the env salt wins), + for identity evidence that is compared whole rather than per token.""" + + return _signal_hash(_salt_bytes(salt_hex), text) + + def new_salt_hex() -> str: """Fresh per-bundle salt (hex). Empty when an external env salt is set, so the bundle stores no salt and the hashes are one-way without the secret.""" diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 5aae38dde7..158cba2a56 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -102,6 +102,7 @@ CheckToClickRecheck, EffectVerificationEvidence, ExecutionTargetKind, + FormIdentity, FreshActuationEvent, HaltObservation, HoverLook, @@ -3789,6 +3790,7 @@ def _record_identity_coverage(workflow: Workflow, report: RunReport) -> None: or step.anchor.structured_identity or step.anchor.identity_template or step.anchor.identifier_crop + or step.anchor.form_identity ): report.identity_armed_steps += 1 else: @@ -10624,6 +10626,7 @@ def _identity_gate_error( or step.anchor.structured_identity or step.anchor.identity_template or step.anchor.identifier_crop + or step.anchor.form_identity ) ) ): @@ -11021,6 +11024,7 @@ def _step_has_identity_contract(self, step: Step, workflow: Workflow) -> bool: or anchor.structured_identity or anchor.identity_template or anchor.identifier_crop + or anchor.form_identity ) def _unbound_submit_refusal( @@ -12933,6 +12937,33 @@ def _act( is confirmed closed. Every other step leaves such a report uncertain. """ + guarded = step.anchor is not None and step.anchor.form_identity is not None + backend = self.backend + try: + error = self._act_closing_surface(step, resolution, params, **kwargs) + finally: + # A create-new form's save: the identity check armed a guard that + # cancels any other submit during the click (ir.FormIdentity). + refusal_of = getattr(backend, "form_submit_guard_refusal", None) + refusal = refusal_of() if guarded and callable(refusal_of) else None + if refusal is not None and error is None: + result = kwargs.get("result") + if result is not None: + result.safety_halt = True + result.failure_category = "safety_halt" + return ( + f"Step '{step.id}' ({step.intent}): {refusal}; the submission was " + "cancelled before it left the page — run aborted" + ) + return error + + def _act_closing_surface( + self, + step: Step, + resolution: Optional[Resolution], + params: Mapping[str, RuntimeParamScalar], + **kwargs: Any, + ) -> Optional[str]: expect = ( getattr(self.backend, "expect_page_close", None) if step.closes_surface @@ -15447,7 +15478,7 @@ def _verify_identity( identity_policy is not None and identity_policy.enforcement.value == "signal_quorum" ): - return self._verify_signal_quorum( + quorum = self._verify_signal_quorum( step=step, resolution=resolution, before_png=before_png, @@ -15456,6 +15487,20 @@ def _verify_identity( bundle_dir=bundle_dir, policy=identity_policy, ) + # An operator's quorum policy and a create-new form's identity + # must both hold. + if quorum.status == "verified" and anchor.form_identity is not None: + form = self._verify_form_identity( + step, resolution, anchor.form_identity + ) + if form.status != "verified": + return form + return quorum + if anchor.form_identity is not None: + # A create-new form's save: the record is what this run typed into + # the form, so the form is checked, in the DOM (see + # ir.FormIdentity). + return self._verify_form_identity(step, resolution, anchor.form_identity) def structured_tier() -> Optional[IdentityCheck]: # PHI-free bundles carry a salted-hash identity_template instead of @@ -16055,6 +16100,39 @@ def _identifier_crops( live_png = identity_mod.crop_region(before_png, live_region) return recorded_png, live_png + def _verify_form_identity( + self, step: Step, resolution: Resolution, identity: FormIdentity + ) -> IdentityCheck: + """Verify a create-new form's save against what this run typed. + + ``verified`` only when the backend can read the form and the control + at the resolved point belongs to the form holding exactly the fields + this run typed into, each with the value typed. A backend that cannot + read the page leaves identity ``unreadable``, which halts an armed + step like any other unreadable identity. + """ + + expected = ( + f"the form holding the {identity.filled_fields} field(s) this run " + "typed, with the values typed" + ) + check = getattr(self.backend, "form_identity_at", None) + if not callable(check): + return IdentityCheck( + status="unreadable", + mode="form", + expected=expected, + observed="this backend cannot read the form's fields", + ) + reason = check(int(resolution.point[0]), int(resolution.point[1]), identity) + if reason is None: + return IdentityCheck( + status="verified", mode="form", expected=expected, observed=expected + ) + return IdentityCheck( + status="mismatch", mode="form", expected=expected, observed=reason + ) + def _verify_identity_ocr( self, step: Step, @@ -16370,6 +16448,7 @@ def _hover_check( or anchor.structured_identity or anchor.identity_template or anchor.identifier_crop + or anchor.form_identity ): return None hover = getattr(self.backend, "hover", None) diff --git a/packages/flow/openadapt_flow/submit_binding.py b/packages/flow/openadapt_flow/submit_binding.py index cd83183c45..0c2c80c456 100644 --- a/packages/flow/openadapt_flow/submit_binding.py +++ b/packages/flow/openadapt_flow/submit_binding.py @@ -47,13 +47,16 @@ import hashlib import json -from typing import Literal, Mapping, Optional, Sequence +import re +from dataclasses import dataclass +from typing import Any, Literal, Mapping, Optional, Sequence from pydantic import BaseModel, ConfigDict, Field, ValidationError from openadapt_flow.ir import ( SUBMIT_BINDING_PATH_PATTERN, SUBMIT_BINDING_TOKEN_PATTERN, + FormIdentity, SubmitBinding, ) @@ -61,10 +64,17 @@ "OBSERVE_SUBMIT_BINDING_JS", "SUBMIT_BINDING_ACTIONABLE_SELECTOR", "SUBMIT_BINDING_FIELD_SELECTOR", + "FormValues", "SubmitBindingObservation", + "decide_form_identity", "decide_submit_binding", "describe_submit_binding", + "form_value_digest", + "form_values_from_page", + "form_values_from_record", + "form_values_record", "observation_from_page", + "verify_form_identity", "verify_submit_binding", ] @@ -96,7 +106,7 @@ #: the list of elements the program (or the demonstrator) typed into. Returns #: the structural observation and ``canon``, its canonical JSON (with the raw #: icon geometry), which the delivery guard recomputes and compares. -_OBSERVE_SUBMIT_BINDING_TEMPLATE = r"""(control, fields) => { +_OBSERVE_SUBMIT_BINDING_TEMPLATE = r"""(control, fields, baseline) => { const ACTIONABLE = __ACTIONABLE__; const FIELD = __FIELD__; const ROW = 'tr, [role="row"], li, [role="listitem"]'; @@ -171,6 +181,57 @@ earlier.compareDocumentPosition(later) & Node.DOCUMENT_POSITION_FOLLOWING ) && !earlier.contains(later); + const formFingerprint = (form, submitter, typed, baseline) => { + // Where and how the form submits (origin and path only), the name and + // type of every control the submission would carry, and the values of + // the controls the run did not type into. Hidden values that changed + // since the first field was filled (a rotating token) are named. + let action = ''; + try { + const target = submitter.hasAttribute('formaction') + ? submitter.formAction : form.action; + const url = new URL(target, doc.baseURI); + action = url.origin + url.pathname; + } catch (_) {} + const method = String((submitter.hasAttribute('formmethod') + ? submitter.formMethod : form.method) || 'get').toLowerCase(); + const controls = []; + const values = []; + const rotated = []; + for (const el of Array.from(form.elements)) { + const name = el.name; + if (!name) continue; + let disabled = false; + try { disabled = el.matches(':disabled'); } catch (_) {} + if (disabled) continue; + const type = String(el.type || el.localName || '').toLowerCase(); + if (el === submitter) { + controls.push(name + '\t' + type); + continue; + } + if (['submit', 'button', 'reset', 'image', 'file'].indexOf(type) >= 0 || + ['fieldset', 'object', 'output'].indexOf(el.localName) >= 0) { + continue; + } + if ((type === 'checkbox' || type === 'radio') && !el.checked) continue; + controls.push(name + '\t' + type); + // A password's value never leaves the page. + if (typed.indexOf(el) >= 0 || type === 'password') continue; + if (el.localName === 'select') { + for (const option of Array.from(el.selectedOptions)) { + values.push([name, type, String(option.value)]); + } + } else { + values.push([name, type, String(el.value)]); + } + if (type === 'hidden' && baseline instanceof WeakMap && + baseline.has(el) && baseline.get(el) !== String(el.value)) { + rotated.push(name); + } + } + controls.sort(); + return {action, method, controls, values, rotated}; + }; const fieldLocal = (el, stop) => { // The first ancestor that holds any field decides: exactly one field // means the control is that field's own adornment (a clear button, @@ -250,12 +311,12 @@ counted: null, icon: null, }; - // A labelled control is identified by its name, not bound. Stop before - // reading anything else about it: its text never leaves the page. - if (result.labelled) { - return {observation: result, canon: JSON.stringify(result)}; - } - result.icon = iconGeometry(control); + // A labelled control is identified by its name, not bound, so its icon, + // position and uniqueness are not read. Its form relation still is: a + // labelled save on a create-new form is tied to the record by the form + // it submits (openadapt_flow.ir.FormIdentity). Its text never leaves the + // page. + if (!result.labelled) result.icon = iconGeometry(control); const seen = new Set(); const live = []; @@ -315,6 +376,15 @@ const afterAll = live.every((field) => follows(field, control)); const beforeAll = live.every((field) => follows(control, field)); result.order = afterAll ? 'after' : (beforeAll ? 'before' : 'between'); + if (result.labelled) { + // A create-new form's save: what the whole form would send, so a run + // can tell this form from an edit form or a form scoped elsewhere + // that looks the same (openadapt_flow.ir.FormIdentity). + if (formOwner === 'shared' && result.submit_role === 'form_submit') { + result.form = formFingerprint(scope, control, live, baseline); + } + return {observation: result, canon: JSON.stringify(result)}; + } let top = Infinity; let left = Infinity; @@ -389,6 +459,92 @@ class SubmitBindingObservation(BaseModel): candidates: Optional[int] = Field(default=None, ge=0, le=100_000) counted: Optional[bool] = None icon_sha256: Optional[str] = Field(default=None, pattern=r"^[0-9a-f]{64}$") + #: A labelled form-submit button of the filled form: SHA-256 of the + #: submission's origin and path, its method, and SHA-256 of the sorted + #: name/type list of the controls it would send. Structure only. + action_sha256: Optional[str] = Field(default=None, pattern=r"^[0-9a-f]{64}$") + form_method: Optional[Literal["get", "post", "dialog"]] = None + controls_sha256: Optional[str] = Field(default=None, pattern=r"^[0-9a-f]{64}$") + + +#: Hidden controls named like a per-session or per-page token (CSRF and the +#: like). Their values differ on every page load, so they are never compared, +#: and their values are never kept. +_TOKEN_NAME = re.compile( + r"csrf|xsrf|token|nonce|authenticity|verification|viewstate|eventvalidation", + re.IGNORECASE, +) + + +@dataclass(frozen=True) +class FormValues: + """Values of the controls a run did not type into, by control name. + + Plaintext, so it never goes into an observation or a bundle: the + compiler keeps salted digests (:func:`decide_form_identity`) and a run + compares digests. ``exempt`` names hidden tokens that are not compared: + named like one, or changed while the demonstration was being typed. + """ + + values: Mapping[str, tuple[str, ...]] + exempt: frozenset[str] + + +def form_values_from_page(raw: object) -> Optional[FormValues]: + """The untouched control values in a page observation, or None when it + has no form fingerprint. Token values are dropped here.""" + + if not isinstance(raw, Mapping): + return None + form = raw.get("form") + if not isinstance(form, Mapping): + return None + rotated = {str(name) for name in form.get("rotated") or ()} + grouped: dict[str, list[str]] = {} + exempt: set[str] = set() + for item in form.get("values") or (): + if not isinstance(item, (list, tuple)) or len(item) != 3: + raise ValueError("form values are malformed") + name, kind, value = (str(part) for part in item) + if kind == "hidden" and (name in rotated or _TOKEN_NAME.search(name)): + exempt.add(name) + continue + grouped.setdefault(name, []).append(value) + return FormValues( + values={name: tuple(sorted(found)) for name, found in grouped.items()}, + exempt=frozenset(exempt), + ) + + +def form_values_record(values: FormValues) -> dict[str, Any]: + """``values`` as JSON for a recording event.""" + + return { + "values": {k: list(v) for k, v in values.values.items()}, + "exempt": sorted(values.exempt), + } + + +def form_values_from_record(raw: object) -> Optional[FormValues]: + """Read :func:`form_values_record` back, or None.""" + + if not isinstance(raw, Mapping): + return None + values = raw.get("values") + if not isinstance(values, Mapping): + return None + return FormValues( + values={str(k): tuple(sorted(str(x) for x in v)) for k, v in values.items()}, + exempt=frozenset(str(x) for x in raw.get("exempt") or ()), + ) + + +def form_value_digest(salt_hex: str, values: Sequence[str]) -> str: + """Salted digest of one control's value(s): the identity-template scheme.""" + + from openadapt_flow.runtime.identity_template import salted_digest + + return salted_digest(salt_hex, "\x1f".join(sorted(values))) def observation_from_page(raw: object) -> SubmitBindingObservation: @@ -405,6 +561,21 @@ def observation_from_page(raw: object) -> SubmitBindingObservation: if not isinstance(raw, Mapping): raise ValueError("submit-binding observation is not an object") data = dict(raw) + form = data.pop("form", None) + if form is not None: + if not isinstance(form, Mapping): + raise ValueError("submit-binding form fingerprint is malformed") + action = str(form.get("action") or "") + controls = form.get("controls") or [] + if not isinstance(controls, list) or not all( + isinstance(c, str) for c in controls + ): + raise ValueError("submit-binding form controls are malformed") + data["action_sha256"] = hashlib.sha256(action.encode("utf-8")).hexdigest() + data["form_method"] = str(form.get("method") or "get").lower() + data["controls_sha256"] = hashlib.sha256( + "\n".join(sorted(controls)).encode("utf-8") + ).hexdigest() icon = data.pop("icon", None) if icon is not None: if not isinstance(icon, str) or len(icon) > _MAX_ICON_GEOMETRY_CHARS: @@ -465,6 +636,139 @@ def _unbindable_reason(observation: SubmitBindingObservation) -> Optional[str]: return None +def _no_form_identity_reason(observation: SubmitBindingObservation) -> Optional[str]: + """Why a labelled control's form can't identify the record, or None.""" + + if not observation.labelled: + return "the control has no name, so a submit binding identifies it" + if observation.in_row: + return ( + "the control is inside a table row or list item, whose own identity " + "ties it to a record" + ) + if observation.fields <= 0: + return "no field the program typed into is on the page" + if observation.form_owner != "shared": + return "the control is not in the form that owns the filled fields" + if observation.fields_in_scope is not True: + return "a filled field lies outside the control's form" + if observation.field_local is not False: + return "the control belongs to a single field's own widget" + if observation.submit_role != "form_submit": + # Another button's effect is whatever its script does, so only the + # form's own submit button identifies what is submitted. + return "the control is not that form's submit button" + return None + + +def decide_form_identity( + observation: SubmitBindingObservation, + values: Optional[FormValues] = None, + *, + salt_hex: Optional[str] = None, +) -> tuple[Optional[FormIdentity], Optional[str]]: + """A labelled save's form identity from the demonstration, or a reason. + + Returns ``(identity, None)`` when the control is the submit button of the + one form that owns every field typed into since the last save and the + recording has the form's fingerprint and untouched values, else + ``(None, reason)``. + """ + + from openadapt_flow.runtime.identity_template import new_salt_hex + + reason = _no_form_identity_reason(observation) + if reason is not None: + return None, reason + if ( + observation.action_sha256 is None + or observation.form_method is None + or observation.controls_sha256 is None + or values is None + ): + return None, "the recording has no fingerprint of the form" + salt = salt_hex if salt_hex is not None else new_salt_hex() + return ( + FormIdentity( + filled_fields=observation.fields, + submit_role="form_submit", + action_sha256=observation.action_sha256, + method=observation.form_method, + controls_sha256=observation.controls_sha256, + value_salt=salt, + value_digests={ + name: form_value_digest(salt, found) + for name, found in sorted(values.values.items()) + }, + exempt=tuple(sorted(values.exempt)), + ), + None, + ) + + +def verify_form_identity( + identity: FormIdentity, + observation: SubmitBindingObservation, + *, + changed_fields: int, + values: Optional[FormValues], +) -> Optional[str]: + """Return None when the live form still identifies the record. + + ``observation`` is the live control's observation against the fields this + run typed into; ``changed_fields`` counts those fields in its form that + did not end up holding exactly what the run typed (a controlled input + that reformatted it) or changed since; ``values`` are the live values of + the controls the run did not type into. Otherwise return the first + difference, in plain words. + """ + + reason = _no_form_identity_reason(observation) + if reason is not None: + return reason + if observation.submit_role != identity.submit_role: + return ( + f"the control's submit role is {observation.submit_role!r}, it was " + f"{identity.submit_role!r}" + ) + if observation.fields != identity.filled_fields: + return ( + f"this run has typed into {observation.fields} field(s) still on the " + f"page; the demonstration had typed into {identity.filled_fields}" + ) + if observation.action_sha256 != identity.action_sha256: + return "the form submits to a different address than in the demonstration" + if observation.form_method != identity.method: + return ( + f"the form submits with {observation.form_method!r}, it submitted with " + f"{identity.method!r} in the demonstration" + ) + if observation.controls_sha256 != identity.controls_sha256: + return "the form would send a different set of fields than in the demonstration" + if changed_fields: + return ( + f"{changed_fields} field(s) of the form don't hold exactly the value " + "this run typed" + ) + if values is None: + return "could not read the values of the form's other fields" + live = { + name: found + for name, found in values.values.items() + if name not in identity.exempt + } + for name in sorted(set(live) | set(identity.value_digests)): + recorded = identity.value_digests.get(name) + if recorded is None or name not in live: + return f"the form's {name!r} field is not the one demonstrated" + if form_value_digest(identity.value_salt, live[name]) != recorded: + return ( + f"the form's {name!r} field, which this run did not type into, " + "holds a different value than in the demonstration" + ) + return None + + def decide_submit_binding( observation: SubmitBindingObservation, *, diff --git a/packages/flow/tests/test_create_form_identity.py b/packages/flow/tests/test_create_form_identity.py new file mode 100644 index 0000000000..5f0d0917b5 --- /dev/null +++ b/packages/flow/tests/test_create_form_identity.py @@ -0,0 +1,485 @@ +"""A create-new form's save is armed by the form the run typed into. + +A form that creates a record (Trac's New Ticket, NetBox's Add Device) shows no +existing record next to its save button, so the save compiled with no identity +check and qualification refused it ("save step ... has no armed identity +check"). The record such a save commits is the one the run typed into the form. +So the compiler gives the labelled save a FormIdentity, and before the click +the replayer requires that the button's form holds exactly the fields this run +typed into, with the values it typed. + +Real headless Chromium throughout: the interactive recorder on a Trac-shaped +form, the real compiler, and the real Replayer against the same page and +variants that must halt before the click with nothing submitted, including +the ones the #1543 review found: an edit form that looks like a create form, +an address or method changed after typing, a record id added while typing, a +parent left on the wrong default, and a click that submits another form. +""" + +from __future__ import annotations + +import http.server +import json +import secrets +import threading +from pathlib import Path +from typing import Any, Callable, Iterator +from urllib.parse import parse_qs, urlsplit + +import pytest + +from openadapt_flow.compiler import compile_recording +from openadapt_flow.interactive_recorder import record_interactive +from openadapt_flow.ir import Workflow +from openadapt_flow.policy import is_identity_armed + +sync_api = pytest.importorskip("playwright.sync_api") + +pytestmark = pytest.mark.timeout(900) + +_TAMPER = """ +""" + + +_REFORMAT = """ +""" + +_BLUR_FORMAT = """ +""" + +_CREATE = '' +_PORTAL = ( + # Rendered outside the form with no form= attribute, as a React portal + # does; its click handler submits the form from script. + '" +) + + +# Variants from the #1543 review: each looks like the same create form, and +# each would submit the wrong thing if only the typed fields were checked. +_SCRIPTS = { + # A script sets a different action and method, and adds a record id, + # after the run has typed. + "action_swap": """ +document.querySelector('[name=field_description]').addEventListener('focus', () => { + const f = document.getElementById('ticket'); f.action = '/ticket/42'; f.method = 'post'; + const h = document.createElement('input'); h.type = 'hidden'; h.name = 'id'; + h.value = '42'; f.appendChild(h); +});""", + # A hidden record id appears while the run types. + "late_hidden": """ +document.querySelector('[name=field_description]').addEventListener('input', () => { + const f = document.getElementById('ticket'); + if (!f.querySelector('[name=id]')) { const h = document.createElement('input'); + h.type = 'hidden'; h.name = 'id'; h.value = '42'; f.appendChild(h); } +});""", + # The Create button's click handler submits another form instead. + "click_other": """ +document.querySelector('[name=submit]').addEventListener('click', (e) => { + e.preventDefault(); document.getElementById('other').requestSubmit(); +});""", + # A hidden revision that changes on every keystroke, as some editors keep. + "rotating": """ +document.querySelector('[name=field_description]').addEventListener('input', () => { + const h = document.querySelector('[name=draft_rev]'); + h.value = 'r' + Math.random().toString(16).slice(2); +});""", + # A React-style keyed remount of the summary input on blur, same value. + "remount": """ +document.querySelector('[name=field_summary]').addEventListener('blur', () => { + const s = document.querySelector('[name=field_summary]'); + const n = s.cloneNode(true); n.value = s.value; s.replaceWith(n); +}, {once: true});""", +} + + +def _page(variant: str) -> str: + # "split": the Create button belongs to another form. "formattr": it sits + # outside the form and names it with form=. "portal": it sits outside the + # form with no form owner at all. + owner = {"split": ' form="other"', "formattr": ' form="ticket"'}.get(variant, "") + inside = ( + _CREATE.format(owner=owner) if variant not in ("formattr", "portal") else "" + ) + outside = ( + _CREATE.format(owner=owner) + if variant == "formattr" + else (_PORTAL if variant == "portal" else "") + ) + title, action, hidden, extra = "New ticket", "/newticket", "", "" + if variant == "edit": + # Ticket 42's edit form: the same blank fields, a record id, another + # address. Its save updates ticket 42. + title, action = "Ticket #42 (edit)", "/ticket/42" + hidden = '' + if variant in ("parent_ok", "parent_wrong"): + hidden = ( + '' + ) + if variant in ("select_ok", "select_wrong"): + chosen = "7" if variant == "select_ok" else "9" + extra = ( + '

" + ) + if variant == "csrf": + # A per-session token, different on every page load (Trac's form has + # one). + hidden = ( + f'' + ) + if variant == "rotating": + hidden = ( + '' + ) + script = _SCRIPTS.get(variant, "") + return f""" +{title} +

{title}

+ + {hidden} +

+

+ {extra} +

{inside}

+
+ +
+
+ +{_TAMPER if variant == "tamper" else ""} +{_REFORMAT if variant == "reformat" else ""} +{_BLUR_FORMAT if variant == "blurformat" else ""} +{f"" if script else ""} +""" + + +def _serve() -> tuple[str, Callable[[], None]]: + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 (http.server API) + query = parse_qs(urlsplit(self.path).query) + body = _page(query.get("v", ["plain"])[0]).encode() + self.send_response(200) + self.send_header("Content-Type", "text/html") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *_args: object) -> None: + pass + + server = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler) + threading.Thread(target=server.serve_forever, daemon=True).start() + return f"http://127.0.0.1:{server.server_address[1]}/newticket", server.shutdown + + +@pytest.fixture(scope="module") +def url() -> Iterator[str]: + base, stop = _serve() + yield base + stop() + + +def _record(url: str, out: Path) -> Path: + def drive(page: Any, pump: Any) -> None: + def settle() -> None: + for _ in range(12): + pump() + + page.wait_for_selector("[name=field_summary]", state="visible") + page.locator("[name=field_summary]").click() + settle() + page.keyboard.type("Printer jams on tray 2") + settle() + page.locator("[name=field_description]").click() + settle() + page.keyboard.type("Seen twice today") + settle() + page.locator("[name=submit]").click() + settle() + assert page.evaluate("window.created") == ["Printer jams on tray 2"] + + recording = record_interactive( + url, + out / "rec", + headless=True, + script=drive, + param_fields=("field_summary", "field_description"), + ) + compile_recording(recording, out / "bundle", name="newticket") + return out / "bundle" + + +@pytest.fixture(scope="module") +def bundle(url: str, tmp_path_factory: pytest.TempPathFactory) -> Path: + return _record(url, tmp_path_factory.mktemp("create-form")) + + +@pytest.fixture(scope="module") +def formattr_bundle(url: str, tmp_path_factory: pytest.TempPathFactory) -> Path: + return _record(f"{url}?v=formattr", tmp_path_factory.mktemp("create-formattr")) + + +def _replay(url: str, bundle: Path, run_dir: Path) -> tuple[Any, list]: + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + from openadapt_flow.runtime import Replayer + + with sync_api.sync_playwright() as playwright: + browser = playwright.chromium.launch(headless=True) + try: + page = browser.new_page(viewport={"width": 1280, "height": 800}) + page.goto(url) + page.wait_for_selector("[name=field_summary]", state="visible") + report = Replayer(PlaywrightBackend(page)).run( + Workflow.load(bundle), + params={ + "field_summary": "Scanner offline on floor 3", + "field_description": "Since this morning", + }, + bundle_dir=bundle, + run_dir=run_dir, + ) + created = page.evaluate("window.created") + finally: + browser.close() + return report, created + + +def test_the_create_button_is_armed_by_its_form(bundle: Path) -> None: + workflow = Workflow.load(bundle) + save = workflow.steps[-1] + assert save.risk == "irreversible" + assert save.anchor is not None and save.anchor.form_identity is not None + assert save.anchor.form_identity.filled_fields == 2 + assert save.anchor.form_identity.submit_role == "form_submit" + assert save.identity_armed is True + assert is_identity_armed(save) + # Counts and tokens only: nothing typed is stored with it. + stored = json.dumps(save.anchor.form_identity.model_dump()) + assert "Printer" not in stored and "Seen" not in stored + + +def test_replay_with_new_values_creates_the_new_ticket( + url: str, bundle: Path, tmp_path: Path +) -> None: + report, created = _replay(url, bundle, tmp_path / "run") + assert report.success, [(r.step_id, r.error) for r in report.results] + assert created == ["Scanner offline on floor 3"] + save = report.results[-1] + assert save.identity is not None + assert (save.identity.mode, save.identity.status) == ("form", "verified") + + +def test_a_field_changed_after_typing_halts_before_the_click( + url: str, bundle: Path, tmp_path: Path +) -> None: + report, created = _replay(f"{url}?v=tamper", bundle, tmp_path / "run") + assert not report.success + assert created == [] + save = next(r for r in report.results if r.identity and r.identity.mode == "form") + assert save.identity.status == "mismatch" + assert "don't hold exactly the value this run typed" in save.identity.observed + + +def test_a_button_of_another_form_halts_before_the_click( + url: str, bundle: Path, tmp_path: Path +) -> None: + report, created = _replay(f"{url}?v=split", bundle, tmp_path / "run") + assert not report.success + assert created == [] + save = next(r for r in report.results if r.identity and r.identity.mode == "form") + assert save.identity.status == "mismatch" + assert "not in the form that owns the filled fields" in save.identity.observed + + +def test_a_controlled_input_that_reformats_halts_before_the_click( + url: str, bundle: Path, tmp_path: Path +) -> None: + """Every keystroke reformats the field. The typed-value read-back on the + summary step stops the run first; whatever notices first, nothing is + submitted.""" + + report, created = _replay(f"{url}?v=reformat", bundle, tmp_path / "run") + assert not report.success + assert created == [] + checked = [r for r in report.results if r.identity and r.identity.mode == "form"] + if checked: + assert checked[-1].identity.status == "mismatch" + assert "don't hold exactly the value" in checked[-1].identity.observed + + +def test_a_value_reformatted_on_blur_halts_at_the_form_check( + url: str, bundle: Path, tmp_path: Path +) -> None: + """The read-back after typing passes; the field is reformatted when focus + moves on. The form check before the save catches it.""" + + report, created = _replay(f"{url}?v=blurformat", bundle, tmp_path / "run") + assert not report.success + assert created == [] + save = next(r for r in report.results if r.identity and r.identity.mode == "form") + assert save.identity.status == "mismatch" + assert "don't hold exactly the value this run typed" in save.identity.observed + + +def test_a_button_outside_the_form_with_form_attribute_is_armed( + url: str, formattr_bundle: Path, tmp_path: Path +) -> None: + save = Workflow.load(formattr_bundle).steps[-1] + assert save.anchor is not None and save.anchor.form_identity is not None + assert save.anchor.form_identity.submit_role == "form_submit" + + report, created = _replay(f"{url}?v=formattr", formattr_bundle, tmp_path / "run") + assert report.success, [(r.step_id, r.error) for r in report.results] + assert created == ["Scanner offline on floor 3"] + assert report.results[-1].identity.status == "verified" + + +def test_a_portal_button_with_no_form_owner_is_not_armed_and_halts( + url: str, formattr_bundle: Path, tmp_path: Path +) -> None: + # Recorded on the portal page: nothing ties the button to the form, so + # the save gets no form identity and stays unarmed (a policy that + # refuses unarmed saves refuses it). + portal = _record(f"{url}?v=portal", tmp_path / "portal") + save = Workflow.load(portal).steps[-1] + assert save.anchor is not None and save.anchor.form_identity is None + assert not is_identity_armed(save) + + # A program recorded with form= meets the portal page: halt before the + # click. + report, created = _replay(f"{url}?v=portal", formattr_bundle, tmp_path / "run") + assert not report.success + assert created == [] + checked = [r for r in report.results if r.identity and r.identity.mode == "form"] + assert checked and checked[-1].identity.status == "mismatch" + assert ( + "not in the form that owns the filled fields" in checked[-1].identity.observed + ) + + +@pytest.fixture(scope="module") +def parent_bundle(url: str, tmp_path_factory: pytest.TempPathFactory) -> Path: + return _record(f"{url}?v=parent_ok", tmp_path_factory.mktemp("create-parent")) + + +@pytest.fixture(scope="module") +def select_bundle(url: str, tmp_path_factory: pytest.TempPathFactory) -> Path: + return _record(f"{url}?v=select_ok", tmp_path_factory.mktemp("create-select")) + + +def _halted_by_form(report: Any, created: list, expected: str) -> None: + assert not report.success + assert created == [] + checked = [r for r in report.results if r.identity and r.identity.mode == "form"] + assert checked, [(r.step_id, r.error) for r in report.results] + assert checked[-1].identity.status == "mismatch" + assert expected in checked[-1].identity.observed + + +@pytest.mark.parametrize( + "variant, expected", + [ + ("edit", "submits to a different address"), + ("action_swap", "submits to a different address"), + ("late_hidden", "different set of fields"), + ], +) +def test_a_form_that_is_not_the_demonstrated_one_halts( + url: str, bundle: Path, tmp_path: Path, variant: str, expected: str +) -> None: + report, created = _replay(f"{url}?v={variant}", bundle, tmp_path / "run") + _halted_by_form(report, created, expected) + + +def test_a_hidden_parent_on_another_record_halts( + url: str, parent_bundle: Path, tmp_path: Path +) -> None: + save = Workflow.load(parent_bundle).steps[-1] + assert save.anchor is not None and save.anchor.form_identity is not None + stored = json.dumps(save.anchor.form_identity.model_dump()) + assert '"7"' not in stored and "pid" in stored # a digest, not the value + + report, created = _replay(f"{url}?v=parent_ok", parent_bundle, tmp_path / "ok") + assert report.success, [(r.step_id, r.error) for r in report.results] + assert created == ["Scanner offline on floor 3"] + + report, created = _replay(f"{url}?v=parent_wrong", parent_bundle, tmp_path / "bad") + _halted_by_form(report, created, "'pid' field, which this run did not type into") + + +def test_a_select_left_on_another_default_halts( + url: str, select_bundle: Path, tmp_path: Path +) -> None: + report, created = _replay(f"{url}?v=select_ok", select_bundle, tmp_path / "ok") + assert report.success, [(r.step_id, r.error) for r in report.results] + + report, created = _replay(f"{url}?v=select_wrong", select_bundle, tmp_path / "bad") + _halted_by_form(report, created, "'pid' field, which this run did not type into") + + +def test_a_click_that_submits_another_form_is_cancelled( + url: str, bundle: Path, tmp_path: Path +) -> None: + report, created = _replay(f"{url}?v=click_other", bundle, tmp_path / "run") + assert not report.success + assert created == [] + save = report.results[-1] + assert save.identity is not None and save.identity.status == "verified" + assert "submitted a different form" in (save.error or "") + + +@pytest.mark.parametrize("variant", ["csrf", "rotating"]) +def test_a_rotating_token_still_verifies( + url: str, tmp_path: Path, variant: str +) -> None: + """A per-session token (named like one) and a hidden value that changed + while the demonstration was typed are not compared, so a new page load + with a new token still verifies, and the token's value is never kept.""" + + bundle = _record(f"{url}?v={variant}", tmp_path / "rec") + save = Workflow.load(bundle).steps[-1] + assert save.anchor is not None and save.anchor.form_identity is not None + assert save.anchor.form_identity.exempt == ( + ("__FORM_TOKEN",) if variant == "csrf" else ("draft_rev",) + ) + report, created = _replay(f"{url}?v={variant}", bundle, tmp_path / "run") + assert report.success, [(r.step_id, r.error) for r in report.results] + assert created == ["Scanner offline on floor 3"] + assert report.results[-1].identity.status == "verified" + + +def test_a_keyed_remount_halts_a_known_limit( + url: str, bundle: Path, tmp_path: Path +) -> None: + """A React-style keyed remount replaces a typed field with a new node, even + with the same value. The new node isn't one the run typed into, so the run + halts: a false halt, and the safe direction (LIMITS.md).""" + + report, created = _replay(f"{url}?v=remount", bundle, tmp_path / "run") + _halted_by_form(report, created, "typed into 1 field(s)") From ded2cc2e27b81930a14c22eed4ac9f83547e3646 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 28 Oct 2026 14:40:17 -0400 Subject: [PATCH 02/11] fix: close #1581's re-review holes: query, what is sent, late submits, token scope - The form's address includes its query parameter names; their values are compared like untouched fields (salted digests under "?name"). - The submit guard compares every submit with what the checked button would have sent (address with query, method, every name/value pair but exempt tokens): a click handler that copies a parent id into an empty field, enables a disabled id, or adds one before requestSubmit(button) is cancelled, and a second submit too. - The guard stays armed after the click: the replayer watches it for 2 s, then reads and disarms it at the start of the next step, halting on any cancelled submit. setTimeout submits are cancelled. - A hidden or query value is exempt only when named like a token and shaped like one (16+ characters, not all digits). A short value under a token-like name, or a value a script set while the form was filled in, is compared. File inputs count in the fingerprint. Tests: the re-review's rows halt before the click with nothing submitted; a per-session token still verifies. LIMITS.md: salt enumeration of low-entropy digests, and the false halt of a page whose click handler fills a field at submit. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/LIMITS.md | 48 +++++-- .../backends/playwright_backend.py | 128 ++++++++++++------ .../openadapt_flow/interactive_recorder.py | 8 +- .../flow/openadapt_flow/runtime/replayer.py | 49 +++++-- .../flow/openadapt_flow/submit_binding.py | 68 +++++++--- .../flow/tests/test_create_form_identity.py | 126 +++++++++++++++-- 6 files changed, 315 insertions(+), 112 deletions(-) diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index 6255559371..c87330863e 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -386,22 +386,40 @@ field typed since the last save, the compiler gives it a **form identity** - the button is still that form's submit button, and the form owns exactly the fields this run typed into, each holding exactly what the run typed; -- the form submits to the same origin and path with the same method, and would - send the same set of field names and types, hidden ones included, so an edit - form that looks like the create form, or an address, method or record id a - script changed or added, halts; +- the form submits to the same origin, path and query parameter names with the + same method, and would send the same set of field names and types, hidden + and file ones included, so an edit form that looks like the create form, or + an address, method or record id a script changed or added, halts; - every field the run did not type into (a hidden parent or record id, a - select left on its default, a checked box) holds its demonstrated value, - stored as a salted digest. Hidden tokens are not compared: ones named like a - token (`csrf`, `token`, `nonce` and the like), whose value changes with every - page load, and ones that changed while the demonstration was typed. A - program that should create under a different parent on each run therefore - halts until that parent is checked another way. - -During the click, a submit from any other form, or from the right form without -its save button (`requestSubmit()` from a click handler), is cancelled and -halts the run. A script that submits with `form.submit()` or `fetch` sends no -submit event, so the guard doesn't see it. + select left on its default, a checked box, a query parameter's value) holds + its demonstrated value, stored as a salted digest. Only a per-page token is + not compared: a hidden field or query value named like one (`csrf`, `token`, + `nonce` and the like) whose value is shaped like one (16 characters or more, + not all digits). A short or numeric value under such a name, and a value a + script set while the form was filled in, are compared. A program that + should create under a different parent on each run therefore halts until + that parent is checked another way. + +From the check until the next step, a guard in the page compares every submit +with what the checked button would have sent (address and query, method, and +every field's value except the exempt tokens). A submit from another form, +from the right form without its save button, with contents that changed after +the check (a click handler that fills in or enables a field), or a second +submit, is cancelled. The run watches the guard for two seconds after the +click, then again at the start of the next step, and halts on any cancelled +submit. A script that submits with `form.submit()` or `fetch` sends no submit +event, so the guard doesn't see it. A submit a script delays past the start +of the next step is still cancelled until the page changes, but the run no +longer reports it. A page whose own click handler fills in or enables a field +as the form is sent changes what is sent after the check, so its save halts +even when the demonstration did the same: a false halt, in the safe +direction. + +The digests of untouched values are salted; with the per-bundle salt stored in +the bundle (the default, unless `OPENADAPT_FLOW_IDENTITY_SALT` is set), a +low-entropy value such as a numeric id or a select option can be recovered by +trying candidates. The recording itself holds those values in plain text, as +it holds the typed ones. A backend that can't read the page leaves the identity unreadable, which halts too. Each field must hold exactly what the run typed, so a controlled input diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 7dc659745a..81fdd9f661 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -676,9 +676,7 @@ class _FramePoint: + OBSERVE_SUBMIT_BINDING_JS + r"""; const fills = window[args.fillKey]; - return observe( - control, Array.isArray(fills) ? fills : [], window[args.fillKey + '_hidden'] - ); + return observe(control, Array.isArray(fills) ? fills : []); }""" ) @@ -696,17 +694,6 @@ class _FramePoint: Object.defineProperty(window, key, {value: fills, configurable: true}); } if (fills.indexOf(el) < 0) fills.push(el); - // Each hidden control's value as typing starts, so a create-new form's - // fingerprint can tell a token that rotates while the form is filled in. - let hidden = window[key + '_hidden']; - if (!(hidden instanceof WeakMap)) { - hidden = new WeakMap(); - Object.defineProperty( - window, key + '_hidden', {value: hidden, configurable: true}); - } - for (const input of el.ownerDocument.querySelectorAll('input[type="hidden"]')) { - if (!hidden.has(input)) hidden.set(input, String(input.value)); - } return true; }""" ) @@ -755,7 +742,7 @@ class _FramePoint: + OBSERVE_SUBMIT_BINDING_JS + r"""; const fills = Array.isArray(window[args.fillKey]) ? window[args.fillKey] : []; - const out = observe(control, fills, window[args.fillKey + '_hidden']); + const out = observe(control, fills); const values = window[args.valueKey]; const form = control.form || control.closest('form'); let changed = 0; @@ -772,10 +759,13 @@ class _FramePoint: }""" ) -# Before a create-new form's save: cancel any submit during the click that is -# not the checked form submitted by the checked button (a click handler that -# submits another form, or the form without its button), and remember why. -# One shot: it removes itself at the first submit or after 30 s. +# Before a create-new form's save: guard the form's submission. At arm time it +# keeps, in the page, what the checked button would send (address with query, +# method, and every name/value pair except the exempt tokens). Every submit +# while it is armed is checked: one from another form, from this form without +# its button, with different contents, or a second one, is cancelled and +# recorded. It stays armed after the click (a script may submit later) until +# the run reads and disarms it, or after 120 s. _ARM_FORM_SUBMIT_GUARD_JS = ( r"""(args) => { const hit = document.elementFromPoint(args.x, args.y); @@ -784,33 +774,71 @@ class _FramePoint: + r""") : null; const form = control ? (control.form || control.closest('form')) : null; if (!form) return false; + const exempt = new Set(args.exempt || []); + const canon = () => { + let data; + try { data = new FormData(form, control); } catch (_) { data = new FormData(form); } + const pairs = []; + for (const [key, value] of data.entries()) { + if (exempt.has(key)) continue; + pairs.push([key, (typeof value === 'string') ? value + : ('file:' + value.name + ':' + value.size)]); + } + pairs.sort((x, y) => (x[0] + '\u0000' + x[1]) < (y[0] + '\u0000' + y[1]) ? -1 : 1); + const target = control.hasAttribute('formaction') ? control.formAction : form.action; + let address = target; + try { + const url = new URL(target, document.baseURI); + const query = []; + for (const [k, v] of url.searchParams) if (!exempt.has('?' + k)) query.push([k, v]); + query.sort(); + address = url.origin + url.pathname + '?' + JSON.stringify(query); + } catch (_) {} + const method = String((control.hasAttribute('formmethod') + ? control.formMethod : form.method) || 'get').toLowerCase(); + return JSON.stringify([address, method, pairs]); + }; const previous = window[args.key]; if (previous && previous.remove) previous.remove(); - const state = {refused: null}; + const expected = canon(); + const state = {refused: [], allowed: 0}; const handler = (event) => { - state.remove(); - if (event.target === form && event.submitter === control) return; - event.preventDefault(); - event.stopImmediatePropagation(); - state.refused = event.target === form - ? 'the form was submitted by something other than its save button' - : 'the click submitted a different form'; + let why = null; + if (event.target !== form) why = 'the click submitted a different form'; + else if (event.submitter !== control) { + why = 'the form was submitted by something other than its save button'; + } else if (canon() !== expected) { + why = 'what the form would send changed after it was checked'; + } else if (state.allowed > 0) { + why = 'the form was submitted a second time'; + } + if (why) { + event.preventDefault(); + event.stopImmediatePropagation(); + state.refused.push(why); + } else { + state.allowed += 1; + } }; state.remove = () => window.removeEventListener('submit', handler, true); window.addEventListener('submit', handler, true); - setTimeout(state.remove, 30000); + setTimeout(state.remove, 120000); Object.defineProperty( window, args.key, {value: state, configurable: true, writable: true}); return true; }""" ) -_FORM_SUBMIT_GUARD_RESULT_JS = r"""(key) => { - const state = window[key]; +# Read the guard's refusals; with ``disarm`` also remove it. +_FORM_SUBMIT_GUARD_RESULT_JS = r"""(args) => { + const state = window[args.key]; if (!state) return null; - if (state.remove) state.remove(); - delete window[key]; - return state.refused || null; + const refused = state.refused.slice(); + if (args.disarm) { + if (state.remove) state.remove(); + delete window[args.key]; + } + return refused.length ? refused.join('; ') : null; }""" # Page-structure identity of a FORM FIELD that sits in no record row. @@ -2418,7 +2446,12 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s try: armed = point.scope.evaluate( _ARM_FORM_SUBMIT_GUARD_JS, - {"x": point.x, "y": point.y, "key": self._form_guard_slot}, + { + "x": point.x, + "y": point.y, + "key": self._form_guard_slot, + "exempt": list(identity.exempt), + }, ) except Exception: armed = False @@ -2427,28 +2460,33 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s self._form_guard_scope = point.scope return None - def form_submit_guard_refusal(self) -> Optional[str]: - """After the click: why the guard cancelled a submit, or None. + def form_submit_guard_refusal(self, *, disarm: bool = False) -> Optional[str]: + """Why the guard cancelled a submit so far, or None. ``disarm`` also + removes the guard. A page that navigated away (the checked form's own + submit) has nothing to report.""" - Disarms the guard. A page that navigated away (the checked form's own - submit) has nothing to report. - """ - - scope, self._form_guard_scope = self._form_guard_scope, None + scope = self._form_guard_scope if scope is None: return None + if disarm: + self._form_guard_scope = None try: refused = scope.evaluate( - _FORM_SUBMIT_GUARD_RESULT_JS, self._form_guard_slot + _FORM_SUBMIT_GUARD_RESULT_JS, + {"key": self._form_guard_slot, "disarm": disarm}, ) except Exception: return None return refused if isinstance(refused, str) else None - def cancel_form_submit_guard(self) -> None: - """Disarm a guard whose click never happened.""" + @property + def form_submit_guard_armed(self) -> bool: + return self._form_guard_scope is not None + + def cancel_form_submit_guard(self) -> Optional[str]: + """Read the guard's last refusals and disarm it.""" - self.form_submit_guard_refusal() + return self.form_submit_guard_refusal(disarm=True) def form_values_at(self, x: int, y: int) -> Optional[dict[str, Any]]: """RECORD-TIME: the values of the form's fields the demonstration did diff --git a/packages/flow/openadapt_flow/interactive_recorder.py b/packages/flow/openadapt_flow/interactive_recorder.py index 49c4dc5660..d3bc157b36 100644 --- a/packages/flow/openadapt_flow/interactive_recorder.py +++ b/packages/flow/openadapt_flow/interactive_recorder.py @@ -443,9 +443,6 @@ def select_attached_page( const SUBMIT_FIELD = __SUBMIT_FIELD__; const observeSubmitBinding = __OBSERVE_SUBMIT_BINDING__; const filledFields = []; - // Each hidden control's value as typing starts: a create-new form's - // fingerprint names the hidden tokens that rotate while it is filled in. - const hiddenBaseline = new WeakMap(); // One identity for this DOCUMENT. The init script builds a fresh closure per // document, so this closure never saw a value an earlier document received. // Python compares this id against the document that received a secret and @@ -1942,7 +1939,7 @@ def select_attached_page( ? target.closest(SUBMIT_ACTIONABLE) : null; if (!control || control.matches( 'canvas, input[type="range"], [role="slider"]')) return null; - const out = observeSubmitBinding(control, filledFields, hiddenBaseline); + const out = observeSubmitBinding(control, filledFields); if (!out || !out.observation) return null; // A labelled control matters only as the save of the form the fields // were typed into (openadapt_flow.ir.FormIdentity). @@ -2152,9 +2149,6 @@ def select_attached_page( if (e.isTrusted && el.matches(SUBMIT_FIELD) && filledFields.indexOf(el) < 0) { filledFields.push(el); - for (const input of document.querySelectorAll('input[type="hidden"]')) { - if (!hiddenBaseline.has(input)) hiddenBaseline.set(input, String(input.value)); - } } } catch (err) {} emitInFrameSpace(o); diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index 20e2c155cf..bc8f97af8d 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -747,6 +747,11 @@ class _WebFormApprovalView: PIXEL_CENSUS_OCR_MAX_SIMILARITY = 0.6 +#: How long after a create-new form's save the replayer keeps watching its +#: submit guard before the next step (a script may submit after the click). +_FORM_GUARD_SETTLE_S = 2.0 + + class Replayer: """Replays a Workflow against a Backend using injected vision. @@ -13006,21 +13011,41 @@ def _act( guarded = step.anchor is not None and step.anchor.form_identity is not None backend = self.backend - try: - error = self._act_closing_surface(step, resolution, params, **kwargs) - finally: - # A create-new form's save: the identity check armed a guard that - # cancels any other submit during the click (ir.FormIdentity). - refusal_of = getattr(backend, "form_submit_guard_refusal", None) - refusal = refusal_of() if guarded and callable(refusal_of) else None - if refusal is not None and error is None: - result = kwargs.get("result") + result = kwargs.get("result") + + def halt(message: str) -> str: if result is not None: result.safety_halt = True result.failure_category = "safety_halt" - return ( - f"Step '{step.id}' ({step.intent}): {refusal}; the submission was " - "cancelled before it left the page — run aborted" + return message + + # A create-new form's save armed a guard that cancels any submit that + # isn't exactly the checked one (ir.FormIdentity). It stays armed + # after its click; the next step reads what it cancelled since, then + # disarms it so that step can submit its own forms. + if not guarded and getattr(backend, "form_submit_guard_armed", False): + late = backend.cancel_form_submit_guard() # type: ignore[attr-defined] + if late is not None: + return halt( + f"Step '{step.id}' ({step.intent}): after the previous save, " + f"{late}; that submission was cancelled before it left the " + "page — run aborted" + ) + error = self._act_closing_surface(step, resolution, params, **kwargs) + refusal_of = getattr(backend, "form_submit_guard_refusal", None) + if not guarded or not callable(refusal_of): + return error + # A script may submit after the click returns (setTimeout): watch the + # guard through a short settle window. + refusal = refusal_of() + deadline = time.monotonic() + _FORM_GUARD_SETTLE_S + while refusal is None and error is None and time.monotonic() < deadline: + time.sleep(0.1) + refusal = refusal_of() + if refusal is not None and error is None: + return halt( + f"Step '{step.id}' ({step.intent}): {refusal}; that submission " + "was cancelled before it left the page — run aborted" ) return error diff --git a/packages/flow/openadapt_flow/submit_binding.py b/packages/flow/openadapt_flow/submit_binding.py index 0c2c80c456..3454f3485f 100644 --- a/packages/flow/openadapt_flow/submit_binding.py +++ b/packages/flow/openadapt_flow/submit_binding.py @@ -106,7 +106,7 @@ #: the list of elements the program (or the demonstrator) typed into. Returns #: the structural observation and ``canon``, its canonical JSON (with the raw #: icon geometry), which the delivery guard recomputes and compares. -_OBSERVE_SUBMIT_BINDING_TEMPLATE = r"""(control, fields, baseline) => { +_OBSERVE_SUBMIT_BINDING_TEMPLATE = r"""(control, fields) => { const ACTIONABLE = __ACTIONABLE__; const FIELD = __FIELD__; const ROW = 'tr, [role="row"], li, [role="listitem"]'; @@ -181,23 +181,23 @@ earlier.compareDocumentPosition(later) & Node.DOCUMENT_POSITION_FOLLOWING ) && !earlier.contains(later); - const formFingerprint = (form, submitter, typed, baseline) => { - // Where and how the form submits (origin and path only), the name and - // type of every control the submission would carry, and the values of - // the controls the run did not type into. Hidden values that changed - // since the first field was filled (a rotating token) are named. + const formFingerprint = (form, submitter, typed) => { + // Where and how the form submits (origin, path and query), the + // name and type of every control the submission would carry, and + // the values of the controls the run did not type into. let action = ''; + const query = []; try { const target = submitter.hasAttribute('formaction') ? submitter.formAction : form.action; const url = new URL(target, doc.baseURI); action = url.origin + url.pathname; + for (const [key, value] of url.searchParams) query.push([key, value]); } catch (_) {} const method = String((submitter.hasAttribute('formmethod') ? submitter.formMethod : form.method) || 'get').toLowerCase(); const controls = []; const values = []; - const rotated = []; for (const el of Array.from(form.elements)) { const name = el.name; if (!name) continue; @@ -209,10 +209,16 @@ controls.push(name + '\t' + type); continue; } - if (['submit', 'button', 'reset', 'image', 'file'].indexOf(type) >= 0 || + if (['submit', 'button', 'reset', 'image'].indexOf(type) >= 0 || ['fieldset', 'object', 'output'].indexOf(el.localName) >= 0) { continue; } + if (type === 'file') { + // How many files it would send: an unexpected upload changes it. + controls.push(name + '\t' + type); + values.push([name, type, String(el.files ? el.files.length : 0)]); + continue; + } if ((type === 'checkbox' || type === 'radio') && !el.checked) continue; controls.push(name + '\t' + type); // A password's value never leaves the page. @@ -224,13 +230,9 @@ } else { values.push([name, type, String(el.value)]); } - if (type === 'hidden' && baseline instanceof WeakMap && - baseline.has(el) && baseline.get(el) !== String(el.value)) { - rotated.push(name); - } } controls.sort(); - return {action, method, controls, values, rotated}; + return {action, query, method, controls, values}; }; const fieldLocal = (el, stop) => { // The first ancestor that holds any field decides: exactly one field @@ -381,7 +383,7 @@ // can tell this form from an edit form or a form scoped elsewhere // that looks the same (openadapt_flow.ir.FormIdentity). if (formOwner === 'shared' && result.submit_role === 'form_submit') { - result.form = formFingerprint(scope, control, live, baseline); + result.form = formFingerprint(scope, control, live); } return {observation: result, canon: JSON.stringify(result)}; } @@ -476,14 +478,26 @@ class SubmitBindingObservation(BaseModel): ) +def _is_token(name: str, value: str) -> bool: + """A hidden value (or query value) that is a per-session or per-page + token: named like one AND shaped like one (at least 16 characters, not + all digits). A name alone (``encounter_token=7``) or a value that merely + changed while the form was filled (a parent id a script sets) is not + enough: those are compared.""" + + bare = name.lstrip("?") + return bool(_TOKEN_NAME.search(bare)) and len(value) >= 16 and not value.isdigit() + + @dataclass(frozen=True) class FormValues: """Values of the controls a run did not type into, by control name. Plaintext, so it never goes into an observation or a bundle: the compiler keeps salted digests (:func:`decide_form_identity`) and a run - compares digests. ``exempt`` names hidden tokens that are not compared: - named like one, or changed while the demonstration was being typed. + compares digests. ``exempt`` names hidden or query tokens that are not + compared: named like a token and shaped like one (see :func:`_is_token`). + Query parameter values are keyed ``?name``. """ values: Mapping[str, tuple[str, ...]] @@ -499,14 +513,18 @@ def form_values_from_page(raw: object) -> Optional[FormValues]: form = raw.get("form") if not isinstance(form, Mapping): return None - rotated = {str(name) for name in form.get("rotated") or ()} grouped: dict[str, list[str]] = {} exempt: set[str] = set() - for item in form.get("values") or (): - if not isinstance(item, (list, tuple)) or len(item) != 3: + items = [tuple(str(part) for part in item) for item in form.get("values") or ()] + for item in form.get("query") or (): + if not isinstance(item, (list, tuple)) or len(item) != 2: + raise ValueError("form query values are malformed") + items.append(("?" + str(item[0]), "query", str(item[1]))) + for item in items: + if len(item) != 3: raise ValueError("form values are malformed") - name, kind, value = (str(part) for part in item) - if kind == "hidden" and (name in rotated or _TOKEN_NAME.search(name)): + name, kind, value = item + if kind in ("hidden", "query") and _is_token(name, value): exempt.add(name) continue grouped.setdefault(name, []).append(value) @@ -566,6 +584,14 @@ def observation_from_page(raw: object) -> SubmitBindingObservation: if not isinstance(form, Mapping): raise ValueError("submit-binding form fingerprint is malformed") action = str(form.get("action") or "") + query = form.get("query") or [] + if not isinstance(query, list) or not all( + isinstance(q, (list, tuple)) and len(q) == 2 for q in query + ): + raise ValueError("submit-binding form query is malformed") + # The address includes its query parameter NAMES; their values are + # compared like the form's other untouched values. + action += "?" + "&".join(sorted(str(q[0]) for q in query)) controls = form.get("controls") or [] if not isinstance(controls, list) or not all( isinstance(c, str) for c in controls diff --git a/packages/flow/tests/test_create_form_identity.py b/packages/flow/tests/test_create_form_identity.py index 5f0d0917b5..66f5725516 100644 --- a/packages/flow/tests/test_create_form_identity.py +++ b/packages/flow/tests/test_create_form_identity.py @@ -73,6 +73,49 @@ # Variants from the #1543 review: each looks like the same create form, and # each would submit the wrong thing if only the typed fields were checked. _SCRIPTS = { + # Re-review round 2. A click handler copies the page's current parent id + # into an empty hidden field as the form is sent. + "copy7": """ +window.currentPid = '7'; +document.querySelector('[name=submit]').addEventListener('click', () => { + document.querySelector('[name=pid]').value = window.currentPid; +});""", + "copy9": """ +window.currentPid = '9'; +document.querySelector('[name=submit]').addEventListener('click', () => { + document.querySelector('[name=pid]').value = window.currentPid; +});""", + # A disabled hidden record id the click handler enables. + "toggle": """ +document.querySelector('[name=submit]').addEventListener('click', () => { + document.querySelector('[name=id]').disabled = false; +});""", + # The click handler waits, then submits another form. + "delayed_other": """ +document.querySelector('[name=submit]').addEventListener('click', (e) => { + e.preventDefault(); + setTimeout(() => document.getElementById('other').requestSubmit(), 300); +});""", + # The click handler waits, adds a record id, then submits with the button. + "delayed_mutate": """ +document.querySelector('[name=submit]').addEventListener('click', (e) => { + e.preventDefault(); + setTimeout(() => { + const f = document.getElementById('ticket'); + const h = document.createElement('input'); h.type = 'hidden'; h.name = 'id'; + h.value = '42'; f.appendChild(h); + f.requestSubmit(document.querySelector('[name=submit]')); + }, 300); +});""", + # A script sets the hidden parent id when typing starts. + "pid_input7": """ +document.querySelector('[name=field_summary]').addEventListener('input', () => { + document.querySelector('[name=pid]').value = '7'; +}, {once: true});""", + "pid_input9": """ +document.querySelector('[name=field_summary]').addEventListener('input', () => { + document.querySelector('[name=pid]').value = '9'; +}, {once: true});""", # A script sets a different action and method, and adds a record id, # after the run has typed. "action_swap": """ @@ -122,6 +165,19 @@ def _page(variant: str) -> str: else (_PORTAL if variant == "portal" else "") ) title, action, hidden, extra = "New ticket", "/newticket", "", "" + if variant in ("query_new", "query_42"): + action = "/save?mode=new" if variant == "query_new" else "/save?id=42" + if variant in ("copy7", "copy9", "pid_input7", "pid_input9"): + hidden = '' + if variant == "toggle": + hidden = '' + if variant in ("enc7", "enc9"): + # Named like a token, but a short number: a record id, not a token. + hidden = ( + '' + ) if variant == "edit": # Ticket 42's edit form: the same blank fields, a record id, another # address. Its save updates ticket 42. @@ -454,26 +510,72 @@ def test_a_click_that_submits_another_form_is_cancelled( assert "submitted a different form" in (save.error or "") -@pytest.mark.parametrize("variant", ["csrf", "rotating"]) -def test_a_rotating_token_still_verifies( - url: str, tmp_path: Path, variant: str -) -> None: - """A per-session token (named like one) and a hidden value that changed - while the demonstration was typed are not compared, so a new page load - with a new token still verifies, and the token's value is never kept.""" +def test_a_per_session_token_still_verifies(url: str, tmp_path: Path) -> None: + """A hidden token named like one and shaped like one (Trac's + __FORM_TOKEN) differs on every page load. It isn't compared, and its + value is never kept, so a new load with a new token still verifies.""" - bundle = _record(f"{url}?v={variant}", tmp_path / "rec") + bundle = _record(f"{url}?v=csrf", tmp_path / "rec") save = Workflow.load(bundle).steps[-1] assert save.anchor is not None and save.anchor.form_identity is not None - assert save.anchor.form_identity.exempt == ( - ("__FORM_TOKEN",) if variant == "csrf" else ("draft_rev",) - ) - report, created = _replay(f"{url}?v={variant}", bundle, tmp_path / "run") + assert save.anchor.form_identity.exempt == ("__FORM_TOKEN",) + report, created = _replay(f"{url}?v=csrf", bundle, tmp_path / "run") assert report.success, [(r.step_id, r.error) for r in report.results] assert created == ["Scanner offline on floor 3"] assert report.results[-1].identity.status == "verified" +@pytest.mark.parametrize( + "recorded, replayed, expected", + [ + # A hidden value that changed while the demonstration was typed is + # not exempt for that reason alone. + ("rotating", "rotating", "'draft_rev' field, which this run did not type"), + # A parent id a script sets when typing starts is compared. + ("pid_input7", "pid_input9", "'pid' field, which this run did not type"), + # A short number under a token-like name is compared. + ("enc7", "enc9", "'encounter_token' field, which this run did not type"), + # The address's query parameter names count. + ("query_new", "query_42", "submits to a different address"), + ], +) +def test_values_that_only_look_like_tokens_are_compared( + url: str, tmp_path: Path, recorded: str, replayed: str, expected: str +) -> None: + bundle = _record(f"{url}?v={recorded}", tmp_path / "rec") + report, created = _replay(f"{url}?v={replayed}", bundle, tmp_path / "run") + _halted_by_form(report, created, expected) + + +@pytest.mark.parametrize( + "recorded, replayed, expected", + [ + ("copy7", "copy9", "what the form would send changed after it was checked"), + ("plain", "toggle", "what the form would send changed after it was checked"), + ("plain", "delayed_other", "submitted a different form"), + ( + "plain", + "delayed_mutate", + "what the form would send changed after it was checked", + ), + ], +) +def test_a_submission_that_differs_from_the_checked_one_is_cancelled( + url: str, tmp_path: Path, recorded: str, replayed: str, expected: str +) -> None: + """The form checks out before the click; then a click handler changes + what is sent, or submits something else, now or 300 ms later. The guard + cancels it and the run halts.""" + + bundle = _record(f"{url}?v={recorded}", tmp_path / "rec") + report, created = _replay(f"{url}?v={replayed}", bundle, tmp_path / "run") + assert not report.success + assert created == [] + save = report.results[-1] + assert save.identity is not None and save.identity.status == "verified" + assert expected in (save.error or ""), save.error + + def test_a_keyed_remount_halts_a_known_limit( url: str, bundle: Path, tmp_path: Path ) -> None: From 95980721cef7abf1a681dc3d1d6296016de259b2 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 28 Oct 2026 15:25:33 -0400 Subject: [PATCH 03/11] fix: hold a create-form save at the network until it matches the checked form Round 3 of #1581's review: page listeners that run after every in-page check (a form submit listener, a document capture listener, a formdata listener) could still change what a save carries. So the Playwright backend now routes every write request, and a GET navigation to the form's address, through a wire guard from the identity check until the next step. A request continues only when its method, address, query and fields (URL-encoded, multipart or flat JSON) equal what the checked button would send, and only once; anything else, including a shape it can't parse, is aborted before it leaves the browser and halts the run. The expected fields are built from the form's controls, never from new FormData(), which would run the page's formdata listeners. A cancelled submit found by the next form check, or at the end of the run, halts. A record-shaped value (UUID-like hex groups) under a token-like name is compared unless it changed while the demonstration was typed (the rotation check is back for this). Tests (test_create_form_wire.py): a server records what arrives; native POST, fetch-JSON and the review's submit-listener, document-capture and formdata cases. Both test files assert each save recorded armed, so a slow recording can't pass a case for the wrong reason. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/LIMITS.md | 27 +- .../backends/playwright_backend.py | 276 ++++++++++++++++-- .../openadapt_flow/interactive_recorder.py | 8 +- .../flow/openadapt_flow/runtime/replayer.py | 31 +- .../flow/openadapt_flow/submit_binding.py | 40 ++- .../flow/tests/test_create_form_identity.py | 20 +- packages/flow/tests/test_create_form_wire.py | 248 ++++++++++++++++ 7 files changed, 596 insertions(+), 54 deletions(-) create mode 100644 packages/flow/tests/test_create_form_wire.py diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index c87330863e..df7a31f213 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -395,25 +395,34 @@ field typed since the last save, the compiler gives it a **form identity** its demonstrated value, stored as a salted digest. Only a per-page token is not compared: a hidden field or query value named like one (`csrf`, `token`, `nonce` and the like) whose value is shaped like one (16 characters or more, - not all digits). A short or numeric value under such a name, and a value a + not all digits) and is not shaped like a record id (hex groups joined by `-` + or `_`, as a UUID is) unless it changed while the demonstration was filled + in. A short, numeric or record-shaped value under such a name, and a value a script set while the form was filled in, are compared. A program that should create under a different parent on each run therefore halts until that parent is checked another way. -From the check until the next step, a guard in the page compares every submit +From the check until the next step, the browser holds every write request the +page sends, and a GET navigation to the form's address, at the network: it +leaves only when its method, address, query and fields equal what the checked +button would send (form fields or a flat JSON object), and only once. Page +listeners that run after every in-page check (a form submit listener, a +document capture listener, a `formdata` listener, a timer) can't change a save +on its way out. A save sent in another shape (nested JSON, a custom encoding) +can't be compared, so it is held too. In the page, a guard also compares every submit with what the checked button would have sent (address and query, method, and every field's value except the exempt tokens). A submit from another form, from the right form without its save button, with contents that changed after the check (a click handler that fills in or enables a field), or a second submit, is cancelled. The run watches the guard for two seconds after the click, then again at the start of the next step, and halts on any cancelled -submit. A script that submits with `form.submit()` or `fetch` sends no submit -event, so the guard doesn't see it. A submit a script delays past the start -of the next step is still cancelled until the page changes, but the run no -longer reports it. A page whose own click handler fills in or enables a field -as the form is sent changes what is sent after the check, so its save halts -even when the demonstration did the same: a false halt, in the safe -direction. +submit. A save sent by script (`form.submit()`, `fetch`) raises no submit +event, but its request is still held at the network. A submission a script +delays past the end of the run is still held until the page changes, but the +run, already over, doesn't report it. A page whose own click or submit +listener fills in or enables a field as the form is sent changes what is sent +after the check, so its save halts even when the demonstration did the same: a +false halt, in the safe direction. The digests of untouched values are salted; with the per-bundle salt stored in the bundle (the default, unless `OPENADAPT_FLOW_IDENTITY_SALT` is set), a diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 81fdd9f661..c20f54ed8a 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -18,7 +18,7 @@ import uuid from dataclasses import dataclass, field from datetime import datetime, timezone -from typing import TYPE_CHECKING, Any, Callable, Literal, Optional +from typing import TYPE_CHECKING, Any, Callable, Literal, Mapping, Optional from urllib.parse import urlsplit from openadapt_flow.backends._page_quiescence import PageQuiescence @@ -676,7 +676,9 @@ class _FramePoint: + OBSERVE_SUBMIT_BINDING_JS + r"""; const fills = window[args.fillKey]; - return observe(control, Array.isArray(fills) ? fills : []); + return observe( + control, Array.isArray(fills) ? fills : [], window[args.fillKey + '_hidden'] + ); }""" ) @@ -694,6 +696,17 @@ class _FramePoint: Object.defineProperty(window, key, {value: fills, configurable: true}); } if (fills.indexOf(el) < 0) fills.push(el); + // Each hidden control's value as typing starts, so a create-new form's + // fingerprint can tell a token that rotates while the form is filled in. + let hidden = window[key + '_hidden']; + if (!(hidden instanceof WeakMap)) { + hidden = new WeakMap(); + Object.defineProperty( + window, key + '_hidden', {value: hidden, configurable: true}); + } + for (const input of el.ownerDocument.querySelectorAll('input[type="hidden"]')) { + if (!hidden.has(input)) hidden.set(input, String(input.value)); + } return true; }""" ) @@ -742,7 +755,7 @@ class _FramePoint: + OBSERVE_SUBMIT_BINDING_JS + r"""; const fills = Array.isArray(window[args.fillKey]) ? window[args.fillKey] : []; - const out = observe(control, fills); + const out = observe(control, fills, window[args.fillKey + '_hidden']); const values = window[args.valueKey]; const form = control.form || control.closest('form'); let changed = 0; @@ -775,29 +788,51 @@ class _FramePoint: const form = control ? (control.form || control.closest('form')) : null; if (!form) return false; const exempt = new Set(args.exempt || []); - const canon = () => { - let data; - try { data = new FormData(form, control); } catch (_) { data = new FormData(form); } + // What the button would send, built from the form's controls: never from + // new FormData(), which runs the page's own formdata listeners. + const expected_parts = () => { const pairs = []; - for (const [key, value] of data.entries()) { - if (exempt.has(key)) continue; - pairs.push([key, (typeof value === 'string') ? value - : ('file:' + value.name + ':' + value.size)]); + for (const el of Array.from(form.elements)) { + const name = el.name; + if (!name || exempt.has(name)) continue; + let disabled = false; + try { disabled = el.matches(':disabled'); } catch (_) {} + if (disabled) continue; + const type = String(el.type || el.localName || '').toLowerCase(); + if (['submit', 'button', 'reset', 'image'].indexOf(type) >= 0) { + if (el === control && type !== 'image') pairs.push([name, String(el.value)]); + continue; + } + if (['fieldset', 'object', 'output'].indexOf(el.localName) >= 0) continue; + if ((type === 'checkbox' || type === 'radio') && !el.checked) continue; + if (type === 'file') { + const files = Array.from(el.files || []); + if (!files.length) pairs.push([name, 'file:']); + for (const f of files) pairs.push([name, 'file:' + f.name]); + continue; + } + if (el.localName === 'select') { + for (const o of Array.from(el.selectedOptions)) pairs.push([name, String(o.value)]); + continue; + } + pairs.push([name, String(el.value)]); } - pairs.sort((x, y) => (x[0] + '\u0000' + x[1]) < (y[0] + '\u0000' + y[1]) ? -1 : 1); const target = control.hasAttribute('formaction') ? control.formAction : form.action; - let address = target; + let address = String(target); + const query = []; try { const url = new URL(target, document.baseURI); - const query = []; + address = url.origin + url.pathname; for (const [k, v] of url.searchParams) if (!exempt.has('?' + k)) query.push([k, v]); - query.sort(); - address = url.origin + url.pathname + '?' + JSON.stringify(query); } catch (_) {} const method = String((control.hasAttribute('formmethod') ? control.formMethod : form.method) || 'get').toLowerCase(); - return JSON.stringify([address, method, pairs]); + const order = (x, y) => (x[0] + '\u0000' + x[1]) < (y[0] + '\u0000' + y[1]) ? -1 : 1; + pairs.sort(order); + query.sort(order); + return {address, query, method, pairs}; }; + const canon = () => JSON.stringify(expected_parts()); const previous = window[args.key]; if (previous && previous.remove) previous.remove(); const expected = canon(); @@ -825,10 +860,111 @@ class _FramePoint: setTimeout(state.remove, 120000); Object.defineProperty( window, args.key, {value: state, configurable: true, writable: true}); - return true; + return expected_parts(); }""" ) + +def _wire_pairs(request: Any) -> Optional[list[tuple[str, str]]]: + """The fields a request carries in its body, or None when its body is + not a form or a flat JSON object.""" + + body = request.post_data_buffer or b"" + ctype = (request.headers.get("content-type") or "").lower() + if not body: + return [] + if ctype.startswith("application/x-www-form-urlencoded"): + from urllib.parse import parse_qsl + + return parse_qsl(body.decode("utf-8", "replace"), keep_blank_values=True) + if ctype.startswith("multipart/form-data"): + from email.parser import BytesParser + from email.policy import HTTP + + message = BytesParser(policy=HTTP).parsebytes( + b"Content-Type: " + ctype.encode() + b"\r\n\r\n" + body + ) + pairs: list[tuple[str, str]] = [] + for part in message.iter_parts(): + name = part.get_param("name", header="content-disposition") + filename = part.get_param("filename", header="content-disposition") + if name is None: + continue + if filename is not None: + pairs.append((str(name), f"file:{filename}")) + else: + payload = part.get_payload(decode=True) or b"" + pairs.append((str(name), payload.decode("utf-8", "replace"))) + return pairs + if ctype.startswith("application/json"): + try: + data = json.loads(body) + except ValueError: + return None + if not isinstance(data, dict) or any( + isinstance(v, (dict, list)) for v in data.values() + ): + return None + return [(str(k), "" if v is None else str(v)) for k, v in data.items()] + return None + + +def _wire_mismatch(request: Any, state: Mapping[str, Any]) -> Optional[str]: + """``""``: not the guard's business. ``None``: the checked submission. + Otherwise why the request must not be sent.""" + + from urllib.parse import parse_qsl, urlsplit + + method = request.method.upper() + expected = state["expected"] + exempt = state["exempt"] + parts = urlsplit(request.url) + address = f"{parts.scheme}://{parts.netloc}{parts.path}" + writes = method not in ("GET", "HEAD", "OPTIONS") + navigation_here = ( + method == "GET" + and request.is_navigation_request() + and address == expected["address"] + ) + if not (writes or navigation_here) or request.redirected_from is not None: + return "" + if state["sent"]: + return "the page sent a second submission after the checked one" + if method.lower() != expected["method"]: + return ( + f"the save was sent with {method}, the checked form sends with " + f"{expected['method'].upper()}" + ) + if address != expected["address"]: + return "the save was sent to a different address than the checked form's" + + def norm(pairs: Any, *, query: bool = False) -> list[tuple[str, str]]: + out = [] + for key, value in pairs: + key = str(key) + if (("?" + key) if query else key) in exempt: + continue + out.append((key, str(value).replace("\r\n", "\n"))) + return sorted(out) + + url_query = parse_qsl(parts.query, keep_blank_values=True) + if method == "GET": + sent = norm(url_query) + else: + if norm(url_query, query=True) != norm(expected["query"], query=True): + return "the save was sent with a different query than the checked form's" + body = _wire_pairs(request) + if body is None: + return ( + "the save was sent in a shape this check can't compare with " + "the form (not form fields or flat JSON)" + ) + sent = norm(body) + if sent != norm(expected["pairs"]): + return "what the save sent differs from the checked form" + return None + + # Read the guard's refusals; with ``disarm`` also remove it. _FORM_SUBMIT_GUARD_RESULT_JS = r"""(args) => { const state = window[args.key]; @@ -1750,6 +1886,7 @@ def __init__( self._fill_value_slot = f"__oaflow_filled_values_{uuid.uuid4().hex}" self._form_guard_slot = f"__oaflow_form_guard_{uuid.uuid4().hex}" self._form_guard_scope: Any = None + self._form_wire: Any = None self._structural_tokens: dict[str, _StructuralGuard] = {} self._guarded_coordinate: Optional[_CoordinateGuard] = None # One-shot expectation that the next click closes this page surface @@ -2415,7 +2552,9 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s reason, in plain words, including when the page can't be read. """ - self.cancel_form_submit_guard() + late = self.cancel_form_submit_guard() + if late is not None: + return f"after the previous save, {late}; that submission was cancelled" try: point = self._frame_point(int(x), int(y)) if point is None: @@ -2454,34 +2593,111 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s }, ) except Exception: - armed = False - if armed is not True: + armed = None + if not isinstance(armed, dict): return "could not guard the form's submission" self._form_guard_scope = point.scope + self._arm_form_wire_guard(armed, set(identity.exempt)) return None + # -- the wire guard: what the browser actually sends ---------------------- + + def _arm_form_wire_guard( + self, expected: Mapping[str, Any], exempt: set[str] + ) -> None: + """Compare the request the save actually sends with the checked form. + + Page listeners (a submit or formdata listener, a timer) can still + change a submission after every in-page check. So until the guard is + disarmed, every write request the page sends, and a GET navigation to + the form's address, is held at the network: it continues only when + its method, address, query and fields equal what the checked button + would have sent, and only once. Anything else is aborted before it + leaves the browser, and recorded. + """ + + self._disarm_form_wire_guard() + state: dict[str, Any] = { + "expected": expected, + "exempt": exempt, + "sent": False, + "refused": [], + } + + def handler(route: Any) -> None: + try: + why = _wire_mismatch(route.request, state) + except Exception: # noqa: BLE001 - unreadable request: refuse + why = "the save's request could not be read" + if why == "": + route.fallback() + return + if why is None: + state["sent"] = True + route.fallback() + return + state["refused"].append(why) + try: + route.abort() + except Exception: # noqa: BLE001 - already handled + pass + + self.page.route("**/*", handler) + self._form_wire = (handler, state) + + def _disarm_form_wire_guard(self) -> list[str]: + wire = getattr(self, "_form_wire", None) + self._form_wire = None + if wire is None: + return [] + handler, state = wire + try: + self.page.unroute("**/*", handler) + except Exception: # noqa: BLE001 - the page is gone + pass + return list(state["refused"]) + + def wait_form_submit_guard(self, ms: int) -> None: + """Let the page and the wire guard run for ``ms`` milliseconds.""" + + try: + self.page.wait_for_timeout(ms) + except Exception: # noqa: BLE001 - the page is gone + time.sleep(ms / 1000) + def form_submit_guard_refusal(self, *, disarm: bool = False) -> Optional[str]: """Why the guard cancelled a submit so far, or None. ``disarm`` also removes the guard. A page that navigated away (the checked form's own submit) has nothing to report.""" scope = self._form_guard_scope - if scope is None: + if scope is None and getattr(self, "_form_wire", None) is None: return None + reasons: list[str] = [] + wire = getattr(self, "_form_wire", None) + if wire is not None: + reasons.extend(wire[1]["refused"]) if disarm: self._form_guard_scope = None - try: - refused = scope.evaluate( - _FORM_SUBMIT_GUARD_RESULT_JS, - {"key": self._form_guard_slot, "disarm": disarm}, - ) - except Exception: - return None - return refused if isinstance(refused, str) else None + self._disarm_form_wire_guard() + if scope is not None: + try: + refused = scope.evaluate( + _FORM_SUBMIT_GUARD_RESULT_JS, + {"key": self._form_guard_slot, "disarm": disarm}, + ) + except Exception: + refused = None + if isinstance(refused, str): + reasons.insert(0, refused) + return "; ".join(dict.fromkeys(reasons)) if reasons else None @property def form_submit_guard_armed(self) -> bool: - return self._form_guard_scope is not None + return ( + self._form_guard_scope is not None + or getattr(self, "_form_wire", None) is not None + ) def cancel_form_submit_guard(self) -> Optional[str]: """Read the guard's last refusals and disarm it.""" diff --git a/packages/flow/openadapt_flow/interactive_recorder.py b/packages/flow/openadapt_flow/interactive_recorder.py index d3bc157b36..04515f5b23 100644 --- a/packages/flow/openadapt_flow/interactive_recorder.py +++ b/packages/flow/openadapt_flow/interactive_recorder.py @@ -443,6 +443,9 @@ def select_attached_page( const SUBMIT_FIELD = __SUBMIT_FIELD__; const observeSubmitBinding = __OBSERVE_SUBMIT_BINDING__; const filledFields = []; + // Each hidden control's value as typing starts: a create-new form's + // fingerprint names the hidden values that change while it is filled in. + const hiddenBaseline = new WeakMap(); // One identity for this DOCUMENT. The init script builds a fresh closure per // document, so this closure never saw a value an earlier document received. // Python compares this id against the document that received a secret and @@ -1939,7 +1942,7 @@ def select_attached_page( ? target.closest(SUBMIT_ACTIONABLE) : null; if (!control || control.matches( 'canvas, input[type="range"], [role="slider"]')) return null; - const out = observeSubmitBinding(control, filledFields); + const out = observeSubmitBinding(control, filledFields, hiddenBaseline); if (!out || !out.observation) return null; // A labelled control matters only as the save of the form the fields // were typed into (openadapt_flow.ir.FormIdentity). @@ -2149,6 +2152,9 @@ def select_attached_page( if (e.isTrusted && el.matches(SUBMIT_FIELD) && filledFields.indexOf(el) < 0) { filledFields.push(el); + for (const input of document.querySelectorAll('input[type="hidden"]')) { + if (!hiddenBaseline.has(input)) hiddenBaseline.set(input, String(input.value)); + } } } catch (err) {} emitInFrameSpace(o); diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index bc8f97af8d..53743d025f 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -2539,6 +2539,7 @@ def acknowledge_delivery(self_nonlocal) -> None: report.success = len(report.results) == len(workflow.steps) and all( result.ok for result in report.results ) + self._halt_on_late_form_submit(report) report.total_ms = (time.monotonic() - t_run) * 1000.0 # Emit the structured HALT record on any unsuccessful run, so the @@ -13037,10 +13038,16 @@ def halt(message: str) -> str: return error # A script may submit after the click returns (setTimeout): watch the # guard through a short settle window. + wait = getattr(backend, "wait_form_submit_guard", None) refusal = refusal_of() deadline = time.monotonic() + _FORM_GUARD_SETTLE_S while refusal is None and error is None and time.monotonic() < deadline: - time.sleep(0.1) + # The wire guard runs only while the browser connection is + # serviced: wait through the backend, not with a bare sleep. + if callable(wait): + wait(100) + else: + time.sleep(0.1) refusal = refusal_of() if refusal is not None and error is None: return halt( @@ -13049,6 +13056,28 @@ def halt(message: str) -> str: ) return error + def _halt_on_late_form_submit(self, report: RunReport) -> None: + """A create-new form's submit guard still armed when the run ends + (the save was the last step): fail the run if it cancelled anything. + It stays armed, so a submission a script sends later is still + cancelled; the next step or run on this page reads it.""" + + backend = self.backend + if not getattr(backend, "form_submit_guard_armed", False): + return + late = backend.form_submit_guard_refusal() # type: ignore[attr-defined] + if late is None or not report.results: + return + last = report.results[-1] + last.ok = False + last.safety_halt = True + last.failure_category = "safety_halt" + last.error = ( + f"after the save, {late}; that submission was cancelled before it " + "left the page — run aborted" + ) + report.success = False + def _act_closing_surface( self, step: Step, diff --git a/packages/flow/openadapt_flow/submit_binding.py b/packages/flow/openadapt_flow/submit_binding.py index 3454f3485f..a3259c2347 100644 --- a/packages/flow/openadapt_flow/submit_binding.py +++ b/packages/flow/openadapt_flow/submit_binding.py @@ -106,7 +106,7 @@ #: the list of elements the program (or the demonstrator) typed into. Returns #: the structural observation and ``canon``, its canonical JSON (with the raw #: icon geometry), which the delivery guard recomputes and compares. -_OBSERVE_SUBMIT_BINDING_TEMPLATE = r"""(control, fields) => { +_OBSERVE_SUBMIT_BINDING_TEMPLATE = r"""(control, fields, baseline) => { const ACTIONABLE = __ACTIONABLE__; const FIELD = __FIELD__; const ROW = 'tr, [role="row"], li, [role="listitem"]'; @@ -181,7 +181,7 @@ earlier.compareDocumentPosition(later) & Node.DOCUMENT_POSITION_FOLLOWING ) && !earlier.contains(later); - const formFingerprint = (form, submitter, typed) => { + const formFingerprint = (form, submitter, typed, baseline) => { // Where and how the form submits (origin, path and query), the // name and type of every control the submission would carry, and // the values of the controls the run did not type into. @@ -198,6 +198,8 @@ ? submitter.formMethod : form.method) || 'get').toLowerCase(); const controls = []; const values = []; + // Hidden values that changed since the first field was filled in. + const rotated = []; for (const el of Array.from(form.elements)) { const name = el.name; if (!name) continue; @@ -230,9 +232,13 @@ } else { values.push([name, type, String(el.value)]); } + if (type === 'hidden' && baseline instanceof WeakMap && + baseline.has(el) && baseline.get(el) !== String(el.value)) { + rotated.push(name); + } } controls.sort(); - return {action, query, method, controls, values}; + return {action, query, method, controls, values, rotated}; }; const fieldLocal = (el, stop) => { // The first ancestor that holds any field decides: exactly one field @@ -383,7 +389,7 @@ // can tell this form from an edit form or a form scoped elsewhere // that looks the same (openadapt_flow.ir.FormIdentity). if (formOwner === 'shared' && result.submit_role === 'form_submit') { - result.form = formFingerprint(scope, control, live); + result.form = formFingerprint(scope, control, live, baseline); } return {observation: result, canon: JSON.stringify(result)}; } @@ -478,15 +484,26 @@ class SubmitBindingObservation(BaseModel): ) -def _is_token(name: str, value: str) -> bool: +#: A record id rather than a random token: hex groups joined by - or _ (a +#: UUID, ``enc-3f2a9c1e-0001``). +_RECORD_LIKE = re.compile( + r"^[A-Za-z]{0,12}[-_]?[0-9A-Fa-f]{4,}(?:[-_][0-9A-Fa-f]{2,})+$" +) + + +def _is_token(name: str, value: str, rotated: bool = False) -> bool: """A hidden value (or query value) that is a per-session or per-page - token: named like one AND shaped like one (at least 16 characters, not - all digits). A name alone (``encounter_token=7``) or a value that merely - changed while the form was filled (a parent id a script sets) is not - enough: those are compared.""" + token, not compared: named like one AND shaped like one (at least 16 + characters, not all digits), and not shaped like a record id unless it + changed while the demonstration was being filled in. A name alone + (``encounter_token=7``), a record-shaped value (``enc-3f2a9c1e-0001``) + and a value a script set while the form was filled in (a parent id) are + compared.""" bare = name.lstrip("?") - return bool(_TOKEN_NAME.search(bare)) and len(value) >= 16 and not value.isdigit() + if not _TOKEN_NAME.search(bare) or len(value) < 16 or value.isdigit(): + return False + return rotated or not _RECORD_LIKE.match(value) @dataclass(frozen=True) @@ -515,6 +532,7 @@ def form_values_from_page(raw: object) -> Optional[FormValues]: return None grouped: dict[str, list[str]] = {} exempt: set[str] = set() + rotated = {str(name) for name in form.get("rotated") or ()} items = [tuple(str(part) for part in item) for item in form.get("values") or ()] for item in form.get("query") or (): if not isinstance(item, (list, tuple)) or len(item) != 2: @@ -524,7 +542,7 @@ def form_values_from_page(raw: object) -> Optional[FormValues]: if len(item) != 3: raise ValueError("form values are malformed") name, kind, value = item - if kind in ("hidden", "query") and _is_token(name, value): + if kind in ("hidden", "query") and _is_token(name, value, name in rotated): exempt.add(name) continue grouped.setdefault(name, []).append(value) diff --git a/packages/flow/tests/test_create_form_identity.py b/packages/flow/tests/test_create_form_identity.py index 66f5725516..6453aba2eb 100644 --- a/packages/flow/tests/test_create_form_identity.py +++ b/packages/flow/tests/test_create_form_identity.py @@ -171,6 +171,13 @@ def _page(variant: str) -> str: hidden = '' if variant == "toggle": hidden = '' + if variant in ("uuidA", "uuidB"): + # Named and shaped like a token, but a record id (hex groups). + hidden = ( + '' + ) if variant in ("enc7", "enc9"): # Named like a token, but a short number: a record id, not a token. hidden = ( @@ -261,7 +268,7 @@ def url() -> Iterator[str]: stop() -def _record(url: str, out: Path) -> Path: +def _record(url: str, out: Path, *, armed: bool = True) -> Path: def drive(page: Any, pump: Any) -> None: def settle() -> None: for _ in range(12): @@ -278,6 +285,9 @@ def settle() -> None: settle() page.locator("[name=submit]").click() settle() + # Let the recorder capture the page after the save before it ends. + for _ in range(30): + pump() assert page.evaluate("window.created") == ["Printer jams on tray 2"] recording = record_interactive( @@ -288,6 +298,10 @@ def settle() -> None: param_fields=("field_summary", "field_description"), ) compile_recording(recording, out / "bundle", name="newticket") + # A save recorded unarmed has nothing to check: say so here, so a slow + # recording can't pass (or fail) a case for the wrong reason. + save = Workflow.load(out / "bundle").steps[-1] + assert (save.anchor is not None and save.anchor.form_identity is not None) is armed return out / "bundle" @@ -422,7 +436,7 @@ def test_a_portal_button_with_no_form_owner_is_not_armed_and_halts( # Recorded on the portal page: nothing ties the button to the form, so # the save gets no form identity and stays unarmed (a policy that # refuses unarmed saves refuses it). - portal = _record(f"{url}?v=portal", tmp_path / "portal") + portal = _record(f"{url}?v=portal", tmp_path / "portal", armed=False) save = Workflow.load(portal).steps[-1] assert save.anchor is not None and save.anchor.form_identity is None assert not is_identity_armed(save) @@ -535,6 +549,8 @@ def test_a_per_session_token_still_verifies(url: str, tmp_path: Path) -> None: ("pid_input7", "pid_input9", "'pid' field, which this run did not type"), # A short number under a token-like name is compared. ("enc7", "enc9", "'encounter_token' field, which this run did not type"), + # A record-shaped value under a token-like name is compared too. + ("uuidA", "uuidB", "'encounter_token' field, which this run did not type"), # The address's query parameter names count. ("query_new", "query_42", "submits to a different address"), ], diff --git a/packages/flow/tests/test_create_form_wire.py b/packages/flow/tests/test_create_form_wire.py new file mode 100644 index 0000000000..f2d8e5a21b --- /dev/null +++ b/packages/flow/tests/test_create_form_wire.py @@ -0,0 +1,248 @@ +"""A create-new form's save is checked on the wire, not in the page. + +The #1581 round-3 review found that page listeners which run after every +in-page check (a form submit listener, a document capture listener, a +formdata listener) can still change what a submission carries. So the +Playwright backend holds the save's request at the network and lets it go +only when its method, address and fields equal what the checked form would +send. + +Real headless Chromium; the server records every request it receives, so +each case asserts what actually arrived. +""" + +from __future__ import annotations + +import http.server +import json +import threading +from pathlib import Path +from typing import Any, Callable, Iterator +from urllib.parse import parse_qs, parse_qsl, urlsplit + +import pytest + +from openadapt_flow.compiler import compile_recording +from openadapt_flow.interactive_recorder import record_interactive +from openadapt_flow.ir import Workflow + +sync_api = pytest.importorskip("playwright.sync_api") + +pytestmark = pytest.mark.timeout(900) + +_SCRIPTS = { + # A form submit listener copies the app's current parent id in. + "sub7": "window.currentPid = '7';", + "sub9": "window.currentPid = '9';", + # A document capture listener adds a record id: an edit-shaped submit. + "bubble42": """ +document.addEventListener('submit', (e) => { + const h = document.createElement('input'); h.type = 'hidden'; h.name = 'id'; + h.value = '42'; e.target.appendChild(h); +}, true);""", + # A formdata listener injects a record id into the entry list. + "fd42": """ +document.getElementById('ticket').addEventListener('formdata', (e) => { + e.formData.set('id', '42'); +});""", + # An SPA: the submit listener sends the form as JSON with fetch. + "fetch": """ +document.getElementById('ticket').addEventListener('submit', (e) => { + e.preventDefault(); + const data = Object.fromEntries(new FormData(e.target, e.submitter).entries()); + fetch('/ticket', {method: 'POST', headers: {'Content-Type': 'application/json'}, + body: JSON.stringify(data)}).then(() => { + document.body.innerHTML = '

Ticket created

'; + }); +});""", + "fetch42": """ +document.getElementById('ticket').addEventListener('submit', (e) => { + e.preventDefault(); + const data = Object.fromEntries(new FormData(e.target, e.submitter).entries()); + data.id = '42'; + fetch('/ticket', {method: 'POST', headers: {'Content-Type': 'application/json'}, + body: JSON.stringify(data)}).then(() => { + document.body.innerHTML = '

Ticket created

'; + }); +});""", +} +_SUBMIT_COPIES = """ +document.getElementById('ticket').addEventListener('submit', () => { + document.querySelector('[name=pid]').value = window.currentPid; +}, true);""" + + +def _page(variant: str) -> str: + pid = ( + '' if variant.startswith("sub") else "" + ) + scripts = _SCRIPTS.get(variant, "") + ( + _SUBMIT_COPIES if variant.startswith("sub") else "" + ) + return f"""New ticket +

New ticket

+
+ {pid} +

+

+

+
+ +""" + + +class _Server: + def __init__(self) -> None: + self.received: list[dict[str, Any]] = [] + server = self + + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self) -> None: # noqa: N802 (http.server API) + query = parse_qs(urlsplit(self.path).query) + self._send(_page(query.get("v", ["plain"])[0])) + + def do_POST(self) -> None: # noqa: N802 + length = int(self.headers.get("Content-Length") or 0) + body = self.rfile.read(length).decode() + if "json" in (self.headers.get("Content-Type") or ""): + fields = json.loads(body) + else: + fields = dict(parse_qsl(body, keep_blank_values=True)) + server.received.append({"path": self.path, "fields": fields}) + self._send( + "Created

Ticket created

" + ) + + def _send(self, html: str) -> None: + body = html.encode() + self.send_response(200) + self.send_header("Content-Type", "text/html") + self.send_header("Content-Length", str(len(body))) + self.end_headers() + self.wfile.write(body) + + def log_message(self, *_args: object) -> None: + pass + + self.http = http.server.ThreadingHTTPServer(("127.0.0.1", 0), Handler) + threading.Thread(target=self.http.serve_forever, daemon=True).start() + self.url = f"http://127.0.0.1:{self.http.server_address[1]}/newticket" + + +@pytest.fixture(scope="module") +def server() -> Iterator[_Server]: + srv = _Server() + yield srv + srv.http.shutdown() + + +_BUNDLES: dict[str, Path] = {} + + +def _bundle(server: _Server, variant: str, root: Path) -> Path: + if variant in _BUNDLES: + return _BUNDLES[variant] + out = root / variant + + def drive(page: Any, pump: Callable[[], Any]) -> None: + def settle() -> None: + for _ in range(12): + pump() + + page.wait_for_selector("[name=field_summary]", state="visible") + page.locator("[name=field_summary]").click() + settle() + page.keyboard.type("Printer jams on tray 2") + settle() + page.locator("[name=field_description]").click() + settle() + page.keyboard.type("Seen twice today") + settle() + page.locator("[name=submit]").click() + settle() + # The save navigates: let the recorder see the page it lands on. + for _ in range(30): + pump() + + rec = record_interactive( + f"{server.url}?v={variant}", + out / "rec", + headless=True, + script=drive, + param_fields=("field_summary", "field_description"), + ) + compile_recording(rec, out / "bundle", name="newticket") + # A slow recording can leave the save unarmed; fail here, not later. + assert Workflow.load(out / "bundle").steps[-1].anchor.form_identity is not None + _BUNDLES[variant] = out / "bundle" + return out / "bundle" + + +def _replay( + server: _Server, variant: str, bundle: Path, run_dir: Path +) -> tuple[Any, list]: + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + from openadapt_flow.runtime import Replayer + + before = len(server.received) + with sync_api.sync_playwright() as playwright: + browser = playwright.chromium.launch(headless=True) + try: + page = browser.new_page(viewport={"width": 1280, "height": 800}) + page.goto(f"{server.url}?v={variant}") + page.wait_for_selector("[name=field_summary]", state="visible") + report = Replayer(PlaywrightBackend(page)).run( + Workflow.load(bundle), + params={ + "field_summary": "Scanner offline on floor 3", + "field_description": "Since this morning", + }, + bundle_dir=bundle, + run_dir=run_dir, + ) + page.wait_for_timeout(500) + finally: + browser.close() + return report, server.received[before:] + + +@pytest.mark.parametrize("variant", ["plain", "sub7", "fetch"]) +def test_the_checked_submission_goes_out_once( + server: _Server, tmp_path_factory: pytest.TempPathFactory, variant: str +) -> None: + bundle = _bundle(server, variant, tmp_path_factory.mktemp("rec")) + assert Workflow.load(bundle).steps[-1].anchor.form_identity is not None + report, received = _replay(server, variant, bundle, tmp_path_factory.mktemp("run")) + if variant == "sub7": + # The submit listener fills the pid after the check, even on the + # demonstrated page: what is sent differs from the checked form, so + # it is held (a safe-direction false halt, documented). + assert not report.success + assert received == [] + return + assert report.success, [(r.step_id, r.error) for r in report.results] + assert len(received) == 1 + assert received[0]["fields"]["field_summary"] == "Scanner offline on floor 3" + assert "id" not in received[0]["fields"] + + +@pytest.mark.parametrize( + "recorded, replayed", + [("sub7", "sub9"), ("plain", "bubble42"), ("plain", "fd42"), ("fetch", "fetch42")], +) +def test_a_submission_changed_after_the_check_never_leaves( + server: _Server, + tmp_path_factory: pytest.TempPathFactory, + recorded: str, + replayed: str, +) -> None: + bundle = _bundle(server, recorded, tmp_path_factory.mktemp("rec")) + report, received = _replay(server, replayed, bundle, tmp_path_factory.mktemp("run")) + assert not report.success + assert received == [], received + errors = " ".join(r.error or "" for r in report.results) + assert ( + "differs from the checked form" in errors + or "changed after it was checked" in errors + ) From 50fa271a91255183b6fb4959f5e65d0ff609eec7 Mon Sep 17 00:00:00 2001 From: abrichr Date: Wed, 28 Oct 2026 15:45:20 -0400 Subject: [PATCH 04/11] feat: one wire check for checked saves, replacing the page-level guard Checks inside a page run alongside the page's own scripts, so the guard that compared submits in the page is gone. A checked save is now held at the network by openadapt_flow.wire_check: a Playwright route lets the save through only when its method, address, query and fields equal what the checked form would send, once, and aborts anything else before it is sent. The DOM checks before the click stay as pre-checks. The same module serves taught browser writes (declared writes, pinned shapes, sign-in values kept out of addresses in every encoding). The create-form tests now submit for real to a server that records what arrives, so each case asserts what left the browser. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/AFTER_A_STOP.md | 4 +- packages/flow/docs/CODE_AS_ACTION.md | 6 +- packages/flow/docs/COMPILE_AGENT.md | 12 +- packages/flow/docs/GUARDED_SAVE.md | 14 +- packages/flow/docs/HARNESS2.md | 8 +- packages/flow/docs/JIT.md | 33 +- packages/flow/docs/JIT_ADMISSION.md | 4 +- packages/flow/docs/L1_INTEGRATION.md | 4 +- packages/flow/docs/LIMITS.md | 37 +- packages/flow/docs/RECORDS.md | 17 +- packages/flow/docs/SELF_REPAIR.md | 31 +- packages/flow/docs/SHADOW_CANDIDATES.md | 2 +- packages/flow/docs/VARIANTS.md | 21 +- packages/flow/docs/backends/RDP.md | 17 +- packages/flow/docs/deployment/ON_PREM_VLM.md | 11 +- packages/flow/docs/design/EFFECT_VERIFIER.md | 5 +- packages/flow/docs/desktop_windows_runbook.md | 13 +- .../docs/showcase-openemr/bundle/workflow.py | 79 +++- .../backends/playwright_backend.py | 279 +++----------- packages/flow/openadapt_flow/wire_check.py | 360 ++++++++++++++++++ .../flow/tests/test_create_form_identity.py | 58 ++- 21 files changed, 665 insertions(+), 350 deletions(-) create mode 100644 packages/flow/openadapt_flow/wire_check.py diff --git a/packages/flow/docs/AFTER_A_STOP.md b/packages/flow/docs/AFTER_A_STOP.md index 38dfab93b9..7789cc9217 100644 --- a/packages/flow/docs/AFTER_A_STOP.md +++ b/packages/flow/docs/AFTER_A_STOP.md @@ -198,7 +198,9 @@ from openadapt_flow.after_stop import approve_undo, offer_undos, run_undo offers = offer_undos(run_dir, workflow) approve_undo(run_dir, workflow, "undo-triage", approver="nurse-a") -result = run_undo(run_dir, workflow, "undo-triage", replayer=replayer, bundle_dir=bundle) +result = run_undo( + run_dir, workflow, "undo-triage", replayer=replayer, bundle_dir=bundle +) assert result.status == "verified" ``` diff --git a/packages/flow/docs/CODE_AS_ACTION.md b/packages/flow/docs/CODE_AS_ACTION.md index 6f595b450d..af7028f72b 100644 --- a/packages/flow/docs/CODE_AS_ACTION.md +++ b/packages/flow/docs/CODE_AS_ACTION.md @@ -19,8 +19,10 @@ result = compile_first_run( "Open the referral for Jane Sample, start a Triage encounter, and save this note: {note}", params={"note": "Follow up in two weeks"}, agent=ClaudeAgent(vision="code", model="claude-sonnet-5-5"), - url="http://localhost:8080/#tasks", verify="verify.yaml", - expect={"patient_id": "p1"}, on_screen=["Jane Sample", "MRN P1"], + url="http://localhost:8080/#tasks", + verify="verify.yaml", + expect={"patient_id": "p1"}, + on_screen=["Jane Sample", "MRN P1"], ) ``` diff --git a/packages/flow/docs/COMPILE_AGENT.md b/packages/flow/docs/COMPILE_AGENT.md index babd72ef87..301a3475be 100644 --- a/packages/flow/docs/COMPILE_AGENT.md +++ b/packages/flow/docs/COMPILE_AGENT.md @@ -186,17 +186,17 @@ result = compile_first_run( on_screen=["Jane Sample", "MRN P1"], agent=ClaudeAgent(vision="selective"), model_key=load_customer_model_key("anthropic"), - confirm_checks="dana@clinic.example", # or a callback that shows them to a person + confirm_checks="dana@clinic.example", # or a callback that shows them to a person drop_fields=["key"], canary_params=[{"note": "Canary test note"}], # a real save; choose safe values - admitter=agent_principal, # principal_from_key(...) + admitter=agent_principal, # principal_from_key(...) ) -result.status # "admitted" -result.run1["agent"] # model calls, tokens, cost, seconds -result.emitted # {"skill": ..., "mcp_server": ..., "cli": ...} +result.status # "admitted" +result.run1["agent"] # model calls, tokens, cost, seconds +result.emitted # {"skill": ..., "mcp_server": ..., "cli": ...} later = run_compiled(result.dir, note="Recheck knee in ten days") -later.verified, later.model_calls, later.receipt_outcome # True, 0, "VERIFIED" +later.verified, later.model_calls, later.receipt_outcome # True, 0, "VERIFIED" ``` `FirstRunCompiler` exposes the same steps one at a time (`record`, diff --git a/packages/flow/docs/GUARDED_SAVE.md b/packages/flow/docs/GUARDED_SAVE.md index 84d7dfef96..9ede4e7e2e 100644 --- a/packages/flow/docs/GUARDED_SAVE.md +++ b/packages/flow/docs/GUARDED_SAVE.md @@ -84,17 +84,19 @@ guarded = GuardedSave.from_file("verify.yaml", "runs/guarded", approval="each_it task = load_task("tasks/referral-task-01.json") try: - guard = guarded.begin(task, screen_text=harness.open_record_text(), agent_id="cua-01") + guard = guarded.begin( + task, screen_text=harness.open_record_text(), agent_id="cua-01" + ) step = guard.commit(screenshot_png=harness.screenshot()) while step.status == "waiting_for_approval": - step = guard.commit(wait_s=30) # a person approves on Desktop or a phone + step = guard.commit(wait_s=30) # a person approves on Desktop or a phone except GuardRefused as refused: - stop(refused.code) # nothing was written + stop(refused.code) # nothing was written -harness.click_save() # once +harness.click_save() # once result = guard.confirm() -result.verdict # GuardVerdict.VERIFIED, REFUTED, or RECONCILIATION_REQUIRED -result.receipt_path # runs/guarded//effect-receipt.json +result.verdict # GuardVerdict.VERIFIED, REFUTED, or RECONCILIATION_REQUIRED +result.receipt_path # runs/guarded//effect-receipt.json ``` - `begin(task, screen_text=..., agent_id=...)` checks identity and reads the diff --git a/packages/flow/docs/HARNESS2.md b/packages/flow/docs/HARNESS2.md index 80e4e74e7e..73e37febf4 100644 --- a/packages/flow/docs/HARNESS2.md +++ b/packages/flow/docs/HARNESS2.md @@ -179,8 +179,12 @@ from openadapt_flow.integrations.harness2.tasks import mockmed_tasks adapter = OpenAdaptGuiAdapter( mockmed_tasks(("lookalike",)), ScriptedGuiAgent(), Path("runs"), evidence="readback" ) -prop = ScriptedProposer(ChannelSpec("claude_cli", "scripted"), audit_dir=Path("runs/audit")) -summary = run_task("mockmed/lookalike", 1, adapter=adapter, prop=prop, work_dir=Path("runs/work")) +prop = ScriptedProposer( + ChannelSpec("claude_cli", "scripted"), audit_dir=Path("runs/audit") +) +summary = run_task( + "mockmed/lookalike", 1, adapter=adapter, prop=prop, work_dir=Path("runs/work") +) print(summary["chosen_score"]["outcome"]) # verified ``` diff --git a/packages/flow/docs/JIT.md b/packages/flow/docs/JIT.md index 21c0b13fa1..74444617e1 100644 --- a/packages/flow/docs/JIT.md +++ b/packages/flow/docs/JIT.md @@ -11,7 +11,10 @@ the system of record before the run reports `VERIFIED`. ```python from openadapt_flow.jit import ( - TaskSignature, WebApp, compile_agent, load_customer_model_key, + TaskSignature, + WebApp, + compile_agent, + load_customer_model_key, ) from openadapt_flow.runtime.effects import RestRecordVerifier @@ -29,11 +32,13 @@ jit = compile_agent( model_key=load_customer_model_key("anthropic"), # your key, not OpenAdapt's ) -first = jit.run(note="Follow up in two weeks") # status: needs_human_admit (DRAFT) -confirmed = jit.confirm_params(principal_id="dana@clinic.example", - accept_params=["note"], confirm="ok") -jit.review(principal_id="dana@clinic.example", reviewed_steps=confirmed["steps"], - confirm="ok") +first = jit.run(note="Follow up in two weeks") # status: needs_human_admit (DRAFT) +confirmed = jit.confirm_params( + principal_id="dana@clinic.example", accept_params=["note"], confirm="ok" +) +jit.review( + principal_id="dana@clinic.example", reviewed_steps=confirmed["steps"], confirm="ok" +) jit.canary(principal_id="dana@clinic.example", note="Canary test note") jit.admit(principal_id="dana@clinic.example", confirm="ok") # at a terminal # Headless (CI): jit.admit_with_key(principal_from_key(load_encrypted_key(...), key_dir)) @@ -182,8 +187,14 @@ To keep the loop in your own code, leave out the agent and use `recording()` as a context manager: ```python -jit = compile_agent(task=..., app=..., verifier=..., policy="clinical-write", - dialect="anthropic", model_key=load_customer_model_key("anthropic")) +jit = compile_agent( + task=..., + app=..., + verifier=..., + policy="clinical-write", + dialect="anthropic", + model_key=load_customer_model_key("anthropic"), +) with jit.recording(note="Follow up in two weeks") as rec: my_cua_loop(rec.request.instruction, rec.computer.execute) print(rec.result.status) # needs_human_admit, unsupported, or capture_failed @@ -294,7 +305,11 @@ exact values the save may write, and any other text it may type. from openadapt_flow.mockmed.teach_scope import encounter_scope jit = compile_agent( - agent, task=task, app=app, verifier=verifier, policy="clinical-write", + agent, + task=task, + app=app, + verifier=verifier, + policy="clinical-write", teach_scope=lambda params: encounter_scope( base_url, patient_id="p1", note=params["note"] ), diff --git a/packages/flow/docs/JIT_ADMISSION.md b/packages/flow/docs/JIT_ADMISSION.md index d4b9f9cfb2..abebc5556b 100644 --- a/packages/flow/docs/JIT_ADMISSION.md +++ b/packages/flow/docs/JIT_ADMISSION.md @@ -70,7 +70,9 @@ The agent then runs canaries and admits, with its own key: ```python from openadapt_flow.jit.authority import ( - SignedAdmissionPolicy, load_encrypted_key, principal_from_key, + SignedAdmissionPolicy, + load_encrypted_key, + principal_from_key, ) agent = principal_from_key( diff --git a/packages/flow/docs/L1_INTEGRATION.md b/packages/flow/docs/L1_INTEGRATION.md index bfc6f76b5a..b1562964f8 100644 --- a/packages/flow/docs/L1_INTEGRATION.md +++ b/packages/flow/docs/L1_INTEGRATION.md @@ -35,8 +35,8 @@ the happy path. from openadapt_flow.emit.l1_artifact import emit_l1_artifact ref = emit_l1_artifact( - downloaded_pdf, # produced by a replayed workflow step - extraction_dir, # the root the L2 layer watches + downloaded_pdf, # produced by a replayed workflow step + extraction_dir, # the root the L2 layer watches file_number="P1", date="2026-07-06", doctype="referral", diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index df7a31f213..aa9e7d1737 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -402,25 +402,24 @@ field typed since the last save, the compiler gives it a **form identity** should create under a different parent on each run therefore halts until that parent is checked another way. -From the check until the next step, the browser holds every write request the -page sends, and a GET navigation to the form's address, at the network: it -leaves only when its method, address, query and fields equal what the checked -button would send (form fields or a flat JSON object), and only once. Page -listeners that run after every in-page check (a form submit listener, a -document capture listener, a `formdata` listener, a timer) can't change a save -on its way out. A save sent in another shape (nested JSON, a custom encoding) -can't be compared, so it is held too. In the page, a guard also compares every submit -with what the checked button would have sent (address and query, method, and -every field's value except the exempt tokens). A submit from another form, -from the right form without its save button, with contents that changed after -the check (a click handler that fills in or enables a field), or a second -submit, is cancelled. The run watches the guard for two seconds after the -click, then again at the start of the next step, and halts on any cancelled -submit. A save sent by script (`form.submit()`, `fetch`) raises no submit -event, but its request is still held at the network. A submission a script -delays past the end of the run is still held until the page changes, but the -run, already over, doesn't report it. A page whose own click or submit -listener fills in or enables a field as the form is sent changes what is sent +The checks above read the page before the click; they are pre-checks. The +safety layer is the wire check (`openadapt_flow/wire_check.py`, shared with +taught browser writes): from the check until the next step, a Playwright route +holds every write request the page sends, and a GET navigation to the form's +address, at the network, where page scripts can't reach it. A request leaves +only when its method, address, query and fields equal what the checked button +would send (form fields or a flat JSON object, every value except the exempt +tokens), and only once; anything else is aborted before it is sent. So a page +listener that runs after the check (a submit, capture or `formdata` listener, +a click handler that fills in or enables a field, a timer), a save sent by +script (`form.submit()`, `fetch`), a submit from another form and a second +submit can't send anything the check didn't see. A save sent in another shape +(nested JSON, a custom encoding) can't be compared, so it is held too. The run +watches the wire check for two seconds after the click, then again at the +start of the next step and at the end of the run, and halts on any held +request. A submission a script delays past the end of the run is still held +until the page closes, but the run, already over, doesn't report it. A page +whose own listener changes a field as the form is sent changes what is sent after the check, so its save halts even when the demonstration did the same: a false halt, in the safe direction. diff --git a/packages/flow/docs/RECORDS.md b/packages/flow/docs/RECORDS.md index 1f27dd417c..cb88d0f0d3 100644 --- a/packages/flow/docs/RECORDS.md +++ b/packages/flow/docs/RECORDS.md @@ -104,8 +104,12 @@ is derived from it: ```python ChannelRead(channel="export", raw="AC-1042", source_kind="live_session") -ChannelRead(channel="export", raw="AC-1042", session="independent", - source_kind="committed_store") +ChannelRead( + channel="export", + raw="AC-1042", + session="independent", + source_kind="committed_store", +) ``` A workflow can use either kind. A write step that needs `system_of_record` values @@ -247,8 +251,13 @@ In a workflow, set `read_gate` on the writing step. Replay checks it against the values read so far in the run and halts before the step acts: ```python -Step(id="post", intent="post amount", action=ActionKind.CLICK, anchor=save, - read_gate=GradeGate(min_grade="same_screen", fields=("amount_due",))) +Step( + id="post", + intent="post amount", + action=ActionKind.CLICK, + anchor=save, + read_gate=GradeGate(min_grade="same_screen", fields=("amount_due",)), +) ``` These grades describe how a read value was checked. Write checks keep their diff --git a/packages/flow/docs/SELF_REPAIR.md b/packages/flow/docs/SELF_REPAIR.md index 9e204f31e1..7c3e71754e 100644 --- a/packages/flow/docs/SELF_REPAIR.md +++ b/packages/flow/docs/SELF_REPAIR.md @@ -217,17 +217,23 @@ Four objects make up the API: ```python from openadapt_flow.repair.self_repair import ( - AppSession, RepairProposal, ReplayTarget, ScreenAccess, SelfRepair, + AppSession, + RepairProposal, + ReplayTarget, + ScreenAccess, + SelfRepair, ) from openadapt_flow.backends.playwright_backend import PlaywrightBackend from openadapt_flow.runtime.effects import RestRecordVerifier URL = "http://127.0.0.1:8765/?fault=ok&idempotency=demo&drift=redesign#tasks" + def open_session(fault): backend, close = PlaywrightBackend.launch(fault or URL) return AppSession(backend=backend, close=close) + target = ReplayTarget( open_session, effect_verifier=RestRecordVerifier("http://127.0.0.1:8765", records_path="/api/db"), @@ -280,15 +286,17 @@ contains: ```python class MyAgent: - dialect = "anthropic" # any name in openadapt_flow.jit.dialects() - agent_id = "agent:my-repair" # recorded; can never approve its own fix - requires_model_key = True # False only for an agent that calls no model + dialect = "anthropic" # any name in openadapt_flow.jit.dialects() + agent_id = "agent:my-repair" # recorded; can never approve its own fix + requires_model_key = True # False only for an agent that calls no model def propose(self, request): # request.intent, request.recorded_label, request.recorded_crop_png, # request.live_png, request.viewport, request.previous_refusals, # request.model_key (the customer's key; never an OpenAdapt key) - action = ask_my_model(request) # e.g. {"action": "left_click", "coordinate": [846, 457]} + action = ask_my_model( + request + ) # e.g. {"action": "left_click", "coordinate": [846, 457]} return RepairProposal(action=action, model_calls=1, cost_usd=0.004) ``` @@ -616,8 +624,9 @@ standard = StandardChanges( key_dir=Path("~/.openadapt/jit").expanduser(), test_record=test_record, ) -repair = SelfRepair(agent, target, store="repair-store", - screen_access=access, standard_changes=standard) +repair = SelfRepair( + agent, target, store="repair-store", screen_access=access, standard_changes=standard +) ``` `test_record_target` is a `ReplayTarget` whose sessions open the test record, @@ -648,8 +657,12 @@ window. An item that halts for any other reason stays halted for a person. ```python from openadapt_flow.repair.preflight_queue import QueueItem, WorklistQueue -queue = WorklistQueue("bundles/triage", test_record, "runs/worklist-2026-10-18", - self_repair=lambda target: SelfRepair(agent, target, ...)) +queue = WorklistQueue( + "bundles/triage", + test_record, + "runs/worklist-2026-10-18", + self_repair=lambda target: SelfRepair(agent, target, ...), +) result = queue.run([QueueItem(item_id, target_for(item_id)) for item_id in ids]) print(result.status, [(i.item_id, i.status) for i in result.items]) ``` diff --git a/packages/flow/docs/SHADOW_CANDIDATES.md b/packages/flow/docs/SHADOW_CANDIDATES.md index 0aa4556426..0953032522 100644 --- a/packages/flow/docs/SHADOW_CANDIDATES.md +++ b/packages/flow/docs/SHADOW_CANDIDATES.md @@ -40,7 +40,7 @@ A label that Capture scrubbed (``) is treated as no label. ```python from openadapt_flow.compiler.motif_induction import induce_candidate, write_candidate -candidate = induce_candidate(export) # dict or ShadowEpisodesExport +candidate = induce_candidate(export) # dict or ShadowEpisodesExport write_candidate(candidate, "shadow-candidates/abc") ``` diff --git a/packages/flow/docs/VARIANTS.md b/packages/flow/docs/VARIANTS.md index 54a5554d44..64eb3134ad 100644 --- a/packages/flow/docs/VARIANTS.md +++ b/packages/flow/docs/VARIANTS.md @@ -234,23 +234,34 @@ state. Repeat `--when-input NAME=VALUE` to allow several values, and use ```python from openadapt_flow.variants import ( - VariantInput, VariantWhen, VariantSet, author_variant_set, add_variant, + VariantInput, + VariantWhen, + VariantSet, + author_variant_set, + add_variant, ) from openadapt_flow.runtime.variant_router import route_variant, execute_variant_set variant_set = author_variant_set( - [VariantInput(name="dark", bundle=..., admission=..., when=VariantWhen(theme="dark")), ...], + [ + VariantInput( + name="dark", bundle=..., admission=..., when=VariantWhen(theme="dark") + ), + ..., + ], name="encounter-intake", out="encounter-intake", ) -decision = route_variant(variant_set, inputs={"payer": "medicare"}, first_screen=png_bytes) +decision = route_variant( + variant_set, inputs={"payer": "medicare"}, first_screen=png_bytes +) report = execute_variant_set( variant_set, parent_dir="encounter-intake", run_dir="runs/item-1", inputs={"payer": "medicare", "patient_id": "..."}, - first_screen=capture, # bytes, None, or a zero-argument callable - child_run=child_executor, # the same executor type a process contract uses + first_screen=capture, # bytes, None, or a zero-argument callable + child_run=child_executor, # the same executor type a process contract uses trusted_signers=trust, ) ``` diff --git a/packages/flow/docs/backends/RDP.md b/packages/flow/docs/backends/RDP.md index bd185f92e8..448a295e5a 100644 --- a/packages/flow/docs/backends/RDP.md +++ b/packages/flow/docs/backends/RDP.md @@ -138,12 +138,15 @@ API the backend expects. Construct it from plain credentials: from openadapt_flow.backends.rdp_backend import AardwolfTransport, FreeRDPBackend transport = AardwolfTransport.from_credentials( - "10.0.0.5", "clinicuser", "password", - domain=None, # omit for a local account; set for a Windows domain + "10.0.0.5", + "clinicuser", + "password", + domain=None, # omit for a local account; set for a Windows domain port=3389, - width=1280, height=800, + width=1280, + height=800, ) -backend = FreeRDPBackend(transport) # connects on construction +backend = FreeRDPBackend(transport) # connects on construction png = backend.screenshot() backend.click(640, 400) backend.close() @@ -170,7 +173,11 @@ command line or in a log, and the transport doesn't keep it. ```python from openadapt_flow.backends.rdp_backend import FreeRDPBackend -from openadapt_flow.backends.xfreerdp import RdpTarget, SessionProfile, XFreeRDPTransport +from openadapt_flow.backends.xfreerdp import ( + RdpTarget, + SessionProfile, + XFreeRDPTransport, +) transport = XFreeRDPTransport( RdpTarget("10.0.0.5", 3389, username="svc-records"), diff --git a/packages/flow/docs/deployment/ON_PREM_VLM.md b/packages/flow/docs/deployment/ON_PREM_VLM.md index 8784cd8a60..d0177bbe55 100644 --- a/packages/flow/docs/deployment/ON_PREM_VLM.md +++ b/packages/flow/docs/deployment/ON_PREM_VLM.md @@ -156,13 +156,16 @@ Runner side (GPU-less): ```python from openadapt_flow.runtime.remote_vlm import ( - RemoteVLMClient, RemoteGrounder, RemoteIdentityVLM, RemoteStateVerifier, + RemoteVLMClient, + RemoteGrounder, + RemoteIdentityVLM, + RemoteStateVerifier, ) client = RemoteVLMClient("https://gpu-box.lan:8077", token=TOKEN, timeout=2.0) -grounder = RemoteGrounder(client) # Grounder protocol, drop-in -identity = RemoteIdentityVLM(client) # verify / mismatch / abstain -state = RemoteStateVerifier(client) # yes / no / uncertain +grounder = RemoteGrounder(client) # Grounder protocol, drop-in +identity = RemoteIdentityVLM(client) # verify / mismatch / abstain +state = RemoteStateVerifier(client) # yes / no / uncertain ``` ## Integration diff --git a/packages/flow/docs/design/EFFECT_VERIFIER.md b/packages/flow/docs/design/EFFECT_VERIFIER.md index f56835f169..b7364ebbcf 100644 --- a/packages/flow/docs/design/EFFECT_VERIFIER.md +++ b/packages/flow/docs/design/EFFECT_VERIFIER.md @@ -30,8 +30,11 @@ FHIR/REST API, a document store), never the screen. ```python class EffectVerifier(Protocol): substrate: str + def capture_pre_state(self, context) -> EffectState: ... - def verify(self, expected: Effect, before: EffectState, context) -> EffectVerdict: ... + def verify( + self, expected: Effect, before: EffectState, context + ) -> EffectVerdict: ... ``` - **`Effect`** is the RFC's typed effect. Kinds: `record_written` (a record diff --git a/packages/flow/docs/desktop_windows_runbook.md b/packages/flow/docs/desktop_windows_runbook.md index 6c84e0bee7..043ab51765 100644 --- a/packages/flow/docs/desktop_windows_runbook.md +++ b/packages/flow/docs/desktop_windows_runbook.md @@ -81,11 +81,13 @@ from openadapt_flow.compiler import compile_recording backend = WindowsBackend("http://:5000", auth_token="") + def driver(rec): - rec.click(x0, y0) # a UIA AutomationId is armed at each click + rec.click(x0, y0) # a UIA AutomationId is armed at each click rec.type_text("value", param="p") rec.press("Enter") + recording = record_desktop_demo(backend, "out/recording", driver) bundle = compile_recording(recording, "out/bundle", name="my-desktop-task") ``` @@ -95,6 +97,7 @@ Check parity of the compiled bundle: ```python from openadapt_flow.adapters.desktop_recorder import structural_armed_coverage from openadapt_flow.ir import Workflow + print(structural_armed_coverage(Workflow.load("out/bundle"))) # armed_coverage == 1.0 ``` @@ -113,10 +116,12 @@ from openadapt_flow.ir import Workflow from openadapt_flow.runtime import Replayer report = Replayer(backend, use_structural=True).run( - Workflow.load("out/bundle"), params={"p": "value"}, - bundle_dir="out/bundle", run_dir="out/run", + Workflow.load("out/bundle"), + params={"p": "value"}, + bundle_dir="out/bundle", + run_dir="out/run", ) -print(report.rung_counts) # {'structural': N} when the UIA rung drives it +print(report.rung_counts) # {'structural': N} when the UIA rung drives it assert report.success ``` diff --git a/packages/flow/docs/showcase-openemr/bundle/workflow.py b/packages/flow/docs/showcase-openemr/bundle/workflow.py index 8c89b151d5..1545dfeb9d 100644 --- a/packages/flow/docs/showcase-openemr/bundle/workflow.py +++ b/packages/flow/docs/showcase-openemr/bundle/workflow.py @@ -2,53 +2,75 @@ workflow 'openemr-showcase' — for code review only; the replayer executes workflow.json, not this file.""" -WORKFLOW_NAME = 'openemr-showcase' -RECORDING_ID = '50758ec282e94c4a9f99f69ab7415678' -PARAMS = {'note': 'Reviewed hypertension follow-up; BP within target range today.'} # param -> example/default value +WORKFLOW_NAME = "openemr-showcase" +RECORDING_ID = "50758ec282e94c4a9f99f69ab7415678" +PARAMS = { + "note": "Reviewed hypertension follow-up; BP within target range today." +} # param -> example/default value def run(flow, params=PARAMS): # step_000: click 'ername' [reversible] # landmark: 'PracticeManagementsolution.' (left_of, 82px) # landmark: 'Password' (left_of, 86px) - flow.click(template='templates/step_000.png', click_point=(705, 404), region=(625, 372, 160, 64), ocr_text='ername') + flow.click( + template="templates/step_000.png", + click_point=(705, 404), + region=(625, 372, 160, 64), + ocr_text="ername", + ) # expect region_stable region=(555, 351, 301, 106) phash='c3dbd8dac3d098d0' (tol 16) # step_001: type 'admin' [reversible] - flow.type_text('admin') + flow.type_text("admin") # expect region_stable region=(570, 365, 138, 77) phash='ce7cb37ccc78b040' (tol 16) # expect text_present text='admin' # step_002: click 'Password' [reversible] # landmark: 'Default-English(Standard)' (below, 56px) # landmark: 'admin' (left_of, 80px) - flow.click(template='templates/step_002.png', click_point=(684, 458), region=(604, 426, 160, 64), ocr_text='Password') + flow.click( + template="templates/step_002.png", + click_point=(684, 458), + region=(604, 426, 160, 64), + ocr_text="Password", + ) # expect region_stable region=(555, 351, 301, 160) phash='c14adac6dacae1d8' (tol 16) # step_003: type 'pass' [reversible] - flow.type_text('pass') + flow.type_text("pass") # expect region_stable region=(571, 419, 133, 77) phash='a27f9178c7c79038' (tol 16) # step_004: click 'Login' [reversible] # landmark: 'Acknowledgments,Licensing and Certification' (below, 45px) # landmark: 'Default-English(Standard)' (right_of, 89px) - flow.click(template='templates/step_004.png', click_point=(639, 566), region=(559, 534, 160, 64), ocr_text='Login') + flow.click( + template="templates/step_004.png", + click_point=(639, 566), + region=(559, 534, 160, 64), + ocr_text="Login", + ) # expect text_present text='CalendarFinderFlowRecallsMessagesPatientFeesModulesInventoryProceduresAdminReportsMiscellaneousPopups' # step_005: click 'Searchbyanydemogre' [reversible] # landmark: 'Week' (below, 172px) # landmark: 'Billy Smith' (left_of, 353px) # identity: armed via PHI-free salted-hash template (2 band tokens); no patient identifier stored - flow.click(template='templates/step_005.png', click_point=(1121, 31), region=(1041, 0, 160, 64), ocr_text='Searchbyanydemogre') + flow.click( + template="templates/step_005.png", + click_point=(1121, 31), + region=(1041, 0, 160, 64), + ocr_text="Searchbyanydemogre", + ) # expect region_stable region=(286, 531, 994, 84) phash='fff881038670f07c' (tol 16) # step_006: type 'Phil' [reversible] - flow.type_text('Phil') + flow.type_text("Phil") # expect region_stable region=(1017, 0, 209, 71) phash='eae01715fb7a801d' (tol 16) # expect text_present text='Phil' # step_007: press Enter [reversible] - flow.press('Enter') + flow.press("Enter") # expect region_stable region=(0, 72, 1280, 728) phash='fbf70460c387a568' (tol 16) # expect text_present text='beninNewBrowserTabSearchwithexactmethod' @@ -56,7 +78,12 @@ def run(flow, params=PARAMS): # landmark: 'Name' (above, 45px) # landmark: 'earchbyName' (above, 104px) # identity: armed via PHI-free salted-hash template (4 band tokens); no patient identifier stored - flow.click(template='templates/step_008.png', click_point=(17, 443), region=(0, 411, 160, 64), ocr_text='ford,Phil') + flow.click( + template="templates/step_008.png", + click_point=(17, 443), + region=(0, 411, 160, 64), + ocr_text="ford,Phil", + ) # expect region_stable region=(0, 195, 1280, 326) phash='fee1e1a6b28180db' (tol 16) # expect text_present text='edicalRecordDashboard-PhilBelford' @@ -75,7 +102,11 @@ def run(flow, params=PARAMS): # step_013: click at (815, 369) [reversible] # landmark: 'None' (right_of, 123px) # landmark: 'Immunizations' (above, 180px) - flow.click(template='templates/step_013.png', click_point=(815, 369), region=(735, 337, 160, 64)) + flow.click( + template="templates/step_013.png", + click_point=(815, 369), + region=(735, 337, 160, 64), + ) # expect region_stable region=(0, 176, 691, 174) phash='c3da3fb5b7408590' (tol 16) # expect text_present text='PatientMessagesforBelford,Phil' @@ -83,22 +114,36 @@ def run(flow, params=PARAMS): # landmark: 'Show All' (below, 98px) # landmark: 'alendarx' (above, 102px) # identity: armed via PHI-free salted-hash template (4 band tokens); no patient identifier stored - flow.click(template='templates/step_014.png', click_point=(90, 281), region=(10, 249, 160, 64), ocr_text='+Add [reversible] - flow.type_text(params['note']) + flow.type_text(params["note"]) # step_017: click 'Save as new messag' [irreversible] # landmark: 'Smith, Billy' (above, 191px) # landmark: 'Active' (left_of, 234px) - flow.click(template='templates/step_017.png', click_point=(489, 452), region=(409, 420, 160, 64), ocr_text='Save as new messag') + flow.click( + template="templates/step_017.png", + click_point=(489, 452), + region=(409, 420, 160, 64), + ocr_text="Save as new messag", + ) # expect region_stable region=(0, 0, 1280, 800) phash='c0d91b9db162c6a7' (tol 16) # expect text_present text='CalendarFinderFlowRecallsMessagesPatientFeesModulesInventoryProceduresAdminReportsMiscellaneousPopups' diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index c20f54ed8a..c7e46a1d19 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -18,7 +18,7 @@ import uuid from dataclasses import dataclass, field from datetime import datetime, timezone -from typing import TYPE_CHECKING, Any, Callable, Literal, Mapping, Optional +from typing import TYPE_CHECKING, Any, Callable, Literal, Optional from urllib.parse import urlsplit from openadapt_flow.backends._page_quiescence import PageQuiescence @@ -49,6 +49,11 @@ verify_form_identity, verify_submit_binding, ) +from openadapt_flow.wire_check import ( + ExpectedSubmission, + WireCheck, + expected_submission, +) VIEWPORT: tuple[int, int] = (1280, 800) # Secret-masked screenshots must not keep bytes from a changing frame tree. @@ -772,24 +777,19 @@ class _FramePoint: }""" ) -# Before a create-new form's save: guard the form's submission. At arm time it -# keeps, in the page, what the checked button would send (address with query, -# method, and every name/value pair except the exempt tokens). Every submit -# while it is armed is checked: one from another form, from this form without -# its button, with different contents, or a second one, is cancelled and -# recorded. It stays armed after the click (a script may submit later) until -# the run reads and disarms it, or after 120 s. -_ARM_FORM_SUBMIT_GUARD_JS = ( +# Before a create-new form's save: what the checked button would send (the +# address with its query, the method, and every field, from the form's +# controls, never from new FormData(), which runs the page's own formdata +# listeners). The wire check holds the save's request to exactly this. +_FORM_EXPECTED_JS = ( r"""(args) => { const hit = document.elementFromPoint(args.x, args.y); const control = hit ? hit.closest(""" + json.dumps(GUARDED_ACTIONABLE_SELECTOR) + r""") : null; const form = control ? (control.form || control.closest('form')) : null; - if (!form) return false; + if (!form) return null; const exempt = new Set(args.exempt || []); - // What the button would send, built from the form's controls: never from - // new FormData(), which runs the page's own formdata listeners. const expected_parts = () => { const pairs = []; for (const el of Array.from(form.elements)) { @@ -832,151 +832,11 @@ class _FramePoint: query.sort(order); return {address, query, method, pairs}; }; - const canon = () => JSON.stringify(expected_parts()); - const previous = window[args.key]; - if (previous && previous.remove) previous.remove(); - const expected = canon(); - const state = {refused: [], allowed: 0}; - const handler = (event) => { - let why = null; - if (event.target !== form) why = 'the click submitted a different form'; - else if (event.submitter !== control) { - why = 'the form was submitted by something other than its save button'; - } else if (canon() !== expected) { - why = 'what the form would send changed after it was checked'; - } else if (state.allowed > 0) { - why = 'the form was submitted a second time'; - } - if (why) { - event.preventDefault(); - event.stopImmediatePropagation(); - state.refused.push(why); - } else { - state.allowed += 1; - } - }; - state.remove = () => window.removeEventListener('submit', handler, true); - window.addEventListener('submit', handler, true); - setTimeout(state.remove, 120000); - Object.defineProperty( - window, args.key, {value: state, configurable: true, writable: true}); return expected_parts(); }""" ) -def _wire_pairs(request: Any) -> Optional[list[tuple[str, str]]]: - """The fields a request carries in its body, or None when its body is - not a form or a flat JSON object.""" - - body = request.post_data_buffer or b"" - ctype = (request.headers.get("content-type") or "").lower() - if not body: - return [] - if ctype.startswith("application/x-www-form-urlencoded"): - from urllib.parse import parse_qsl - - return parse_qsl(body.decode("utf-8", "replace"), keep_blank_values=True) - if ctype.startswith("multipart/form-data"): - from email.parser import BytesParser - from email.policy import HTTP - - message = BytesParser(policy=HTTP).parsebytes( - b"Content-Type: " + ctype.encode() + b"\r\n\r\n" + body - ) - pairs: list[tuple[str, str]] = [] - for part in message.iter_parts(): - name = part.get_param("name", header="content-disposition") - filename = part.get_param("filename", header="content-disposition") - if name is None: - continue - if filename is not None: - pairs.append((str(name), f"file:{filename}")) - else: - payload = part.get_payload(decode=True) or b"" - pairs.append((str(name), payload.decode("utf-8", "replace"))) - return pairs - if ctype.startswith("application/json"): - try: - data = json.loads(body) - except ValueError: - return None - if not isinstance(data, dict) or any( - isinstance(v, (dict, list)) for v in data.values() - ): - return None - return [(str(k), "" if v is None else str(v)) for k, v in data.items()] - return None - - -def _wire_mismatch(request: Any, state: Mapping[str, Any]) -> Optional[str]: - """``""``: not the guard's business. ``None``: the checked submission. - Otherwise why the request must not be sent.""" - - from urllib.parse import parse_qsl, urlsplit - - method = request.method.upper() - expected = state["expected"] - exempt = state["exempt"] - parts = urlsplit(request.url) - address = f"{parts.scheme}://{parts.netloc}{parts.path}" - writes = method not in ("GET", "HEAD", "OPTIONS") - navigation_here = ( - method == "GET" - and request.is_navigation_request() - and address == expected["address"] - ) - if not (writes or navigation_here) or request.redirected_from is not None: - return "" - if state["sent"]: - return "the page sent a second submission after the checked one" - if method.lower() != expected["method"]: - return ( - f"the save was sent with {method}, the checked form sends with " - f"{expected['method'].upper()}" - ) - if address != expected["address"]: - return "the save was sent to a different address than the checked form's" - - def norm(pairs: Any, *, query: bool = False) -> list[tuple[str, str]]: - out = [] - for key, value in pairs: - key = str(key) - if (("?" + key) if query else key) in exempt: - continue - out.append((key, str(value).replace("\r\n", "\n"))) - return sorted(out) - - url_query = parse_qsl(parts.query, keep_blank_values=True) - if method == "GET": - sent = norm(url_query) - else: - if norm(url_query, query=True) != norm(expected["query"], query=True): - return "the save was sent with a different query than the checked form's" - body = _wire_pairs(request) - if body is None: - return ( - "the save was sent in a shape this check can't compare with " - "the form (not form fields or flat JSON)" - ) - sent = norm(body) - if sent != norm(expected["pairs"]): - return "what the save sent differs from the checked form" - return None - - -# Read the guard's refusals; with ``disarm`` also remove it. -_FORM_SUBMIT_GUARD_RESULT_JS = r"""(args) => { - const state = window[args.key]; - if (!state) return null; - const refused = state.refused.slice(); - if (args.disarm) { - if (state.remove) state.remove(); - delete window[args.key]; - } - return refused.length ? refused.join('; ') : null; -}""" - # Page-structure identity of a FORM FIELD that sits in no record row. # # A data-entry form (OpenEMR's new-patient page, most CRUD screens) lays its @@ -1884,8 +1744,6 @@ def __init__( # references only) for submit bindings. A new backend starts empty. self._fill_store_key = f"__oaflow_filled_{uuid.uuid4().hex}" self._fill_value_slot = f"__oaflow_filled_values_{uuid.uuid4().hex}" - self._form_guard_slot = f"__oaflow_form_guard_{uuid.uuid4().hex}" - self._form_guard_scope: Any = None self._form_wire: Any = None self._structural_tokens: dict[str, _StructuralGuard] = {} self._guarded_coordinate: Optional[_CoordinateGuard] = None @@ -2583,82 +2441,51 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s if reason is not None: return reason try: - armed = point.scope.evaluate( - _ARM_FORM_SUBMIT_GUARD_JS, - { - "x": point.x, - "y": point.y, - "key": self._form_guard_slot, - "exempt": list(identity.exempt), - }, + expected = point.scope.evaluate( + _FORM_EXPECTED_JS, + {"x": point.x, "y": point.y, "exempt": list(identity.exempt)}, ) except Exception: - armed = None - if not isinstance(armed, dict): - return "could not guard the form's submission" - self._form_guard_scope = point.scope - self._arm_form_wire_guard(armed, set(identity.exempt)) + expected = None + if not isinstance(expected, dict): + return "could not read what the form would send" + self._arm_wire_check(expected_submission(expected, identity.exempt)) return None - # -- the wire guard: what the browser actually sends ---------------------- + # -- the wire check (openadapt_flow.wire_check) -------------------------- - def _arm_form_wire_guard( - self, expected: Mapping[str, Any], exempt: set[str] - ) -> None: - """Compare the request the save actually sends with the checked form. - - Page listeners (a submit or formdata listener, a timer) can still - change a submission after every in-page check. So until the guard is - disarmed, every write request the page sends, and a GET navigation to - the form's address, is held at the network: it continues only when - its method, address, query and fields equal what the checked button - would have sent, and only once. Anything else is aborted before it - leaves the browser, and recorded. + def _arm_wire_check(self, submission: ExpectedSubmission) -> None: + """Hold the save's request at the network to the checked form. + + Until it is disarmed, every write request the page sends, and a GET + navigation to the form's address, goes through a + :class:`~openadapt_flow.wire_check.WireCheck`: the checked submission + goes once; anything else is aborted before it is sent. """ - self._disarm_form_wire_guard() - state: dict[str, Any] = { - "expected": expected, - "exempt": exempt, - "sent": False, - "refused": [], - } + self._disarm_wire_check() + check = WireCheck(submission=submission) def handler(route: Any) -> None: - try: - why = _wire_mismatch(route.request, state) - except Exception: # noqa: BLE001 - unreadable request: refuse - why = "the save's request could not be read" - if why == "": - route.fallback() - return - if why is None: - state["sent"] = True - route.fallback() - return - state["refused"].append(why) - try: - route.abort() - except Exception: # noqa: BLE001 - already handled - pass + check.handle(route, proceed=route.fallback) self.page.route("**/*", handler) - self._form_wire = (handler, state) + self._form_wire = (handler, check) - def _disarm_form_wire_guard(self) -> list[str]: - wire = getattr(self, "_form_wire", None) + def _disarm_wire_check(self) -> list[str]: + wire = self._form_wire self._form_wire = None if wire is None: return [] - handler, state = wire + handler, check = wire try: self.page.unroute("**/*", handler) except Exception: # noqa: BLE001 - the page is gone pass - return list(state["refused"]) + return list(check.refused) def wait_form_submit_guard(self, ms: int) -> None: - """Let the page and the wire guard run for ``ms`` milliseconds.""" + """Let the page and the wire check run for ``ms`` milliseconds.""" try: self.page.wait_for_timeout(ms) @@ -2666,41 +2493,23 @@ def wait_form_submit_guard(self, ms: int) -> None: time.sleep(ms / 1000) def form_submit_guard_refusal(self, *, disarm: bool = False) -> Optional[str]: - """Why the guard cancelled a submit so far, or None. ``disarm`` also - removes the guard. A page that navigated away (the checked form's own - submit) has nothing to report.""" + """Why the wire check refused a request so far, or None. ``disarm`` + also removes it.""" - scope = self._form_guard_scope - if scope is None and getattr(self, "_form_wire", None) is None: + wire = self._form_wire + if wire is None: return None - reasons: list[str] = [] - wire = getattr(self, "_form_wire", None) - if wire is not None: - reasons.extend(wire[1]["refused"]) + reasons = list(wire[1].refused) if disarm: - self._form_guard_scope = None - self._disarm_form_wire_guard() - if scope is not None: - try: - refused = scope.evaluate( - _FORM_SUBMIT_GUARD_RESULT_JS, - {"key": self._form_guard_slot, "disarm": disarm}, - ) - except Exception: - refused = None - if isinstance(refused, str): - reasons.insert(0, refused) + self._disarm_wire_check() return "; ".join(dict.fromkeys(reasons)) if reasons else None @property def form_submit_guard_armed(self) -> bool: - return ( - self._form_guard_scope is not None - or getattr(self, "_form_wire", None) is not None - ) + return self._form_wire is not None def cancel_form_submit_guard(self) -> Optional[str]: - """Read the guard's last refusals and disarm it.""" + """Read the wire check's last refusals and disarm it.""" return self.form_submit_guard_refusal(disarm=True) diff --git a/packages/flow/openadapt_flow/wire_check.py b/packages/flow/openadapt_flow/wire_check.py new file mode 100644 index 0000000000..a18c07c556 --- /dev/null +++ b/packages/flow/openadapt_flow/wire_check.py @@ -0,0 +1,360 @@ +"""The wire check: only declared writes leave the browser. + +Checks inside a page run alongside the page's own scripts, which can change +a submission after any check (a submit or formdata listener, a timer, a +fetch). So the browser's outgoing requests are checked where scripts can't +reach them: at the network, through a Playwright route. A write request is +let through only when it is one the run declared; anything else is aborted +before it is sent and recorded, and the run halts on it. + +A :class:`WireCheck` declares, in any combination: + +* ``submission``: the exact request a checked form's save sends (method, + address, query and every field), once (a create-new form's save, + :class:`~openadapt_flow.ir.FormIdentity`). +* ``allowed``: the writes a contract lists, as ``"METHOD /path"``; with + ``pinned``, the exact taught writes in order, each with the digest of the + field names it carries (a taught browser write). +* ``secrets``: sign-in values that must never travel in an address, in any + encoding. +* ``origin``: the only site writes may go to. + +With none of ``submission``, ``allowed`` declared, every write is refused (a +read-only run). Reads (GET, HEAD, OPTIONS) pass, except a navigation to a +declared submission's address, which is the submission itself for a GET +form. Bodies are compared as URL-encoded fields, multipart fields or JSON; +a body that can't be read the declared way is refused. This is one of the +three things safety rests on (``docs/THREAT_MODEL.md``): the record check +proves what landed, this proves nothing else was sent, and undo or a person +handles the rest. +""" + +from __future__ import annotations + +import base64 +import hashlib +import html as _html +import json +from dataclasses import dataclass, field +from typing import Any, Callable, Iterable, Mapping, Optional, Sequence +from urllib.parse import parse_qsl, quote, quote_plus, unquote, unquote_plus, urlsplit + +__all__ = [ + "READ_METHODS", + "ExpectedSubmission", + "PinnedWrite", + "WireCheck", + "body_fields", + "body_shape", + "carries_secret", + "expected_submission", + "mask_secrets", + "secret_forms", +] + +READ_METHODS = frozenset({"GET", "HEAD", "OPTIONS"}) + + +# -- sign-in values --------------------------------------------------------------- + + +def secret_forms(value: str) -> set[str]: + """Every way ``value`` can show up in a URL, a body or a page: as is, + percent- and form-encoded, JSON-escaped, HTML-escaped and in base64.""" + + raw = value.encode("utf-8") + forms = { + value, + quote(value, safe=""), + quote(value), + quote_plus(value), + quote(value, safe="").lower(), + quote_plus(value).lower(), + json.dumps(value)[1:-1], + _html.escape(value), + base64.b64encode(raw).decode(), + base64.urlsafe_b64encode(raw).decode(), + } + forms |= {f.rstrip("=") for f in list(forms)} + return {f for f in forms if len(f) >= 3} + + +def carries_secret(text: str, secrets: Iterable[str]) -> bool: + """True when ``text`` holds any form of any of ``secrets``.""" + + candidates = {text, unquote(text), unquote_plus(text)} + return any( + form in candidate + for value in secrets + if value + for form in secret_forms(value) + for candidate in candidates + ) + + +def mask_secrets(text: str, secrets: Iterable[str], mask: str = "•" * 4) -> str: + """``text`` with every form of every secret replaced by ``mask``.""" + + for value in secrets: + if not value: + continue + for form in sorted(secret_forms(value), key=len, reverse=True): + text = text.replace(form, mask) + return text + + +# -- request bodies ----------------------------------------------------------------- + + +def _content_type(request: Any) -> str: + return (request.headers.get("content-type") or "").lower() + + +def body_fields(request: Any) -> Optional[list[tuple[str, str]]]: + """The fields a request carries in its body: URL-encoded, multipart + (files as ``file:``) or a flat JSON object. None when the body is + none of those (nested JSON, another encoding).""" + + body = request.post_data_buffer or b"" + ctype = _content_type(request) + if not body: + return [] + if ctype.startswith("application/x-www-form-urlencoded"): + return parse_qsl(body.decode("utf-8", "replace"), keep_blank_values=True) + if ctype.startswith("multipart/form-data"): + from email.parser import BytesParser + from email.policy import HTTP + + message = BytesParser(policy=HTTP).parsebytes( + b"Content-Type: " + ctype.encode() + b"\r\n\r\n" + body + ) + pairs: list[tuple[str, str]] = [] + for part in message.iter_parts(): + name = part.get_param("name", header="content-disposition") + filename = part.get_param("filename", header="content-disposition") + if name is None: + continue + if filename is not None: + pairs.append((str(name), f"file:{filename}")) + else: + payload = part.get_payload(decode=True) + text = ( + payload.decode("utf-8", "replace") + if isinstance(payload, bytes) + else "" + ) + pairs.append((str(name), text)) + return pairs + if ctype.startswith("application/json"): + try: + data = json.loads(body) + except ValueError: + return None + if not isinstance(data, dict) or any( + isinstance(v, (dict, list)) for v in data.values() + ): + return None + return [(str(k), "" if v is None else str(v)) for k, v in data.items()] + return None + + +def body_shape(request: Any) -> str: + """A digest of the field NAMES a write carries (form fields, multipart + parts or JSON key paths, nested ones included), never their values: the + same save with new values has the same shape.""" + + names: list[str] = [] + fields = body_fields(request) + if fields is not None: + names = [name for name, _ in fields] + elif _content_type(request).startswith("application/json"): + try: + data = json.loads(request.post_data_buffer or b"null") + except ValueError: + data = None + + def walk(node: Any, prefix: str) -> None: + if isinstance(node, dict): + for key, item in node.items(): + names.append(prefix + str(key)) + walk(item, prefix + str(key) + ".") + elif isinstance(node, list): + for item in node: + walk(item, prefix + "[].") + + walk(data, "") + else: + names = [_content_type(request).split(";")[0], "body"] + return hashlib.sha256("\n".join(sorted(set(names))).encode()).hexdigest()[:16] + + +# -- what may be sent ------------------------------------------------------------------ + + +@dataclass(frozen=True) +class ExpectedSubmission: + """The one request a checked form's save sends. Fields are compared as a + multiset, ignoring the ``exempt`` names (``?name`` for query values).""" + + method: str + address: str # origin and path + query: tuple[tuple[str, str], ...] = () + fields: tuple[tuple[str, str], ...] = () + exempt: frozenset[str] = frozenset() + + +@dataclass(frozen=True) +class PinnedWrite: + """One taught write: ``"METHOD /path"`` and its :func:`body_shape`.""" + + write: str + shape: str + + +@dataclass +class WireCheck: + """What may leave the browser; see the module docstring.""" + + origin: Optional[str] = None + secrets: Sequence[str] = () + submission: Optional[ExpectedSubmission] = None + allowed: Optional[frozenset[str]] = None + pinned: Optional[Sequence[PinnedWrite]] = None + #: Refusals, in order: each request that was not sent, and why. + refused: list[str] = field(default_factory=list) + #: The writes that were sent, as ``"METHOD /path"``, with their shapes. + sent: list[str] = field(default_factory=list) + shapes: list[str] = field(default_factory=list) + _submitted: bool = False + + def judge(self, request: Any) -> Optional[str]: + """``None`` when ``request`` may go; else why it must not.""" + + if self.secrets and carries_secret(request.url, self.secrets): + return "a request with a sign-in value in its address" + if request.redirected_from is not None: + return None # the server's answer to a request already judged + method = request.method.upper() + parts = urlsplit(request.url) + address = f"{parts.scheme}://{parts.netloc}{parts.path}" + origin = f"{parts.scheme}://{parts.netloc}".lower() + sub = self.submission + is_write = method not in READ_METHODS + submission_get = ( + sub is not None + and method == "GET" + and sub.method.upper() == "GET" + and request.is_navigation_request() + and address == sub.address + ) + if not is_write and not submission_get: + return None + write = f"{method} {parts.path or '/'}" + if self.origin is not None and origin != self.origin: + return f"{write} to another site" + if sub is not None: + return self._judge_submission(request, method, address, parts.query) + if self.allowed is None: + return f"{write}, a write this run doesn't send" + if write not in self.allowed: + return f"{write}, which the write contract doesn't list" + shape = body_shape(request) + if self.pinned is not None: + i = len(self.sent) + want = self.pinned[i] if i < len(self.pinned) else None + if want is None or (want.write, want.shape) != (write, shape): + return f"{write}, which is not taught write {i + 1}" + return None + + def _judge_submission( + self, request: Any, method: str, address: str, query: str + ) -> Optional[str]: + sub = self.submission + assert sub is not None + if self._submitted: + return "the page sent a second submission after the checked one" + if method != sub.method.upper(): + return ( + f"the save was sent with {method}, the checked form sends with " + f"{sub.method.upper()}" + ) + if address != sub.address: + return "the save was sent to a different address than the checked form's" + + def norm(pairs: Iterable[tuple[str, str]], *, as_query: bool = False): + out = [] + for key, value in pairs: + key = str(key) + if (("?" + key) if as_query else key) in sub.exempt: + continue + out.append((key, str(value).replace("\r\n", "\n"))) + return sorted(out) + + url_query = parse_qsl(query, keep_blank_values=True) + if method == "GET": + sent = norm(url_query) + else: + if norm(url_query, as_query=True) != norm(sub.query, as_query=True): + return ( + "the save was sent with a different query than the checked form's" + ) + body = body_fields(request) + if body is None: + return ( + "the save was sent in a shape this check can't compare with " + "the form (not form fields or flat JSON)" + ) + sent = norm(body) + if sent != norm(sub.fields): + return "what the save sent differs from the checked form" + return None + + def handle( + self, route: Any, *, proceed: Optional[Callable[[], Any]] = None + ) -> None: + """A Playwright route handler: send or abort, and record either.""" + + request = route.request + try: + why = self.judge(request) + except Exception: # noqa: BLE001 - a request this check can't read + why = "a request this check could not read" + if why is not None: + self.refused.append(why) + try: + route.abort() + except Exception: # noqa: BLE001 - already handled + pass + return + method = request.method.upper() + if request.redirected_from is None and ( + method not in READ_METHODS + or ( + self.submission is not None + and request.is_navigation_request() + and method == self.submission.method.upper() + ) + ): + parts = urlsplit(request.url) + self.sent.append(f"{method} {parts.path or '/'}") + try: + self.shapes.append(body_shape(request)) + except Exception: # noqa: BLE001 + self.shapes.append("") + if self.submission is not None: + self._submitted = True + (proceed or route.continue_)() + + +def expected_submission( + raw: Mapping[str, Any], exempt: Iterable[str] +) -> ExpectedSubmission: + """From the page's ``{address, query, method, pairs}`` of a checked form.""" + + return ExpectedSubmission( + method=str(raw.get("method") or "get").upper(), + address=str(raw.get("address") or ""), + query=tuple((str(k), str(v)) for k, v in raw.get("query") or ()), + fields=tuple((str(k), str(v)) for k, v in raw.get("pairs") or ()), + exempt=frozenset(exempt), + ) diff --git a/packages/flow/tests/test_create_form_identity.py b/packages/flow/tests/test_create_form_identity.py index 6453aba2eb..ae74e49087 100644 --- a/packages/flow/tests/test_create_form_identity.py +++ b/packages/flow/tests/test_create_form_identity.py @@ -24,7 +24,7 @@ import threading from pathlib import Path from typing import Any, Callable, Iterator -from urllib.parse import parse_qs, urlsplit +from urllib.parse import parse_qs, parse_qsl, urlsplit import pytest @@ -222,8 +222,7 @@ def _page(variant: str) -> str: return f""" {title}

{title}

-
+ {hidden}