Portal: relay Flow's save-approval tasks to the paired phone

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

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(

openadapt_desktop/portal/server.py

@@ -69,6 +69,22 @@
 )
 _ROUTE_TASK_DETAIL = re.compile(rf"^/api/portal/tasks/(?P<run_id>{_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>{_REQUEST_ID})$"
+)
+_ROUTE_SAVE_FRAME = re.compile(
+    rf"^/api/portal/save-approvals/(?P<request_id>{_REQUEST_ID})/frame$"
+)
+_ROUTE_SAVE_ACTION = re.compile(
+    rf"^/api/portal/save-approvals/(?P<request_id>{_REQUEST_ID})"
+    rf"/actions/(?P<action_id>{_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
         )

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"