From f03800cbe97d178b3c14e6fd4500a175df322888 Mon Sep 17 00:00:00 2001 From: Richard Abrich <774615+abrichr@users.noreply.github.com> Date: Sat, 17 Oct 2026 22:38:54 -0400 Subject: [PATCH 1/5] feat(flow): person-signed approver grants, no self-approval, one-tap phone approval - An agent, rule or system approves saves only under an approver grant a person signed with a policy_signers key (reusing the JIT authority's Ed25519 registry). An unsigned, tampered, expired or agent-signed grant means no delegate approval; the run falls back to a person. - Nobody approves their own work: each run records who started it, and a delegate can't answer a run it (or whoever it acts for) started, or a run whose starter is unknown. - One-tap phone approval (opt-in): waiting saves go to Cloud over the runner's own connection; a person approves from their phone after an AAL2 sign-in. The run accepts only a Cloud-signed decision for the exact request digest from a pinned key, writes it once, and re-checks the form before the save. Desktop keeps working and wins if it answers first. - item-approval show and the report name who decided: a person (Desktop or phone) or a named delegate with its grant hash. --- .../flow/docs/APPROVAL_AND_VERIFICATION.md | 110 +++- packages/flow/docs/APPROVAL_MODES.md | 23 +- packages/flow/openadapt_flow/__main__.py | 323 +++++++++++- packages/flow/openadapt_flow/agent/cli.py | 10 +- .../flow/openadapt_flow/agent/delegated.py | 17 +- packages/flow/openadapt_flow/agent/runner.py | 7 + .../approval_mode/cloud_decision.py | 195 +++++++ .../flow/openadapt_flow/approval_mode/gate.py | 5 + .../approval_mode/item_approval.py | 480 ++++++++++++++++- .../openadapt_flow/approval_mode/model.py | 15 +- .../approval_mode/phone_relay.py | 172 ++++++ .../approval_mode/signed_policy.py | 307 +++++++++++ .../flow/openadapt_flow/contracts/approval.py | 1 + .../contracts/schemas/approval-record-v1.json | 1 + packages/flow/openadapt_flow/deployment.py | 4 + .../flow/tests/_approver_grant_support.py | 92 ++++ .../flow/tests/test_approval_delegated.py | 38 +- .../flow/tests/test_approval_signed_grant.py | 495 ++++++++++++++++++ 18 files changed, 2192 insertions(+), 103 deletions(-) create mode 100644 packages/flow/openadapt_flow/approval_mode/cloud_decision.py create mode 100644 packages/flow/openadapt_flow/approval_mode/phone_relay.py create mode 100644 packages/flow/openadapt_flow/approval_mode/signed_policy.py create mode 100644 packages/flow/tests/_approver_grant_support.py create mode 100644 packages/flow/tests/test_approval_signed_grant.py diff --git a/packages/flow/docs/APPROVAL_AND_VERIFICATION.md b/packages/flow/docs/APPROVAL_AND_VERIFICATION.md index 9ba3f3385..378bce4f9 100644 --- a/packages/flow/docs/APPROVAL_AND_VERIFICATION.md +++ b/packages/flow/docs/APPROVAL_AND_VERIFICATION.md @@ -43,9 +43,72 @@ non-person only when the policy pins the customer's opt-in digest. Under the `clinical-write` and `regulated` policies every save is a clinical write. -The run writes its policy once into `item-approval/approver-policy.json`. -Every reader (the gate, the CLI, a resume) applies that file. An unreadable -policy means person-only. +### A person signs the grant + +A policy on its own is unsigned JSON, so it never lets an agent or system +answer. A delegate answers only under an **approver grant** +(`openadapt-flow/approver-grant/v1`) that a person signed, with the same +signing as program admission (`openadapt_flow.jit.authority`): + +```json +{ + "grant": { + "schema": "openadapt-flow/approver-grant/v1", + "grant_id": "intake-reviewer-2026", + "signer": "person-0001", + "signed_at": "2026-10-17T12:00:00Z", + "expires_at": "2026-11-16T12:00:00Z", + "approver_policy": { + "allowed": { + "read_only": ["person"], + "reversible": ["person"], + "consequential": ["person", "agent"], + "irreversible": ["person"], + "clinical_write": ["person"] + } + }, + "delegates": ["agent:approver-bot"], + "workflows": ["referral-intake"] + }, + "signer_public_key": "", + "signature": "" +} +``` + +- The signer's key is registered as a `policy_signer` in `principals.json` + in the key directory (`$OPENADAPT_JIT_KEY_DIR`, default + `~/.openadapt/jit`): `openadapt-flow jit keygen`, then + `openadapt-flow jit register --role policy_signer`. A key registered as an + agent can't also sign, and a signer name that names an agent or model is + refused. Keep the key directory away from the agents it governs. +- `openadapt-flow approver-grant sign GRANT.json --key KEY --out SIGNED.json` + signs. It needs a person at an interactive terminal and a typed `yes`, and + the key is always encrypted. `approver-grant verify SIGNED.json` checks one. +- `openadapt-flow run --approver-policy SIGNED.json` uses it. The grant + lasts at most 366 days, names its delegates (any other id is refused), + and can be limited to named workflows. The person who signs can't be one + of its delegates, and a grant that lets only a person answer is refused. +- An unsigned policy that admits a delegate, a grant whose signature, signer + or key doesn't verify, or one that expired gives no authority: the run is + person-only and prints why (`approver_policy_unsigned`, + `approver_grant_signature_invalid`, `approver_grant_expired`, ...). + +The run writes the grant and its policy once into `item-approval/` +(`approver-grant.json`, `approver-policy.json`). Every reader (the gate, the +CLI, a resume) re-verifies the grant against the registry each time it reads +an answer, so a grant that expired mid-run, or a file swapped in the run +directory, stops working. An unreadable file means person-only. + +### Nobody approves their own work + +The run records who started it, once, in `item-approval/run-initiator.json` +(`run --started-by ID`; openadapt-agent passes the calling agent's id). A +delegate can't answer a run it started, or answer on behalf of whoever +started it (`self_approval`). A run an agent started without an id +(`agent:unidentified`) takes no delegate answers at all, and neither does a +run that never recorded its initiator. A file planted before the run with +another value refuses the run. There is no tool or flag through which an +agent changes the approval mode, signs a grant, or names itself a delegate. A delegated approver answers by one of three routes: @@ -59,11 +122,41 @@ A delegated approver answers by one of three routes: say `approve`, `decline` with a closed reason, or `pending`. Any failure leaves the save to a person; a person can always answer in Desktop. -The run re-checks every answer: the policy must allow the approver's kind -for the step class, a person answers only from Desktop, and a delegate -never fills in the person fields. The report's `ItemApprovalRef` records -`decided_by: automation`, the route and the approver. Only a person's -decision becomes a training label. Triage never approves anything. +The run re-checks every answer: the grant must verify, name the delegate +and cover the workflow, its policy must allow the approver's kind for the +step class, the delegate can't have started the run, a person answers only +from Desktop or their phone (below), and a delegate never fills in the +person fields. The report's `ItemApprovalRef` records `decided_by: +automation`, the route and the approver, whose `policy_ref` is the grant's +digest. `item-approval show` prints each request's `decided_by`: a person +(Cloud user id and route) or the delegate's id and the grant digest, which +Desktop and Cloud show on the run. Only a person's decision becomes a +training label. Triage never approves anything. + +### A person approves from their phone + +Phone approval is opt-in (`runtime.phone_approval: true` or +`run --phone-approval`), and local-only installs never use it. An owner or +admin also turns on **Approve saves from your phone** on Cloud's Needs +attention page; until then Cloud refuses the save and it waits in Desktop as +before. With both on, +each waiting save also goes to Cloud through the runner's own outbound +connection. Cloud pushes a notification with an opaque id and fixed words +(never a value, a name or the workflow's name) and, if the person chose, +an email with a single-use link. The link opens Cloud's approve page; after +sign-in with AAL2 it shows exactly the filled form Desktop would show, and +**Approve and save** or **Don't save**. + +Cloud signs the decision (`openadapt.cloud-item-decision/v1`) for that exact +request digest, which binds the run, the save step, the screenshot and the +form's field state. The run accepts it only when it verifies against the +decision key pinned on this runner (`item-approval pin-cloud-keys`), the +approver signed in with AAL2, and nothing else answered first. The answer is +written once and used for exactly one save, so a replayed link or decision +has nothing to answer. Then the run re-checks the form before the save, as +for a Desktop approval, and stops if anything changed while it waited. A +Desktop answer that lands first wins, and the run withdraws the phone link. + ## How a save is checked @@ -125,7 +218,6 @@ with a Desktop key kept in the keychain would close that later. ## Not built -- Desktop and Cloud don't show delegated approvers yet. - Eligibility isn't yet one of the `problems_only` requirements; a person reads it before turning the switch on. - The Desktop admission path above. diff --git a/packages/flow/docs/APPROVAL_MODES.md b/packages/flow/docs/APPROVAL_MODES.md index a9c1e2edf..7f4e864d5 100644 --- a/packages/flow/docs/APPROVAL_MODES.md +++ b/packages/flow/docs/APPROVAL_MODES.md @@ -49,10 +49,12 @@ openadapt-flow item-approval approve RUN_DIR \ openadapt-flow item-approval decline RUN_DIR ... --reason wrong_values ``` -- An answer names the request digest the person saw. Only - `--decided-by human` from the `desktop_run_it` route is accepted. A - decline carries one closed reason: `wrong_values`, `wrong_record` or - `screen_problem`. +- An answer names the request digest the person saw. A person answers with + `--decided-by human` from the `desktop_run_it` route, or from their phone + through Cloud (`cloud_phone`, opt-in; see + [`APPROVAL_AND_VERIFICATION.md`](APPROVAL_AND_VERIFICATION.md)), which + needs a Cloud-signed decision for that exact request. A decline carries + one closed reason: `wrong_values`, `wrong_record` or `screen_problem`. - Answers are created once and never replaced. The run checks every answer itself, so an answer file written around the command is held to the same rules. An approval is used for exactly one save; a loop that reaches the @@ -256,10 +258,13 @@ strict expected-failure tests for them. ## Delegated approvers -By default only a person answers a save. A run can carry an approver policy -(`--approver-policy`) that lets an agent, rule or system answer some step -classes, and a signed webhook (`--approver-webhook`) to ask. See -[`APPROVAL_AND_VERIFICATION.md`](APPROVAL_AND_VERIFICATION.md). +By default only a person answers a save. A run can carry an approver grant +a person signed (`--approver-policy SIGNED.json`) that lets named agents or +systems answer some step classes, and a signed webhook +(`--approver-webhook`) to ask. An unsigned or tampered grant gives no +authority, and a delegate never answers a run it started. Desktop and Cloud +show who approved each save: a person, or the delegate's name and the +grant's digest. See [`APPROVAL_AND_VERIFICATION.md`](APPROVAL_AND_VERIFICATION.md). ## Not built yet @@ -268,5 +273,3 @@ classes, and a signed webhook (`--approver-webhook`) to ask. See - A Desktop list queue for batch review. [`DECISION_TRIAGE.md`](DECISION_TRIAGE.md) describes the engine part that orders a list's verified saves for review. -- Delegated approvers in Desktop and Cloud. Flow records them; neither app - shows who or what approved yet. diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index c17b63d05..dd37ccb0e 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -2054,7 +2054,10 @@ def _cmd_replay(args: argparse.Namespace) -> int: # NO actuation has begun on either the desktop or the web path below. _maybe_attest_pre_state(run_dir, args) approval_gate = _approval_gate_for_run( - args, run_dir, policy_name=getattr(args, "policy", None) or cfg.policy.policy + args, + run_dir, + policy_name=getattr(args, "policy", None) or cfg.policy.policy, + cfg=cfg, ) fault_driver = getattr(args, "_qualification_fault_driver", None) if _normalize_kind(backend_cfg.kind) != "web": @@ -2292,22 +2295,38 @@ def _approval_wait_seconds(args: argparse.Namespace) -> float: def _approver_settings(args: argparse.Namespace): - """The run's approver policy and delegated approver, from its flags.""" + """The run's approver grant and delegated approver, from its flags. + + ``--approver-policy`` is a person-signed grant + (``openadapt-flow approver-grant sign``) or a person-only policy. An + unsigned policy that admits an agent, or a grant that doesn't verify + against the signer registry, gives no authority: the run stays + person-only and says why. + """ import os + import sys from openadapt_flow.approval_mode.approver import ( ApproverAuth, ApproverIdentityV1, ApproverKind, - ApproverPolicyV1, WebhookApprover, ) + from openadapt_flow.approval_mode.signed_policy import load_approver_policy + from openadapt_flow.jit.lifecycle import default_key_dir - policy = None + loaded = None path = getattr(args, "approver_policy", None) if path: - policy = ApproverPolicyV1.model_validate_json(Path(path).read_text()) + loaded = load_approver_policy(path, default_key_dir()) + if loaded.refusal is not None: + print( + f"approver policy not used ({loaded.refusal}): only a person " + "answers this run's saves. A delegate needs a grant a " + "registered person signed (openadapt-flow approver-grant sign).", + file=sys.stderr, + ) approver = None url = getattr(args, "approver_webhook", None) if url: @@ -2321,11 +2340,75 @@ def _approver_settings(args: argparse.Namespace): id=getattr(args, "approver_webhook_id", None), ), ) - return policy, approver + return loaded, approver + + +def _phone_relay_for_run(args: argparse.Namespace, cfg=None): + """The opt-in phone approval relay, or ``None`` (Desktop approval only). + + On only with ``--phone-approval`` or ``runtime.phone_approval: true``. + Asking for it without this runner's Cloud pairing, or without a pinned + Cloud decision key, refuses the run rather than silently dropping it. + """ + + wanted = bool(getattr(args, "phone_approval", False)) or bool( + getattr(getattr(cfg, "runtime", None), "phone_approval", False) + ) + if not wanted: + return None + from openadapt_flow.approval_mode.cloud_decision import ( + default_keys_path, + load_pinned_keys, + ) + from openadapt_flow.approval_mode.phone_relay import ( + PhoneRelayError, + relay_from_env, + ) + + if not load_pinned_keys(): + raise SystemExit( + "run REFUSED: phone approval needs Cloud's decision key pinned on " + f"this runner ({default_keys_path()}); run " + "`openadapt-flow item-approval pin-cloud-keys`. Nothing was executed." + ) + try: + return relay_from_env() + except PhoneRelayError as exc: + raise SystemExit(f"run REFUSED: {exc}. Nothing was executed.") from None + + +def _prepare_approval_store( + args: argparse.Namespace, run_dir: Path, *, record_initiator: bool +): + """The run's item-approval store with its grant and initiator recorded.""" + + from openadapt_flow.approval_mode.item_approval import ( + ItemApprovalRefused, + ItemApprovalStore, + ) + + store = ItemApprovalStore(run_dir) + loaded, approver = _approver_settings(args) + try: + if loaded is not None and loaded.signed is not None: + store.write_grant(loaded.signed) + elif loaded is not None and loaded.refusal is None: + store.write_policy(loaded.policy) + if record_initiator: + store.write_initiator(getattr(args, "started_by", None)) + except ItemApprovalRefused as exc: + raise SystemExit( + f"run REFUSED: {exc} ({exc.code}). Nothing was executed." + ) from None + return store, approver def _approval_gate_for_run( - args: argparse.Namespace, run_dir: Path, *, policy_name: Optional[str] = None + args: argparse.Namespace, + run_dir: Path, + *, + policy_name: Optional[str] = None, + cfg=None, ): """Build the run's approval gate from the mode ``run`` resolved, if any.""" @@ -2333,16 +2416,15 @@ def _approval_gate_for_run( if mode is None: return None from openadapt_flow.approval_mode.gate import ApprovalGate - from openadapt_flow.approval_mode.item_approval import ItemApprovalStore - policy, approver = _approver_settings(args) + store, approver = _prepare_approval_store(args, run_dir, record_initiator=True) return ApprovalGate( mode, - approval_store=ItemApprovalStore(run_dir), + approval_store=store, approval_wait_s=_approval_wait_seconds(args), - approver_policy=policy, approver=approver, policy_name=policy_name, + phone_relay=_phone_relay_for_run(args, cfg), ) @@ -2970,21 +3052,27 @@ def _carry_item_approval_to_pause(run_dir: Path, store, pending): def _resume_approval_gate( - args: argparse.Namespace, mode, run_dir: Path, *, policy_name: Optional[str] = None + args: argparse.Namespace, + mode, + run_dir: Path, + *, + policy_name: Optional[str] = None, + cfg=None, ): if mode is None: return None from openadapt_flow.approval_mode.gate import ApprovalGate - from openadapt_flow.approval_mode.item_approval import ItemApprovalStore - policy, approver = _approver_settings(args) + # A resume keeps the run's own grant and initiator; it never re-records + # who started the run. + store, approver = _prepare_approval_store(args, run_dir, record_initiator=False) return ApprovalGate( mode, - approval_store=ItemApprovalStore(run_dir), + approval_store=store, approval_wait_s=_approval_wait_seconds(args), - approver_policy=policy, approver=approver, policy_name=policy_name, + phone_relay=_phone_relay_for_run(args, cfg), ) @@ -3260,7 +3348,7 @@ def _resume_with(backend: "Backend") -> "RunReport": runtime_config=cfg.runtime, checkpoint_key=ckpt_key, approval_gate=_resume_approval_gate( - args, resume_mode, run_dir, policy_name=cfg.policy.policy + args, resume_mode, run_dir, policy_name=cfg.policy.policy, cfg=cfg ), ) return resume( @@ -3476,9 +3564,13 @@ def _cmd_item_approval(args: argparse.Namespace) -> int: request_summary, ) + verb = args.item_approval_cmd + if verb == "pin-cloud-keys": + return _item_approval_pin_cloud_keys(args) run_dir = Path(args.run_dir) store = ItemApprovalStore(run_dir) - verb = args.item_approval_cmd + if verb == "apply-cloud-decision": + return _item_approval_apply_cloud_decision(args, store) if verb == "show": rows = [request_summary(store, request) for request in store.requests()] if not args.all: @@ -3568,7 +3660,7 @@ def refuse(code: str, message: str) -> int: else args.auth_method, decline_reason=getattr(args, "reason", None), approver=delegate, - policy=store.policy(), + policy=store.context(), ) store.write_answer(answer) except ItemApprovalRefused as exc: @@ -3588,6 +3680,132 @@ def refuse(code: str, message: str) -> int: return 0 +def _item_approval_pin_cloud_keys(args: argparse.Namespace) -> int: + """Pin Cloud's phone-decision keys on this runner (once, at pairing).""" + import json + import os + + from openadapt_flow.approval_mode.cloud_decision import default_keys_path + from openadapt_flow.approval_mode.phone_relay import ( + PhoneRelayError, + relay_from_env, + ) + + target = Path(args.out) if args.out else default_keys_path() + try: + keys = relay_from_env().decision_keys() + except (PhoneRelayError, OSError, ValueError) as exc: + print(json.dumps({"ok": False, "code": "pin_failed", "message": str(exc)})) + return 2 + target.parent.mkdir(parents=True, exist_ok=True) + tmp = target.with_suffix(".tmp") + fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) + with os.fdopen(fd, "w", encoding="utf-8") as handle: + handle.write(json.dumps(keys, indent=2, sort_keys=True) + "\n") + os.replace(tmp, target) + print(json.dumps({"ok": True, "path": str(target), "key_ids": sorted(keys)})) + return 0 + + +def _item_approval_apply_cloud_decision(args: argparse.Namespace, store) -> int: + """Apply a Cloud-signed phone decision to the request it names.""" + import json + + from openadapt_flow.approval_mode.item_approval import ( + ItemApprovalRefused, + apply_cloud_decision, + ) + + try: + raw = json.loads(Path(args.decision_file).read_text(encoding="utf-8")) + request_id = str(raw.get("request_id", "")) if isinstance(raw, dict) else "" + request = store.read_request(request_id) + except (OSError, ValueError, ItemApprovalRefused): + print(json.dumps({"ok": False, "code": "request_unknown"})) + return 2 + answer = apply_cloud_decision(store, request, raw) + if answer is None: + print(json.dumps({"ok": False, "code": "cloud_decision_refused"})) + return 2 + resume_required = _answer_durable_item_pause(Path(args.run_dir), request, answer) + print( + json.dumps( + { + "ok": True, + "request_id": request.request_id, + "decision": answer.decision, + "answer_sha256": answer.digest(), + "resume_required": resume_required, + } + ) + ) + return 0 + + +def _cmd_approver_grant(args: argparse.Namespace) -> int: + """Sign or check a person's approver grant (who may answer saves).""" + import json + import sys + + from openadapt_flow.approval_mode.signed_policy import ( + ApproverGrant, + GrantRefused, + SignedApproverGrant, + sign_grant, + verify_grant, + ) + from openadapt_flow.jit import authority + from openadapt_flow.jit.lifecycle import default_key_dir + + key_dir = Path(args.key_dir) if args.key_dir else default_key_dir() + if args.approver_grant_cmd == "verify": + try: + signed = SignedApproverGrant.model_validate_json( + Path(args.grant).read_text(encoding="utf-8") + ) + verified = verify_grant(signed, key_dir) + except (OSError, ValueError) as exc: + code = getattr(exc, "code", "approver_grant_unreadable") + print(json.dumps({"ok": False, "code": code, "message": str(exc)})) + return 2 + print( + json.dumps( + { + "ok": True, + "policy_ref": verified.policy_ref, + "signer": verified.signer, + "delegates": list(verified.grant.delegates), + "workflows": list(verified.grant.workflows), + "expires_at": verified.grant.expires_at, + } + ) + ) + return 0 + # sign: a person, at a terminal, with their encrypted policy-signer key. + if not (sys.stdin.isatty() and sys.stdout.isatty()): + print( + "approver-grant sign needs a person at an interactive terminal; " + "an agent or script can't sign a grant.", + file=sys.stderr, + ) + return 2 + grant = ApproverGrant.model_validate_json(Path(args.grant).read_text("utf-8")) + print(grant.model_dump_json(indent=2, by_alias=True)) + reply = input(f"Sign this grant as {grant.signer}? Type yes to sign: ") + if reply.strip().lower() not in ("yes", "ok"): + print("Not signed.") + return 1 + try: + signed = sign_grant(grant, authority.load_encrypted_key(args.key)) + verified = verify_grant(signed, key_dir) + except (authority.AuthorityError, GrantRefused) as exc: + print(f"Not signed: {exc}", file=sys.stderr) + return 2 + signed.save(args.out) + print(f"signed grant {verified.policy_ref} written to {args.out}") + return 0 + + def _answer_durable_item_pause(run_dir: Path, request, answer) -> bool: """Carry an answer to a durable pause the run left after waiting. @@ -6364,10 +6582,33 @@ def _add_approval_flags(p: argparse.ArgumentParser) -> None: default=None, metavar="FILE", help=( - "An openadapt.approver-policy/v1 JSON file: which kinds of approver " - "(person, rule, agent, system) may answer each class of save. " - "Without it only a person may answer. A clinical_write save takes " - "a non-person approver only when the policy pins an opt-in." + "A person-signed approver grant (openadapt-flow approver-grant " + "sign): which agents or systems may answer which classes of save, " + "until when. Without a grant that verifies against the signer " + "registry, only a person may answer; an unsigned or tampered file " + "gives no authority. A clinical_write save takes a non-person " + "approver only when the grant's policy pins an opt-in." + ), + ) + p.add_argument( + "--started-by", + default=None, + metavar="ID", + help=( + "The agent or system that started this run (opaque id). It can " + "never answer this run's saves. openadapt-agent sets it." + ), + ) + p.add_argument( + "--phone-approval", + action="store_true", + default=False, + help=( + "Also send each waiting save to OpenAdapt Cloud so a person can " + "approve it from their phone (opt-in; same as " + "runtime.phone_approval). Needs this runner's Cloud pairing " + "(OPENADAPT_RUNNER_TOKEN, OPENADAPT_RUNNER_ID) and Cloud's " + "decision key pinned (item-approval pin-cloud-keys)." ), ) p.add_argument( @@ -7848,6 +8089,42 @@ def build_parser() -> argparse.ArgumentParser: choices=["wrong_values", "wrong_record", "screen_problem"], ) q.set_defaults(func=_cmd_item_approval) + q = item_sub.add_parser( + "apply-cloud-decision", + help=( + "Apply a Cloud-signed decision a person made on their phone. It " + "must name this run's waiting request and verify against the " + "pinned Cloud decision key" + ), + ) + q.add_argument("run_dir", help="The run directory") + q.add_argument("--decision-file", required=True, metavar="JSON") + q.set_defaults(func=_cmd_item_approval) + q = item_sub.add_parser( + "pin-cloud-keys", + help="Pin OpenAdapt Cloud's phone-decision keys on this runner", + ) + q.add_argument("--out", default=None, help="Where to write the pinned keys") + q.set_defaults(func=_cmd_item_approval) + + p = sub.add_parser( + "approver-grant", + help=( + "Sign or check a grant that lets named agents or systems answer " + "some saves. Only a registered person signs, at a terminal" + ), + ) + grant_sub = p.add_subparsers(dest="approver_grant_cmd", required=True) + q = grant_sub.add_parser("sign", help="Sign a grant with your policy-signer key") + q.add_argument("grant", help="Unsigned openadapt-flow/approver-grant/v1 JSON") + q.add_argument("--key", required=True, help="Your encrypted policy-signer key") + q.add_argument("--out", required=True) + q.add_argument("--key-dir", default=None) + q.set_defaults(func=_cmd_approver_grant) + q = grant_sub.add_parser("verify", help="Check a signed grant against the registry") + q.add_argument("grant", help="Signed grant JSON") + q.add_argument("--key-dir", default=None) + q.set_defaults(func=_cmd_approver_grant) p = sub.add_parser( "bench", help="Replay a bundle N times against MockMed and aggregate" diff --git a/packages/flow/openadapt_flow/agent/cli.py b/packages/flow/openadapt_flow/agent/cli.py index 64ba8cfe1..f9d343caa 100644 --- a/packages/flow/openadapt_flow/agent/cli.py +++ b/packages/flow/openadapt_flow/agent/cli.py @@ -250,10 +250,12 @@ def build_parser() -> argparse.ArgumentParser: "Let the calling agent answer saves that wait for an approval, as " "the opaque id AGENT_ID (for example agent:intake-bot). Adds the " "list_approval_requests and answer_approval_request tools. Flow " - "accepts the agent's answer only where the run's approver policy " - "(openadapt-flow run --approver-policy) allows an agent for that " - "step; by default only a person may answer. Flow records the agent " - "as the approver." + "accepts the agent's answer only under a person-signed approver " + "grant (openadapt-flow run --approver-policy) that names AGENT_ID " + "and allows an agent for that step; by default only a person may " + "answer. Runs this server starts record AGENT_ID as their " + "initiator, so the agent can never approve its own runs. Flow " + "records the agent and the grant's digest as the approver." ), ) p.add_argument( diff --git a/packages/flow/openadapt_flow/agent/delegated.py b/packages/flow/openadapt_flow/agent/delegated.py index 411623fd4..68ec15958 100644 --- a/packages/flow/openadapt_flow/agent/delegated.py +++ b/packages/flow/openadapt_flow/agent/delegated.py @@ -6,9 +6,11 @@ Even then the agent decides nothing on its own authority. Its answer goes to ``openadapt-flow item-approval`` as ``decided_by: automation`` from the -``mcp`` route, naming the agent, and Flow accepts it only where the run's -approver policy allows an agent for that step. The default policy allows -only a person, and a clinical write needs the customer's pinned opt-in. +``mcp`` route, naming the agent, and Flow accepts it only under a +person-signed approver grant that names the agent and allows an agent for +that step. Without one only a person answers, and a clinical write needs the +customer's pinned opt-in. Runs this server starts name the agent as their +initiator, so it can never approve its own run. Flow records the agent as the approver, and its decision never becomes a training label. @@ -71,11 +73,12 @@ ) ANSWER_DESCRIPTION = ( "Approve or decline one waiting save as this agent. The operator allowed " - "it for this server, and Flow accepts it only where the run's approver " - "policy allows an agent for that step (never a clinical write without " - "the customer's opt-in). Flow records you as the approver. Send the " + "it for this server, and Flow accepts it only under a grant a person " + "signed that names you and allows an agent for that step (never a " + "clinical write without the customer's opt-in). You can't answer a run " + "you started. Flow records you and the grant as the approver. Send the " "request_id and request_sha256 from list_approval_requests. A person " - "can still answer in OpenAdapt Desktop first." + "can still answer in OpenAdapt Desktop or on their phone first." ) diff --git a/packages/flow/openadapt_flow/agent/runner.py b/packages/flow/openadapt_flow/agent/runner.py index 4d5f38128..90493ac07 100644 --- a/packages/flow/openadapt_flow/agent/runner.py +++ b/packages/flow/openadapt_flow/agent/runner.py @@ -703,6 +703,13 @@ def _build_command( cmd += ["--policy", self.config.policy] if self.config.approval_mode: cmd += ["--approval-mode", self.config.approval_mode] + # The calling agent started this run, so it can never answer the + # run's saves. Without an operator-given id Flow can't tell agents + # apart, and no delegate may answer at all. + cmd += [ + "--started-by", + self.config.delegate_approver_id or "agent:unidentified", + ] if cloud_execution_id: # Flow fetches Cloud's signed receipt after a VERIFIED outcome and # writes it into report.json (needs OPENADAPT_EXECUTE_API_KEY). diff --git a/packages/flow/openadapt_flow/approval_mode/cloud_decision.py b/packages/flow/openadapt_flow/approval_mode/cloud_decision.py new file mode 100644 index 000000000..287a95cae --- /dev/null +++ b/packages/flow/openadapt_flow/approval_mode/cloud_decision.py @@ -0,0 +1,195 @@ +"""A person's save decision made on their phone, signed by Cloud. + +Phone approval is opt-in. When a deployment turns it on, the runner sends +each waiting save to Cloud; Cloud pushes an opaque link to the approver's +phone, the person signs in (AAL2), sees exactly what will be saved, and taps +Approve or Don't save. Cloud signs that decision with its decision key, and +the runner hands it to the waiting run. + +This module is the run's half. A Cloud decision is accepted only when: + +* it is signed by a key pinned on this runner (key id -> base64 raw Ed25519 + public key in ``$OPENADAPT_CLOUD_DECISION_KEYS``, default + ``$OPENADAPT_HOME/cloud-decision-keys.json``). No pinned key, no phone + route: local-only installs keep Desktop approval and nothing else; +* it names the exact request id and request digest (which binds the run, + the save step, the screenshot and the form's field state) the person saw; +* the approver is a Cloud user who signed in with AAL2. An agent or a model + can never produce one: Cloud signs only for a person's session; +* its ``link_id`` is single-use: the answer file is created once, and the + approval is consumed by exactly one save, so a replayed decision has + nothing to answer. + +After an approval the run still re-reads the form before the save and stops +if anything changed while it waited, as for a Desktop approval. +""" + +from __future__ import annotations + +import base64 +import binascii +import json +import os +from pathlib import Path +from typing import Any, Literal, Mapping, Optional + +from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator + +from openadapt_flow.approval_mode.model import ( + OPAQUE_ID_PATTERN, + PREFIXED_SHA256_PATTERN, + DeclineReason, +) + +CLOUD_DECISION_SCHEMA: Literal["openadapt.cloud-item-decision/v1"] = ( + "openadapt.cloud-item-decision/v1" +) +_DOMAIN = b"openadapt.cloud-item-decision/v1\n" +CLOUD_PHONE_ROUTE: Literal["cloud_phone"] = "cloud_phone" +KEYS_ENV = "OPENADAPT_CLOUD_DECISION_KEYS" + + +def _canonical(value: Any) -> bytes: + return json.dumps( + value, sort_keys=True, separators=(",", ":"), ensure_ascii=False + ).encode("utf-8") + + +class CloudDecisionV1(BaseModel): + """What Cloud signs when a person approves or declines on their phone.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + schema_version: Literal["openadapt.cloud-item-decision/v1"] = CLOUD_DECISION_SCHEMA + request_id: str = Field(pattern=r"^[0-9a-f]{32}$") + request_sha256: str = Field(pattern=PREFIXED_SHA256_PATTERN) + decision: Literal["approve", "decline"] + decline_reason: Optional[DeclineReason] = None + #: The Cloud user id of the person who decided. Never a name. + approver_id: str = Field(pattern=OPAQUE_ID_PATTERN) + auth_method: Literal["cloud_aal2"] + decided_at: str = Field(min_length=10, max_length=64) + #: The single-use link the person opened. + link_id: str = Field(pattern=r"^[A-Za-z0-9_-]{16,64}$") + runner_id: str = Field(pattern=OPAQUE_ID_PATTERN) + key_id: str = Field(pattern=r"^[A-Za-z0-9._-]{1,64}$") + signature: str = Field(min_length=86, max_length=100) + + @model_validator(mode="after") + def _reason(self) -> "CloudDecisionV1": + if (self.decision == "decline") != (self.decline_reason is not None): + raise ValueError("a decline carries exactly one closed reason") + return self + + +class CloudDecisionRejected(ValueError): + def __init__(self, code: str, message: str) -> None: + super().__init__(message) + self.code = code + + +def signing_payload(fields: Mapping[str, Any]) -> bytes: + """The exact bytes Cloud signs: domain prefix plus canonical JSON.""" + + unsigned = {k: v for k, v in fields.items() if k != "signature"} + return _DOMAIN + _canonical(unsigned) + + +def sign_decision(fields: Mapping[str, Any], private_key: Any) -> CloudDecisionV1: + """Reference signer (Cloud's signer must match it byte for byte).""" + + unsigned = {k: v for k, v in dict(fields).items() if k != "signature"} + placeholder = CloudDecisionV1.model_validate( + {**unsigned, "signature": "A" * 86 + "=="} + ) + body = placeholder.model_dump(mode="json") + signature = private_key.sign(signing_payload(body)) + return CloudDecisionV1.model_validate( + {**body, "signature": base64.b64encode(signature).decode("ascii")} + ) + + +def default_keys_path() -> Path: + explicit = os.environ.get(KEYS_ENV) + if explicit: + return Path(explicit) + home = Path(os.environ.get("OPENADAPT_HOME") or Path.home() / ".openadapt") + return home / "cloud-decision-keys.json" + + +def load_pinned_keys(path: Optional[Path | str] = None) -> dict[str, str]: + """The runner's pinned Cloud decision keys. Empty when none are pinned.""" + + target = Path(path) if path is not None else default_keys_path() + try: + data = json.loads(target.read_text(encoding="utf-8")) + except (OSError, ValueError): + return {} + if not isinstance(data, dict): + return {} + return {str(k): str(v) for k, v in data.items() if isinstance(v, str)} + + +def verify_decision( + raw: Any, + *, + request_id: str, + request_sha256: str, + pinned_keys: Mapping[str, str], +) -> CloudDecisionV1: + """Accept a Cloud decision only for this exact request, signed by a pinned key.""" + + from cryptography.exceptions import InvalidSignature + from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey + + try: + if isinstance(raw, CloudDecisionV1): + decision = raw + elif isinstance(raw, (bytes, str)): + decision = CloudDecisionV1.model_validate_json(raw) + else: + decision = CloudDecisionV1.model_validate(raw) + except ValidationError as exc: + raise CloudDecisionRejected( + "cloud_decision_malformed", "malformed decision" + ) from exc + encoded = pinned_keys.get(decision.key_id) + if not encoded: + raise CloudDecisionRejected( + "cloud_decision_key_not_pinned", + "the decision key is not pinned on this runner", + ) + try: + Ed25519PublicKey.from_public_bytes( + base64.b64decode(encoded, validate=True) + ).verify( + base64.b64decode(decision.signature, validate=True), + signing_payload(decision.model_dump(mode="json")), + ) + except (InvalidSignature, ValueError, TypeError, binascii.Error) as exc: + raise CloudDecisionRejected( + "cloud_decision_signature_invalid", "the decision signature is invalid" + ) from exc + if decision.request_id != request_id: + raise CloudDecisionRejected( + "request_mismatch", "the decision is for another request" + ) + if decision.request_sha256 != request_sha256: + raise CloudDecisionRejected( + "request_digest_mismatch", + "the decision is for another version of this save", + ) + return decision + + +__all__ = [ + "CLOUD_DECISION_SCHEMA", + "CLOUD_PHONE_ROUTE", + "CloudDecisionRejected", + "CloudDecisionV1", + "KEYS_ENV", + "load_pinned_keys", + "sign_decision", + "signing_payload", + "verify_decision", +] diff --git a/packages/flow/openadapt_flow/approval_mode/gate.py b/packages/flow/openadapt_flow/approval_mode/gate.py index 1a2c59558..b932edbc0 100644 --- a/packages/flow/openadapt_flow/approval_mode/gate.py +++ b/packages/flow/openadapt_flow/approval_mode/gate.py @@ -87,6 +87,7 @@ ) if TYPE_CHECKING: + from openadapt_flow.approval_mode.phone_relay import PhoneRelay from openadapt_flow.ir import RunReport, Step, StepResult, Workflow SignedInVerdict = Literal["match", "mismatch", "unreadable"] @@ -261,6 +262,7 @@ def __init__( approver_policy: Optional["ApproverPolicyV1"] = None, approver: Optional["Approver"] = None, policy_name: Optional[str] = None, + phone_relay: Optional["PhoneRelay"] = None, ) -> None: self.mode = resolve_approval_mode(mode) if approval_store is None or isinstance(approval_store, ItemApprovalStore): @@ -271,6 +273,8 @@ def __init__( # the run's store records the policy so every reader applies the same. self.approver_policy = approver_policy self.approver = approver + #: Opt-in phone approval through Cloud. ``None``: Desktop only. + self.phone_relay = phone_relay self._policy_name = policy_name if approver_policy is not None and self.approval_store is not None: self.approval_store.write_policy(approver_policy) @@ -621,6 +625,7 @@ def wait_refusal(detail: str) -> GateVerdict: monotonic=self._monotonic, sleep=self._sleep, approver=self.approver, + phone_relay=self.phone_relay, ) if waited is None: return wait_refusal(code or "approval_wait_timed_out") diff --git a/packages/flow/openadapt_flow/approval_mode/item_approval.py b/packages/flow/openadapt_flow/approval_mode/item_approval.py index 814ef8704..fad3108b3 100644 --- a/packages/flow/openadapt_flow/approval_mode/item_approval.py +++ b/packages/flow/openadapt_flow/approval_mode/item_approval.py @@ -26,11 +26,13 @@ from __future__ import annotations import hashlib +import json import os import stat import time import uuid -from collections.abc import Callable +from collections.abc import Callable, Mapping +from dataclasses import dataclass, field from datetime import datetime, timedelta, timezone from pathlib import Path from typing import Any, Literal, Optional @@ -53,6 +55,13 @@ StepClass, WebhookRefused, ) +from openadapt_flow.approval_mode.cloud_decision import ( + CLOUD_PHONE_ROUTE, + CloudDecisionRejected, + CloudDecisionV1, + load_pinned_keys, + verify_decision, +) from openadapt_flow.approval_mode.model import ( ITEM_APPROVAL_ROUTE, OPAQUE_ID_PATTERN, @@ -62,6 +71,14 @@ DeclineReason, ItemApprovalRef, ) +from openadapt_flow.approval_mode.phone_relay import PHONE_POLL_S, PhoneRelay +from openadapt_flow.approval_mode.signed_policy import ( + GrantRefused, + SignedApproverGrant, + VerifiedGrant, + allows_non_person, + verify_grant, +) from openadapt_flow.approval_mode.switch import canonical_json REQUEST_SCHEMA: Literal["openadapt.item-approval-request/v1"] = ( @@ -75,12 +92,22 @@ #: How often a waiting run asks a delegated approver again (seconds). DELEGATE_POLL_S = 5.0 POLICY_FILENAME = "approver-policy.json" +#: The person-signed grant behind a policy that lets a delegate answer. +GRANT_FILENAME = "approver-grant.json" +#: Who started the run, written once by the run. A delegate never answers a +#: run it started. +INITIATOR_FILENAME = "run-initiator.json" +#: The initiator an agent-started run records when the agent has no id. +UNIDENTIFIED_AGENT = "agent:unidentified" +#: Routes a person answers by: Desktop's Run it, or their phone through Cloud. +PERSON_ROUTES = frozenset({ITEM_APPROVAL_ROUTE, CLOUD_PHONE_ROUTE}) #: How long a run waits for an answer before it stops before the save. DEFAULT_APPROVAL_WAIT_S = 900.0 MAX_APPROVAL_WAIT_S = 4 * 3600.0 #: How long an answer to a request stays usable, including by a later resume. REQUEST_TTL_S = 4 * 3600.0 _MAX_FILE_BYTES = 64 * 1024 +_MAX_FRAME_BYTES = 8 * 1024 * 1024 _REQUEST_ID_PATTERN = r"^[0-9a-f]{32}$" _HEX_SHA256 = r"^[0-9a-f]{64}$" @@ -193,12 +220,16 @@ class ItemApprovalAnswer(BaseModel): #: for a person, whose older fields above already name them. Left out of #: the file when absent, so earlier answers keep their digest. approver: Optional[ApproverIdentityV1] = None + #: The Cloud-signed decision behind a phone answer (``cloud_phone``). + cloud_decision: Optional[CloudDecisionV1] = None @model_serializer(mode="wrap") def _serialize_compatible(self, handler: Any) -> dict[str, Any]: data: dict[str, Any] = handler(self) if self.approver is None: data.pop("approver", None) + if self.cloud_decision is None: + data.pop("cloud_decision", None) return data def digest(self) -> str: @@ -235,21 +266,116 @@ def reference(self, request: ItemApprovalRequest) -> ItemApprovalRef: ) +@dataclass(frozen=True) +class ApproverContext: + """Everything an answer is checked against, read from the run's store. + + ``policy`` is person-only unless ``grant`` is a person-signed grant that + verified just now. ``initiator`` is who started the run (``None`` for a + person or operator); ``initiator_known`` is False when the run never + recorded it, and then no delegate may answer. ``cloud_keys`` are the + runner's pinned Cloud decision keys; empty means no phone route. + """ + + policy: ApproverPolicyV1 = DEFAULT_APPROVER_POLICY + grant: Optional[VerifiedGrant] = None + grant_refusal: Optional[str] = None + initiator: Optional[str] = None + initiator_known: bool = False + cloud_keys: Mapping[str, str] = field(default_factory=dict) + + +def _as_context( + policy: "ApproverPolicyV1 | ApproverContext | None", +) -> ApproverContext: + if isinstance(policy, ApproverContext): + return policy + # A bare policy carries no signed grant, so it can never admit a delegate. + return ApproverContext(policy=policy or DEFAULT_APPROVER_POLICY) + + +def _delegate_refusal( + request: ItemApprovalRequest, + approver: ApproverIdentityV1, + context: ApproverContext, +) -> Optional[str]: + """Why this delegate can't answer this request under the run's grant.""" + + grant = context.grant + if grant is None and context.grant_refusal is not None: + # The run asked for delegates, but the grant didn't verify: say why. + return context.grant_refusal + refusal = context.policy.refusal(request.step_class, approver) + if refusal is not None: + return refusal.value + if grant is None: + return "approver_policy_unsigned" + if not grant.names(approver.id): + return "delegate_not_in_grant" + if not grant.covers(request.workflow_name): + return "workflow_not_in_grant" + if approver.policy_ref is not None and approver.policy_ref != grant.policy_ref: + return "policy_ref_mismatch" + if not context.initiator_known: + return "run_initiator_unknown" + if context.initiator == UNIDENTIFIED_AGENT: + # An agent we can't name started the run: any delegate might be it. + return "self_approval_unprovable" + if context.initiator is not None and ( + context.initiator.lower() == (approver.id or "").lower() + or context.initiator.lower() == (approver.on_behalf_of or "").lower() + ): + return "self_approval" + return None + + +def _cloud_phone_refusal( + request: ItemApprovalRequest, + answer: ItemApprovalAnswer, + context: ApproverContext, +) -> Optional[str]: + decision = answer.cloud_decision + if decision is None: + return "cloud_decision_missing" + try: + verify_decision( + decision, + request_id=request.request_id, + request_sha256=request.digest(), + pinned_keys=context.cloud_keys, + ) + except CloudDecisionRejected as exc: + return exc.code + if ( + answer.approver_kind != "cloud_user" + or answer.approver_id != decision.approver_id + or answer.auth_method != "cloud_aal2" + or answer.decision != decision.decision + or answer.decline_reason != decision.decline_reason + ): + return "answer_fields_inconsistent" + return None + + def answer_refusal( request: ItemApprovalRequest, answer: ItemApprovalAnswer, *, now: Optional[datetime] = None, - policy: Optional[ApproverPolicyV1] = None, + policy: "ApproverPolicyV1 | ApproverContext | None" = None, ) -> Optional[str]: """Return a closed reason this answer can't be used, or ``None``. The run calls this itself; it never trusts that the CLI checked. - ``policy`` is the run's approver policy (person-only when ``None``). A - person answers through Desktop's Run it. An agent, rule or system answers - only where the policy allows its kind for the request's step class. + ``policy`` is the run's :class:`ApproverContext` (``store.context()``); a + bare policy or ``None`` means no delegate may answer. A person answers + through Desktop's Run it, or on their phone through Cloud with a + Cloud-signed decision. An agent or system answers only under a verified, + person-signed grant that names it, allows its kind for the step class, + and wasn't started by it. """ + context = _as_context(policy) if answer.request_id != request.request_id: return "request_mismatch" if answer.request_sha256 != request.digest(): @@ -258,10 +384,16 @@ def answer_refusal( if approver is None or approver.is_person: if answer.decided_by != "human": return "not_decided_by_a_person" - if answer.route != ITEM_APPROVAL_ROUTE: + if answer.route not in PERSON_ROUTES: return "route_not_allowed" if approver is not None and approver.auth.value != answer.approver_kind: return "answer_fields_inconsistent" + if answer.route == CLOUD_PHONE_ROUTE: + refusal = _cloud_phone_refusal(request, answer, context) + if refusal is not None: + return refusal + elif answer.cloud_decision is not None: + return "answer_fields_inconsistent" else: if answer.decided_by != "automation": return "answer_fields_inconsistent" @@ -270,11 +402,11 @@ def answer_refusal( if answer.approver_kind != "unauthenticated" or answer.approver_id is not None: # The older person fields stay empty; ``approver`` names who decided. return "answer_fields_inconsistent" - refusal = (policy or DEFAULT_APPROVER_POLICY).refusal( - request.step_class, approver - ) - if refusal is not None: - return refusal.value + if answer.cloud_decision is not None: + return "answer_fields_inconsistent" + refusal_code = _delegate_refusal(request, approver, context) + if refusal_code is not None: + return refusal_code try: answer.reference(request) except ValidationError: @@ -374,9 +506,20 @@ def _fsync_dir(path: Path) -> None: class ItemApprovalStore: """The ``item-approval`` folder of one run directory.""" - def __init__(self, run_dir: Path | str) -> None: + def __init__( + self, + run_dir: Path | str, + *, + key_dir: Optional[Path | str] = None, + cloud_keys: Optional[Mapping[str, str]] = None, + ) -> None: self.run_dir = Path(run_dir) self.dir = self.run_dir / APPROVAL_DIRNAME + #: Where the signer registry lives (``$OPENADAPT_JIT_KEY_DIR`` or + #: ``~/.openadapt/jit`` when ``None``). Keep it away from the agents. + self._key_dir = Path(key_dir) if key_dir is not None else None + #: Pinned Cloud decision keys; ``None`` reads the runner's file. + self._cloud_keys = dict(cloud_keys) if cloud_keys is not None else None # -- layout -------------------------------------------------------------- def _checked_dir(self, *, create: bool) -> Path: @@ -416,26 +559,126 @@ def write_policy(self, policy: ApproverPolicyV1) -> None: try: _write_exclusive(self.dir / POLICY_FILENAME, payload) except FileExistsError: - if self.policy() != policy: + if self._recorded_policy() != policy: raise ItemApprovalRefused( "policy_changed", "this run already has a different approver policy" ) from None - def policy(self) -> ApproverPolicyV1: - """The run's approver policy. Person-only when none was written. + def write_grant(self, signed: SignedApproverGrant) -> None: + """Record the person-signed grant once, and the policy it carries.""" + + self._checked_dir(create=True) + payload = canonical_json(signed.model_dump(mode="json", by_alias=True)) + try: + _write_exclusive(self.dir / GRANT_FILENAME, payload) + except FileExistsError: + existing = self._read_grant() + if existing != signed: + raise ItemApprovalRefused( + "policy_changed", "this run already has a different approver grant" + ) from None + self.write_policy(signed.grant.approver_policy) + + def _read_grant(self) -> Optional[SignedApproverGrant]: + try: + raw = _read_regular(self._path(GRANT_FILENAME)) + except (OSError, ItemApprovalRefused): + return None + try: + return SignedApproverGrant.model_validate_json(raw) + except ValueError: + return None + + def write_initiator(self, initiator: Optional[str]) -> None: + """Record who started the run, once. ``None``: a person or operator. + + The run writes this before it asks anything. A file that is already + there with another value (planted before the run) refuses the run. + """ - An unreadable policy file is never "no policy": it means person-only. + self._checked_dir(create=True) + payload = canonical_json({"started_by": initiator}) + try: + _write_exclusive(self.dir / INITIATOR_FILENAME, payload) + except FileExistsError: + known, existing = self._read_initiator() + if not known or existing != initiator: + raise ItemApprovalRefused( + "initiator_changed", "this run already names another initiator" + ) from None + + def _read_initiator(self) -> tuple[bool, Optional[str]]: + try: + raw = _read_regular(self._path(INITIATOR_FILENAME)) + data = json.loads(raw) + except (OSError, ItemApprovalRefused, ValueError): + return False, None + if not isinstance(data, dict) or set(data) != {"started_by"}: + return False, None + value = data["started_by"] + if value is not None and not isinstance(value, str): + return False, None + return True, value + + def _key_directory(self) -> Path: + if self._key_dir is not None: + return self._key_dir + from openadapt_flow.jit.lifecycle import default_key_dir + + return default_key_dir() + + def context(self, *, now: Optional[datetime] = None) -> ApproverContext: + """What every answer in this run is checked against, verified now. + + The grant's signature, signer and dates are re-checked on every call, + so a grant that expired, or a file swapped mid-run, stops working. """ + known, initiator = self._read_initiator() + if self._cloud_keys is None: + cloud_keys = load_pinned_keys() + else: + cloud_keys = dict(self._cloud_keys) + + def make(**fields: Any) -> ApproverContext: + return ApproverContext( + initiator=initiator, + initiator_known=known, + cloud_keys=cloud_keys, + **fields, + ) + + recorded = self._recorded_policy() + if recorded is None or not allows_non_person(recorded): + return make(policy=recorded or DEFAULT_APPROVER_POLICY) + signed = self._read_grant() + if signed is None: + return make(grant_refusal="approver_policy_unsigned") + try: + verified = verify_grant(signed, self._key_directory(), now=now) + except GrantRefused as exc: + return make(grant_refusal=exc.code) + if verified.policy != recorded: + return make(grant_refusal="approver_grant_policy_mismatch") + return make(policy=verified.policy, grant=verified) + + def _recorded_policy(self) -> Optional[ApproverPolicyV1]: try: raw = _read_regular(self._path(POLICY_FILENAME)) except (OSError, ItemApprovalRefused): - return DEFAULT_APPROVER_POLICY + return None try: return ApproverPolicyV1.model_validate_json(raw) except ValueError: return DEFAULT_APPROVER_POLICY + def policy(self) -> ApproverPolicyV1: + """The run's effective approver policy. Person-only when none was + written, when it is unreadable, or when it would let a delegate answer + without a person-signed grant that verifies now.""" + + return self.context().policy + # -- requests ------------------------------------------------------------ def open_request( self, @@ -588,7 +831,7 @@ def status( return "invalid_answer" if answer is None: return "expired" if request.expired(now) else "waiting" - refusal = answer_refusal(request, answer, now=now, policy=self.policy()) + refusal = answer_refusal(request, answer, now=now, policy=self.context(now=now)) if refusal == "request_expired": return "expired" if refusal is not None: @@ -644,7 +887,10 @@ def find_unused_approval( return None if answer is None or answer.decision != "approve": return None - if answer_refusal(request, answer, now=now, policy=self.policy()) is not None: + if ( + answer_refusal(request, answer, now=now, policy=self.context(now=now)) + is not None + ): return None return request, answer @@ -658,33 +904,73 @@ def wait_for_answer( monotonic: Callable[[], float] = time.monotonic, sleep: Callable[[float], None] = time.sleep, approver: Optional[Approver] = None, + phone_relay: Optional["PhoneRelay"] = None, ) -> tuple[Optional[ItemApprovalAnswer], Optional[str]]: """Wait for a usable answer. Returns ``(answer, None)`` or ``(None, code)``. ``code`` is ``approval_wait_timed_out`` when nobody answered in time, or the closed reason an answer was refused. A refused answer is final for this request: the run stops before the save rather than waiting on. + + With ``phone_relay`` (opt-in phone approval) the request also goes to + Cloud, and a Cloud-signed phone decision for this exact request becomes a + ``cloud_phone`` answer. Whichever answer is written first wins. """ + try: + return _wait_for_answer( + store, + request, + timeout_s=timeout_s, + poll_s=poll_s, + monotonic=monotonic, + sleep=sleep, + approver=approver, + phone_relay=phone_relay, + ) + finally: + if phone_relay is not None: + _withdraw(store, request, phone_relay) + + +def _wait_for_answer( + store: ItemApprovalStore, + request: ItemApprovalRequest, + *, + timeout_s: float, + poll_s: float, + monotonic: Callable[[], float], + sleep: Callable[[float], None], + approver: Optional[Approver], + phone_relay: Optional["PhoneRelay"], +) -> tuple[Optional[ItemApprovalAnswer], Optional[str]]: + deadline = monotonic() + max(0.0, min(timeout_s, MAX_APPROVAL_WAIT_S)) - policy = store.policy() + context = store.context() delegate = approver - if delegate is not None and policy.refusal(request.step_class, delegate.identity): - # The policy doesn't let this approver decide this step: ask a person. + if delegate is not None and _delegate_refusal(request, delegate.identity, context): + # The grant doesn't let this approver decide this step: ask a person. delegate = None next_ask = 0.0 + relay = phone_relay + if relay is not None and not _publish(store, request, relay): + relay = None # Cloud didn't take it: Desktop still can. + next_phone = 0.0 while True: try: answer = store.read_answer(request.request_id) except ItemApprovalRefused as exc: return None, exc.code + if answer is None and relay is not None and monotonic() >= next_phone: + next_phone = monotonic() + PHONE_POLL_S + answer = _take_phone_decision(store, request, relay) if answer is None and delegate is not None and monotonic() >= next_ask: next_ask = monotonic() + DELEGATE_POLL_S answer = _ask_delegate(store, request, delegate) if answer is None and getattr(delegate, "_failed", False): delegate = None if answer is not None: - refusal = answer_refusal(request, answer, policy=policy) + refusal = answer_refusal(request, answer, policy=store.context()) if refusal is not None: return None, refusal return answer, None @@ -695,6 +981,94 @@ def wait_for_answer( sleep(poll_s) +def _publish( + store: ItemApprovalStore, request: ItemApprovalRequest, relay: "PhoneRelay" +) -> bool: + try: + frame = _read_regular( + store.run_dir / request.screenshot, limit=_MAX_FRAME_BYTES + ) + return bool(relay.publish(request, frame)) + except Exception: # noqa: BLE001 - any relay failure leaves Desktop approval + return False + + +def _withdraw( + store: ItemApprovalStore, request: ItemApprovalRequest, relay: "PhoneRelay" +) -> None: + try: + answer = store.read_answer(request.request_id) + except ItemApprovalRefused: + answer = None + if answer is not None and answer.route == CLOUD_PHONE_ROUTE: + return # The phone answered; Cloud already closed the link. + reason = ( + "answered_elsewhere" + if answer is not None + else ("expired" if request.expired() else "run_stopped_waiting") + ) + try: + relay.withdraw(request, reason) # type: ignore[arg-type] + except Exception: # noqa: BLE001 - Cloud expires the link on its own + pass + + +def _take_phone_decision( + store: ItemApprovalStore, request: ItemApprovalRequest, relay: "PhoneRelay" +) -> Optional[ItemApprovalAnswer]: + """Turn a Cloud-signed phone decision into this request's answer.""" + + try: + raw = relay.poll(request) + except Exception: # noqa: BLE001 - try again next poll; Desktop still can + return None + if raw is None: + return None + return apply_cloud_decision(store, request, raw) + + +def apply_cloud_decision( + store: ItemApprovalStore, + request: ItemApprovalRequest, + raw: Any, + *, + now: Optional[datetime] = None, +) -> Optional[ItemApprovalAnswer]: + """Verify a Cloud decision for ``request`` and write it as the answer. + + Returns ``None`` (and writes nothing) for a decision that doesn't verify, + or when another answer was already written. + """ + + context = store.context(now=now) + try: + decision = verify_decision( + raw, + request_id=request.request_id, + request_sha256=request.digest(), + pinned_keys=context.cloud_keys, + ) + except CloudDecisionRejected: + return None + try: + answer = make_answer( + request, + decision=decision.decision, + decided_by="human", + route=CLOUD_PHONE_ROUTE, + approver_id=decision.approver_id, + auth_method=decision.auth_method, + decline_reason=decision.decline_reason, + now=now, + policy=context, + cloud_decision=decision, + ) + store.write_answer(answer) + except ItemApprovalRefused: + return None + return answer + + def request_facts(request: ItemApprovalRequest) -> dict[str, str]: """The PHI-free facts a delegated approver sees. No intent, no image.""" @@ -739,7 +1113,7 @@ def _ask_delegate( auth_method="none", decline_reason=decided.decline_reason, approver=delegate.identity, - policy=store.policy(), + policy=store.context(), ) store.write_answer(answer) except ItemApprovalRefused: @@ -759,14 +1133,27 @@ def make_answer( decline_reason: Optional[str] = None, now: Optional[datetime] = None, approver: Optional[ApproverIdentityV1] = None, - policy: Optional[ApproverPolicyV1] = None, + policy: "ApproverPolicyV1 | ApproverContext | None" = None, + cloud_decision: Optional[CloudDecisionV1] = None, ) -> ItemApprovalAnswer: """Build and check one answer. Raises :class:`ItemApprovalRefused`. - ``approver`` names an agent, rule or system; it is refused unless - ``policy`` (the run's, read from its store) allows that kind here. + ``approver`` names an agent or system; it is refused unless ``policy`` + (``store.context()``) holds a verified, person-signed grant that names it + and allows its kind here. The answer records the grant's digest as the + approver's ``policy_ref``. ``cloud_decision`` is the Cloud-signed phone + decision behind a ``cloud_phone`` answer. """ + context = _as_context(policy) + if ( + approver is not None + and not approver.is_person + and approver.policy_ref is None + and context.grant is not None + ): + approver = approver.model_copy(update={"policy_ref": context.grant.policy_ref}) + approver_kind: Literal["cloud_user", "unauthenticated"] = ( "cloud_user" if approver_id is not None else "unauthenticated" ) @@ -783,12 +1170,13 @@ def make_answer( auth_method=auth_method, # type: ignore[arg-type] answered_at=_iso(now or _utc_now()), approver=approver, + cloud_decision=cloud_decision, ) except ValidationError as exc: raise ItemApprovalRefused( "answer_fields_invalid", "the answer's fields are not valid" ) from exc - refusal = answer_refusal(request, answer, now=now, policy=policy) + refusal = answer_refusal(request, answer, now=now, policy=context) if refusal is not None: raise ItemApprovalRefused(refusal, f"the answer was refused ({refusal})") return answer @@ -817,11 +1205,46 @@ def request_summary( "expires_at": request.expires_at, "qualification_case_kind": request.qualification_case_kind, "step_class": request.step_class.value, + "decided_by": decided_by_summary(store, request), + } + + +def decided_by_summary( + store: ItemApprovalStore, request: ItemApprovalRequest +) -> Optional[dict[str, Any]]: + """Who answered this request, for Desktop and Cloud to show. + + A person: their Cloud user id (or none when Desktop was signed out) and + where they answered (``desktop_run_it`` or ``cloud_phone``). An agent or + system: its id and the digest of the person-signed grant it acted under. + ``None`` when nobody answered yet or the answer isn't readable. + """ + + try: + answer = store.read_answer(request.request_id) + except ItemApprovalRefused: + return None + if answer is None: + return None + identity = answer.approver_identity() + return { + "decision": answer.decision, + "kind": identity.kind.value, + "id": identity.id, + "auth_method": answer.auth_method, + "route": answer.route, + "policy_ref": identity.policy_ref, + "answered_at": answer.answered_at, } __all__ = [ "ANSWER_SCHEMA", + "ApproverContext", + "GRANT_FILENAME", + "INITIATOR_FILENAME", + "PERSON_ROUTES", + "UNIDENTIFIED_AGENT", "APPROVAL_DIRNAME", "DEFAULT_APPROVAL_WAIT_S", "MAX_APPROVAL_WAIT_S", @@ -833,6 +1256,7 @@ def request_summary( "ItemApprovalStore", "POLICY_FILENAME", "answer_refusal", + "apply_cloud_decision", "make_answer", "request_facts", "request_summary", diff --git a/packages/flow/openadapt_flow/approval_mode/model.py b/packages/flow/openadapt_flow/approval_mode/model.py index 15e3d82eb..6439a0c83 100644 --- a/packages/flow/openadapt_flow/approval_mode/model.py +++ b/packages/flow/openadapt_flow/approval_mode/model.py @@ -167,6 +167,8 @@ class ItemDecisionKind(str, Enum): DECLINE_REASONS: tuple[str, ...] = ("wrong_values", "wrong_record", "screen_problem") #: The only route that may approve or decline an item in phase 0. ITEM_APPROVAL_ROUTE: Literal["desktop_run_it"] = "desktop_run_it" +#: Routes a person answers by: Desktop's Run it, or their phone through Cloud. +PERSON_APPROVAL_ROUTES = frozenset({"desktop_run_it", "cloud_phone"}) #: How the approver was authenticated. ``none`` only for ``unauthenticated``. ApproverAuthMethod = Literal["cloud_aal2", "cloud_aal1", "none"] @@ -187,9 +189,10 @@ class ItemApprovalRef(BaseModel): decision: Literal["approved", "declined"] decline_reason: Optional[DeclineReason] = None decided_by: Literal["human", "automation"] = "human" - #: ``desktop_run_it`` for a person; ``local_cli``, ``webhook`` or ``mcp`` + #: ``desktop_run_it`` or ``cloud_phone`` (a Cloud-signed decision from + #: the person's phone) for a person; ``local_cli``, ``webhook`` or ``mcp`` #: for a delegated approver. - route: Literal["desktop_run_it", "local_cli", "webhook", "mcp"] = ( + route: Literal["desktop_run_it", "cloud_phone", "local_cli", "webhook", "mcp"] = ( ITEM_APPROVAL_ROUTE ) approver_kind: Literal["cloud_user", "unauthenticated"] @@ -215,8 +218,12 @@ def _consistent(self) -> "ItemApprovalRef": raise ValueError("an automated decision, and only one, names its approver") if self.approver is not None and self.approver.is_person: raise ValueError("a person's decision uses the person fields") - if (self.route == ITEM_APPROVAL_ROUTE) != (self.decided_by == "human"): - raise ValueError("a person answers in Desktop; a delegate by its own route") + if (self.route in PERSON_APPROVAL_ROUTES) != (self.decided_by == "human"): + raise ValueError( + "a person answers in Desktop or on their phone; a delegate by its own route" + ) + if self.route == "cloud_phone" and self.auth_method != "cloud_aal2": + raise ValueError("a phone approval needs a Cloud sign-in with AAL2") if self.approver_kind == "cloud_user": if self.approver_id is None or self.auth_method == "none": raise ValueError("a Cloud approver needs an id and a sign-in method") diff --git a/packages/flow/openadapt_flow/approval_mode/phone_relay.py b/packages/flow/openadapt_flow/approval_mode/phone_relay.py new file mode 100644 index 000000000..664dc5951 --- /dev/null +++ b/packages/flow/openadapt_flow/approval_mode/phone_relay.py @@ -0,0 +1,172 @@ +"""Send a waiting save to Cloud so a person can approve it from their phone. + +Off unless the deployment opts in (``runtime.phone_approval: true`` or +``run --phone-approval``). A local-only or no-outbound install never builds a +relay, and every save keeps waiting for a person in Desktop's Run it. + +What crosses to Cloud, and when: + +* ``publish``: the request's ids and digests, the step's own intent, and the + same filled-form screenshot Desktop shows. Cloud keeps the screenshot + encrypted and only for the request's life (at most 4 hours); it shows it + only to a signed-in (AAL2) member of the organization. The push + notification Cloud sends carries an opaque id and fixed words, never the + screenshot, a name, a value or the workflow's name. +* ``poll``: the Cloud-signed decision, once a person tapped Approve or + Don't save. The run verifies it against the runner's pinned decision key + and the exact request digest (:mod:`.cloud_decision`), then writes it as a + ``cloud_phone`` answer, created once. A Desktop answer that landed first + wins and the phone decision is ignored. +* ``withdraw``: when the request was answered elsewhere, expired or the run + stopped waiting, so the phone link stops working. + +Any network or Cloud failure leaves the save to Desktop. Nothing here retries +a save, and an approval still only lets the run re-check the form and save +exactly what was shown. +""" + +from __future__ import annotations + +import base64 +import os +from dataclasses import dataclass +from typing import Any, Literal, Optional, Protocol + +from openadapt_flow.approval_mode.cloud_decision import CLOUD_DECISION_SCHEMA + +PUBLISH_SCHEMA: Literal["openadapt.save-approval-publish/v1"] = ( + "openadapt.save-approval-publish/v1" +) +WithdrawReason = Literal["answered_elsewhere", "expired", "run_stopped_waiting"] +#: How often the run asks Cloud for a phone decision (seconds). +PHONE_POLL_S = 3.0 +CLOUD_URL_ENV = "OPENADAPT_CLOUD_URL" +RUNNER_TOKEN_ENV = "OPENADAPT_RUNNER_TOKEN" +RUNNER_ID_ENV = "OPENADAPT_RUNNER_ID" + + +class PhoneRelay(Protocol): + runner_id: str + + def publish(self, request: Any, frame_png: bytes) -> bool: ... + + def poll(self, request: Any) -> Optional[dict[str, Any]]: ... + + def withdraw(self, request: Any, reason: WithdrawReason) -> None: ... + + +class PhoneRelayError(RuntimeError): + pass + + +def publish_body(request: Any, frame_png: bytes, runner_id: str) -> dict[str, Any]: + """The exact publish body. Cloud refuses any other field.""" + + return { + "schema": PUBLISH_SCHEMA, + "runner_id": runner_id, + "request_id": request.request_id, + "request_sha256": request.digest(), + "run_id": request.run_id, + "workflow_name": request.workflow_name, + "step_id": request.step_id, + "step_index": request.step_index, + "step_intent": request.step_intent, + "step_class": request.step_class.value, + "mode": request.mode.value, + "form_state_sha256": request.form_state_sha256, + "screenshot_sha256": request.screenshot_sha256, + "screenshot_png_b64": base64.b64encode(frame_png).decode("ascii"), + "created_at": request.created_at, + "expires_at": request.expires_at, + } + + +@dataclass +class CloudPhoneRelay: + """The HTTPS relay to Cloud's runner API (``Authorization: Bearer oar_…``).""" + + base_url: str + token: str + runner_id: str + timeout_s: float = 15.0 + transport: Any = None + + def __post_init__(self) -> None: + if not self.base_url.startswith("https://") and not self.base_url.startswith( + "http://127.0.0.1" + ): + raise PhoneRelayError("phone approval needs an https Cloud URL") + if not self.token.startswith("oar_"): + raise PhoneRelayError("phone approval needs this runner's Cloud token") + import httpx + + self._client = httpx.Client( + base_url=self.base_url.rstrip("/"), + timeout=self.timeout_s, + transport=self.transport, + headers={"authorization": f"Bearer {self.token}"}, + ) + + def publish(self, request: Any, frame_png: bytes) -> bool: + response = self._client.post( + "/api/runners/save-approvals", + json=publish_body(request, frame_png, self.runner_id), + ) + return response.status_code in (200, 201) + + def poll(self, request: Any) -> Optional[dict[str, Any]]: + response = self._client.get(f"/api/runners/save-approvals/{request.request_id}") + if response.status_code != 200: + return None + body = response.json() + if not isinstance(body, dict) or body.get("status") != "decided": + return None + decision = body.get("decision") + if not isinstance(decision, dict): + return None + if decision.get("schema_version") != CLOUD_DECISION_SCHEMA: + return None + return decision + + def withdraw(self, request: Any, reason: WithdrawReason) -> None: + self._client.post( + f"/api/runners/save-approvals/{request.request_id}/withdraw", + json={"reason": reason}, + ) + + def decision_keys(self) -> dict[str, str]: + response = self._client.get("/api/runners/save-approvals/keys") + if response.status_code != 200: + raise PhoneRelayError(f"Cloud answered {response.status_code}") + keys = (response.json() or {}).get("decision_keys") + if not isinstance(keys, dict) or not keys: + raise PhoneRelayError("Cloud returned no decision keys") + return {str(k): str(v) for k, v in keys.items()} + + +def relay_from_env(env: Optional[dict[str, str]] = None) -> CloudPhoneRelay: + """Build the relay from the runner's Cloud settings. Raises when missing: + an operator who asked for phone approval must not silently get none.""" + + source = os.environ if env is None else env + url = source.get(CLOUD_URL_ENV) or "https://app.openadapt.ai" + token = source.get(RUNNER_TOKEN_ENV) or "" + runner_id = source.get(RUNNER_ID_ENV) or "" + if not token or not runner_id: + raise PhoneRelayError( + f"phone approval needs {RUNNER_TOKEN_ENV} and {RUNNER_ID_ENV} " + "(pair this computer with OpenAdapt Cloud first)" + ) + return CloudPhoneRelay(url, token, runner_id) + + +__all__ = [ + "CloudPhoneRelay", + "PHONE_POLL_S", + "PUBLISH_SCHEMA", + "PhoneRelay", + "PhoneRelayError", + "publish_body", + "relay_from_env", +] diff --git a/packages/flow/openadapt_flow/approval_mode/signed_policy.py b/packages/flow/openadapt_flow/approval_mode/signed_policy.py new file mode 100644 index 000000000..6646d6d67 --- /dev/null +++ b/packages/flow/openadapt_flow/approval_mode/signed_policy.py @@ -0,0 +1,307 @@ +"""A person-signed grant that lets named agents or systems answer saves. + +Without a grant, only a person answers a save. An ``ApproverPolicyV1`` file on +its own is unsigned JSON, so anyone who could write the run directory could +widen it. Flow therefore accepts a non-person approver only under an +:class:`ApproverGrant` that a person signed, using the same signing as program +admission (:mod:`openadapt_flow.jit.authority`, monorepo #1207): + +* the person's Ed25519 key is registered as a ``policy_signer`` in + ``principals.json`` in the key directory (``$OPENADAPT_JIT_KEY_DIR`` or + ``~/.openadapt/jit``); an agent key can't also be a signer, and a signer + name that names an agent or model is refused; +* the grant names its delegates (agent or system ids), the approver policy + they act under, an expiry, and optionally the workflows it covers; +* the signature is domain-separated Ed25519 over canonical JSON. Its digest + (``policy_ref``) is recorded on every answer a delegate gives. + +An unsigned, tampered, expired or unregistered grant gives no authority: the +run falls back to person-only and records why. Flow re-verifies the grant every +time it reads an answer, so a file swapped mid-run is refused too. + +Nothing here lets an agent widen its own authority. There is no API to sign a +grant with an agent key, and the delegate that started a run can never answer +that run's saves (see ``ItemApprovalStore.write_initiator``). +""" + +from __future__ import annotations + +import hashlib +import json +import re +from dataclasses import dataclass +from datetime import datetime, timezone +from pathlib import Path +from typing import Any, Literal, Optional + +from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator + +from openadapt_flow.approval_mode.approver import ( + DEFAULT_APPROVER_POLICY, + ApproverKind, + ApproverPolicyV1, +) + +GRANT_SCHEMA: Literal["openadapt-flow/approver-grant/v1"] = ( + "openadapt-flow/approver-grant/v1" +) +GRANT_DOMAIN = b"openadapt-flow.approval.approver-grant/v1\x00" +_ID = r"^[A-Za-z0-9][A-Za-z0-9._:@/-]{0,255}$" +_SIGNER_ID = r"^[A-Za-z0-9][A-Za-z0-9._:@-]{0,127}$" +#: Longest a grant may last. A person re-signs to extend it. +MAX_GRANT_DAYS = 366 + +GrantRefusal = Literal[ + "approver_policy_unsigned", + "approver_grant_unreadable", + "approver_grant_signer_unregistered", + "approver_grant_signer_key_mismatch", + "approver_grant_signature_invalid", + "approver_grant_expired", + "approver_grant_not_yet_valid", + "approver_grant_policy_mismatch", +] + + +def _canonical(data: Any) -> bytes: + return json.dumps(data, sort_keys=True, separators=(",", ":")).encode("utf-8") + + +def _utc(value: str) -> datetime: + parsed = datetime.fromisoformat(value.replace("Z", "+00:00")) + if parsed.tzinfo is None: + raise ValueError(f"{value!r} has no time zone; use UTC (…Z)") + return parsed.astimezone(timezone.utc) + + +def allows_non_person(policy: ApproverPolicyV1) -> bool: + """True when ``policy`` lets anything other than a person answer.""" + + return any( + kind is not ApproverKind.PERSON + for kinds in policy.allowed.values() + for kind in kinds + ) + + +class ApproverGrant(BaseModel): + """What one person lets named delegates approve, and until when.""" + + model_config = ConfigDict(extra="forbid", frozen=True, populate_by_name=True) + + schema_: Literal["openadapt-flow/approver-grant/v1"] = Field( + default=GRANT_SCHEMA, alias="schema" + ) + grant_id: str = Field(pattern=_ID) + #: The person who signs, as registered under ``policy_signers``. + signer: str = Field(pattern=_SIGNER_ID) + signed_at: str = Field(min_length=10, max_length=64) + expires_at: str = Field(min_length=10, max_length=64) + #: Which kinds may answer each class of save under this grant. + approver_policy: ApproverPolicyV1 + #: The agent or system ids that may answer. Any other id is refused. + delegates: tuple[str, ...] = Field(min_length=1) + #: Workflow names the grant covers. Empty means every workflow. + workflows: tuple[str, ...] = () + + @model_validator(mode="after") + def _check(self) -> "ApproverGrant": + signed = _utc(self.signed_at) + expires = _utc(self.expires_at) + if expires <= signed: + raise ValueError("the grant expires before it was signed") + if (expires - signed).days > MAX_GRANT_DAYS: + raise ValueError(f"a grant lasts at most {MAX_GRANT_DAYS} days") + for delegate in self.delegates: + if not re.match(_ID, delegate): + raise ValueError(f"invalid delegate id {delegate!r}") + if len({d.lower() for d in self.delegates}) != len(self.delegates): + raise ValueError("each delegate is named once") + if self.signer.lower() in {d.lower() for d in self.delegates}: + raise ValueError( + "the person who signs a grant can't be one of its delegates" + ) + if not allows_non_person(self.approver_policy): + raise ValueError("a grant that lets only a person answer grants nothing") + return self + + def digest(self) -> str: + """``sha256:`` of the grant: the ``policy_ref`` answers record.""" + + payload = self.model_dump(mode="json", by_alias=True) + return ( + "sha256:" + hashlib.sha256(GRANT_DOMAIN + _canonical(payload)).hexdigest() + ) + + +class SignedApproverGrant(BaseModel): + model_config = ConfigDict(extra="forbid", frozen=True) + + grant: ApproverGrant + signer_public_key: str = Field(pattern=r"^[0-9a-f]{64}$") + signature: str = Field(min_length=1, max_length=200) + + def save(self, path: Path | str) -> Path: + out = Path(path) + out.write_text(self.model_dump_json(indent=2, by_alias=True) + "\n") + return out + + +@dataclass(frozen=True) +class VerifiedGrant: + """A grant whose signature, signer and dates Flow checked just now.""" + + grant: ApproverGrant + policy_ref: str + signer: str + + @property + def policy(self) -> ApproverPolicyV1: + return self.grant.approver_policy + + def names(self, delegate_id: Optional[str]) -> bool: + return delegate_id is not None and delegate_id in self.grant.delegates + + def covers(self, workflow_name: str) -> bool: + return not self.grant.workflows or workflow_name in self.grant.workflows + + +class GrantRefused(ValueError): + def __init__(self, code: GrantRefusal, message: str) -> None: + super().__init__(message) + self.code: GrantRefusal = code + + +def sign_grant(grant: ApproverGrant, key: Any) -> SignedApproverGrant: + """A person signs ``grant`` with their registered policy-signer key.""" + + from openadapt_flow.jit.authority import _sign, public_hex + + return SignedApproverGrant( + grant=grant, + signer_public_key=public_hex(key), + signature=_sign( + key, GRANT_DOMAIN, grant.model_dump(mode="json", by_alias=True) + ), + ) + + +def verify_grant( + signed: SignedApproverGrant, + key_dir: Path | str, + *, + now: Optional[datetime] = None, +) -> VerifiedGrant: + """Check ``signed`` against the registry in ``key_dir``. + + Raises: + GrantRefused: the signer isn't a registered person, the key or + signature doesn't match, or the grant isn't valid now. + """ + + from openadapt_flow.jit.authority import PrincipalRegistry, _verify + + try: + registry = PrincipalRegistry.load(Path(key_dir)) + except (OSError, ValueError) as exc: + raise GrantRefused( + "approver_grant_signer_unregistered", "the signer registry is unreadable" + ) from exc + grant = signed.grant + registered = registry.policy_signers.get(grant.signer) + if registered is None: + raise GrantRefused( + "approver_grant_signer_unregistered", + f"{grant.signer!r} is not a registered person who signs policy", + ) + if registered != signed.signer_public_key: + raise GrantRefused( + "approver_grant_signer_key_mismatch", + "the grant was signed with another key", + ) + payload = grant.model_dump(mode="json", by_alias=True) + if not _verify(registered, GRANT_DOMAIN, payload, signed.signature): + raise GrantRefused( + "approver_grant_signature_invalid", "the grant's signature does not verify" + ) + at = now or datetime.now(timezone.utc) + if _utc(grant.expires_at) <= at: + raise GrantRefused( + "approver_grant_expired", f"the grant expired at {grant.expires_at}" + ) + if _utc(grant.signed_at) > at: + raise GrantRefused( + "approver_grant_not_yet_valid", "the grant is signed in the future" + ) + return VerifiedGrant(grant=grant, policy_ref=grant.digest(), signer=grant.signer) + + +@dataclass(frozen=True) +class LoadedApproverPolicy: + """What ``--approver-policy`` resolved to for one run.""" + + policy: ApproverPolicyV1 + signed: Optional[SignedApproverGrant] = None + verified: Optional[VerifiedGrant] = None + #: Why a wider policy was not used, when it wasn't. + refusal: Optional[GrantRefusal] = None + + +def load_approver_policy( + path: Path | str, + key_dir: Path | str, + *, + now: Optional[datetime] = None, +) -> LoadedApproverPolicy: + """Resolve an ``--approver-policy`` file. Never raises for a bad file. + + * A signed grant that verifies: its policy and delegates apply. + * A plain ``openadapt.approver-policy/v1`` that lets only a person + answer: applies as-is (it changes nothing). + * Anything else (an unsigned policy that admits an agent, a tampered or + expired grant, an unreadable file): person-only, with the reason. + """ + + try: + raw = json.loads(Path(path).read_text(encoding="utf-8")) + except (OSError, ValueError): + return LoadedApproverPolicy( + DEFAULT_APPROVER_POLICY, refusal="approver_grant_unreadable" + ) + if isinstance(raw, dict) and "grant" in raw: + try: + signed = SignedApproverGrant.model_validate(raw) + except ValidationError: + return LoadedApproverPolicy( + DEFAULT_APPROVER_POLICY, refusal="approver_grant_unreadable" + ) + try: + verified = verify_grant(signed, key_dir, now=now) + except GrantRefused as exc: + return LoadedApproverPolicy(DEFAULT_APPROVER_POLICY, refusal=exc.code) + return LoadedApproverPolicy(verified.policy, signed, verified) + try: + policy = ApproverPolicyV1.model_validate(raw) + except ValidationError: + return LoadedApproverPolicy( + DEFAULT_APPROVER_POLICY, refusal="approver_grant_unreadable" + ) + if allows_non_person(policy): + return LoadedApproverPolicy( + DEFAULT_APPROVER_POLICY, refusal="approver_policy_unsigned" + ) + return LoadedApproverPolicy(policy) + + +__all__ = [ + "GRANT_SCHEMA", + "ApproverGrant", + "GrantRefused", + "LoadedApproverPolicy", + "SignedApproverGrant", + "VerifiedGrant", + "allows_non_person", + "load_approver_policy", + "sign_grant", + "verify_grant", +] diff --git a/packages/flow/openadapt_flow/contracts/approval.py b/packages/flow/openadapt_flow/contracts/approval.py index 0f82a7fd1..225a22c80 100644 --- a/packages/flow/openadapt_flow/contracts/approval.py +++ b/packages/flow/openadapt_flow/contracts/approval.py @@ -523,6 +523,7 @@ def counts_as_clean(tier: ReadBackTier, outcome: ReadBackOutcome) -> bool: #: Where a decision was made. ApprovalRoute = Literal[ "desktop_run_it", + "cloud_phone", "review_item", "review_batch", "local_cli", diff --git a/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json b/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json index 9b1dbdc3b..a87164357 100644 --- a/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json +++ b/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json @@ -191,6 +191,7 @@ "route": { "enum": [ "desktop_run_it", + "cloud_phone", "review_item", "review_batch", "local_cli", diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index 7b706dfc5..8eefb53b2 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -584,6 +584,10 @@ class RuntimeSection(BaseModel): #: ``--approval-mode`` wins. For the Standard and Regulated profiles a #: missing or unknown value means ``each_item``. approval_mode: Optional[str] = None + #: Opt-in phone approval: each waiting save also goes to OpenAdapt Cloud, + #: which pushes an opaque link to the approver's phone. Off by default; + #: a local-only install keeps Desktop approval and sends nothing. + phone_approval: bool = False #: Tier-3 durable runtime: checkpoint each verified step, durably pause on #: halt, resumable via ``resume``. Off by default. durable: bool = False diff --git a/packages/flow/tests/_approver_grant_support.py b/packages/flow/tests/_approver_grant_support.py new file mode 100644 index 000000000..61e47efd1 --- /dev/null +++ b/packages/flow/tests/_approver_grant_support.py @@ -0,0 +1,92 @@ +"""Test helpers: a person-signed approver grant in a temporary key directory. + +Made-up ids only. The signer key is generated per test and registered as a +``policy_signer``, as ``openadapt-flow jit register`` would. +""" + +from __future__ import annotations + +from datetime import datetime, timedelta, timezone +from pathlib import Path +from typing import Optional + +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +from openadapt_flow.approval_mode.approver import ApproverPolicyV1 +from openadapt_flow.approval_mode.item_approval import ItemApprovalStore +from openadapt_flow.approval_mode.signed_policy import ( + ApproverGrant, + SignedApproverGrant, + sign_grant, +) +from openadapt_flow.jit.authority import public_hex, register_principal + +SIGNER = "person-0001" +DELEGATES = ("agent:approver-bot", "system:acme") +_KEYS: dict[Path, Ed25519PrivateKey] = {} + + +def key_dir(tmp_path: Path) -> Path: + return tmp_path / "keys" + + +def signer_key(tmp_path: Path) -> Ed25519PrivateKey: + directory = key_dir(tmp_path) + key = _KEYS.get(directory) + if key is None: + key = Ed25519PrivateKey.generate() + register_principal( + directory, + role="policy_signer", + principal_id=SIGNER, + public_key_hex=public_hex(key), + ) + _KEYS[directory] = key + return key + + +def _iso(when: datetime) -> str: + return when.astimezone(timezone.utc).isoformat().replace("+00:00", "Z") + + +def make_grant( + policy: ApproverPolicyV1, + *, + delegates: tuple[str, ...] = DELEGATES, + workflows: tuple[str, ...] = (), + signed_at: Optional[datetime] = None, + days: int = 30, +) -> ApproverGrant: + start = signed_at or datetime.now(timezone.utc) - timedelta(minutes=1) + return ApproverGrant( + grant_id="grant-0001", + signer=SIGNER, + signed_at=_iso(start), + expires_at=_iso(start + timedelta(days=days)), + approver_policy=policy, + delegates=delegates, + workflows=workflows, + ) + + +def signed_grant( + tmp_path: Path, policy: ApproverPolicyV1, **kwargs +) -> SignedApproverGrant: + return sign_grant(make_grant(policy, **kwargs), signer_key(tmp_path)) + + +def grant_store( + tmp_path: Path, + policy: ApproverPolicyV1, + *, + run: str = "run", + initiator: Optional[str] = None, + cloud_keys: Optional[dict[str, str]] = None, + **kwargs, +) -> ItemApprovalStore: + store = ItemApprovalStore( + tmp_path / run, key_dir=key_dir(tmp_path), cloud_keys=cloud_keys or {} + ) + store.write_grant(signed_grant(tmp_path, policy, **kwargs)) + store.write_initiator(initiator) + return store diff --git a/packages/flow/tests/test_approval_delegated.py b/packages/flow/tests/test_approval_delegated.py index aa39dc316..78bedef0c 100644 --- a/packages/flow/tests/test_approval_delegated.py +++ b/packages/flow/tests/test_approval_delegated.py @@ -41,6 +41,7 @@ wait_for_answer, ) from openadapt_flow.approval_mode.model import ApprovalMode, ItemApprovalRef +from tests._approver_grant_support import grant_store, signed_grant FRAME = b"\x89PNG\r\n\x1a\nmade-up filled form" FORM = ("dom", "sha256:" + "1" * 64) @@ -80,7 +81,7 @@ def _agent_answer(request, store, approver=AGENT, route="local_cli"): approver_id=None, auth_method="none", approver=approver, - policy=store.policy(), + policy=store.context(), ) @@ -106,8 +107,7 @@ def test_the_default_policy_refuses_an_agent(tmp_path): def test_a_policy_that_allows_agents_records_the_agent(tmp_path): - store = ItemApprovalStore(tmp_path / "run") - store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) + store = grant_store(tmp_path, _allow_agents(StepClass.CONSEQUENTIAL)) request = _open(store) answer = _agent_answer(request, store) store.write_answer(answer) @@ -116,33 +116,37 @@ def test_a_policy_that_allows_agents_records_the_agent(tmp_path): assert isinstance(ref, ItemApprovalRef) assert ref.decided_by == "automation" assert ref.route == "local_cli" - assert ref.approver == AGENT + assert ref.approver == AGENT.model_copy( + update={"policy_ref": store.context().grant.policy_ref} + ) assert not is_training_label(answer.approver_identity()) def test_clinical_write_needs_the_opt_in(tmp_path): with pytest.raises(ValueError): _allow_agents(StepClass.CLINICAL_WRITE) # no opt-in: refused outright - store = ItemApprovalStore(tmp_path / "run") - store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) + store = grant_store(tmp_path, _allow_agents(StepClass.CONSEQUENTIAL)) request = _open(store, StepClass.CLINICAL_WRITE) with pytest.raises(ItemApprovalRefused) as err: _agent_answer(request, store) assert err.value.code == "clinical_write_needs_opt_in" - opted = ItemApprovalStore(tmp_path / "run2") - opted.write_policy(_allow_agents(StepClass.CLINICAL_WRITE, opt_in=OPT_IN)) + opted = grant_store( + tmp_path, _allow_agents(StepClass.CLINICAL_WRITE, opt_in=OPT_IN), run="run2" + ) request = _open(opted, StepClass.CLINICAL_WRITE) - assert _agent_answer(request, opted).approver == AGENT + assert _agent_answer(request, opted).approver.id == AGENT.id def test_a_run_keeps_its_first_policy(tmp_path): - store = ItemApprovalStore(tmp_path / "run") - store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) + store = grant_store(tmp_path, _allow_agents(StepClass.CONSEQUENTIAL)) store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) # same: fine with pytest.raises(ItemApprovalRefused) as err: store.write_policy(DEFAULT_APPROVER_POLICY) assert err.value.code == "policy_changed" + with pytest.raises(ItemApprovalRefused) as err: + store.write_grant(signed_grant(tmp_path, _allow_agents(StepClass.IRREVERSIBLE))) + assert err.value.code == "policy_changed" def test_an_unreadable_policy_means_person_only(tmp_path): @@ -153,8 +157,7 @@ def test_an_unreadable_policy_means_person_only(tmp_path): def test_an_agent_cannot_pose_as_a_person(tmp_path): - store = ItemApprovalStore(tmp_path / "run") - store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) + store = grant_store(tmp_path, _allow_agents(StepClass.CONSEQUENTIAL)) request = _open(store) with pytest.raises(ItemApprovalRefused) as err: make_answer( @@ -235,8 +238,7 @@ def sleep(seconds): def test_a_delegate_answers_a_waiting_save(tmp_path): - store = ItemApprovalStore(tmp_path / "run") - store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) + store = grant_store(tmp_path, _allow_agents(StepClass.CONSEQUENTIAL)) request = _open(store) delegate = _Delegate([None, DelegatedDecision(decision="approve")]) monotonic, sleep = _clock() @@ -250,7 +252,8 @@ def test_a_delegate_answers_a_waiting_save(tmp_path): approver=delegate, ) assert code is None and answer is not None - assert answer.decision == "approve" and answer.approver == WEBHOOK_SYSTEM + assert answer.decision == "approve" and answer.approver.id == WEBHOOK_SYSTEM.id + assert answer.approver.policy_ref == store.context().grant.policy_ref # The delegate saw no intent, no screenshot bytes and no values. facts = delegate.seen[0] assert "step_intent" not in facts and FRAME.decode( @@ -277,8 +280,7 @@ def test_a_delegate_the_policy_forbids_is_never_asked(tmp_path): def test_a_failing_delegate_leaves_the_save_to_a_person(tmp_path): - store = ItemApprovalStore(tmp_path / "run") - store.write_policy(_allow_agents(StepClass.CONSEQUENTIAL)) + store = grant_store(tmp_path, _allow_agents(StepClass.CONSEQUENTIAL)) request = _open(store) delegate = _Delegate([WebhookRefused("reply_signature_invalid")]) monotonic, sleep = _clock() diff --git a/packages/flow/tests/test_approval_signed_grant.py b/packages/flow/tests/test_approval_signed_grant.py new file mode 100644 index 000000000..d3b69206a --- /dev/null +++ b/packages/flow/tests/test_approval_signed_grant.py @@ -0,0 +1,495 @@ +"""Signed approver grants, self-approval, and phone decisions. Made-up ids. + +Invariants under test: + +* only a person-signed grant that verifies now lets an agent or system + answer a save; an unsigned, tampered, expired or wrongly signed one means + person-only; +* the agent or system that started a run can't answer that run's saves; +* a phone answer counts only with a Cloud-signed decision for the exact + request digest, from a person signed in with AAL2, verified against a key + pinned on this runner, and it is used once; +* every answer names who decided: a person (and where), or the delegate and + the digest of the grant it acted under. +""" + +from __future__ import annotations + +import base64 +import json +from datetime import datetime, timedelta, timezone + +import pytest +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +from openadapt_flow.approval_mode.approver import ( + ApproverAuth, + ApproverIdentityV1, + ApproverKind, + ApproverPolicyV1, + StepClass, +) +from openadapt_flow.approval_mode.cloud_decision import sign_decision +from openadapt_flow.approval_mode.item_approval import ( + ItemApprovalRefused, + ItemApprovalStore, + apply_cloud_decision, + decided_by_summary, + make_answer, + wait_for_answer, +) +from openadapt_flow.approval_mode.model import ApprovalMode +from openadapt_flow.approval_mode.signed_policy import ( + ApproverGrant, + load_approver_policy, + sign_grant, +) +from openadapt_flow.jit.authority import public_hex, register_principal +from tests._approver_grant_support import ( + SIGNER, + grant_store, + key_dir, + make_grant, + signed_grant, + signer_key, +) + +FRAME = b"\x89PNG\r\n\x1a\nmade-up filled form" +FORM = ("dom", "sha256:" + "1" * 64) +AGENT = ApproverIdentityV1( + kind=ApproverKind.AGENT, auth=ApproverAuth.LOCAL_CLI, id="agent:approver-bot" +) + + +def _policy() -> ApproverPolicyV1: + allowed = {c.value: ["person"] for c in StepClass} + allowed[StepClass.CONSEQUENTIAL.value] = ["person", "agent", "system"] + return ApproverPolicyV1.model_validate({"allowed": allowed}) + + +def _open(store: ItemApprovalStore, workflow: str = "referral"): + return store.open_request( + run_id="run-0001", + workflow_name=workflow, + step_id="save", + step_index=3, + step_intent="Click 'Save'", + mode=ApprovalMode.EACH_ITEM, + bundle_content_digest="a" * 64, + runtime_inputs_digest="b" * 64, + frame_png=FRAME, + form_state=FORM, + ) + + +def _agent_answer(store, request, approver=AGENT): + return make_answer( + request, + decision="approve", + decided_by="automation", + route="local_cli", + approver_id=None, + auth_method="none", + approver=approver, + policy=store.context(), + ) + + +def _refused(store, request, approver=AGENT) -> str: + with pytest.raises(ItemApprovalRefused) as err: + _agent_answer(store, request, approver) + return err.value.code + + +# -- grants ----------------------------------------------------------------------- + + +def test_a_signed_grant_lets_its_named_agent_answer(tmp_path): + store = grant_store(tmp_path, _policy()) + request = _open(store) + answer = _agent_answer(store, request) + store.write_answer(answer) + grant = store.context().grant + assert grant is not None and grant.signer == SIGNER + assert answer.approver.policy_ref == grant.policy_ref + summary = decided_by_summary(store, request) + assert summary["kind"] == "agent" and summary["id"] == AGENT.id + assert summary["policy_ref"] == grant.policy_ref + + +def test_an_unsigned_policy_that_admits_an_agent_is_person_only(tmp_path): + store = ItemApprovalStore(tmp_path / "run", key_dir=key_dir(tmp_path)) + store.write_policy(_policy()) + store.write_initiator(None) + request = _open(store) + assert _refused(store, request) == "approver_policy_unsigned" + + +def test_a_planted_policy_file_gives_no_authority(tmp_path): + store = ItemApprovalStore(tmp_path / "run", key_dir=key_dir(tmp_path)) + store._checked_dir(create=True) + (store.dir / "approver-policy.json").write_text(_policy().model_dump_json()) + store.write_initiator(None) + request = _open(store) + assert store.policy().allowed[StepClass.CONSEQUENTIAL] == (ApproverKind.PERSON,) + assert _refused(store, request) == "approver_policy_unsigned" + + +def test_a_tampered_grant_is_refused(tmp_path): + store = grant_store(tmp_path, _policy()) + request = _open(store) + path = store.dir / "approver-grant.json" + data = json.loads(path.read_text()) + data["grant"]["delegates"] = ["agent:approver-bot", "agent:intruder"] + path.chmod(0o600) + path.unlink() + path.write_text(json.dumps(data)) + assert _refused(store, request) == "approver_grant_signature_invalid" + + +def test_a_grant_swapped_for_another_policy_is_refused(tmp_path): + store = grant_store(tmp_path, _policy()) + request = _open(store) + other = signed_grant( + tmp_path, + ApproverPolicyV1.model_validate( + { + "allowed": { + **{c.value: ["person"] for c in StepClass}, + "irreversible": ["person", "agent"], + } + } + ), + ) + path = store.dir / "approver-grant.json" + path.unlink() + path.write_text(other.model_dump_json(by_alias=True)) + assert _refused(store, request) == "approver_grant_policy_mismatch" + + +def test_an_expired_grant_is_refused(tmp_path): + start = datetime.now(timezone.utc) - timedelta(days=40) + store = grant_store(tmp_path, _policy(), signed_at=start, days=30) + request = _open(store) + assert _refused(store, request) == "approver_grant_expired" + + +def test_a_grant_signed_by_an_unregistered_key_is_refused(tmp_path): + signer_key(tmp_path) # registers the real signer + impostor = Ed25519PrivateKey.generate() + store = ItemApprovalStore(tmp_path / "run", key_dir=key_dir(tmp_path)) + store.write_grant(sign_grant(make_grant(_policy()), impostor)) + store.write_initiator(None) + request = _open(store) + assert _refused(store, request) == "approver_grant_signer_key_mismatch" + + +def test_an_agent_key_cannot_sign_a_grant(tmp_path): + agent_key = Ed25519PrivateKey.generate() + register_principal( + key_dir(tmp_path), + role="agent", + principal_id="agent-signer", + public_key_hex=public_hex(agent_key), + ) + grant = make_grant(_policy()).model_copy(update={"signer": "agent-signer"}) + store = ItemApprovalStore(tmp_path / "run", key_dir=key_dir(tmp_path)) + store.write_grant(sign_grant(grant, agent_key)) + store.write_initiator(None) + request = _open(store) + assert _refused(store, request) == "approver_grant_signer_unregistered" + + +def test_a_grant_must_name_the_delegate_and_the_workflow(tmp_path): + store = grant_store(tmp_path, _policy(), workflows=("intake",)) + other = ApproverIdentityV1( + kind=ApproverKind.AGENT, auth=ApproverAuth.LOCAL_CLI, id="agent:other" + ) + assert _refused(store, _open(store, "intake"), other) == "delegate_not_in_grant" + assert _refused(store, _open(store, "referral")) == "workflow_not_in_grant" + assert _agent_answer(store, _open(store, "intake")).approver.id == AGENT.id + + +def test_an_answer_pinned_to_another_grant_is_refused(tmp_path): + store = grant_store(tmp_path, _policy()) + pinned = AGENT.model_copy(update={"policy_ref": "sha256:" + "e" * 64}) + assert _refused(store, _open(store), pinned) == "policy_ref_mismatch" + + +def test_grant_shape_rules(): + with pytest.raises(ValueError): + make_grant(_policy(), delegates=(SIGNER,)) # signer can't be a delegate + with pytest.raises(ValueError): + make_grant(ApproverPolicyV1()) # a person-only grant grants nothing + with pytest.raises(ValueError): + make_grant(_policy(), days=400) + + +def test_load_approver_policy(tmp_path): + keys = key_dir(tmp_path) + good = tmp_path / "grant.json" + signed_grant(tmp_path, _policy()).save(good) + loaded = load_approver_policy(good, keys) + assert loaded.verified is not None and loaded.refusal is None + unsigned = tmp_path / "policy.json" + unsigned.write_text(_policy().model_dump_json()) + loaded = load_approver_policy(unsigned, keys) + assert loaded.refusal == "approver_policy_unsigned" + assert loaded.policy == ApproverPolicyV1() + person_only = tmp_path / "person.json" + person_only.write_text(ApproverPolicyV1().model_dump_json()) + assert load_approver_policy(person_only, keys).refusal is None + (tmp_path / "bad.json").write_text("{") + assert load_approver_policy(tmp_path / "bad.json", keys).refusal == ( + "approver_grant_unreadable" + ) + + +# -- self-approval ---------------------------------------------------------------- + + +def test_an_agent_cannot_approve_a_run_it_started(tmp_path): + store = grant_store(tmp_path, _policy(), initiator=AGENT.id) + assert _refused(store, _open(store)) == "self_approval" + + +def test_an_agent_cannot_approve_on_behalf_of_the_initiator(tmp_path): + store = grant_store(tmp_path, _policy(), initiator="agent:runner") + proxy = AGENT.model_copy(update={"on_behalf_of": "agent:runner"}) + assert _refused(store, _open(store), proxy) == "self_approval" + + +def test_no_delegate_answers_a_run_an_unnamed_agent_started(tmp_path): + store = grant_store(tmp_path, _policy(), initiator="agent:unidentified") + assert _refused(store, _open(store)) == "self_approval_unprovable" + + +def test_no_delegate_answers_when_the_initiator_was_never_recorded(tmp_path): + store = ItemApprovalStore(tmp_path / "run", key_dir=key_dir(tmp_path)) + store.write_grant(signed_grant(tmp_path, _policy())) + assert _refused(store, _open(store)) == "run_initiator_unknown" + + +def test_a_planted_initiator_refuses_the_run(tmp_path): + store = ItemApprovalStore(tmp_path / "run") + store._checked_dir(create=True) + (store.dir / "run-initiator.json").write_text('{"started_by": null}x') + with pytest.raises(ItemApprovalRefused) as err: + store.write_initiator(None) + assert err.value.code == "initiator_changed" + store2 = ItemApprovalStore(tmp_path / "run2") + store2.write_initiator("agent:a") + store2.write_initiator("agent:a") # same value: fine + with pytest.raises(ItemApprovalRefused): + store2.write_initiator(None) + + +# -- phone decisions -------------------------------------------------------------- + +CLOUD_KEY = Ed25519PrivateKey.generate() +CLOUD_PUB = base64.b64encode( + CLOUD_KEY.public_key().public_bytes( + serialization.Encoding.Raw, serialization.PublicFormat.Raw + ) +).decode() +PINNED = {"cloud-decision-1": CLOUD_PUB} + + +def _decision(request, *, key=CLOUD_KEY, **overrides): + fields = { + "request_id": request.request_id, + "request_sha256": request.digest(), + "decision": "approve", + "approver_id": "cloud-user-0001", + "auth_method": "cloud_aal2", + "decided_at": datetime.now(timezone.utc).isoformat(), + "link_id": "L" * 22, + "runner_id": "runner-0001", + "key_id": "cloud-decision-1", + } + fields.update(overrides) + return sign_decision(fields, key).model_dump(mode="json") + + +def _phone_store(tmp_path, pinned=PINNED): + store = ItemApprovalStore(tmp_path / "run", cloud_keys=pinned) + store.write_initiator(None) + return store + + +def test_a_signed_phone_decision_becomes_the_answer(tmp_path): + store = _phone_store(tmp_path) + request = _open(store) + answer = apply_cloud_decision(store, request, _decision(request)) + assert answer is not None and answer.route == "cloud_phone" + assert store.status(request) == "approved" + ref = answer.reference(request) + assert ref.route == "cloud_phone" and ref.decided_by == "human" + assert ref.approver_id == "cloud-user-0001" and ref.auth_method == "cloud_aal2" + summary = decided_by_summary(store, request) + assert summary["kind"] == "person" and summary["route"] == "cloud_phone" + # Used once: the second save can't reuse it, and a replay writes nothing. + assert store.consume(request.request_id) is True + assert store.consume(request.request_id) is False + assert apply_cloud_decision(store, request, _decision(request)) is None + + +def test_a_phone_decline_carries_its_reason(tmp_path): + store = _phone_store(tmp_path) + request = _open(store) + raw = _decision(request, decision="decline", decline_reason="wrong_values") + answer = apply_cloud_decision(store, request, raw) + assert answer is not None and answer.decline_reason == "wrong_values" + assert store.status(request) == "declined" + + +@pytest.mark.parametrize( + "mutate", + [ + lambda r: {"request_sha256": "sha256:" + "0" * 64}, + lambda r: {"key_id": "unpinned"}, + lambda r: {"auth_method": "cloud_aal1"}, + ], +) +def test_phone_decisions_that_dont_bind_are_refused(tmp_path, mutate): + store = _phone_store(tmp_path) + request = _open(store) + try: + raw = _decision(request, **mutate(request)) + except ValueError: + return # refused at the contract: an AAL1 decision can't even be built + assert apply_cloud_decision(store, request, raw) is None + assert store.status(request) == "waiting" + + +def test_a_decision_for_one_request_cant_answer_another(tmp_path): + store = _phone_store(tmp_path) + first, second = _open(store), _open(store) + raw = _decision(first) + raw_for_second = {**raw, "request_id": second.request_id} + assert apply_cloud_decision(store, second, raw_for_second) is None + assert apply_cloud_decision(store, second, raw) is None + + +def test_a_forged_signature_is_refused(tmp_path): + store = _phone_store(tmp_path) + request = _open(store) + forged = _decision(request, key=Ed25519PrivateKey.generate()) + assert apply_cloud_decision(store, request, forged) is None + + +def test_no_pinned_key_means_no_phone_route(tmp_path): + store = _phone_store(tmp_path, pinned={}) + request = _open(store) + assert apply_cloud_decision(store, request, _decision(request)) is None + + +def test_a_hand_written_phone_answer_is_refused(tmp_path): + store = _phone_store(tmp_path) + request = _open(store) + with pytest.raises(ItemApprovalRefused) as err: + make_answer( + request, + decision="approve", + decided_by="human", + route="cloud_phone", + approver_id="cloud-user-0001", + auth_method="cloud_aal2", + policy=store.context(), + ) + assert err.value.code == "cloud_decision_missing" + + +class _Relay: + runner_id = "runner-0001" + + def __init__(self, decisions): + self.decisions = list(decisions) + self.published: list[bytes] = [] + self.withdrawn: list[str] = [] + + def publish(self, request, frame_png): + self.published.append(frame_png) + return True + + def poll(self, request): + return self.decisions.pop(0) if self.decisions else None + + def withdraw(self, request, reason): + self.withdrawn.append(reason) + + +def _clock(): + state = {"t": 0.0} + return (lambda: state["t"]), (lambda s: state.__setitem__("t", state["t"] + s)) + + +def test_the_run_waits_for_a_phone_decision(tmp_path): + store = _phone_store(tmp_path) + request = _open(store) + relay = _Relay([None, _decision(request)]) + monotonic, sleep = _clock() + answer, code = wait_for_answer( + store, + request, + timeout_s=60, + poll_s=1, + monotonic=monotonic, + sleep=sleep, + phone_relay=relay, + ) + assert code is None and answer.route == "cloud_phone" + assert relay.published == [FRAME] + assert relay.withdrawn == [] # the phone answered; nothing to withdraw + + +def test_a_desktop_answer_wins_and_the_phone_link_is_withdrawn(tmp_path): + store = _phone_store(tmp_path) + request = _open(store) + store.write_answer( + make_answer( + request, + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_id="cloud-user-0002", + auth_method="cloud_aal2", + ) + ) + relay = _Relay([_decision(request)]) + monotonic, sleep = _clock() + answer, code = wait_for_answer( + store, request, timeout_s=5, monotonic=monotonic, sleep=sleep, phone_relay=relay + ) + assert answer.route == "desktop_run_it" + assert relay.withdrawn == ["answered_elsewhere"] + + +def test_a_relay_failure_leaves_desktop_approval(tmp_path): + class Broken(_Relay): + def publish(self, request, frame_png): + raise OSError("offline") + + store = _phone_store(tmp_path) + request = _open(store) + relay = Broken([]) + monotonic, sleep = _clock() + answer, code = wait_for_answer( + store, + request, + timeout_s=3, + poll_s=1, + monotonic=monotonic, + sleep=sleep, + phone_relay=relay, + ) + assert answer is None and code == "approval_wait_timed_out" + assert relay.withdrawn == ["run_stopped_waiting"] + + +def test_unknown_grant_fields_are_refused(): + data = json.loads(make_grant(_policy()).model_dump_json(by_alias=True)) + data["self_enable"] = True + with pytest.raises(ValueError): + ApproverGrant.model_validate(data) From 4d5666f4c5ed19f82bfb1d82658c7fd34efcbf62 Mon Sep 17 00:00:00 2001 From: Richard Abrich <774615+abrichr@users.noreply.github.com> Date: Sat, 17 Oct 2026 22:48:37 -0400 Subject: [PATCH 2/5] chore(flow): refresh the public artifact inventory for the approval-record schema --- packages/flow/public-artifacts.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/flow/public-artifacts.json b/packages/flow/public-artifacts.json index 6ad16517b..5b00de59d 100644 --- a/packages/flow/public-artifacts.json +++ b/packages/flow/public-artifacts.json @@ -1881,7 +1881,7 @@ }, { "path": "openadapt_flow/contracts/schemas/approval-record-v1.json", - "sha256": "abe5c142b2792ce618fbf73b0b3c7327a0fe4d7bb8bf0d96030147292de341da" + "sha256": "0eeacfb80916b6833f577a2f6eb92f7fd4dc54c1144b7d3661dd01acf9258fc2" }, { "path": "openadapt_flow/contracts/schemas/approver-identity-v1.json", From 3b244f9478c862bb6fd087c494c4d97f36909517 Mon Sep 17 00:00:00 2001 From: Richard Abrich <774615+abrichr@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:15:14 -0400 Subject: [PATCH 3/5] refactor(flow): save approvals ride the decision portal's signed task, not a Cloud relay Drop the separate Cloud phone relay (phone_relay, cloud_decision, --phone-approval, runtime.phone_approval, item-approval pin-cloud-keys and apply-cloud-decision). A waiting save is now a HumanDecisionTaskV1 with task_kind save_approval / question approve_save, served by the attended console the decision portal already relays to: GET /api/save-approvals GET /api/save-approvals/{request_id} signed task + local presentation GET /api/save-approvals/{request_id}/frame exact screenshot, digest-checked POST /api/save-approvals/{request_id}/actions/{verify_and_resume|reject} The task's capability_digest is the request digest (run, step, screenshot, form state). The phone echoes the task digest and signature; the answer is written once on the new decision_portal person route, and the run still re-reads the form before saving. The hosted lane does not carry save approvals: Cloud never receives the screenshot. --- .../flow/docs/APPROVAL_AND_VERIFICATION.md | 61 +-- packages/flow/docs/APPROVAL_MODES.md | 4 +- packages/flow/openadapt_flow/__main__.py | 136 +----- .../approval_mode/cloud_decision.py | 195 --------- .../flow/openadapt_flow/approval_mode/gate.py | 5 - .../approval_mode/item_approval.py | 239 ++--------- .../openadapt_flow/approval_mode/model.py | 20 +- .../approval_mode/phone_relay.py | 172 -------- packages/flow/openadapt_flow/console/app.py | 49 +++ .../openadapt_flow/console/save_approvals.py | 386 ++++++++++++++++++ .../flow/openadapt_flow/contracts/approval.py | 2 +- .../contracts/human_decision.py | 4 + .../contracts/schemas/approval-record-v1.json | 2 +- .../contracts/schemas/execute-v1-openapi.json | 6 +- .../schemas/human-decision-task-v1.json | 6 +- .../schemas/human-decision-task-v2.json | 6 +- packages/flow/openadapt_flow/deployment.py | 4 - packages/flow/public-artifacts.json | 8 +- .../flow/tests/_approver_grant_support.py | 5 +- .../flow/tests/test_approval_signed_grant.py | 213 +--------- .../flow/tests/test_console_save_approvals.py | 205 ++++++++++ 21 files changed, 752 insertions(+), 976 deletions(-) delete mode 100644 packages/flow/openadapt_flow/approval_mode/cloud_decision.py delete mode 100644 packages/flow/openadapt_flow/approval_mode/phone_relay.py create mode 100644 packages/flow/openadapt_flow/console/save_approvals.py create mode 100644 packages/flow/tests/test_console_save_approvals.py diff --git a/packages/flow/docs/APPROVAL_AND_VERIFICATION.md b/packages/flow/docs/APPROVAL_AND_VERIFICATION.md index 378bce4f9..cde7042de 100644 --- a/packages/flow/docs/APPROVAL_AND_VERIFICATION.md +++ b/packages/flow/docs/APPROVAL_AND_VERIFICATION.md @@ -125,37 +125,48 @@ A delegated approver answers by one of three routes: The run re-checks every answer: the grant must verify, name the delegate and cover the workflow, its policy must allow the approver's kind for the step class, the delegate can't have started the run, a person answers only -from Desktop or their phone (below), and a delegate never fills in the -person fields. The report's `ItemApprovalRef` records `decided_by: +from Desktop or the decision portal on their phone (below), and a delegate +never fills in the person fields. The report's `ItemApprovalRef` records `decided_by: automation`, the route and the approver, whose `policy_ref` is the grant's digest. `item-approval show` prints each request's `decided_by`: a person -(Cloud user id and route) or the delegate's id and the grant digest, which -Desktop and Cloud show on the run. Only a person's decision becomes a +(Cloud user id, or none, and route) or the delegate's id and the grant +digest, which Desktop and Cloud show on the run. Only a person's decision becomes a training label. Triage never approves anything. ### A person approves from their phone -Phone approval is opt-in (`runtime.phone_approval: true` or -`run --phone-approval`), and local-only installs never use it. An owner or -admin also turns on **Approve saves from your phone** on Cloud's Needs -attention page; until then Cloud refuses the save and it waits in Desktop as -before. With both on, -each waiting save also goes to Cloud through the runner's own outbound -connection. Cloud pushes a notification with an opaque id and fixed words -(never a value, a name or the workflow's name) and, if the person chose, -an email with a single-use link. The link opens Cloud's approve page; after -sign-in with AAL2 it shows exactly the filled form Desktop would show, and -**Approve and save** or **Don't save**. - -Cloud signs the decision (`openadapt.cloud-item-decision/v1`) for that exact -request digest, which binds the run, the save step, the screenshot and the -form's field state. The run accepts it only when it verifies against the -decision key pinned on this runner (`item-approval pin-cloud-keys`), the -approver signed in with AAL2, and nothing else answered first. The answer is -written once and used for exactly one save, so a replayed link or decision -has nothing to answer. Then the run re-checks the form before the save, as -for a Desktop approval, and stops if anything changed while it waited. A -Desktop answer that lands first wins, and the run withdraws the phone link. +A save waiting for a person also shows on the decision portal, the phone +page Desktop serves from this runner (see Desktop's `docs/DECISION_PORTAL.md` +for pairing and network setup). Nothing new leaves the runner: the phone +talks to the runner's own console, as it already does for paused runs, and a +local-only or no-outbound install works the same way. + +The console lists waiting saves at `GET /api/save-approvals`. Each one is a +signed `openadapt.human-decision-task/v1` task with `task_kind: +save_approval` and question `approve_save`. The task is PHI-free like every +decision task: opaque digests and closed codes, no value, no name, no +workflow name. Its `capability_digest` is the request digest, which binds +the run, the save step, the screenshot and the form's field state. Next to +the task, the console returns a local-only presentation block with the +step's wording and the route of the exact screenshot the request was opened +with (`GET /api/save-approvals/ID/frame`, checked against its digest), so the +person sees exactly what will be saved. + +The phone answers with `POST /api/save-approvals/ID/actions/verify_and_resume` +(approve) or `.../actions/reject` with one closed `decline_reason`, echoing +the task's digest and signature. Only the attended console the portal relays +to takes an answer. A task that changed, or belongs to another save, is +refused. The answer is written once, as a person on the `decision_portal` +route, so a second tap or a replay has nothing to answer, and a Desktop +answer that lands first wins. A paired phone proves a device, not who held +it, so the answer records an unauthenticated person, like a signed-out +Desktop answer. The receipt says `accepted_pending_runner`: then the run +re-checks the form before the save, as for a Desktop approval, and stops if +anything changed while it waited. + +The hosted lane (Cloud's Needs attention page) does not carry save +approvals. Cloud never receives the screenshot, and a person must see +exactly what will be saved before approving it. ## How a save is checked diff --git a/packages/flow/docs/APPROVAL_MODES.md b/packages/flow/docs/APPROVAL_MODES.md index 7f4e864d5..fb248f52a 100644 --- a/packages/flow/docs/APPROVAL_MODES.md +++ b/packages/flow/docs/APPROVAL_MODES.md @@ -51,9 +51,9 @@ openadapt-flow item-approval decline RUN_DIR ... --reason wrong_values - An answer names the request digest the person saw. A person answers with `--decided-by human` from the `desktop_run_it` route, or from their phone - through Cloud (`cloud_phone`, opt-in; see + on the runner's decision portal (`decision_portal`; see [`APPROVAL_AND_VERIFICATION.md`](APPROVAL_AND_VERIFICATION.md)), which - needs a Cloud-signed decision for that exact request. A decline carries + echoes the signed task for that exact request. A decline carries one closed reason: `wrong_values`, `wrong_record` or `screen_problem`. - Answers are created once and never replaced. The run checks every answer itself, so an answer file written around the command is held to the same diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index dd37ccb0e..6f6b40465 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -2057,7 +2057,6 @@ def _cmd_replay(args: argparse.Namespace) -> int: args, run_dir, policy_name=getattr(args, "policy", None) or cfg.policy.policy, - cfg=cfg, ) fault_driver = getattr(args, "_qualification_fault_driver", None) if _normalize_kind(backend_cfg.kind) != "web": @@ -2343,40 +2342,6 @@ def _approver_settings(args: argparse.Namespace): return loaded, approver -def _phone_relay_for_run(args: argparse.Namespace, cfg=None): - """The opt-in phone approval relay, or ``None`` (Desktop approval only). - - On only with ``--phone-approval`` or ``runtime.phone_approval: true``. - Asking for it without this runner's Cloud pairing, or without a pinned - Cloud decision key, refuses the run rather than silently dropping it. - """ - - wanted = bool(getattr(args, "phone_approval", False)) or bool( - getattr(getattr(cfg, "runtime", None), "phone_approval", False) - ) - if not wanted: - return None - from openadapt_flow.approval_mode.cloud_decision import ( - default_keys_path, - load_pinned_keys, - ) - from openadapt_flow.approval_mode.phone_relay import ( - PhoneRelayError, - relay_from_env, - ) - - if not load_pinned_keys(): - raise SystemExit( - "run REFUSED: phone approval needs Cloud's decision key pinned on " - f"this runner ({default_keys_path()}); run " - "`openadapt-flow item-approval pin-cloud-keys`. Nothing was executed." - ) - try: - return relay_from_env() - except PhoneRelayError as exc: - raise SystemExit(f"run REFUSED: {exc}. Nothing was executed.") from None - - def _prepare_approval_store( args: argparse.Namespace, run_dir: Path, *, record_initiator: bool ): @@ -2408,7 +2373,6 @@ def _approval_gate_for_run( run_dir: Path, *, policy_name: Optional[str] = None, - cfg=None, ): """Build the run's approval gate from the mode ``run`` resolved, if any.""" @@ -2424,7 +2388,6 @@ def _approval_gate_for_run( approval_wait_s=_approval_wait_seconds(args), approver=approver, policy_name=policy_name, - phone_relay=_phone_relay_for_run(args, cfg), ) @@ -3057,7 +3020,6 @@ def _resume_approval_gate( run_dir: Path, *, policy_name: Optional[str] = None, - cfg=None, ): if mode is None: return None @@ -3072,7 +3034,6 @@ def _resume_approval_gate( approval_wait_s=_approval_wait_seconds(args), approver=approver, policy_name=policy_name, - phone_relay=_phone_relay_for_run(args, cfg), ) @@ -3348,7 +3309,7 @@ def _resume_with(backend: "Backend") -> "RunReport": runtime_config=cfg.runtime, checkpoint_key=ckpt_key, approval_gate=_resume_approval_gate( - args, resume_mode, run_dir, policy_name=cfg.policy.policy, cfg=cfg + args, resume_mode, run_dir, policy_name=cfg.policy.policy ), ) return resume( @@ -3565,12 +3526,8 @@ def _cmd_item_approval(args: argparse.Namespace) -> int: ) verb = args.item_approval_cmd - if verb == "pin-cloud-keys": - return _item_approval_pin_cloud_keys(args) run_dir = Path(args.run_dir) store = ItemApprovalStore(run_dir) - if verb == "apply-cloud-decision": - return _item_approval_apply_cloud_decision(args, store) if verb == "show": rows = [request_summary(store, request) for request in store.requests()] if not args.all: @@ -3680,68 +3637,6 @@ def refuse(code: str, message: str) -> int: return 0 -def _item_approval_pin_cloud_keys(args: argparse.Namespace) -> int: - """Pin Cloud's phone-decision keys on this runner (once, at pairing).""" - import json - import os - - from openadapt_flow.approval_mode.cloud_decision import default_keys_path - from openadapt_flow.approval_mode.phone_relay import ( - PhoneRelayError, - relay_from_env, - ) - - target = Path(args.out) if args.out else default_keys_path() - try: - keys = relay_from_env().decision_keys() - except (PhoneRelayError, OSError, ValueError) as exc: - print(json.dumps({"ok": False, "code": "pin_failed", "message": str(exc)})) - return 2 - target.parent.mkdir(parents=True, exist_ok=True) - tmp = target.with_suffix(".tmp") - fd = os.open(tmp, os.O_WRONLY | os.O_CREAT | os.O_TRUNC, 0o600) - with os.fdopen(fd, "w", encoding="utf-8") as handle: - handle.write(json.dumps(keys, indent=2, sort_keys=True) + "\n") - os.replace(tmp, target) - print(json.dumps({"ok": True, "path": str(target), "key_ids": sorted(keys)})) - return 0 - - -def _item_approval_apply_cloud_decision(args: argparse.Namespace, store) -> int: - """Apply a Cloud-signed phone decision to the request it names.""" - import json - - from openadapt_flow.approval_mode.item_approval import ( - ItemApprovalRefused, - apply_cloud_decision, - ) - - try: - raw = json.loads(Path(args.decision_file).read_text(encoding="utf-8")) - request_id = str(raw.get("request_id", "")) if isinstance(raw, dict) else "" - request = store.read_request(request_id) - except (OSError, ValueError, ItemApprovalRefused): - print(json.dumps({"ok": False, "code": "request_unknown"})) - return 2 - answer = apply_cloud_decision(store, request, raw) - if answer is None: - print(json.dumps({"ok": False, "code": "cloud_decision_refused"})) - return 2 - resume_required = _answer_durable_item_pause(Path(args.run_dir), request, answer) - print( - json.dumps( - { - "ok": True, - "request_id": request.request_id, - "decision": answer.decision, - "answer_sha256": answer.digest(), - "resume_required": resume_required, - } - ) - ) - return 0 - - def _cmd_approver_grant(args: argparse.Namespace) -> int: """Sign or check a person's approver grant (who may answer saves).""" import json @@ -6599,18 +6494,6 @@ def _add_approval_flags(p: argparse.ArgumentParser) -> None: "never answer this run's saves. openadapt-agent sets it." ), ) - p.add_argument( - "--phone-approval", - action="store_true", - default=False, - help=( - "Also send each waiting save to OpenAdapt Cloud so a person can " - "approve it from their phone (opt-in; same as " - "runtime.phone_approval). Needs this runner's Cloud pairing " - "(OPENADAPT_RUNNER_TOKEN, OPENADAPT_RUNNER_ID) and Cloud's " - "decision key pinned (item-approval pin-cloud-keys)." - ), - ) p.add_argument( "--approver-webhook", default=None, @@ -8089,23 +7972,6 @@ def build_parser() -> argparse.ArgumentParser: choices=["wrong_values", "wrong_record", "screen_problem"], ) q.set_defaults(func=_cmd_item_approval) - q = item_sub.add_parser( - "apply-cloud-decision", - help=( - "Apply a Cloud-signed decision a person made on their phone. It " - "must name this run's waiting request and verify against the " - "pinned Cloud decision key" - ), - ) - q.add_argument("run_dir", help="The run directory") - q.add_argument("--decision-file", required=True, metavar="JSON") - q.set_defaults(func=_cmd_item_approval) - q = item_sub.add_parser( - "pin-cloud-keys", - help="Pin OpenAdapt Cloud's phone-decision keys on this runner", - ) - q.add_argument("--out", default=None, help="Where to write the pinned keys") - q.set_defaults(func=_cmd_item_approval) p = sub.add_parser( "approver-grant", diff --git a/packages/flow/openadapt_flow/approval_mode/cloud_decision.py b/packages/flow/openadapt_flow/approval_mode/cloud_decision.py deleted file mode 100644 index 287a95cae..000000000 --- a/packages/flow/openadapt_flow/approval_mode/cloud_decision.py +++ /dev/null @@ -1,195 +0,0 @@ -"""A person's save decision made on their phone, signed by Cloud. - -Phone approval is opt-in. When a deployment turns it on, the runner sends -each waiting save to Cloud; Cloud pushes an opaque link to the approver's -phone, the person signs in (AAL2), sees exactly what will be saved, and taps -Approve or Don't save. Cloud signs that decision with its decision key, and -the runner hands it to the waiting run. - -This module is the run's half. A Cloud decision is accepted only when: - -* it is signed by a key pinned on this runner (key id -> base64 raw Ed25519 - public key in ``$OPENADAPT_CLOUD_DECISION_KEYS``, default - ``$OPENADAPT_HOME/cloud-decision-keys.json``). No pinned key, no phone - route: local-only installs keep Desktop approval and nothing else; -* it names the exact request id and request digest (which binds the run, - the save step, the screenshot and the form's field state) the person saw; -* the approver is a Cloud user who signed in with AAL2. An agent or a model - can never produce one: Cloud signs only for a person's session; -* its ``link_id`` is single-use: the answer file is created once, and the - approval is consumed by exactly one save, so a replayed decision has - nothing to answer. - -After an approval the run still re-reads the form before the save and stops -if anything changed while it waited, as for a Desktop approval. -""" - -from __future__ import annotations - -import base64 -import binascii -import json -import os -from pathlib import Path -from typing import Any, Literal, Mapping, Optional - -from pydantic import BaseModel, ConfigDict, Field, ValidationError, model_validator - -from openadapt_flow.approval_mode.model import ( - OPAQUE_ID_PATTERN, - PREFIXED_SHA256_PATTERN, - DeclineReason, -) - -CLOUD_DECISION_SCHEMA: Literal["openadapt.cloud-item-decision/v1"] = ( - "openadapt.cloud-item-decision/v1" -) -_DOMAIN = b"openadapt.cloud-item-decision/v1\n" -CLOUD_PHONE_ROUTE: Literal["cloud_phone"] = "cloud_phone" -KEYS_ENV = "OPENADAPT_CLOUD_DECISION_KEYS" - - -def _canonical(value: Any) -> bytes: - return json.dumps( - value, sort_keys=True, separators=(",", ":"), ensure_ascii=False - ).encode("utf-8") - - -class CloudDecisionV1(BaseModel): - """What Cloud signs when a person approves or declines on their phone.""" - - model_config = ConfigDict(extra="forbid", frozen=True) - - schema_version: Literal["openadapt.cloud-item-decision/v1"] = CLOUD_DECISION_SCHEMA - request_id: str = Field(pattern=r"^[0-9a-f]{32}$") - request_sha256: str = Field(pattern=PREFIXED_SHA256_PATTERN) - decision: Literal["approve", "decline"] - decline_reason: Optional[DeclineReason] = None - #: The Cloud user id of the person who decided. Never a name. - approver_id: str = Field(pattern=OPAQUE_ID_PATTERN) - auth_method: Literal["cloud_aal2"] - decided_at: str = Field(min_length=10, max_length=64) - #: The single-use link the person opened. - link_id: str = Field(pattern=r"^[A-Za-z0-9_-]{16,64}$") - runner_id: str = Field(pattern=OPAQUE_ID_PATTERN) - key_id: str = Field(pattern=r"^[A-Za-z0-9._-]{1,64}$") - signature: str = Field(min_length=86, max_length=100) - - @model_validator(mode="after") - def _reason(self) -> "CloudDecisionV1": - if (self.decision == "decline") != (self.decline_reason is not None): - raise ValueError("a decline carries exactly one closed reason") - return self - - -class CloudDecisionRejected(ValueError): - def __init__(self, code: str, message: str) -> None: - super().__init__(message) - self.code = code - - -def signing_payload(fields: Mapping[str, Any]) -> bytes: - """The exact bytes Cloud signs: domain prefix plus canonical JSON.""" - - unsigned = {k: v for k, v in fields.items() if k != "signature"} - return _DOMAIN + _canonical(unsigned) - - -def sign_decision(fields: Mapping[str, Any], private_key: Any) -> CloudDecisionV1: - """Reference signer (Cloud's signer must match it byte for byte).""" - - unsigned = {k: v for k, v in dict(fields).items() if k != "signature"} - placeholder = CloudDecisionV1.model_validate( - {**unsigned, "signature": "A" * 86 + "=="} - ) - body = placeholder.model_dump(mode="json") - signature = private_key.sign(signing_payload(body)) - return CloudDecisionV1.model_validate( - {**body, "signature": base64.b64encode(signature).decode("ascii")} - ) - - -def default_keys_path() -> Path: - explicit = os.environ.get(KEYS_ENV) - if explicit: - return Path(explicit) - home = Path(os.environ.get("OPENADAPT_HOME") or Path.home() / ".openadapt") - return home / "cloud-decision-keys.json" - - -def load_pinned_keys(path: Optional[Path | str] = None) -> dict[str, str]: - """The runner's pinned Cloud decision keys. Empty when none are pinned.""" - - target = Path(path) if path is not None else default_keys_path() - try: - data = json.loads(target.read_text(encoding="utf-8")) - except (OSError, ValueError): - return {} - if not isinstance(data, dict): - return {} - return {str(k): str(v) for k, v in data.items() if isinstance(v, str)} - - -def verify_decision( - raw: Any, - *, - request_id: str, - request_sha256: str, - pinned_keys: Mapping[str, str], -) -> CloudDecisionV1: - """Accept a Cloud decision only for this exact request, signed by a pinned key.""" - - from cryptography.exceptions import InvalidSignature - from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PublicKey - - try: - if isinstance(raw, CloudDecisionV1): - decision = raw - elif isinstance(raw, (bytes, str)): - decision = CloudDecisionV1.model_validate_json(raw) - else: - decision = CloudDecisionV1.model_validate(raw) - except ValidationError as exc: - raise CloudDecisionRejected( - "cloud_decision_malformed", "malformed decision" - ) from exc - encoded = pinned_keys.get(decision.key_id) - if not encoded: - raise CloudDecisionRejected( - "cloud_decision_key_not_pinned", - "the decision key is not pinned on this runner", - ) - try: - Ed25519PublicKey.from_public_bytes( - base64.b64decode(encoded, validate=True) - ).verify( - base64.b64decode(decision.signature, validate=True), - signing_payload(decision.model_dump(mode="json")), - ) - except (InvalidSignature, ValueError, TypeError, binascii.Error) as exc: - raise CloudDecisionRejected( - "cloud_decision_signature_invalid", "the decision signature is invalid" - ) from exc - if decision.request_id != request_id: - raise CloudDecisionRejected( - "request_mismatch", "the decision is for another request" - ) - if decision.request_sha256 != request_sha256: - raise CloudDecisionRejected( - "request_digest_mismatch", - "the decision is for another version of this save", - ) - return decision - - -__all__ = [ - "CLOUD_DECISION_SCHEMA", - "CLOUD_PHONE_ROUTE", - "CloudDecisionRejected", - "CloudDecisionV1", - "KEYS_ENV", - "load_pinned_keys", - "sign_decision", - "signing_payload", - "verify_decision", -] diff --git a/packages/flow/openadapt_flow/approval_mode/gate.py b/packages/flow/openadapt_flow/approval_mode/gate.py index b932edbc0..1a2c59558 100644 --- a/packages/flow/openadapt_flow/approval_mode/gate.py +++ b/packages/flow/openadapt_flow/approval_mode/gate.py @@ -87,7 +87,6 @@ ) if TYPE_CHECKING: - from openadapt_flow.approval_mode.phone_relay import PhoneRelay from openadapt_flow.ir import RunReport, Step, StepResult, Workflow SignedInVerdict = Literal["match", "mismatch", "unreadable"] @@ -262,7 +261,6 @@ def __init__( approver_policy: Optional["ApproverPolicyV1"] = None, approver: Optional["Approver"] = None, policy_name: Optional[str] = None, - phone_relay: Optional["PhoneRelay"] = None, ) -> None: self.mode = resolve_approval_mode(mode) if approval_store is None or isinstance(approval_store, ItemApprovalStore): @@ -273,8 +271,6 @@ def __init__( # the run's store records the policy so every reader applies the same. self.approver_policy = approver_policy self.approver = approver - #: Opt-in phone approval through Cloud. ``None``: Desktop only. - self.phone_relay = phone_relay self._policy_name = policy_name if approver_policy is not None and self.approval_store is not None: self.approval_store.write_policy(approver_policy) @@ -625,7 +621,6 @@ def wait_refusal(detail: str) -> GateVerdict: monotonic=self._monotonic, sleep=self._sleep, approver=self.approver, - phone_relay=self.phone_relay, ) if waited is None: return wait_refusal(code or "approval_wait_timed_out") diff --git a/packages/flow/openadapt_flow/approval_mode/item_approval.py b/packages/flow/openadapt_flow/approval_mode/item_approval.py index fad3108b3..062893ba5 100644 --- a/packages/flow/openadapt_flow/approval_mode/item_approval.py +++ b/packages/flow/openadapt_flow/approval_mode/item_approval.py @@ -12,8 +12,9 @@ - ``item-approval/answer-.json`` (:class:`ItemApprovalAnswer`), written once by ``openadapt-flow item-approval approve|decline``. It names the request digest the person saw, who decided (``decided_by`` must be - ``human``), the route (only Desktop Run it) and the approver (a Cloud user - id, or ``unauthenticated`` when Desktop was signed out). + ``human`` for a person), the route (Desktop Run it, or the decision + portal on a paired phone) and the approver (a Cloud user id, or + ``unauthenticated`` when nobody was signed in). - ``item-approval/consumed-``, created once by the run when it uses an approval for exactly one save. An approval is never used twice. @@ -31,8 +32,8 @@ import stat import time import uuid -from collections.abc import Callable, Mapping -from dataclasses import dataclass, field +from collections.abc import Callable +from dataclasses import dataclass from datetime import datetime, timedelta, timezone from pathlib import Path from typing import Any, Literal, Optional @@ -55,14 +56,8 @@ StepClass, WebhookRefused, ) -from openadapt_flow.approval_mode.cloud_decision import ( - CLOUD_PHONE_ROUTE, - CloudDecisionRejected, - CloudDecisionV1, - load_pinned_keys, - verify_decision, -) from openadapt_flow.approval_mode.model import ( + DECISION_PORTAL_ROUTE, ITEM_APPROVAL_ROUTE, OPAQUE_ID_PATTERN, PREFIXED_SHA256_PATTERN, @@ -71,7 +66,6 @@ DeclineReason, ItemApprovalRef, ) -from openadapt_flow.approval_mode.phone_relay import PHONE_POLL_S, PhoneRelay from openadapt_flow.approval_mode.signed_policy import ( GrantRefused, SignedApproverGrant, @@ -99,8 +93,9 @@ INITIATOR_FILENAME = "run-initiator.json" #: The initiator an agent-started run records when the agent has no id. UNIDENTIFIED_AGENT = "agent:unidentified" -#: Routes a person answers by: Desktop's Run it, or their phone through Cloud. -PERSON_ROUTES = frozenset({ITEM_APPROVAL_ROUTE, CLOUD_PHONE_ROUTE}) +#: Routes a person answers by: Desktop's Run it, or their phone through the +#: runner's decision portal. +PERSON_ROUTES = frozenset({ITEM_APPROVAL_ROUTE, DECISION_PORTAL_ROUTE}) #: How long a run waits for an answer before it stops before the save. DEFAULT_APPROVAL_WAIT_S = 900.0 MAX_APPROVAL_WAIT_S = 4 * 3600.0 @@ -220,16 +215,12 @@ class ItemApprovalAnswer(BaseModel): #: for a person, whose older fields above already name them. Left out of #: the file when absent, so earlier answers keep their digest. approver: Optional[ApproverIdentityV1] = None - #: The Cloud-signed decision behind a phone answer (``cloud_phone``). - cloud_decision: Optional[CloudDecisionV1] = None @model_serializer(mode="wrap") def _serialize_compatible(self, handler: Any) -> dict[str, Any]: data: dict[str, Any] = handler(self) if self.approver is None: data.pop("approver", None) - if self.cloud_decision is None: - data.pop("cloud_decision", None) return data def digest(self) -> str: @@ -273,8 +264,7 @@ class ApproverContext: ``policy`` is person-only unless ``grant`` is a person-signed grant that verified just now. ``initiator`` is who started the run (``None`` for a person or operator); ``initiator_known`` is False when the run never - recorded it, and then no delegate may answer. ``cloud_keys`` are the - runner's pinned Cloud decision keys; empty means no phone route. + recorded it, and then no delegate may answer. """ policy: ApproverPolicyV1 = DEFAULT_APPROVER_POLICY @@ -282,7 +272,6 @@ class ApproverContext: grant_refusal: Optional[str] = None initiator: Optional[str] = None initiator_known: bool = False - cloud_keys: Mapping[str, str] = field(default_factory=dict) def _as_context( @@ -329,34 +318,6 @@ def _delegate_refusal( return None -def _cloud_phone_refusal( - request: ItemApprovalRequest, - answer: ItemApprovalAnswer, - context: ApproverContext, -) -> Optional[str]: - decision = answer.cloud_decision - if decision is None: - return "cloud_decision_missing" - try: - verify_decision( - decision, - request_id=request.request_id, - request_sha256=request.digest(), - pinned_keys=context.cloud_keys, - ) - except CloudDecisionRejected as exc: - return exc.code - if ( - answer.approver_kind != "cloud_user" - or answer.approver_id != decision.approver_id - or answer.auth_method != "cloud_aal2" - or answer.decision != decision.decision - or answer.decline_reason != decision.decline_reason - ): - return "answer_fields_inconsistent" - return None - - def answer_refusal( request: ItemApprovalRequest, answer: ItemApprovalAnswer, @@ -369,8 +330,8 @@ def answer_refusal( The run calls this itself; it never trusts that the CLI checked. ``policy`` is the run's :class:`ApproverContext` (``store.context()``); a bare policy or ``None`` means no delegate may answer. A person answers - through Desktop's Run it, or on their phone through Cloud with a - Cloud-signed decision. An agent or system answers only under a verified, + through Desktop's Run it, or on their phone through the runner's + decision portal. An agent or system answers only under a verified, person-signed grant that names it, allows its kind for the step class, and wasn't started by it. """ @@ -388,12 +349,6 @@ def answer_refusal( return "route_not_allowed" if approver is not None and approver.auth.value != answer.approver_kind: return "answer_fields_inconsistent" - if answer.route == CLOUD_PHONE_ROUTE: - refusal = _cloud_phone_refusal(request, answer, context) - if refusal is not None: - return refusal - elif answer.cloud_decision is not None: - return "answer_fields_inconsistent" else: if answer.decided_by != "automation": return "answer_fields_inconsistent" @@ -402,8 +357,6 @@ def answer_refusal( if answer.approver_kind != "unauthenticated" or answer.approver_id is not None: # The older person fields stay empty; ``approver`` names who decided. return "answer_fields_inconsistent" - if answer.cloud_decision is not None: - return "answer_fields_inconsistent" refusal_code = _delegate_refusal(request, approver, context) if refusal_code is not None: return refusal_code @@ -511,15 +464,12 @@ def __init__( run_dir: Path | str, *, key_dir: Optional[Path | str] = None, - cloud_keys: Optional[Mapping[str, str]] = None, ) -> None: self.run_dir = Path(run_dir) self.dir = self.run_dir / APPROVAL_DIRNAME #: Where the signer registry lives (``$OPENADAPT_JIT_KEY_DIR`` or #: ``~/.openadapt/jit`` when ``None``). Keep it away from the agents. self._key_dir = Path(key_dir) if key_dir is not None else None - #: Pinned Cloud decision keys; ``None`` reads the runner's file. - self._cloud_keys = dict(cloud_keys) if cloud_keys is not None else None # -- layout -------------------------------------------------------------- def _checked_dir(self, *, create: bool) -> Path: @@ -635,16 +585,11 @@ def context(self, *, now: Optional[datetime] = None) -> ApproverContext: """ known, initiator = self._read_initiator() - if self._cloud_keys is None: - cloud_keys = load_pinned_keys() - else: - cloud_keys = dict(self._cloud_keys) def make(**fields: Any) -> ApproverContext: return ApproverContext( initiator=initiator, initiator_known=known, - cloud_keys=cloud_keys, **fields, ) @@ -743,6 +688,27 @@ def read_request(self, request_id: str) -> ItemApprovalRequest: raise ItemApprovalRefused("request_unknown", "request id mismatch") return request + def read_frame(self, request: ItemApprovalRequest) -> bytes: + """The screenshot the person is shown, checked against its digest. + + Raises :class:`ItemApprovalRefused` when it is missing, too large, or + isn't the exact image the request was opened with. + """ + + try: + frame = _read_regular( + self.run_dir / request.screenshot, limit=_MAX_FRAME_BYTES + ) + except OSError as exc: + raise ItemApprovalRefused( + "frame_unreadable", "the screenshot is missing or unreadable" + ) from exc + if hashlib.sha256(frame).hexdigest() != request.screenshot_sha256: + raise ItemApprovalRefused( + "frame_digest_mismatch", "the screenshot isn't the one requested" + ) + return frame + def requests(self) -> list[ItemApprovalRequest]: """Every readable request in this run, oldest first.""" @@ -904,7 +870,6 @@ def wait_for_answer( monotonic: Callable[[], float] = time.monotonic, sleep: Callable[[float], None] = time.sleep, approver: Optional[Approver] = None, - phone_relay: Optional["PhoneRelay"] = None, ) -> tuple[Optional[ItemApprovalAnswer], Optional[str]]: """Wait for a usable answer. Returns ``(answer, None)`` or ``(None, code)``. @@ -912,39 +877,12 @@ def wait_for_answer( the closed reason an answer was refused. A refused answer is final for this request: the run stops before the save rather than waiting on. - With ``phone_relay`` (opt-in phone approval) the request also goes to - Cloud, and a Cloud-signed phone decision for this exact request becomes a - ``cloud_phone`` answer. Whichever answer is written first wins. + The answer can come from Desktop's Run it, from the person's phone + through the decision portal (:mod:`openadapt_flow.console.save_approvals` + writes it), or from a delegate the run's signed grant allows. Whichever + answer is written first wins; the file is created exclusively. """ - try: - return _wait_for_answer( - store, - request, - timeout_s=timeout_s, - poll_s=poll_s, - monotonic=monotonic, - sleep=sleep, - approver=approver, - phone_relay=phone_relay, - ) - finally: - if phone_relay is not None: - _withdraw(store, request, phone_relay) - - -def _wait_for_answer( - store: ItemApprovalStore, - request: ItemApprovalRequest, - *, - timeout_s: float, - poll_s: float, - monotonic: Callable[[], float], - sleep: Callable[[float], None], - approver: Optional[Approver], - phone_relay: Optional["PhoneRelay"], -) -> tuple[Optional[ItemApprovalAnswer], Optional[str]]: - deadline = monotonic() + max(0.0, min(timeout_s, MAX_APPROVAL_WAIT_S)) context = store.context() delegate = approver @@ -952,18 +890,11 @@ def _wait_for_answer( # The grant doesn't let this approver decide this step: ask a person. delegate = None next_ask = 0.0 - relay = phone_relay - if relay is not None and not _publish(store, request, relay): - relay = None # Cloud didn't take it: Desktop still can. - next_phone = 0.0 while True: try: answer = store.read_answer(request.request_id) except ItemApprovalRefused as exc: return None, exc.code - if answer is None and relay is not None and monotonic() >= next_phone: - next_phone = monotonic() + PHONE_POLL_S - answer = _take_phone_decision(store, request, relay) if answer is None and delegate is not None and monotonic() >= next_ask: next_ask = monotonic() + DELEGATE_POLL_S answer = _ask_delegate(store, request, delegate) @@ -981,94 +912,6 @@ def _wait_for_answer( sleep(poll_s) -def _publish( - store: ItemApprovalStore, request: ItemApprovalRequest, relay: "PhoneRelay" -) -> bool: - try: - frame = _read_regular( - store.run_dir / request.screenshot, limit=_MAX_FRAME_BYTES - ) - return bool(relay.publish(request, frame)) - except Exception: # noqa: BLE001 - any relay failure leaves Desktop approval - return False - - -def _withdraw( - store: ItemApprovalStore, request: ItemApprovalRequest, relay: "PhoneRelay" -) -> None: - try: - answer = store.read_answer(request.request_id) - except ItemApprovalRefused: - answer = None - if answer is not None and answer.route == CLOUD_PHONE_ROUTE: - return # The phone answered; Cloud already closed the link. - reason = ( - "answered_elsewhere" - if answer is not None - else ("expired" if request.expired() else "run_stopped_waiting") - ) - try: - relay.withdraw(request, reason) # type: ignore[arg-type] - except Exception: # noqa: BLE001 - Cloud expires the link on its own - pass - - -def _take_phone_decision( - store: ItemApprovalStore, request: ItemApprovalRequest, relay: "PhoneRelay" -) -> Optional[ItemApprovalAnswer]: - """Turn a Cloud-signed phone decision into this request's answer.""" - - try: - raw = relay.poll(request) - except Exception: # noqa: BLE001 - try again next poll; Desktop still can - return None - if raw is None: - return None - return apply_cloud_decision(store, request, raw) - - -def apply_cloud_decision( - store: ItemApprovalStore, - request: ItemApprovalRequest, - raw: Any, - *, - now: Optional[datetime] = None, -) -> Optional[ItemApprovalAnswer]: - """Verify a Cloud decision for ``request`` and write it as the answer. - - Returns ``None`` (and writes nothing) for a decision that doesn't verify, - or when another answer was already written. - """ - - context = store.context(now=now) - try: - decision = verify_decision( - raw, - request_id=request.request_id, - request_sha256=request.digest(), - pinned_keys=context.cloud_keys, - ) - except CloudDecisionRejected: - return None - try: - answer = make_answer( - request, - decision=decision.decision, - decided_by="human", - route=CLOUD_PHONE_ROUTE, - approver_id=decision.approver_id, - auth_method=decision.auth_method, - decline_reason=decision.decline_reason, - now=now, - policy=context, - cloud_decision=decision, - ) - store.write_answer(answer) - except ItemApprovalRefused: - return None - return answer - - def request_facts(request: ItemApprovalRequest) -> dict[str, str]: """The PHI-free facts a delegated approver sees. No intent, no image.""" @@ -1134,15 +977,13 @@ def make_answer( now: Optional[datetime] = None, approver: Optional[ApproverIdentityV1] = None, policy: "ApproverPolicyV1 | ApproverContext | None" = None, - cloud_decision: Optional[CloudDecisionV1] = None, ) -> ItemApprovalAnswer: """Build and check one answer. Raises :class:`ItemApprovalRefused`. ``approver`` names an agent or system; it is refused unless ``policy`` (``store.context()``) holds a verified, person-signed grant that names it and allows its kind here. The answer records the grant's digest as the - approver's ``policy_ref``. ``cloud_decision`` is the Cloud-signed phone - decision behind a ``cloud_phone`` answer. + approver's ``policy_ref``. """ context = _as_context(policy) @@ -1170,7 +1011,6 @@ def make_answer( auth_method=auth_method, # type: ignore[arg-type] answered_at=_iso(now or _utc_now()), approver=approver, - cloud_decision=cloud_decision, ) except ValidationError as exc: raise ItemApprovalRefused( @@ -1215,7 +1055,7 @@ def decided_by_summary( """Who answered this request, for Desktop and Cloud to show. A person: their Cloud user id (or none when Desktop was signed out) and - where they answered (``desktop_run_it`` or ``cloud_phone``). An agent or + where they answered (``desktop_run_it`` or ``decision_portal``). An agent or system: its id and the digest of the person-signed grant it acted under. ``None`` when nobody answered yet or the answer isn't readable. """ @@ -1256,7 +1096,6 @@ def decided_by_summary( "ItemApprovalStore", "POLICY_FILENAME", "answer_refusal", - "apply_cloud_decision", "make_answer", "request_facts", "request_summary", diff --git a/packages/flow/openadapt_flow/approval_mode/model.py b/packages/flow/openadapt_flow/approval_mode/model.py index 6439a0c83..404751610 100644 --- a/packages/flow/openadapt_flow/approval_mode/model.py +++ b/packages/flow/openadapt_flow/approval_mode/model.py @@ -167,8 +167,10 @@ class ItemDecisionKind(str, Enum): DECLINE_REASONS: tuple[str, ...] = ("wrong_values", "wrong_record", "screen_problem") #: The only route that may approve or decline an item in phase 0. ITEM_APPROVAL_ROUTE: Literal["desktop_run_it"] = "desktop_run_it" -#: Routes a person answers by: Desktop's Run it, or their phone through Cloud. -PERSON_APPROVAL_ROUTES = frozenset({"desktop_run_it", "cloud_phone"}) +#: A person answering on their phone through the runner's decision portal. +DECISION_PORTAL_ROUTE: Literal["decision_portal"] = "decision_portal" +#: Routes a person answers by: Desktop's Run it, or the decision portal. +PERSON_APPROVAL_ROUTES = frozenset({"desktop_run_it", "decision_portal"}) #: How the approver was authenticated. ``none`` only for ``unauthenticated``. ApproverAuthMethod = Literal["cloud_aal2", "cloud_aal1", "none"] @@ -189,12 +191,12 @@ class ItemApprovalRef(BaseModel): decision: Literal["approved", "declined"] decline_reason: Optional[DeclineReason] = None decided_by: Literal["human", "automation"] = "human" - #: ``desktop_run_it`` or ``cloud_phone`` (a Cloud-signed decision from - #: the person's phone) for a person; ``local_cli``, ``webhook`` or ``mcp`` - #: for a delegated approver. - route: Literal["desktop_run_it", "cloud_phone", "local_cli", "webhook", "mcp"] = ( - ITEM_APPROVAL_ROUTE - ) + #: ``desktop_run_it`` or ``decision_portal`` (the person's phone, paired + #: to this runner's decision portal) for a person; ``local_cli``, + #: ``webhook`` or ``mcp`` for a delegated approver. + route: Literal[ + "desktop_run_it", "decision_portal", "local_cli", "webhook", "mcp" + ] = ITEM_APPROVAL_ROUTE approver_kind: Literal["cloud_user", "unauthenticated"] approver_id: Optional[str] = Field(default=None, pattern=OPAQUE_ID_PATTERN) auth_method: ApproverAuthMethod @@ -222,8 +224,6 @@ def _consistent(self) -> "ItemApprovalRef": raise ValueError( "a person answers in Desktop or on their phone; a delegate by its own route" ) - if self.route == "cloud_phone" and self.auth_method != "cloud_aal2": - raise ValueError("a phone approval needs a Cloud sign-in with AAL2") if self.approver_kind == "cloud_user": if self.approver_id is None or self.auth_method == "none": raise ValueError("a Cloud approver needs an id and a sign-in method") diff --git a/packages/flow/openadapt_flow/approval_mode/phone_relay.py b/packages/flow/openadapt_flow/approval_mode/phone_relay.py deleted file mode 100644 index 664dc5951..000000000 --- a/packages/flow/openadapt_flow/approval_mode/phone_relay.py +++ /dev/null @@ -1,172 +0,0 @@ -"""Send a waiting save to Cloud so a person can approve it from their phone. - -Off unless the deployment opts in (``runtime.phone_approval: true`` or -``run --phone-approval``). A local-only or no-outbound install never builds a -relay, and every save keeps waiting for a person in Desktop's Run it. - -What crosses to Cloud, and when: - -* ``publish``: the request's ids and digests, the step's own intent, and the - same filled-form screenshot Desktop shows. Cloud keeps the screenshot - encrypted and only for the request's life (at most 4 hours); it shows it - only to a signed-in (AAL2) member of the organization. The push - notification Cloud sends carries an opaque id and fixed words, never the - screenshot, a name, a value or the workflow's name. -* ``poll``: the Cloud-signed decision, once a person tapped Approve or - Don't save. The run verifies it against the runner's pinned decision key - and the exact request digest (:mod:`.cloud_decision`), then writes it as a - ``cloud_phone`` answer, created once. A Desktop answer that landed first - wins and the phone decision is ignored. -* ``withdraw``: when the request was answered elsewhere, expired or the run - stopped waiting, so the phone link stops working. - -Any network or Cloud failure leaves the save to Desktop. Nothing here retries -a save, and an approval still only lets the run re-check the form and save -exactly what was shown. -""" - -from __future__ import annotations - -import base64 -import os -from dataclasses import dataclass -from typing import Any, Literal, Optional, Protocol - -from openadapt_flow.approval_mode.cloud_decision import CLOUD_DECISION_SCHEMA - -PUBLISH_SCHEMA: Literal["openadapt.save-approval-publish/v1"] = ( - "openadapt.save-approval-publish/v1" -) -WithdrawReason = Literal["answered_elsewhere", "expired", "run_stopped_waiting"] -#: How often the run asks Cloud for a phone decision (seconds). -PHONE_POLL_S = 3.0 -CLOUD_URL_ENV = "OPENADAPT_CLOUD_URL" -RUNNER_TOKEN_ENV = "OPENADAPT_RUNNER_TOKEN" -RUNNER_ID_ENV = "OPENADAPT_RUNNER_ID" - - -class PhoneRelay(Protocol): - runner_id: str - - def publish(self, request: Any, frame_png: bytes) -> bool: ... - - def poll(self, request: Any) -> Optional[dict[str, Any]]: ... - - def withdraw(self, request: Any, reason: WithdrawReason) -> None: ... - - -class PhoneRelayError(RuntimeError): - pass - - -def publish_body(request: Any, frame_png: bytes, runner_id: str) -> dict[str, Any]: - """The exact publish body. Cloud refuses any other field.""" - - return { - "schema": PUBLISH_SCHEMA, - "runner_id": runner_id, - "request_id": request.request_id, - "request_sha256": request.digest(), - "run_id": request.run_id, - "workflow_name": request.workflow_name, - "step_id": request.step_id, - "step_index": request.step_index, - "step_intent": request.step_intent, - "step_class": request.step_class.value, - "mode": request.mode.value, - "form_state_sha256": request.form_state_sha256, - "screenshot_sha256": request.screenshot_sha256, - "screenshot_png_b64": base64.b64encode(frame_png).decode("ascii"), - "created_at": request.created_at, - "expires_at": request.expires_at, - } - - -@dataclass -class CloudPhoneRelay: - """The HTTPS relay to Cloud's runner API (``Authorization: Bearer oar_…``).""" - - base_url: str - token: str - runner_id: str - timeout_s: float = 15.0 - transport: Any = None - - def __post_init__(self) -> None: - if not self.base_url.startswith("https://") and not self.base_url.startswith( - "http://127.0.0.1" - ): - raise PhoneRelayError("phone approval needs an https Cloud URL") - if not self.token.startswith("oar_"): - raise PhoneRelayError("phone approval needs this runner's Cloud token") - import httpx - - self._client = httpx.Client( - base_url=self.base_url.rstrip("/"), - timeout=self.timeout_s, - transport=self.transport, - headers={"authorization": f"Bearer {self.token}"}, - ) - - def publish(self, request: Any, frame_png: bytes) -> bool: - response = self._client.post( - "/api/runners/save-approvals", - json=publish_body(request, frame_png, self.runner_id), - ) - return response.status_code in (200, 201) - - def poll(self, request: Any) -> Optional[dict[str, Any]]: - response = self._client.get(f"/api/runners/save-approvals/{request.request_id}") - if response.status_code != 200: - return None - body = response.json() - if not isinstance(body, dict) or body.get("status") != "decided": - return None - decision = body.get("decision") - if not isinstance(decision, dict): - return None - if decision.get("schema_version") != CLOUD_DECISION_SCHEMA: - return None - return decision - - def withdraw(self, request: Any, reason: WithdrawReason) -> None: - self._client.post( - f"/api/runners/save-approvals/{request.request_id}/withdraw", - json={"reason": reason}, - ) - - def decision_keys(self) -> dict[str, str]: - response = self._client.get("/api/runners/save-approvals/keys") - if response.status_code != 200: - raise PhoneRelayError(f"Cloud answered {response.status_code}") - keys = (response.json() or {}).get("decision_keys") - if not isinstance(keys, dict) or not keys: - raise PhoneRelayError("Cloud returned no decision keys") - return {str(k): str(v) for k, v in keys.items()} - - -def relay_from_env(env: Optional[dict[str, str]] = None) -> CloudPhoneRelay: - """Build the relay from the runner's Cloud settings. Raises when missing: - an operator who asked for phone approval must not silently get none.""" - - source = os.environ if env is None else env - url = source.get(CLOUD_URL_ENV) or "https://app.openadapt.ai" - token = source.get(RUNNER_TOKEN_ENV) or "" - runner_id = source.get(RUNNER_ID_ENV) or "" - if not token or not runner_id: - raise PhoneRelayError( - f"phone approval needs {RUNNER_TOKEN_ENV} and {RUNNER_ID_ENV} " - "(pair this computer with OpenAdapt Cloud first)" - ) - return CloudPhoneRelay(url, token, runner_id) - - -__all__ = [ - "CloudPhoneRelay", - "PHONE_POLL_S", - "PUBLISH_SCHEMA", - "PhoneRelay", - "PhoneRelayError", - "publish_body", - "relay_from_env", -] diff --git a/packages/flow/openadapt_flow/console/app.py b/packages/flow/openadapt_flow/console/app.py index dd90b4b5c..b1ad1be0a 100644 --- a/packages/flow/openadapt_flow/console/app.py +++ b/packages/flow/openadapt_flow/console/app.py @@ -36,6 +36,7 @@ from openadapt_flow.console import actions as actions_mod from openadapt_flow.console import attention as attention_mod from openadapt_flow.console import data, human_decisions, json_artifacts +from openadapt_flow.console import save_approvals as save_approvals_mod from openadapt_flow.runtime.durable.approval import ResumeRefused from openadapt_flow.runtime.durable.attended import ( AttendedActionRefused, @@ -573,6 +574,54 @@ def attention_notification() -> dict[str, Any]: attention_mod.list_attention(runs) ).model_dump() + # -- save approvals (decision portal) ----------------------------------- + + @app.get("/api/save-approvals") + def save_approval_list() -> list[dict[str, Any]]: + return [ + item.model_dump() for item in save_approvals_mod.list_save_approvals(runs) + ] + + @app.get("/api/save-approvals/{request_id}") + def save_approval_detail(request_id: str) -> dict[str, Any]: + try: + return save_approvals_mod.save_approval_detail(runs, request_id) + except save_approvals_mod.SaveApprovalRefused as exc: + raise HTTPException(status_code=404, detail=exc.code) from exc + + @app.get("/api/save-approvals/{request_id}/frame") + def save_approval_frame(request_id: str) -> Response: + try: + frame = save_approvals_mod.save_approval_frame(runs, request_id) + except save_approvals_mod.SaveApprovalRefused as exc: + raise HTTPException(status_code=404, detail=exc.code) from exc + return Response( + content=frame, + media_type="image/png", + headers={"Cache-Control": "no-store", "X-Content-Type-Options": "nosniff"}, + ) + + @app.post("/api/save-approvals/{request_id}/actions/{action_id}") + def decide_save_approval( + request_id: str, + action_id: str, + payload: save_approvals_mod.SaveApprovalActionRequest, + ) -> JSONResponse: + # Answering is an authority action: only the attended console (the + # one the decision portal relays to) takes it. + if not attend: + raise HTTPException(status_code=404, detail="no such action") + if action_id != payload.action: + raise HTTPException( + status_code=400, + detail="action path and request do not match", + ) + try: + receipt = save_approvals_mod.decide_save_approval(runs, request_id, payload) + except save_approvals_mod.SaveApprovalRefused as exc: + raise HTTPException(status_code=409, detail=exc.code) from exc + return JSONResponse(content=receipt.model_dump(mode="json")) + @app.get("/api/attention/{run_id:path}") def attention_detail(run_id: str) -> dict[str, Any]: run_dir = _resolve_run(runs, run_id) diff --git a/packages/flow/openadapt_flow/console/save_approvals.py b/packages/flow/openadapt_flow/console/save_approvals.py new file mode 100644 index 000000000..0261ecb1a --- /dev/null +++ b/packages/flow/openadapt_flow/console/save_approvals.py @@ -0,0 +1,386 @@ +"""Save approvals on the decision portal: the phone half of ``each_item``. + +Under an approval mode a run stops before a save and writes an +:class:`~openadapt_flow.approval_mode.item_approval.ItemApprovalRequest`. This +module offers that request to the person through the same signed +:class:`~openadapt_flow.contracts.human_decision.HumanDecisionTaskV1` contract +the decision portal already renders for paused runs, so a paired phone can +approve or decline the save with one tap: + +* :func:`list_save_approvals` finds every waiting request under the console's + run root. +* :func:`save_approval_detail` returns the signed, PHI-free task plus a + local-only ``presentation`` block (the step's own wording and the screenshot + route). The screenshot is the exact image the request was opened with; the + person sees what will be saved. +* :func:`decide_save_approval` admits one answer. The phone must echo the + digest and signature of the task it was shown, so an answer binds the exact + request (run, step, screenshot digest and form-state digest). The answer + file is created exclusively, so a second tap, a replay, or a later answer + from Desktop is refused. The waiting run re-reads the form before it saves + and stops if anything changed; nothing here saves anything. + +What this module never does +--------------------------- + +* It never sends anything off the runner. The task is the portal's local + task (``required_authn = local_session``); the hosted lane does not carry + save approvals, because Cloud never receives the screenshot and a person + must see exactly what will be saved before approving it. +* It never lets an agent, a rule or a model answer: every answer here is + ``decided_by = human`` on the ``decision_portal`` route, and the run checks + that again itself. +* It never names a person it cannot prove. A paired phone proves a device, + not who held it, so the answer is recorded as an unauthenticated person on + the decision portal, exactly like a signed-out Desktop answer. +""" + +from __future__ import annotations + +import hashlib +import hmac +import os +import secrets +from pathlib import Path +from typing import Any, Literal, Optional + +from pydantic import BaseModel, ConfigDict, Field + +from openadapt_flow.approval_mode.approver import StepClass +from openadapt_flow.approval_mode.item_approval import ( + APPROVAL_DIRNAME, + ItemApprovalRefused, + ItemApprovalRequest, + ItemApprovalStore, + make_answer, +) +from openadapt_flow.approval_mode.model import DECISION_PORTAL_ROUTE, DeclineReason +from openadapt_flow.console import data +from openadapt_flow.contracts.human_decision import ( + HUMAN_DECISION_TASK_SCHEMA, + HumanDecisionAction, + HumanDecisionReceiptReason, + HumanDecisionReceiptState, + HumanDecisionReceiptV1, + HumanDecisionTaskV1, + sign_human_decision_task_hmac, +) + +#: The run-local HMAC key that signs this run's save-approval tasks. Created +#: once, 0600, inside the run's private ``item-approval`` folder. +TASK_KEY_FILENAME = "portal-task-key" +_TASK_KEY_BYTES = 32 +_ISSUER_KEY_ID = "local_save_approval_v1" +_REQUEST_ID_LENGTH = 32 + +_RISK_CLASS: dict[StepClass, str] = { + StepClass.READ_ONLY: "read_only", + StepClass.REVERSIBLE: "state_changing", + StepClass.CONSEQUENTIAL: "consequential", + StepClass.IRREVERSIBLE: "irreversible", + StepClass.CLINICAL_WRITE: "irreversible", +} + + +class SaveApprovalRefused(ValueError): + """An answer the portal will not record. ``code`` is closed.""" + + def __init__(self, code: str, message: str) -> None: + super().__init__(message) + self.code = code + + +class SaveApprovalActionRequest(BaseModel): + """What the phone sends back: the task it saw and one decision.""" + + model_config = ConfigDict(extra="forbid") + + action: Literal["verify_and_resume", "reject"] + task_digest: str = Field(pattern=r"^sha256:[0-9a-f]{64}$") + task_signature: str = Field(pattern=r"^hmac-sha256:[0-9a-f]{64}$") + #: Required to decline, refused to approve. Same closed list as Desktop. + decline_reason: Optional[DeclineReason] = None + + +class SaveApprovalSummary(BaseModel): + """One waiting save in the portal's list. PHI-free.""" + + model_config = ConfigDict(extra="forbid") + + request_id: str + step_class: str + created_at: str + expires_at: str + + +# -- finding requests ---------------------------------------------------------- + + +def _has_requests(path: Path) -> bool: + folder = path / APPROVAL_DIRNAME + return ( + not path.is_symlink() + and path.is_dir() + and folder.is_dir() + and not folder.is_symlink() + ) + + +def _waiting(root: Path) -> list[tuple[Path, ItemApprovalRequest]]: + found: list[tuple[Path, ItemApprovalRequest]] = [] + for run_dir in data._scan(Path(root), _has_requests): + store = ItemApprovalStore(run_dir) + for request in store.requests(): + if store.status(request) == "waiting": + found.append((run_dir, request)) + found.sort(key=lambda pair: (pair[1].created_at, pair[1].request_id)) + return found + + +def list_save_approvals(root: Path) -> list[SaveApprovalSummary]: + """Every save waiting for a person under ``root``, oldest first.""" + + return [ + SaveApprovalSummary( + request_id=request.request_id, + step_class=request.step_class.value, + created_at=request.created_at, + expires_at=request.expires_at, + ) + for _, request in _waiting(root) + ] + + +def find_waiting(root: Path, request_id: str) -> tuple[Path, ItemApprovalRequest]: + """The waiting request with this id. Raises :class:`SaveApprovalRefused`.""" + + if ( + not isinstance(request_id, str) + or len(request_id) != _REQUEST_ID_LENGTH + or any(char not in "0123456789abcdef" for char in request_id) + ): + raise SaveApprovalRefused("request_unknown", "no such save approval") + for run_dir, request in _waiting(root): + if request.request_id == request_id: + return run_dir, request + raise SaveApprovalRefused( + "request_not_waiting", + "this save is no longer waiting; it was answered, used or expired", + ) + + +# -- the signed task ----------------------------------------------------------- + + +def _task_key(store: ItemApprovalStore) -> bytes: + """This run's portal task key, created once and never rewritten.""" + + path = store.dir / TASK_KEY_FILENAME + flags = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_NOFOLLOW", 0) + try: + descriptor = os.open(path, flags, 0o600) + except FileExistsError: + pass + else: + with os.fdopen(descriptor, "wb") as handle: + handle.write(secrets.token_bytes(_TASK_KEY_BYTES)) + handle.flush() + os.fsync(handle.fileno()) + read_flags = os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0) + try: + descriptor = os.open(path, read_flags) + except OSError as exc: + raise SaveApprovalRefused( + "task_key_unreadable", "the portal task key is unreadable" + ) from exc + with os.fdopen(descriptor, "rb") as handle: + key = handle.read(_TASK_KEY_BYTES + 1) + if len(key) != _TASK_KEY_BYTES: + raise SaveApprovalRefused( + "task_key_unreadable", "the portal task key is malformed" + ) + return key + + +def _sha256_prefixed(value: str) -> str: + return "sha256:" + hashlib.sha256(value.encode("utf-8")).hexdigest() + + +def save_approval_task( + run_dir: Path, request: ItemApprovalRequest +) -> HumanDecisionTaskV1: + """The signed, PHI-free decision task for one waiting save. + + Every field is a closed code or an opaque digest. ``capability_digest`` is + the request's own digest, so the task names exactly the run, step, + screenshot and form state the person is asked about. + """ + + store = ItemApprovalStore(run_dir) + bundle = request.bundle_content_digest + fields: dict[str, Any] = { + "schema_version": HUMAN_DECISION_TASK_SCHEMA, + "task_id": f"task_save_{request.request_id}", + "task_revision": 1, + "tenant_id": None, + "runner_id": None, + "run_id": _sha256_prefixed(request.run_id).replace("sha256:", "run_"), + "pause_id": f"save_{request.request_id}", + "capability_digest": request.digest(), + "bundle_digest": ( + f"sha256:{bundle}" + if bundle is not None + else _sha256_prefixed(f"unbound:{request.workflow_name}") + ), + "task_kind": "save_approval", + "delivery_state": "not_delivered", + "risk_class": _RISK_CLASS.get(request.step_class, "unknown"), + "substrate": "unknown", + "question": {"template": "approve_save", "safe_slots": {}}, + "evidence": { + "frame_available_locally": True, + "sensitive_evidence_local_only": True, + }, + "allowed_actions": ["verify_and_resume", "reject"], + "required_authn": "local_session", + "created_at": request.created_at, + "expires_at": request.expires_at, + "nonce": f"save_nonce_{request.request_id}", + "issuer_key_id": _ISSUER_KEY_ID, + "signature_algorithm": "hmac-sha256", + } + return sign_human_decision_task_hmac(key=_task_key(store), fields=fields) + + +def save_approval_detail(root: Path, request_id: str) -> dict[str, Any]: + """The portal's view of one waiting save: signed task + local presentation. + + ``presentation`` stays on this runner. It holds the step's own wording and + the route of the exact screenshot the request was opened with. + """ + + run_dir, request = find_waiting(root, request_id) + task = save_approval_task(run_dir, request) + return { + "task": task.model_dump(mode="json"), + "task_digest": task.digest, + "presentation": { + "question": "Save this?", + "step_intent": request.step_intent, + "step_index": request.step_index, + "step_class": request.step_class.value, + "explanation": ( + "This is exactly what OpenAdapt will save. Before it saves, " + "it reads the form again; if anything changed, it stops and " + "asks again." + ), + "screenshot_route": f"/api/save-approvals/{request.request_id}/frame", + "screenshot_sha256": request.screenshot_sha256, + "decline_reasons": ["wrong_values", "wrong_record", "screen_problem"], + "expires_at": request.expires_at, + }, + } + + +def save_approval_frame(root: Path, request_id: str) -> bytes: + """The exact screenshot of the waiting save, checked against its digest.""" + + run_dir, request = find_waiting(root, request_id) + try: + return ItemApprovalStore(run_dir).read_frame(request) + except ItemApprovalRefused as exc: + raise SaveApprovalRefused(exc.code, str(exc)) from exc + + +# -- answering ----------------------------------------------------------------- + + +def decide_save_approval( + root: Path, + request_id: str, + payload: SaveApprovalActionRequest, +) -> HumanDecisionReceiptV1: + """Record one person's answer from the portal. Single use. + + Raises :class:`SaveApprovalRefused` when the save is no longer waiting, + the task the phone echoes isn't the current one, or the answer itself is + refused by the run's own checks. + """ + + run_dir, request = find_waiting(root, request_id) + task = save_approval_task(run_dir, request) + if not ( + hmac.compare_digest(payload.task_digest, task.digest) + and hmac.compare_digest(payload.task_signature, task.signature) + ): + raise SaveApprovalRefused( + "task_changed", "this save changed since it was shown; reload it" + ) + approve = payload.action == "verify_and_resume" + if approve == (payload.decline_reason is not None): + raise SaveApprovalRefused( + "decline_reason_required" if not approve else "decline_reason_not_allowed", + "a decline needs exactly one reason; an approval takes none", + ) + store = ItemApprovalStore(run_dir) + try: + answer = make_answer( + request, + decision="approve" if approve else "decline", + decided_by="human", + route=DECISION_PORTAL_ROUTE, + approver_id=None, + auth_method="none", + decline_reason=payload.decline_reason, + policy=store.context(), + ) + store.write_answer(answer) + except ItemApprovalRefused as exc: + raise SaveApprovalRefused(exc.code, str(exc)) from exc + request_digest = ( + "sha256:" + + hashlib.sha256(payload.model_dump_json().encode("utf-8")).hexdigest() + ) + return HumanDecisionReceiptV1( + task_id=task.task_id, + pause_id=task.pause_id, + capability_digest=task.capability_digest, + request_digest=request_digest, + decision_digest=answer.digest(), + action=HumanDecisionAction(payload.action), + # An approval is not a save: the waiting run re-reads the form and + # saves only if nothing changed. A decline stops this save. + state=( + HumanDecisionReceiptState.ACCEPTED_PENDING_RUNNER + if approve + else HumanDecisionReceiptState.REJECTED + ), + reason_code=( + HumanDecisionReceiptReason.PENDING_RUNNER + if approve + else HumanDecisionReceiptReason.REJECTED_BY_OPERATOR + ), + report_success=False, + decided_at=answer.answered_at, + ) + + +def notification_count(root: Path) -> int: + """How many saves are waiting, for the PHI-free OS notification.""" + + return len(_waiting(root)) + + +__all__ = [ + "TASK_KEY_FILENAME", + "SaveApprovalActionRequest", + "SaveApprovalRefused", + "SaveApprovalSummary", + "decide_save_approval", + "find_waiting", + "list_save_approvals", + "notification_count", + "save_approval_detail", + "save_approval_frame", + "save_approval_task", +] diff --git a/packages/flow/openadapt_flow/contracts/approval.py b/packages/flow/openadapt_flow/contracts/approval.py index 225a22c80..6aa84dd8a 100644 --- a/packages/flow/openadapt_flow/contracts/approval.py +++ b/packages/flow/openadapt_flow/contracts/approval.py @@ -523,7 +523,7 @@ def counts_as_clean(tier: ReadBackTier, outcome: ReadBackOutcome) -> bool: #: Where a decision was made. ApprovalRoute = Literal[ "desktop_run_it", - "cloud_phone", + "decision_portal", "review_item", "review_batch", "local_cli", diff --git a/packages/flow/openadapt_flow/contracts/human_decision.py b/packages/flow/openadapt_flow/contracts/human_decision.py index 086539ddd..ff7c11810 100644 --- a/packages/flow/openadapt_flow/contracts/human_decision.py +++ b/packages/flow/openadapt_flow/contracts/human_decision.py @@ -140,6 +140,9 @@ class HumanDecisionTaskKind(str, Enum): DELIVERY_UNCERTAIN = "delivery_uncertain" HALT = "halt" OPERATOR_REVIEW = "operator_review" + #: A save waiting for a person under an approval mode. Not a halt: the + #: run is healthy and asks before it writes. + SAVE_APPROVAL = "save_approval" class HumanDecisionQuestionTemplate(str, Enum): @@ -149,6 +152,7 @@ class HumanDecisionQuestionTemplate(str, Enum): COMPLETE_HUMAN_STEP = "complete_human_step" REVIEW_UNCERTAIN_DELIVERY = "review_uncertain_delivery" REVIEW_HALT = "review_halt" + APPROVE_SAVE = "approve_save" class HumanDecisionAction(str, Enum): diff --git a/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json b/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json index a87164357..31b444abd 100644 --- a/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json +++ b/packages/flow/openadapt_flow/contracts/schemas/approval-record-v1.json @@ -191,7 +191,7 @@ "route": { "enum": [ "desktop_run_it", - "cloud_phone", + "decision_portal", "review_item", "review_batch", "local_cli", diff --git a/packages/flow/openadapt_flow/contracts/schemas/execute-v1-openapi.json b/packages/flow/openadapt_flow/contracts/schemas/execute-v1-openapi.json index 7bb3ffd35..0a284a596 100644 --- a/packages/flow/openadapt_flow/contracts/schemas/execute-v1-openapi.json +++ b/packages/flow/openadapt_flow/contracts/schemas/execute-v1-openapi.json @@ -1069,7 +1069,8 @@ "resolve_ambiguity", "complete_human_step", "review_uncertain_delivery", - "review_halt" + "review_halt", + "approve_save" ], "title": "HumanDecisionQuestionTemplate", "type": "string" @@ -1182,7 +1183,8 @@ "human_step", "delivery_uncertain", "halt", - "operator_review" + "operator_review", + "save_approval" ], "title": "HumanDecisionTaskKind", "type": "string" diff --git a/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v1.json b/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v1.json index 76f44b333..091deda98 100644 --- a/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v1.json +++ b/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v1.json @@ -131,7 +131,8 @@ "resolve_ambiguity", "complete_human_step", "review_uncertain_delivery", - "review_halt" + "review_halt", + "approve_save" ], "title": "HumanDecisionQuestionTemplate", "type": "string" @@ -244,7 +245,8 @@ "human_step", "delivery_uncertain", "halt", - "operator_review" + "operator_review", + "save_approval" ], "title": "HumanDecisionTaskKind", "type": "string" diff --git a/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v2.json b/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v2.json index af0db56d2..dc7169cbc 100644 --- a/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v2.json +++ b/packages/flow/openadapt_flow/contracts/schemas/human-decision-task-v2.json @@ -162,7 +162,8 @@ "resolve_ambiguity", "complete_human_step", "review_uncertain_delivery", - "review_halt" + "review_halt", + "approve_save" ], "title": "HumanDecisionQuestionTemplate", "type": "string" @@ -275,7 +276,8 @@ "human_step", "delivery_uncertain", "halt", - "operator_review" + "operator_review", + "save_approval" ], "title": "HumanDecisionTaskKind", "type": "string" diff --git a/packages/flow/openadapt_flow/deployment.py b/packages/flow/openadapt_flow/deployment.py index 8eefb53b2..7b706dfc5 100644 --- a/packages/flow/openadapt_flow/deployment.py +++ b/packages/flow/openadapt_flow/deployment.py @@ -584,10 +584,6 @@ class RuntimeSection(BaseModel): #: ``--approval-mode`` wins. For the Standard and Regulated profiles a #: missing or unknown value means ``each_item``. approval_mode: Optional[str] = None - #: Opt-in phone approval: each waiting save also goes to OpenAdapt Cloud, - #: which pushes an opaque link to the approver's phone. Off by default; - #: a local-only install keeps Desktop approval and sends nothing. - phone_approval: bool = False #: Tier-3 durable runtime: checkpoint each verified step, durably pause on #: halt, resumable via ``resume``. Off by default. durable: bool = False diff --git a/packages/flow/public-artifacts.json b/packages/flow/public-artifacts.json index 5b00de59d..1cdc933b3 100644 --- a/packages/flow/public-artifacts.json +++ b/packages/flow/public-artifacts.json @@ -1881,7 +1881,7 @@ }, { "path": "openadapt_flow/contracts/schemas/approval-record-v1.json", - "sha256": "0eeacfb80916b6833f577a2f6eb92f7fd4dc54c1144b7d3661dd01acf9258fc2" + "sha256": "db5d96377a7ec89ccd2f6f69370f3ee70fce74e51eacdd6446188145daa6aa42" }, { "path": "openadapt_flow/contracts/schemas/approver-identity-v1.json", @@ -1985,7 +1985,7 @@ }, { "path": "openadapt_flow/contracts/schemas/execute-v1-openapi.json", - "sha256": "210a6d02d20e0fb43ef70aa003c3cb39ada4984e85bfb10d52662ba2c7c4d963" + "sha256": "a89f98f766eb56234b7ebb6738d8fef81ace9ea17c2b2bf7f3c69b75a3aba621" }, { "path": "openadapt_flow/contracts/schemas/execution-requirements-v1.json", @@ -1997,11 +1997,11 @@ }, { "path": "openadapt_flow/contracts/schemas/human-decision-task-v1.json", - "sha256": "d10e3cf79684e1dd07758b3f82b679e4275971161a91d8d27494acfd611d4fee" + "sha256": "b266bc4a77663ac92e24897b19806f360fe026c465f33579b4a27540001d0607" }, { "path": "openadapt_flow/contracts/schemas/human-decision-task-v2.json", - "sha256": "99e767662eaada4b2c0000026bcc59f33a2edff885274939fa425497e0e47ef9" + "sha256": "85f9093287b6bad853a50e4498690d51ef2f97cc538e4d6fa964b8ac3c5e9e1d" }, { "path": "openadapt_flow/contracts/schemas/plain-outcome-v1.json", diff --git a/packages/flow/tests/_approver_grant_support.py b/packages/flow/tests/_approver_grant_support.py index 61e47efd1..4fbc97d8e 100644 --- a/packages/flow/tests/_approver_grant_support.py +++ b/packages/flow/tests/_approver_grant_support.py @@ -81,12 +81,9 @@ def grant_store( *, run: str = "run", initiator: Optional[str] = None, - cloud_keys: Optional[dict[str, str]] = None, **kwargs, ) -> ItemApprovalStore: - store = ItemApprovalStore( - tmp_path / run, key_dir=key_dir(tmp_path), cloud_keys=cloud_keys or {} - ) + store = ItemApprovalStore(tmp_path / run, key_dir=key_dir(tmp_path)) store.write_grant(signed_grant(tmp_path, policy, **kwargs)) store.write_initiator(initiator) return store diff --git a/packages/flow/tests/test_approval_signed_grant.py b/packages/flow/tests/test_approval_signed_grant.py index d3b69206a..99ef67770 100644 --- a/packages/flow/tests/test_approval_signed_grant.py +++ b/packages/flow/tests/test_approval_signed_grant.py @@ -1,4 +1,4 @@ -"""Signed approver grants, self-approval, and phone decisions. Made-up ids. +"""Signed approver grants and self-approval. Made-up ids. Invariants under test: @@ -6,21 +6,16 @@ answer a save; an unsigned, tampered, expired or wrongly signed one means person-only; * the agent or system that started a run can't answer that run's saves; -* a phone answer counts only with a Cloud-signed decision for the exact - request digest, from a person signed in with AAL2, verified against a key - pinned on this runner, and it is used once; * every answer names who decided: a person (and where), or the delegate and the digest of the grant it acted under. """ from __future__ import annotations -import base64 import json from datetime import datetime, timedelta, timezone import pytest -from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey from openadapt_flow.approval_mode.approver import ( @@ -30,14 +25,11 @@ ApproverPolicyV1, StepClass, ) -from openadapt_flow.approval_mode.cloud_decision import sign_decision from openadapt_flow.approval_mode.item_approval import ( ItemApprovalRefused, ItemApprovalStore, - apply_cloud_decision, decided_by_summary, make_answer, - wait_for_answer, ) from openadapt_flow.approval_mode.model import ApprovalMode from openadapt_flow.approval_mode.signed_policy import ( @@ -285,209 +277,6 @@ def test_a_planted_initiator_refuses_the_run(tmp_path): store2.write_initiator(None) -# -- phone decisions -------------------------------------------------------------- - -CLOUD_KEY = Ed25519PrivateKey.generate() -CLOUD_PUB = base64.b64encode( - CLOUD_KEY.public_key().public_bytes( - serialization.Encoding.Raw, serialization.PublicFormat.Raw - ) -).decode() -PINNED = {"cloud-decision-1": CLOUD_PUB} - - -def _decision(request, *, key=CLOUD_KEY, **overrides): - fields = { - "request_id": request.request_id, - "request_sha256": request.digest(), - "decision": "approve", - "approver_id": "cloud-user-0001", - "auth_method": "cloud_aal2", - "decided_at": datetime.now(timezone.utc).isoformat(), - "link_id": "L" * 22, - "runner_id": "runner-0001", - "key_id": "cloud-decision-1", - } - fields.update(overrides) - return sign_decision(fields, key).model_dump(mode="json") - - -def _phone_store(tmp_path, pinned=PINNED): - store = ItemApprovalStore(tmp_path / "run", cloud_keys=pinned) - store.write_initiator(None) - return store - - -def test_a_signed_phone_decision_becomes_the_answer(tmp_path): - store = _phone_store(tmp_path) - request = _open(store) - answer = apply_cloud_decision(store, request, _decision(request)) - assert answer is not None and answer.route == "cloud_phone" - assert store.status(request) == "approved" - ref = answer.reference(request) - assert ref.route == "cloud_phone" and ref.decided_by == "human" - assert ref.approver_id == "cloud-user-0001" and ref.auth_method == "cloud_aal2" - summary = decided_by_summary(store, request) - assert summary["kind"] == "person" and summary["route"] == "cloud_phone" - # Used once: the second save can't reuse it, and a replay writes nothing. - assert store.consume(request.request_id) is True - assert store.consume(request.request_id) is False - assert apply_cloud_decision(store, request, _decision(request)) is None - - -def test_a_phone_decline_carries_its_reason(tmp_path): - store = _phone_store(tmp_path) - request = _open(store) - raw = _decision(request, decision="decline", decline_reason="wrong_values") - answer = apply_cloud_decision(store, request, raw) - assert answer is not None and answer.decline_reason == "wrong_values" - assert store.status(request) == "declined" - - -@pytest.mark.parametrize( - "mutate", - [ - lambda r: {"request_sha256": "sha256:" + "0" * 64}, - lambda r: {"key_id": "unpinned"}, - lambda r: {"auth_method": "cloud_aal1"}, - ], -) -def test_phone_decisions_that_dont_bind_are_refused(tmp_path, mutate): - store = _phone_store(tmp_path) - request = _open(store) - try: - raw = _decision(request, **mutate(request)) - except ValueError: - return # refused at the contract: an AAL1 decision can't even be built - assert apply_cloud_decision(store, request, raw) is None - assert store.status(request) == "waiting" - - -def test_a_decision_for_one_request_cant_answer_another(tmp_path): - store = _phone_store(tmp_path) - first, second = _open(store), _open(store) - raw = _decision(first) - raw_for_second = {**raw, "request_id": second.request_id} - assert apply_cloud_decision(store, second, raw_for_second) is None - assert apply_cloud_decision(store, second, raw) is None - - -def test_a_forged_signature_is_refused(tmp_path): - store = _phone_store(tmp_path) - request = _open(store) - forged = _decision(request, key=Ed25519PrivateKey.generate()) - assert apply_cloud_decision(store, request, forged) is None - - -def test_no_pinned_key_means_no_phone_route(tmp_path): - store = _phone_store(tmp_path, pinned={}) - request = _open(store) - assert apply_cloud_decision(store, request, _decision(request)) is None - - -def test_a_hand_written_phone_answer_is_refused(tmp_path): - store = _phone_store(tmp_path) - request = _open(store) - with pytest.raises(ItemApprovalRefused) as err: - make_answer( - request, - decision="approve", - decided_by="human", - route="cloud_phone", - approver_id="cloud-user-0001", - auth_method="cloud_aal2", - policy=store.context(), - ) - assert err.value.code == "cloud_decision_missing" - - -class _Relay: - runner_id = "runner-0001" - - def __init__(self, decisions): - self.decisions = list(decisions) - self.published: list[bytes] = [] - self.withdrawn: list[str] = [] - - def publish(self, request, frame_png): - self.published.append(frame_png) - return True - - def poll(self, request): - return self.decisions.pop(0) if self.decisions else None - - def withdraw(self, request, reason): - self.withdrawn.append(reason) - - -def _clock(): - state = {"t": 0.0} - return (lambda: state["t"]), (lambda s: state.__setitem__("t", state["t"] + s)) - - -def test_the_run_waits_for_a_phone_decision(tmp_path): - store = _phone_store(tmp_path) - request = _open(store) - relay = _Relay([None, _decision(request)]) - monotonic, sleep = _clock() - answer, code = wait_for_answer( - store, - request, - timeout_s=60, - poll_s=1, - monotonic=monotonic, - sleep=sleep, - phone_relay=relay, - ) - assert code is None and answer.route == "cloud_phone" - assert relay.published == [FRAME] - assert relay.withdrawn == [] # the phone answered; nothing to withdraw - - -def test_a_desktop_answer_wins_and_the_phone_link_is_withdrawn(tmp_path): - store = _phone_store(tmp_path) - request = _open(store) - store.write_answer( - make_answer( - request, - decision="approve", - decided_by="human", - route="desktop_run_it", - approver_id="cloud-user-0002", - auth_method="cloud_aal2", - ) - ) - relay = _Relay([_decision(request)]) - monotonic, sleep = _clock() - answer, code = wait_for_answer( - store, request, timeout_s=5, monotonic=monotonic, sleep=sleep, phone_relay=relay - ) - assert answer.route == "desktop_run_it" - assert relay.withdrawn == ["answered_elsewhere"] - - -def test_a_relay_failure_leaves_desktop_approval(tmp_path): - class Broken(_Relay): - def publish(self, request, frame_png): - raise OSError("offline") - - store = _phone_store(tmp_path) - request = _open(store) - relay = Broken([]) - monotonic, sleep = _clock() - answer, code = wait_for_answer( - store, - request, - timeout_s=3, - poll_s=1, - monotonic=monotonic, - sleep=sleep, - phone_relay=relay, - ) - assert answer is None and code == "approval_wait_timed_out" - assert relay.withdrawn == ["run_stopped_waiting"] - - def test_unknown_grant_fields_are_refused(): data = json.loads(make_grant(_policy()).model_dump_json(by_alias=True)) data["self_enable"] = True diff --git a/packages/flow/tests/test_console_save_approvals.py b/packages/flow/tests/test_console_save_approvals.py new file mode 100644 index 000000000..9489a0efd --- /dev/null +++ b/packages/flow/tests/test_console_save_approvals.py @@ -0,0 +1,205 @@ +"""Save approvals on the decision portal. Made-up runs and images. + +Invariants under test: + +* the portal shows a waiting save as a signed, PHI-free decision task, plus + the exact screenshot the request was opened with; +* an answer binds the task the phone was shown: a changed or forged task is + refused, and the answer is single-use (a second tap is refused); +* only a person answers here (``decided_by = human`` on the + ``decision_portal`` route), and a decline needs a closed reason; +* answering is offered only by the attended console the portal relays to. +""" + +from __future__ import annotations + +import hashlib +import json +from datetime import datetime, timedelta, timezone +from pathlib import Path + +import pytest +from fastapi.testclient import TestClient + +from openadapt_flow.approval_mode.item_approval import ( + ItemApprovalStore, + decided_by_summary, +) +from openadapt_flow.approval_mode.model import ApprovalMode +from openadapt_flow.console import save_approvals +from openadapt_flow.console.app import create_app +from openadapt_flow.contracts.human_decision import HumanDecisionTaskV1 + +FRAME = b"\x89PNG\r\n\x1a\nmade-up filled form" +FORM = ("dom", "sha256:" + "1" * 64) + + +def _open(root: Path, run: str = "run-a", **kwargs): + store = ItemApprovalStore(root / run) + store.write_initiator(None) + request = store.open_request( + run_id=run, + workflow_name="made-up-intake", + step_id="made-up-save-step", + step_index=3, + step_intent="Click 'Save'", + mode=ApprovalMode.EACH_ITEM, + bundle_content_digest="a" * 64, + runtime_inputs_digest=None, + frame_png=FRAME, + form_state=FORM, + **kwargs, + ) + return store, request + + +def _answer(root: Path, request_id: str, action="verify_and_resume", **extra): + detail = save_approvals.save_approval_detail(root, request_id) + payload = save_approvals.SaveApprovalActionRequest( + action=action, + task_digest=detail["task_digest"], + task_signature=detail["task"]["signature"], + **extra, + ) + return save_approvals.decide_save_approval(root, request_id, payload) + + +def test_a_waiting_save_is_a_signed_phi_free_task(tmp_path): + store, request = _open(tmp_path) + [row] = save_approvals.list_save_approvals(tmp_path) + assert row.request_id == request.request_id + detail = save_approvals.save_approval_detail(tmp_path, request.request_id) + task = HumanDecisionTaskV1.model_validate(detail["task"]) + assert task.task_kind.value == "save_approval" + assert task.question.template.value == "approve_save" + assert task.capability_digest == request.digest() + assert [a.value for a in task.allowed_actions] == ["verify_and_resume", "reject"] + assert task.required_authn.value == "local_session" + assert task.evidence.sensitive_evidence_local_only is True + # Nothing a person typed, and no run or workflow label, is in the task. + wire = json.dumps(detail["task"]) + for text in ("made-up-intake", "Click", "run-a", "made-up-save-step"): + assert text not in wire + # The same request always yields the same signed task. + again = save_approvals.save_approval_detail(tmp_path, request.request_id) + assert again["task"] == detail["task"] + # The screenshot is exactly the one the request was opened with. + frame = save_approvals.save_approval_frame(tmp_path, request.request_id) + assert hashlib.sha256(frame).hexdigest() == request.screenshot_sha256 + assert detail["presentation"]["step_intent"] == "Click 'Save'" + + +def test_an_approval_is_recorded_once_as_a_person_on_the_portal(tmp_path): + store, request = _open(tmp_path) + receipt = _answer(tmp_path, request.request_id) + assert receipt.state.value == "accepted_pending_runner" + assert receipt.report_success is False + assert store.status(request) == "approved" + answer = store.read_answer(request.request_id) + assert answer.decided_by == "human" and answer.route == "decision_portal" + assert answer.approver_kind == "unauthenticated" + assert decided_by_summary(store, request)["route"] == "decision_portal" + # Single use: the save is no longer waiting, so a second tap is refused. + assert save_approvals.list_save_approvals(tmp_path) == [] + with pytest.raises(save_approvals.SaveApprovalRefused) as err: + _answer(tmp_path, request.request_id) + assert err.value.code == "request_not_waiting" + + +def test_a_decline_needs_one_closed_reason(tmp_path): + store, request = _open(tmp_path) + with pytest.raises(save_approvals.SaveApprovalRefused) as err: + _answer(tmp_path, request.request_id, action="reject") + assert err.value.code == "decline_reason_required" + with pytest.raises(save_approvals.SaveApprovalRefused): + _answer(tmp_path, request.request_id, decline_reason="wrong_values") + receipt = _answer( + tmp_path, request.request_id, action="reject", decline_reason="wrong_record" + ) + assert receipt.state.value == "rejected" + assert store.read_answer(request.request_id).decline_reason == "wrong_record" + + +def test_an_answer_for_another_task_is_refused(tmp_path): + _, first = _open(tmp_path, "run-a") + _, second = _open(tmp_path, "run-b") + detail = save_approvals.save_approval_detail(tmp_path, first.request_id) + payload = save_approvals.SaveApprovalActionRequest( + action="verify_and_resume", + task_digest=detail["task_digest"], + task_signature=detail["task"]["signature"], + ) + with pytest.raises(save_approvals.SaveApprovalRefused) as err: + save_approvals.decide_save_approval(tmp_path, second.request_id, payload) + assert err.value.code == "task_changed" + forged = payload.model_copy(update={"task_signature": "hmac-sha256:" + "0" * 64}) + with pytest.raises(save_approvals.SaveApprovalRefused): + save_approvals.decide_save_approval(tmp_path, first.request_id, forged) + + +def test_an_expired_save_is_not_offered(tmp_path): + old = datetime.now(timezone.utc) - timedelta(hours=5) + _open(tmp_path, now=old, ttl_s=60) + assert save_approvals.list_save_approvals(tmp_path) == [] + + +def test_a_swapped_screenshot_is_refused(tmp_path): + store, request = _open(tmp_path) + (store.run_dir / request.screenshot).chmod(0o600) + (store.run_dir / request.screenshot).write_bytes(b"\x89PNG other") + with pytest.raises(save_approvals.SaveApprovalRefused) as err: + save_approvals.save_approval_frame(tmp_path, request.request_id) + assert err.value.code == "frame_digest_mismatch" + + +def _client(root: Path, *, attend: bool) -> TestClient: + bundles = root / "bundles" + bundles.mkdir(exist_ok=True) + app = create_app(bundles, root, attend=attend) + return TestClient( + app, + base_url="http://127.0.0.1", + headers={ + "Authorization": f"Bearer {app.state.console_access_token}", + "Origin": "http://127.0.0.1", + "X-OpenAdapt-CSRF": app.state.console_csrf_token, + }, + ) + + +def test_console_routes_relay_the_task_and_take_one_answer(tmp_path): + _, request = _open(tmp_path) + client = _client(tmp_path, attend=True) + [row] = client.get("/api/save-approvals").json() + detail = client.get(f"/api/save-approvals/{row['request_id']}").json() + frame = client.get(detail["presentation"]["screenshot_route"]) + assert frame.status_code == 200 and frame.content == FRAME + assert frame.headers["cache-control"] == "no-store" + body = { + "action": "verify_and_resume", + "task_digest": detail["task_digest"], + "task_signature": detail["task"]["signature"], + } + url = f"/api/save-approvals/{request.request_id}/actions/verify_and_resume" + assert client.post(url, json={**body, "action": "reject"}).status_code == 400 + first = client.post(url, json=body) + assert first.status_code == 200 + assert first.json()["state"] == "accepted_pending_runner" + assert client.post(url, json=body).status_code == 409 + assert client.get(f"/api/save-approvals/{request.request_id}").status_code == 404 + + +def test_only_the_attended_console_takes_answers(tmp_path): + _, request = _open(tmp_path) + client = _client(tmp_path, attend=False) + detail = client.get(f"/api/save-approvals/{request.request_id}").json() + response = client.post( + f"/api/save-approvals/{request.request_id}/actions/verify_and_resume", + json={ + "action": "verify_and_resume", + "task_digest": detail["task_digest"], + "task_signature": detail["task"]["signature"], + }, + ) + assert response.status_code == 404 + assert client.get("/api/save-approvals/not-an-id").status_code == 404 From 158381773692c0c05daf071e55ab2ae5f0966f98 Mon Sep 17 00:00:00 2001 From: Richard Abrich <774615+abrichr@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:37:27 -0400 Subject: [PATCH 4/5] fix(flow): name approver-grant through command() in the refusal hint --- packages/flow/openadapt_flow/__main__.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index 6f6b40465..e5820fbfa 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -2320,10 +2320,12 @@ def _approver_settings(args: argparse.Namespace): if path: loaded = load_approver_policy(path, default_key_dir()) if loaded.refusal is not None: + from openadapt_flow.cli_hints import command + print( f"approver policy not used ({loaded.refusal}): only a person " "answers this run's saves. A delegate needs a grant a " - "registered person signed (openadapt-flow approver-grant sign).", + f"registered person signed ({command('approver-grant sign')}).", file=sys.stderr, ) approver = None From 4eed4bce23553870af01a07e935bdffcf0925b6d Mon Sep 17 00:00:00 2001 From: abrichr Date: Sun, 18 Oct 2026 01:58:52 -0400 Subject: [PATCH 5/5] fix(approval): a phone answer reaches a run that stopped waiting A run that gets no answer in time stops before the save and leaves a durable item_approval pause. The command line carried a later answer to that pause; the phone (decide_save_approval) only wrote it. So a phone approval sat until someone ran `resume`, and a phone decline left the pause open while the receipt said the run would continue. The carry step moves to approval_mode/durable_carry.py, and both the CLI and the decision portal call it right after writing the answer. Co-Authored-By: Claude Opus 5.5 --- packages/flow/openadapt_flow/__main__.py | 59 ++----------- .../approval_mode/durable_carry.py | 87 +++++++++++++++++++ .../openadapt_flow/console/save_approvals.py | 7 ++ .../tests/test_approval_mode_each_item.py | 51 +++++++++++ 4 files changed, 150 insertions(+), 54 deletions(-) create mode 100644 packages/flow/openadapt_flow/approval_mode/durable_carry.py diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index 43d4ddbca..34990795b 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -3706,63 +3706,14 @@ def _cmd_approver_grant(args: argparse.Namespace) -> int: def _answer_durable_item_pause(run_dir: Path, request, answer) -> bool: """Carry an answer to a durable pause the run left after waiting. - A run that stopped waiting (no answer in time) left a durable - ``item_approval`` pause. An approval then authorizes exactly one resume of - that pause; a decline ends it. Returns True when ``resume`` must follow. + See :func:`openadapt_flow.approval_mode.durable_carry.carry_answer_to_durable_pause`. + Returns True when ``resume`` must follow. """ - from openadapt_flow import crypto - from openadapt_flow.runtime.durable.approval import ( - ResumeRefused, - issue_resume_approval, + from openadapt_flow.approval_mode.durable_carry import ( + carry_answer_to_durable_pause, ) - from openadapt_flow.runtime.durable.checkpoint import CheckpointStore - from openadapt_flow.runtime.durable.program_checkpoint import bundle_version - try: - store = CheckpointStore(run_dir, key=crypto.resolve_key(None)) - pending = store.read_pending() - manifest = store.read_manifest() - except Exception: # noqa: BLE001 - no durable pause to carry the answer to - return False - if ( - pending is None - or manifest is None - or pending.category != "item_approval" - or pending.status != "pending" - or pending.step_id != request.step_id - or pending.step_index != request.step_index - or manifest.run_id != request.run_id - ): - return False - if answer.decision == "decline": - store.cas_pending( - store.model_digest(pending), - pending.model_copy(update={"status": "rejected"}), - ) - return False - approver = ( - f"desktop:{answer.approver_id}" - if answer.approver_id is not None - else "desktop:unauthenticated" - ) - try: - approval = issue_resume_approval( - pending, - approver=approver, - resolution="approve_item", - bundle_version=bundle_version(manifest.bundle_dir), - workflow_name=pending.workflow_name, - run_id=manifest.run_id, - run_dir=run_dir, - ) - store.commit_approval_transition( - expected_pending=pending, - approval=approval, - target_status="approved", - ) - except (ResumeRefused, OSError): - return False - return True + return carry_answer_to_durable_pause(run_dir, request, answer) def _cmd_bench(args: argparse.Namespace) -> int: diff --git a/packages/flow/openadapt_flow/approval_mode/durable_carry.py b/packages/flow/openadapt_flow/approval_mode/durable_carry.py new file mode 100644 index 000000000..f00d1a9d4 --- /dev/null +++ b/packages/flow/openadapt_flow/approval_mode/durable_carry.py @@ -0,0 +1,87 @@ +"""Carry a save answer to the durable pause a run left when it stopped waiting. + +A run that gets no answer in time stops before the save and leaves a durable +``item_approval`` pause. An answer that arrives later, from the command line, +an agent or the phone, has to reach that pause: an approval authorizes exactly +one resume of it, and a decline ends it. Every answering route calls +:func:`carry_answer_to_durable_pause` right after it writes the answer. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any, Optional + + +def carry_answer_to_durable_pause( + run_dir: Path, + request: Any, + answer: Any, + *, + approver: Optional[str] = None, +) -> bool: + """Apply ``answer`` to the run's durable pause, if it left one for ``request``. + + Returns True when a ``resume`` must follow (an approval was committed). + Returns False when there is no matching pause (the run is still waiting, or + never paused durably), when the answer is a decline (the pause is ended), + or when the pause refuses the approval. + """ + + from openadapt_flow import crypto + from openadapt_flow.runtime.durable.approval import ( + ResumeRefused, + issue_resume_approval, + ) + from openadapt_flow.runtime.durable.checkpoint import CheckpointStore + from openadapt_flow.runtime.durable.program_checkpoint import bundle_version + + try: + store = CheckpointStore(run_dir, key=crypto.resolve_key(None)) + pending = store.read_pending() + manifest = store.read_manifest() + except Exception: # noqa: BLE001 - no durable pause to carry the answer to + return False + if ( + pending is None + or manifest is None + or pending.category != "item_approval" + or pending.status != "pending" + or pending.step_id != request.step_id + or pending.step_index != request.step_index + or manifest.run_id != request.run_id + ): + return False + if answer.decision == "decline": + store.cas_pending( + store.model_digest(pending), + pending.model_copy(update={"status": "rejected"}), + ) + return False + if approver is None: + approver = ( + f"desktop:{answer.approver_id}" + if answer.approver_id is not None + else "desktop:unauthenticated" + ) + try: + approval = issue_resume_approval( + pending, + approver=approver, + resolution="approve_item", + bundle_version=bundle_version(manifest.bundle_dir), + workflow_name=pending.workflow_name, + run_id=manifest.run_id, + run_dir=run_dir, + ) + store.commit_approval_transition( + expected_pending=pending, + approval=approval, + target_status="approved", + ) + except (ResumeRefused, OSError): + return False + return True + + +__all__ = ["carry_answer_to_durable_pause"] diff --git a/packages/flow/openadapt_flow/console/save_approvals.py b/packages/flow/openadapt_flow/console/save_approvals.py index 0261ecb1a..2e2d21316 100644 --- a/packages/flow/openadapt_flow/console/save_approvals.py +++ b/packages/flow/openadapt_flow/console/save_approvals.py @@ -47,6 +47,7 @@ from pydantic import BaseModel, ConfigDict, Field from openadapt_flow.approval_mode.approver import StepClass +from openadapt_flow.approval_mode.durable_carry import carry_answer_to_durable_pause from openadapt_flow.approval_mode.item_approval import ( APPROVAL_DIRNAME, ItemApprovalRefused, @@ -337,6 +338,12 @@ def decide_save_approval( store.write_answer(answer) except ItemApprovalRefused as exc: raise SaveApprovalRefused(exc.code, str(exc)) from exc + # A run that stopped waiting left a durable pause; the answer has to reach + # it, or an approval waits for a resume that never sees it and a decline + # leaves the pause open. + carry_answer_to_durable_pause( + run_dir, request, answer, approver=f"{DECISION_PORTAL_ROUTE}:person" + ) request_digest = ( "sha256:" + hashlib.sha256(payload.model_dump_json().encode("utf-8")).hexdigest() diff --git a/packages/flow/tests/test_approval_mode_each_item.py b/packages/flow/tests/test_approval_mode_each_item.py index eab22da5f..a08d24d71 100644 --- a/packages/flow/tests/test_approval_mode_each_item.py +++ b/packages/flow/tests/test_approval_mode_each_item.py @@ -653,3 +653,54 @@ def contexts(result): assert not gate.before_step(start).proceed # nobody answers this time assert not gate.before_save(edge).proceed assert len(store.requests()) == 2 + + +# -- the phone answers after the run stopped waiting --------------------------------- + + +def _answer_from_phone(item: "Item", action: str, **extra: Any): + from openadapt_flow.console import save_approvals + + request = item.store.requests()[0] + root = item.run_dir.parent + detail = save_approvals.save_approval_detail(root, request.request_id) + payload = save_approvals.SaveApprovalActionRequest( + action=action, + task_digest=detail["task_digest"], + task_signature=detail["task"]["signature"], + **extra, + ) + return save_approvals.decide_save_approval(root, request.request_id, payload) + + +def test_a_phone_approval_after_the_wait_reaches_the_pause_and_resumes( + tmp_path, monkeypatch +): + """The phone wrote the answer but never carried it to the durable pause, + so the run that had stopped waiting stayed paused.""" + monkeypatch.setenv("OPENADAPT_BUNDLE_KEY", _KEY) + item = Item(tmp_path, person=Person([None]), wait_s=5) + assert item.run().transaction_outcome == "HALTED_BEFORE_EFFECT" + assert item.pending().status == "pending" + + receipt = _answer_from_phone(item, "verify_and_resume") + + assert receipt.state.value == "accepted_pending_runner" + assert item.pending().status == "approved" + resumed_backend = FormBackend() + item.person = Person([]) + item.person.store = ItemApprovalStore(item.run_dir) + resumed = resume(item.run_dir, item.replayer(backend=resumed_backend), key=_KEY) + assert _save_delivered(resumed_backend) + assert resumed.transaction_outcome == "VERIFIED" + + +def test_a_phone_decline_after_the_wait_ends_the_pause(tmp_path, monkeypatch): + monkeypatch.setenv("OPENADAPT_BUNDLE_KEY", _KEY) + item = Item(tmp_path, person=Person([None]), wait_s=5) + item.run() + + receipt = _answer_from_phone(item, "reject", decline_reason="wrong_values") + + assert receipt.state.value == "rejected" + assert item.pending().status == "rejected"