mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-15 06:52:04 +00:00
Stop a delegate deferring somebody else's medication reminders
Found by an adversarial pass over the release candidate before it was tagged, and it is the reason the tag waited. `POST /api/medications/intake` is admitted for delegates because a caregiver standing next to the patient has to be able to say the tablet went down. It is an update, though, and the same body can carry two other things. Snoozing writes `snoozedUntil` on the OWNER's medication row, and the reminder cron skips that medication for as long as the stamp is in the future. The field takes any future instant. So somebody with write access could stop another person's medication reminders for years, and three things made it quiet: the owner is deliberately not notified of a delegated snooze, the activity feed renders the whole family as "marked a dose", and the only accurate trace was a line in the admin audit log. The second was flipping a dose the owner had already recorded — taken to skipped and back, which rewrites their compliance history and refunds inventory. The consent screen promises that changing what is already there stays with the owner. Both refuse for a delegate now and stay open to the owner, who is the only person either was ever for. A refused attempt is filed under the owner and gets its own sentence in the activity panel rather than borrowing the one for marking a dose, because folding them together is what made the gap dangerous. Two more from the same pass: The consent notice named allergies among the things a delegate may add. That left the admitted set before this release shipped, and it said so in six languages on the one screen where somebody decides who may write into their health record. It also omitted half of what IS admitted. The copy now matches the frozen list, and the test asserts the withdrawal rather than only the presence of the rest. The banner told a WRITE delegate nothing at all — the mode line rendered only in the read case, guarded by a comment stating that write grants did not exist yet. This is the release that made them exist. It matters that the banner say it, because the rule that a refused control is absent rather than disabled rests on the banner naming the mode globally. And a third instance of the substitution this release already fixed twice: the BMI chart divided the owner's weights by the height of whoever was looking. Inside a shared record it draws nothing now rather than a number belonging to two different people.
This commit is contained in:
1 parent
7ff420477c
commit
69343c8820
15 files changed
+260
-32
No files matched your search
@@ -283,6 +283,61 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
return apiError("Intake event not found", 404);
|
||||
}
|
||||
|
||||
// v1.36.1 — this route is admitted for delegates because marking a dose is
|
||||
// the thing a person standing next to the patient needs to do. It is an
|
||||
// UPDATE, though, and two of the transitions it can express are not that.
|
||||
//
|
||||
// Snoozing writes `snoozedUntil` on the OWNER's medication row, and the
|
||||
// reminder cron skips a medication for as long as that stamp is in the
|
||||
// future. The field is unbounded, the owner is deliberately not notified of
|
||||
// a delegated snooze (there is nothing useful to say about a thirty-minute
|
||||
// deferral), and the activity feed renders the whole family as "marked a
|
||||
// dose". So a delegate could switch the owner's medication reminders off for
|
||||
// years, and every surface that exists to make delegation visible would
|
||||
// describe it as marking a dose. In an adherence product that is the worst
|
||||
// thing on this route.
|
||||
//
|
||||
// Flipping an already-resolved event is the other one: taken to skipped and
|
||||
// back rewrites the owner's compliance history and refunds inventory. The
|
||||
// shipped promise, on the consent screen and in the release notes, is that
|
||||
// editing what is already there stays with the owner. Marking a dose that is
|
||||
// still open is a contribution; changing a decision the owner already
|
||||
// recorded is an edit.
|
||||
//
|
||||
// Both refuse for a delegate and stay open to the owner, who is the only
|
||||
// person the snooze and the correction were ever for.
|
||||
if (actor.id !== user.id) {
|
||||
const alreadyResolved = existing.takenAt !== null || existing.skipped;
|
||||
const refusal =
|
||||
status === "snoozed"
|
||||
? "snooze"
|
||||
: alreadyResolved
|
||||
? "resolved_event"
|
||||
: null;
|
||||
if (refusal) {
|
||||
annotate({
|
||||
action: { name: "medications.intake.update.refused" },
|
||||
meta: { reason: refusal },
|
||||
});
|
||||
// Filed under the OWNER. `auditLog` stamps the actor itself from the
|
||||
// request's acting context, which is what puts this on the trail the
|
||||
// owner's activity panel reads — a refused attempt to change their
|
||||
// record is worth as much to them as a successful one.
|
||||
await auditLog("medications.intake.update.refused", {
|
||||
userId: user.id,
|
||||
ipAddress: getClientIp(request),
|
||||
details: { intakeId, status, reason: refusal },
|
||||
});
|
||||
return apiError(
|
||||
refusal === "snooze"
|
||||
? "Snoozing a reminder is not part of shared access"
|
||||
: "Changing a dose that is already recorded is not part of shared access",
|
||||
403,
|
||||
{ errorCode: "sharing.not_permitted" },
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// v1.8.5 — resolve + server-validate the optional injection site for a
|
||||
// "taken" toggle. A site outside the medication's effective allowed
|
||||
// set is a hard 422; non-injection / tracking-off / non-taken drops it.
|
||||
@@ -557,9 +612,8 @@ export const POST = apiHandler(async (request: NextRequest) => {
|
||||
}
|
||||
|
||||
// v1.36.x — "somebody else marked your dose". The helper refuses on self and
|
||||
// on a snooze, resolves both names itself, and never throws; awaited because
|
||||
// the latency it adds lands on the caregiver's request and never on the
|
||||
// owner's own hot path.
|
||||
// on a snooze, resolves both names itself, and never throws.
|
||||
//
|
||||
// Fire and forget, like every neighbour above. The dispatcher awaits each
|
||||
// channel in the cascade, so an unreachable one would have added its whole
|
||||
// timeout to the caregiver's request — the person standing next to the
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
import { useQuery } from "@tanstack/react-query";
|
||||
import { useAuth } from "@/hooks/use-auth";
|
||||
import { useRecordCapabilities } from "@/hooks/use-record-capabilities";
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { apiGet } from "@/lib/api/api-fetch";
|
||||
import {
|
||||
@@ -684,8 +685,23 @@ export function HealthChart({
|
||||
// every chart card.
|
||||
const viewportWidth = useViewportWidth();
|
||||
|
||||
// v1.36.1 — `useAuth()` answers about the person at the keyboard, and under a
|
||||
// switch that is the delegate. Dividing the OWNER's weights by the
|
||||
// DELEGATE's height squared produces a plausible-looking BMI that is simply
|
||||
// somebody else's, shaded by fixed category bands, on a page that names the
|
||||
// owner. The same substitution was fixed for the reference bands in this
|
||||
// release; this is the same mistake one component over, and a wrong health
|
||||
// figure is worse than an absent one.
|
||||
//
|
||||
// Inside a shared record the chart therefore draws no BMI at all rather than
|
||||
// a wrong one. Publishing the owner's height to every client that can read
|
||||
// their weights is a bigger decision than this fix, and it belongs with the
|
||||
// per-module scope work rather than here.
|
||||
const { inSharedRecord } = useRecordCapabilities();
|
||||
const bmiDivisor =
|
||||
valueMode === "bmi" && user?.heightCm ? (user.heightCm / 100) ** 2 : null;
|
||||
valueMode === "bmi" && !inSharedRecord && user?.heightCm
|
||||
? (user.heightCm / 100) ** 2
|
||||
: null;
|
||||
|
||||
// v1.4.28 FB-D2 (R1.2 H0) — derive a bounded date window from the
|
||||
// active range selector so the chart fetches only what it will
|
||||
|
||||
@@ -75,13 +75,19 @@ export function SharedRecordBanner() {
|
||||
<span className="text-foreground font-medium">
|
||||
{t("recordSharing.banner.viewing", { name })}
|
||||
</span>{" "}
|
||||
{/* Resolved server-side. `canWrite` is false for every grant in v1, and
|
||||
the day it is not, this line stops claiming otherwise on its own. */}
|
||||
{!active.canWrite && (
|
||||
<span className="text-muted-foreground">
|
||||
{t("recordSharing.banner.readOnly")}
|
||||
</span>
|
||||
)}
|
||||
{/* Resolved server-side, and it says something at BOTH levels now.
|
||||
v1.36.1 is the release that made `canWrite` true, and until this
|
||||
line changed the write case rendered nothing at all: a delegate who
|
||||
could add to somebody's health record was told only whose record it
|
||||
was. That matters more than the read case, not less, and the rest of
|
||||
the product leans on it — a control the record refuses is absent
|
||||
rather than disabled, on the stated grounds that the banner names
|
||||
the mode globally. It has to actually do that. */}
|
||||
<span className="text-muted-foreground">
|
||||
{active.canWrite
|
||||
? t("recordSharing.banner.canAdd")
|
||||
: t("recordSharing.banner.readOnly")}
|
||||
</span>
|
||||
</p>
|
||||
<button
|
||||
type="button"
|
||||
|
||||
@@ -89,7 +89,16 @@ describe("offering a level", () => {
|
||||
const html = render(<GrantInviteCard />);
|
||||
// The thing a person reading "write access" gets wrong.
|
||||
expect(html).toContain("cannot edit or delete anything");
|
||||
expect(html).toContain("marking a dose as taken");
|
||||
expect(html).toContain("marking a dose taken or skipped");
|
||||
// v1.36.1 — the copy named allergies, which left the admitted set before
|
||||
// this shipped, and it did so on the one screen where somebody decides who
|
||||
// may write into their health record. Assert the withdrawal too, not only
|
||||
// the presence of the rest: a consent notice that over-promises is worse
|
||||
// than one that is merely incomplete.
|
||||
expect(html).not.toContain("allergies");
|
||||
// Deferring a reminder is the capability the same request could reach and
|
||||
// delegation does not cover; the notice has to say so.
|
||||
expect(html).toContain("cannot defer one of your reminders");
|
||||
// And the narrow option still says it is narrow.
|
||||
expect(html).toContain("They cannot add or change anything.");
|
||||
});
|
||||
|
||||
@@ -22,9 +22,13 @@ import {
|
||||
* ## The two answers, and why there are two
|
||||
*
|
||||
* A grant at WRITE admits a short, closed list of verbs: entering readings,
|
||||
* lab results, allergies, family history, an illness entry, a biomarker, a
|
||||
* tracked value, a side effect, a medication, and marking a dose taken. It
|
||||
* admits nothing else. Editing, deleting, restoring, purging, importing and
|
||||
* lab results, an illness entry, a biomarker, a tracked value, a side effect, a
|
||||
* medication, and marking a dose taken or skipped. It admits nothing else.
|
||||
* Allergies and family history were on this list and left it before v1.36.1
|
||||
* shipped — the argument for them held, but the only surface that posts to
|
||||
* either lives under `/settings`, which a switch closes, so the permission had
|
||||
* no reachable caller. `delegable-surface-guard.test.ts` carries that argument
|
||||
* in full and is the list that binds; this paragraph has to agree with it. Editing, deleting, restoring, purging, importing and
|
||||
* bulk-acting stay with the owner, including on an entry the delegate added
|
||||
* themselves a minute ago, and so does every create the list does not name
|
||||
* (a mood entry, a screener, a cycle day). Two booleans carry that:
|
||||
|
||||
@@ -74,10 +74,14 @@ export interface NotifyDelegatedIntakeInput {
|
||||
* Tell the owner that somebody else marked one of their doses.
|
||||
*
|
||||
* Never throws: a notification that fails is not a reason to fail the write
|
||||
* that produced it. Await it — the call sites reach this only on a delegated
|
||||
* write, so the added latency lands on the caregiver's request and never on
|
||||
* the owner's own hot path, and awaiting is what makes "one row in
|
||||
* `push_attempts`" a thing a test can assert rather than a thing it can race.
|
||||
* that produced it. The call sites do NOT await it, and this line used to say
|
||||
* the opposite. The dispatcher awaits each channel of the cascade in turn, so
|
||||
* one unreachable channel added its whole timeout to the request of the person
|
||||
* standing next to the patient waiting for a dose to register — which is the
|
||||
* one request in this product that should never be slow. The owner learning a
|
||||
* moment later is the better trade. The integration test polls the
|
||||
* `push_attempts` ledger rather than relying on the await, so nothing about
|
||||
* the proof depends on the caller's choice here.
|
||||
*/
|
||||
export async function notifyDelegatedIntake(
|
||||
input: NotifyDelegatedIntakeInput,
|
||||
|
||||
@@ -64,6 +64,16 @@ export function recordActivityVerbLine(
|
||||
case "medication.intake":
|
||||
case "medications.intake.update":
|
||||
return t("recordSharing.activityVerb.medicationIntake", { name });
|
||||
// The refused half of the same route, and it earns a line of its own for
|
||||
// the reason the whole panel exists. Deferring a reminder and rewriting a
|
||||
// dose the owner already recorded are the two things that request can
|
||||
// express and delegation does not cover. Folding them into "marked a dose"
|
||||
// is what made the gap dangerous in the first place: the owner would have
|
||||
// seen the sentence for a contribution against an attempt to switch their
|
||||
// reminders off. An attempt that was refused is still something the person
|
||||
// whose record it is should be able to see.
|
||||
case "medications.intake.update.refused":
|
||||
return t("recordSharing.activityVerb.medicationIntakeRefused", { name });
|
||||
// A batch post is still readings arriving; the owner does not care that the
|
||||
// client sent them together.
|
||||
case "measurement.create.batch":
|
||||
|
||||
Reference in new issue
Block a user