Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 35 additions & 8 deletions src/lib/caring-contacts/db/postgres-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,33 @@ const PLAN_COLUMNS = `id, team_id, patient_id, referral_id, pathway_version_id,
discharge_at, created_at, completed_at, sending_preference, patient_name, patient_mobile_number,
patient_identifiers`;

/**
* What a LIST read of plans selects: `PLAN_COLUMNS` minus every patient column (#RZVMPD).
*
* `PLAN_COLUMNS` above is right for `readPlanRecord` and `selectPlanForUpdate`, whose callers
* genuinely need the patient detail -- `getEpisode` projects all three and `markRetentionCleared`
* clears them. `listPlans` needs none of them: `toPlanRecord` maps the plan, the ids, the dates,
* the outcome, the contacts and the attestations, and touches no patient column at all. So the
* caseload read was pulling every patient's name, mobile number and identifier list for the whole
* team, on every render, to throw all three away.
*
* NOTHING WAS RELEASED, AND THAT IS NOT THE POINT. `PlanRecord` excludes `patientDetail`
* structurally, so the guarantee held; what did not hold is that the data need never have entered
* the process. This narrows in the QUERY, not only in the mapping afterwards -- the standard
* `listPatientNames` below already sets.
*
* `patient_name` is dropped here too, not only the mobile number and the identifiers. Names have
* their own read with its own capability check and its own `patientNameDirectory` access-audit
* object type (Ruling 91), which exists so "who read patients' names, and when" is answerable.
* Pulling names inside a read audited as `plan` under-counts that trail.
*
* A SEPARATE CONSTANT RATHER THAN A NARROWED `PLAN_COLUMNS`. Narrowing the shared one in place
* would leave `getEpisode` projecting `undefined` for three patient fields, which no type checks
* and which is a worse defect than the one this fixes.
*/
const PLAN_LIST_COLUMNS = `id, team_id, patient_id, referral_id, pathway_version_id, state, version,
outcome, discharge_at, created_at, completed_at, sending_preference`;

const CONTACT_COLUMNS = `id, plan_id, team_id, sequence, state, version, cadence_label, calendar_day,
send_at, message_type, suppressed_reason`;

