[Task]: FLIP-PT pentest remediation tracker — Final Report v2.0 (2026-05-06)

[atriaybagur]

Addresses FLIP-PT-005 (remainder — the accession-ID log scrubbing landed in #839). Contributes to #429.

What this does

Defines the platform logging policy (docs/source/sys-admin.rst#logging-policy) and brings every service in line with it. The rule set, applied at every log level: no cohort SQL (log a 12-hex SHA-256 fingerprint instead), no accession numbers or patient-level attributes (ordinals / counts / fingerprints), no secrets (headers, bodies, presigned URLs), no full URLs (host + path, query string dropped), no verbatim S3 keys (bucket + hashed key, or the model-id/file-name identifiers the key derives from).

Per service

• data-access-api — get_records error paths logged raw driver text, and psycopg2/SQLAlchemy interpolate the executing statement into it (LINE 1: … context, [SQL: …] suffix); sqlglot parse errors quote the offending fragment. All now log exception class + SQLSTATE + query fingerprint (utils/log_hygiene.py; normalisation matches the query-cache key, so cache and error lines for one query correlate — and the hub stores every cohort query, so ops can re-hash to find matching lines). The parse-reject 400 detail becomes a fixed string: sqlglot's message embeds the SQL, trust-api logs error bodies, and friendly parse feedback is the hub pre-check's job (a parse-invalid query normally never reaches a trust). The UndefinedTable/UndefinedColumn operator-echo diagnostics are preserved (identifier only, context dropped). OMOP engine gains hide_parameters=True (stops person_id lists rendering in wrapped driver errors).
• imaging-api — the two router sites that logged the decoded cohort SQL at INFO now log its fingerprint; the DQR study-query response body (patient-level study metadata) is no longer logged at DEBUG; PACS-loop {e} logs become class-only (a pydantic ValidationError renders its input, a requests error the full URL).
• trust-api — utils/http.py logged request headers (the trust-internal service key), the JSON body (decrypted cohort SQL), and params (encoded_query is base64-wrapped SQL) at DEBUG; now method + URL only. Transport-error logs render host + path (httpx merges params into request.url), and the 502 detail carries the exception class instead of str(e).
• flip-utils (runs inside fl-client/fl-server; run logs can leave the trust) — get_dataframe logged the entire row-level cohort response body at INFO; now status code only. Accession numbers in download logs become fingerprints (Utils.hash_for_log), and cleanup no longer names accession-derived paths (in messages or in the raised exception).
• flip-api — cohort-path catch-all logs become class-only (the bound INSERT parameters carry the raw SQL), the model-retrieval debug log drops the query text, s3_client extends FLIP-PT-080: disable SQLAlchemy engine echo on data-access-api #431's bucket= key_hash= error_code= style to every remaining verbatim-key message, and both engines set hide_parameters=True.

Deliberate scope boundaries

• HTTP error response bodies (detail=str(e) sweep, centralised handler) stay with the in-flight Security: consolidate ten pentest-tracker fixes (#879, #888-#894, #912, #913) #930 — only bodies that were already being logged, or that carried SQL, changed here.
• imaging-api's XNAT users/upload services still build exception messages from XNAT response bodies; those surface through the same str(e) sites Security: consolidate ten pentest-tracker fixes (#879, #888-#894, #912, #913) #930's sweep covers, so they're left for it rather than duplicated here.
• S3 keys composed of platform identifiers (<model_id>/<file_name>, both user-visible metadata) remain loggable in that decomposed form — the policy doc records this decision explicitly.
• This changes the S-8 review's "log the full driver error for ops" stance in data-access-api: the comment block and tests now document the replacement diagnostic (class + SQLSTATE + fingerprint, correlatable with hub-stored SQL).

Verification

• flip-api: make local_test (ruff + mypy + 1616 unit tests) green
• data-access-api: ruff + mypy + 185 unit tests green
• imaging-api: ruff + mypy + 195 tests green
• trust-api: ruff + mypy + 106 unit tests green
• flip-utils: ruff + 694 unit tests green; mypy error count identical to the unmodified baseline (67 pre-existing)
• docs: Sphinx build green (one pre-existing unrelated warning)
• New unit tests pin each scrubbed site with caplog / patched-logger assertions that the sensitive value stays out of the log while the fingerprint appears

flip-utils changes are log-line-only (no control-flow change), but since the package runs inside both FL backends' images, a smoke on one backend before merge is cheap insurance if reviewers want it.

[codecov]

Codecov Report

❌ Patch coverage is 85.55556% with 13 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
flip-api/src/flip_api/utils/s3_client.py	52.00%	12 Missing ⚠️
...data-access-api/data_access_api/services/cohort.py	95.00%	1 Missing ⚠️

📢 Thoughts on this report? Let us know!