Skip to main content

Denied record reads answer as not-found (SEC-011)

A record-scoped read that the caller may not see now answers exactly as a genuinely unknown id does — same status, same message. Before this, these paths looked the record up first and authorized second, so an unknown id got 404 while a denied one got 403.

That difference is an existence oracle. The opaque id alone confirmed that the record is real and that someone the caller may not see holds it — no content ever had to be returned. For a patient lookup that is itself the disclosure: "does this person have a record at this hospital" is frequently the sensitive question, and clinical ids travel in URLs, referral letters and support tickets where an outsider can collect them.

  • Owned by: PatientAccessGuard (core/.../engines/clinical/service/PatientAccessGuard.java) — authorizeOrNotFound, authorizeOrElseThrow, isAuthorized, and the original authorize. The demographic engine's PersonAccessGuard carries the same authorizeOrNotFound (M13-008B), built on its isVisible rather than on a 403-throwing overload — it has never had one, which is why the SEC-011 ArchUnit rule rations PatientAccessGuard.authorize alone.
  • Applies to: the record-scoped reads and writes of the clinical engine listed below. The demographic engine already answered this way — patient reads and the confidentiality flag (SEC-015) — so the two engines now agree.
  • Predecessor: SEC-006 translated denial by hand inside EncounterLocationServiceImpl. SEC-011 moved the translation into the guard, because a per-service catch-and-rethrow does not scale and drifts: three services had already been left behind.

What an API consumer observes​

A 404 is no longer evidence that a record does not exist. It means "there is nothing here for you" — which covers both an id that names nothing and an id that names something you may not see. The two are indistinguishable on purpose: same errorCode, same errorMessage, same apiPath, and nothing in the body varies but the errorTime clock reading.

:::warning Every response field, not just the status The guarantee is that a denial and a miss are byte-identical, so any field added to the error response has to be identical on both paths too — a new field that is populated on one and null on the other rebuilds the oracle somewhere nobody is looking.

This currently holds by construction rather than by design. Both paths raise ResourceNotFoundException (or InvalidRequestException for the 400 paths), and GlobalExceptionHandler passes null for businessErrorCode on those handlers, so the error-code field M10B-005A introduced cannot separate them. That is a property of how those handlers happen to be written, and nothing enforces it. Re-check it whenever the error contract changes. :::

Record-scoped paths — addressed by a record id​

EndpointUnknown idDenied
GET /api/v1/clinical/visits/{visitId}, POST …/{visitId}/end404 "Visit not found"identical
GET·PUT·DELETE /api/v1/clinical/encounters/{encounterId}404 "Encounter not found"identical
GET·DELETE /api/v1/clinical/observations/{observationId}404 "Observation not found"identical
GET·PUT·DELETE /api/v1/clinical/diagnoses/{diagnosisId}404 "Diagnosis not found"identical
GET·PUT·DELETE /api/v1/clinical/conditions/{conditionId}404 "Condition not found"identical
GET·PUT·DELETE /api/v1/clinical/allergies/{allergyId}404 "Allergy not found"identical
GET·PUT·DELETE /api/v1/clinical/notes/{clinicalNoteId}404 "Clinical note not found"identical
GET·PUT·DELETE /api/v1/clinical/care-plans/{carePlanId}404 "Care plan not found"identical
GET·DELETE /api/v1/clinical/encounter-locations/{encounterLocationId}404 "Encounter-location not found"identical
GET /api/v1/clinical/encounters/{encounterId}/locations404 "Encounter-location not found"identical
GET /api/v1/clinical/patients/{patientId}/summary-panel404 "Patient not found"identical
GET /api/v1/demographic/person-merges/candidates?personId= (M13-008B)404 "Person not found"identical — and so is a person in another tenant

Paths whose miss answer is not a 404​

Two endpoints refuse an unknown id some other way, and denial matches that answer rather than a 404 — see Match the path's own miss answer.

EndpointUnknown idDenied
POST /api/v1/clinical/encounter-locations400 "Unknown encounter: {id}"identical
GET /api/v1/clinical/patients/{patientId}/current-location204 No Contentidentical

What still answers 403 — and why that is not a leak​

create* and patient-scoped list endpoints keep the plain 403, because they are subject-scoped: the caller supplied the patient id in the request body or query, and no record is resolved before the decision. A refusal confirms nothing the caller did not already assert, so there is no existence to protect — and answering "not found" would only mislabel a permission problem as a missing patient, sending an integrator to hunt a data bug that does not exist.

EndpointDenied
POST /api/v1/clinical/visits, /encounters, /observations, /diagnoses, /conditions, /allergies, /notes, /care-plans403
GET /api/v1/clinical/visits/page, /encounters/page, /observations/page (paged, patientId= required)403
GET /api/v1/clinical/patients/{patientId}/clinical-summary403

List reads filter rather than refuse. An unpaged search drops records the caller may not see, so a 200 with fewer rows than expected is normal and is not an error — an unreadable record is already indistinguishable from one that does not exist, so there was never a status to reconcile there.

What integrators must change​

  • Never treat 404 as proof of non-existence. Do not create a replacement record, do not delete a local reference, do not mark a synced row as removed upstream. A duplicate patient or visit created on the strength of a 404 is exactly the harm the platform's identity model exists to prevent.
  • Do not try to tell the two apart by diffing bodies, headers or messages. They are identical by construction, and any difference you find is a defect worth reporting, not a signal to rely on.
  • On the record-scoped paths above, a 403 is no longer the patient gate — it is the endpoint permission (@RequiresAccess) refusing before any record is touched, and it says nothing about whether the record exists.
  • Retry as a different user, not with a different id. The usual cause of an unexpected 404 is missing visibility, consent, an access grant or a provider scope for that patient — see Identity & access.