Expand Down Expand Up @@ -2329,7 +2356,7 @@ export function createPostgresRepository(
async listPlans(context: ReadContext) {
if (!mayReadOwnTeam(context, READ_ACTIONS.plan)) return [];
return runRead(context, async (connection) => {
const plans = await connection.query(`select ${PLAN_COLUMNS} from caring_contacts.plans order by id`);
const plans = await connection.query(`select ${PLAN_LIST_COLUMNS} from caring_contacts.plans order by id`);
const contacts = await connection.query(
`select ${CONTACT_COLUMNS} from caring_contacts.contacts order by plan_id, sequence`,
);
Expand Down Expand Up @@ -2371,13 +2398,13 @@ export function createPostgresRepository(
* the mobile number or the identifier list into the process at all -- its narrowing is in the
* query, not only in the mapping afterwards.
*
* Read that as a claim about this method, NOT about the page. `PLAN_COLUMNS` includes
* `patient_mobile_number` and `patient_identifiers`, and `listPlans` selects it verbatim, so the
* Patients directory still pulls both for its whole caseload on every render and discards them
* in `toPlanRecord`. What the projection changes is what is RELEASED, which is the substance:
* nothing outside this file can obtain those fields through it. Narrowing `listPlans`' own
* column list is a real privacy improvement on a hot path and is tracked separately, because it
* deserves its own review rather than riding along with a names read.
* That claim now holds of the PAGE as well, not only of this method (#RZVMPD, 2026-09-02).
* `listPlans` used to select `PLAN_COLUMNS` verbatim, so the Patients directory pulled every
* patient's name, mobile number and identifier list for its whole caseload on every render and
* discarded all three in `toPlanRecord`. It selects `PLAN_LIST_COLUMNS` instead, which carries
* no patient column at all -- so this projection is once again the only list-shaped read in
* this store that touches a name, which is what makes its `patientNameDirectory` access-audit
* entry a complete answer to "who read patients' names".
*/
async listPatientNames(context: ReadContext) {
if (!mayReadAllOwnTeam(context, PATIENT_NAME_READ_ACTIONS)) return [];
Expand Down
5 changes: 4 additions & 1 deletion src/lib/caring-contacts/schedule-view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@
//
// IT RELEASES NO PATIENT IDENTITY. Every field below comes from a `PlanRecord`, which carries none
// -- the patient is named only by the synthetic `patientId`. A screen that needs names reads them
// through `listPatientNames`, which is audited in its own right.
// through `listPatientNames`, which is audited in its own right. Since #RZVMPD that is true of the
// QUERY too and not only of the type: `listPlans` selects `PLAN_LIST_COLUMNS`, which names no
// patient column, so a caseload render no longer pulls names, mobile numbers and identifiers into
// the process merely to discard them.
//
// Pure and deterministic: no clock, no storage, no ambient time. The day being asked about is an
// argument.
Expand Down
42 changes: 40 additions & 2 deletions tests/caring-contacts-domain-isolation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,9 @@ describe("caring-contacts properties that only a source scan can hold", () => {

it("never fetches the first-contact reason for a list read", () => {
// `first_contact_reason` is free text a clinician wrote about one patient. It is deliberately
// absent from `PLAN_COLUMNS` -- the list `readPlanRecord` and `listPlans` select -- so rendering
// a caseload never pulls it into the process at all.
// absent from `PLAN_COLUMNS` -- the list `readPlanRecord` and `selectPlanForUpdate` select -- so
// rendering a caseload never pulls it into the process at all. (`listPlans` selects the narrower
// `PLAN_LIST_COLUMNS`, guarded separately below.)
//
// Nothing observable through the repository can hold that. `toPlanRecord` maps field by field,
// so adding the column to `PLAN_COLUMNS` fetches a clinical note for every plan in the team and
Expand Down Expand Up @@ -254,6 +255,43 @@ describe("caring-contacts properties that only a source scan can hold", () => {
expect(declaration?.[1]).not.toContain("preferred_name");
});

it("never fetches any patient column for a list read (#RZVMPD)", () => {
// The same argument as the two scans above, applied to the columns those two took as GIVEN.
// `first_contact_reason` and `preferred_name` were kept out of the list read because they are
// patient content -- while the list read went on selecting the patient's name, mobile number
// and identifier list for the whole caseload on every render, and `toPlanRecord` discarded all
// three. The narrowing was in the mapping, which releases nothing but fetches everything; it
// is now in the query.
//
// Nothing observable through the repository can hold this, for exactly the reason the other
// two scans exist: widening `PLAN_LIST_COLUMNS` back out changes no behaviour and leaves every
// behavioural test green. Only a scan of the query text can fail.
const source = postgresStore();
const listDeclaration = /const PLAN_LIST_COLUMNS = `([\s\S]*?)`;/.exec(source);

// Positive control: the constant was found and really is a plan column list. `state` is chosen
// deliberately -- it must survive any future narrowing, unlike the patient columns asserted
// absent below, so this control cannot be satisfied by an empty or truncated match.
expect(listDeclaration).not.toBeNull();
expect(listDeclaration?.[1]).toContain("state");
expect(listDeclaration?.[1]).toContain("discharge_at");

for (const column of [
"patient_name",
"patient_mobile_number",
"patient_identifiers",
"first_contact_reason",
"preferred_name",
]) {
expect(listDeclaration?.[1]).not.toContain(column);
}

// And the list read really does use it. Without this the constant could be narrowed correctly
// and never wired up, which is the failure mode that reads as a fix.
expect(source).toMatch(/select \$\{PLAN_LIST_COLUMNS\} from caring_contacts\.plans order by id/);
expect(source).not.toMatch(/select \$\{PLAN_COLUMNS\} from caring_contacts\.plans order by id/);
});

it("never derives the preferred name from the stored patient name", () => {
// Owner decision, 2026-08-26, and the one a later "simplification" is most likely to undo: the
// preferred name is ASKED FOR. A store that split `patient_name` would greet `Mr John Smith` as
Expand Down
107 changes: 107 additions & 0 deletions tests/caring-contacts-postgres-repository.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -617,3 +617,110 @@ describe("a retention clearance reaches every store of free text about the patie
}
});
});

// ---------------------------------------------------------------------------
// #RZVMPD — the caseload list read fetches no patient column (postgres only).
//
// `tests/caring-contacts-domain-isolation.test.ts` scans the column constants and their wiring,
// which is what catches a widened `PLAN_LIST_COLUMNS`. This is the other half: what the store
// actually puts on the wire. A scan cannot see a second, hand-written list query added later that
// never names the constant at all, and that is precisely the shape the original defect had --
// `listPlans` selecting a list it was not narrowed for.
//
// It is stated as a property of EVERY statement the call issues rather than of one expected
// string, so a future `listPlans` that fans out into more reads is held to the same rule instead
// of quietly escaping it.
// ---------------------------------------------------------------------------
describe("the caseload list read never puts a patient column on the wire (postgres only)", () => {
/** 2026-03-02 11:00 AWST, the instant the shared contract fixes its own clock to. */
const NOW = "2026-03-02T03:00:00.000Z";

const COORDINATOR: Actor = {
id: actorId("COLUMNS-ACTOR"),
teamId: teamId("COLUMNS-TEAM"),
roles: ["coordinator"],
};

it("issues no statement naming patient_name, patient_mobile_number or patient_identifiers", async () => {
const issued: string[] = [];
const recorded = poolAsSqlConnectionPool(pool);
const store = createPostgresRepository(
{
async withConnection(work) {
return recorded.withConnection((connection) =>
work({
async query(text, values) {
issued.push(text);
return connection.query(text, values);
},
}),
);
},
},
fixedClock(NOW),
);

const PLAN = planId("COLUMNS-PLAN");
const context = (key: string) => ({ actor: COORDINATOR, idempotencyKey: idempotencyKey(key) });

const referral = await store.createReferral(
{ referralId: referralId("COLUMNS-REFERRAL"), patientId: patientId("COLUMNS-PATIENT") },
context("columns-referral"),
);
expect(referral.ok).toBe(true);
const pathway = await store.savePathwayVersion(
{
version: {
id: pathwayVersionId("COLUMNS-PATHWAY"),
teamId: COORDINATOR.teamId,
state: "draft",
authorId: COORDINATOR.id,
approvals: [],
publishedAt: null,
retiredAt: null,
retirementUrgency: null,
snapshot: {
cadenceLabels: ["Day 3"],
messageTextByType: { standard: "Checking in.", first: "Welcome.", closing: "Last one." },
},
},
},
context("columns-pathway"),
);
expect(pathway.ok).toBe(true);

const created = await store.createPlan(
{
planId: PLAN,
assurances: PLAN_ASSURANCE_VALUES,
referralId: referralId("COLUMNS-REFERRAL"),
patientId: patientId("COLUMNS-PATIENT"),
pathwayVersionId: pathwayVersionId("COLUMNS-PATHWAY"),
dischargeAt: new Date("2026-03-02T02:00:00.000Z"),
sendingPreference: "morning",
patientDetail: {
patientName: "Rowan Delacroix",
patientMobileNumber: "+61 491 570 156",
patientIdentifiers: ["UR-00219384"],
culturalIdentity: null,
preferredName: "Rowan",
},
},
context("columns-create"),
);
if (!created.ok) throw new Error(`createPlan refused: ${created.reason}`);

// Only what the caseload render costs. Everything above is fixture.
issued.length = 0;
const plans = await store.listPlans({ actor: COORDINATOR });

// Positive control: the read really ran and really returned the plan, so an empty `issued`
// cannot pass this test by doing nothing.
expect(plans.map((record) => record.plan.id)).toEqual([PLAN]);
expect(issued.some((text) => text.includes("from caring_contacts.plans"))).toBe(true);

for (const column of ["patient_name", "patient_mobile_number", "patient_identifiers"]) {
expect(issued.filter((text) => text.includes(column))).toEqual([]);
}
});
});