Conversation
`facilityIds` is a staff coverage grant - which sites a worker is rostered at - and it was being applied to a patient-scoped token reading their own chart too. `MedicationDispense` at the practice's second site was invisible to the portal even though the compartment (patientId equality) already matched: the facility clause, written for a staff reader, was ANDed on ahead of it and excluded the row regardless of whose chart it was. The narrowing is now skipped when the caller is compartment-pinned and the row is being read through that same compartment column - i.e. a patient reading their own record - and unchanged for a staff caller (no compartment) and for an 'open' compartment row that is not the caller's own chart (the facility list itself, say). The fix lives once in the shared `inScope` (memory) / `scoped` (Prisma) narrowing every `CollectionSpec` goes through, so it applies uniformly to every facility-scoped, chart-compartmented spec: StockPosting, ChargeItem, Patient, Appointment and Encounter. Added a unit test on the Prisma port (`appointmentSpec`, chosen because it reproduces the mechanism rather than one resource) and an HTTP-level regression reproducing #329's own MedicationDispense measurement. Mutation checked by forcing `ownChart` to false in each port in turn - both new tests failed as expected, 404 and one row rather than two - then reverted.
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
ankit-yc
left a comment
ankit-yc
left a comment
There was a problem hiding this comment.
Reproduced the mechanism independently rather than trusting the description: read both diff hunks in memory.ts and prisma.ts side by side, and the two ownChart definitions agree token-for-token on the conditions (compartment !== undefined && spec.compartment !== 'open').
Verified the fix cannot leak cross-patient data: skipping facility narrowing only happens for ownChart reads, and the compartment equality check that follows (readColumn(row, spec.compartment.column) === compartment) still runs unconditionally afterward - so a staff-facility-grant bypass only ever applies to rows already proven to belong to the caller's own chart. The open-compartment exclusion is correct too: an open-compartment row (e.g. a facility row itself) isn't "the caller's own chart", so ordinary staff narrowing still gates it.
Both new tests are real regressions, not tautologies: the HTTP-level MedicationDispense case reproduces #329's own measurement (byId 200, search returns both postings sorted), and the Prisma-port appointmentSpec test carries its own negative control (a same-facility, different-patient row that must stay excluded) - so a compartment that failed open would not pass it for the wrong reason. Mutation check (forcing ownChart to false) is described and the failure mode (404 / one row instead of two) is exactly what the bug looked like in production.
Fixed once at the shared narrowing both ports go through, so it covers StockPosting/ChargeItem/Patient/Appointment/Encounter together rather than only MedicationDispense - matches this repo's own "fix the class, not the instance" convention.
CI: this PR's own checks are green except sonar / Sonar scan (api) / CI Required, which are failing repo-wide right now on a SonarCloud 403 (SONAR_TOKEN_API rejected - filed as #489, an org-admin token rotation, not a defect in this diff).
2 participants
Summary
facilityIdsis a staff coverage grant and was being applied to a patient-scoped (compartment-pinned) token reading their own chart, on top of the compartment's own patientId equality.MedicationDispenserecorded at the practice's second facility was invisible (404 / missing from search), even though the compartment already matched - the facility clause excluded it regardless.CollectionSpecgoes through (inScopein the memory port,scopedin the Prisma port), so it applies uniformly to every facility-scoped, chart-compartmented spec:StockPosting,ChargeItem,Patient,Appointment,Encounter. Unchanged for a staff caller (no compartment) and for anopen-compartment row that isn't the caller's own chart.Test plan
pnpm --filter api lintpnpm --filter api type-checkpnpm --filter api test(4172 passed, 14 skipped)pnpm run format:checkpnpm verifyappointmentSpec) and an HTTP-level regression reproducing Should a patient see their own dispense recorded at another site of the practice? #329's ownMedicationDispensemeasurementownCharttofalsein each port in turn, both new tests failed as expected (404 / one row instead of two), reverted