diff --git a/packages/agent/tests/test_approval_flow.py b/packages/agent/tests/test_approval_flow.py index 81bd68d5d7..28d271cc24 100644 --- a/packages/agent/tests/test_approval_flow.py +++ b/packages/agent/tests/test_approval_flow.py @@ -149,9 +149,33 @@ def test_nobody_answers_needs_review_and_the_server_never_kills_the_wait(mockmed assert _records(mockmed["base_url"]) == [] -def test_a_person_approves_and_the_run_saves_once(mockmed): +def _approver_key(tmp_path, monkeypatch): + """A person's approver key, registered in a throwaway key directory. + + The run and the answer both read that directory, so only an answer this + key signed counts as the person's. + """ + from openadapt_flow.jit.authority import ( + KEY_PASSPHRASE_ENV, + generate_encrypted_key, + register_principal, + ) + from openadapt_flow.jit.lifecycle import KEY_DIR_ENV + + key_dir = tmp_path / "keys" + monkeypatch.setenv(KEY_DIR_ENV, str(key_dir)) + monkeypatch.setenv(KEY_PASSPHRASE_ENV, "synthetic") + key_path = key_dir / "front-desk.approver.pem" + public = generate_encrypted_key(key_path) + register_principal(key_dir, role="approver", principal_id="front-desk", public_key_hex=public) + return key_path + + +def test_a_person_approves_and_the_run_saves_once(mockmed, tmp_path, monkeypatch): import urllib.request + key_path = _approver_key(tmp_path, monkeypatch) + urllib.request.urlopen( urllib.request.Request(f"{mockmed['base_url']}api/reset", data=b"{}", method="POST"), timeout=10, @@ -190,6 +214,8 @@ def test_a_person_approves_and_the_run_saves_once(mockmed): "--route", "desktop_run_it", "--unauthenticated", + "--approver-key", + str(key_path), ) assert approved.returncode == 0, approved.stdout + approved.stderr final = bridge.dispatch("get_run", {"run_id": result["run_id"], "wait_seconds": 120}) diff --git a/packages/agent/tests/test_guard_tools.py b/packages/agent/tests/test_guard_tools.py index e5ea9748b7..86e18a0dd2 100644 --- a/packages/agent/tests/test_guard_tools.py +++ b/packages/agent/tests/test_guard_tools.py @@ -9,6 +9,7 @@ import pytest from openadapt_flow.agent.guard_tools import GuardBridge +from openadapt_flow.approval_mode.demo_approver import demo_approver from openadapt_flow.agent.mcp import build_server from openadapt_flow.guard import GUARD_TASK_SCHEMA, GuardedSave from openadapt_flow.guard_demo import ( @@ -45,7 +46,10 @@ def _server(base, tmp_path, approval="each_item"): ) ) guarded = GuardedSave( - parse_verify_config(verify_yaml(base)), tmp_path / "state", approval=approval + parse_verify_config(verify_yaml(base)), + tmp_path / "state", + approval=approval, + key_dir=tmp_path / "keys", ) return build_server(guard=GuardBridge(guarded, tasks)) @@ -65,6 +69,9 @@ def test_guarded_save_over_mcp(mockmed, tmp_path, mcp_client): base, db = mockmed server = _server(base, tmp_path) agent = FakeAgent(base) + # The test plays the person: a throwaway approver key registered in the + # guard's own key directory signs the answer. + signer = demo_approver(tmp_path / "keys") async def call(client, name, args): result = await client.call_tool(name, args) @@ -92,6 +99,8 @@ async def flow(): waiting["approval_run_dir"], waiting["approval_request_id"], waiting["approval_request_sha256"], + signer=signer, + key_dir=tmp_path / "keys", ) go = await call(client, "guard_commit", {"guard_id": begun["guard_id"]}) agent.click_save("p1", NOTE, fault="no_persist") diff --git a/packages/agent/tests/test_ledger.py b/packages/agent/tests/test_ledger.py index 77856a4e9d..f68568cb66 100644 --- a/packages/agent/tests/test_ledger.py +++ b/packages/agent/tests/test_ledger.py @@ -17,6 +17,7 @@ from openadapt_flow.agent.ledger_demo import WORKFLOW, SyntheticReferralApp, measure from openadapt_flow.agent.mcp import build_server from openadapt_flow.agent.runner import RunnerConfig +from openadapt_flow.approval_mode.demo_approver import demo_approver from openadapt_flow.approval_mode.item_approval import ItemApprovalStore, make_answer from openadapt_flow.approval_mode.model import ITEM_APPROVAL_ROUTE from openadapt_flow.console.save_approvals import ( @@ -37,14 +38,34 @@ ] +#: The person's approver key for each test's runs folder. The tests play the +#: person, so a throwaway key registered in the test's own key directory +#: signs every answer; the ledger reads that directory. +_PERSON: dict = {} + + +def _person(runs): + """(key_dir, signer) for this runs folder: the test's registered person.""" + key_dir = runs.with_name(runs.name + "-approver-keys") + if runs not in _PERSON: + _PERSON[runs] = (key_dir, demo_approver(key_dir)) + return _PERSON[runs] + + @pytest.fixture def world(tmp_path): runs = tmp_path / "runs" runs.mkdir() app = SyntheticReferralApp() bridge = AgentBridge(None, RunnerConfig(runs_dir=runs), sandbox=app, mode="sandbox") - ledger = TaskLedger(runs / "task-ledger") - return runs, app, build_server(bridge, ledger=ledger), ledger + key_dir, _signer = _person(runs) + ledger = TaskLedger(runs / "task-ledger", key_dir=key_dir) + yield runs, app, build_server(bridge, ledger=ledger), ledger + _PERSON.pop(runs, None) + + +def _store(runs, run_dir): + return ItemApprovalStore(run_dir, key_dir=_person(runs)[0]) def _inputs(item="R01", clinic="north", **extra): @@ -61,7 +82,7 @@ def answer(runs, decision="approve", *, decided_by="human", route=ITEM_APPROVAL_ """A person answers every waiting question, as the phone or Run it does.""" answered = 0 for run_dir, request in _waiting(runs): - store = ItemApprovalStore(run_dir) + store = _store(runs, run_dir) store.write_answer( make_answer( request, @@ -72,6 +93,7 @@ def answer(runs, decision="approve", *, decided_by="human", route=ITEM_APPROVAL_ approver_id=None, auth_method="none", policy=store.context(), + signer=_person(runs)[1] if decided_by == "human" else None, ) ) answered += 1 @@ -293,7 +315,7 @@ async def script(call): answer(runs, route="local_cli") # A forged answer file is read and refused by the ledger's own check. ((run_dir, request),) = _waiting(runs) - store = ItemApprovalStore(run_dir) + store = _store(runs, run_dir) forged = make_answer( request, decision="approve", @@ -302,6 +324,7 @@ async def script(call): approver_id=None, auth_method="none", policy=store.context(), + signer=_person(runs)[1], ).model_copy(update={"decided_by": "automation"}) store.write_answer(forged) view = ledger.get_answer(job["job_id"], asked["question_id"], 0) @@ -371,7 +394,7 @@ async def script(call): assert len(list_save_approvals(runs)) == 2 # The person says R11 saved and R26 didn't. for run_dir, request in _waiting(runs): - store = ItemApprovalStore(run_dir) + store = _store(runs, run_dir) decision = "approve" if "R11" in request.step_intent else "decline" store.write_answer( make_answer( @@ -383,6 +406,7 @@ async def script(call): approver_id=None, auth_method="none", policy=store.context(), + signer=_person(runs)[1], ) ) after = ledger.summary(job["job_id"]) @@ -598,7 +622,7 @@ def test_polling_a_stopped_run_asks_only_once(world): def test_an_unanswered_question_keeps_the_item_waiting_for_a_person(tmp_path): - ledger = TaskLedger(tmp_path / "task-ledger", question_ttl_s=1) + ledger = TaskLedger(tmp_path / "task-ledger", question_ttl_s=1, key_dir=_person(tmp_path)[0]) job = ledger.open_job("Synthetic work.", [], [{"item_id": "R01"}]) ledger.ask_user(job["job_id"], "R01: north or south?", "R01", 0) import time @@ -614,3 +638,4 @@ def test_an_unanswered_question_keeps_the_item_waiting_for_a_person(tmp_path): assert ledger.get_answer(job["job_id"], asked["question_id"], 0)["status"] == "yes" ledger.check_run({"request_id": job["job_id"] + ".R01", "inputs": {}}) assert ledger.summary(job["job_id"])["counts"] == {"pending": 1} + _PERSON.pop(tmp_path, None) diff --git a/packages/desktop/README.md b/packages/desktop/README.md index f3f1664198..7a120f009a 100644 --- a/packages/desktop/README.md +++ b/packages/desktop/README.md @@ -209,6 +209,7 @@ setting. | `record`, `list`, `info` | Record a task and inspect recordings | | `compile`, `qualify`, `replay`, `run` | Compile a recording, fill in its checks from the recording, replay it, or run it with a deployment settings file | | `scrub`, `review`, `approve`, `dismiss` | Remove personal data from a recording and review it before anything leaves the computer | +| `approver-key setup` | Create the keys that sign your answers to saves in Run it and on the paired phone, and print the command a person runs once to register each one | | `login`, `credential`, `rotate`, `push`, `report-break` | Sign in to OpenAdapt Cloud, manage the stored sign-in, send a recording or task, or report a stopped run | | `storage`, `health`, `cleanup`, `config` | Inspect and maintain local storage and settings | | `upload`, `backends` | Older upload commands. `upload --backend hosted_ingest` runs `push`; upload to your own S3 storage is turned off in this release. | diff --git a/packages/desktop/docs/OPERATOR_SCREENS.md b/packages/desktop/docs/OPERATOR_SCREENS.md index 95a7f110f3..50367db1d4 100644 --- a/packages/desktop/docs/OPERATOR_SCREENS.md +++ b/packages/desktop/docs/OPERATOR_SCREENS.md @@ -122,10 +122,23 @@ How it works: holds the command queue (`openadapt_desktop/ipc.py` `CONCURRENT_COMMANDS`). They read and answer only a run this engine started and hasn't finished, and only a request that is waiting now, for the exact digest the person saw. -- Desktop has no person-level sign-in (its Cloud credential belongs to the - organization), so each answer is recorded with `--unauthenticated` and - `--route desktop_run_it`. Flow re-checks the answer, re-reads the form and - runs the right-record check again before it saves. +- Desktop signs each answer with this computer's approver key, and Flow + counts it as a person's only because of that signature. Desktop has no + person-level sign-in (its Cloud credential belongs to the organization), + so the answer names no Cloud user (`--unauthenticated`, `--route + desktop_run_it`); it passes `--approver-key`, and the key's passphrase + reaches Flow only through the child process's environment. Flow re-checks + the answer, re-reads the form and runs the right-record check again before + it saves. +- Set the key up once per computer, at a terminal. With + `OPENADAPT_JIT_KEY_PASSPHRASE` set, run `openadapt-flow jit keygen --out + ~/.openadapt/approver/desktop-approver.pem`, then a person registers the + public key it prints: `openadapt-flow jit register --role approver + --principal NAME --public-key HEX`. Desktop then needs the same + passphrase in its environment. `openadapt-desktop approver-key setup` does + the keygen step instead and keeps a random passphrase in the OS keychain; + the register step is the same. Until a key is set up and registered, + Desktop sends no answer, and Run it shows these steps. - A late answer (the run already stopped waiting) is never turned into a save: Desktop doesn't resume the run, and says to run the item again. - Before the practice item runs, Desktop calls `openadapt-flow qualify drills diff --git a/packages/desktop/openadapt_desktop/approver_key.py b/packages/desktop/openadapt_desktop/approver_key.py new file mode 100644 index 0000000000..5dc547fea8 --- /dev/null +++ b/packages/desktop/openadapt_desktop/approver_key.py @@ -0,0 +1,207 @@ +"""The approver keys that sign a person's answer to a save. + +Flow counts an answer from Desktop's Run it, or from the paired phone through +the decision portal, as a person's only when it carries an Ed25519 signature +by a key registered under ``approvers`` in Flow's key directory +(``openadapt-flow jit register --role approver``). Desktop keeps one such key +for each route (``desktop`` for Run it, ``portal`` for the phone's answers, +which the Flow console it starts signs). For each: + +* the key itself, encrypted (PKCS#8, the same format as ``openadapt-flow jit + keygen``), in an owner-only file under ``~/.openadapt/approver``; +* its random passphrase in this computer's protected credential store (the + OS keychain), never in a file or on a command line. + +:func:`setup_approver_key` creates both and returns the public key. A person +then registers it at a terminal; Desktop never registers its own key. Without +a key and passphrase, :func:`approver_key_environment` returns ``None`` and +Desktop records no answer at all rather than an unsigned one. +""" + +from __future__ import annotations + +import os +import secrets +from dataclasses import dataclass +from pathlib import Path +from typing import Literal + +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +# Look the keyring helpers up on the module at call time. Binding them by name +# would bypass the test suite's in-memory keyring and reach the OS keychain. +from openadapt_desktop.auth import store as _store +from openadapt_desktop.private_fs import ensure_private_dir + +#: Where the encrypted keys live. Read at call time so tests can move it. +KEY_DIR = Path.home() / ".openadapt" / "approver" +#: The environment variable Flow reads a key's passphrase from. +PASSPHRASE_ENV = "OPENADAPT_JIT_KEY_PASSPHRASE" +#: The environment variable Flow's console reads the portal's key path from. +PORTAL_KEY_ENV = "OPENADAPT_PORTAL_APPROVER_KEY" + +Route = Literal["desktop", "portal"] +#: route -> (key file, keychain account, suggested approver name) +_ROUTES: dict[str, tuple[str, str, str]] = { + "desktop": ( + "desktop-approver.pem", + "desktop-approver-key-passphrase-v1", + "desktop-approver", + ), + "portal": ( + "portal-approver.pem", + "portal-approver-key-passphrase-v1", + "decision-portal", + ), +} +DEFAULT_PRINCIPAL = _ROUTES["desktop"][2] + + +class ApproverKeyError(RuntimeError): + """The Desktop approver key can't be created or opened.""" + + +@dataclass(frozen=True) +class ApproverKeySetup: + """A new or existing approver key, and how a person registers it.""" + + key_path: Path + public_key: str + created: bool + route: Route = "desktop" + + def register_command(self, principal: str | None = None) -> str: + name = principal or _ROUTES[self.route][2] + return ( + "openadapt-flow jit register --role approver " + f"--principal {name} --public-key {self.public_key}" + ) + + +def approver_key_path(route: Route = "desktop") -> Path: + return Path(KEY_DIR) / _ROUTES[route][0] + + +def _public_hex(key: Ed25519PrivateKey) -> str: + return ( + key.public_key() + .public_bytes(serialization.Encoding.Raw, serialization.PublicFormat.Raw) + .hex() + ) + + +def _open(path: Path, passphrase: str) -> Ed25519PrivateKey: + try: + key = serialization.load_pem_private_key( + path.read_bytes(), password=passphrase.encode("utf-8") + ) + except (OSError, TypeError, ValueError) as exc: + raise ApproverKeyError( + "The Desktop approver key can't be opened with the passphrase in " + "this computer's credential store." + ) from exc + if not isinstance(key, Ed25519PrivateKey): + raise ApproverKeyError("The Desktop approver key isn't an Ed25519 key.") + return key + + +def setup_approver_key(route: Route = "desktop") -> ApproverKeySetup: + """Create the approver key for ``route``, or return the one set up already. + + Fails closed when the OS credential store is unavailable: a key whose + passphrase sits next to it would protect nothing. + """ + + path = approver_key_path(route) + account = _ROUTES[route][1] + kr = _store._keyring() + passphrase = _store._kr_get(kr, account) + if path.exists(): + if not passphrase: + raise ApproverKeyError( + f"{path} exists but its passphrase isn't in this computer's " + "credential store. Move the file aside and set up a new key." + ) + return ApproverKeySetup(path, _public_hex(_open(path, passphrase)), False, route) + passphrase = secrets.token_urlsafe(32) + if not _store._kr_set(kr, account, passphrase): + raise ApproverKeyError( + "This computer's protected credential store is unavailable, so no " + "approver key was created." + ) + key = Ed25519PrivateKey.generate() + pem = key.private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.BestAvailableEncryption(passphrase.encode("utf-8")), + ) + ensure_private_dir(path.parent) + fd = os.open(path, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o600) + with os.fdopen(fd, "wb") as handle: + handle.write(pem) + return ApproverKeySetup(path, _public_hex(key), True, route) + + +def approver_key_environment( + route: Route = "desktop", +) -> tuple[Path, dict[str, str]] | None: + """The key path and the child-process environment that opens it. + + ``None`` when the key or its passphrase is missing. The passphrase goes + only into the Flow child's environment, never onto its command line. + """ + + path = approver_key_path(route) + if not path.is_file(): + return None + # The keychain holds the passphrase of a key Desktop created. A key made + # at a terminal with ``openadapt-flow jit keygen`` uses the passphrase in + # Desktop's own environment, as the Flow CLI does. + passphrase = _store._kr_get(_store._keyring(), _ROUTES[route][1]) or os.environ.get( + PASSPHRASE_ENV + ) + if not passphrase: + return None + return path, {PASSPHRASE_ENV: passphrase} + + +def setup_instructions(route: Route = "desktop") -> str: + """The one-time terminal steps that make this computer's answers count.""" + + path = approver_key_path(route) + name = _ROUTES[route][2] + return ( + "Set up the approver key once, at a terminal: with " + f"{PASSPHRASE_ENV} set, run `openadapt-flow jit keygen --out {path}`, " + "then have a person register the public key it prints with " + f"`openadapt-flow jit register --role approver --principal {name} " + "--public-key `. Start Desktop with the same " + f"{PASSPHRASE_ENV}, or run `openadapt-desktop approver-key setup` " + "instead of keygen to keep the passphrase in this computer's keychain." + ) + + +def portal_approver_environment() -> dict[str, str]: + """The Flow console's environment for signing the phone's answers. + + Empty when the portal key isn't set up: the console then records no + answer to a save (Flow refuses it), and everything else works as before. + """ + + found = approver_key_environment("portal") + if found is None: + return {} + path, env = found + return {PORTAL_KEY_ENV: str(path), **env} + + +__all__ = [ + "ApproverKeyError", + "ApproverKeySetup", + "approver_key_environment", + "approver_key_path", + "portal_approver_environment", + "setup_approver_key", + "setup_instructions", +] diff --git a/packages/desktop/openadapt_desktop/cli.py b/packages/desktop/openadapt_desktop/cli.py index 40fb66a58e..c067c569be 100644 --- a/packages/desktop/openadapt_desktop/cli.py +++ b/packages/desktop/openadapt_desktop/cli.py @@ -505,6 +505,29 @@ def cmd_cleanup(args: argparse.Namespace, engine: types.SimpleNamespace) -> None print(f" Freed: {_format_bytes(actions['bytes_freed'])}") +def cmd_approver_key(args: argparse.Namespace, engine: types.SimpleNamespace) -> None: + """Create this computer's approver key and print how to register it.""" + from openadapt_desktop.approver_key import ApproverKeyError, setup_approver_key + + labels = {"desktop": "Run it", "portal": "the paired phone (decision portal)"} + for route in ("desktop", "portal"): + try: + setup = setup_approver_key(route) + except ApproverKeyError as exc: + print(f"Approver key setup failed: {exc}") + sys.exit(1) + state = "Created" if setup.created else "Found" + print(f"{state} the approver key for {labels[route]}: {setup.key_path}") + print(f" Public key: {setup.public_key}") + principal = args.principal if route == "desktop" else None + print(f" Register it: {setup.register_command(principal)}") + engine.audit.log("approver_key_setup", route=route, created=setup.created) + print( + "A person runs the register commands at a terminal. Until then, Flow " + "refuses answers from this computer." + ) + + def cmd_config(args: argparse.Namespace, engine: types.SimpleNamespace) -> None: """Show current configuration.""" print(engine.config.model_dump_json(indent=2)) @@ -868,6 +891,7 @@ def cmd_doctor(args: argparse.Namespace, engine: types.SimpleNamespace) -> None: "config": cmd_config, "capabilities": cmd_capabilities, "doctor": cmd_doctor, + "approver-key": cmd_approver_key, } @@ -1008,6 +1032,18 @@ def main(argv: list[str] | None = None) -> None: # doctor subparsers.add_parser("doctor", help="Check dependencies and configuration") + # approver-key + p = subparsers.add_parser( + "approver-key", + help="Set up the key that signs a person's answers to saves", + ) + p.add_argument("action", choices=["setup"]) + p.add_argument( + "--principal", + default="desktop-approver", + help="The approver name to show in the register command", + ) + args = parser.parse_args(argv) config = EngineConfig() diff --git a/packages/desktop/openadapt_desktop/dispatch.py b/packages/desktop/openadapt_desktop/dispatch.py index b7dd7f4c1b..08c7cfa090 100644 --- a/packages/desktop/openadapt_desktop/dispatch.py +++ b/packages/desktop/openadapt_desktop/dispatch.py @@ -63,6 +63,11 @@ } ) _TEACHABLE_RUN_OUTCOMES = frozenset({"HALTED", "halt"}) +#: Flow refusals that mean this computer's approver key isn't set up or +#: registered. Desktop shows Flow's message, which says what to do. +_APPROVER_KEY_SETUP_CODES = frozenset( + {"human_answer_unsigned", "human_approver_unregistered", "approver_key_unusable"} +) _TEACH_CORRECTION_SCHEMA = "openadapt.desktop-teach-correction/v1" _TEACH_CORRECTION_META_KEY = "teach_correction" @@ -4342,11 +4347,12 @@ def answer_item_approval(self, **params: Any) -> dict: "error": "Desktop couldn't send your answer. Nothing was approved.", } if not answer.get("ok"): - return { - "ok": False, - "error": "Flow refused the answer, so nothing was approved.", - "code": str(answer.get("code") or ""), - } + code = str(answer.get("code") or "") + error = "Flow refused the answer, so nothing was approved." + if code in _APPROVER_KEY_SETUP_CODES: + # The person needs to set up or register the approver key. + error = str(answer.get("message") or error) + return {"ok": False, "error": error, "code": code} if answer.get("resume_required"): # The run stopped waiting before the answer arrived. Desktop # doesn't resume it: run the item again. diff --git a/packages/desktop/openadapt_desktop/flow_bridge.py b/packages/desktop/openadapt_desktop/flow_bridge.py index f8e60afb17..ae59328163 100644 --- a/packages/desktop/openadapt_desktop/flow_bridge.py +++ b/packages/desktop/openadapt_desktop/flow_bridge.py @@ -1080,9 +1080,15 @@ def item_approval_answer( ) -> dict: """Answer one waiting request as the person at this computer. + Flow counts the answer as a person's only when Desktop's approver key + signs it (:mod:`openadapt_desktop.approver_key`), so Desktop passes + ``--approver-key`` and puts the key's passphrase in the Flow child's + environment, never on its command line. Without a key, nothing is sent + and the answer is ``{"ok": False, "code": "human_answer_unsigned"}``. Desktop has no person-level sign-in (its Cloud credential belongs to - the organization), so every answer is recorded as ``--unauthenticated``: - Flow keeps it without anyone's name. Returns Flow's answer + the organization), so the answer carries no Cloud name + (``--unauthenticated``); the signature names the registered approver. + Returns Flow's answer (``{"ok": True, "decision", "answer_sha256", "resume_required"}``) or its refusal (``{"ok": False, "code", "message"}``). """ @@ -1113,7 +1119,24 @@ def item_approval_answer( args += ["--reason", str(reason)] elif reason is not None: raise ValueError("An approval has no reason") - result = self._run(args, timeout=60) + from openadapt_desktop.approver_key import ( + approver_key_environment, + setup_instructions, + ) + + key = approver_key_environment() + if key is None: + return { + "ok": False, + "code": "human_answer_unsigned", + "message": ( + "This computer has no approver key, so nothing was approved. " + + setup_instructions() + ), + } + key_path, key_env = key + args += ["--approver-key", str(key_path)] + result = self._run(args, timeout=60, env_overrides=key_env) try: answer = json.loads(result.stdout) except json.JSONDecodeError: diff --git a/packages/desktop/openadapt_desktop/portal/service.py b/packages/desktop/openadapt_desktop/portal/service.py index 98668fe575..238f9e2e89 100644 --- a/packages/desktop/openadapt_desktop/portal/service.py +++ b/packages/desktop/openadapt_desktop/portal/service.py @@ -537,6 +537,12 @@ def _spawn_console( expected_runner_id=remote_decision_runner_id, ) command.extend(["--remote-decisions", "--remote-decision-host", host]) + # The console signs the phone's answer to a save with the portal's + # approver key; without it Flow records no such answer. The passphrase + # goes only into the child's environment. + from openadapt_desktop.approver_key import portal_approver_environment + + env.update(portal_approver_environment()) process = self._popen( command, stdout=subprocess.PIPE, diff --git a/packages/desktop/tests/test_engine/test_item_approval.py b/packages/desktop/tests/test_engine/test_item_approval.py index a520a944a0..a517d1793a 100644 --- a/packages/desktop/tests/test_engine/test_item_approval.py +++ b/packages/desktop/tests/test_engine/test_item_approval.py @@ -345,6 +345,41 @@ def answer(run_dir, **kwargs): assert "already stopped waiting" in flow.answer["error"] +def test_a_missing_approver_key_tells_the_person_how_to_set_it_up(task) -> None: + disp, _events = task + + def unsigned(flow: _GatedFlow, shown: dict) -> None: + def answer(run_dir, **kwargs): + flow.answers.append(kwargs) + return { + "ok": False, + "code": "human_answer_unsigned", + "message": "This computer has no approver key, so nothing was approved.", + } + + flow.item_approval_answer = answer + request = shown["request"] + flow.answer = disp.dispatch( + "answer_item_approval", + { + "workflow_id": "wf-note", + "decision": "approve", + "request_id": request["request_id"], + "request_sha256": request["request_sha256"], + }, + ) + flow.answers.clear() + + flow = _GatedFlow(disp, unsigned) + disp.services._flow_bridge = flow + + _run(disp) + + assert flow.answer["ok"] is False + assert flow.answer["code"] == "human_answer_unsigned" + assert "no approver key" in flow.answer["error"] + + def test_no_run_means_nothing_to_approve(task) -> None: disp, _events = task answer = disp.dispatch( @@ -431,7 +466,9 @@ def approve(**_params): # ------------------------------------------------------------ the Flow bridge -def _bridge(calls: list, *, returncode: int = 0, stdout: str = "") -> FlowBridge: +def _bridge( + calls: list, *, returncode: int = 0, stdout: str = "", envs: list | None = None +) -> FlowBridge: class Proc: def __init__(self) -> None: self.returncode = returncode @@ -440,11 +477,23 @@ def __init__(self) -> None: def run(cmd, capture_output=True, text=True, timeout=None, env=None): calls.append(cmd) + if envs is not None: + envs.append(env) return Proc() return FlowBridge(runner=run) +@pytest.fixture +def approver_key(monkeypatch, tmp_path: Path): + """This computer's approver key, in a throwaway folder and fake keychain.""" + + from openadapt_desktop import approver_key as module + + monkeypatch.setattr(module, "KEY_DIR", tmp_path / "approver") + return module.setup_approver_key() + + def test_bridge_asks_flow_to_wait_before_each_save(monkeypatch, tmp_path: Path) -> None: _fake_pinned_flow(monkeypatch) calls: list = [] @@ -473,9 +522,12 @@ def test_bridge_asks_flow_to_wait_before_each_save(monkeypatch, tmp_path: Path) assert case[case.index("--approval-wait") + 1] == "600" -def test_bridge_answers_as_a_signed_out_person_from_run_it(monkeypatch, tmp_path: Path) -> None: +def test_bridge_signs_a_signed_out_persons_answer_from_run_it( + monkeypatch, tmp_path: Path, approver_key +) -> None: _fake_pinned_flow(monkeypatch) calls: list = [] + envs: list = [] ok = json.dumps( { "ok": True, @@ -485,7 +537,7 @@ def test_bridge_answers_as_a_signed_out_person_from_run_it(monkeypatch, tmp_path "resume_required": False, } ) - bridge = _bridge(calls, stdout=ok) + bridge = _bridge(calls, stdout=ok, envs=envs) answer = bridge.item_approval_answer( tmp_path, @@ -503,6 +555,12 @@ def test_bridge_answers_as_a_signed_out_person_from_run_it(monkeypatch, tmp_path assert "--unauthenticated" in command assert "--approver-id" not in command assert command[command.index("--reason") + 1] == "screen_problem" + # Signed with Desktop's approver key; the passphrase reaches Flow only + # through the child's environment. + assert command[command.index("--approver-key") + 1] == str(approver_key.key_path) + (env,) = envs + passphrase = env["OPENADAPT_JIT_KEY_PASSPHRASE"] + assert passphrase and not any(passphrase in part for part in command) with pytest.raises(ValueError): bridge.item_approval_answer( tmp_path, @@ -521,7 +579,116 @@ def test_bridge_answers_as_a_signed_out_person_from_run_it(monkeypatch, tmp_path ) -def test_bridge_reports_a_refused_answer(monkeypatch, tmp_path: Path) -> None: +def test_bridge_sends_no_answer_without_an_approver_key(monkeypatch, tmp_path: Path) -> None: + from openadapt_desktop import approver_key as module + + monkeypatch.setattr(module, "KEY_DIR", tmp_path / "approver") + _fake_pinned_flow(monkeypatch) + calls: list = [] + answer = _bridge(calls, stdout="{}").item_approval_answer( + tmp_path, decision="approve", request_id=REQUEST_ID, request_sha256=REQUEST_SHA + ) + assert answer["ok"] is False + assert answer["code"] == "human_answer_unsigned" + message = answer["message"] + key_path = str(module.approver_key_path()) + assert f"openadapt-flow jit keygen --out {key_path}" in message + assert "OPENADAPT_JIT_KEY_PASSPHRASE" in message + assert "openadapt-flow jit register --role approver --principal" in message + assert "--public-key" in message + assert calls == [] # never an unsigned answer + + +def test_a_key_made_with_flow_keygen_signs_with_desktops_passphrase( + monkeypatch, tmp_path: Path +) -> None: + """A key a person made at a terminal with jit keygen, not with Desktop.""" + + from cryptography.hazmat.primitives import serialization + from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + + from openadapt_desktop import approver_key as module + + monkeypatch.setattr(module, "KEY_DIR", tmp_path / "approver") + monkeypatch.setenv("OPENADAPT_JIT_KEY_PASSPHRASE", "synthetic-terminal-passphrase") + path = module.approver_key_path() + path.parent.mkdir(parents=True) + path.write_bytes( + Ed25519PrivateKey.generate().private_bytes( + serialization.Encoding.PEM, + serialization.PrivateFormat.PKCS8, + serialization.BestAvailableEncryption(b"synthetic-terminal-passphrase"), + ) + ) + found = module.approver_key_environment() + assert found == ( + path, + {"OPENADAPT_JIT_KEY_PASSPHRASE": "synthetic-terminal-passphrase"}, + ) + + +def test_approver_key_setup_is_encrypted_private_and_stable( + tmp_path: Path, approver_key, fake_keyring +) -> None: + import os + + from cryptography.hazmat.primitives import serialization + + from openadapt_desktop.approver_key import setup_approver_key + + assert approver_key.created is True + assert len(approver_key.public_key) == 64 + data = approver_key.key_path.read_bytes() + with pytest.raises(TypeError): # encrypted: no passphrase, no key + serialization.load_pem_private_key(data, password=None) + if os.name != "nt": + assert approver_key.key_path.stat().st_mode & 0o777 == 0o600 + assert not any( + value.encode() in data for value in fake_keyring._store.values() + ) # the passphrase lives in the keychain, not next to the key + again = setup_approver_key() + assert again.created is False + assert again.public_key == approver_key.public_key + command = again.register_command("front-desk") + assert "jit register --role approver --principal front-desk" in command + assert approver_key.public_key in command + + +def test_the_portal_gets_its_own_approver_key(tmp_path: Path, approver_key) -> None: + from openadapt_desktop.approver_key import ( + approver_key_environment, + portal_approver_environment, + setup_approver_key, + ) + + assert portal_approver_environment() == {} # not set up yet + portal = setup_approver_key("portal") + assert portal.key_path != approver_key.key_path + assert portal.public_key != approver_key.public_key + assert "--principal decision-portal" in portal.register_command() + env = portal_approver_environment() + assert env["OPENADAPT_PORTAL_APPROVER_KEY"] == str(portal.key_path) + desktop = approver_key_environment() + assert desktop is not None + assert env["OPENADAPT_JIT_KEY_PASSPHRASE"] != desktop[1]["OPENADAPT_JIT_KEY_PASSPHRASE"] + + +def test_approver_key_setup_fails_closed_without_a_keychain( + monkeypatch, tmp_path: Path +) -> None: + from openadapt_desktop import approver_key as module + + monkeypatch.setattr(module, "KEY_DIR", tmp_path / "approver") + monkeypatch.setattr("openadapt_desktop.auth.store._keyring", lambda: None) + with pytest.raises(module.ApproverKeyError): + module.setup_approver_key() + assert not module.approver_key_path().exists() + assert module.approver_key_environment() is None + + +def test_bridge_reports_a_refused_answer( + monkeypatch, tmp_path: Path, approver_key +) -> None: _fake_pinned_flow(monkeypatch) refused = json.dumps({"ok": False, "code": "request_expired", "message": "expired"}) bridge = _bridge([], returncode=2, stdout=refused) diff --git a/packages/desktop/tests/test_portal/test_service.py b/packages/desktop/tests/test_portal/test_service.py index 6416ae83eb..00a72dcc88 100644 --- a/packages/desktop/tests/test_portal/test_service.py +++ b/packages/desktop/tests/test_portal/test_service.py @@ -567,6 +567,40 @@ def test_a_deployment_without_remote_decisions_never_asks_for_the_flag( assert "OPENADAPT_RUNNER_TOKEN" not in seen["env"] +def test_the_console_signs_phone_answers_with_the_portal_approver_key( + monkeypatch, tmp_path +) -> None: + from openadapt_desktop import approver_key + + seen, popen = _capture_spawn(monkeypatch) + passphrase = "synthetic-portal-passphrase" + monkeypatch.setattr( + approver_key, + "portal_approver_environment", + lambda: { + "OPENADAPT_PORTAL_APPROVER_KEY": str(tmp_path / "portal-approver.pem"), + "OPENADAPT_JIT_KEY_PASSPHRASE": passphrase, + }, + ) + service = PortalService(configured(tmp_path / "data"), popen=popen) + service._start_console().stop() + assert seen["env"]["OPENADAPT_PORTAL_APPROVER_KEY"].endswith("portal-approver.pem") + assert seen["env"]["OPENADAPT_JIT_KEY_PASSPHRASE"] == passphrase + # The passphrase reaches the console only through its environment. + assert passphrase not in " ".join(seen["command"]) + + +def test_without_a_portal_approver_key_the_console_gets_none(monkeypatch, tmp_path) -> None: + from openadapt_desktop import approver_key + + monkeypatch.setattr(approver_key, "KEY_DIR", tmp_path / "no-keys") + monkeypatch.delenv("OPENADAPT_PORTAL_APPROVER_KEY", raising=False) + seen, popen = _capture_spawn(monkeypatch) + service = PortalService(configured(tmp_path / "data"), popen=popen) + service._start_console().stop() + assert "OPENADAPT_PORTAL_APPROVER_KEY" not in seen["env"] + + def test_remote_decisions_refuses_when_this_computer_is_not_registered( monkeypatch, tmp_path ) -> None: diff --git a/packages/flow/docs/AFTER_A_STOP.md b/packages/flow/docs/AFTER_A_STOP.md index 757441cef0..38dfab93b9 100644 --- a/packages/flow/docs/AFTER_A_STOP.md +++ b/packages/flow/docs/AFTER_A_STOP.md @@ -362,9 +362,10 @@ in [`benchmark/undo_proposal/results/`](../benchmark/undo_proposal/results/). offered for a workflow that has a loop. - **One attempt per run.** If an undo isn't verified, a person repairs the record by hand. -- **The approval is a local record.** Like a durable resume approval, it's as - strong as the operator's control of the run directory. It isn't signed, and - it isn't sent to the phone approval portal. +- **The approval is a local record.** It's as strong as the operator's + control of the run directory. Unlike a durable resume approval, which a + registered approver key signs, it isn't signed, and it isn't sent to the + phone approval portal. - **Undo crops from encrypted demonstrations.** `undo declare --from-bundle` refuses an encrypted demonstration bundle. - **Lint covers straight-line bundles only**, and it can't tell whether a check diff --git a/packages/flow/docs/APPROVAL_AND_VERIFICATION.md b/packages/flow/docs/APPROVAL_AND_VERIFICATION.md index 641a6485a6..d1de1b2364 100644 --- a/packages/flow/docs/APPROVAL_AND_VERIFICATION.md +++ b/packages/flow/docs/APPROVAL_AND_VERIFICATION.md @@ -125,8 +125,9 @@ 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 the decision portal on 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), with an answer a +registered approver key signed, 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, or none, and route) or the delegate's id and the grant @@ -170,9 +171,12 @@ 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 +answer that lands first wins. The portal signs the answer with its approver +key (`OPENADAPT_PORTAL_APPROVER_KEY`, registered with `jit register --role +approver`; see [`APPROVAL_MODES.md`](APPROVAL_MODES.md)), and a portal +without that key records no answer. A paired phone proves a device, not who +held it, so the answer names no Cloud user, 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. diff --git a/packages/flow/docs/APPROVAL_MODES.md b/packages/flow/docs/APPROVAL_MODES.md index 79c86b1189..a4e2252953 100644 --- a/packages/flow/docs/APPROVAL_MODES.md +++ b/packages/flow/docs/APPROVAL_MODES.md @@ -48,19 +48,25 @@ Desktop reads the requests and answers one of them: ```bash openadapt-flow item-approval show RUN_DIR # JSON; waiting requests -openadapt-flow item-approval approve RUN_DIR \ +OPENADAPT_JIT_KEY_PASSPHRASE=... openadapt-flow item-approval approve RUN_DIR \ --request-id ID --request-sha256 SHA256 \ - --decided-by human --route desktop_run_it \ - --approver-id CLOUD_USER_ID --auth-method cloud_aal2 # or --unauthenticated + --decided-by human --route desktop_run_it --unauthenticated \ + --approver-key APPROVER_KEY.pem 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 - on the runner's decision portal (`decision_portal`; see +- An answer names the request digest the person saw. A person answers from + the `desktop_run_it` route, or from their phone on the runner's decision + portal (`decision_portal`; see [`APPROVAL_AND_VERIFICATION.md`](APPROVAL_AND_VERIFICATION.md)), which echoes the signed task for that exact request. A decline carries one closed reason: `wrong_values`, `wrong_record` or `screen_problem`. +- A person's answer counts only when a registered approver key signs it + (below). `--decided-by human`, the route, `--unauthenticated` and a + Cloud user id are claims any process could make, so on their own they are + refused (`human_answer_unsigned`). A claimed Cloud sign-in + (`--auth-method cloud_aal2` or `cloud_aal1`) is refused too + (`auth_method_unverified`): nothing on the runner can check it. - 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 @@ -88,6 +94,39 @@ openadapt-flow item-approval decline RUN_DIR ... --reason wrong_values in a signed-in browser the run attached to, where the form is still open, checks the form against the approved request before the save. +### The approver key + +A person's answer carries an Ed25519 signature over the whole answer, by a +key registered under `approvers` in the key directory's `principals.json` +(`$OPENADAPT_JIT_KEY_DIR`, default `~/.openadapt/jit`). The run checks the +signature on every answer, including an answer file written around the +command. Each route signs with its own key: + +- Desktop's Run it signs with the Desktop approver key. Desktop creates it + with `openadapt-desktop approver-key setup`, which keeps the encrypted key + in an owner-only file and its passphrase in the OS keychain, and prints the + public key. +- The decision portal signs with the key at `OPENADAPT_PORTAL_APPROVER_KEY` + after the person answered on their phone. Without it, the portal records + no answer. +- At a terminal, a person passes their own key with `--approver-key`. The + key's passphrase comes from `OPENADAPT_JIT_KEY_PASSPHRASE`, never from the + command line. + +Create a key with `openadapt-flow jit keygen --out KEY.pem` (with the +passphrase variable set) and have a person register it at a terminal: + +```bash +openadapt-flow jit register --role approver --principal front-desk \ + --public-key PUBLIC_KEY_HEX +``` + +An approver key holds no other role, and an approver id that names an agent +or model is refused. The run refuses an answer signed by an unregistered key, +an agent's key, or the other route's key, an answer changed after it was +signed, and an answer signed by whoever started the run. Keep the key +directory and the approver keys away from the agents the run governs. + The report's `approval` block lists every answer as codes and digests: no values, no screenshot and no form state. An item is `approved` only when a person approved every save that was delivered and the engine's own outcome diff --git a/packages/flow/docs/GUARDED_SAVE.md b/packages/flow/docs/GUARDED_SAVE.md index f762d9d918..84d7dfef96 100644 --- a/packages/flow/docs/GUARDED_SAVE.md +++ b/packages/flow/docs/GUARDED_SAVE.md @@ -149,9 +149,14 @@ openadapt-flow item-approval show runs/guarded/ ``` ```bash -openadapt-flow item-approval approve runs/guarded/ --request-id --request-sha256 --decided-by human --route desktop_run_it --unauthenticated +openadapt-flow item-approval approve runs/guarded/ --request-id --request-sha256 --decided-by human --route desktop_run_it --unauthenticated --approver-key ``` +The answer counts only because the person's registered approver key signs +it (`OPENADAPT_JIT_KEY_PASSPHRASE` holds the key's passphrase; see +[`APPROVAL_MODES.md`](APPROVAL_MODES.md#the-approver-key)). `GuardedSave` +reads registered keys from `key_dir`, or from `$OPENADAPT_JIT_KEY_DIR`. + If `--guard-state` is the runs folder the console watches, or a folder directly inside it, the paired phone also shows the request. The receipt binds the approval: its `evidence.authorization.approver` names a person, and `self_approved` is diff --git a/packages/flow/docs/SELF_REPAIR.md b/packages/flow/docs/SELF_REPAIR.md index 543c3e86eb..9869333f07 100644 --- a/packages/flow/docs/SELF_REPAIR.md +++ b/packages/flow/docs/SELF_REPAIR.md @@ -192,9 +192,13 @@ show it: - **Terminal.** `run --self-repair` prints the exact command when it asks: ```bash - openadapt-flow item-approval approve runs/item-17 --request-id --request-sha256 --decided-by human --route desktop_run_it --unauthenticated + openadapt-flow item-approval approve runs/item-17 --request-id --request-sha256 --decided-by human --route desktop_run_it --unauthenticated --approver-key ``` + The person's registered approver key signs the answer, with its + passphrase in `OPENADAPT_JIT_KEY_PASSPHRASE` (see + [`APPROVAL_MODES.md`](APPROVAL_MODES.md#the-approver-key)). + - **Reject in the store.** Rejecting the candidate with `RepairStore.reject` while the run waits ends the wait as a decline. diff --git a/packages/flow/openadapt_flow/__main__.py b/packages/flow/openadapt_flow/__main__.py index b0c6e9a66c..5cee4c658c 100644 --- a/packages/flow/openadapt_flow/__main__.py +++ b/packages/flow/openadapt_flow/__main__.py @@ -4078,6 +4078,24 @@ def _cmd_resume(args: argparse.Namespace) -> int: "Nothing was executed." ) return 3 + elif pending.category != "business_decision": + # An item's authority is its signed answer, checked above and again by + # the run; a business decision carries its own signed receipt. Any + # other stored approval must be signed by a registered approver key: + # approval.json is a file any process could write. + from openadapt_flow.jit.lifecycle import default_key_dir + from openadapt_flow.runtime.durable.approval import stored_approval_refusal + + stored = store.read_approval() + if stored is not None: + code = stored_approval_refusal(stored, default_key_dir()) + if code is not None: + print( + f"Resume REFUSED ({code}): the approval isn't signed by a " + "registered approver key. Approve with openadapt flow approve " + "--approver-key. Nothing was executed." + ) + return 3 resume_mode = _resume_approval_mode(args, manifest.governed_authorization) if isinstance(resume_mode, int): return resume_mode @@ -4361,6 +4379,35 @@ def _resume_with(backend: "Backend") -> "RunReport": return 0 if report.success else 1 +def _approver_signer(args: argparse.Namespace, *, method: str) -> Any: + """The registered approver key from ``--approver-key``, or ``None`` after + printing why. A person's approval is signed with it; nothing else counts.""" + + from openadapt_flow.approval_mode.human_attestation import ( + AttestationRefused, + load_signer, + ) + from openadapt_flow.jit.lifecycle import default_key_dir + + key_path = getattr(args, "approver_key", None) + if not key_path: + print( + "A person's approval is signed with their registered approver key: " + "pass --approver-key (with OPENADAPT_JIT_KEY_PASSPHRASE set). " + "Create one with jit keygen, and register it at a terminal with " + "jit register --role approver." + ) + return None + key_dir = ( + Path(args.key_dir) if getattr(args, "key_dir", None) else default_key_dir() + ) + try: + return load_signer(key_path, key_dir, method=method) # type: ignore[arg-type] + except AttestationRefused as exc: + print(f"Approval refused ({exc.code}): {exc}") + return None + + def _cmd_approve(args: argparse.Namespace) -> int: """Record an AUTHENTICATED approval for a durably-paused run (P0-5). @@ -4370,8 +4417,6 @@ def _cmd_approve(args: argparse.Namespace) -> int: approval is refused). Also flips the pending escalation's ``status`` to ``approved`` for the audit trail / back-compat. """ - import getpass - from openadapt_flow import crypto from openadapt_flow.runtime.durable.approval import ( ResumeRefused, @@ -4421,9 +4466,18 @@ def _cmd_approve(args: argparse.Namespace) -> int: ) return 1 - # The approver identity defaults to the invoking OS user (a resume with a - # blank approver is refused by the durable library); --approver overrides. - approver = args.approver or getpass.getuser() + # The approver is whoever the registered approver key belongs to. A name + # on the command line, or the OS user, is a claim any process can make. + signer = _approver_signer(args, method="person_key") + if signer is None: + return 1 + if args.approver and args.approver != signer.principal_id: + print( + f"--approver {args.approver!r} doesn't match the approver key, which " + f"is registered as {signer.principal_id!r}." + ) + return 1 + approver = signer.principal_id try: bundle_ver = bundle_version(manifest.bundle_dir) except OSError: @@ -4461,6 +4515,11 @@ def _cmd_approve(args: argparse.Namespace) -> int: run_dir=run_dir, authorize_uncertain_retry=authorize_uncertain_retry, ) + from openadapt_flow.approval_mode.human_attestation import RESUME_DOMAIN + + approval = approval.model_copy( + update={"attestation": signer.attest(RESUME_DOMAIN, approval.signed_payload())} + ) # Bind the exact authority and pause-state transition in one filesystem # transaction. A concurrent resume cannot replace the retained approver or # leave approval.json describing authority that the live attempt did not use. @@ -4502,7 +4561,8 @@ def _cmd_item_approval(args: argparse.Namespace) -> int: verb = args.item_approval_cmd run_dir = Path(args.run_dir) - store = ItemApprovalStore(run_dir) + key_dir = getattr(args, "key_dir", None) + store = ItemApprovalStore(run_dir, key_dir=Path(key_dir) if key_dir else None) if verb == "show": rows = [request_summary(store, request) for request in store.requests()] if not args.all: @@ -4567,6 +4627,29 @@ def refuse(code: str, message: str) -> int: "approver_invalid", "a Cloud approver needs --auth-method cloud_aal2 or cloud_aal1", ) + signer = None + if delegate is None: + from openadapt_flow.approval_mode.human_attestation import ( + ROUTE_METHOD, + AttestationRefused, + load_signer, + ) + + method = ROUTE_METHOD.get(args.route) + if method is None: + return refuse("route_not_allowed", "a person answers from desktop_run_it") + if not getattr(args, "approver_key", None): + return refuse( + "human_answer_unsigned", + "a person's answer is signed with the Desktop approver key: pass " + "--approver-key. --decided-by human on its own proves nothing", + ) + try: + signer = load_signer( + args.approver_key, store.key_directory(), method=method + ) + except AttestationRefused as exc: + return refuse(exc.code, str(exc)) try: request = store.read_request(args.request_id) if args.request_sha256 != request.digest(): @@ -4593,6 +4676,7 @@ def refuse(code: str, message: str) -> int: decline_reason=getattr(args, "reason", None), approver=delegate, policy=store.context(), + signer=signer, ) store.write_answer(answer) except ItemApprovalRefused as exc: @@ -7508,6 +7592,25 @@ def _add_backend_flags(p: argparse.ArgumentParser) -> None: ) +def _add_approver_key_flags(p: argparse.ArgumentParser) -> None: + p.add_argument( + "--approver-key", + default=None, + help=( + "The person's encrypted approver key (jit keygen; registered with " + "jit register --role approver). Its passphrase comes from " + "OPENADAPT_JIT_KEY_PASSPHRASE. A person's approval is signed with " + "it; without it the approval is refused" + ), + ) + p.add_argument( + "--key-dir", + default=None, + help="Key directory with principals.json (default $OPENADAPT_JIT_KEY_DIR " + "or ~/.openadapt/jit)", + ) + + def _add_approval_flags(p: argparse.ArgumentParser) -> None: """Add the per-item approval flags to ``run``, ``resume`` and ``run-case``.""" @@ -9197,8 +9300,8 @@ def build_parser() -> argparse.ArgumentParser: "--approver", default=None, help=( - "Approver identity recorded on the approval (defaults to the " - "invoking OS user; a blank identity is refused at resume)" + "Optional check: the approver id the --approver-key must be " + "registered as. The approval names the key's registered approver" ), ) p.add_argument( @@ -9217,6 +9320,7 @@ def build_parser() -> argparse.ArgumentParser: "retry of the possibly dispatched step (ordinary approval refuses)" ), ) + _add_approver_key_flags(p) p.set_defaults(func=_cmd_approve) p = sub.add_parser( @@ -9295,6 +9399,7 @@ def build_parser() -> argparse.ArgumentParser: action="store_true", help="Desktop was signed out: record the answer without a name", ) + _add_approver_key_flags(q) if verb == "decline": q.add_argument( "--reason", diff --git a/packages/flow/openadapt_flow/agent/ledger.py b/packages/flow/openadapt_flow/agent/ledger.py index 9968531fcc..bcf8953b09 100644 --- a/packages/flow/openadapt_flow/agent/ledger.py +++ b/packages/flow/openadapt_flow/agent/ledger.py @@ -274,9 +274,19 @@ class TaskLedger: ``/task-ledger``), so questions show on the phone and in Desktop. """ - def __init__(self, root: Path | str, *, question_ttl_s: float = 4 * 3600.0): + def __init__( + self, + root: Path | str, + *, + question_ttl_s: float = 4 * 3600.0, + key_dir: Optional[Path | str] = None, + ): self.root = Path(root) self.question_ttl_s = question_ttl_s + #: Where a person's registered approver keys are read to check their + #: signed answers. ``None``: ``$OPENADAPT_JIT_KEY_DIR`` or + #: ``~/.openadapt/jit``. + self.key_dir = Path(key_dir) if key_dir is not None else None self._lock = threading.RLock() #: guard_id -> task_id, from guard_begin results this process saw. self._guards: dict[str, str] = {} @@ -706,7 +716,7 @@ def _ask( from openadapt_flow.approval_mode.model import ApprovalMode question_id = "q-" + uuid.uuid4().hex[:16] - store = ItemApprovalStore(self._question_dir(question_id)) + store = ItemApprovalStore(self._question_dir(question_id), key_dir=self.key_dir) # Person-only, pinned before anything is asked. The ledger is the # initiator, so no delegate grant can let an agent answer it. store.write_policy(DEFAULT_APPROVER_POLICY) @@ -760,7 +770,9 @@ def _poll(self, job: dict[str, Any], question: dict[str, Any]) -> bool: if question["status"] != "waiting": return False - store = ItemApprovalStore(self._question_dir(question["question_id"])) + store = ItemApprovalStore( + self._question_dir(question["question_id"]), key_dir=self.key_dir + ) try: request = store.read_request(question["request_id"]) if request.digest() != question["request_sha256"]: diff --git a/packages/flow/openadapt_flow/agent/ledger_demo.py b/packages/flow/openadapt_flow/agent/ledger_demo.py index 8dd4abeaae..37abc1768a 100644 --- a/packages/flow/openadapt_flow/agent/ledger_demo.py +++ b/packages/flow/openadapt_flow/agent/ledger_demo.py @@ -224,9 +224,19 @@ class SimulatedPerson: and decline one that came from a document. """ - def __init__(self, runs_root: Path, app: SyntheticReferralApp) -> None: + def __init__( + self, + runs_root: Path, + app: SyntheticReferralApp, + *, + signer: Any, + key_dir: Path, + ) -> None: self.runs_root = runs_root self.app = app + #: The simulated person's approver key, registered in ``key_dir``. + self.signer = signer + self.key_dir = key_dir self.answered: list[dict[str, str]] = [] self._stop = threading.Event() self._thread = threading.Thread(target=self._loop, daemon=True) @@ -262,7 +272,7 @@ def _loop(self) -> None: while not self._stop.is_set(): for run_dir, request in _waiting(self.runs_root): - store = ItemApprovalStore(run_dir) + store = ItemApprovalStore(run_dir, key_dir=self.key_dir) decision = self._decide(request.step_id, request.step_intent) store.write_answer( make_answer( @@ -276,6 +286,7 @@ def _loop(self) -> None: approver_id=None, auth_method="none", policy=store.context(), + signer=self.signer, ) ) self.answered.append({"kind": request.step_id, "answer": decision}) @@ -606,10 +617,21 @@ def run_arm(arm: str, workdir: Path) -> dict[str, Any]: runs.mkdir(parents=True) app = SyntheticReferralApp() bridge = AgentBridge(None, RunnerConfig(runs_dir=runs), sandbox=app, mode="sandbox") - ledger = None if arm == "no_ledger" else TaskLedger(runs / "task-ledger") + from openadapt_flow.approval_mode.demo_approver import demo_approver + + # The demo plays the person on synthetic data, so it registers a throwaway + # approver key in its own key directory and points the ledger there. Real + # use registers a person's key at a terminal (jit register --role approver). + key_dir = workdir / f"{arm}-demo-keys" + signer = demo_approver(key_dir) + ledger = ( + None + if arm == "no_ledger" + else TaskLedger(runs / "task-ledger", key_dir=key_dir) + ) server = build_server(bridge, ledger=ledger) started = time.monotonic() - with SimulatedPerson(runs, app) as person: + with SimulatedPerson(runs, app, signer=signer, key_dir=key_dir) as person: async def go() -> Agent: async with _session(server) as session: diff --git a/packages/flow/openadapt_flow/approval_mode/demo_approver.py b/packages/flow/openadapt_flow/approval_mode/demo_approver.py new file mode 100644 index 0000000000..a0555e6baf --- /dev/null +++ b/packages/flow/openadapt_flow/approval_mode/demo_approver.py @@ -0,0 +1,47 @@ +"""A throwaway approver key for demos that play a person on made-up data. + +A person's answer to a save counts only when a key registered under +``approvers`` signs it (:mod:`.human_attestation`). A demo that simulates the +person (``guard_demo``, the task-ledger demo) needs such a key, so it makes one +here, in a key directory the demo created for itself, and points its own run +at that directory. Nothing outside the demo trusts that directory, and the key +lives only in memory. + +Never use this for real work: there a person registers their key at a +terminal (``jit register --role approver``), in the key directory the +deployment's runs read. +""" + +from __future__ import annotations + +from pathlib import Path + +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey + +from openadapt_flow.approval_mode.human_attestation import ( + ApproverSigner, + AttestationMethod, +) + + +def demo_approver( + key_dir: Path, + principal_id: str = "demo-person", + *, + method: AttestationMethod = "desktop_key", +) -> ApproverSigner: + """Register a new in-memory approver key in the demo's own ``key_dir``.""" + + from openadapt_flow.jit.authority import public_hex, register_principal + + key = Ed25519PrivateKey.generate() + register_principal( + Path(key_dir), + role="approver", + principal_id=principal_id, + public_key_hex=public_hex(key), + ) + return ApproverSigner(principal_id=principal_id, method=method, signing=key) + + +__all__ = ["demo_approver"] diff --git a/packages/flow/openadapt_flow/approval_mode/durable_carry.py b/packages/flow/openadapt_flow/approval_mode/durable_carry.py index f00d1a9d4e..6106cb74c8 100644 --- a/packages/flow/openadapt_flow/approval_mode/durable_carry.py +++ b/packages/flow/openadapt_flow/approval_mode/durable_carry.py @@ -59,11 +59,11 @@ def carry_answer_to_durable_pause( ) return False if approver is None: - approver = ( - f"desktop:{answer.approver_id}" - if answer.approver_id is not None - else "desktop:unauthenticated" + # The registered approver whose key signed the answer, when it was. + name = answer.approver_id or ( + answer.attestation.signer if answer.attestation is not None else None ) + approver = f"desktop:{name}" if name is not None else "desktop:unauthenticated" try: approval = issue_resume_approval( pending, diff --git a/packages/flow/openadapt_flow/approval_mode/human_attestation.py b/packages/flow/openadapt_flow/approval_mode/human_attestation.py new file mode 100644 index 0000000000..d9b017e519 --- /dev/null +++ b/packages/flow/openadapt_flow/approval_mode/human_attestation.py @@ -0,0 +1,180 @@ +"""Proof that a person, not an agent, answered a save. + +A person's approval used to be a claim: ``--decided-by human`` with +``--unauthenticated`` or a named ``--auth-method`` was enough, and any process +that could run the CLI or write the run directory could approve its own save. +Now a person's answer counts only when it carries a :class:`HumanAttestation`: +an Ed25519 signature over the answer, by a key registered under ``approvers`` +in the key directory's ``principals.json`` +(:class:`~openadapt_flow.jit.authority.PrincipalRegistry`). + +* Desktop's Run it signs with the Desktop approver key (``desktop_key``). +* The decision portal signs, after the person answered on their phone, with + the portal's approver key (``portal_key``). +* ``openadapt-flow approve`` signs a durable resume approval with the + person's own approver key (``person_key``); ``resume`` refuses a stored + approval without one. + +The registry and the keys follow the trust root of program admission +(:mod:`openadapt_flow.jit.authority`): a person registers each key at a +terminal (``jit register --role approver``), an approver key can't hold any +other role, and an approver id that names an agent or model is refused. Keep +the key directory and the approver keys away from the agents they govern. + +Every check fails closed: no key directory, no registered approver, a wrong +key, a changed answer, or a route signed with the other route's method is a +refusal. A claimed Cloud sign-in (``cloud_aal1``/``cloud_aal2``) is refused +too, because nothing here can verify it. +""" + +from __future__ import annotations + +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Literal, Mapping, Optional + +from cryptography.hazmat.primitives.asymmetric.ed25519 import Ed25519PrivateKey +from pydantic import BaseModel, ConfigDict, Field + +from openadapt_flow.approval_mode.model import ( + DECISION_PORTAL_ROUTE, + ITEM_APPROVAL_ROUTE, +) + +#: The decision portal's encrypted approver key. Its passphrase comes from +#: ``OPENADAPT_JIT_KEY_PASSPHRASE``. Unset: the portal can't record an answer. +PORTAL_KEY_ENV = "OPENADAPT_PORTAL_APPROVER_KEY" +ANSWER_DOMAIN = b"openadapt-flow.approval.human-answer/v1\x00" +RESUME_DOMAIN = b"openadapt-flow.durable.resume-approval/v1\x00" +_ID = r"^[A-Za-z0-9][A-Za-z0-9._:@-]{0,127}$" + +#: ``desktop_key``: Desktop's Run it. ``portal_key``: the decision portal. +#: ``person_key``: a person at a terminal (``openadapt-flow approve``). +AttestationMethod = Literal["desktop_key", "portal_key", "person_key"] + +#: The signing method each person route must use. +ROUTE_METHOD: dict[str, AttestationMethod] = { + ITEM_APPROVAL_ROUTE: "desktop_key", + DECISION_PORTAL_ROUTE: "portal_key", +} + + +class HumanAttestation(BaseModel): + """A registered approver key's signature over one answer.""" + + model_config = ConfigDict(extra="forbid", frozen=True) + + signer: str = Field(pattern=_ID) + method: AttestationMethod + signature: str = Field(min_length=1, max_length=256) + + +@dataclass(frozen=True) +class ApproverSigner: + """An approver key ready to sign, and who the registry says it is.""" + + principal_id: str + method: AttestationMethod + signing: Ed25519PrivateKey + + def attest(self, domain: bytes, payload: Any) -> HumanAttestation: + from openadapt_flow.jit.authority import _sign + + return HumanAttestation( + signer=self.principal_id, + method=self.method, + signature=_sign(self.signing, domain, payload), + ) + + +class AttestationRefused(ValueError): + def __init__(self, code: str, message: str) -> None: + super().__init__(message) + self.code = code + + +def load_signer( + key_path: Path | str, + key_dir: Path, + *, + method: AttestationMethod, + env: Optional[Mapping[str, str]] = None, +) -> ApproverSigner: + """Open an encrypted approver key and name it from the registry. + + The principal is the name the key is registered under, never a name the + caller passes. Raises :class:`AttestationRefused`. + """ + + from openadapt_flow.jit.authority import ( + AuthorityError, + PrincipalRegistry, + load_encrypted_key, + public_hex, + ) + + try: + key = load_encrypted_key(key_path, env=env) + registry = PrincipalRegistry.load(key_dir) + except (AuthorityError, OSError, ValueError) as exc: + raise AttestationRefused("approver_key_unusable", str(exc)) from exc + found = registry.lookup(public_hex(key)) + if found is None or found[0] != "approver": + raise AttestationRefused( + "human_approver_unregistered", + "this key isn't registered as an approver; a person registers it " + "with jit register --role approver", + ) + return ApproverSigner(principal_id=found[1], method=method, signing=key) + + +def refusal( + attestation: Optional[HumanAttestation], + *, + domain: bytes, + payload: Any, + key_dir: Optional[Path], + method: Optional[AttestationMethod] = None, +) -> Optional[str]: + """A closed reason this attestation proves nothing, or ``None``.""" + + if attestation is None: + return "human_answer_unsigned" + if method is not None and attestation.method != method: + return "attestation_method_mismatch" + if key_dir is None: + return "human_answer_unverifiable" + from openadapt_flow.jit.authority import PrincipalRegistry, _verify + + try: + registry = PrincipalRegistry.load(key_dir) + except (OSError, ValueError): + return "human_answer_unverifiable" + public = registry.approvers.get(attestation.signer) + if public is None: + return "human_approver_unregistered" + if not _verify(public, domain, payload, attestation.signature): + return "human_answer_signature_invalid" + return None + + +def portal_signer( + key_dir: Path, *, env: Optional[Mapping[str, str]] = None +) -> ApproverSigner: + """The decision portal's approver key, from :data:`PORTAL_KEY_ENV`. + + Raises :class:`AttestationRefused` when it isn't configured, so a portal + without a key records no answer rather than an unsigned one. + """ + + import os + + source = os.environ if env is None else env + path = source.get(PORTAL_KEY_ENV) + if not path: + raise AttestationRefused( + "human_answer_unverifiable", + f"the decision portal has no approver key; set {PORTAL_KEY_ENV} to " + "a key registered with jit register --role approver", + ) + return load_signer(path, key_dir, method="portal_key", env=env) diff --git a/packages/flow/openadapt_flow/approval_mode/item_approval.py b/packages/flow/openadapt_flow/approval_mode/item_approval.py index 6622cbe46a..159caed5a6 100644 --- a/packages/flow/openadapt_flow/approval_mode/item_approval.py +++ b/packages/flow/openadapt_flow/approval_mode/item_approval.py @@ -56,6 +56,15 @@ StepClass, WebhookRefused, ) +from openadapt_flow.approval_mode.human_attestation import ( + ANSWER_DOMAIN, + ROUTE_METHOD, + ApproverSigner, + HumanAttestation, +) +from openadapt_flow.approval_mode.human_attestation import ( + refusal as attestation_refusal, +) from openadapt_flow.approval_mode.model import ( DECISION_PORTAL_ROUTE, ITEM_APPROVAL_ROUTE, @@ -305,17 +314,29 @@ 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 + #: A registered approver key's signature over the rest of the answer. + #: Required for a person's answer (:mod:`.human_attestation`). + attestation: Optional[HumanAttestation] = 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.attestation is None: + data.pop("attestation", None) return data def digest(self) -> str: return _digest(self) + def signed_payload(self) -> dict[str, Any]: + """What an approver key signs: the answer without its signature.""" + + data = self.model_dump(mode="json") + data.pop("attestation", None) + return data + def approver_identity(self) -> ApproverIdentityV1: """Who decided, for the audit trail.""" @@ -362,6 +383,9 @@ class ApproverContext: grant_refusal: Optional[str] = None initiator: Optional[str] = None initiator_known: bool = False + #: Where registered approver keys are looked up. ``None`` means a + #: person's answer can't be verified, so it is refused. + key_dir: Optional[Path] = None def _as_context( @@ -408,6 +432,39 @@ def _delegate_refusal( return None +def _person_refusal( + answer: ItemApprovalAnswer, context: ApproverContext +) -> Optional[str]: + """Why this answer isn't a verified person's, or ``None``. + + ``decided_by``, the route, ``--unauthenticated`` and a named sign-in are + claims anyone who can run the CLI could make. Only a registered approver + key's signature, for the route's own method, makes the answer a + person's. A Cloud sign-in can't be verified here, so claiming one is + refused rather than trusted. Self-approval is refused whoever claims to + have answered. + """ + + if answer.auth_method != "none": + return "auth_method_unverified" + refusal_code = attestation_refusal( + answer.attestation, + domain=ANSWER_DOMAIN, + payload=answer.signed_payload(), + key_dir=context.key_dir, + method=ROUTE_METHOD.get(answer.route), + ) + if refusal_code is not None: + return refusal_code + signer = answer.attestation.signer if answer.attestation else "" + if answer.approver_id is not None and answer.approver_id != signer: + return "answer_fields_inconsistent" + initiator = (context.initiator or "").lower() + if initiator and initiator in {signer.lower(), (answer.approver_id or "").lower()}: + return "self_approval" + return None + + def answer_refusal( request: ItemApprovalRequest, answer: ItemApprovalAnswer, @@ -439,7 +496,12 @@ def answer_refusal( return "route_not_allowed" if approver is not None and approver.auth.value != answer.approver_kind: return "answer_fields_inconsistent" + refusal_code = _person_refusal(answer, context) + if refusal_code is not None: + return refusal_code else: + if answer.attestation is not None: + return "answer_fields_inconsistent" if answer.decided_by != "automation": return "answer_fields_inconsistent" if answer.route not in DELEGATED_ROUTES: @@ -665,6 +727,11 @@ def _read_initiator(self) -> tuple[bool, Optional[str]]: return False, None return True, value + def key_directory(self) -> Path: + """Where this run's registered keys (``principals.json``) are read.""" + + return self._key_directory() + def _key_directory(self) -> Path: if self._key_dir is not None: return self._key_dir @@ -685,6 +752,7 @@ def make(**fields: Any) -> ApproverContext: return ApproverContext( initiator=initiator, initiator_known=known, + key_dir=self._key_directory(), **fields, ) @@ -1100,9 +1168,14 @@ def make_answer( now: Optional[datetime] = None, approver: Optional[ApproverIdentityV1] = None, policy: "ApproverPolicyV1 | ApproverContext | None" = None, + signer: Optional[ApproverSigner] = None, ) -> ItemApprovalAnswer: """Build and check one answer. Raises :class:`ItemApprovalRefused`. + A person's answer must be signed: ``signer`` is the Desktop or decision + portal approver key (:func:`.human_attestation.load_signer`). Without it, + the answer is refused (``human_answer_unsigned``). + ``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 @@ -1139,6 +1212,12 @@ def make_answer( raise ItemApprovalRefused( "answer_fields_invalid", "the answer's fields are not valid" ) from exc + if signer is not None: + answer = answer.model_copy( + update={ + "attestation": signer.attest(ANSWER_DOMAIN, answer.signed_payload()) + } + ) refusal = answer_refusal(request, answer, now=now, policy=context) if refusal is not None: raise ItemApprovalRefused(refusal, f"the answer was refused ({refusal})") diff --git a/packages/flow/openadapt_flow/cli_self_repair.py b/packages/flow/openadapt_flow/cli_self_repair.py index f889b4a690..72aa77facf 100644 --- a/packages/flow/openadapt_flow/cli_self_repair.py +++ b/packages/flow/openadapt_flow/cli_self_repair.py @@ -448,8 +448,11 @@ def announce(run_dir: Path, request_id: str, request_sha256: str) -> None: + command( f"item-approval approve {run_dir} --request-id {request_id} " f"--request-sha256 {request_sha256} --decided-by human " - "--route desktop_run_it --unauthenticated" - ), + "--route desktop_run_it --unauthenticated " + "--approver-key YOUR_APPROVER_KEY.pem" + ) + + "\n(with OPENADAPT_JIT_KEY_PASSPHRASE set; the key is registered " + "with jit register --role approver)", flush=True, ) diff --git a/packages/flow/openadapt_flow/console/save_approvals.py b/packages/flow/openadapt_flow/console/save_approvals.py index fa1f5d0c00..e383bda46c 100644 --- a/packages/flow/openadapt_flow/console/save_approvals.py +++ b/packages/flow/openadapt_flow/console/save_approvals.py @@ -358,6 +358,15 @@ def decide_save_approval( "a decline needs exactly one reason; an approval takes none", ) store = ItemApprovalStore(run_dir) + from openadapt_flow.approval_mode.human_attestation import ( + AttestationRefused, + portal_signer, + ) + + try: + signer = portal_signer(store.key_directory()) + except AttestationRefused as exc: + raise SaveApprovalRefused(exc.code, str(exc)) from exc try: answer = make_answer( request, @@ -368,6 +377,7 @@ def decide_save_approval( auth_method="none", decline_reason=payload.decline_reason, policy=store.context(), + signer=signer, ) store.write_answer(answer) except ItemApprovalRefused as exc: diff --git a/packages/flow/openadapt_flow/guard.py b/packages/flow/openadapt_flow/guard.py index 92b04395ad..76a4b505c9 100644 --- a/packages/flow/openadapt_flow/guard.py +++ b/packages/flow/openadapt_flow/guard.py @@ -306,11 +306,17 @@ class GuardedSave: verify: VerifyConfig state_dir: Path approval: ApprovalSetting = "each_item" + #: Where registered approver keys (``principals.json``) are read to check + #: a person's signed answer. ``None``: ``$OPENADAPT_JIT_KEY_DIR`` or + #: ``~/.openadapt/jit``. Keep it away from the agent this guard governs. + key_dir: Optional[Path] = None def __post_init__(self) -> None: if self.approval not in APPROVAL_MODES: raise ValueError("approval must be each_item or not_gated") self.state_dir = Path(self.state_dir) + if self.key_dir is not None: + self.key_dir = Path(self.key_dir) @classmethod def from_file( @@ -514,7 +520,7 @@ def _await_approval( from openadapt_flow.approval_mode.model import ApprovalMode from openadapt_flow.approval_mode.switch import canonical_json - store = ItemApprovalStore(self.dir) + store = ItemApprovalStore(self.dir, key_dir=self.owner.key_dir) if self._request is None: if not screenshot_png: raise GuardRefused( @@ -589,7 +595,9 @@ def _await_approval( } self._approver = { "kind": "person", - "id": answer.approver_id or "unauthenticated-person", + "id": answer.approver_id + or (answer.attestation.signer if answer.attestation else None) + or "unauthenticated-person", "policy_ref": self._policy_ref, } return None diff --git a/packages/flow/openadapt_flow/guard_demo.py b/packages/flow/openadapt_flow/guard_demo.py index 55cdae2511..8a18c3e52c 100644 --- a/packages/flow/openadapt_flow/guard_demo.py +++ b/packages/flow/openadapt_flow/guard_demo.py @@ -29,12 +29,15 @@ import zlib from html.parser import HTMLParser from pathlib import Path -from typing import Any, Optional +from typing import TYPE_CHECKING, Any, Optional from openadapt_flow.guard import GuardedSave, GuardRefused, GuardTask from openadapt_flow.mockmed.chart_pages import READER_PASSWORD, READER_USERNAME from openadapt_flow.verify_run import parse_verify_config +if TYPE_CHECKING: + from openadapt_flow.approval_mode.human_attestation import ApproverSigner + NOTE = "Referral to orthopedics for knee pain" @@ -149,8 +152,20 @@ def click_save(self, patient_id: str, note: str, *, fault: str = "") -> None: self._send(url, json.dumps(body).encode(), "application/json") -def person_approves(run_dir: str, request_id: str, request_sha256: str) -> None: - """What ``openadapt-flow item-approval approve`` does for a person.""" +def person_approves( + run_dir: str, + request_id: str, + request_sha256: str, + *, + signer: "ApproverSigner", + key_dir: Optional[Path] = None, +) -> None: + """What ``openadapt-flow item-approval approve`` does for a person. + + ``signer`` is the person's registered approver key; the answer counts only + because it signs it. ``key_dir`` is where that key is registered (the + guard's own ``key_dir``). + """ from openadapt_flow.approval_mode.item_approval import ( ItemApprovalStore, @@ -158,7 +173,7 @@ def person_approves(run_dir: str, request_id: str, request_sha256: str) -> None: ) from openadapt_flow.approval_mode.model import ITEM_APPROVAL_ROUTE - store = ItemApprovalStore(Path(run_dir)) + store = ItemApprovalStore(Path(run_dir), key_dir=key_dir) request = store.read_request(request_id) if request.digest() != request_sha256: raise ValueError("not the request the person saw") @@ -171,6 +186,7 @@ def person_approves(run_dir: str, request_id: str, request_sha256: str) -> None: approver_id=None, auth_method="none", policy=store.context(), + signer=signer, ) ) @@ -180,9 +196,14 @@ def run_case( guard: GuardedSave, agent: FakeAgent, *, + signer: "ApproverSigner", task_id: Optional[str] = None, ) -> dict[str, Any]: - """Run one demo case and return what happened, step by step.""" + """Run one demo case and return what happened, step by step. + + ``signer`` is the simulated person's approver key, registered in + ``guard.key_dir``. + """ task = referral_task(task_id or f"demo-{case}-0001") opened = "p2" if case == "wrong_patient" else "p1" @@ -208,6 +229,8 @@ def run_case( commit.approval_run_dir, commit.approval_request_id, commit.approval_request_sha256 or "", + signer=signer, + key_dir=guard.key_dir, ) steps.append("person approved the save") commit = g.commit() @@ -237,13 +260,24 @@ def main(argv: Optional[list[str]] = None) -> int: try: base = url.rstrip("/") with tempfile.TemporaryDirectory() as tmp: + from openadapt_flow.approval_mode.demo_approver import demo_approver + + # The demo plays the person on made-up data, so it registers a + # throwaway approver key in its own temporary key directory and + # points the guard there. Real use registers a person's key at a + # terminal (jit register --role approver). + key_dir = Path(tmp) / "demo-keys" + signer = demo_approver(key_dir) guard = GuardedSave( - parse_verify_config(verify_yaml(base)), Path(tmp), approval="each_item" + parse_verify_config(verify_yaml(base)), + Path(tmp) / "state", + approval="each_item", + key_dir=key_dir, ) agent = FakeAgent(base) for case in ("correct", "wrong_patient", "dropped"): before = len(db.snapshot()["records"]) - out = run_case(case, guard, agent) + out = run_case(case, guard, agent, signer=signer) after = len(db.snapshot()["records"]) print(f"== {case}") for step in out["steps"]: diff --git a/packages/flow/openadapt_flow/jit/authority.py b/packages/flow/openadapt_flow/jit/authority.py index 66b5ca5938..bbcdc4f297 100644 --- a/packages/flow/openadapt_flow/jit/authority.py +++ b/packages/flow/openadapt_flow/jit/authority.py @@ -90,7 +90,7 @@ #: from the environment so it stays out of shell history. KEY_PASSPHRASE_ENV = "OPENADAPT_JIT_KEY_PASSPHRASE" -Role = Literal["policy_signer", "admitter_key", "agent"] +Role = Literal["policy_signer", "admitter_key", "agent", "approver"] _ID = r"^[A-Za-z0-9][A-Za-z0-9._:@-]{0,127}$" _HEX_KEY = re.compile(r"^[0-9a-f]{64}$") @@ -231,12 +231,16 @@ class PrincipalRegistry(BaseModel): admitter_keys: dict[str, str] = Field(default_factory=dict) #: agent principal id -> public key the agent signs with. agents: dict[str, str] = Field(default_factory=dict) + #: person or decision-portal id -> public key that signs a person's answer + #: to a save (:mod:`openadapt_flow.approval_mode.human_attestation`). Held + #: by Desktop or the decision portal, never by an agent. + approvers: dict[str, str] = Field(default_factory=dict) @model_validator(mode="after") def _check(self) -> "PrincipalRegistry": seen: dict[str, str] = {} keys: dict[str, str] = {} - for role in ("policy_signers", "admitter_keys", "agents"): + for role in ("policy_signers", "admitter_keys", "agents", "approvers"): for pid, pub in getattr(self, role).items(): if not re.match(_ID, pid): raise ValueError(f"invalid principal id {pid!r}") @@ -255,6 +259,12 @@ def _check(self) -> "PrincipalRegistry": raise ValueError( f"{pid!r} names an agent or model; only a person signs policy" ) + for pid in self.approvers: + if _is_unregistered_agent_name(pid): + raise ValueError( + f"{pid!r} names an agent or model; only a person's Desktop or " + "decision portal signs an answer" + ) return self @classmethod @@ -277,6 +287,7 @@ def lookup(self, public_key_hex: str) -> Optional[tuple[Role, str]]: ("policy_signer", self.policy_signers), ("admitter_key", self.admitter_keys), ("agent", self.agents), + ("approver", self.approvers), ) for role, entries in roles: for pid, pub in entries.items(): @@ -295,6 +306,7 @@ def register_principal( "policy_signer": "policy_signers", "admitter_key": "admitter_keys", "agent": "agents", + "approver": "approvers", }.get(role) if field_name is None: raise AuthorityError(f"unknown role {role!r}") @@ -639,6 +651,10 @@ def principal_from_key( raise AuthorityError( f"{pid!r} signs admission policy; a person admits at a terminal" ) + if role == "approver": + raise AuthorityError( + f"{pid!r} signs a person's answers to saves; it can't admit a program" + ) if role == "admitter_key": if policy is not None: raise AuthorityError( diff --git a/packages/flow/openadapt_flow/jit/cli.py b/packages/flow/openadapt_flow/jit/cli.py index c4f7dcb7bf..6659e4cb16 100644 --- a/packages/flow/openadapt_flow/jit/cli.py +++ b/packages/flow/openadapt_flow/jit/cli.py @@ -332,11 +332,14 @@ def common(p: argparse.ArgumentParser, *, program: bool = True) -> None: common(p, program=False) p.add_argument("--out", required=True) p = verbs.add_parser( - "register", help="Register a key as a policy signer, admitter key or agent" + "register", + help="Register a key as a policy signer, admitter key, agent or approver", ) common(p, program=False) p.add_argument( - "--role", required=True, choices=["policy_signer", "admitter_key", "agent"] + "--role", + required=True, + choices=["policy_signer", "admitter_key", "agent", "approver"], ) p.add_argument("--principal", required=True) p.add_argument("--public-key", required=True, help="hex, from jit keygen") diff --git a/packages/flow/openadapt_flow/runtime/durable/approval.py b/packages/flow/openadapt_flow/runtime/durable/approval.py index 47ccbdfcf8..cea375a75a 100644 --- a/packages/flow/openadapt_flow/runtime/durable/approval.py +++ b/packages/flow/openadapt_flow/runtime/durable/approval.py @@ -28,6 +28,8 @@ from pydantic import BaseModel, Field +from openadapt_flow.approval_mode.human_attestation import HumanAttestation + def _now() -> str: return datetime.now(timezone.utc).isoformat() @@ -114,6 +116,46 @@ class ApprovalRecord(BaseModel): #: landed. This separate, explicit authority is set only after an operator #: reconciles the live/system-of-record state and decides one retry is safe. authorize_uncertain_retry: bool = False + #: A registered approver key's signature over the rest of this record. + #: ``openadapt-flow approve`` signs it, and ``resume`` refuses a stored + #: approval without a valid one (:func:`stored_approval_refusal`). + attestation: Optional[HumanAttestation] = None + + def signed_payload(self) -> dict[str, Any]: + """What an approver key signs: the record without its signature.""" + + data = self.model_dump(mode="json") + data.pop("attestation", None) + return data + + +def stored_approval_refusal( + approval: "ApprovalRecord", key_dir: Optional[Path] +) -> Optional[str]: + """Why a stored resume approval isn't a verified person's, or ``None``. + + The approver name on the record is a claim; only a registered approver + key's signature over the record, by that same name, proves a person + approved. Fails closed when the key directory can't be read. + """ + + from openadapt_flow.approval_mode.human_attestation import ( + RESUME_DOMAIN, + refusal, + ) + + code = refusal( + approval.attestation, + domain=RESUME_DOMAIN, + payload=approval.signed_payload(), + key_dir=key_dir, + ) + if code is not None: + return code + assert approval.attestation is not None + if approval.attestation.signer != approval.approver: + return "approver_name_mismatch" + return None def approval_pause_digest(pending: Any) -> str: diff --git a/packages/flow/tests/_approver_keys.py b/packages/flow/tests/_approver_keys.py new file mode 100644 index 0000000000..f0a4ecfde2 --- /dev/null +++ b/packages/flow/tests/_approver_keys.py @@ -0,0 +1,87 @@ +"""Provision a registered approver key for tests that answer as a person. + +A person's answer to a save, and a durable resume approval, count only when a +key registered under ``approvers`` signs them +(:mod:`openadapt_flow.approval_mode.human_attestation`). Tests that play the +person use this throwaway key directory; agents in real use never hold it. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Any + +import pytest + +from openadapt_flow.approval_mode.human_attestation import ( + PORTAL_KEY_ENV, + ApproverSigner, + load_signer, +) +from openadapt_flow.jit.authority import ( + KEY_PASSPHRASE_ENV, + generate_encrypted_key, + register_principal, +) +from openadapt_flow.jit.lifecycle import KEY_DIR_ENV + +PASSPHRASE = "approver-test-passphrase" +_CURRENT: list[ApproverSigner] = [] + + +def provision( + key_dir: Path, + principal_id: str = "front-desk", + *, + method: str = "desktop_key", +) -> tuple[Path, ApproverSigner]: + """Create, register and open an approver key in ``key_dir``.""" + + env = {KEY_PASSPHRASE_ENV: PASSPHRASE} + key_path = key_dir / f"{principal_id.replace(':', '_')}.approver.pem" + public = generate_encrypted_key(key_path, env=env) + register_principal( + key_dir, role="approver", principal_id=principal_id, public_key_hex=public + ) + return key_path, load_signer(key_path, key_dir, method=method, env=env) # type: ignore[arg-type] + + +@pytest.fixture +def approver(tmp_path: Path, monkeypatch: pytest.MonkeyPatch): + """A registered Desktop approver key in this test's key directory. + + Sets the key directory and passphrase for the CLI and the run, and the + decision portal's key. Returns ``(key_path, signer)``. + """ + + key_dir = tmp_path / "approver-keys" + monkeypatch.setenv(KEY_DIR_ENV, str(key_dir)) + monkeypatch.setenv(KEY_PASSPHRASE_ENV, PASSPHRASE) + key_path, signer = provision(key_dir) + portal_path, _portal = provision(key_dir, "decision-portal", method="portal_key") + monkeypatch.setenv(PORTAL_KEY_ENV, str(portal_path)) + _CURRENT.append(signer) + yield key_path, signer + _CURRENT.remove(signer) + + +def person_signer() -> ApproverSigner: + """The Desktop approver key the active ``approver`` fixture registered. + + For helpers that play the person deep inside a run (a fake ``sleep`` + that answers each request). Use the fixture in the test or module. + """ + + if not _CURRENT: + raise RuntimeError("use the approver fixture to answer as a person") + return _CURRENT[-1] + + +def answer_as_person(store: Any, request: Any, **fields: Any) -> Any: + """``make_answer`` signed by the registered Desktop approver key.""" + + from openadapt_flow.approval_mode.item_approval import make_answer + + fields.setdefault("policy", store.context()) + fields.setdefault("signer", person_signer()) + return make_answer(request, **fields) diff --git a/packages/flow/tests/conftest.py b/packages/flow/tests/conftest.py index 81e994a87f..84da92ac8e 100644 --- a/packages/flow/tests/conftest.py +++ b/packages/flow/tests/conftest.py @@ -12,6 +12,7 @@ if str(REPO_ROOT) not in sys.path: sys.path.insert(0, str(REPO_ROOT)) +from tests._approver_keys import approver # noqa: E402,F401 - shared fixture from tests.repo_tree_guard import ( # noqa: E402 - needs the sys.path line above capture_baseline, check_and_report, diff --git a/packages/flow/tests/test_agent_guard.py b/packages/flow/tests/test_agent_guard.py index dc244d5132..b57057b97e 100644 --- a/packages/flow/tests/test_agent_guard.py +++ b/packages/flow/tests/test_agent_guard.py @@ -318,7 +318,7 @@ def test_observe_only_never_blocks_and_still_reads_back(sor, tmp_path): assert guard.summary.would_block[-1]["code"] == "already_committed" -def test_each_item_holds_the_save_until_a_person_approves(sor, tmp_path): +def test_each_item_holds_the_save_until_a_person_approves(sor, tmp_path, approver): page = FakePage(FakeAgent(sor[0]), "p1") guard = _guard(sor, tmp_path, page, approval="each_item") page.save() @@ -333,7 +333,9 @@ def test_each_item_holds_the_save_until_a_person_approves(sor, tmp_path): assert waiting is not None g = waiting[0] request = g._request - person_approves(str(g.dir), request.request_id, request.digest()) + person_approves( + str(g.dir), request.request_id, request.digest(), signer=approver[1] + ) guard.poll() assert page.sent == ["r1"] guard.poll() diff --git a/packages/flow/tests/test_api_step_approval.py b/packages/flow/tests/test_api_step_approval.py index 61a4a18935..7fcdeee8be 100644 --- a/packages/flow/tests/test_api_step_approval.py +++ b/packages/flow/tests/test_api_step_approval.py @@ -23,13 +23,11 @@ import urllib.parse as up from typing import Any, Optional +import pytest import requests from openadapt_flow.approval_mode.gate import ApprovalGate -from openadapt_flow.approval_mode.item_approval import ( - ItemApprovalStore, - make_answer, -) +from openadapt_flow.approval_mode.item_approval import ItemApprovalStore from openadapt_flow.approval_mode.model import ItemDecisionKind from openadapt_flow.compiler.api_proposal import admit_api_step from openadapt_flow.ir import WebFormField, Workflow @@ -37,15 +35,19 @@ from openadapt_flow.runtime.actuators.web_form import WebFormActuator from openadapt_flow.runtime.effects import RestRecordVerifier from openadapt_flow.runtime.replayer import Replayer +from tests._approver_keys import answer_as_person from tests.test_api_tier_web_form import TARGET, EnterWritesBackend, _span_workflow from tests.test_replayer import FakeVision +# Every person in these tests signs with a registered Desktop approver key. +pytestmark = pytest.mark.usefixtures("approver") + APPROVE: dict[str, Any] = dict( decision="approve", decided_by="human", route="desktop_run_it", - approver_id="cloud-user-0001", - auth_method="cloud_aal2", + approver_id=None, + auth_method="none", ) PARAMS = {"lname": "Marsh"} FORM = """
@@ -108,7 +110,9 @@ def sleep(self, seconds: float) -> None: self.seen.append(request.request_id) decision = self.decisions.pop(0) if self.decisions else None if decision is not None: - self.store.write_answer(make_answer(request, **decision)) + self.store.write_answer( + answer_as_person(self.store, request, **decision) + ) def _workflow() -> Workflow: diff --git a/packages/flow/tests/test_api_step_governed.py b/packages/flow/tests/test_api_step_governed.py index 7bbd174410..8917ed28f1 100644 --- a/packages/flow/tests/test_api_step_governed.py +++ b/packages/flow/tests/test_api_step_governed.py @@ -308,3 +308,7 @@ def test_the_record_check_must_select_by_the_identity_parameter(): unrelated = save.effects[0].model_copy(update={"new_records": []}) with pytest.raises(ApiProposalError, match="selects no field"): web_form_identity(save.api_binding, [unrelated], "subject_name", "lname") + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_api_step_managed.py b/packages/flow/tests/test_api_step_managed.py index ec620a798f..16fa4737e3 100644 --- a/packages/flow/tests/test_api_step_managed.py +++ b/packages/flow/tests/test_api_step_managed.py @@ -13,6 +13,8 @@ from __future__ import annotations +import pytest + from openadapt_flow import compatibility as matrix from openadapt_flow.runner.dispatch_envelope import ( _BINDING_FACTORY, @@ -22,6 +24,9 @@ from tests.test_api_step_governed import PARAMS from tests.test_compatibility_matrix import Run +# The person in the shared governed run signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") + RUN_ID = "00000000-0000-4000-8000-0000000000a1" diff --git a/packages/flow/tests/test_api_step_multi_surface.py b/packages/flow/tests/test_api_step_multi_surface.py index f726aeddf0..9fad299f14 100644 --- a/packages/flow/tests/test_api_step_multi_surface.py +++ b/packages/flow/tests/test_api_step_multi_surface.py @@ -214,3 +214,7 @@ def test_a_span_on_a_windows_surface_runs_on_the_gui(): assert _span_reason(_router_replayer(wf, active="windows_1"), wf) == ( "the API step's surface is not a browser, so the GUI steps run" ) + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_api_step_qualified.py b/packages/flow/tests/test_api_step_qualified.py index d03d3268fb..304ebf9a5f 100644 --- a/packages/flow/tests/test_api_step_qualified.py +++ b/packages/flow/tests/test_api_step_qualified.py @@ -14,6 +14,8 @@ from __future__ import annotations +import pytest + from openadapt_flow import compatibility as matrix from openadapt_flow.ir import QualifiedEffectRequirement from openadapt_flow.qualification import api_folded_step_ids @@ -101,3 +103,7 @@ def test_folded_steps_are_covered_only_by_an_api_path_save(tmp_path): def test_the_matrix_says_the_api_step_works_under_a_qualification_admission(): assert matrix.cell("api_step", "qualified").status == "works" + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_approval_delegated.py b/packages/flow/tests/test_approval_delegated.py index e44e2a2182..1dd8f36fcb 100644 --- a/packages/flow/tests/test_approval_delegated.py +++ b/packages/flow/tests/test_approval_delegated.py @@ -36,12 +36,14 @@ from openadapt_flow.approval_mode.item_approval import ( ItemApprovalRefused, ItemApprovalStore, + answer_refusal, make_answer, request_facts, wait_for_answer, ) from openadapt_flow.approval_mode.model import ApprovalMode, ItemApprovalRef from tests._approver_grant_support import grant_store, signed_grant +from tests._approver_keys import answer_as_person FRAME = b"\x89PNG\r\n\x1a\nmade-up filled form" FORM = ("dom", "sha256:" + "1" * 64) @@ -187,23 +189,57 @@ def test_step_class_comes_from_the_run_policy(): assert step_class_for(save, policy_name="regulated") is StepClass.CLINICAL_WRITE -def test_a_person_answer_keeps_its_old_bytes(tmp_path): - """No approver and the default step class: nothing new is serialized.""" +def test_a_person_answer_keeps_its_old_bytes(tmp_path, approver): + """No approver and the default step class: no delegate field is serialized. + + A person's answer now also needs its approver key's signature. The + signature is added as one ``attestation`` field over the answer's earlier + bytes, which are otherwise unchanged; without it the answer is refused. + """ store = ItemApprovalStore(tmp_path / "run") request = _open(store) - answer = make_answer( + answer = answer_as_person( + store, request, decision="approve", decided_by="human", route="desktop_run_it", - approver_id="cloud-user-0001", - auth_method="cloud_aal2", + approver_id=None, + auth_method="none", ) - assert "approver" not in answer.model_dump(mode="json") + dumped = answer.model_dump(mode="json") + assert "approver" not in dumped assert "step_class" not in request.model_dump(mode="json") ref = answer.reference(request).model_dump(mode="json") assert "approver" not in ref + # The signature covers exactly the earlier bytes and adds only itself. + assert set(dumped) - set(answer.signed_payload()) == {"attestation"} + unsigned = answer.model_copy(update={"attestation": None}) + assert unsigned.model_dump(mode="json") == answer.signed_payload() + assert answer_refusal(request, answer, policy=store.context()) is None + assert answer_refusal(request, unsigned, policy=store.context()) == ( + "human_answer_unsigned" + ) + + +def test_a_claimed_cloud_sign_in_no_longer_makes_a_person_answer(tmp_path, approver): + """Nothing here can verify a Cloud sign-in, so claiming one is refused.""" + + store = ItemApprovalStore(tmp_path / "run") + request = _open(store) + with pytest.raises(ItemApprovalRefused) as err: + answer_as_person( + store, + request, + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_id="cloud-user-0001", + auth_method="cloud_aal2", + ) + assert err.value.code == "auth_method_unverified" + assert store.read_answer(request.request_id) is None # -- delegated waiting ------------------------------------------------------------ diff --git a/packages/flow/tests/test_approval_mode_cli.py b/packages/flow/tests/test_approval_mode_cli.py index 61cef12542..481c71420f 100644 --- a/packages/flow/tests/test_approval_mode_cli.py +++ b/packages/flow/tests/test_approval_mode_cli.py @@ -307,6 +307,13 @@ def _answer_argv(store, request, verb="approve", *extra): SIGNED_IN = ("--approver-id", "cloud-user-0001", "--auth-method", "cloud_aal2") +def _signed(approver) -> tuple[str, ...]: + """A person in Run it: Desktop's registered approver key signs the answer.""" + + key_path, _signer = approver + return ("--unauthenticated", "--approver-key", str(key_path)) + + def _json(capsys) -> dict: return json.loads(capsys.readouterr().out.strip().splitlines()[-1]) @@ -325,21 +332,25 @@ def test_show_lists_what_the_person_must_answer(tmp_path, capsys): assert Path(row["screenshot"]).is_file() -def test_a_person_approves_once(tmp_path, capsys): +def test_a_person_approves_once(tmp_path, capsys, approver): import openadapt_flow.__main__ as main store, request = _request(tmp_path) - assert main.main(_answer_argv(store, request, "approve", *PERSON, *SIGNED_IN)) == 0 + argv = _answer_argv(store, request, "approve", *PERSON, *_signed(approver)) + assert main.main(argv) == 0 result = _json(capsys) assert result["ok"] is True and result["decision"] == "approve" assert result["resume_required"] is False assert store.status(request) == "approved" + answer = store.read_answer(request.request_id) + assert answer.attestation is not None + assert answer.attestation.signer == "front-desk" - assert main.main(_answer_argv(store, request, "approve", *PERSON, *SIGNED_IN)) == 2 + assert main.main(argv) == 2 assert _json(capsys)["code"] == "request_approved" -def test_a_signed_out_decline_is_recorded_without_a_name(tmp_path, capsys): +def test_a_signed_out_decline_is_recorded_without_a_name(tmp_path, capsys, approver): import openadapt_flow.__main__ as main store, request = _request(tmp_path) @@ -348,7 +359,7 @@ def test_a_signed_out_decline_is_recorded_without_a_name(tmp_path, capsys): request, "decline", *PERSON, - "--unauthenticated", + *_signed(approver), "--reason", "wrong_values", ) @@ -356,21 +367,34 @@ def test_a_signed_out_decline_is_recorded_without_a_name(tmp_path, capsys): answer = store.read_answer(request.request_id) assert answer.approver_kind == "unauthenticated" and answer.approver_id is None assert answer.decline_reason == "wrong_values" + assert answer.attestation is not None @pytest.mark.parametrize( ("argv_tail", "code"), [ ( - ("--decided-by", "automation", "--route", "desktop_run_it", *SIGNED_IN), + ( + "--decided-by", + "automation", + "--route", + "desktop_run_it", + "--unauthenticated", + ), "not_decided_by_a_person", ), ( - ("--decided-by", "unknown", "--route", "desktop_run_it", *SIGNED_IN), + ( + "--decided-by", + "unknown", + "--route", + "desktop_run_it", + "--unauthenticated", + ), "not_decided_by_a_person", ), ( - ("--decided-by", "human", "--route", "phone", *SIGNED_IN), + ("--decided-by", "human", "--route", "phone", "--unauthenticated"), "route_not_allowed", ), ((*PERSON,), "approver_invalid"), @@ -382,39 +406,88 @@ def test_a_signed_out_decline_is_recorded_without_a_name(tmp_path, capsys): ), ], ) -def test_anything_but_a_person_in_run_it_is_refused(tmp_path, capsys, argv_tail, code): +def test_anything_but_a_person_in_run_it_is_refused( + tmp_path, capsys, approver, argv_tail, code +): import openadapt_flow.__main__ as main + key_path, _signer = approver store, request = _request(tmp_path) - assert main.main(_answer_argv(store, request, "approve", *argv_tail)) == 2 + argv = _answer_argv( + store, request, "approve", *argv_tail, "--approver-key", str(key_path) + ) + assert main.main(argv) == 2 assert _json(capsys)["code"] == code assert store.status(request) == "waiting" -def test_an_answer_for_a_request_the_person_didnt_see_is_refused(tmp_path, capsys): +def test_a_person_without_the_approver_key_is_refused(tmp_path, capsys, approver): + """--decided-by human on its own is a claim any process can make.""" + + import openadapt_flow.__main__ as main + + store, request = _request(tmp_path) + argv = _answer_argv(store, request, "approve", *PERSON, "--unauthenticated") + assert main.main(argv) == 2 + assert _json(capsys)["code"] == "human_answer_unsigned" + assert store.status(request) == "waiting" + + +def test_a_claimed_cloud_sign_in_is_refused_even_with_the_key( + tmp_path, capsys, approver +): + """Nothing here can verify a Cloud sign-in, so naming one is refused.""" + + import openadapt_flow.__main__ as main + + key_path, _signer = approver + store, request = _request(tmp_path) + argv = _answer_argv( + store, + request, + "approve", + *PERSON, + *SIGNED_IN, + "--approver-key", + str(key_path), + ) + assert main.main(argv) == 2 + assert _json(capsys)["code"] == "auth_method_unverified" + assert store.status(request) == "waiting" + + +def test_an_answer_for_a_request_the_person_didnt_see_is_refused( + tmp_path, capsys, approver +): import openadapt_flow.__main__ as main store, request = _request(tmp_path) - argv = _answer_argv(store, request, "approve", *PERSON, *SIGNED_IN) + argv = _answer_argv(store, request, "approve", *PERSON, *_signed(approver)) argv[argv.index("--request-sha256") + 1] = "sha256:" + "9" * 64 assert main.main(argv) == 2 assert _json(capsys)["code"] == "request_digest_mismatch" - argv = _answer_argv(store, request, "approve", *PERSON, *SIGNED_IN) + argv = _answer_argv(store, request, "approve", *PERSON, *_signed(approver)) argv[argv.index("--request-id") + 1] = "0" * 32 assert main.main(argv) == 2 assert _json(capsys)["code"] == "request_unknown" -def test_decline_requires_a_closed_reason(tmp_path): +def test_decline_requires_a_closed_reason(tmp_path, approver): import openadapt_flow.__main__ as main store, request = _request(tmp_path) with pytest.raises(SystemExit): - main.main(_answer_argv(store, request, "decline", *PERSON, *SIGNED_IN)) + main.main(_answer_argv(store, request, "decline", *PERSON, *_signed(approver))) with pytest.raises(SystemExit): main.main( _answer_argv( - store, request, "decline", *PERSON, *SIGNED_IN, "--reason", "because" + store, + request, + "decline", + *PERSON, + *_signed(approver), + "--reason", + "because", ) ) diff --git a/packages/flow/tests/test_approval_mode_each_item.py b/packages/flow/tests/test_approval_mode_each_item.py index a08d24d719..2e4c130901 100644 --- a/packages/flow/tests/test_approval_mode_each_item.py +++ b/packages/flow/tests/test_approval_mode_each_item.py @@ -23,7 +23,6 @@ from openadapt_flow.approval_mode.gate import ApprovalGate, SaveContext, StepContext from openadapt_flow.approval_mode.item_approval import ( ItemApprovalStore, - make_answer, ) from openadapt_flow.approval_mode.model import ItemDecisionKind from openadapt_flow.execution_profiles import ExecutionProfile @@ -41,15 +40,18 @@ _ReadyVision, _sealed, ) +from tests._approver_keys import answer_as_person from tests.test_replayer import FakeBackend -APPROVER = "cloud-user-0001" +# Every person in these tests signs with a registered Desktop approver key. +pytestmark = pytest.mark.usefixtures("approver") + APPROVE: dict[str, Any] = dict( decision="approve", decided_by="human", route="desktop_run_it", - approver_id=APPROVER, - auth_method="cloud_aal2", + approver_id=None, + auth_method="none", ) @@ -95,7 +97,7 @@ def sleep(self, seconds: float) -> None: decision = self.decisions.pop(0) if self.decisions else None if decision is None: continue - self.store.write_answer(make_answer(request, **decision)) + self.store.write_answer(answer_as_person(self.store, request, **decision)) def _save_delivered(backend: FakeBackend) -> bool: @@ -196,7 +198,7 @@ def test_the_run_waits_and_saves_only_after_a_persons_approval(tmp_path): assert (row.decided_by, row.route, row.approver_id) == ( "human", "desktop_run_it", - APPROVER, + None, ) assert any( check.code == "each_item" and check.detail == "approved_by_person" @@ -271,7 +273,7 @@ def test_no_answer_in_time_stops_before_the_save_and_a_later_approval_resumes( # Later, the person approves the request in Desktop's Run it. request = item.store.requests()[0] - answer = make_answer(request, **APPROVE) + answer = answer_as_person(item.store, request, **APPROVE) item.store.write_answer(answer) assert _answer_durable_item_pause(item.run_dir, request, answer) is True assert item.pending().status == "approved" @@ -308,7 +310,7 @@ def test_an_approval_that_raced_the_timeout_is_carried_to_the_pause_by_resume( item.run() assert item.pending().status == "pending" request = item.store.requests()[0] - answer = make_answer(request, **APPROVE) + answer = answer_as_person(item.store, request, **APPROVE) item.store.write_answer(answer) store = CheckpointStore(item.run_dir, key=_KEY) @@ -335,19 +337,20 @@ def test_resume_carries_only_a_persons_approval(tmp_path, monkeypatch, answer_ki request = item.store.requests()[0] if answer_kind == "decline": item.store.write_answer( - make_answer( + answer_as_person( + item.store, request, decision="decline", decided_by="human", route="desktop_run_it", - approver_id=APPROVER, - auth_method="cloud_aal2", + approver_id=None, + auth_method="none", decline_reason="wrong_values", ) ) elif answer_kind == "not_a_person": # Written around the command, so nothing checked it on the way in. - good = make_answer(request, **APPROVE) + good = answer_as_person(item.store, request, **APPROVE) item.store.write_answer( ItemApprovalAnswer.model_validate( {**good.model_dump(mode="json"), "decided_by": "automation"} @@ -432,7 +435,9 @@ def test_an_answer_written_around_the_cli_is_still_checked_by_the_run( def forge(): for request in item.store.requests(): if item.store.status(request) == "waiting": - payload = make_answer(request, **APPROVE).model_dump(mode="json") + payload = answer_as_person(item.store, request, **APPROVE).model_dump( + mode="json" + ) payload.update(tamper) path = item.store.dir / f"answer-{request.request_id}.json" path.write_text(json.dumps(payload)) diff --git a/packages/flow/tests/test_approval_mode_item_approval.py b/packages/flow/tests/test_approval_mode_item_approval.py index 234362d76c..fbcdd2a458 100644 --- a/packages/flow/tests/test_approval_mode_item_approval.py +++ b/packages/flow/tests/test_approval_mode_item_approval.py @@ -4,7 +4,8 @@ tests pin the file contract Desktop's Run it relies on, and every way an answer is refused: a non-human or non-Desktop decider, a different request, an answer from before the request or after it expired, a second answer, a -reused approval, a malformed or redirected file. +reused approval, a malformed or redirected file, a claimed Cloud sign-in. +The person signs with a registered Desktop approver key (``approver``). """ from __future__ import annotations @@ -16,6 +17,7 @@ import pytest +from openadapt_flow.approval_mode.human_attestation import ANSWER_DOMAIN from openadapt_flow.approval_mode.item_approval import ( ItemApprovalAnswer, ItemApprovalRefused, @@ -49,13 +51,24 @@ def _open(store: ItemApprovalStore, **overrides): return store.open_request(**fields) -def _approve(request, **overrides): +@pytest.fixture(autouse=True) +def _signer(approver): + global SIGNER + SIGNER = approver[1] + + +SIGNER = None + + +def _approve(store, request, **overrides): fields = dict( decision="approve", decided_by="human", route="desktop_run_it", - approver_id=APPROVER, - auth_method="cloud_aal2", + approver_id=None, + auth_method="none", + policy=store.context(), + signer=SIGNER, ) fields.update(overrides) return make_answer(request, **fields) @@ -84,12 +97,12 @@ def test_a_request_is_two_private_files_and_waits(tmp_path): def test_an_approval_is_recorded_once_and_used_once(tmp_path): store = ItemApprovalStore(tmp_path / "run") request = _open(store) - answer = _approve(request) + answer = _approve(store, request) store.write_answer(answer) assert store.status(request) == "approved" with pytest.raises(ItemApprovalRefused) as second: - store.write_answer(_approve(request)) + store.write_answer(_approve(store, request)) assert second.value.code == "already_answered" assert store.consume(request.request_id) is True @@ -97,7 +110,9 @@ def test_an_approval_is_recorded_once_and_used_once(tmp_path): assert store.status(request) == "used" rows = store.consumed_approvals("run-0001") assert len(rows) == 1 and isinstance(rows[0], ItemApprovalRef) - assert rows[0].approver_id == APPROVER + assert rows[0].approver_id is None + assert answer.attestation is not None + assert answer.attestation.signer == SIGNER.principal_id assert rows[0].decided_by == "human" assert rows[0].answer_sha256 == answer.digest() @@ -111,9 +126,15 @@ def test_an_approval_is_recorded_once_and_used_once(tmp_path): ({"route": "local_console"}, "route_not_allowed"), ( {"approver_id": None, "auth_method": "cloud_aal2"}, - "answer_fields_inconsistent", + "auth_method_unverified", + ), + ({"approver_id": APPROVER}, "answer_fields_inconsistent"), + # A claimed Cloud sign-in can't be verified here, so it is refused. + ( + {"approver_id": APPROVER, "auth_method": "cloud_aal2"}, + "auth_method_unverified", ), - ({"auth_method": "none"}, "answer_fields_inconsistent"), + ({"signer": None}, "human_answer_unsigned"), ({"decision": "decline"}, "answer_fields_inconsistent"), ({"decline_reason": "wrong_values"}, "answer_fields_inconsistent"), ({"decided_by": "a robot"}, "answer_fields_invalid"), @@ -123,14 +144,14 @@ def test_only_a_person_in_desktop_run_it_can_answer(tmp_path, overrides, code): store = ItemApprovalStore(tmp_path / "run") request = _open(store) with pytest.raises(ItemApprovalRefused) as refused: - _approve(request, **overrides) + _approve(store, request, **overrides) assert refused.value.code == code def test_an_unauthenticated_approval_names_nobody(tmp_path): store = ItemApprovalStore(tmp_path / "run") request = _open(store) - answer = _approve(request, approver_id=None, auth_method="none") + answer = _approve(store, request, approver_id=None, auth_method="none") ref = answer.reference(request) assert ref.approver_kind == "unauthenticated" assert ref.approver_id is None @@ -139,31 +160,47 @@ def test_an_unauthenticated_approval_names_nobody(tmp_path): def test_a_decline_needs_one_closed_reason(tmp_path): store = ItemApprovalStore(tmp_path / "run") request = _open(store) - answer = _approve(request, decision="decline", decline_reason="wrong_record") + answer = _approve(store, request, decision="decline", decline_reason="wrong_record") store.write_answer(answer) assert store.status(request) == "declined" with pytest.raises(ItemApprovalRefused): - _approve(request, decision="decline", decline_reason="other") + _approve(store, request, decision="decline", decline_reason="other") def test_an_answer_for_another_request_or_out_of_time_is_refused(tmp_path): store = ItemApprovalStore(tmp_path / "run") request = _open(store) other = _open(store) - answer = _approve(other) + answer = _approve(store, other) assert answer_refusal(request, answer) == "request_mismatch" forged = answer.model_copy(update={"request_id": request.request_id}) assert answer_refusal(request, forged) == "request_digest_mismatch" created = datetime.fromisoformat(request.created_at) - early = _approve(request).model_copy( + early = _approve(store, request).model_copy( update={"answered_at": (created - timedelta(seconds=5)).isoformat()} ) - assert answer_refusal(request, early) == "answer_predates_request" + # Changing a signed answer breaks its signature; re-signed, it is still + # refused for when it was given. + assert answer_refusal(request, early, policy=store.context()) == ( + "human_answer_signature_invalid" + ) + early = early.model_copy( + update={"attestation": SIGNER.attest(ANSWER_DOMAIN, early.signed_payload())} + ) + assert ( + answer_refusal(request, early, policy=store.context()) + == "answer_predates_request" + ) late_now = datetime.fromisoformat(request.expires_at) + timedelta(seconds=1) - assert answer_refusal(request, _approve(request), now=late_now) == "request_expired" + assert ( + answer_refusal( + request, _approve(store, request), now=late_now, policy=store.context() + ) + == "request_expired" + ) with pytest.raises(ItemApprovalRefused) as expired: - _approve(request, now=late_now) + _approve(store, request, now=late_now) assert expired.value.code in {"answer_after_expiry", "request_expired"} @@ -190,7 +227,7 @@ def test_waiting_returns_the_answer_the_person_gives(tmp_path): def sleep(seconds): clock[0] += seconds if clock[0] >= 2 and store.status(request) == "waiting": - store.write_answer(_approve(request)) + store.write_answer(_approve(store, request)) answer, code = wait_for_answer( store, request, timeout_s=60, poll_s=1, monotonic=lambda: clock[0], sleep=sleep @@ -207,7 +244,7 @@ def test_a_malformed_or_non_human_answer_file_ends_the_wait(tmp_path): assert store.status(request) == "invalid_answer" second = _open(store) - payload = _approve(second).model_dump(mode="json") + payload = _approve(store, second).model_dump(mode="json") payload["decided_by"] = "automation" (store.dir / f"answer-{second.request_id}.json").write_text(json.dumps(payload)) answer, code = wait_for_answer(store, second, timeout_s=5, poll_s=1) @@ -219,7 +256,7 @@ def test_a_redirected_answer_file_is_never_followed(tmp_path): store = ItemApprovalStore(tmp_path / "run") request = _open(store) outside = tmp_path / "outside.json" - outside.write_text(_approve(request).model_dump_json()) + outside.write_text(_approve(store, request).model_dump_json()) os.symlink(outside, store.dir / f"answer-{request.request_id}.json") with pytest.raises(ItemApprovalRefused) as refused: store.read_answer(request.request_id) @@ -242,7 +279,7 @@ def test_a_redirected_approval_folder_is_refused(tmp_path): def test_a_resumed_run_uses_only_the_newest_unused_approval(tmp_path): store = ItemApprovalStore(tmp_path / "run") older = _open(store) - store.write_answer(_approve(older)) + store.write_answer(_approve(store, older)) newer = _open(store) # The person approved the older request, but the newest request for the # same save is the one they last saw; an older answer can't stand in. @@ -250,7 +287,7 @@ def test_a_resumed_run_uses_only_the_newest_unused_approval(tmp_path): store.find_unused_approval(run_id="run-0001", step_id="save", step_index=3) is None ) - store.write_answer(_approve(newer)) + store.write_answer(_approve(store, newer)) found = store.find_unused_approval(run_id="run-0001", step_id="save", step_index=3) assert found is not None and found[0].request_id == newer.request_id assert store.consume(newer.request_id) @@ -275,7 +312,7 @@ def test_unknown_or_traversing_request_ids_are_refused(tmp_path): def test_the_answer_model_rejects_unknown_fields(tmp_path): store = ItemApprovalStore(tmp_path / "run") request = _open(store) - payload = _approve(request).model_dump(mode="json") + payload = _approve(store, request).model_dump(mode="json") payload["note"] = "free text is not allowed" with pytest.raises(ValueError): ItemApprovalAnswer.model_validate(payload) diff --git a/packages/flow/tests/test_auto_repair_cli.py b/packages/flow/tests/test_auto_repair_cli.py index 16d297c83c..020c0fd839 100644 --- a/packages/flow/tests/test_auto_repair_cli.py +++ b/packages/flow/tests/test_auto_repair_cli.py @@ -29,6 +29,10 @@ pytest.importorskip("playwright.sync_api") +# The phone portal records the person's tap signed with its registered +# approver key (OPENADAPT_PORTAL_APPROVER_KEY); without one it records nothing. +pytestmark = pytest.mark.usefixtures("approver") + def _config(tmp_path: Path, base_url: str, *, auto: bool = True) -> Path: store = tmp_path / "repair-store" diff --git a/packages/flow/tests/test_cli_new_commands.py b/packages/flow/tests/test_cli_new_commands.py index 4979de390d..1bca6efc92 100644 --- a/packages/flow/tests/test_cli_new_commands.py +++ b/packages/flow/tests/test_cli_new_commands.py @@ -173,6 +173,13 @@ def test_deployment_runtime_bad_effects_config_exits() -> None: # --------------------------------------------------------------------------- +def _approve(run: Path, approver) -> int: + """``approve`` as a person: signed with the registered approver key.""" + + key_path, _signer = approver + return main(["approve", str(run), "--approver-key", str(key_path)]) + + def _paused_run( tmp_path: Path, *, @@ -254,12 +261,17 @@ def _paused_run( return run -def test_approve_flips_status_and_is_idempotent(tmp_path: Path, capsys) -> None: +def test_approve_flips_status_and_is_idempotent( + tmp_path: Path, capsys, approver +) -> None: run = _paused_run(tmp_path) - assert main(["approve", str(run)]) == 0 + # Without the person's approver key nothing is approved (no OS-user default). + assert main(["approve", str(run)]) == 1 + assert CheckpointStore(run).read_pending().status != "approved" + assert _approve(run, approver) == 0 assert CheckpointStore(run).read_pending().status == "approved" # Second approve is a no-op success. - assert main(["approve", str(run)]) == 0 + assert _approve(run, approver) == 0 assert "already approved" in capsys.readouterr().out @@ -286,10 +298,10 @@ def test_resume_require_approval_refuses_pending(tmp_path: Path, capsys) -> None assert "without --require-approval" not in output -def test_resume_without_url_exits(tmp_path: Path) -> None: +def test_resume_without_url_exits(tmp_path: Path, approver) -> None: run = _paused_run(tmp_path) # Approved + no --url and no backend.url -> a clear config error. - main(["approve", str(run)]) + assert _approve(run, approver) == 0 with pytest.raises(SystemExit, match="needs the target app URL"): main(["resume", str(run), "--require-approval"]) @@ -542,24 +554,24 @@ def test_resume_parser_has_backend_flags() -> None: assert args.agent_url == "http://a:5001" -def test_resume_web_backend_still_requires_url(tmp_path: Path) -> None: +def test_resume_web_backend_still_requires_url(tmp_path: Path, approver) -> None: # The web default is unchanged: with no --url and no backend.url the web # path still fails loud on the missing target URL (it must launch a browser # and navigate a page). --backend web is the explicit form of the default. run = _paused_run(tmp_path) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 with pytest.raises(SystemExit, match="needs the target app URL"): main(["resume", str(run), "--require-approval", "--backend", "web"]) def test_resume_web_default_builds_playwright_backend( - tmp_path: Path, monkeypatch + tmp_path: Path, monkeypatch, approver ) -> None: # The default (web / no --backend) still drives the browser: it launches # Playwright, builds the PlaywrightBackend via the factory, and hands it to # the durable resume entrypoint — byte-for-byte the historical path. run = _paused_run(tmp_path) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 captured: dict = {} _install_fake_browser(monkeypatch, captured) @@ -592,11 +604,11 @@ def _fake_resume(run_dir, replayer, key=None, execution_target_kind=None, **_kw) def test_resume_wires_all_configured_appliance_tiers( - tmp_path: Path, monkeypatch + tmp_path: Path, monkeypatch, approver ) -> None: """Resume uses the shared deployment constructor, including VLM handles.""" run = _paused_run(tmp_path) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 config = tmp_path / "deployment.yaml" config.write_text("runtime:\n allow_model_grounding: true\n") @@ -649,10 +661,12 @@ def _grounder(*, fallback=None): assert captured["ctor"]["durable"] is True -def test_managed_resume_refuses_model_grounding_profile(tmp_path: Path, capsys) -> None: +def test_managed_resume_refuses_model_grounding_profile( + tmp_path: Path, capsys, approver +) -> None: """A managed resume preserves its local-only evidence contract.""" run = _paused_run(tmp_path, managed=True) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 config = tmp_path / "deployment.yaml" config.write_text("runtime:\n allow_model_grounding: true\n") manifest = CheckpointStore(run).read_manifest() @@ -694,12 +708,12 @@ def test_managed_resume_refuses_model_grounding_profile(tmp_path: Path, capsys) def test_resume_windows_config_builds_windows_backend( - tmp_path: Path, monkeypatch + tmp_path: Path, monkeypatch, approver ) -> None: # A windows-kind resume builds a real WindowsBackend from the factory with # NO browser and NO --url, and hands it to the durable resume entrypoint. run = _paused_run(tmp_path) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 captured: dict = {} @@ -741,7 +755,7 @@ def __init__(self, backend, **kwargs): def test_resume_uses_recorded_citrix_target_and_readiness( - tmp_path: Path, monkeypatch + tmp_path: Path, monkeypatch, approver ) -> None: run = _paused_run( tmp_path, @@ -752,7 +766,7 @@ def test_resume_uses_recorded_citrix_target_and_readiness( rdp_readiness_text="Claims queue", ), ) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 captured: dict = {} @@ -795,10 +809,10 @@ def fake_build(cfg, **kwargs): def test_resume_refuses_blank_citrix_readiness_before_backend( - tmp_path: Path, monkeypatch, capsys + tmp_path: Path, monkeypatch, capsys, approver ) -> None: run = _paused_run(tmp_path) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 config = tmp_path / "citrix.yaml" config.write_text( "backend:\n kind: citrix\n rdp_window: wfica32\n rdp_readiness_text: ' '\n" @@ -829,10 +843,10 @@ def test_resume_refuses_blank_citrix_readiness_before_backend( def test_resume_bundle_load_refusal_has_safe_constant_guidance( - tmp_path: Path, capsys + tmp_path: Path, capsys, approver ) -> None: run = _paused_run(tmp_path) - main(["approve", str(run)]) + assert _approve(run, approver) == 0 (tmp_path / "b" / "workflow.json").unlink() assert main(["resume", str(run), "--require-approval"]) == 3 @@ -861,10 +875,10 @@ def test_version_flag_prints_version_and_exits_zero(capsys) -> None: def test_resume_refuses_another_surface_than_the_bundle_was_recorded_on( - tmp_path: Path, monkeypatch, capsys + tmp_path: Path, monkeypatch, capsys, approver ) -> None: run = _paused_run(tmp_path, surface="macos") - main(["approve", str(run)]) + assert _approve(run, approver) == 0 import openadapt_flow.backends.factory as factory @@ -890,9 +904,11 @@ def test_resume_refuses_another_surface_than_the_bundle_was_recorded_on( assert "Nothing was executed" in out -def test_resume_with_a_surface_override_records_it(tmp_path: Path, monkeypatch) -> None: +def test_resume_with_a_surface_override_records_it( + tmp_path: Path, monkeypatch, approver +) -> None: run = _paused_run(tmp_path, surface="macos") - main(["approve", str(run)]) + assert _approve(run, approver) == 0 import openadapt_flow.backends.factory as factory import openadapt_flow.report as report_mod diff --git a/packages/flow/tests/test_compatibility_matrix.py b/packages/flow/tests/test_compatibility_matrix.py index 2f433758a3..e14eff0356 100644 --- a/packages/flow/tests/test_compatibility_matrix.py +++ b/packages/flow/tests/test_compatibility_matrix.py @@ -376,3 +376,7 @@ def test_self_repair_where_the_matrix_says(feature, profile): assert item.evidence else: assert refusal is None + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_console_save_approvals.py b/packages/flow/tests/test_console_save_approvals.py index 8159482ca7..824fc60b8d 100644 --- a/packages/flow/tests/test_console_save_approvals.py +++ b/packages/flow/tests/test_console_save_approvals.py @@ -7,7 +7,9 @@ * 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; + ``decision_portal`` route), signed by the portal's registered approver key, + and a decline needs a closed reason; +* a portal without an approver key records no answer at all; * answering is offered only by the attended console the portal relays to. """ @@ -21,6 +23,7 @@ import pytest from fastapi.testclient import TestClient +from openadapt_flow.approval_mode.human_attestation import PORTAL_KEY_ENV from openadapt_flow.approval_mode.item_approval import ( ItemApprovalStore, decided_by_summary, @@ -30,6 +33,9 @@ from openadapt_flow.console.app import create_app from openadapt_types.human_decision import HumanDecisionTaskV1 +# The portal signs every answer with its registered approver key. +pytestmark = pytest.mark.usefixtures("approver") + FRAME = b"\x89PNG\r\n\x1a\nmade-up filled form" FORM = ("dom", "sha256:" + "1" * 64) @@ -98,6 +104,9 @@ def test_an_approval_is_recorded_once_as_a_person_on_the_portal(tmp_path): answer = store.read_answer(request.request_id) assert answer.decided_by == "human" and answer.route == "decision_portal" assert answer.approver_kind == "unauthenticated" + assert answer.attestation is not None + assert answer.attestation.method == "portal_key" + assert answer.attestation.signer == "decision-portal" 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) == [] @@ -120,6 +129,32 @@ def test_a_decline_needs_one_closed_reason(tmp_path): assert store.read_answer(request.request_id).decline_reason == "wrong_record" +def test_a_portal_without_an_approver_key_records_no_answer(tmp_path, monkeypatch): + store, request = _open(tmp_path) + monkeypatch.delenv(PORTAL_KEY_ENV) + with pytest.raises(save_approvals.SaveApprovalRefused) as err: + _answer(tmp_path, request.request_id) + assert err.value.code == "human_answer_unverifiable" + assert store.read_answer(request.request_id) is None + assert store.status(request) == "waiting" + # The console route refuses the same way and still offers the save. + client = _client(tmp_path, attend=True) + 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 == 409 + assert response.json()["detail"] == "human_answer_unverifiable" + assert store.read_answer(request.request_id) is None + rows = save_approvals.list_save_approvals(tmp_path) + assert [row.request_id for row in rows] == [request.request_id] + + def test_an_answer_for_another_task_is_refused(tmp_path): _, first = _open(tmp_path, "run-a") _, second = _open(tmp_path, "run-b") diff --git a/packages/flow/tests/test_console_save_fields.py b/packages/flow/tests/test_console_save_fields.py index e127bf6c40..916b84ccda 100644 --- a/packages/flow/tests/test_console_save_fields.py +++ b/packages/flow/tests/test_console_save_fields.py @@ -21,6 +21,7 @@ from fastapi.testclient import TestClient from pydantic import ValidationError +from openadapt_flow.approval_mode.human_attestation import portal_signer from openadapt_flow.approval_mode.item_approval import ( MAX_SAVE_FIELDS, ItemApprovalRequest, @@ -105,7 +106,9 @@ def test_the_fields_are_inside_the_request_digest(tmp_path): assert detail["task"]["capability_digest"] == with_fields.digest() -def test_a_value_changed_after_the_phone_saw_it_makes_the_answer_stale(tmp_path): +def test_a_value_changed_after_the_phone_saw_it_makes_the_answer_stale( + tmp_path, approver +): store, request = _open(tmp_path, FIELDS) shown = save_approvals.save_approval_detail(tmp_path, request.request_id) path = store.dir / f"request-{request.request_id}.json" @@ -139,7 +142,7 @@ def test_the_detail_is_served_no_store(tmp_path): assert response.json()["presentation"]["fields"][0]["after"] == VALUE -def test_the_values_stay_out_of_everything_else(tmp_path): +def test_the_values_stay_out_of_everything_else(tmp_path, approver): store, request = _open(tmp_path, FIELDS) client = _client(tmp_path) detail = client.get(f"/api/save-approvals/{request.request_id}").json() @@ -161,6 +164,7 @@ def test_the_values_stay_out_of_everything_else(tmp_path): auth_method="none", decline_reason=None, policy=store.context(), + signer=portal_signer(store.key_directory()), ) assert VALUE not in json.dumps(answer.reference(request).model_dump(mode="json")) assert VALUE not in json.dumps(answer.model_dump(mode="json")) diff --git a/packages/flow/tests/test_desktop_check_it_cli_mockmed.py b/packages/flow/tests/test_desktop_check_it_cli_mockmed.py index 67d1908f25..c01fa0acf8 100644 --- a/packages/flow/tests/test_desktop_check_it_cli_mockmed.py +++ b/packages/flow/tests/test_desktop_check_it_cli_mockmed.py @@ -43,6 +43,8 @@ from openadapt_flow.backends.playwright_backend import PlaywrightBackend # noqa: E402 from openadapt_flow.compiler import compile_recording # noqa: E402 from openadapt_flow.ir import RunReport, Workflow # noqa: E402 +from openadapt_flow.jit.authority import KEY_PASSPHRASE_ENV # noqa: E402 +from openadapt_flow.jit.lifecycle import KEY_DIR_ENV # noqa: E402 from openadapt_flow.mockmed.fault_server import serve # noqa: E402 from openadapt_flow.qualify.environment import ( # noqa: E402 WEB_OBSERVER_CONTRACT_SHA256, @@ -93,6 +95,7 @@ ) from openadapt_flow.verification import VerificationTier # noqa: E402 from scripts import export_public_demo_evidence as demo # noqa: E402 +from tests import _approver_keys as approver_keys # noqa: E402 REPO_ROOT = Path(__file__).resolve().parents[1] BUNDLE_KEY = "desktop-check-it-test-key" @@ -117,9 +120,10 @@ def _flow(*argv: str, env: dict[str, str]) -> subprocess.CompletedProcess[str]: class DesktopRunIt: """Answers the run's waiting approval request, as Desktop's Run it would.""" - def __init__(self, run_dir: Path, env: dict[str, str]) -> None: + def __init__(self, run_dir: Path, env: dict[str, str], approver_key: Path) -> None: self.run_dir = run_dir self.env = env + self.approver_key = approver_key self.shown: list[dict[str, Any]] = [] self.answers: list[dict[str, Any]] = [] self.errors: list[str] = [] @@ -162,10 +166,11 @@ def _loop(self) -> None: "human", "--route", "desktop_run_it", - "--approver-id", - "cloud-user-0001", - "--auth-method", - "cloud_aal2", + "--unauthenticated", + # Desktop signs the person's answer with its registered + # approver key; a claimed Cloud sign-in proves nothing here. + "--approver-key", + str(self.approver_key), env=self.env, ) if answered.returncode != 0: @@ -185,10 +190,21 @@ def check_it(tmp_path_factory: pytest.TempPathFactory): home = work / "openadapt-home" saved_env = { name: os.environ.get(name) - for name in ("OPENADAPT_HOME", "OPENADAPT_BUNDLE_KEY") + for name in ( + "OPENADAPT_HOME", + "OPENADAPT_BUNDLE_KEY", + KEY_DIR_ENV, + KEY_PASSPHRASE_ENV, + ) } os.environ["OPENADAPT_HOME"] = str(home) os.environ["OPENADAPT_BUNDLE_KEY"] = BUNDLE_KEY + # The person's answers are signed by Desktop's registered approver key, + # and the run verifies them against this key directory. + key_dir = work / "approver-keys" + os.environ[KEY_DIR_ENV] = str(key_dir) + os.environ[KEY_PASSPHRASE_ENV] = approver_keys.PASSPHRASE + approver_key, _signer = approver_keys.provision(key_dir) env = { **os.environ, "OPENADAPT_HOME": str(home), @@ -350,7 +366,7 @@ def run_case(case_id: str) -> SimpleNamespace: "--approval-wait", "120", ] - with DesktopRunIt(run_dir, env) as person: + with DesktopRunIt(run_dir, env, approver_key) as person: exit_code = main.main(argv) report = RunReport.model_validate_json( (run_dir / "report.json").read_text(encoding="utf-8") @@ -377,6 +393,7 @@ def run_case(case_id: str) -> SimpleNamespace: base_url=base_url, entry_url=entry_url, env=env, + approver_key=approver_key, ) finally: observer_patch.undo() @@ -416,8 +433,9 @@ def test_the_practice_item_waits_for_desktop_and_saves_once_after_approval(check assert run.report.approval.mode.value == "each_item" assert run.report.approval.decision.value == "approved" (approval,) = run.report.approval.approvals - assert approval.approver_id == "cloud-user-0001" - assert approval.auth_method == "cloud_aal2" + # A signed Run it answer names no Cloud user: its proof is the signature. + assert approval.approver_id is None + assert approval.auth_method == "none" # Desktop saw one request, with a screenshot of the filled form. (shown,) = run.person.shown assert Path(shown["screenshot"]).is_file() @@ -604,6 +622,8 @@ def run(run_dir: Path, wait_s: str) -> int: "--route", "desktop_run_it", "--unauthenticated", + "--approver-key", + str(check_it.approver_key), env=check_it.env, ) assert approved.returncode == 0, approved.stdout + approved.stderr @@ -615,7 +635,7 @@ def run(run_dir: Path, wait_s: str) -> int: # 3. The item again, answered in time. again = check_it.work / "runs" / "run-it-again" - with DesktopRunIt(again, check_it.env) as person: + with DesktopRunIt(again, check_it.env, check_it.approver_key) as person: assert run(again, "120") == 0 assert person.errors == [] assert len(person.answers) == 1 diff --git a/packages/flow/tests/test_durable_pipelined_readback.py b/packages/flow/tests/test_durable_pipelined_readback.py index c7f22b9337..18a3d1d9a0 100644 --- a/packages/flow/tests/test_durable_pipelined_readback.py +++ b/packages/flow/tests/test_durable_pipelined_readback.py @@ -274,3 +274,7 @@ def broken(self, *args, **kwargs): closer.start() closer.join(timeout=30) assert not closer.is_alive() + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_guarded_save.py b/packages/flow/tests/test_guarded_save.py index 87c6f365fc..3349372137 100644 --- a/packages/flow/tests/test_guarded_save.py +++ b/packages/flow/tests/test_guarded_save.py @@ -66,9 +66,9 @@ def _rows(db) -> list[dict]: # -- the three cases ---------------------------------------------------------- -def test_correct_save_is_verified(sor, agent, tmp_path): +def test_correct_save_is_verified(sor, agent, tmp_path, approver): guarded = _guarded(sor, tmp_path) - out = run_case("correct", guarded, agent) + out = run_case("correct", guarded, agent, signer=approver[1]) assert out["verdict"] == "VERIFIED" assert out["outcome"] == "verified" assert out["retry_allowed"] is False @@ -86,9 +86,9 @@ def test_correct_save_is_verified(sor, agent, tmp_path): assert evidence_problem(doc["evidence"], receipt) is None -def test_wrong_patient_is_refused_before_the_write(sor, agent, tmp_path): +def test_wrong_patient_is_refused_before_the_write(sor, agent, tmp_path, approver): guarded = _guarded(sor, tmp_path) - out = run_case("wrong_patient", guarded, agent) + out = run_case("wrong_patient", guarded, agent, signer=approver[1]) assert out["refused"] == "identity_mismatch" assert out["saved"] is False assert _rows(sor[1]) == [] @@ -96,9 +96,11 @@ def test_wrong_patient_is_refused_before_the_write(sor, agent, tmp_path): assert guarded.task_state("demo-wrong_patient-0001") is None -def test_dropped_save_needs_reconciliation_and_is_never_retried(sor, agent, tmp_path): +def test_dropped_save_needs_reconciliation_and_is_never_retried( + sor, agent, tmp_path, approver +): guarded = _guarded(sor, tmp_path) - out = run_case("dropped", guarded, agent) + out = run_case("dropped", guarded, agent, signer=approver[1]) assert out["saved"] is True assert out["verdict"] == "RECONCILIATION_REQUIRED" assert out["outcome"] == "reconciliation_required" @@ -173,7 +175,7 @@ def test_record_changed_before_commit_is_refused(sor, agent, tmp_path): assert guarded.task_state("demo-moved-0001") is None -def test_declined_save_is_refused(sor, agent, tmp_path): +def test_declined_save_is_refused(sor, agent, tmp_path, approver): from openadapt_flow.approval_mode.item_approval import ( ItemApprovalStore, make_answer, @@ -198,6 +200,7 @@ def test_declined_save_is_refused(sor, agent, tmp_path): approver_id=None, auth_method="none", policy=store.context(), + signer=approver[1], ) ) with pytest.raises(GuardRefused): @@ -322,7 +325,7 @@ def _write_task(tasks: Path, task_id: str, patient_id: str = "p1") -> None: ) -def test_mcp_tools_end_to_end(sor, agent, tmp_path): +def test_mcp_tools_end_to_end(sor, agent, tmp_path, approver): tasks = tmp_path / "tasks" _write_task(tasks, "mcp-task-0001") bridge = GuardBridge(_guarded(sor, tmp_path), tasks) @@ -343,6 +346,7 @@ def test_mcp_tools_end_to_end(sor, agent, tmp_path): waiting["approval_run_dir"], waiting["approval_request_id"], waiting["approval_request_sha256"], + signer=approver[1], ) go = bridge.dispatch("guard_commit", {"guard_id": begun["guard_id"]}) assert go["status"] == "go" diff --git a/packages/flow/tests/test_human_approval_verified.py b/packages/flow/tests/test_human_approval_verified.py new file mode 100644 index 0000000000..c6ae1dd40e --- /dev/null +++ b/packages/flow/tests/test_human_approval_verified.py @@ -0,0 +1,323 @@ +"""A person's approval counts only when a registered approver key signed it. + +Before this, ``--decided-by human`` with ``--unauthenticated``, or a named +``--auth-method``, was enough: any process that could run the CLI or write the +run directory could approve its own gated save, and the durable ``approve`` +named whoever the OS user was. These tests play that agent. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from openadapt_flow.approval_mode.human_attestation import load_signer +from openadapt_flow.approval_mode.item_approval import ( + ItemApprovalAnswer, + ItemApprovalRefused, + ItemApprovalStore, + answer_refusal, + make_answer, +) +from openadapt_flow.approval_mode.model import ApprovalMode +from openadapt_flow.jit.authority import ( + KEY_PASSPHRASE_ENV, + generate_encrypted_key, + register_principal, +) +from tests._approver_keys import PASSPHRASE + +FRAME = b"\x89PNG\r\n\x1a\nmade-up filled form" + + +def _request(store: ItemApprovalStore): + return store.open_request( + run_id="run-0001", + workflow_name="referral", + 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=("dom", "sha256:" + "1" * 64), + ) + + +def _cli(*argv: str) -> tuple[int, dict]: + import contextlib + import io + + from openadapt_flow.__main__ import main + + out = io.StringIO() + with contextlib.redirect_stdout(out): + code = main(list(argv)) + return code, json.loads(out.getvalue().strip().splitlines()[-1]) + + +def _answer_args(store, request, *extra: str) -> list[str]: + return [ + "item-approval", + "approve", + str(store.run_dir), + "--request-id", + request.request_id, + "--request-sha256", + request.digest(), + "--decided-by", + "human", + "--route", + "desktop_run_it", + *extra, + ] + + +def test_an_agent_claiming_to_be_a_person_is_refused(tmp_path, approver): + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + + code, out = _cli(*_answer_args(store, request, "--unauthenticated")) + + assert code == 2 and out["code"] == "human_answer_unsigned" + assert store.read_answer(request.request_id) is None + + +def test_an_unsigned_answer_written_straight_to_the_run_is_refused(tmp_path, approver): + """The run checks every answer itself; the CLI's checks aren't the control.""" + + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + with pytest.raises(ItemApprovalRefused) as raised: + make_answer( + request, + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_id=None, + auth_method="none", + policy=store.context(), + ) + assert raised.value.code == "human_answer_unsigned" + forged = ItemApprovalAnswer( + request_id=request.request_id, + request_sha256=request.digest(), + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_kind="unauthenticated", + auth_method="none", + answered_at=request.created_at, + ) + assert answer_refusal(request, forged, policy=store.context()) == ( + "human_answer_unsigned" + ) + + +def test_a_claimed_cloud_sign_in_is_refused(tmp_path, approver): + key_path, _signer = approver + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + + code, out = _cli( + *_answer_args( + store, + request, + "--approver-id", + "cloud-user-0001", + "--auth-method", + "cloud_aal2", + ) + ) + assert code == 2 and out["code"] == "human_answer_unsigned" + + code, out = _cli( + *_answer_args( + store, + request, + "--approver-id", + "front-desk", + "--auth-method", + "cloud_aal2", + "--approver-key", + str(key_path), + ) + ) + assert code == 2 and out["code"] == "auth_method_unverified" + + +def test_a_signed_answer_from_the_desktop_key_passes(tmp_path, approver): + key_path, _signer = approver + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + + code, out = _cli( + *_answer_args( + store, request, "--unauthenticated", "--approver-key", str(key_path) + ) + ) + + assert code == 0 and out["ok"] is True + answer = store.read_answer(request.request_id) + assert answer is not None and answer.attestation is not None + assert answer.attestation.signer == "front-desk" + assert answer.attestation.method == "desktop_key" + assert answer_refusal(request, answer, policy=store.context()) is None + + +def test_a_changed_signed_answer_is_refused(tmp_path, approver): + _key_path, signer = approver + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + answer = make_answer( + request, + decision="decline", + decline_reason="wrong_values", + decided_by="human", + route="desktop_run_it", + approver_id=None, + auth_method="none", + policy=store.context(), + signer=signer, + ) + flipped = answer.model_copy(update={"decision": "approve", "decline_reason": None}) + assert answer_refusal(request, flipped, policy=store.context()) == ( + "human_answer_signature_invalid" + ) + + +def test_the_portal_key_cant_answer_for_desktop(tmp_path, approver): + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + portal = load_signer( + store.key_directory() / "decision-portal.approver.pem", + store.key_directory(), + method="portal_key", + ) + with pytest.raises(ItemApprovalRefused) as raised: + make_answer( + request, + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_id=None, + auth_method="none", + policy=store.context(), + signer=portal, + ) + assert raised.value.code == "attestation_method_mismatch" + + +def test_an_agent_key_is_not_an_approver(tmp_path, approver, monkeypatch): + key_dir = Path(approver[0]).parent + agent_key = tmp_path / "agent.pem" + env = {KEY_PASSPHRASE_ENV: PASSPHRASE} + public = generate_encrypted_key(agent_key, env=env) + register_principal( + key_dir, role="agent", principal_id="agent:ops", public_key_hex=public + ) + store = ItemApprovalStore(tmp_path / "run") + request = _request(store) + + code, out = _cli( + *_answer_args( + store, request, "--unauthenticated", "--approver-key", str(agent_key) + ) + ) + assert code == 2 and out["code"] == "human_approver_unregistered" + + +def test_an_approver_never_answers_a_run_it_started(tmp_path, approver): + _key_path, signer = approver + store = ItemApprovalStore(tmp_path / "run") + store.write_initiator("front-desk") + request = _request(store) + with pytest.raises(ItemApprovalRefused) as raised: + make_answer( + request, + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_id=None, + auth_method="none", + policy=store.context(), + signer=signer, + ) + assert raised.value.code == "self_approval" + + +def test_no_registry_means_no_person_answer(tmp_path, approver): + _key_path, signer = approver + store = ItemApprovalStore(tmp_path / "run", key_dir=tmp_path / "empty-keys") + request = _request(store) + with pytest.raises(ItemApprovalRefused) as raised: + make_answer( + request, + decision="approve", + decided_by="human", + route="desktop_run_it", + approver_id=None, + auth_method="none", + policy=store.context(), + signer=signer, + ) + assert raised.value.code == "human_approver_unregistered" + + +# -- durable resume approval ------------------------------------------------- + + +def test_durable_approve_needs_the_approver_key_not_the_os_user( + tmp_path, approver, capsys +): + from openadapt_flow.__main__ import main + from openadapt_flow.runtime.durable.checkpoint import CheckpointStore + from tests.test_cli_new_commands import _paused_run + + run = _paused_run(tmp_path) + assert main(["approve", str(run)]) == 1 + assert "--approver-key" in capsys.readouterr().out + assert CheckpointStore(run).read_pending().status == "pending" + + key_path, _signer = approver + assert ( + main(["approve", str(run), "--approver-key", str(key_path), "--approver", "x"]) + == 1 + ) + assert main(["approve", str(run), "--approver-key", str(key_path)]) == 0 + approval = CheckpointStore(run).read_approval() + assert approval is not None and approval.approver == "front-desk" + assert approval.attestation is not None + assert approval.attestation.signer == "front-desk" + + +def test_resume_refuses_an_unsigned_stored_approval(tmp_path, approver, capsys): + """An agent that writes approval.json itself gets nothing.""" + + from openadapt_flow.__main__ import main + from openadapt_flow.runtime.durable import bundle_version, issue_resume_approval + from openadapt_flow.runtime.durable.checkpoint import CheckpointStore + from tests.test_cli_new_commands import _paused_run + + run = _paused_run(tmp_path) + store = CheckpointStore(run) + pending = store.read_pending() + manifest = store.read_manifest() + forged = issue_resume_approval( + pending, + approver="front-desk", + resolution="approved", + bundle_version=bundle_version(manifest.bundle_dir), + run_id=manifest.run_id, + workflow_name=manifest.workflow_name, + run_dir=run, + ) + store.commit_approval_transition( + expected_pending=pending, approval=forged, target_status="approved" + ) + + assert main(["resume", str(run), "--require-approval"]) == 3 + assert "human_answer_unsigned" in capsys.readouterr().out diff --git a/packages/flow/tests/test_self_repair.py b/packages/flow/tests/test_self_repair.py index b4209fec58..cb47ca0ff6 100644 --- a/packages/flow/tests/test_self_repair.py +++ b/packages/flow/tests/test_self_repair.py @@ -586,3 +586,7 @@ def test_a_give_up_still_counts_its_model_calls(mockmed, tmp_path) -> None: assert result.evidence.model_calls == 1 assert result.evidence.est_model_cost_usd == 0.02 assert _records(base_url) == [] + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_self_repair_bridge.py b/packages/flow/tests/test_self_repair_bridge.py index c291692803..7e242cbcb5 100644 --- a/packages/flow/tests/test_self_repair_bridge.py +++ b/packages/flow/tests/test_self_repair_bridge.py @@ -190,3 +190,7 @@ def test_bridges_that_save_invent_values_or_run_long_are_refused( assert len(_records(base_url)) == 0 assert RepairStore(tmp_path / "repair-store").list_candidates() == [] assert not (tmp_path / "run" / "item-approval").exists() + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver") diff --git a/packages/flow/tests/test_self_repair_cli.py b/packages/flow/tests/test_self_repair_cli.py index f5276beaf0..f678582ede 100644 --- a/packages/flow/tests/test_self_repair_cli.py +++ b/packages/flow/tests/test_self_repair_cli.py @@ -30,6 +30,10 @@ pytest.importorskip("playwright.sync_api") +# The phone portal records the person's tap signed with its registered +# approver key (OPENADAPT_PORTAL_APPROVER_KEY); without one it records nothing. +pytestmark = pytest.mark.usefixtures("approver") + #: Set by the test before the CLI loads the agent. TARGET: Optional[tuple[int, int]] = None @@ -139,6 +143,8 @@ def test_run_self_repair_cli_resumes_after_one_phone_tap( out = capsys.readouterr().out assert answered, out assert "item-approval approve" in out + # The printed Run it command signs with the person's approver key. + assert "--approver-key" in out assert "Self-repair: resumed" in out resumed = RunReport.model_validate_json( (run_dir / "self-repair" / "resume" / "report.json").read_text() diff --git a/packages/flow/tests/test_standard_change.py b/packages/flow/tests/test_standard_change.py index ba0c911e33..cbac821c20 100644 --- a/packages/flow/tests/test_standard_change.py +++ b/packages/flow/tests/test_standard_change.py @@ -971,3 +971,7 @@ def test_a_rollback_by_any_route_holds_the_step(mockmed, keys, tmp_path) -> None clear_hold(store_root, "triage", "step_001", by="ops-lead") assert hold_reason(store_root, "triage", "step_001") is None + + +# Every person in these tests signs with a registered approver key. +pytestmark = pytest.mark.usefixtures("approver")