Three things that make this harder than it looks​

Closing the oracle one way re-opens it the other​

listForEncounter used to answer 200 [] for an unknown encounter. Once denial started answering 404, the absence of a 404 became the tell: an empty list meant "this encounter is real, you just cannot see its movements". So the endpoint had to start answering 404 for an unknown encounter too.

Adopting this convention means sweeping a path's not-found answers as well as its denial answers. Half the change is worse than none, because it converts one oracle into its mirror image.

Match the path's own miss answer, not a global 404​

createEncounterLocation already answered 400 "Unknown encounter: {id}" for an encounter that does not exist, so denial there is that same 400 — not a 404. Forcing a uniform 404 on every path would have made the denial distinguishable again, in reverse: a 404 from an endpoint that never otherwise produces one is as loud a signal as a 403 was.

The rule is therefore "answer what this path already answers", not "answer 404".

Non-disclosure loses to patient safety​

endActivePlacementsForVisit deliberately keeps the guard's 403. It is the discharge path: folding a denial into a quiet "0 placements ended" would report a discharge that did not free the bed, and leave a discharged patient's bed marked occupied on the ward's board. A silent wrong answer about occupancy is worse than a loud refusal — and there is nothing to protect there anyway, since the method is not exposed as an endpoint and no attacker can probe it with ids.

endVisit is on that same discharge path and shows where the line actually falls. It does answer 404 on denial, because it is reachable by visit id and so is probeable — but it throws rather than degrading into a quiet "already ended", which would report a discharge that never happened. So the two questions are separate: what a refusal says is chosen for non-disclosure, whether it refuses at all is chosen for safety. Throwing is free of half-written state here because the refusal precedes every mutation.

Exemptions like these are listed with their justification rather than silently skipped. If a new one is needed, say so in the code comment and here.

A record that cannot name its patient is still a 403​

A null patient id fails closed as 403, even inside authorizeOrNotFound and authorizeOrElseThrow, and returns false from isAuthorized. SEC-003's rule outranks SEC-011's inside the guard.

That is deliberate. A null id is not a denial about a real record — it is a record that cannot say whose it is, which is a defect on our side and is never attacker-supplied (the caller passes an id it read off the entity it just loaded). Translating it into a 404 would disguise our own bug as an ordinary miss and hide it behind a plausible answer, which is precisely how the original SEC-003 bypass looked: a guard that returned silently on a null id, so any caller that failed to supply one skipped authorization entirely.

Adding a new record-scoped read​

Pick the method by the answer the path already gives for a genuinely unknown id:

That path's answer for an unknown idUseNote
404authorizeOrNotFound(patientId, mode, THIS_PATHS_NOT_FOUND)Pass that path's own message constant
Anything else that fails (400, 409, …)authorizeOrElseThrow(patientId, mode, denied -> new InvalidRequestException(…))Build the path's own miss exception
An empty value (204, empty list, empty page, empty aggregate)isAuthorized(patientId, mode)Return the empty value, do not throw
Nothing — the caller named the patient itselfauthorize(patientId, mode)Opt-in only; see below

And then:

  1. Never a generic message. A denial that says "not found" in different words is the same oracle with an extra step. If the constant lives in another class and cannot be shared, duplicate the string and say in a comment that it must stay byte-identical — PatientSummaryConceptServiceImpl does this, because panelFor calls getPatient on the very next line and the two answers have to agree word for word.
  2. Sweep the miss paths too. Before merging, ask what the endpoint answers for an id that names nothing, and make the denial match it — including the 200 [] case, which needs the not-found side fixed rather than the denial side.
  3. Guard before the read when the path names the patient's owner. listForEncounter authorizes off the encounter in the URL without reading a single placement. When the record must be loaded before its owner is known — the usual case — authorize between the load and the mapping, so nothing derived from the row escapes.
  4. ⚠ An empty-value assertion proves nothing on its own. A removed guard produces the same empty list a denied caller gets, so a test that only asserts "empty" passes with no security at all. Every isAuthorized path must also assert that the underlying read never happened (verify(repository, never())…), which is the pattern EncounterLocationServiceImplTest uses.
  5. Assert the exact message, not just the status. Byte-identical is the acceptance criterion, and a test that only checks 404 will not notice a denial that says something subtly different.

Plain authorize is now opt-in​

authorize still exists and still throws 403 — but using it on a new call site is a deliberate choice, not the default. PatientAccessDenialConventionArchitectureTest holds the named allow-list of callers permitted to keep it, so adding one means adding an entry with its justification in the same commit: a security review with a diff, rather than an omission nobody sees. The list names callers, not the guard method, because exempting the method answers "may this be a 403" once and nobody re-asks.

It is checked in both directions — an entry that no longer calls plain authorize, or whose denials are now translated, fails the build too. An allow-list that can rot is an allow-list nobody trusts.

Known exclusion. The encounter-provider participation endpoints (POST·GET /api/v1/clinical/encounters/{encounterId}/providers, DELETE /api/v1/clinical/encounter-providers/{encounterProviderId}) still answer 403, and are allow-listed with a reason rather than converted: that service currently fails open when the encounter cannot be resolved, and tidying how its denials are reported before fixing a guard that can be skipped entirely would flatter it. Both change together, under their own ticket.