Persist approved cohorts and serve them frozen (#857)

AGENTS.md

@@ -473,7 +473,8 @@ TruffleHog, detect-secrets, large file check (max 1000KB), merge conflict marker
 - Trust-internal service key for trust-api / imaging-api / fl-client → imaging-api / data-access-api auth (per-trust, never leaves trust env). See **Trust-internal Service Authentication** below.
 - FL clients intentionally have no Central Hub credentials.
 - Cohort-query validation is three-layer and **deliberately asymmetric — do not "sync" the layers**. Only the trust-side `data_access_api.services.cohort.validate_query` is authoritative (single parse-validate-emit; length, single-statement, SELECT-only, no `INSERT`/`UPDATE`/`DELETE`/`MERGE` anywhere in the tree — a writable CTE parses as a top-level `Select`, so the shape check alone misses it — `omop`-schema pin, literal `LIMIT`/`OFFSET`; re-emits from the checked AST; backed by the read-only `data_analyst_reader` role — pass its return value to the engine, never the caller's raw string). The hub-side `flip_api.cohort_services.submit_cohort_query.validate_query` is a *fast-feedback validity pre-check only, not a security control*: it exists so a malformed query fails in-hand instead of after an async fan-out to every trust, and enforces only what every trust would reject anyway. The flip-ui cohort form validates required-field only. A trust must stay safe regardless of what the hub checked, so hub drift is safe by construction. **No layer uses a keyword denylist** — the removed one blocked legitimate `SUBSTRING()` while stopping nothing; blind extraction is defeated by the literal-`LIMIT` rule and DDL/DML by the read-only role. See [`trust/data-access-api/README.md`](trust/data-access-api/README.md#cohort-query-validation).
-- Row-level cohort egress is gated on `COHORT_QUERY_THRESHOLD` at **both** row-level routes — `/cohort/dataframe` (FL training data) and `/cohort/accession-ids` (the accession list that decides whose imaging is pulled into XNAT) — sharing one fixed refusal string so a below-threshold cohort is indistinguishable from an empty one. The threshold is the trust's own disclosure floor (default 10, set per trust in its kit file), enforced trust-side rather than relying on the hub's staging guard. Both gates evaluate the **live** cohort on every call: FLIP stores the cohort only as a SQL string and re-runs it against OMOP at every stage, so a project can import cleanly and later start refusing (FLIP#857).
+- Row-level cohort egress is gated on `COHORT_QUERY_THRESHOLD` at **both** row-level routes — `/cohort/dataframe` (FL training data) and `/cohort/accession-ids` (the accession list that decides whose imaging is pulled into XNAT) — sharing one fixed refusal string so a below-threshold cohort is indistinguishable from an empty one. The threshold is the trust's own disclosure floor (default 10, set per trust in its kit file), enforced trust-side rather than relying on the hub's staging guard.
+- Both row-level routes serve **only the frozen approved-cohort snapshot** (FLIP#857): at approval a `PERSIST_COHORT` TrustTask makes each trust run the query of record ONCE and persist the dataframe (`data_access_api/services/cohort_snapshot.py` — parquet + meta per project UUID on the trust's `COHORT_SNAPSHOT_STORAGE_DIR` bind mount, atomic-rename writes, no TTL; the hub records aggregates in `cohort_snapshot_status` and warns on drift vs the approved count). The caller-supplied SQL on those routes is **ignored** — researcher FL code can no longer execute SQL under a project id, the cohort cannot grow after approval, and every training round fetches identical data. A project with no snapshot is refused (fail-closed); a snapshot without `accession_id` serves an empty accession list (tabular project — imaging no-ops); imaging-api resolves the XNAT project's `secondary_ID` (the hub id) before asking, so the ~10s status poll reads the frozen pointer set instead of re-running SQL. Re-approval atomically replaces the artefact — how OMOP-side removals/opt-outs propagate, never silently mid-training. Live OMOP is evaluated only by `/cohort` statistics (pre-approval) and `/cohort/snapshot` (once per approval); the threshold gate reads the frozen count but the live threshold value. The snapshot **write** routes (`/cohort/snapshot`, `/cohort/snapshot/delete`) that define/destroy this artefact carry a second auth gate beyond the shared trust-internal key — proof of possessing `AES_KEY_BASE64` (held by trust-api + data-access-api, not fl-client) — so researcher FL code, which holds the trust-internal key for its reads, cannot rewrite or delete a project's frozen cohort. See the **Trust-internal Service Authentication** section's "Cohort-admin gate".
 - Do not hardcode env values in Dockerfiles or compose files.
 - 72-hour supply-chain cooldown on Python/npm package installs — enforced by uv `exclude-newer` (`[tool.uv]` in every `pyproject.toml`) and npm `min-release-age` (`flip-ui/.npmrc`, requires npm >= 11.10 which Node 24 LTS ships), backstopped by a `uv lock --check` CI gate in `secret-scanning.yml`. See CONTRIBUTING.md ("Dependency cooldown").
 
@@ -487,14 +488,16 @@ TruffleHog, detect-secrets, large file check (max 1000KB), merge conflict marker
 
 **Generating keys.** The key is minted by `register_trust` (`make register-trusts`), which writes `TRUST_INTERNAL_SERVICE_KEY` into the trust's kit file. Re-register to rotate.
 
+**Cohort-admin gate (write routes, FLIP#857).** The trust-internal key does not distinguish callers, and fl-client legitimately holds it (it reads the frozen cohort via `flip.get_dataframe` and pulls imaging). That is safe for the read routes, but data-access-api's cohort-**defining** writes — `POST /cohort/snapshot` (materialise/replace the frozen artefact everyone then trains on) and `POST /cohort/snapshot/delete` — must not be reachable by researcher training code. They therefore carry a **second** router-level gate on top of the trust-internal key: `authenticate_cohort_admin`, proof of possessing `AES_KEY_BASE64`. trust-api and data-access-api hold that key (they encrypt/decrypt hub payloads with it); fl-client deliberately does not (it is not in fl-client's compose env). The proof is the **SHA-256 of the key**, not the key itself, so it never travels the wire or lands in a log; receivers compare it with `hmac.compare_digest`. A caller with a valid trust-internal key but no proof gets **403** (authenticated, not authorised); the dependency order (trust-internal first) means a caller with neither still gets 401 first. No new secret is minted or distributed — the gate reuses the existing AES-key possession boundary, so kit files, `register_trust`, and compose are unchanged. Header name: `COHORT_ADMIN_KEY_HEADER` (default `X-Cohort-Admin-Key`), configured on both trust-api and data-access-api.
+
 **Per-service code.** The auth check lives in each receiving service's `utils/internal_auth.py`:
 
 - `trust/imaging-api/imaging_api/utils/internal_auth.py` — applied at the router level on every imaging-api router except `/health`.
-- `trust/data-access-api/data_access_api/utils/internal_auth.py` — applied at the router level on `/cohort` (covers `/cohort`, `/cohort/dataframe`, `/cohort/accession-ids`).
+- `trust/data-access-api/data_access_api/utils/internal_auth.py` — `authenticate_internal_service` gates the read router (`/cohort` statistics, `/cohort/dataframe`, `/cohort/accession-ids`); the write router (`/cohort/snapshot`, `/cohort/snapshot/delete`) additionally requires `authenticate_cohort_admin` (the AES-possession proof above).
 
 The senders construct the header inline at call sites:
 
-- `trust-api/trust_api/services/task_handlers.py::_trust_internal_headers()` — used on outbound imaging-api and data-access-api calls.
+- `trust-api/trust_api/services/task_handlers.py::trust_internal_headers()` — used on outbound imaging-api and data-access-api calls; `cohort_admin_headers()` in the same module layers the cohort-admin proof on top for the snapshot write.
 - `imaging-api/imaging_api/services_external/data_access.py` — used on the outbound `/cohort/accession-ids` call.
 - The `flip` Python package — lives at [`flip-utils/flip/`](flip-utils/flip/) in this mono-repo, consumed by both the NVFLARE and Flower fl-client / fl-server images built from `fl-services/`. Wraps every fl-client call to imaging-api (`flip.get_by_accession_number`, etc.) and data-access-api (`flip.get_dataframe`). The package reads `TRUST_INTERNAL_SERVICE_KEY` from `os.environ` and forwards it on every request. **User-uploaded training code (`client_app.py`, `server_app.py`, anything under `tutorials/`) does not deal with the header directly** — it calls `flip.*` and the package handles transport-level auth.
 

CLAUDE.md

@@ -473,7 +473,8 @@ TruffleHog, detect-secrets, large file check (max 1000KB), merge conflict marker
 - Trust-internal service key for trust-api / imaging-api / fl-client → imaging-api / data-access-api auth (per-trust, never leaves trust env). See **Trust-internal Service Authentication** below.
 - FL clients intentionally have no Central Hub credentials.
 - Cohort-query validation is three-layer and **deliberately asymmetric — do not "sync" the layers**. Only the trust-side `data_access_api.services.cohort.validate_query` is authoritative (single parse-validate-emit; length, single-statement, SELECT-only, no `INSERT`/`UPDATE`/`DELETE`/`MERGE` anywhere in the tree — a writable CTE parses as a top-level `Select`, so the shape check alone misses it — `omop`-schema pin, literal `LIMIT`/`OFFSET`; re-emits from the checked AST; backed by the read-only `data_analyst_reader` role — pass its return value to the engine, never the caller's raw string). The hub-side `flip_api.cohort_services.submit_cohort_query.validate_query` is a *fast-feedback validity pre-check only, not a security control*: it exists so a malformed query fails in-hand instead of after an async fan-out to every trust, and enforces only what every trust would reject anyway. The flip-ui cohort form validates required-field only. A trust must stay safe regardless of what the hub checked, so hub drift is safe by construction. **No layer uses a keyword denylist** — the removed one blocked legitimate `SUBSTRING()` while stopping nothing; blind extraction is defeated by the literal-`LIMIT` rule and DDL/DML by the read-only role. See [`trust/data-access-api/README.md`](trust/data-access-api/README.md#cohort-query-validation).
-- Row-level cohort egress is gated on `COHORT_QUERY_THRESHOLD` at **both** row-level routes — `/cohort/dataframe` (FL training data) and `/cohort/accession-ids` (the accession list that decides whose imaging is pulled into XNAT) — sharing one fixed refusal string so a below-threshold cohort is indistinguishable from an empty one. The threshold is the trust's own disclosure floor (default 10, set per trust in its kit file), enforced trust-side rather than relying on the hub's staging guard. Both gates evaluate the **live** cohort on every call: FLIP stores the cohort only as a SQL string and re-runs it against OMOP at every stage, so a project can import cleanly and later start refusing (FLIP#857).
+- Row-level cohort egress is gated on `COHORT_QUERY_THRESHOLD` at **both** row-level routes — `/cohort/dataframe` (FL training data) and `/cohort/accession-ids` (the accession list that decides whose imaging is pulled into XNAT) — sharing one fixed refusal string so a below-threshold cohort is indistinguishable from an empty one. The threshold is the trust's own disclosure floor (default 10, set per trust in its kit file), enforced trust-side rather than relying on the hub's staging guard.
+- Both row-level routes serve **only the frozen approved-cohort snapshot** (FLIP#857): at approval a `PERSIST_COHORT` TrustTask makes each trust run the query of record ONCE and persist the dataframe (`data_access_api/services/cohort_snapshot.py` — parquet + meta per project UUID on the trust's `COHORT_SNAPSHOT_STORAGE_DIR` bind mount, atomic-rename writes, no TTL; the hub records aggregates in `cohort_snapshot_status` and warns on drift vs the approved count). The caller-supplied SQL on those routes is **ignored** — researcher FL code can no longer execute SQL under a project id, the cohort cannot grow after approval, and every training round fetches identical data. A project with no snapshot is refused (fail-closed); a snapshot without `accession_id` serves an empty accession list (tabular project — imaging no-ops); imaging-api resolves the XNAT project's `secondary_ID` (the hub id) before asking, so the ~10s status poll reads the frozen pointer set instead of re-running SQL. Re-approval atomically replaces the artefact — how OMOP-side removals/opt-outs propagate, never silently mid-training. Live OMOP is evaluated only by `/cohort` statistics (pre-approval) and `/cohort/snapshot` (once per approval); the threshold gate reads the frozen count but the live threshold value. The snapshot **write** routes (`/cohort/snapshot`, `/cohort/snapshot/delete`) that define/destroy this artefact carry a second auth gate beyond the shared trust-internal key — proof of possessing `AES_KEY_BASE64` (held by trust-api + data-access-api, not fl-client) — so researcher FL code, which holds the trust-internal key for its reads, cannot rewrite or delete a project's frozen cohort. See the **Trust-internal Service Authentication** section's "Cohort-admin gate".
 - Do not hardcode env values in Dockerfiles or compose files.
 - 72-hour supply-chain cooldown on Python/npm package installs — enforced by uv `exclude-newer` (`[tool.uv]` in every `pyproject.toml`) and npm `min-release-age` (`flip-ui/.npmrc`, requires npm >= 11.10 which Node 24 LTS ships), backstopped by a `uv lock --check` CI gate in `secret-scanning.yml`. See CONTRIBUTING.md ("Dependency cooldown").
 
@@ -487,14 +488,16 @@ TruffleHog, detect-secrets, large file check (max 1000KB), merge conflict marker
 
 **Generating keys.** The key is minted by `register_trust` (`make register-trusts`), which writes `TRUST_INTERNAL_SERVICE_KEY` into the trust's kit file. Re-register to rotate.
 
+**Cohort-admin gate (write routes, FLIP#857).** The trust-internal key does not distinguish callers, and fl-client legitimately holds it (it reads the frozen cohort via `flip.get_dataframe` and pulls imaging). That is safe for the read routes, but data-access-api's cohort-**defining** writes — `POST /cohort/snapshot` (materialise/replace the frozen artefact everyone then trains on) and `POST /cohort/snapshot/delete` — must not be reachable by researcher training code. They therefore carry a **second** router-level gate on top of the trust-internal key: `authenticate_cohort_admin`, proof of possessing `AES_KEY_BASE64`. trust-api and data-access-api hold that key (they encrypt/decrypt hub payloads with it); fl-client deliberately does not (it is not in fl-client's compose env). The proof is the **SHA-256 of the key**, not the key itself, so it never travels the wire or lands in a log; receivers compare it with `hmac.compare_digest`. A caller with a valid trust-internal key but no proof gets **403** (authenticated, not authorised); the dependency order (trust-internal first) means a caller with neither still gets 401 first. No new secret is minted or distributed — the gate reuses the existing AES-key possession boundary, so kit files, `register_trust`, and compose are unchanged. Header name: `COHORT_ADMIN_KEY_HEADER` (default `X-Cohort-Admin-Key`), configured on both trust-api and data-access-api.
+
 **Per-service code.** The auth check lives in each receiving service's `utils/internal_auth.py`:
 
 - `trust/imaging-api/imaging_api/utils/internal_auth.py` — applied at the router level on every imaging-api router except `/health`.
-- `trust/data-access-api/data_access_api/utils/internal_auth.py` — applied at the router level on `/cohort` (covers `/cohort`, `/cohort/dataframe`, `/cohort/accession-ids`).
+- `trust/data-access-api/data_access_api/utils/internal_auth.py` — `authenticate_internal_service` gates the read router (`/cohort` statistics, `/cohort/dataframe`, `/cohort/accession-ids`); the write router (`/cohort/snapshot`, `/cohort/snapshot/delete`) additionally requires `authenticate_cohort_admin` (the AES-possession proof above).
 
 The senders construct the header inline at call sites:
 
-- `trust-api/trust_api/services/task_handlers.py::_trust_internal_headers()` — used on outbound imaging-api and data-access-api calls.
+- `trust-api/trust_api/services/task_handlers.py::trust_internal_headers()` — used on outbound imaging-api and data-access-api calls; `cohort_admin_headers()` in the same module layers the cohort-admin proof on top for the snapshot write.
 - `imaging-api/imaging_api/services_external/data_access.py` — used on the outbound `/cohort/accession-ids` call.
 - The `flip` Python package — lives at [`flip-utils/flip/`](flip-utils/flip/) in this mono-repo, consumed by both the NVFLARE and Flower fl-client / fl-server images built from `fl-services/`. Wraps every fl-client call to imaging-api (`flip.get_by_accession_number`, etc.) and data-access-api (`flip.get_dataframe`). The package reads `TRUST_INTERNAL_SERVICE_KEY` from `os.environ` and forwards it on every request. **User-uploaded training code (`client_app.py`, `server_app.py`, anything under `tutorials/`) does not deal with the header directly** — it calls `flip.*` and the package handles transport-level auth.