# Code Review Guide A file-by-file walkthrough of this repository, in suggested reading order. Each entry gives a one-line purpose plus targeted review points — questions to ask while you read the actual code. This is a map and a checklist; it does not replace reading the files. **Suggested path:** Section 1 (orientation) → 2 (skeleton/config) → 3 (domain models) → 4 (webhook pipeline) → 5 (services — where PHI leaves the building) → 6 (intake_app) → 7 (infra) → 8 (scripts/tests/docs). Appendix A is a risk checklist to work through last, with file:line pointers. **While you read, keep these running commands handy:** ```bash docker compose up # db + django + cms-mock; migrations & fixtures run on first start DJANGO_SETTINGS_MODULE=access_core.settings.test .venv/bin/python manage.py test access_core/ # 274 tests, SQLite :memory: .venv/bin/ruff check access_core/ && .venv/bin/ruff format --check access_core/ # infra (per environment dir): terraform init -backend=false && terraform validate && terraform fmt -check -recursive ``` --- ## 0. Repository map | Path | What it is | |------|-----------| | `access_core/` | Django project: 6 domain apps, shared `services/`, `settings/`, custom fields | | `intake_app/` | FastAPI patient-facing intake micro-app (separate dependency set, run on host) | | `infra/` | Terraform: 9 reusable modules + dev/staging/prod environment roots | | `access_fhir_sandbox/` | Dockerfile for the upstream CMS ACCESS mock server (pinned commit, port 3001) | | `scripts/` | Operational one-offs: form builder, integration tests, cohort demo, data munging | | `sample_data/` | Synthetic fixtures and example payloads (all non-PHI by construction) | | `db/local.sqlite3` | Local dev SQLite artifact (gitignored; Postgres is the real target) | | `docs/` | Architecture/security/testing docs + 16 SVG diagrams in `diagrams/` | --- ## 1. Orientation — read these first ### `README.md` Project overview, app list, key design decisions, doc map, local dev commands. **Review points:** Confirm the "Key Design Decisions" and "Django Apps" sections still match the code (they were touched in the consistency-fix round). The Local Development section is your runbook for everything else in this guide. ### `.env.example` Template of every environment variable the stack consumes, with dev defaults. **Review points:** Cross-check against `access_core/settings/base.py` and both compose files — every var a service reads should be documented here. Note `HEALTHIE_WEBHOOK_SECRET` defaults to a weak dev value; `CMS_API_BASE_URL` points at sandbox (`https://sandbox.bcda.cms.gov`), mock is host port **3001**. ### `requirements.txt` (root, Django app) Django 4.2.x, psycopg2-binary, gunicorn <22, python-dotenv <2, cryptography, requests, boto3, pyyaml, python-json-logger 4.x. **Review points:** `cryptography` backs PHI envelope encryption — the only hard security dependency in the repo. Now exact-pinned (`==49.0.0`, see requirements.txt policy comment); a hashed lockfile with `--require-hashes` remains a deferred supply-chain follow-up (Appendix A #5). ### `intake_app/requirements.txt` FastAPI, uvicorn[standard], jinja2 **==3.1.3 exact pin**, pydantic[email], httpx, python-multipart. **Review points:** Independent dependency set from root with no shared lockfile. The exact jinja2 pin is the only strict pin in either file — check it's intentional (template rendering of user input). ### `.github/workflows/ci.yml` Two jobs: `lint-and-test` (matrix Python 3.12/3.13 × Postgres 16: ruff check+format, migrate, full test suite; on main/py3.13 also brings up db + cms-mock and runs `scripts/test_django_integration.py`) and `build-docker` (on main/tags: buildx, ECR login us-east-1, push + force-new-deployment to ECS staging for non-PR pushes). **Review points:** An AWS account ID is hardcoded in the file. Integration tests only run on main — PRs get unit coverage only. Confirm the workflow's compose usage matches `docker-compose.yml` service names (`db`, `django`, `tests`, `cms-mock`). --- ## 2. App skeleton & configuration ### `manage.py` Standard Django entrypoint; dev default settings module. **Review points:** Nothing exotic — verify the default `DJANGO_SETTINGS_MODULE` fallback matches what compose/CI pass explicitly. ### `Dockerfile` (root) Multi-stage python:3.13-slim build of the **Django app**: deps installed in `/install`, runtime stage copies code, non-root `appuser`, healthcheck = `manage.py check --deploy`, CMD gunicorn (4 workers / 2 threads). Exposes 8000. **Review points:** Compose overrides CMD with migrate → loaddata → runserver for dev; the image's own CMD is what staging/prod ECS uses — confirm that path actually runs migrations before gunicorn starts in `infra/modules/ecs/main.tf`. ### `docker-compose.yml` / `docker-compose.staging.yml` Dev stack: `db` (Postgres 16, port `${POSTGRES_PORT:-5432}`), `django`, `tests`, `cms-mock` (from `access_fhir_sandbox/Dockerfile`, host port 3001). Staging compose is a **standalone** file (not an override) with `${VAR:?...}` guards for required secrets. **Review points:** The django service hardcodes `POSTGRES_HOST: db`; every other env var passes through from the shell — compare against `.env.example` for completeness. Dev webhooks run synchronous (`HEALTHIE_WEBHOOK_VERIFY_SIGNATURE=false` is a documented dev shortcut). ### `access_core/settings/base.py` All shared settings; loads `.env` then `.env.local` (override=True); resolves Postgres creds, Healthie/CMS endpoints, PHI key source, JSON logging. **Review points:** The dotenv override chain means **`.env.local` silently wins over your shell env for the Django app** — a common "why isn't my export taking effect?" trap (e.g., it sets `POSTGRES_HOST=l.nso.me`). `ALLOWED_HOSTS`, `SECURE_*` flags, and the PHI key resolution (`PHI_KMS_KEY_ID` vs `PHI_LOCAL_ENCRYPTION_KEY`) live here. ### `access_core/settings/dev.py` / `local.py` / `staging.py` / `test.py` Environment deltas. `test.py`: SQLite `:memory:` — smoke commands must run in one process or against Postgres. `staging.py`: production-ish hardening + JSON log format via `pythonjsonlogger.json.JsonFormatter`. **Review points:** `staging.py` reads `DB_NAME/USER/PASSWORD/HOST/PORT` (different var names than base's `POSTGRES_*`) — easy to misconfigure; the ECS task def in infra injects the right ones, double-check. ### `access_core/constants.py` Shared enums: `TRACK_CHOICES = CKM/eCKM/MSK` (BH intentionally absent), status vocabularies reused across apps. **Review points:** Single source of truth for track/status choices — when reviewing any model with a `choices=` field, verify it references these rather than redefining literals. ### `access_core/fields.py` `EncryptedField(TextField)` and `EncryptedDateField` — transparent encrypt-on-write / decrypt-on-read wrapping the `enc:v1` envelope from `services/phi_crypto.py`. **Review points:** Encryption is at rest only; values are plaintext in Python memory and in query results. Grep for every usage (only DOB on IntakeRecord + Beneficiary today) — any new field using this must also be considered for the pseudonymization design doc's exclusions. ### `access_core/urls.py`, `wsgi.py`, `asgi.py` Route table: `/admin/`, `/health/` (health check), `/api/intake/…` → intake app. wsgi/asgi are standard gunicorn entrypoints. **Review points:** The **only** non-admin HTTP surface is the health check and the Healthie webhook — everything else (CMS ops, curation, governance) is management commands + admin today. That shapes which attack surfaces you care about: webhook signature, admin auth, RDS network isolation. --- ## 3. Domain models by app Read in identity → intake → clinical → monitoring → curation → governance order; that's the data lifecycle (intake record → promoted beneficiary → observations/care periods → curated reports → audit). Each app has the same shape: `models.py` (the substance), `admin.py` (exposure surface — review it for PHI visibility), empty `urls.py` placeholder, `migrations/`, `tests/test_models.py`. ### 3.1 `access_core/identity/` **Purpose:** Internal users, RBAC, external-ID mapping, the two-stage identity model, consent. **Key models (9):** `User` (no password fields — auth lives outside Django), `Role` (9 roles incl. `offshore_reviewer`), `UserRole`, `RolePermission`, `AccessGrant` (scopes: global/beneficiary/care_period/task), `ExternalSystemIdentifier` (Healthie patient id, CMS account ids), **`Beneficiary`** — the promoted identity carrying `dob = EncryptedDateField()`, `BeneficiaryIdentifier` (email/phone/address/MBI rows), `PatientConsent`. **Migration 0004_encrypt_beneficiary_dob.py:** RunPython data migration converting legacy plaintext DOB rows to encrypted envelopes; reversible. **Review points:** - This is the core PHI model — verify `is_synthetic` handling on the intake→beneficiary promotion path (default differs between the two models). - `AccessGrant.clean()` validation only runs under `.full_clean()`, not in bulk ops or admin saves. - **admin.py:** `BeneficiaryAdmin` shows/edits names+dob; `BeneficiaryIdentifierAdmin` exposes raw identifier values with no readonly hardening. - tests/test_models.py: basic creation + encrypted-dob round-trip via string equality — no assertion that the DB row actually holds a ciphertext envelope, no AccessGrant/consent coverage. ### 3.2 `access_core/intake/` **Purpose:** Everything arriving from Healthie before promotion: raw webhook capture, normalized intake record + sub-forms, form submission events. **Key models:** `IntakeRecord` (status lifecycle incl. `pending_consent → pending_alignment → active_access_beneficiary / rejected`; DOB encrypted), `RawIngestionRecord` (idempotent webhook raw store — the retry/replay backbone), `FormSubmissionEvent`, and per-section sub-forms (`IntakeIdentifier`, assessment, lifestyle, female/male health, MSK, safety screening, participation readiness). **Migrations:** 0002 alters DOB + related fields; 0003 adds source-system tracking on form submissions; **0004_encrypt_intakerecord_dob.py** mirrors identity's data migration. **Review points:** `IntakeRecord` is the join point for almost everything else (CMS services, signals, cohort generator) — its status field semantics are worth reading against `services/cms_integration_signals.py`. The sub-form models exist to keep one giant table from forming; check consumers populate them consistently. ### 3.3 `access_core/clinical/` **Purpose:** Clinical observations and their curation lifecycle: labs, vitals, symptoms, imaging findings, PROM scores, OAP measures, LOINC codes. **Key models:** `ClinicalObservation` (12-state status pipeline from `imported` through `qc_passed` / `clinician_review_required` → `approved` → `locked_for_reporting` → `submitted_to_cms`; data sources incl. labcorp/quest/patient_upload/device), `LoincCode`, OAP measure observations. **Review points:** The status pipeline is the spine of CMS reporting eligibility — verify only `approved` (or later) rows can be reported in `services/cms_reporting.py`. Note FKs back to `intake.RawIngestionRecord` and `curation.ClinicalDocument` for provenance. **management/commands/ is empty** — ingestion happens via webhook consumers, not commands; nothing here has HTTP routes either (urls.py empty). ### 3.4 `access_core/monitoring/` **Purpose:** The largest app (~26 models): clinical tracks, CMS eligibility requests/checks + alignment records, care periods + protocol versioning, daily checkins, monitoring plans, adherence, notifications. **Key model groups:** - *CMS state:* `EligibilityRequest` (+ `EligibilityRequestCondition` ICD-10 codes — reviewer-created; Healthie intake data carries no track/diagnoses, and a patient may have one request per track; admin offers code typeahead over a local CDC snapshot table, see §5 `icd10_search.py`), `EligibilityCheck` (FK → its request; check_status now includes `error`), `Alignment`, `Unalignment`, `EnrollmentDecision`. - *Care:* `ClinicalTrack`, `CarePeriod` (FK → `ProtocolVersion`; operational statuses `active/ended/withdrawn/unaligned`), `ProtocolVersion`, `ProtocolAssignmentVersion`. - *Daily data:* `PatientExecutionLog` (col `protocol_assignment_id`), `DailyStatusLog`, `TrackSpecificDailyLog`, `MonitoringPlanTemplate/Plan/Item/Result`, `DailyCheckinTemplate/Item/Submission/Response`, `RawDailyCheckinSubmission`, `MissedCheckinEvent`. - *Rollups:* `PatientAdherenceSummary`, `MonthlyPatientSummary`, `PatientNotification`. **Review points:** Heaviest plaintext-PHI app (symptom flags, vitals, summary JSON) — no encryption here by design (DOB-only rollout), so network/admin isolation is the control. **No unique constraint on (beneficiary, care_period, log_date)** for daily logs → duplicate submissions possible; check whether consumers guard it. `DailyCheckinItem.safety_rule` free-form JSON drives safety-signal logic — review how it's interpreted, not just stored. ### 3.5 `access_core/curation/` **Purpose:** Document storage/metadata and the curation workflow: source documents → clinical documents → observation drafts → curated observations; public reporting with suppression rules. **Key models (12):** `SourceDocumentRecord`, `ClinicalDocument` (`storage_path` plain text, `is_encrypted` flag default True), `DocumentMetadata`, `DocumentVersion`, `DocumentAccessEvent` (access audit trail), `DataCurationTask` (5 task types), `CuratedObservationDraft`, `CurationReviewEvent`, `ReportingView`, `AggregateOutcome`, `SuppressionRule`, `PublicReportExport`. **Review points:** Suppression logic (`suppression_required`, `min_cell_size`) is **model-only — no enforcement code exists yet**; if you review reporting features, that's the gap. Document access events give you per-document audit but only if something writes them (check consumers/commands). ### 3.6 `access_core/governance/` **Purpose:** Physician review decisions, safety signals, clinical event log, care-team tasks, the audit trail, and offshore pseudonymous case/packet records. **Key models (8):** `PhysicianReview`, `ClinicalReviewTask`, `SafetySignalEvent` (red/yellow/green), `ClinicalEventLog`, `CareTeamReviewTask`, **`AuditEvent`** (`changed_fields` JSON + SHA-256 `old_value_hash`/`new_value_hash`, IP/UA, indexed — stores hashes, never raw values), `PseudonymousCaseRecord` (carries the unique `case_id`), `PseudonymousClinicalPacket`. **Review points:** - **Audit trail writer implemented** — `governance/audit.py` appends `AuditEvent` rows via signals on an explicit registry of ~73 PHI/decision models (reference tables and high-churn machine logs excluded). Human actions carry user/IP/user-agent from a request ContextVar (`AuditRequestMiddleware`); system paths get null user. Edits record changed field names + SHA-256 row hashes, never raw values; no-op saves write nothing; `CMSApiRequest` creation is reported as `submit_to_cms`. Audit failures log and swallow (never break the primary operation). `AuditEventAdmin` is fully read-only with add/delete disabled — append-only at the Django layer; per policy no operator has direct DB access, so no DB trigger. - De-identification on the offshore packet is **by convention**: `structured_data`/`signal_details` are free-form JSON with no enforcement that PHI stays out (see `docs/security/PHI_Pseudonymization_Design.md`). - `AuditEvent.target_record_id` is a bare UUID — no FK integrity. --- ## 4. The webhook pipeline (`access_core/intake/`) The only real-time ingestion path. Read in this order: `views.py` → `handlers.py` → `consumers.py`. ### `intake/views.py` Single endpoint `healthie_webhook` (POST-only, csrf_exempt) at `/api/intake/webhooks/healthie/`. Verifies the HMAC signature via `parse_webhook()` unless disabled, dispatches by exact `event_type`, returns 200 or **500 to trigger Healthie's retry**. **Review points:** Transient failures (connection error, timeout, Healthie 5xx) → 500 + Healthie retry, but downstream creates are not idempotent beyond the raw record — a retried webhook can duplicate normalized rows. Permanent Healthie 4xx (bad/expired API key, missing resource) is classified in `consumers._fetch_healthie_resource`: the raw record is marked `failed` and we ack with 200 so retries don't loop until operator intervention. All processing is synchronous in the request thread (no queue/locking); concurrent duplicates race. ### `intake/handlers.py` Tiny decorator registry: `register_handler(event_type)`, `get_handler()`, `get_registered_events()`; imports consumers at module bottom to trigger registration and break the views↔consumers circular import. **Review points:** Exact-string event matching — a new Healthie event type silently falls through to "store raw + 200". Use `get_registered_events()` as your checklist of what's handled. ### `intake/consumers.py` (~1580 lines — the biggest file in the repo) Each handler first stores the webhook idempotently (`_store_raw_ingestion`, get_or_create on source_record_id, status reset for retries), then **fetches full data via Healthie GraphQL** (webhooks are thin pointers). Registered events: `form_answer_group.created` (primary intake — normalizes into IntakeRecord + sub-forms when form ID == `HEALTHIE_INTAKE_FORM_ID`, else daily-checkin path; also `.signed` → FormSubmissionEvent), `patient.created/updated` (link patient→intake), `document.created/updated` (ClinicalDocument row, expiring URL not bytes), `metric_entry.*` (ClinicalObservation + OAPMeasureObservation — later auto-reported to CMS via signals), `entry.*` (symptom entries; MetricEntry explicitly skipped to avoid dupes), `care_plan.{created,updated,activated,deactivated,deleted}` (monitoring plans), `patient.merged` (log-only), `applied_tag.created/deleted` (log + raw store). **Review points:** This is where external PHI enters the DB on every event — check normalization for field-by-field mapping errors against `docs/intake/Cognito_to_Healthie_Form_Mapping.yaml`. The GraphQL fetch means each webhook triggers outbound HTTP back to Healthie (latency + failure modes in-request). Hardcoded form ID coupling (`HEALTHIE_INTAKE_FORM_ID` = 2360739) ties this file to the live Healthie form. ### `intake/management/commands/` - `generate_synthetic_cohort.py` — builds a full synthetic patient cohort across CKM/eCKM/MSK tracks (rejects BH). Your main demo/testing data source; review its realism assumptions and that everything is stamped `is_synthetic=True`. - `seed_synthetic_data.py` + `_synthetic_cleanup.py` — smaller seed + cleanup helpers used by compose's first-boot loaddata path. - `replay_webhook_fixture.py` — manual webhook replay for local testing (the "polling worker" stand-in until SQS/Fargate exists). - `retry_failed_ingestions.py` — operator recovery path: re-runs the registered handler for `RawIngestionRecord` rows stuck in `failed` (default) or stale `processing`, e.g. after rotating the Healthie API key. `--dry-run` lists without touching; per-record errors are tallied and fail the command non-zero. Also exposed as a Django admin action on `RawIngestionRecord`. --- ## 5. Services layer (`access_core/services/`) Where PHI leaves the building and where external systems are called. Read `phi_crypto.py` first, then Healthie, then CMS. ### `phi_crypto.py` Core encryption: `encrypt_value()` / `decrypt_value()` / `is_encrypted()` producing/parsing **`enc:v1::`** envelopes — AES-256-GCM (`cryptography.hazmat AESGCM`). Two backends: `_KmsBackend` (AWS KMS data key, production) and `_LocalBackend` (raw local dev key). Mode resolved from `PHI_KMS_KEY_ID` vs `PHI_LOCAL_ENCRYPTION_KEY`. **Review points:** The envelope format is your migration story for future field rollouts — any change must stay backward-readable. Key rotation semantics: check how a KMS data-key refresh interacts with already-encrypted rows (envelope stores per-value key material, so it should not need rewrites — verify). ### `healthie.py` Minimal Healthie GraphQL client: authenticated session + generic `query()` only (consumers build their own queries). The old form-management/upload helpers had zero call sites and were removed — the FastAPI intake app keeps its own richer client in `intake_app/app/healthie.py`. **Review points:** Basic-auth API key in header; 30s timeout, no retries. All GraphQL strings live with their callers (consumers) — review query construction there if it changes. ### `healthie_webhook.py` HMAC-SHA256 signature verification + payload parsing (`verify_webhook_signature`, `parse_webhook`, `WebhookPayload`). Signs `" {method} {path} {query} {digest-hash} application/json {contentLength}"`; contentLength = length of **compact re-serialized** JSON; key is the full `whsec_…` string; compared with `hmac.compare_digest`. **Review points:** The re-serialization trick can mismatch Healthie's canonicalization on non-ASCII/whitespace edges — fallback fails verification (fail-closed, good). `HEALTHIE_WEBHOOK_VERIFY_SIGNATURE=false` bypass exists for dev only — confirm it cannot be set in staging/prod via env injection. ### `icd10_search.py` Local search over the bundled CDC NCHS annual snapshot (`monitoring.Icd10Code`, seeded from `access_core/monitoring/data/icd10cm-code-descriptions-2027/`; refresh with `manage.py load_icd10_codes`). No runtime external dependency — searches run against Postgres and work offline. Codes are stored dotless as CDC publishes them ("E119") and returned in canonical dotted form ("E11.9"). Only caller is the admin typeahead endpoint on `EligibilityRequestAdmin` (hand-rolled vanilla-JS typeahead — select2's `` compat shim cannot do ajax, see the header comment in `static/monitoring/icd10_autocomplete.js`). No PHI involved — search terms are code fragments / condition names. **Review points:** snapshot is a fiscal-year versioned table; annual refresh replaces it wholesale (codes absent from the new file are deleted = deletion tracking). Lookup failure surfaces as HTTP 502, the JS ignores failed responses, and the field degrades gracefully to manual code entry. Model `clean()` enforces ICD-10-CM *format* only — validity against the current year's code list is left to CMS/mock validation. ### CMS modules (`cms_client.py` + five workflow modules) - **`cms_client.py` (~810 lines):** Low-level client — OAuth2 token management (thread-locked cache), `post_operation`, `get_submission_status` (single GET per call, no blocking poll loop; the URL shape for both comes from the configured `CMS_API_POST_PATH` / `CMS_API_SUBMISSION_STATUS_PATH` path templates — placeholders like `{version}`/`{submission_id}`, blank values drop their segment, and a mock-style query-param form such as `?submissionID={submission_id}` is pure configuration; there is no localhost sniffing left to defeat), structured exception hierarchy (`CmsApiError` → connection/server/rate-limit/auth/validation), FHIR resource builders. Retries: exponential backoff capped 300s, default 3 tries on POST (`CMS_POST_MAX_RETRIES`). All `$submission-status` polling happens in the durable worker (`cms_polling_worker` / `poll_cms_submissions`) — which is what makes the Operations Manual v0.9.12 resume requirement (store submission IDs + request context, continue after interruption) actually met: submissions persist in `CMSApiSubmissionStatus` at submit time and due rows are re-polled on a schedule. See `docs/operations/CMS_Resilience_Config.md` "Documented CMS Polling Behavior." - **`cms_eligibility.py`:** `$check-eligibility` — builds US Core Patient from IntakeRecord; track + ICD-10 conditions come from the reviewer-created `EligibilityRequest` (zero or more conditions sent — none invented), submits, polls, stores outcome on an `EligibilityCheck`; keyword-matching parser (fail-closed). MBI handling is gated by `ALLOW_SYNTHETIC_MBI`: when disabled (staging/prod default) a record with no real cmsMBI identifier **fails closed** instead of sending the PHI-derived synthetic pseudonym (`"S"+sha256(first:last:dob)[:10]`), and stored `CMSApiRequest/Response` payloads are redacted at save time (Patient name/birthDate/telecom/address → `[REDACTED]`, MBI kept). Gate is on in dev/local/test only. - **`cms_alignment.py`:** `$align` — the promotion point: on success creates `Beneficiary` + `CarePeriod` inside `transaction.atomic()`, copies name/dob/identifiers, stores CMS submission id as `ExternalSystemIdentifier(system_name="cms")`; failure marks intake `rejected`. - **`cms_unalignment.py`:** `$unalign` — flips active CarePeriods to `operational_status="unaligned"` via bulk `.update()` (bypasses signals by design). - **`cms_reporting.py`:** `$report-data` — batch-reports approved OAPMeasureObservations as US Core Observations with a 9-measure LOINC mapping; idempotency = "exclude already-submitted" (no DB lock → concurrent runs could double-report); fuzzy measure-name matching can mis-map codes. - **`cms_integration_signals.py`:** Wires Django pre_save/post_save signals to auto-trigger alignment (`pending_consent→pending_alignment`), unalignment (care-period terminal statuses), and reporting (OAP approved). 5-minute idempotency guard; all errors swallowed + logged (**fail-open**); old-status captured in process memory via `_old_status_store`. **Review points (the important ones):** - **Synchronous multi-minute CMS HTTP (incl. polling) runs inside signal handlers, i.e., in the saving request/worker thread** — a single save can block a gunicorn worker for minutes; under multiple workers each has its own in-memory `_old_status_store`, so state is per-process (`_cleanup_stale_entries` is a no-op placeholder). - CMSApiRequest/Response audit rows contain the **full patient payload** (name, DOB, MBI, address, telecom) — check where those are stored and who can read them. - Blocking `time.sleep` retries in request context; worst-case ~5 min per poll cycle. --- ## 6. `intake_app/` — FastAPI patient-facing micro-app Separate service (own requirements, own venv), **not** a docker-compose service — runs on the host via `run.sh` → `uvicorn app.main:app --reload --port ${PORT:-8000}` (note: port collides with Django's published 8000; `--reload` = single-process, which is what makes its in-memory state work at all). ### `intake_app/app/main.py` (~365 lines) Patient-facing multi-section form: `/start`, GET/POST `/section/{num}`, file upload + confirm, finalize → creates Healthie patient, submits answers, uploads files. In-memory `sessions` dict + two in-process `RateLimiter`s (lines 56–60). Honeypot field filtering on submit. **Review points:** - **Unsanitized uploaded filename**: line 348 joins `file.filename` straight into the temp path (`../../temp_uploads/…`) → path traversal / arbitrary file write; no server-side size limit despite metadata claiming 10 MB. - In-memory sessions/rate-limiter: lost on restart, invalid if you ever run >1 worker — fine for dev, not a deployment story. - PHI (name/dob/email/gender/phone) read from **hardcoded Healthie module IDs** and POSTed to Healthie — the ID coupling means form restructuring breaks this silently. - Recaptcha keys are loaded in config but never actually verified here. ### `intake_app/app/config.py` / `schemas.py` / `form_engine.py` / `healthie.py` - **config.py:** env-driven `Settings` singleton; cached at first call (env changes after startup ignored). - **schemas.py:** Pydantic models for rendering/submission — note `SectionSubmit`/`FinalSubmit` are largely dead code (main.py parses raw forms). - **form_engine.py:** fetches the Healthie form definition (GraphQL) and splits it into sections on label-delimited modules; TTL cache. Remote fetch failure → 500 on first page load (no fallback). - **healthie.py:** httpx GraphQL client singleton (`createClient`, `submitFormAnswers`, presigned upload/confirm); raises on any GraphQL error, no retries. ### `intake_app/scripts/setup_metadata.py` One-shot: pushes conditional-section rules + file-upload config onto live Healthie form modules via `updateCustomModule`. Reads `.env.local` directly; hardcoded module IDs must match the live form; run after form structural changes (no idempotency check beyond overwrite). --- ## 7. `infra/` — Terraform Read order: `README.md` → modules in dependency order (`vpc` → `database`, `kms`, `storage`, `queue`, `endpoints` → `ecs`, `alerting`) → env roots (dev → staging → prod). Every module follows main.tf / variables.tf / outputs.tf. Start the review against `docs/architecture/AWS_Component_Diagram.md` and keep `AWS_Cost_Estimate.md` open for sanity checks. ### `infra/README.md` Module inventory, per-env differences, state backend (`metaflow-access-core-terraform-state` S3 + DynamoDB locks), and the **Pre-go-live checklist** (Healthie API key/webhook secret, CMS client ID/secret → app-secrets JSON). **Review points:** The checklist is your deployment gate — verify each item has a corresponding wiring point in `modules/ecs/main.tf` task-def secrets. ### `modules/vpc/` Isolated VPC per environment (no shared), private/public subnets, security groups. No NAT gateways by design (dev/staging intentionally can't reach the internet from private subnets — dev SQS is inert for this reason; noted in comments). **Review points:** Confirm endpoint prefixes and SG rules match what `endpoints/` routes to. ### `modules/database/` RDS Postgres + per-env retention (dev 1 / staging 7 / prod 14 days) + KMS encryption **except dev** (synthetic data only — documented as by-design). Grants scoped via VPC CIDRs, not 0.0.0.0/0. **Review points:** Dev-unencrypted is a deliberate cost/simplicity tradeoff for synthetic-only data; make sure nothing in the runbooks assumes dev parity with prod. ### `modules/kms/` Two keys: PHI field-encryption key and pseudonymization key (latter reserved — runtime service deferred). Field key carries an extra RDS-service-principal statement so RDS can use it for storage encryption; app task roles get access via required input `app_principal_arns`. **Review points:** Env roots pass the task-role ARN as a **statically computed string** (`${name}-fargate-task`) rather than reading module output — this breaks the kms↔ecs Terraform cycle. If you rename anything in `modules/ecs`, this silently goes stale. ### `modules/storage/` S3: AES256 (dev) / KMS-encrypted (staging+prod); versioning + lifecycle only on staging/prod. Object logging and bucket policies are **planned, not built**. **Review points:** `ClinicalDocument.storage_path` values will live here — check the path layout convention against consumers before documents start landing. ### `modules/queue/` SQS webhook queue + DLQ, KMS-encrypted messages, redrive policy. In dev the queues are unreachable by design (no SQS endpoint/NAT). **Review points:** A single-instance CMS polling worker exists (`manage.py cms_polling_worker`, deployed as `-worker` by `modules/ecs` when `enable_worker=true`; dev/local run it via the `cms-worker` service in `docker-compose.yml`) and is now the **only** consumer of `$submission-status` — all four CMS services are submit-only. The **webhook queue consumer still does not exist** — the SQS queue is plumbed but nothing receives from it yet; that remains the biggest infra↔code gap in the repo. ### `modules/endpoints/` VPC interface endpoints for AWS APIs only (S3, SQS, Secrets Manager, KMS). Non-AWS internet egress from private tasks (CMS ACCESS API, Healthie) goes through the optional egress-only NAT in `modules/vpc` (`enable_egress_nat`, on for staging/prod, off in dev). ALB SG allows 80+443; HTTPS listener with http_only flag. **Review points:** This module is what makes "private subnets only" actually work — if you add a new AWS service the app calls, it needs an endpoint here or dev/staging break. ### `modules/ecs/` Fargate: task def (secrets injected from Secrets Manager for `POSTGRES_PASSWORD`, `DJANGO_SECRET_KEY`; env vars incl. `PHI_KMS_KEY_ID`, `ALLOWED_HOSTS=*.elb.amazonaws.com[+public_domain]`, bucket name), service, autoscaling (prod only; staging is a fixed fleet, desired_count default 2). CloudWatch: app log group + three alarms behind `enable_alarms` — gunicorn 5xx bursts, webhook queue depth, and permanent Healthie GraphQL fetch failures (log metric filter on the stable `HEALTHIE_FETCH_PERMANENT_FAILURE` token; threshold 1 since one bad key breaks every subsequent webhook). All alarm actions feed `alarm_action_arns` from the env roots. **Review points:** The static task-role ARN assumption from kms/ lives here. `ALLOWED_HOSTS` via wildcard ELB domain keeps ALB reachable before Cutover — documented as tighten-later. ### `modules/alerting/` SNS topic (`-alerts`) + optional email subscriptions (`alert_emails`, each requires a one-time confirmation click). Wired into the ecs module's alarm actions by staging/prod env roots; dev has no alarms and no alerting module. **Review points:** No subscription exists until an operator passes `-var='alert_emails=[...]'` — until then alarms are console-visible only, which is easy to mistake for "no alerts needed". Consider adding a PagerDuty/Slack subscription once on-call exists. ### `environments/{dev,staging,prod}/main.tf + variables.tf` Compose the modules per environment; `caller_identity`/`partition` data sources feed ARNs; prod adds autoscaling/multi-AZ and (per docs) an immutable-tag requirement. **Review points:** Dev/staging/prod are structurally identical with deliberate deltas — diffing the three roots against each other is a fast way to spot drift. Validate all three: `terraform init -backend=false && terraform validate` in each dir; `terraform fmt -check -recursive infra`. --- ## 8. Scripts, data, tests, docs ### `scripts/` | File | When you run it | Review points | |------|-----------------|---------------| | `build_healthie_form.py` | After mapping changes; `--dry-run` first | Builds the live Healthie form from `docs/intake/Cognito_to_Healthie_Form_Mapping.yaml` in 3 phases (create → modules → options). Destructive against a real form — dry-run discipline matters. | | `demo_access_lifecycle.py` | Local demo of the full intake→CMS lifecycle | Exercises the same paths as production signals; good for manual walkthroughs. | | `test_access_mock.py` | Mock up (`docker compose up cms-mock`); `--base http://l.nso.me:3001` default | Standalone end-to-end against the mock: eligibility → alignment → reporting → quarterly timing → unalignment → post-unalign rejection + edge cases. No Django involved. | | `test_django_integration.py` | After migrations, mock up; also invoked by CI on main | Django-level integration — real management commands against the mock server, verifying state transitions. Sets env before `django.setup()`. | | `parse_cognito_form.py` / `generate_cognito_to_healthie_mapping.py` | One-off Cognito→Healthie mapping generation | Producers of the YAML mappings in `docs/intake/`; outputs are committed — re-run only when source data changes. | | `init_db.sh`, `xlsx_to_json/` | Ad-hoc local helpers | xlsx_to_json is a stdin→JSON converter with its own venv; nothing depends on it. | ### `sample_data/` Synthetic fixtures + example payloads (Cognito form data, synthetic CMS response/curation task/daily checkin/imaging/lab/PROM examples). All non-PHI by construction — safe to commit and use in tests. **Review points:** These feed compose's first-boot loaddata; if you add models used at startup, fixture coverage may need updating. ### `db/local.sqlite3` Local dev artifact (gitignored). Postgres is the real target; SQLite exists only because settings fall back to it for ad-hoc local runs. ### Test map (`access_core/*/tests/`) 274 tests total, all under `manage.py test access_core/`. Coverage concentrates on models + CMS client/workflow behavior: - **Strongest:** `cms_integration/tests/` (client, eligibility, alignment, unalignment, reporting, signals — 7 files), `services/tests/test_phi_crypto.py` (envelope round-trips, both backends), and `governance/tests/test_audit.py` (audit writer create/edit/delete/no-op, actor capture via middleware + context, submit_to_cms hook, fail-safe swallow, admin append-only). - **Weakest:** one smoke `test_models.py` per app (identity/intake/clinical/monitoring/curation/governance); intake also has `test_webhook.py`. Notable gaps: no assertion that DOB is actually ciphertext at rest; no suppression-rule enforcement tests (no code exists); no duplicate-submission guard tests for daily logs. ### `docs/` map — verify code against these while reviewing | Doc | What it claims to describe | Verify against | |-----|---------------------------|----------------| | `architecture/Technical_Assessment.md` | System overview + design decisions | Everything; it's the narrative spine | | `architecture/Database_Schema_Draft.md` | Logical schema, naming notes (Django tables = `_`) | All `models.py`; only DOB encrypted; audit = field names + SHA-256 hashes | | `architecture/Data_Flow_Diagram.md` | Phases A–G of data movement; implemented vs planned markers | Section 4/5 above (webhook pipeline, CMS signals) | | `architecture/AWS_Component_Diagram.md` | Per-env AWS topology | `infra/environments/*`; env table matches Terraform deltas | | `architecture/ACCESS_Traceability_Matrix.md` | Requirement → implementation traceability | Spot-check a few rows against actual code | | `architecture/Intake_Micro_App.md`, `Healthie_Forms_Vs_Cognito.md`, `Future_Integration_Roadmap.md`, `AWS_Cost_Estimate.md` | Intake app design, form strategy, roadmap, cost model | intake_app/, scripts/, infra/ respectively | | `security/PHI_Pseudonymization_Design.md` | Pseudonymization (design-level; runtime service deferred) | `phi_crypto.py`, governance packet models — note what is *not* built yet | | `security/Security_Access_Control_Plan.md` | RBAC model, offshore access gates | identity app + AccessGrant scopes | | `operations/CMS_Resilience_Config.md` | Retry/backoff/retry-queue behavior for CMS calls | `cms_client.py` retry logic | | `testing/Manual_Testing_Plan.md` | Manual test scripts (mock on port 3001) | scripts/, docker-compose.yml | | `regulatory/ACCESS_API_Notes.md`, `healthie/*.md`, `intake/Cognito_Field_Mapping.*` + `Cognito_to_Healthie_Form_Mapping.yaml` | CMS API notes, Healthie workflow, field mappings | consumers.py normalization, build_healthie_form.py | | `diagrams/` (16 SVGs) | Visual companions to the docs above | Open alongside each doc; regenerate if code diverges | --- ## Appendix A — Risk & follow-up checklist Work through these last, in rough priority order. All pointers verified against current HEAD (re-verified as items are resolved — see RESOLVED markers). | # | Area | Location | What to check / decide | |---|------|----------|------------------------| | 1 | ~~**Audit trail has no writer + is mutable**~~ — RESOLVED | `access_core/governance/audit.py` (writer), governance/admin.py (`AuditEventAdmin`) | Writer implemented: signals on a ~73-model PHI/decision registry, request-context actor capture, SHA-256 row hashes, fail-safe. Admin fully read-only, add/delete disabled; append-only enforced at Django layer (no direct DB access by policy). Actor FK now `auth.User` (was unused `identity.User`). | | 2 | **Unsanitized upload filename → path traversal** | intake_app/app/main.py:348 (TEMP_UPLOAD_DIR line 23) | `file.filename` joined raw into a path outside the app dir; no size limit enforced server-side. Highest-severity code bug found in review. | | 3 | **Synchronous CMS submit POST in signal handlers** | access_core/services/cms_integration_signals.py (`_trigger_cms_alignment`), reporting queue at :359 | No more multi-minute blocking: services are submit-only (one bounded POST, ~seconds worst case under `CMS_POST_MAX_RETRIES`) and every `$submission-status` check runs in `cms_polling_worker`. Residuals: the submit POST is still on the saving request's thread; per-process `_old_status_store` breaks under multiple gunicorn workers; errors swallowed (fail-open). Fully closing this needs an SQS consumer that takes submits off the request thread. | | 4 | **Fargate webhook queue has no consumer** | infra/modules/queue/main.tf vs modules/ecs/main.tf | Queue/DLQ/KMS plumbed, nothing drains it. Partially closed: the `cms_polling_worker` (ECS service in staging/prod, compose service in dev/local) runs `poll_cms_submissions` on a schedule — durable PENDING re-polling for every CMS submission. Remaining gap: an SQS consumer for webhook ingestion and moving submit POSTs off the request thread (see #3). | | 5 | ~~**Unpinned PHI-crypto dependency**~~ — RESOLVED | requirements.txt, intake_app/requirements.txt | All deps now exact `==` pins from the tested dev-venv versions (policy comment in each file: bump only after a green full-suite run). Deferred follow-up: hashed lockfile + `--require-hashes` in CI/Docker for transitive-dep supply-chain integrity. | | 6 | ~~**GraphQL mutation injection surface**~~ — RESOLVED | access_core/services/healthie.py (f-string-built mutations) | The f-string mutation builders (`create_form`/`add_module`/`update_module`/upload flow) had no in-repo call sites and were removed; the client is now a generic `query()` whose query strings live with their trusted callers. Re-introduce parameterized variables (not string interpolation) if form-building helpers return. | | 7 | **Webhook retry non-idempotency** | intake/views.py + consumers `_store_raw_ingestion` | Transport failures are now classified in `consumers._fetch_healthie_resource`: permanent Healthie 4xx → raw record marked `failed` + 200 ack (no retry loop); transient errors (connection/timeout/5xx) still propagate to the view's blanket except → 500 → Healthie retries with backoff, raw store resets for reprocessing — but downstream creates can still duplicate on retry (e.g., no unique constraint on daily logs: monitoring models). Residual gap: if Healthie exhausts retries on a persistent transient fault, the record stays `processing` forever — recoverable via `retry_failed_ingestions --status processing --older-than-minutes N` (admin action too), but nothing alerts you to it yet. | | 8 | ~~**PHI in CMS audit rows & synthetic MBI**~~ — RESOLVED | services/fhir_builder.py (gate), cms_integration/models.py (redaction) | `ALLOW_SYNTHETIC_MBI` gate: off by default → no real MBI means fail-closed (no PHI-derived pseudonym sent to any endpoint); Patient-level fields redacted from stored CMS request/response payloads at save time. On in dev/local/test (synthetic data, mock/sandbox). Staging env-overridable for sandbox-only demo runs. | | 9 | **Concurrent double-reporting** | services/cms_reporting.py | Idempotency = "exclude already-submitted", no DB lock; two concurrent runs can both report the same observation. | | 10 | **Admin PHI exposure** | identity/admin.py, monitoring/admin.py, curation/admin.py | Raw identifiers, symptom/vital JSON, document paths viewable/editable by any admin user with no readonly or permission scoping beyond Django's admin gate. | | 11 | **De-identification by convention** | governance packet models + docs/security/PHI_Pseudonymization_Design.md | `structured_data`/`signal_details` free-form JSON; only the DOB-at-rest encryption is enforced. Runtime pseudonymization service deferred — keep it that way on purpose or build it. | | 12 | **intake_app deployment story** | intake_app/run.sh, app/main.py (sessions line ~56, rate limiters :56–60) | Host-run single-process uvicorn with in-memory sessions; port 8000 collides with Django's published port; recaptcha loaded but unverified. Fine for dev — not a plan. | | 13 | **Suppression rules unenforced** | curation models (`suppression_required`, `min_cell_size`) | Model-only; no code enforces cell-size suppression before PublicReportExport. | | 14 | **Static task-role ARN coupling** | infra/env roots → modules/kms (app_principal_arns) ↔ modules/ecs role name | Computed string, not a module output — renaming the ECS role silently breaks KMS grants with no validate-time error. | | 15 | **CI gaps** | .github/workflows/ci.yml | Integration tests skipped on PRs; AWS account ID hardcoded in the workflow file. | --- *Guide generated from code at commit `27bffe3` (consistency-fix round). When you land new features, update Section 4/5 flow notes and the Appendix as you go — this doc is meant to stay a living review aid.*