diff --git a/packages/flow/benchmark/odoo_contacts/README.md b/packages/flow/benchmark/odoo_contacts/README.md new file mode 100644 index 000000000..fbaebec75 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/README.md @@ -0,0 +1,59 @@ +# Odoo 18 Community: create a contact + +The skills registry's first non-healthcare candidate. Odoo Community is +LGPL-3.0. Its images are pulled by digest at run time +(`environment.lock.json`), and nothing of Odoo is stored here. All data is +synthetic. + +| File | What it is | +| --- | --- | +| `compose.yml`, `fixture.py` | The test copy: Odoo 18.0 and Postgres 16 on loopback ports 9412 and 9413, the `oa` database with Contacts and no demo data. A dedicated user (`openadapt.actor@example.invalid`, with Contact Creation) and a read-only role for the record check (`oa_record_check`). | +| `qualify_hooks.py` | Signs the dedicated user in, opens a new contact, archives contacts between cases, and writes a signed-in browser state (what a registry user binds as `SIGNED_IN_STATE`). | +| `demo.py` | The scripted demonstration (choose Individual, fill name, street, city, ZIP, phone and email, save), then compile. | +| `verify.yaml` | The record check: one parameterized, read-only SELECT of the typed fields from `res_partner`, keyed by email. | +| `results/2026-10-28-click-teach/qualification/` | The qualification evidence. The compiled bundle isn't committed; `demo.py compile` rebuilds it. | + +```bash +cd packages/flow +export ODOO_RECORD_PASSWORD=... # from the fixture's secrets.json +PYTHONPATH=.:benchmark/odoo_contacts python benchmark/odoo_contacts/demo.py record --out REC +PYTHONPATH=.:benchmark/odoo_contacts python benchmark/odoo_contacts/demo.py compile --recording REC --bundle B +PYTHONPATH=.:benchmark/odoo_contacts openadapt-flow qualify B \ + --verify benchmark/odoo_contacts/verify.yaml --out Q --certify clinical-write +``` + +`PYTHONPATH` includes this folder because an installed `benchmark` package +can shadow this repository's namespace package. + +## Result (2026-10-28) + +The demonstration saved the contact, and the record check's SELECT read all +six fields back. The compiled program has 14 steps and 6 parameters. +Qualification under `clinical-write` refused, as it should, before any case +ran: + +- **Short labels.** The clicks into City and Phone are unarmed: the only + text beside each is a one-word label, too little for an identity check. + Reaching them by keyboard from a checked click (Street, Email) works. + Those keystrokes need the focus check (#1579) to carry a postcondition. +- **An icon-only save button.** Odoo's save button is a cloud icon with no + text, so its click can't carry an identity check. This is the same class + as Medplum's Create button (#1581, form identity). Odoo's own save + shortcut is Control+S on macOS and Alt+S elsewhere, so a taught shortcut + would replay on one platform only. + +So this skill isn't in the registry yet. Under the threat model (#1598) the +record check is what proves the save, and identity checks are fail-closed +pre-checks only, so the next step is the icon-button identity in #1581, not +a page heuristic here. + +## Setting the fixture up + +- Odoo keeps a long-polling connection open, so a page never goes network + idle. The hooks wait for the form instead. +- A plain internal user can't create contacts, and Odoo then shows no save + button. The dedicated user needs Contact Creation + (`base.group_partner_manager`). + +Effort: about 0.8 hours of lease time, from bring-up to the refused +qualification. diff --git a/packages/flow/benchmark/odoo_contacts/compose.yml b/packages/flow/benchmark/odoo_contacts/compose.yml new file mode 100644 index 000000000..53fd023d7 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/compose.yml @@ -0,0 +1,41 @@ +# Odoo 18 Community (LGPL-3.0), self-hosted for the skills registry's first +# non-healthcare entry: create a contact. Synthetic data only. Images are +# pinned by digest; the web and database ports are published on loopback only; +# secrets come +# from the fixture's generated state (fixture.py). +services: + db: + image: ${ODOO_PG_IMAGE} + environment: + POSTGRES_USER: odoo + POSTGRES_PASSWORD: ${ODOO_DB_PASSWORD} + POSTGRES_DB: postgres + # Loopback only: the record check reads res_partner through a read-only role. + ports: + - "127.0.0.1:${ODOO_DB_PORT}:5432" + volumes: + - pgdata:/var/lib/postgresql/data + healthcheck: + test: ["CMD-SHELL", "pg_isready -U odoo"] + interval: 5s + timeout: 5s + retries: 30 + + odoo: + image: ${ODOO_IMAGE} + depends_on: + db: + condition: service_healthy + environment: + HOST: db + USER: odoo + PASSWORD: ${ODOO_DB_PASSWORD} + command: ["odoo", "--db-filter=^oa$", "--without-demo=all", "--proxy-mode"] + ports: + - "127.0.0.1:${ODOO_PORT}:8069" + volumes: + - odoodata:/var/lib/odoo + +volumes: + pgdata: + odoodata: diff --git a/packages/flow/benchmark/odoo_contacts/demo.py b/packages/flow/benchmark/odoo_contacts/demo.py new file mode 100644 index 000000000..0c0df37c1 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/demo.py @@ -0,0 +1,121 @@ +"""Teach Odoo's new-contact form: a scripted demonstration, then compile. + + PYTHONPATH=.:benchmark/odoo_contacts python benchmark/odoo_contacts/demo.py \ + record --out DIR [--values FILE] + PYTHONPATH=.:benchmark/odoo_contacts python benchmark/odoo_contacts/demo.py \ + compile --recording DIR --bundle DIR + +Signing in and opening a new contact are setup, not part of the +demonstration (``qualify_hooks.open_session``). The demonstration marks the +contact an Individual (the form starts on Company), fills it, and saves it +with the form's save button. Every action goes through the ``Recorder``, so +the frames and events are what a person's demonstration would leave; record +time uses Playwright locators only to find where to click. + +Synthetic values only. +""" + +from __future__ import annotations + +import argparse +import json +from pathlib import Path +from typing import Any + +#: The demonstration's contact (synthetic). Replays bring their own values. +DEFAULT_VALUES = { + "name": "Rowan Achterberg", + "street": "41 Quarry Lane", + "city": "Worcester", + "zip": "01609", + "phone": "508-555-0147", + "email": "rowan.achterberg@example.invalid", +} + +SETTLE = { + "settle_timeout_s": 10.0, + "settle_stable_frames": 3, + "settle_interval_s": 0.3, +} + + +def _click(recorder: Any, page: Any, locator: Any) -> None: + box = locator.bounding_box() + if box is None: + raise RuntimeError("target is not on the page") + recorder.click(int(box["x"] + box["width"] / 2), int(box["y"] + box["height"] / 2)) + page.wait_for_timeout(300) + + +def demonstrate(recorder: Any, page: Any, values: dict[str, str]) -> None: + """Fill a new person contact once and save it.""" + + _click(recorder, page, page.get_by_label("Individual")) + _click(recorder, page, page.locator("div[name=name] input")) + recorder.type_text(values["name"], param="name") + for placeholder, param in (("Street...", "street"), ("City", "city"), ("ZIP", "zip")): + _click(recorder, page, page.get_by_placeholder(placeholder, exact=True)) + recorder.type_text(values[param], param=param) + # Phone and Email have a label beside them, not a placeholder. + for param in ("phone", "email"): + _click(recorder, page, page.locator(f"#{param}_0")) + recorder.type_text(values[param], param=param) + _click(recorder, page, page.get_by_role("button", name="Save manually")) + page.wait_for_url(lambda url: not url.rstrip("/").endswith("/new"), timeout=30_000) + page.wait_for_timeout(1500) + + +def cmd_record(args: argparse.Namespace) -> int: + from types import SimpleNamespace + + import qualify_hooks + + from openadapt_flow.recorder import Recorder + + out = Path(args.out) + if out.exists(): + raise SystemExit(f"recording path already exists: {out}") + values = dict(DEFAULT_VALUES) + if args.values: + values.update(json.loads(Path(args.values).read_text())) + config = SimpleNamespace(app=SimpleNamespace(viewport=(1280, 900))) + backend, close = qualify_hooks.open_session(config) + try: + recorder = Recorder(backend, out, app_url=backend.page.url, **SETTLE) + demonstrate(recorder, backend.page, values) + print("recording:", recorder.finish()) + print("saved:", backend.page.url) + finally: + close() + return 0 + + +def cmd_compile(args: argparse.Namespace) -> int: + from openadapt_flow.compiler import compile_recording + + bundle = Path(args.bundle) + if bundle.exists(): + raise SystemExit(f"bundle path already exists: {bundle}") + workflow = compile_recording(Path(args.recording), bundle, name="odoo-new-contact") + print("bundle:", bundle, "steps:", len(workflow.steps)) + print("params:", sorted(workflow.params) if workflow.params else []) + return 0 + + +def main(argv: Any = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + sub = parser.add_subparsers(dest="cmd", required=True) + p = sub.add_parser("record") + p.add_argument("--out", required=True) + p.add_argument("--values", default=None, help="JSON file of field values") + p.set_defaults(func=cmd_record) + p = sub.add_parser("compile") + p.add_argument("--recording", required=True) + p.add_argument("--bundle", required=True) + p.set_defaults(func=cmd_compile) + args = parser.parse_args(argv) + return args.func(args) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/packages/flow/benchmark/odoo_contacts/environment.lock.json b/packages/flow/benchmark/odoo_contacts/environment.lock.json new file mode 100644 index 000000000..089a73634 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/environment.lock.json @@ -0,0 +1,12 @@ +{ + "schema_version": 1, + "generated_for": "OpenAdapt skills registry: Odoo 18 Community, create a contact (synthetic data)", + "license": "LGPL-3.0 (Odoo Community); images pulled at run time, never stored in this repository", + "services": { + "odoo": "odoo:18.0@sha256:9668abacd894276ed009e8db7db1d1ffb8068af6982f76a3400ed0d68e40432d", + "postgres": "postgres:16@sha256:1beaa3084eee2676f20a9cd9928f0bf2c5bf674f8e49f81017912183d377c61c" + }, + "platforms": "both images publish linux/arm64 and linux/amd64", + "project": "openadapt-si-odoo", + "database": "oa" +} diff --git a/packages/flow/benchmark/odoo_contacts/fixture.py b/packages/flow/benchmark/odoo_contacts/fixture.py new file mode 100644 index 000000000..04a0ccdbc --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/fixture.py @@ -0,0 +1,304 @@ +"""A local Odoo 18 Community (LGPL-3.0) with synthetic contacts. + +The skills registry's first non-healthcare entry: create a contact in the +Contacts app. Odoo's images are pulled by digest at run time +(environment.lock.json); nothing of Odoo is stored here. + +- The web app (``ui_url``) is what a clerk uses. +- The record check reads ``res_partner`` through a read-only database role + (``oa_record_check``) on the loopback database port. +- The oracle reads ``res_partner`` as the database owner from inside the + database container, a channel the product never uses. + +Generated secrets live in ``state_dir``, outside the repository. All data is +synthetic. +""" + +from __future__ import annotations + +import json +import os +import secrets +import subprocess +import time +import urllib.request +import xmlrpc.client +from pathlib import Path +from typing import Any, Optional +from urllib.error import HTTPError, URLError + +HERE = Path(__file__).resolve().parent +COMPOSE = HERE / "compose.yml" +PROJECT = "openadapt-si-odoo" +WEB_PORT = 9412 +DB_PORT = 9413 +DATABASE = "oa" +DEFAULT_STATE = Path.home() / ".openadapt" / "fixtures" / "odoo-state" +ACTOR_LOGIN = "openadapt.actor@example.invalid" +RECORD_ROLE = "oa_record_check" + + +class FixtureError(RuntimeError): + pass + + +class OdooFixture: + def __init__(self, state_dir: Optional[Path] = None) -> None: + self.state_dir = Path( + state_dir or os.environ.get("ODOO_STATE_DIR") or DEFAULT_STATE + ) + self.lock = json.loads((HERE / "environment.lock.json").read_text()) + for name, image in self.lock["services"].items(): + if "@sha256:" not in image: + raise FixtureError(f"{name} image is not pinned by digest: {image}") + + @property + def ui_url(self) -> str: + return f"http://localhost:{WEB_PORT}/" + + # -- secrets and compose --------------------------------------------------- + + def secrets(self) -> dict[str, str]: + path = self.state_dir / "secrets.json" + if path.exists(): + return json.loads(path.read_text()) + self.state_dir.mkdir(parents=True, exist_ok=True, mode=0o700) + data = { + "db_password": secrets.token_urlsafe(24), + "admin_password": secrets.token_urlsafe(18), + "actor_password": secrets.token_urlsafe(18), + "record_check_password": secrets.token_urlsafe(24), + } + path.touch(mode=0o600) + path.write_text(json.dumps(data, indent=2)) + return data + + def _env(self) -> dict[str, str]: + s = self.secrets() + images = self.lock["services"] + return { + "ODOO_IMAGE": images["odoo"], + "ODOO_PG_IMAGE": images["postgres"], + "ODOO_DB_PASSWORD": s["db_password"], + "ODOO_PORT": str(WEB_PORT), + "ODOO_DB_PORT": str(DB_PORT), + } + + def compose(self, *args: str, timeout: float = 1800) -> str: + proc = subprocess.run( + ["docker", "compose", "-p", PROJECT, "-f", str(COMPOSE), *args], + capture_output=True, + text=True, + env={**os.environ, **self._env()}, + timeout=timeout, + ) + if proc.returncode != 0: + raise FixtureError( + f"compose {' '.join(args[:2])} failed: {proc.stderr[-1500:]}" + ) + return proc.stdout + + def psql(self, sql: str) -> str: + return self.compose( + "exec", + "-T", + "db", + "psql", + "-U", + "odoo", + "-d", + DATABASE, + "-At", + "-v", + "ON_ERROR_STOP=1", + "-c", + sql, + ) + + # -- lifecycle ------------------------------------------------------------- + + def up(self, *, wait_s: float = 900.0) -> None: + """Start the database, create ``oa`` with Contacts once, start Odoo.""" + + self.compose("up", "-d", "db") + # Postgres answers (with 0 or 1) once it is up; 1 means ``oa`` exists. + self._wait(lambda: self._db_exists().strip() in ("0", "1"), wait_s, "database") + if self._db_exists().strip() != "1": + self.compose( + "run", + "--rm", + "odoo", + "odoo", + "-d", + DATABASE, + "-i", + "base,contacts", + "--without-demo=all", + "--stop-after-init", + ) + self.compose("up", "-d", "odoo") + self._wait(self._web_ready, wait_s, "Odoo web") + self.bootstrap() + + def down(self, *, volumes: bool = True) -> None: + self.compose("down", *(["-v"] if volumes else [])) + + def _db_exists(self) -> str: + try: + return self.compose( + "exec", + "-T", + "db", + "psql", + "-U", + "odoo", + "-d", + "postgres", + "-At", + "-c", + f"SELECT count(*) FROM pg_database WHERE datname = '{DATABASE}';", + ) + except FixtureError: + return "" + + def _web_ready(self) -> bool: + try: + with urllib.request.urlopen(f"{self.ui_url}web/login", timeout=5) as r: # noqa: S310 + return r.status == 200 + except (URLError, HTTPError, OSError): + return False + + @staticmethod + def _wait(check: Any, wait_s: float, what: str) -> None: + deadline = time.monotonic() + wait_s + while time.monotonic() < deadline: + if check(): + return + time.sleep(5) + raise FixtureError(f"{what} not ready after {wait_s}s") + + # -- accounts -------------------------------------------------------------- + + def _rpc(self, login: str, password: str) -> tuple[int, Any]: + common = xmlrpc.client.ServerProxy(f"{self.ui_url}xmlrpc/2/common") + uid = common.authenticate(DATABASE, login, password, {}) + if not uid: + raise FixtureError(f"Odoo refused the sign-in for {login}") + return uid, xmlrpc.client.ServerProxy(f"{self.ui_url}xmlrpc/2/object") + + def admin(self) -> tuple[int, Any, str]: + """The administrator's XML-RPC session (its default password replaced once).""" + + s = self.secrets() + try: + uid, models = self._rpc("admin", s["admin_password"]) + return uid, models, s["admin_password"] + except FixtureError: + uid, models = self._rpc("admin", "admin") + models.execute_kw( + DATABASE, + uid, + "admin", + "res.users", + "write", + [[uid], {"password": s["admin_password"]}], + ) + uid, models = self._rpc("admin", s["admin_password"]) + return uid, models, s["admin_password"] + + def bootstrap(self) -> None: + """The dedicated user and the record check's read-only role, once.""" + + s = self.secrets() + uid, models, pw = self.admin() + # An internal user who may create and edit contacts ("Contact + # Creation"); without it Odoo shows no save button. + groups = [ + models.execute_kw( + DATABASE, + uid, + pw, + "ir.model.data", + "check_object_reference", + ["base", xmlid], + )[1] + for xmlid in ("group_user", "group_partner_manager") + ] + found = models.execute_kw( + DATABASE, uid, pw, "res.users", "search", [[["login", "=", ACTOR_LOGIN]]] + ) + if found: + models.execute_kw( + DATABASE, + uid, + pw, + "res.users", + "write", + [found, {"groups_id": [[4, g] for g in groups]}], + ) + else: + models.execute_kw( + DATABASE, + uid, + pw, + "res.users", + "create", + [ + { + "name": "OpenAdapt Actor", + "login": ACTOR_LOGIN, + "password": s["actor_password"], + "groups_id": [[6, 0, groups]], + } + ], + ) + exists = self.psql( + f"SELECT count(*) FROM pg_roles WHERE rolname = '{RECORD_ROLE}';" + ) + verb = "ALTER" if exists.strip() == "1" else "CREATE" + self.psql( + f"{verb} ROLE {RECORD_ROLE} WITH LOGIN NOSUPERUSER NOCREATEDB NOCREATEROLE " + f"PASSWORD '{s['record_check_password']}';" + ) + self.psql( + f"ALTER ROLE {RECORD_ROLE} SET default_transaction_read_only = on; " + f"GRANT SELECT ON res_partner TO {RECORD_ROLE};" + ) + + # -- the oracle and the harness's own writes --------------------------------- + + CONTACT_SQL = ( + "SELECT json_build_object('id', id::text, 'name', coalesce(name, ''), " + "'email', coalesce(email, ''), 'phone', coalesce(phone, ''), " + "'street', coalesce(street, ''), 'city', coalesce(city, ''), " + "'zip', coalesce(zip, '')) FROM res_partner " + "WHERE active AND NOT is_company AND type = 'contact' " + "AND id NOT IN (SELECT partner_id FROM res_users) " + "AND id NOT IN (SELECT partner_id FROM res_company) ORDER BY id;" + ) + + def contacts(self) -> list[dict[str, str]]: + """Every live person contact (not a user, not a company): the oracle.""" + + return [ + json.loads(line) + for line in self.psql(self.CONTACT_SQL).splitlines() + if line + ] + + def create_contact(self, fields: dict[str, str]) -> str: + uid, models, pw = self.admin() + return str( + models.execute_kw(DATABASE, uid, pw, "res.partner", "create", [fields]) + ) + + def archive_all_contacts(self) -> int: + """Reset: archive every person contact the benchmark made.""" + + ids = [int(c["id"]) for c in self.contacts()] + if ids: + uid, models, pw = self.admin() + models.execute_kw( + DATABASE, uid, pw, "res.partner", "write", [ids, {"active": False}] + ) + return len(ids) diff --git a/packages/flow/benchmark/odoo_contacts/qualify_hooks.py b/packages/flow/benchmark/odoo_contacts/qualify_hooks.py new file mode 100644 index 000000000..d939944e4 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/qualify_hooks.py @@ -0,0 +1,142 @@ +"""Sign-in and reset for the local Odoo fixture. + +``benchmark/odoo_contacts/verify.yaml`` names these functions. They are +environment setup for a synthetic test copy, not workflow logic: + +- ``open_session`` signs the dedicated user into Odoo and opens a new + contact; +- ``reset`` archives every person contact (the cases create their own); +- ``save_signed_in_state`` writes a Playwright storage state for the + dedicated user: what a user of the registry skill binds as + ``SIGNED_IN_STATE``, since a shipped contract names no Python hooks. + +Environment (optional): ``ODOO_STATE_DIR``. +""" + +from __future__ import annotations + +import atexit +import importlib.util +import os +import sys +from pathlib import Path +from typing import Any, Callable + +_HERE = Path(__file__).resolve().parent + + +def _fixture_module() -> Any: + # Loaded by path: another installed ``benchmark`` package can shadow + # this repository's namespace package. + name = "openadapt_benchmark_odoo_fixture" + if name not in sys.modules: + spec = importlib.util.spec_from_file_location(name, _HERE / "fixture.py") + assert spec is not None and spec.loader is not None + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return sys.modules[name] + + +_fixture: Any = None +_browser: Any = None + + +def fixture() -> Any: + global _fixture + if _fixture is None: + _fixture = _fixture_module().OdooFixture() + return _fixture + + +def record_password() -> str: + """The record check's read-only database password, exported for the verifier.""" + + password = fixture().secrets()["record_check_password"] + os.environ["ODOO_RECORD_PASSWORD"] = password + return password + + +def _shared_browser() -> Any: + global _browser + if _browser is None: + from playwright.sync_api import sync_playwright + + from openadapt_flow._browser_setup import ensure_chromium_installed + + ensure_chromium_installed() + pw = sync_playwright().start() + browser = pw.chromium.launch(headless=True) + _browser = (pw, browser) + + def _stop() -> None: + try: + browser.close() + finally: + pw.stop() + + atexit.register(_stop) + return _browser[1] + + +def sign_in(page: Any) -> None: + fx = fixture() + login = _fixture_module().ACTOR_LOGIN + page.goto(f"{fx.ui_url}web/login", wait_until="load", timeout=60_000) + page.locator("input[name=login]").fill(login) + page.locator("input[name=password]").fill(fx.secrets()["actor_password"]) + page.get_by_role("button", name="Log in").click() + page.wait_for_url(lambda url: "/web/login" not in url, timeout=60_000) + + +def open_form(page: Any) -> None: + fx = fixture() + page.goto(f"{fx.ui_url}odoo/contacts/new", wait_until="load", timeout=60_000) + page.locator(".o_form_view").wait_for(state="visible", timeout=30_000) + + +def open_session(config: Any) -> tuple[Any, Callable[[], None]]: + """A fresh signed-in context at a new contact.""" + + from openadapt_flow.backends.playwright_backend import PlaywrightBackend + + record_password() + width, height = config.app.viewport + context = _shared_browser().new_context( + viewport={"width": width, "height": height}, device_scale_factor=1 + ) + page = context.new_page() + sign_in(page) + open_form(page) + return PlaywrightBackend(page), context.close + + +def save_signed_in_state(path: str) -> str: + """Write the dedicated user's signed-in browser state to ``path``.""" + + context = _shared_browser().new_context() + try: + page = context.new_page() + sign_in(page) + context.storage_state(path=path) + finally: + context.close() + os.chmod(path, 0o600) + return path + + +def reset() -> None: + """Archive every person contact on the test copy.""" + + fixture().archive_all_contacts() + record_password() + + +if __name__ == "__main__": + # python benchmark/odoo_contacts/qualify_hooks.py state PATH | reset + if sys.argv[1:2] == ["state"]: + print(save_signed_in_state(sys.argv[2])) + elif sys.argv[1:2] == ["reset"]: + reset() + else: + raise SystemExit("usage: qualify_hooks.py state PATH | reset") diff --git a/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/qualify-notes.json b/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/qualify-notes.json new file mode 100644 index 000000000..6dff7c625 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/qualify-notes.json @@ -0,0 +1,7 @@ +[ + { + "change": "record_checks_from_verify_yaml", + "effects": 6, + "step": "step_013" + } +] diff --git a/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/qualify.log b/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/qualify.log new file mode 100644 index 000000000..bc781d9de --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/qualify.log @@ -0,0 +1,13 @@ +qualify REFUSED: REFUSE: workflow 'odoo-new-contact' under 'standard' profile vs policy 'clinical-write' + [PASS] Execution profile: standard (production); durability required and enabled; settled-state detection required and enabled + [REFUSE] Certification: bundle is NOT certified under 'clinical-write': 5 policy violation(s) (e.g. prohibit_unarmed_clicks: identity-applicable step is UNARMED — would act with no identity verification (row text outside the target's label is too generic (< 12 squashed chars after volatile-line filtering))) + steps: step_005, step_009, step_013 + [REFUSE] Identity coverage: 2/8 entity-sensitive/consequential action(s) are UNARMED -- would act with no verified target identity + steps: step_005, step_009 + [PASS] Effect coverage: 1/1 consequential write(s) declare the required postcondition and confirmed system-of-record effect contracts + [PASS] Approval fallback: a verifier satisfying the required evidence tier is configured; 1 declared write(s) are covered + [PASS] Interstitial admission: no automatic interstitial actions declared + [WARN] Encrypted bundle: workflow.json plaintext (not required); WARNING: 18 template/screenshot asset(s) are unsealed (plaintext at rest) -- protect via disk encryption or run with --strict-templates to refuse + steps: templates/identifiers/step_003.png, templates/identifiers/step_007.png, templates/identifiers/step_011.png, templates/layout_fingerprint.json, templates/step_000.png, templates/step_000_expect.png, templates/step_001.png, templates/step_001_expect.png, ... (+10 more) + [PASS] Sealed manifest + version pin: integrity-sealed manifest re-verified (digest 3750eaac9ddb9659..., compiler 2.4.0) + -> RUN REFUSED (fail-closed): Certification, Identity coverage. Nothing was executed. diff --git a/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/verify.resolved.json b/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/verify.resolved.json new file mode 100644 index 000000000..27cb460fc --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/results/2026-10-28-click-teach/qualification/verify.resolved.json @@ -0,0 +1,132 @@ +{ + "app": { + "entry_url": null, + "headless": true, + "name": "Odoo 18 Community (local fixture, synthetic contacts)", + "reset": { + "command": null, + "http": null, + "python": "qualify_hooks:reset", + "timeout_s": 600.0 + }, + "session": "qualify_hooks:open_session", + "storage_state": null, + "surface": null, + "target": "web", + "url": "http://localhost:9412/", + "version": "odoo:18.0 (benchmark/odoo_contacts lock)", + "viewport": [ + 1280, + 800 + ] + }, + "campaign": { + "cases": [ + "representative", + "fault-ambiguity", + "fault-wrong-identity", + "fault-stale-identity", + "fault-weak-effect", + "fault-missing-effect" + ], + "id": "odoo-local-new-contact-v1", + "policy": "clinical-write", + "trials": 1 + }, + "effects": null, + "environment_id": "odoo-local-fixture-18.0", + "inputs": { + "file": null, + "params": {}, + "source": "recorded", + "unique": [] + }, + "minimum_effect_strength": "independent_system_of_record", + "minimum_readback": null, + "qualification_id": null, + "record": { + "duplicate_key": null, + "exact_new_set": false, + "expected_count": 1, + "fields": { + "city": "city", + "email": "email", + "name": "name", + "phone": "phone", + "street": "street", + "zip": "zip" + }, + "identity_field": "id", + "match": [ + "email" + ], + "name": "contact", + "timeout_s": 5.0 + }, + "save_step": "step_013", + "schema_version": "openadapt.verify/v1", + "verifier": { + "session": null, + "verifier": { + "access_token": null, + "access_token_env": null, + "auth": null, + "base_url": null, + "candidates": [], + "document_field_paths": {}, + "document_format": "json", + "document_text_pattern": null, + "evidence_keep_fields": null, + "evidence_redact_fields": [], + "field_paths": null, + "file_content_probe": null, + "file_min_size": 1, + "file_mtime_window_s": null, + "file_pattern": "*", + "fresh_session": null, + "glob": "*", + "graphql_freshness_field": null, + "graphql_freshness_window_s": null, + "graphql_query": null, + "graphql_records_path": "data", + "graphql_variables": {}, + "kind": "sql", + "mail_pattern": "*.eml", + "option_params": {}, + "options": {}, + "path_params": {}, + "poll_interval_s": 0.2, + "readback_min_ratio": 0.9, + "readback_region": null, + "records_drop_fields": [], + "records_key": "records", + "records_path": "/api/db", + "resource_type": "Observation", + "root": null, + "search_param_exprs": {}, + "search_params": {}, + "secret_env": {}, + "smart_backend": null, + "sql_connect_args": { + "dbname": "oa", + "host": "127.0.0.1", + "port": 9413, + "user": "oa_record_check" + }, + "sql_driver": "psycopg2", + "sql_password_env": "ODOO_RECORD_PASSWORD", + "sql_query": "SELECT id::text AS id, name, email, phone, street, city, zip FROM res_partner WHERE email = %(email)s AND active", + "sql_query_params": { + "email": { + "literal": null, + "param": "email" + } + }, + "sqlite_database": null, + "surfaces": {}, + "timeout_s": 5.0, + "ui_readback": null, + "verify_tls": true + } + } +} diff --git a/packages/flow/benchmark/odoo_contacts/verify.yaml b/packages/flow/benchmark/odoo_contacts/verify.yaml new file mode 100644 index 000000000..817c844f5 --- /dev/null +++ b/packages/flow/benchmark/odoo_contacts/verify.yaml @@ -0,0 +1,58 @@ +# Qualify the Odoo new-contact workflow on the local fixture. +# +# PYTHONPATH=.:benchmark/odoo_contacts openadapt-flow qualify path/to/bundle \ +# --verify benchmark/odoo_contacts/verify.yaml --out runs/odoo-qual \ +# --certify clinical-write +# +# Synthetic contacts only. The record check reads the contact back from +# Odoo's database through a read-only role (oa_record_check), never the +# screen: one parameterized SELECT of the typed fields, keyed by email. +schema_version: openadapt.verify/v1 +environment_id: odoo-local-fixture-18.0 +minimum_effect_strength: independent_system_of_record + +app: + target: web + name: Odoo 18 Community (local fixture, synthetic contacts) + url: http://localhost:9412/ + version: odoo:18.0 (benchmark/odoo_contacts lock) + # Signs the dedicated user in and opens a new contact; the reset archives + # the contacts the cases created. + session: qualify_hooks:open_session + reset: + python: qualify_hooks:reset + +verifier: + kind: sql + sql_query: >- + SELECT id::text AS id, name, email, phone, street, city, zip + FROM res_partner WHERE email = %(email)s AND active + sql_query_params: + email: {param: email} + sql_driver: psycopg2 + sql_connect_args: {host: 127.0.0.1, port: 9413, user: oa_record_check, dbname: oa} + # Set by the hooks module (qualify_hooks.record_password) from the fixture. + sql_password_env: ODOO_RECORD_PASSWORD + +record: + name: contact + match: [email] + identity_field: id + fields: + name: name + email: email + phone: phone + street: street + city: city + zip: zip + +# The form's save button. +save_step: step_013 + +inputs: + source: recorded + +campaign: + id: odoo-local-new-contact-v1 + trials: 1 + policy: clinical-write diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index f5b482560..88ac68760 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -2179,23 +2179,37 @@ def _build_effect_verifier_unredacted( if not cfg.sql_query: raise ValueError("effects.kind 'sql' requires effects.sql_query") from openadapt_flow.runtime.effects import SqlRecordVerifier + from openadapt_flow.runtime.effects.sql import ( + READ_ONLY_DRIVERS, + read_only_connection, + ) connect: Callable[[], Any] if cfg.sqlite_database: import sqlite3 database = cfg.sqlite_database - connect = lambda: sqlite3.connect(database) # noqa: E731 + connect = lambda: read_only_connection( # noqa: E731 + sqlite3.connect(database), "sqlite3" + ) elif cfg.sql_driver: import importlib - module = importlib.import_module(cfg.sql_driver) + driver = cfg.sql_driver + module = importlib.import_module(driver) connect_args = dict(cfg.sql_connect_args) if cfg.sql_password_env: connect_args["password"] = _require_env( cfg.sql_password_env, "effects.sql_password_env" ) - connect = lambda: module.connect(**connect_args) # noqa: E731 + # The read-back never writes: where the driver can, its session + # is read-only, so a query that would write is refused there. + if driver in READ_ONLY_DRIVERS: + connect = lambda: read_only_connection( # noqa: E731 + module.connect(**connect_args), driver + ) + else: + connect = lambda: module.connect(**connect_args) # noqa: E731 else: raise ValueError( "effects.kind 'sql' requires effects.sqlite_database or " diff --git a/packages/flow/openadapt_flow/runtime/effects/sql.py b/packages/flow/openadapt_flow/runtime/effects/sql.py index 7d89d263d..e4ac99866 100644 --- a/packages/flow/openadapt_flow/runtime/effects/sql.py +++ b/packages/flow/openadapt_flow/runtime/effects/sql.py @@ -27,7 +27,10 @@ verifier under a dedicated READ-ONLY account (no INSERT/UPDATE/DELETE, no EXECUTE on writing functions, no sequence privileges), as ``docs/EFFECT_KIT.md`` instructs. The filter then guards against config -mistakes; the role guards against everything. +mistakes; the role guards against everything. In between, +:func:`read_only_connection` asks the driver itself for a read-only session +(``PRAGMA query_only``, a ``READ ONLY`` transaction), so a write is refused +even when the account could make one. Driver-neutral: the caller supplies a zero-arg ``connect`` callable returning a DB-API 2.0 connection (``sqlite3.connect`` in tests and CI; ``pymysql`` / @@ -88,6 +91,37 @@ _TABLE_NAME = re.compile(r"^[A-Za-z0-9_ ]+$") +#: Drivers :func:`read_only_connection` can make read-only, and how. +READ_ONLY_DRIVERS = frozenset( + {"sqlite3", "psycopg", "psycopg2", "pymysql", "MySQLdb", "mysql.connector"} +) + + +def read_only_connection(conn: Any, driver: str) -> Any: + """``conn`` with the driver's own read-only session switched on. + + Every transaction the read-back opens is then read-only on the server, + whatever the query says. Raises ``ValueError`` for a driver this can't + do for, so a caller that needs it fails closed. + """ + + if driver == "sqlite3": + conn.execute("PRAGMA query_only = ON") + elif driver == "psycopg2": + conn.set_session(readonly=True) + elif driver == "psycopg": + conn.read_only = True + elif driver in ("pymysql", "MySQLdb", "mysql.connector"): + cursor = conn.cursor() + try: + cursor.execute("SET SESSION TRANSACTION READ ONLY") + finally: + cursor.close() + else: + raise ValueError(f"no driver-level read-only session for {driver!r}") + return conn + + def assert_read_only_sql(query: str) -> str: """Validate that ``query`` is a single read-only SELECT; return it. diff --git a/packages/flow/openadapt_flow/skills.py b/packages/flow/openadapt_flow/skills.py index 078ad457c..83d453479 100644 --- a/packages/flow/openadapt_flow/skills.py +++ b/packages/flow/openadapt_flow/skills.py @@ -458,12 +458,103 @@ def fetch_full_contract( #: The read-back kinds a shipped contract may use: each reads a system the #: user's binding names, and none loads a plugin. SHIPPED_VERIFIER_KINDS = ("rest", "fhir", "sql", "graphql") -#: DB-API drivers a shipped SQL check may name (``sql_driver`` imports it). +#: DB-API drivers a shipped SQL check may name (``sql_driver`` imports it): +#: those whose session the read-back can make read-only at the driver. SHIPPED_SQL_DRIVERS = frozenset( - {"psycopg", "psycopg2", "pymysql", "MySQLdb", "mysql.connector", "pymssql"} + {"psycopg", "psycopg2", "pymysql", "MySQLdb", "mysql.connector"} ) +#: Words a shipped read-back query may not contain: anything that writes, +#: changes the schema or session, calls out, or reads the server's files. +_SQL_FORBIDDEN = re.compile( + r"\b(insert|update|delete|merge|upsert|drop|alter|create|grant|revoke|" + r"truncate|copy|call|do|execute|exec|prepare|set|reset|lock|vacuum|analyze|" + r"into|notify|listen|load|import|attach|pragma|replace|dblink\w*|lo_\w+|" + r"pg_\w+|sleep|benchmark|outfile|dumpfile)\b", + re.IGNORECASE, +) +_SQL_PLACEHOLDER = re.compile(r"%\(([A-Za-z_][A-Za-z0-9_]*)\)s") +#: One select-list item: a plain column, optionally qualified, cast to text +#: and renamed. No function call, expression, literal or placeholder. +_SQL_COLUMN = re.compile( + r"(?i)^(?:[a-z_][a-z0-9_]*\.)?[a-z_][a-z0-9_]*" + r"(?:::(?:text|varchar))?(?:\s+as\s+[a-z_][a-z0-9_]*)?$" +) +_SQL_SHAPE = re.compile(r"(?is)^select\s+(.*?)(?:\s+from\s+(.*))?$") +_SQL_CALL = re.compile(r"[A-Za-z0-9_\"]\s*\(") + + +def _sql_shape_problems(text: str) -> list[str]: + """The query's shape: plain columns, from tables, inputs only after WHERE. + + This judges what the query reads, not its words: a select list that + echoes the inputs, or calls a function, would make any save match. + """ + + shape = _SQL_SHAPE.match(text) + if shape is None: + return ["verifier.sql_query must be SELECT FROM "] + columns, rest = shape.groups() + problems = [] + if rest is None: + problems.append("verifier.sql_query must be SELECT FROM
") + rest = "" + for item in columns.split(","): + if not _SQL_COLUMN.match(item.strip()): + problems.append( + f"verifier.sql_query selects {item.strip()!r}: select only plain " + "column names (optionally ::text and AS)" + ) + where = re.search(r"(?i)\bwhere\b", rest) + source = rest[: where.start()] if where else rest + if _SQL_PLACEHOLDER.search(source): + problems.append("verifier.sql_query binds inputs only after WHERE") + if _SQL_CALL.search(source) or "(" in source: + problems.append("verifier.sql_query reads tables, not functions or subqueries") + return problems + + +def shipped_sql_problems(query: Any, params: Any) -> list[str]: + """Why a shipped SQL read-back isn't one parameterized, read-only SELECT.""" + + if not isinstance(query, str) or not query.strip(): + return ["verifier.sql_query must be a SELECT"] + text = query.strip() + if text.endswith(";"): + text = text[:-1].rstrip() + problems = [] + if ";" in text: + problems.append("verifier.sql_query must be a single statement") + if not re.match(r"(?is)^select\s", text): + problems.append("verifier.sql_query must start with SELECT") + if "--" in text or "/*" in text or "#" in text: + problems.append("verifier.sql_query may not carry comments") + if "'" in text or "$$" in text or "${" in text: + problems.append( + "verifier.sql_query may not carry literals or ${} text: bind every " + "value with %(name)s" + ) + forbidden = sorted({m.group(1).lower() for m in _SQL_FORBIDDEN.finditer(text)}) + if forbidden: + problems.append( + f"verifier.sql_query is read-only, and may not use {', '.join(forbidden)}" + ) + problems.extend(_sql_shape_problems(text)) + bound = params if isinstance(params, dict) else {} + for name in _SQL_PLACEHOLDER.findall(text): + if name not in bound: + problems.append(f"verifier.sql_query binds %({name})s, which has no value") + if "%" in _SQL_PLACEHOLDER.sub("", text): + problems.append("verifier.sql_query binds values only as %(name)s") + for name, value in bound.items(): + if not (isinstance(value, dict) and set(value) == {"param"}): + problems.append( + f"verifier.sql_query_params.{name} must be {{param: ...}}, a typed input" + ) + return problems + + def shipped_contract_problems(data: Any, kinds: Mapping[str, str]) -> list[str]: """Why a contract that came with a pulled skill may not run, if it may not. @@ -560,6 +651,12 @@ def walk(node: Any, path: str, keyed: bool = True) -> None: for section in ("verifier", "system_of_record"): node = data.get(section) while isinstance(node, dict): + if "sql_query" in node or node.get("kind") == "sql": + problems.extend( + shipped_sql_problems( + node.get("sql_query"), node.get("sql_query_params") + ) + ) kind = node.get("kind") if kind is not None and kind not in SHIPPED_VERIFIER_KINDS: problems.append( diff --git a/packages/flow/registry/verify-requirements.txt b/packages/flow/registry/verify-requirements.txt index 257b90716..a91dfa973 100644 --- a/packages/flow/registry/verify-requirements.txt +++ b/packages/flow/registry/verify-requirements.txt @@ -1,6 +1,6 @@ # Hash-pinned wheels for the registry release workflow, which checks its own # attestation with openadapt_flow.registry_attest before publishing. Regenerate: -# uv pip compile <(printf "sigstore==4.5.1\npydantic==2.12.5\ncryptography==50.0.0\n") \ +# uv pip compile <(printf "sigstore==4.5.1\npydantic==2.12.5\ncryptography==50.0.0\npyyaml==6.0.3\n") \ # --generate-hashes --python-version 3.12 --python-platform x86_64-manylinux_2_28 annotated-types==0.8.0 \ --hash=sha256:13b2beaad985e05e2d6407ee4c4f35590b11f8d693a258a561055cac8f64cab7 \ @@ -484,6 +484,80 @@ pygments==2.21.0 \ pyjwt==2.15.1 \ --hash=sha256:42d59d631f7768a1028a64c7ff581a9bf7519804daf91fc5b6c56e30eec5e193 \ --hash=sha256:4f259e80cdfb6b3fc18a7de51fd1ef9ec79652f25019bae68975ca2468a34df8 +pyyaml==6.0.3 \ + --hash=sha256:00c4bdeba853cc34e7dd471f16b4114f4162dc03e6b7afcc2128711f0eca823c \ + --hash=sha256:0150219816b6a1fa26fb4699fb7daa9caf09eb1999f3b70fb6e786805e80375a \ + --hash=sha256:02893d100e99e03eda1c8fd5c441d8c60103fd175728e23e431db1b589cf5ab3 \ + --hash=sha256:02ea2dfa234451bbb8772601d7b8e426c2bfa197136796224e50e35a78777956 \ + --hash=sha256:0f29edc409a6392443abf94b9cf89ce99889a1dd5376d94316ae5145dfedd5d6 \ + --hash=sha256:10892704fc220243f5305762e276552a0395f7beb4dbf9b14ec8fd43b57f126c \ + --hash=sha256:16249ee61e95f858e83976573de0f5b2893b3677ba71c9dd36b9cf8be9ac6d65 \ + --hash=sha256:1d37d57ad971609cf3c53ba6a7e365e40660e3be0e5175fa9f2365a379d6095a \ + --hash=sha256:1ebe39cb5fc479422b83de611d14e2c0d3bb2a18bbcb01f229ab3cfbd8fee7a0 \ + --hash=sha256:214ed4befebe12df36bcc8bc2b64b396ca31be9304b8f59e25c11cf94a4c033b \ + --hash=sha256:2283a07e2c21a2aa78d9c4442724ec1eb15f5e42a723b99cb3d822d48f5f7ad1 \ + --hash=sha256:22ba7cfcad58ef3ecddc7ed1db3409af68d023b7f940da23c6c2a1890976eda6 \ + --hash=sha256:27c0abcb4a5dac13684a37f76e701e054692a9b2d3064b70f5e4eb54810553d7 \ + --hash=sha256:28c8d926f98f432f88adc23edf2e6d4921ac26fb084b028c733d01868d19007e \ + --hash=sha256:2e71d11abed7344e42a8849600193d15b6def118602c4c176f748e4583246007 \ + --hash=sha256:34d5fcd24b8445fadc33f9cf348c1047101756fd760b4dacb5c3e99755703310 \ + --hash=sha256:37503bfbfc9d2c40b344d06b2199cf0e96e97957ab1c1b546fd4f87e53e5d3e4 \ + --hash=sha256:3c5677e12444c15717b902a5798264fa7909e41153cdf9ef7ad571b704a63dd9 \ + --hash=sha256:3ff07ec89bae51176c0549bc4c63aa6202991da2d9a6129d7aef7f1407d3f295 \ + --hash=sha256:41715c910c881bc081f1e8872880d3c650acf13dfa8214bad49ed4cede7c34ea \ + --hash=sha256:418cf3f2111bc80e0933b2cd8cd04f286338bb88bdc7bc8e6dd775ebde60b5e0 \ + --hash=sha256:44edc647873928551a01e7a563d7452ccdebee747728c1080d881d68af7b997e \ + --hash=sha256:4a2e8cebe2ff6ab7d1050ecd59c25d4c8bd7e6f400f5f82b96557ac0abafd0ac \ + --hash=sha256:4ad1906908f2f5ae4e5a8ddfce73c320c2a1429ec52eafd27138b7f1cbe341c9 \ + --hash=sha256:501a031947e3a9025ed4405a168e6ef5ae3126c59f90ce0cd6f2bfc477be31b7 \ + --hash=sha256:5190d403f121660ce8d1d2c1bb2ef1bd05b5f68533fc5c2ea899bd15f4399b35 \ + --hash=sha256:5498cd1645aa724a7c71c8f378eb29ebe23da2fc0d7a08071d89469bf1d2defb \ + --hash=sha256:5cf4e27da7e3fbed4d6c3d8e797387aaad68102272f8f9752883bc32d61cb87b \ + --hash=sha256:5e0b74767e5f8c593e8c9b5912019159ed0533c70051e9cce3e8b6aa699fcd69 \ + --hash=sha256:5ed875a24292240029e4483f9d4a4b8a1ae08843b9c54f43fcc11e404532a8a5 \ + --hash=sha256:5fcd34e47f6e0b794d17de1b4ff496c00986e1c83f7ab2fb8fcfe9616ff7477b \ + --hash=sha256:5fdec68f91a0c6739b380c83b951e2c72ac0197ace422360e6d5a959d8d97b2c \ + --hash=sha256:6344df0d5755a2c9a276d4473ae6b90647e216ab4757f8426893b5dd2ac3f369 \ + --hash=sha256:64386e5e707d03a7e172c0701abfb7e10f0fb753ee1d773128192742712a98fd \ + --hash=sha256:652cb6edd41e718550aad172851962662ff2681490a8a711af6a4d288dd96824 \ + --hash=sha256:66291b10affd76d76f54fad28e22e51719ef9ba22b29e1d7d03d6777a9174198 \ + --hash=sha256:66e1674c3ef6f541c35191caae2d429b967b99e02040f5ba928632d9a7f0f065 \ + --hash=sha256:6adc77889b628398debc7b65c073bcb99c4a0237b248cacaf3fe8a557563ef6c \ + --hash=sha256:79005a0d97d5ddabfeeea4cf676af11e647e41d81c9a7722a193022accdb6b7c \ + --hash=sha256:7c6610def4f163542a622a73fb39f534f8c101d690126992300bf3207eab9764 \ + --hash=sha256:7f047e29dcae44602496db43be01ad42fc6f1cc0d8cd6c83d342306c32270196 \ + --hash=sha256:8098f252adfa6c80ab48096053f512f2321f0b998f98150cea9bd23d83e1467b \ + --hash=sha256:850774a7879607d3a6f50d36d04f00ee69e7fc816450e5f7e58d7f17f1ae5c00 \ + --hash=sha256:8d1fab6bb153a416f9aeb4b8763bc0f22a5586065f86f7664fc23339fc1c1fac \ + --hash=sha256:8da9669d359f02c0b91ccc01cac4a67f16afec0dac22c2ad09f46bee0697eba8 \ + --hash=sha256:8dc52c23056b9ddd46818a57b78404882310fb473d63f17b07d5c40421e47f8e \ + --hash=sha256:9149cad251584d5fb4981be1ecde53a1ca46c891a79788c0df828d2f166bda28 \ + --hash=sha256:93dda82c9c22deb0a405ea4dc5f2d0cda384168e466364dec6255b293923b2f3 \ + --hash=sha256:96b533f0e99f6579b3d4d4995707cf36df9100d67e0c8303a0c55b27b5f99bc5 \ + --hash=sha256:9c57bb8c96f6d1808c030b1687b9b5fb476abaa47f0db9c0101f5e9f394e97f4 \ + --hash=sha256:9c7708761fccb9397fe64bbc0395abcae8c4bf7b0eac081e12b809bf47700d0b \ + --hash=sha256:9f3bfb4965eb874431221a3ff3fdcddc7e74e3b07799e0e84ca4a0f867d449bf \ + --hash=sha256:a33284e20b78bd4a18c8c2282d549d10bc8408a2a7ff57653c0cf0b9be0afce5 \ + --hash=sha256:a80cb027f6b349846a3bf6d73b5e95e782175e52f22108cfa17876aaeff93702 \ + --hash=sha256:b30236e45cf30d2b8e7b3e85881719e98507abed1011bf463a8fa23e9c3e98a8 \ + --hash=sha256:b3bc83488de33889877a0f2543ade9f70c67d66d9ebb4ac959502e12de895788 \ + --hash=sha256:b865addae83924361678b652338317d1bd7e79b1f4596f96b96c77a5a34b34da \ + --hash=sha256:b8bb0864c5a28024fac8a632c443c87c5aa6f215c0b126c449ae1a150412f31d \ + --hash=sha256:ba1cc08a7ccde2d2ec775841541641e4548226580ab850948cbfda66a1befcdc \ + --hash=sha256:bdb2c67c6c1390b63c6ff89f210c8fd09d9a1217a465701eac7316313c915e4c \ + --hash=sha256:c1ff362665ae507275af2853520967820d9124984e0f7466736aea23d8611fba \ + --hash=sha256:c2514fceb77bc5e7a2f7adfaa1feb2fb311607c9cb518dbc378688ec73d8292f \ + --hash=sha256:c3355370a2c156cffb25e876646f149d5d68f5e0a3ce86a5084dd0b64a994917 \ + --hash=sha256:c458b6d084f9b935061bc36216e8a69a7e293a2f1e68bf956dcd9e6cbcd143f5 \ + --hash=sha256:d0eae10f8159e8fdad514efdc92d74fd8d682c933a6dd088030f3834bc8e6b26 \ + --hash=sha256:d76623373421df22fb4cf8817020cbb7ef15c725b9d5e45f17e189bfc384190f \ + --hash=sha256:ebc55a14a21cb14062aa4162f906cd962b28e2e9ea38f9b4391244cd8de4ae0b \ + --hash=sha256:eda16858a3cab07b80edaf74336ece1f986ba330fdb8ee0d6c0d68fe82bc96be \ + --hash=sha256:ee2922902c45ae8ccada2c5b501ab86c36525b883eff4255313a253a3160861c \ + --hash=sha256:efd7b85f94a6f21e4932043973a7ba2613b059c4a000551892ac9f1d11f5baf3 \ + --hash=sha256:f7057c9a337546edc7973c0d3ba84ddcdf0daa14533c2065749c9075001090e6 \ + --hash=sha256:fa160448684b4e94d80416c0fa4aac48967a969efe22931448d853ada8baf926 \ + --hash=sha256:fc09d0aa354569bc501d4e787133afc08552722d3ab34836a80547331bb5d4a0 requests==2.34.2 \ --hash=sha256:2a0d60c172f83ac6ab31e4554906c0f3b3588d37b5cb939b1c061f4907e278e0 \ --hash=sha256:f288924cae4e29463698d6d60bc6a4da69c89185ad1e0bcc4104f584e960b9ed diff --git a/packages/flow/scripts/build_registry_index.py b/packages/flow/scripts/build_registry_index.py index b80520f93..7d3e47fd2 100644 --- a/packages/flow/scripts/build_registry_index.py +++ b/packages/flow/scripts/build_registry_index.py @@ -52,9 +52,36 @@ def build(archives: Optional[Path] = None, asset_base: Optional[str] = None) -> } sources["schema_version"] = 1 validate_index(sources) + for item in sources["workflows"]: + check_shipped_contract(item) return sources +def check_shipped_contract(item: dict) -> None: + """Refuse a bundle whose shipped record check could run code or write. + + The same rules ``skill qualify`` applies on the user's machine + (``skills.shipped_contract_problems``): every address, credential name, + path and command is a declared variable, and a SQL read-back is one + parameterized, read-only SELECT. + """ + + if not item.get("contract"): + return + import yaml + + from openadapt_flow.skills import shipped_contract_problems + + path = ROOT / "registry" / item["path"] / item["contract"] + kinds = {v["name"]: v["kind"] for v in item.get("contract_vars") or []} + problems = shipped_contract_problems(yaml.safe_load(path.read_text()), kinds) + if problems: + raise SystemExit( + f"{item['id']}: its shipped record check may not ship:\n " + + "\n ".join(problems) + ) + + def main() -> int: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) parser.add_argument("--check", action="store_true") diff --git a/packages/flow/tests/test_shipped_sql.py b/packages/flow/tests/test_shipped_sql.py new file mode 100644 index 000000000..57d6cda84 --- /dev/null +++ b/packages/flow/tests/test_shipped_sql.py @@ -0,0 +1,245 @@ +"""A skill's shipped SQL read-back is one parameterized, read-only SELECT. + +A public registry bundle may ship a minimal SQL record check (the typed +fields, keyed by the identity field, on the app's documented schema). The +rule (``skills.shipped_sql_problems``) holds at ``skill qualify`` and when the +index is built, so a bundle whose query could write, call out or splice text +never ships. +""" + +from __future__ import annotations + +import json +import shutil +from pathlib import Path + +import pytest +import yaml + +from openadapt_flow import skills + +ROOT = Path(__file__).resolve().parents[1] + +ODOO_SELECT = ( + "SELECT id::text AS id, name, email, phone, street, city, zip " + "FROM res_partner WHERE email = %(email)s AND active" +) +PARAMS = {"email": {"param": "email"}} + + +def test_the_minimal_select_passes(): + assert skills.shipped_sql_problems(ODOO_SELECT, PARAMS) == [] + assert skills.shipped_sql_problems(ODOO_SELECT + ";", PARAMS) == [] + + +@pytest.mark.parametrize( + "query, message", + [ + (ODOO_SELECT + "; DELETE FROM res_partner", "single statement"), + ( + "UPDATE res_partner SET active = false WHERE email = %(email)s", + "start with SELECT", + ), + ("SELECT * INTO stolen FROM res_partner WHERE email = %(email)s", "into"), + ("SELECT pg_read_file('/etc/passwd')", "literals"), + ("SELECT pg_sleep(9) FROM res_partner WHERE email = %(email)s", "pg_sleep"), + ("SELECT name FROM res_partner WHERE email = '" + "x@y.z'", "literals"), + ( + "SELECT name FROM res_partner WHERE email = ${EMAIL}", + "literals or \\$\\{\\}", + ), + ("SELECT name FROM res_partner -- WHERE email = %(email)s", "comments"), + ("SELECT name FROM res_partner WHERE email = %(other)s", "has no value"), + ("SELECT name FROM res_partner WHERE email = %s", r"only as %\(name\)s"), + ("SELECT name FROM res_partner FOR UPDATE", "update"), + ], +) +def test_a_query_that_is_not_one_read_only_select_is_refused(query, message): + problems = skills.shipped_sql_problems(query, PARAMS) + assert any(__import__("re").search(message, p) for p in problems), problems + + +ECHO_PARAMS = {"email": {"param": "email"}, "name": {"param": "name"}} + + +@pytest.mark.parametrize( + "query, message", + [ + # Writes a sequence: setval and nextval are not the word "set". + ( + "SELECT setval(%(name)s::regclass, 1) AS id, " + "nextval(%(email)s::regclass) AS email", + "plain column names", + ), + # Changes a session setting. + ("SELECT set_config(%(name)s, %(email)s, false) AS id", "plain column names"), + # Runs SQL taken from a typed input. + ( + "SELECT query_to_xml(%(name)s, true, true, %(email)s)::text AS id", + "plain column names", + ), + # Echoes the inputs as the stored fields: any write would match. + ( + "SELECT id::text AS id, %(name)s AS name, %(email)s AS email " + "FROM res_partner WHERE id = (SELECT max(id) FROM res_partner)", + "plain column names", + ), + ( + "SELECT name FROM res_partner, query_to_xml(%(name)s, true, true, " + "%(email)s) WHERE email = %(email)s", + "only after WHERE", + ), + ( + "SELECT name FROM lower(%(name)s) WHERE email = %(email)s", + "not functions", + ), + ("SELECT name, email", "SELECT FROM
"), + ], +) +def test_a_query_whose_effect_is_not_a_plain_read_is_refused(query, message): + problems = skills.shipped_sql_problems(query, ECHO_PARAMS) + assert any(__import__("re").search(message, p) for p in problems), problems + + +def test_the_read_back_runs_on_a_read_only_connection(tmp_path): + """Whatever the query says, the driver refuses a write from the read-back.""" + + import sqlite3 + + from openadapt_flow.deployment import EffectsConfig, build_effect_verifier + + db = tmp_path / "sor.db" + conn = sqlite3.connect(db) + conn.execute("CREATE TABLE t (id INTEGER PRIMARY KEY, who TEXT)") + conn.commit() + conn.close() + verifier = build_effect_verifier( + EffectsConfig( + kind="sql", sql_query="SELECT id, who FROM t", sqlite_database=str(db) + ) + ) + read_back = verifier._connect() + with pytest.raises(sqlite3.OperationalError, match="readonly"): + read_back.execute("INSERT INTO t (who) VALUES ('x')") + read_back.close() + + +def test_every_shipped_driver_has_a_read_only_session(): + from openadapt_flow.runtime.effects.sql import ( + READ_ONLY_DRIVERS, + read_only_connection, + ) + + assert skills.SHIPPED_SQL_DRIVERS <= READ_ONLY_DRIVERS + with pytest.raises(ValueError, match="no driver-level read-only"): + read_only_connection(object(), "pymssql") + + +@pytest.mark.parametrize( + "driver, expected", + [ + ("psycopg2", [("set_session", {"readonly": True})]), + ("pymysql", [("execute", "SET SESSION TRANSACTION READ ONLY")]), + ], +) +def test_the_driver_is_asked_for_a_read_only_session(driver, expected): + from openadapt_flow.runtime.effects.sql import read_only_connection + + calls = [] + + class Cursor: + def execute(self, sql): + calls.append(("execute", sql)) + + def close(self): + pass + + class Conn: + def set_session(self, **kw): + calls.append(("set_session", kw)) + + def cursor(self): + return Cursor() + + read_only_connection(Conn(), driver) + assert calls == expected + + +def test_a_bound_value_must_be_a_typed_input(): + problems = skills.shipped_sql_problems(ODOO_SELECT, {"email": "x@y.z"}) + assert any("must be {param: ...}" in p for p in problems) + + +def _odoo_contract(query: str = ODOO_SELECT) -> dict: + return { + "schema_version": "openadapt.verify/v1", + "app": { + "target": "web", + "name": "Odoo", + "version": "18.0", + "url": "${APP_URL}", + "storage_state": "${SIGNED_IN_STATE}", + "reset": {"command": "${RESET_COMMAND}"}, + }, + "verifier": { + "kind": "sql", + "sql_query": query, + "sql_query_params": PARAMS, + "sql_driver": "psycopg2", + "sql_connect_args": { + "host": "${DB_HOST}", + "port": "${DB_PORT}", + "user": "${DB_USER}", + "dbname": "${DB_NAME}", + }, + "sql_password_env": "${DB_PASSWORD_VAR}", + }, + "record": {"name": "contact", "match": ["email"], "fields": {"email": "email"}}, + } + + +KINDS = { + "APP_URL": "url", + "SIGNED_IN_STATE": "path", + "RESET_COMMAND": "command", + "DB_HOST": "url", + "DB_PORT": "command", + "DB_USER": "command", + "DB_NAME": "command", + "DB_PASSWORD_VAR": "env_var_name", +} + + +def test_a_shipped_sql_contract_is_checked_as_a_whole(): + assert skills.shipped_contract_problems(_odoo_contract(), KINDS) == [] + bad = _odoo_contract(ODOO_SELECT + "; DROP TABLE res_partner") + assert any( + "single statement" in p for p in skills.shipped_contract_problems(bad, KINDS) + ) + + +def test_the_index_build_refuses_a_bundle_whose_sql_could_write(tmp_path, monkeypatch): + import runpy + + root = tmp_path / "flow" + shutil.copytree(ROOT / "registry", root / "registry") + bundle = root / "registry/bundles/openemr/new-referral" + (bundle / "verify.yaml").write_text( + yaml.safe_dump( + _odoo_contract("DELETE FROM res_partner WHERE email = %(email)s") + ) + ) + sources = json.loads((root / "registry/sources.json").read_text()) + for item in sources["workflows"]: + if item["id"] == "openemr/new-referral": + item.update( + contract="verify.yaml", + contract_vars=[ + {"name": name, "kind": kind} for name, kind in KINDS.items() + ], + ) + (root / "registry/sources.json").write_text(json.dumps(sources)) + script = runpy.run_path(str(ROOT / "scripts/build_registry_index.py")) + monkeypatch.setitem(script["build"].__globals__, "ROOT", root) + with pytest.raises(SystemExit, match="start with SELECT"): + script["build"]()