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.
 - 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").
 

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.
 - 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").
 

docs/source/security.rst

@@ -151,9 +151,15 @@ independent controls would each have to fail before anything unintended could ex
   reveal that a handful of patients matched — the threshold is the trust's own
   disclosure floor (default 10), set by each trust in its deployment kit: trusts need
   not agree on a shared value, and the hub cannot lower it;
-- cached results are scoped to the requesting project and expire in minutes, so no
-  project is served another's data and no result outlives a withdrawal of consent or a
-  correction to a record.
+- row-level data is released only from the **cohort frozen at project approval**: each
+  Trust materialises the approved query's result once and serves that immutable,
+  project-scoped artefact from then on, ignoring any SQL supplied at request time. The
+  cohort a project trains on is therefore exactly the cohort that was approved — it cannot
+  silently grow as the live database grows — and training code cannot run queries of its
+  own. Withdrawals and record corrections propagate at explicit re-approval events, which
+  atomically replace the frozen artefact, and at project teardown, which deletes it —
+  never mid-training, where a silently shifting dataset would corrupt the model without
+  anyone approving the change.
 
 This is achieved **without restricting researchers to a fixed menu of queries** —
 arbitrary analytical SQL remains available. The constraint is on the shape and privilege

flip-api/src/flip_api/db/migrations/versions/b3f1c857a001_persist_cohort_task_and_snapshot_status.py

@@ -0,0 +1,68 @@
+# Copyright (c) 2026 Guy's and St Thomas' NHS Foundation Trust & King's College London
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#     http://www.apache.org/licenses/LICENSE-2.0
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+"""persist_cohort task type and cohort_snapshot_status
+
+Approved-cohort snapshots (FLIP#857): adds the PERSIST_COHORT member to the native
+``tasktype`` Postgres enum (the approval-time task that makes each trust freeze its
+cohort) and the ``cohort_snapshot_status`` table — the hub's per-(project, trust)
+audit record of what was frozen (aggregates only; the row-level cohort never leaves
+the trust). ADD VALUE cannot run inside the migration transaction, hence the
+autocommit block; it is appended last so migrated databases keep the same enum
+order as fresh ones.
+
+Revision ID: b3f1c857a001
+Revises: 46edb903e4d1
+Create Date: 2026-08-26 18:20:00.000000
+
+"""
+from collections.abc import Sequence
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision: str = 'b3f1c857a001'
+down_revision: str | None = '46edb903e4d1'
+branch_labels: str | Sequence[str] | None = None
+depends_on: str | Sequence[str] | None = None
+
+
+def upgrade() -> None:
+    """Apply this revision."""
+    with op.get_context().autocommit_block():
+        op.execute("ALTER TYPE tasktype ADD VALUE IF NOT EXISTS 'PERSIST_COHORT'")
+    op.create_table(
+        'cohort_snapshot_status',
+        sa.Column('id', sa.Uuid(), nullable=False),
+        sa.Column('project_id', sa.Uuid(), nullable=True),
+        sa.Column('trust_id', sa.Uuid(), nullable=True),
+        sa.Column('query_id', sa.Uuid(), nullable=True),
+        sa.Column('row_count', sa.Integer(), nullable=False),
+        sa.Column('approved_record_count', sa.Integer(), nullable=True),
+        sa.Column('has_accessions', sa.Boolean(), nullable=False),
+        sa.Column('query_hash', sa.String(), nullable=True),
+        sa.Column('snapshot_at', sa.DateTime(), nullable=False),
+        sa.Column('created_at', sa.DateTime(), nullable=False),
+        sa.ForeignKeyConstraint(['project_id'], ['projects.id']),
+        sa.ForeignKeyConstraint(['trust_id'], ['trust.id']),
+        sa.PrimaryKeyConstraint('id'),
+    )
+
+
+def downgrade() -> None:
+    """Revert this revision.
+
+    Postgres cannot drop an enum value, so PERSIST_COHORT stays in the type on
+    downgrade — harmless, as pre-#857 code never writes it.
+    """
+    op.drop_table('cohort_snapshot_status')

flip-api/src/flip_api/db/models/main_models.py

@@ -403,3 +403,31 @@ class XNATProjectStatus(SQLModel, table=True):
     query_at_creation: UUID | None = Field(default=None)
     last_reimport: Annotated[datetime, Field(default_factory=lambda: datetime.now(timezone.utc))]
     reimport_count: int = Field(default=0)
+
+
+class CohortSnapshotStatus(SQLModel, table=True):
+    """The hub's per-trust record of what cohort was frozen at approval (FLIP#857).
+
+    Aggregates only — the hub never sees a row of the cohort. Written by the PERSIST_COHORT
+    task's post-processing from the trust's snapshot response; one row per (project, trust),
+    updated in place on re-approval (a re-snapshot replaces the trust-side artefact, so the
+    latest facts are the ones that describe what is being served). ``approved_record_count``
+    is the count the project was staged/approved on (from the aggregated cohort statistics);
+    a mismatch with ``row_count`` means the live cohort drifted between submission and
+    approval, and is logged as a warning when the row is written — surfaced, never silently
+    adopted.
+    """
+
+    __tablename__ = "cohort_snapshot_status"  # type: ignore
+    id: UUID = Field(default_factory=uuid4, primary_key=True)
+    project_id: UUID | None = Field(default=None, foreign_key="projects.id")
+    trust_id: UUID | None = Field(default=None, foreign_key="trust.id")
+    # Which Queries row was frozen — the missing link #857 calls out.
+    query_id: UUID | None = Field(default=None)
+    row_count: int = Field()
+    approved_record_count: int | None = Field(default=None)
+    # False = the frozen cohort has no accession_id column (tabular project; imaging no-ops).
+    has_accessions: bool = Field(default=False)
+    query_hash: str | None = Field(default=None)
+    snapshot_at: datetime = Field()
+    created_at: Annotated[datetime, Field(default_factory=lambda: datetime.now(timezone.utc))]

flip-api/src/flip_api/domain/interfaces/trust.py

@@ -114,6 +114,22 @@ class ICreateImagingProject(BaseModel):
     dicom_to_nifti: bool = True
 
 
+class IPersistCohort(BaseModel):
+    """Payload of the approval-time PERSIST_COHORT task (FLIP#857).
+
+    Carries everything the trust needs to freeze the approved cohort: the query of record
+    and the hub project id both in the clear (for the trust's own logging/keying) and
+    encrypted (what the trust forwards to data-access-api, whose routes take the encrypted
+    form — mirroring the cohort-query task).
+    """
+
+    project_id: UUID  # This is the central hub project ID
+    trust_id: UUID
+    encrypted_project_id: str
+    query: str
+    query_id: UUID | None = None
+
+
 class ICreatedImagingUser(BaseModel):
     """Represents a user created on XNAT. Used to be called IImageUser in the old repo."""
 

flip-api/src/flip_api/domain/schemas/status.py

@@ -184,6 +184,11 @@ class TaskType(StrEnum):
     GET_IMAGING_STATUS = "get_imaging_status"
     REIMPORT_STUDIES = "reimport_studies"
     UPDATE_USER_PROFILE = "update_user_profile"
+    # Approval-time cohort freeze (FLIP#857): the trust materialises the approved cohort
+    # once and persists it; the row-level routes serve only that artefact from then on.
+    # Queued BEFORE the CREATE_IMAGING task so the frozen accession set exists by the time
+    # imaging retrieval asks for it.
+    PERSIST_COHORT = "persist_cohort"
 
 
 class XNATImageStatus(StrEnum):