From 3c66e27a1ee504ee88bc2d9398cf8328fdf9d983 Mon Sep 17 00:00:00 2001 From: Richard Abrich <774615+abrichr@users.noreply.github.com> Date: Sat, 17 Oct 2026 23:16:40 -0400 Subject: [PATCH] feat(portal): relay Flow's save-approval tasks to the paired phone Allowlist Flow's /api/save-approvals routes (list, signed task detail, exact screenshot, decision) behind /api/portal/save-approvals with the same session, origin and CSRF checks as attended decisions. Desktop forwards the phone's answer verbatim and adds no authority. Contract documented in docs/DECISION_PORTAL.md for the portal UI work to build on. --- docs/DECISION_PORTAL.md | 37 +++++++++++ openadapt_desktop/portal/flow_client.py | 26 +++++++- openadapt_desktop/portal/server.py | 55 +++++++++++++++- tests/test_portal/test_server.py | 84 +++++++++++++++++++++++++ 4 files changed, 199 insertions(+), 3 deletions(-) diff --git a/docs/DECISION_PORTAL.md b/docs/DECISION_PORTAL.md index 33626ae..75b9675 100644 --- a/docs/DECISION_PORTAL.md +++ b/docs/DECISION_PORTAL.md @@ -193,6 +193,43 @@ action, and one-use idempotency key. After an accepted answer, Flow re-reads the live application and repeats its required state, identity, and target checks before any action continues. +## Approving a save + +Under an approval mode (`each_item`, or `problems_only` once earned) a run stops +before a save and waits for a person. The paired phone answers that wait the +same way it answers a paused run: Flow signs a decision task, and the portal +relays it without adding authority. This section is the contract the phone +screens build on. + +| Portal route | Flow console route | What it returns | +|---|---|---| +| `GET /api/portal/save-approvals` | `GET /api/save-approvals` | Waiting saves: `request_id`, `step_class`, `created_at`, `expires_at` | +| `GET /api/portal/save-approvals/{request_id}` | `GET /api/save-approvals/{request_id}` | `task` (signed `openadapt.human-decision-task/v1`, `task_kind: save_approval`, question `approve_save`), `task_digest`, and a local-only `presentation` block | +| `GET /api/portal/save-approvals/{request_id}/frame` | `GET /api/save-approvals/{request_id}/frame` | The exact screenshot the request was opened with (PNG, digest-checked by Flow, `no-store`) | +| `POST /api/portal/save-approvals/{request_id}/actions/{action}` | `POST /api/save-approvals/{request_id}/actions/{action}` | A `HumanDecisionReceiptV1` | + +`request_id` is 32 lowercase hex characters; anything else is refused before +Flow is called. `presentation` carries `question`, `step_intent`, +`step_index`, `step_class`, `explanation`, `screenshot_sha256`, +`decline_reasons` and `expires_at`. Its `screenshot_route` names the Flow +console path; the phone loads the portal's `.../frame` route above. + +The task's `allowed_actions` are `verify_and_resume` (approve this save) and +`reject` (don't save). The POST body is +`{"action", "task_digest", "task_signature"}` plus, for `reject` only, one +`decline_reason`: `wrong_values`, `wrong_record` or `screen_problem`. The same +origin and session CSRF checks as every other decision apply. Flow refuses a +task that changed or belongs to another save, and writes the answer once, so a +second tap gets `409 request_not_waiting`. The receipt is +`accepted_pending_runner` for an approval (the run re-reads the form before it +saves and stops if anything changed) or `rejected` for a decline. + +As with every portal decision, pairing proves a device, not a person: Flow +records a save answered here as an unauthenticated person on the +`decision_portal` route. Save approvals stay on the runner. The hosted lane +does not carry them, because the screenshot never leaves the runner and a +person must see exactly what will be saved before approving it. + ## Protected evidence Task projections, decision outcomes, and evidence crops are served diff --git a/openadapt_desktop/portal/flow_client.py b/openadapt_desktop/portal/flow_client.py index 898454c..e1a630c 100644 --- a/openadapt_desktop/portal/flow_client.py +++ b/openadapt_desktop/portal/flow_client.py @@ -51,6 +51,8 @@ #: here too so the two boundaries cannot silently drift apart. _SAFE_RUN_ID = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]{0,127}$") _SAFE_ACTION_ID = re.compile(r"^[a-z][a-z_]{0,39}$") +#: Flow's item-approval request ids are 32 lowercase hex characters. +_SAFE_REQUEST_ID = re.compile(r"^[0-9a-f]{32}$") RouteMethod = Literal["GET", "POST"] @@ -73,8 +75,10 @@ class FlowRoute: #: The complete set of upstream console routes the portal may reach. This is -#: the wire contract with ``openadapt-flow``; every entry exists on Flow -#: ``origin/main`` today (``openadapt_flow/console/app.py``). +#: the wire contract with ``openadapt-flow`` (``openadapt_flow/console/app.py``). +#: The ``save_approval*`` routes are Flow's save-approval endpoints +#: (``openadapt_flow/console/save_approvals.py``); against an older console +#: they answer 404 and the portal relays that refusal unchanged. FLOW_ROUTES: dict[str, FlowRoute] = { "session": FlowRoute("GET", "/api/session", protected=False), "notification": FlowRoute("GET", "/api/attention/notification", protected=False), @@ -86,6 +90,16 @@ class FlowRoute: "task_evidence": FlowRoute( "GET", "/api/runs/{run_id}/artifact", protected=True ), + "save_approvals": FlowRoute("GET", "/api/save-approvals", protected=True), + "save_approval_detail": FlowRoute( + "GET", "/api/save-approvals/{request_id}", protected=True + ), + "save_approval_frame": FlowRoute( + "GET", "/api/save-approvals/{request_id}/frame", protected=True + ), + "save_approval_action": FlowRoute( + "POST", "/api/save-approvals/{request_id}/actions/{action_id}", protected=True + ), } @@ -155,6 +169,7 @@ def request( *, run_id: str = "", action_id: str = "", + request_id: str = "", params: dict[str, str] | None = None, json_body: Any | None = None, ) -> FlowResponse: @@ -177,6 +192,13 @@ def request( "refusing an unexpected run identifier", reason="bad_identifier" ) path = path.replace("{run_id}", quote(str(run_id), safe="")) + if "{request_id}" in path: + if not _SAFE_REQUEST_ID.fullmatch(str(request_id)): + raise FlowConsoleUnavailable( + "refusing an unexpected save approval identifier", + reason="bad_identifier", + ) + path = path.replace("{request_id}", str(request_id)) if "{action_id}" in path: if not _SAFE_ACTION_ID.fullmatch(str(action_id)): raise FlowConsoleUnavailable( diff --git a/openadapt_desktop/portal/server.py b/openadapt_desktop/portal/server.py index c7b39f2..4300903 100644 --- a/openadapt_desktop/portal/server.py +++ b/openadapt_desktop/portal/server.py @@ -69,6 +69,22 @@ ) _ROUTE_TASK_DETAIL = re.compile(rf"^/api/portal/tasks/(?P{_RUN_ID})$") +# Saves waiting for a person under an approval mode. Flow serves each one as a +# signed ``save_approval`` decision task plus the exact screenshot; the portal +# relays them like any other task and adds no authority. +_REQUEST_ID = r"[0-9a-f]{32}" +_ROUTE_SAVES = re.compile(r"^/api/portal/save-approvals$") +_ROUTE_SAVE_DETAIL = re.compile( + rf"^/api/portal/save-approvals/(?P{_REQUEST_ID})$" +) +_ROUTE_SAVE_FRAME = re.compile( + rf"^/api/portal/save-approvals/(?P{_REQUEST_ID})/frame$" +) +_ROUTE_SAVE_ACTION = re.compile( + rf"^/api/portal/save-approvals/(?P{_REQUEST_ID})" + rf"/actions/(?P{_ACTION_ID})$" +) + _SECURITY_HEADERS = { "Content-Security-Policy": ( "default-src 'self'; script-src 'self'; style-src 'self'; " @@ -254,6 +270,32 @@ def handle( action.group("action_id"), ) + if method == "GET" and _ROUTE_SAVES.fullmatch(path): + return self._relay("save_approvals") + + save = _ROUTE_SAVE_DETAIL.fullmatch(path) + if method == "GET" and save: + return self._relay( + "save_approval_detail", request_id=save.group("request_id") + ) + + save_frame = _ROUTE_SAVE_FRAME.fullmatch(path) + if method == "GET" and save_frame: + return self._relay( + "save_approval_frame", request_id=save_frame.group("request_id") + ) + + save_action = _ROUTE_SAVE_ACTION.fullmatch(path) + if method == "POST" and save_action: + return self._decide( + headers, + body, + session, + save_action.group("request_id"), + save_action.group("action_id"), + route="save_approval_action", + ) + detail = _ROUTE_TASK_DETAIL.fullmatch(path) if method == "GET" and detail: run_id = self._safe_run_id(detail.group("run_id")) @@ -298,8 +340,15 @@ def _decide( session: DeviceSession, run_id: str | None, action_id: str, + *, + route: str = "task_action", ): - """Relay one decision to Flow. Desktop adds no authority of its own.""" + """Relay one decision to Flow. Desktop adds no authority of its own. + + ``route`` is ``task_action`` for a paused run (``run_id`` is the + attention id) or ``save_approval_action`` for a waiting save + (``run_id`` is the save approval's request id). + """ if run_id is None: return self._refusal(404, "not_found", "No such decision.") if len(body) > _MAX_BODY_BYTES: @@ -324,6 +373,10 @@ def _decide( # The phone's payload is forwarded as-is. Flow validates it against the # signed task digest, the engine capability, and the allowed-action # list; Desktop must not pre-approve or rewrite any of that. + if route == "save_approval_action": + return self._relay( + route, request_id=run_id, action_id=action_id, json_body=payload + ) return self._relay( "task_action", run_id=run_id, action_id=action_id, json_body=payload ) diff --git a/tests/test_portal/test_server.py b/tests/test_portal/test_server.py index 0432d2d..4e2c1c6 100644 --- a/tests/test_portal/test_server.py +++ b/tests/test_portal/test_server.py @@ -417,3 +417,87 @@ def test_the_relay_refuses_an_identifier_the_http_surface_would_have_rejected() with pytest.raises(FlowConsoleUnavailable) as refused: client.request("task_action", run_id="ok", action_id="DROP TABLE") assert refused.value.reason == "bad_identifier" + + +# ------------------------------------------------------------ save approvals + +SAVE_ID = "0123456789abcdef0123456789abcdef" + + +def test_a_waiting_save_is_relayed_like_any_other_task() -> None: + png = b"\x89PNG\r\n\x1a\nfilled form" + flow = FakeFlow({"save_approval_frame": FlowResponse(200, None, png, "image/png")}) + app, pairings = build(flow) + headers = paired(app, pairings) + status, _, _ = app.handle("GET", "/api/portal/save-approvals", headers, b"") + assert status == 200 and flow.calls[-1] == ("save_approvals", {}) + status, _, _ = app.handle( + "GET", f"/api/portal/save-approvals/{SAVE_ID}", headers, b"" + ) + assert status == 200 + assert flow.calls[-1] == ("save_approval_detail", {"request_id": SAVE_ID}) + status, response_headers, body = app.handle( + "GET", f"/api/portal/save-approvals/{SAVE_ID}/frame", headers, b"" + ) + assert status == 200 and body == png + assert "no-store" in response_headers["Cache-Control"] + + +def test_a_save_decision_is_forwarded_verbatim_with_the_session_checks() -> None: + flow = FakeFlow( + { + "save_approval_action": FlowResponse( + 200, {"state": "accepted_pending_runner"}, None, "application/json" + ) + } + ) + app, pairings = build(flow) + headers = paired(app, pairings) + payload = { + "action": "verify_and_resume", + "task_digest": "sha256:" + "b" * 64, + "task_signature": "hmac-sha256:" + "c" * 64, + } + url = f"/api/portal/save-approvals/{SAVE_ID}/actions/verify_and_resume" + status, _, body = app.handle("POST", url, headers, json.dumps(payload).encode()) + assert status == 200 + assert json.loads(body) == {"state": "accepted_pending_runner"} + route, kwargs = flow.calls[-1] + assert route == "save_approval_action" + assert kwargs == { + "request_id": SAVE_ID, + "action_id": "verify_and_resume", + "json_body": payload, + } + calls = len(flow.calls) + for bad in ( + headers | {"x-openadapt-portal-csrf": "wrong"}, + headers | {"origin": "https://attacker.example"}, + ): + status, _, _ = app.handle("POST", url, bad, json.dumps(payload).encode()) + assert status == 403 + assert len(flow.calls) == calls # nothing reached Flow + + +@pytest.mark.parametrize( + "path", + [ + "/api/portal/save-approvals/../tasks", + "/api/portal/save-approvals/NOT-HEX-0123456789abcdef01234567", + f"/api/portal/save-approvals/{SAVE_ID}/frame/extra", + ], +) +def test_malformed_save_ids_never_reach_flow(path: str) -> None: + flow = FakeFlow() + app, pairings = build(flow) + status, _, _ = app.handle("GET", path, paired(app, pairings), b"") + assert status == 404 and flow.calls == [] + + +def test_the_client_refuses_a_malformed_save_id() -> None: + from openadapt_desktop.portal.flow_client import FlowConsoleClient + + client = FlowConsoleClient(port=7863, access_token="t") + with pytest.raises(FlowConsoleUnavailable) as refused: + client.request("save_approval_detail", request_id="../session") + assert refused.value.reason == "bad_identifier"