From 37605dd5fd04a1ef8517d3ec078ed88348c0a8f4 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 00:22:13 -0400 Subject: [PATCH 1/4] feat: arm a create-new form's save 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"). `record --identifier FIELD` didn't help: its pixel crop pictures the demo's own value, so a run with a new value halted. The record such a save commits is the one the run typed into the form. The submit-binding observer now also reads a labelled control's form relation, and the compiler gives a consequential labelled click that has no other identity evidence, and is a button of the one form owning every field typed since the last save, an Anchor.form_identity (counts and tokens only). Before the click the replayer checks, in the DOM, that the button's form holds exactly the fields this run typed into and that each still holds the value the run left in it (the Playwright backend snapshots each field after typing). A rewritten field, a missing field or a button of another form halts before the click; a backend that can't read the page leaves identity unreadable, which halts too. Found by the onboarding-effort benchmark (#1447). Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/LIMITS.md | 15 ++ .../backends/playwright_backend.py | 103 +++++++++- .../flow/openadapt_flow/compiler/compile.py | 29 ++- .../openadapt_flow/compiler/fixup_passes.py | 1 + .../openadapt_flow/interactive_recorder.py | 29 +-- packages/flow/openadapt_flow/ir.py | 34 ++++ packages/flow/openadapt_flow/policy.py | 3 +- packages/flow/openadapt_flow/recorder.py | 15 +- .../flow/openadapt_flow/runtime/replayer.py | 43 ++++ .../flow/openadapt_flow/submit_binding.py | 99 ++++++++- .../flow/tests/test_create_form_identity.py | 192 ++++++++++++++++++ 11 files changed, 530 insertions(+), 33 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 4211a7e91..aefeb3481 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -343,6 +343,21 @@ 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 a button of the one form that owns every field +typed since the last save, the compiler gives it a **form identity** +(`Anchor.form_identity`; counts and tokens only, no values). Before the click, +a run requires that the button's form holds exactly the fields that run typed +into, and that each still holds the value the run left in it. A field the page +rewrote after typing, a field that is gone, or a button of another form halts +before the click (identity mode `form`). A backend that can't read the page +leaves the identity unreadable, which halts too. A field whose own script +reformats its value when it loses focus (a phone-number mask, for example) +also halts, because its value no longer matches what was typed. + 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 8e95edacb..7dc73027c 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, @@ -43,6 +44,7 @@ OBSERVE_SUBMIT_BINDING_JS, SUBMIT_BINDING_FIELD_SELECTOR, observation_from_page, + verify_form_identity, verify_submit_binding, ) @@ -478,6 +480,56 @@ class _FramePoint: }""" ) +# Remember what a field holds once the run has finished typing into it, for +# a create-new form's identity check (openadapt_flow.ir.FormIdentity). The +# WeakMap lives in the field's own document under a per-backend random key and +# is only ever compared in the page; no value is returned to Python. +_SNAPSHOT_FILLED_VALUE_JS = r"""(el, key) => { + let values = window[key]; + if (!(values instanceof WeakMap)) { + values = new WeakMap(); + Object.defineProperty(window, key, {value: values, configurable: true}); + } + const value = ('value' in el) ? String(el.value) : String(el.innerText || ''); + values.set(el, value); + 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); + 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 || ''); + if (!(values instanceof WeakMap) || !values.has(field) || + values.get(field) !== now) { + changed += 1; + } + } + return {observation: out.observation, changed: changed}; +}""" +) + # 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 @@ -1160,6 +1212,7 @@ 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_key = f"__oaflow_filled_values_{uuid.uuid4().hex}" self._structural_tokens: dict[str, _StructuralGuard] = {} self._guarded_coordinate: Optional[_CoordinateGuard] = None # One-shot expectation that the next click closes this page surface @@ -1741,6 +1794,51 @@ def _register_focused_field(self) -> None: _REGISTER_FILLED_FIELD_JS, arg=self._fill_store_key, with_arg=True ) + def _snapshot_focused_field(self) -> None: + """Remember what the focused field holds now that typing is done. + + A create-new form's identity check compares each filled field with + this snapshot before the save. A field with no snapshot counts as + changed, so a failure here makes that check refuse: the safe + direction. + """ + + self._focused_element_call( + _SNAPSHOT_FILLED_VALUE_JS, arg=self._fill_value_key, 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's form owns exactly the fields this run + typed into, the control plays the recorded submit role, and every one + of those fields still holds what the run typed. Otherwise the reason, + in plain words, including when the page can't be read. + """ + + 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_key, + }, + ) + if not isinstance(raw, dict): + return "no control at the resolved point" + observation = observation_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" + return verify_form_identity(identity, observation, changed_fields=changed) + def _focused_element_call( self, js: str, *, arg: Any = None, with_arg: bool = False ) -> Any: @@ -2952,13 +3050,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() + return receipt def _handle_screenshot_frame_lifecycle(self, _frame: Any = None) -> None: """Advance the irreversible frame-tree generation.""" @@ -3533,6 +3633,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() 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 f4fdf3221..fd4e0070f 100644 --- a/packages/flow/openadapt_flow/compiler/compile.py +++ b/packages/flow/openadapt_flow/compiler/compile.py @@ -3529,11 +3529,18 @@ 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, observation_from_page, ) @@ -3564,6 +3571,24 @@ 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)) + 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 575b10480..e795c6300 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 43e459af5..cc4cea6f9 100644 --- a/packages/flow/openadapt_flow/interactive_recorder.py +++ b/packages/flow/openadapt_flow/interactive_recorder.py @@ -1927,10 +1927,10 @@ 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 @@ -1938,7 +1938,11 @@ def select_attached_page( 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; + 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; } } @@ -4139,15 +4143,12 @@ 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"] diff --git a/packages/flow/openadapt_flow/ir.py b/packages/flow/openadapt_flow/ir.py index 6c88d7982..ca3af73bd 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -344,6 +344,31 @@ 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 + holds 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. The recorder observes, right before the demonstrated + click on a labelled save, that the control belongs to the same HTML form + as every field typed into since the last save (and is not in a table row + or a single field's own widget). Before the click at run time, the + replayer requires the same: the control's form owns exactly + ``filled_fields`` fields this run typed into, the control has the same + ``submit_role``, and every one of those fields still holds the value it + held when the run finished typing it. Anything else halts before the + click. + + Counts and closed-vocabulary tokens 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", "form_control"] + + class Anchor(BaseModel): """Redundant evidence for locating a step's target on screen. @@ -445,6 +470,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, @@ -3664,6 +3697,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 eb772b288..34ae41958 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 f27a6b6a9..7c3dc79be 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. @@ -693,14 +692,12 @@ 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} diff --git a/packages/flow/openadapt_flow/runtime/replayer.py b/packages/flow/openadapt_flow/runtime/replayer.py index e5df26bc3..898933921 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, @@ -3752,6 +3753,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: @@ -10587,6 +10589,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 ) ) ): @@ -10965,6 +10968,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( @@ -15381,6 +15385,11 @@ def _verify_identity( """ anchor = step.anchor assert anchor is not None + 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, and nothing else + # can stand in for that check (see ir.FormIdentity). + return self._verify_form_identity(step, resolution, anchor.form_identity) project = getattr(workflow, "qualification", None) if project is not None: identity_policy = project.identity_policies.get(step.id) @@ -15633,6 +15642,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, @@ -15948,6 +15990,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 cd83183c4..b6cc95768 100644 --- a/packages/flow/openadapt_flow/submit_binding.py +++ b/packages/flow/openadapt_flow/submit_binding.py @@ -54,6 +54,7 @@ from openadapt_flow.ir import ( SUBMIT_BINDING_PATH_PATTERN, SUBMIT_BINDING_TOKEN_PATTERN, + FormIdentity, SubmitBinding, ) @@ -62,9 +63,11 @@ "SUBMIT_BINDING_ACTIONABLE_SELECTOR", "SUBMIT_BINDING_FIELD_SELECTOR", "SubmitBindingObservation", + "decide_form_identity", "decide_submit_binding", "describe_submit_binding", "observation_from_page", + "verify_form_identity", "verify_submit_binding", ] @@ -250,12 +253,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 +318,9 @@ 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) { + return {observation: result, canon: JSON.stringify(result)}; + } let top = Infinity; let left = Infinity; @@ -465,6 +471,87 @@ 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 not in ("form_submit", "form_control"): + return "the control is not a button of that form" + return None + + +def decide_form_identity( + observation: SubmitBindingObservation, +) -> tuple[Optional[FormIdentity], Optional[str]]: + """A labelled save's form identity from the demonstration, or a reason. + + Returns ``(identity, None)`` when the control is a button of the one form + that owns every field typed into since the last save, else + ``(None, reason)``. + """ + + reason = _no_form_identity_reason(observation) + if reason is not None: + return None, reason + assert observation.submit_role in ("form_submit", "form_control") + return ( + FormIdentity( + filled_fields=observation.fields, + submit_role=observation.submit_role, + ), + None, + ) + + +def verify_form_identity( + identity: FormIdentity, + observation: SubmitBindingObservation, + *, + changed_fields: int, +) -> 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 whose + value differs from what the run left in them. Otherwise return the first + difference, in plain words. + """ + + reason = _no_form_identity_reason(observation) + if reason is not None: + return reason + 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.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 changed_fields: + return ( + f"{changed_fields} field(s) of the form no longer hold the value this " + "run typed" + ) + 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 000000000..560b56681 --- /dev/null +++ b/packages/flow/tests/test_create_form_identity.py @@ -0,0 +1,192 @@ +"""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 two +variants that must halt before the click. +""" + +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, 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 = """ +""" + + +def _page(variant: str) -> str: + # In the "split" variant the Create button belongs to another form. + owner = ' form="other"' if variant == "split" else "" + return f""" +New ticket +

New ticket

+
+

+

+

+
+
+
+ +{_TAMPER if variant == "tamper" 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() + + +@pytest.fixture(scope="module") +def bundle(url: str, tmp_path_factory: pytest.TempPathFactory) -> Path: + out = tmp_path_factory.mktemp("create-form") + + 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" + + +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 "no longer hold 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 From 17a13f311cb27e45216e6d76d5d2e37950417f18 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 10:15:30 -0400 Subject: [PATCH 2/4] fix: a create-form field must hold exactly what the run typed; form= and portal saves Review cases for React apps: - A controlled input that reformats the value now halts. The Playwright backend records, after typing, whether the field holds exactly the typed text (a select: its chosen option's label), and the form check counts a field that didn't as changed. A field reformatted on every keystroke stops at the typed-value read-back; one reformatted on blur stops at the form check. - A submit button outside the
element is armed when its form= attribute names the form. One with no form owner (a portal that submits from script) gets no form identity and stays unarmed, and a program recorded with form= halts before the click on such a page. Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/LIMITS.md | 11 +- .../backends/playwright_backend.py | 54 +++++--- .../flow/openadapt_flow/submit_binding.py | 9 +- .../flow/tests/test_create_form_identity.py | 129 ++++++++++++++++-- 4 files changed, 168 insertions(+), 35 deletions(-) diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index 5fbff448a..42539c9b4 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -386,9 +386,14 @@ a run requires that the button's form holds exactly the fields that run typed into, and that each still holds the value the run left in it. A field the page rewrote after typing, a field that is gone, or a button of another form halts before the click (identity mode `form`). A backend that can't read the page -leaves the identity unreadable, which halts too. A field whose own script -reformats its value when it loses focus (a phone-number mask, for example) -also halts, because its value no longer matches what was typed. +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 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. For consequential or entity-sensitive workflows, require a policy that refuses unarmed actions, review the inferred risk and tags, and test identity against diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 5d7c12296..8e9b5483d 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -480,18 +480,31 @@ class _FramePoint: }""" ) -# Remember what a field holds once the run has finished typing into it, for -# a create-new form's identity check (openadapt_flow.ir.FormIdentity). The -# WeakMap lives in the field's own document under a per-backend random key and -# is only ever compared in the page; no value is returned to Python. -_SNAPSHOT_FILLED_VALUE_JS = r"""(el, key) => { - let values = window[key]; +# 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, key, {value: values, configurable: true}); + 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 || ''); - values.set(el, value); + 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; }""" @@ -521,10 +534,8 @@ class _FramePoint: if (owner !== form) continue; const now = ('value' in field) ? String(field.value) : String(field.innerText || ''); - if (!(values instanceof WeakMap) || !values.has(field) || - values.get(field) !== now) { - changed += 1; - } + const left = (values instanceof WeakMap) ? values.get(field) : null; + if (!left || !left.typed || left.value !== now) changed += 1; } return {observation: out.observation, changed: changed}; }""" @@ -1993,17 +2004,20 @@ def _register_focused_field(self) -> None: _REGISTER_FILLED_FIELD_JS, arg=self._fill_store_key, with_arg=True ) - def _snapshot_focused_field(self) -> None: - """Remember what the focused field holds now that typing is done. + 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 counts as - changed, so a failure here makes that check refuse: the safe - direction. + 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=self._fill_value_key, with_arg=True + _SNAPSHOT_FILLED_VALUE_JS, + arg={"key": self._fill_value_key, "typed": typed}, + with_arg=True, ) def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[str]: @@ -3256,7 +3270,7 @@ def type_text_guarded( # keyboard, so delivery stays bound to the checked target. deliver=lambda locator: locator.press_sequentially(text, timeout=1000), ) - self._snapshot_focused_field() + self._snapshot_focused_field(text) return receipt def _handle_screenshot_frame_lifecycle(self, _frame: Any = None) -> None: @@ -3832,7 +3846,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() + 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/submit_binding.py b/packages/flow/openadapt_flow/submit_binding.py index b6cc95768..bd9ff454f 100644 --- a/packages/flow/openadapt_flow/submit_binding.py +++ b/packages/flow/openadapt_flow/submit_binding.py @@ -526,8 +526,9 @@ def verify_form_identity( """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 whose - value differs from what the run left in them. Otherwise return the first + 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. Otherwise return the first difference, in plain words. """ @@ -546,8 +547,8 @@ def verify_form_identity( ) if changed_fields: return ( - f"{changed_fields} field(s) of the form no longer hold the value this " - "run typed" + f"{changed_fields} field(s) of the form don't hold exactly the value " + "this run typed" ) return None diff --git a/packages/flow/tests/test_create_form_identity.py b/packages/flow/tests/test_create_form_identity.py index 560b56681..f63dabba4 100644 --- a/packages/flow/tests/test_create_form_identity.py +++ b/packages/flow/tests/test_create_form_identity.py @@ -41,9 +41,46 @@ """ +_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. + '" +) + + def _page(variant: str) -> str: - # In the "split" variant the Create button belongs to another form. - owner = ' form="other"' if variant == "split" else "" + # "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 in ("plain", "tamper", "split", "reformat", "blurformat") + else "" + ) + outside = ( + _CREATE.format(owner=owner) + if variant == "formattr" + else (_PORTAL if variant == "portal" else "") + ) return f""" New ticket

New ticket

@@ -52,12 +89,15 @@ def _page(variant: str) -> str:

-

+

{inside}

+
{_TAMPER if variant == "tamper" else ""} +{_REFORMAT if variant == "reformat" else ""} +{_BLUR_FORMAT if variant == "blurformat" else ""} """ @@ -87,10 +127,7 @@ def url() -> Iterator[str]: stop() -@pytest.fixture(scope="module") -def bundle(url: str, tmp_path_factory: pytest.TempPathFactory) -> Path: - out = tmp_path_factory.mktemp("create-form") - +def _record(url: str, out: Path) -> Path: def drive(page: Any, pump: Any) -> None: def settle() -> None: for _ in range(12): @@ -120,6 +157,16 @@ def settle() -> None: 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 @@ -178,7 +225,7 @@ def test_a_field_changed_after_typing_halts_before_the_click( assert created == [] save = next(r for r in report.results if r.identity and r.identity.mode == "form") assert save.identity.status == "mismatch" - assert "no longer hold the value this run typed" in save.identity.observed + 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( @@ -190,3 +237,69 @@ def test_a_button_of_another_form_halts_before_the_click( 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 + ) From 81d00f5a4ef13819a7ba498dfc7dea016d021096 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 10:43:37 -0400 Subject: [PATCH 3/4] fix: a create-form save's identity covers the whole form, and only its own submit Review of #1543: form identity checked only that the typed fields kept their typed values, so an edit form that looked like the create form, an address, method or record id a script changed or added, a hidden parent or an untouched select on another record, and a click handler that submitted another form all verified and submitted wrong data. - Fingerprint: the observer records, for a labelled submit button of the filled form, the submission's origin and path, its method, and the sorted name/type list of every control it would send (hidden ones included). FormIdentity keeps their SHA-256 and the run recomputes them right before the click; any difference halts. - Untouched values: every field the run didn't type into is kept as a salted digest (the identity-template scheme) and compared. Hidden tokens are exempt: named like one (csrf, token, nonce...), since their value changes every page load, or changed while the demonstration was typed. Their values are never kept. Passwords never leave the page. - Only a form's own submit button gets a form identity. Right before the click a one-shot capture-phase submit guard cancels any submit that isn't that form from that button, and the step halts. - A signal-quorum identity policy and a form identity must now both hold. Tests: the review's six holes halt before the click with nothing submitted, a per-session CSRF token and a hidden value that rotated during the demonstration still verify, and a keyed React remount is pinned as a known false halt (LIMITS.md). Co-Authored-By: Claude Opus 5.5 --- packages/flow/docs/LIMITS.md | 52 ++-- .../backends/playwright_backend.py | 128 +++++++++- .../flow/openadapt_flow/compiler/compile.py | 17 +- .../openadapt_flow/interactive_recorder.py | 18 +- packages/flow/openadapt_flow/ir.py | 46 +++- packages/flow/openadapt_flow/recorder.py | 15 ++ .../runtime/identity_template.py | 7 + .../flow/openadapt_flow/runtime/replayer.py | 48 +++- .../flow/openadapt_flow/submit_binding.py | 240 +++++++++++++++++- .../flow/tests/test_create_form_identity.py | 198 ++++++++++++++- 10 files changed, 701 insertions(+), 68 deletions(-) diff --git a/packages/flow/docs/LIMITS.md b/packages/flow/docs/LIMITS.md index 42539c9b4..625555937 100644 --- a/packages/flow/docs/LIMITS.md +++ b/packages/flow/docs/LIMITS.md @@ -379,21 +379,43 @@ 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 a button of the one form that owns every field -typed since the last save, the compiler gives it a **form identity** -(`Anchor.form_identity`; counts and tokens only, no values). Before the click, -a run requires that the button's form holds exactly the fields that run typed -into, and that each still holds the value the run left in it. A field the page -rewrote after typing, a field that is gone, or a button of another form halts -before the click (identity mode `form`). 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 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. +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 diff --git a/packages/flow/openadapt_flow/backends/playwright_backend.py b/packages/flow/openadapt_flow/backends/playwright_backend.py index 8e9b5483d..fdf5900dc 100644 --- a/packages/flow/openadapt_flow/backends/playwright_backend.py +++ b/packages/flow/openadapt_flow/backends/playwright_backend.py @@ -43,6 +43,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, verify_form_identity, verify_submit_binding, @@ -458,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'] + ); }""" ) @@ -476,6 +480,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; }""" ) @@ -524,7 +539,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; @@ -541,6 +556,47 @@ 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. +_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 @@ -1367,6 +1423,8 @@ 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_key = f"__oaflow_filled_values_{uuid.uuid4().hex}" + self._form_guard_key = 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 @@ -2023,12 +2081,16 @@ def _snapshot_focused_field(self, typed: str) -> None: 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's form owns exactly the fields this run - typed into, the control plays the recorded submit role, and every one - of those fields still holds what the run typed. Otherwise the reason, - in plain words, including when the page can't be read. + 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: @@ -2045,12 +2107,64 @@ def form_identity_at(self, x: int, y: int, identity: FormIdentity) -> Optional[s 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" - return verify_form_identity(identity, observation, changed_fields=changed) + 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_key}, + ) + 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_key) + 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 diff --git a/packages/flow/openadapt_flow/compiler/compile.py b/packages/flow/openadapt_flow/compiler/compile.py index 4d4f9f692..6ca6a9cdd 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 @@ -3553,6 +3560,7 @@ def _attach_submit_bindings(steps: list[Step], observations: dict[str, object]) from openadapt_flow.submit_binding import ( decide_form_identity, decide_submit_binding, + form_values_from_record, observation_from_page, ) @@ -3593,7 +3601,10 @@ def _attach_submit_bindings(steps: list[Step], observations: dict[str, object]) # 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_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: diff --git a/packages/flow/openadapt_flow/interactive_recorder.py b/packages/flow/openadapt_flow/interactive_recorder.py index cc4cea6f9..49c4dc566 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 @@ -1937,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). @@ -2147,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); @@ -4153,8 +4161,14 @@ def _record_pointer(self, ev: dict[str, Any]) -> None: 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 e92cd3efc..3caaf7e03 100644 --- a/packages/flow/openadapt_flow/ir.py +++ b/packages/flow/openadapt_flow/ir.py @@ -366,28 +366,46 @@ def _serialize_compatible(self, handler: Any) -> dict[str, Any]: class FormIdentity(BaseModel): - """Record identity of a save on a create-new form: the form it submits - holds exactly what this run typed. + """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. The recorder observes, right before the demonstrated - click on a labelled save, that the control belongs to the same HTML form - as every field typed into since the last save (and is not in a table row - or a single field's own widget). Before the click at run time, the - replayer requires the same: the control's form owns exactly - ``filled_fields`` fields this run typed into, the control has the same - ``submit_role``, and every one of those fields still holds the value it - held when the run finished typing it. Anything else halts before the - click. - - Counts and closed-vocabulary tokens only; no field text or value. + 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", "form_control"] + 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): diff --git a/packages/flow/openadapt_flow/recorder.py b/packages/flow/openadapt_flow/recorder.py index 0d83f5ce5..4235c80d6 100644 --- a/packages/flow/openadapt_flow/recorder.py +++ b/packages/flow/openadapt_flow/recorder.py @@ -710,6 +710,9 @@ def _record(self, event: dict[str, Any], act: Callable[[], None]) -> None: 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) @@ -954,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 acc0ada0d..8e60778b1 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 10eca4e5f..158cba2a5 100644 --- a/packages/flow/openadapt_flow/runtime/replayer.py +++ b/packages/flow/openadapt_flow/runtime/replayer.py @@ -12937,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 @@ -15444,11 +15471,6 @@ def _verify_identity( """ anchor = step.anchor assert anchor is not None - 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, and nothing else - # can stand in for that check (see ir.FormIdentity). - return self._verify_form_identity(step, resolution, anchor.form_identity) project = getattr(workflow, "qualification", None) if project is not None: identity_policy = project.identity_policies.get(step.id) @@ -15456,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, @@ -15465,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 diff --git a/packages/flow/openadapt_flow/submit_binding.py b/packages/flow/openadapt_flow/submit_binding.py index bd9ff454f..0c2c80c45 100644 --- a/packages/flow/openadapt_flow/submit_binding.py +++ b/packages/flow/openadapt_flow/submit_binding.py @@ -47,7 +47,9 @@ 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 @@ -62,10 +64,15 @@ "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", @@ -99,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"]'; @@ -174,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, @@ -319,6 +377,12 @@ 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)}; } @@ -395,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: @@ -411,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: @@ -489,29 +654,53 @@ def _no_form_identity_reason(observation: SubmitBindingObservation) -> Optional[ 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 not in ("form_submit", "form_control"): - return "the control is not a button of that form" + 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 a button of the one form - that owns every field typed into since the last save, else + 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 - assert observation.submit_role in ("form_submit", "form_control") + 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=observation.submit_role, + 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, ) @@ -522,34 +711,61 @@ def verify_form_identity( 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. Otherwise return the first + 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.submit_role != identity.submit_role: + 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 control's submit role is {observation.submit_role!r}, it was " - f"{identity.submit_role!r}" + 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 diff --git a/packages/flow/tests/test_create_form_identity.py b/packages/flow/tests/test_create_form_identity.py index f63dabba4..5f0d0917b 100644 --- a/packages/flow/tests/test_create_form_identity.py +++ b/packages/flow/tests/test_create_form_identity.py @@ -9,14 +9,18 @@ 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 two -variants that must halt before the click. +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 @@ -66,29 +70,102 @@ ) +# 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 in ("plain", "tamper", "split", "reformat", "blurformat") - else "" + _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""" -New ticket -

New ticket

- +{title} +

{title}

+ + {hidden}

+ {extra}

{inside}

@@ -98,6 +175,7 @@ def _page(variant: str) -> str: {_TAMPER if variant == "tamper" else ""} {_REFORMAT if variant == "reformat" else ""} {_BLUR_FORMAT if variant == "blurformat" else ""} +{f"" if script else ""} """ @@ -303,3 +381,105 @@ def test_a_portal_button_with_no_form_owner_is_not_armed_and_halts( 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 e74703aa93fefba3f6ade35e16eddc82b87a4af3 Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 25 Oct 2026 10:51:38 -0400 Subject: [PATCH 4/4] chore: pin a reviewed gitleaks false positive from #1543's first commit `arg=self._fill_value_key` passes a per-backend page key (a uuid made at runtime) and matched generic-api-key. The line is gone since 81d00f5a4; the fingerprint pins commit, file, rule and line, so nothing else is relaxed. Co-Authored-By: Claude Opus 5.5 --- .gitleaksignore | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.gitleaksignore b/.gitleaksignore index b61b5d9e4..6c10f7ece 100644 --- a/.gitleaksignore +++ b/.gitleaksignore @@ -60,3 +60,6 @@ e8ec96725c4451aa64cb97230c18d04b55d98503:packages/flow/tests/e2e/test_mcp_teach_ # test_network_capture_redact_json.py; the next commit builds them at run time. 52502adf407793c211fb3c0ac5aba37556206d8c:packages/flow/tests/test_network_capture_redact_json.py:generic-api-key:17 52502adf407793c211fb3c0ac5aba37556206d8c:packages/flow/tests/test_network_capture_redact_json.py:generic-api-key:146 +# PR #1543: `arg=self._fill_value_key` names a per-backend page key (a uuid made at +# runtime), not a credential. The line no longer exists after 81d00f5a4. +37605dd5fd04a1ef8517d3ec078ed88348c0a8f4:packages/flow/openadapt_flow/backends/playwright_backend.py:generic-api-key:1807