mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 12:32:43 +00:00
feat(02-11): secure guardian notification delivery
- Remove record-mutating affordances from Guardian channel payloads\n- Reject managed Telegram callbacks and verify channel attribution
This commit is contained in:
1 parent
accb98e223
commit
60b4845fea
8 files changed
+276
-41
No files matched your search
@@ -6,23 +6,15 @@ describe("managed Guardian notification payload safety", () => {
|
||||
it("recognises only cross-account record delivery as managed Guardian delivery", () => {
|
||||
expect(
|
||||
isManagedGuardianDelivery({
|
||||
eventType: "MEDICATION_REMINDER",
|
||||
userId: "record-1",
|
||||
recordUserId: "record-1",
|
||||
recipientUserId: "guardian-1",
|
||||
title: "t",
|
||||
message: "m",
|
||||
}),
|
||||
).toBe(true);
|
||||
|
||||
expect(
|
||||
isManagedGuardianDelivery({
|
||||
eventType: "MEDICATION_REMINDER",
|
||||
userId: "user-1",
|
||||
recordUserId: "user-1",
|
||||
recipientUserId: "user-1",
|
||||
title: "t",
|
||||
message: "m",
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
@@ -165,7 +165,7 @@ describe("web-push reminder push — stable tag + badge", () => {
|
||||
expect(body.tag).toBe("REMINDER");
|
||||
});
|
||||
|
||||
it("uses an ordinary landing route without a persistent prompt for a managed Guardian", async () => {
|
||||
it("uses an ordinary landing route without actions for a managed Guardian", async () => {
|
||||
findManyMock.mockResolvedValue([SUB]);
|
||||
vi.spyOn(global, "fetch").mockResolvedValue(
|
||||
new Response(null, { status: 201 }),
|
||||
@@ -186,7 +186,7 @@ describe("web-push reminder push — stable tag + badge", () => {
|
||||
generateRequestDetailsMock.mock.calls[0][1] as string,
|
||||
);
|
||||
expect(body.url).toBe("/");
|
||||
expect(body.requireInteraction).toBe(false);
|
||||
expect(body.actions).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -0,0 +1,16 @@
|
||||
import type { NotificationPayload } from "@/lib/notifications/types";
|
||||
|
||||
/**
|
||||
* A cross-account notification can only reach a Guardian after the
|
||||
* dispatcher has revalidated the marked record and its active MANAGE grant.
|
||||
* Senders use this narrow invariant to remove record-mutating affordances.
|
||||
*/
|
||||
export function isManagedGuardianDelivery(
|
||||
payload: Pick<NotificationPayload, "recordUserId" | "recipientUserId">,
|
||||
): boolean {
|
||||
return (
|
||||
payload.recordUserId !== undefined &&
|
||||
payload.recipientUserId !== undefined &&
|
||||
payload.recordUserId !== payload.recipientUserId
|
||||
);
|
||||
}
|
||||
@@ -620,7 +620,6 @@ describe("sendViaApns — dispatcher fan-out", () => {
|
||||
title: "t",
|
||||
message: "m",
|
||||
eventType: "MEDICATION_REMINDER",
|
||||
userId: "record-1",
|
||||
recordUserId: "record-1",
|
||||
recipientUserId: "guardian-1",
|
||||
metadata: {
|
||||
|
||||
@@ -31,6 +31,7 @@ import { getEvent } from "@/lib/logging/context";
|
||||
import type { SendOutcome } from "@/lib/notifications/retry-policy";
|
||||
import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-attempt-record";
|
||||
import { plainPushText } from "@/lib/notifications/strip-emoji";
|
||||
import { isManagedGuardianDelivery } from "@/lib/notifications/managed-delivery";
|
||||
|
||||
export interface ApnsPayload {
|
||||
/** APNs `aps.alert.title` + `aps.alert.body`. */
|
||||
@@ -628,10 +629,7 @@ export async function sendViaApns(
|
||||
process.env.APNS_CRITICAL_ENTITLEMENT === "true";
|
||||
|
||||
let result: Awaited<ReturnType<typeof sendApnsPush>> | null = null;
|
||||
const managedDelivery =
|
||||
payload.recordUserId !== undefined &&
|
||||
payload.recipientUserId !== undefined &&
|
||||
payload.recordUserId !== payload.recipientUserId;
|
||||
const managedDelivery = isManagedGuardianDelivery(payload);
|
||||
// Discreet mode (cycle privacy): the lock-screen-visible routing
|
||||
// metadata must not carry the event name. Collapse the category /
|
||||
// threadId / collapseId and the data eventType to a generic value so
|
||||
@@ -671,10 +669,16 @@ export async function sendViaApns(
|
||||
// In discreet mode also drop cycle-semantic metadata (e.g. `phase`)
|
||||
// so a future cycle push can't name a cycle concept on the wire even
|
||||
// though the allowlist permits `phase` (QA LOW).
|
||||
// A Guardian may read a managed record but must never receive
|
||||
// an iOS action identifier or deep link that can mutate it.
|
||||
// Keep the ordinary event envelope so opening the notification
|
||||
// follows the normal authorised record-selection flow.
|
||||
...pickIosMetadata(
|
||||
payload.discreet
|
||||
? stripCycleMetadata(payload.metadata)
|
||||
: payload.metadata,
|
||||
managedDelivery
|
||||
? undefined
|
||||
: payload.discreet
|
||||
? stripCycleMetadata(payload.metadata)
|
||||
: payload.metadata,
|
||||
),
|
||||
},
|
||||
threadId: routingEvent,
|
||||
|
||||
@@ -9,6 +9,7 @@ import { recordPushAttemptForPayload } from "@/lib/notifications/senders/push-at
|
||||
import { isPublicUrl } from "@/lib/validations/notifications";
|
||||
import { safeFetch } from "@/lib/safe-fetch";
|
||||
import { plainPushText } from "@/lib/notifications/strip-emoji";
|
||||
import { isManagedGuardianDelivery } from "@/lib/notifications/managed-delivery";
|
||||
|
||||
/**
|
||||
* Send Web Push notification to all subscribed devices of a user.
|
||||
@@ -30,6 +31,7 @@ export async function sendViaWebPush(
|
||||
payload: NotificationPayload,
|
||||
): Promise<SendOutcome> {
|
||||
try {
|
||||
const managedDelivery = isManagedGuardianDelivery(payload);
|
||||
// Lazy import to avoid issues when web-push is not installed
|
||||
const webpush = await import("web-push");
|
||||
|
||||
@@ -116,6 +118,7 @@ export async function sendViaWebPush(
|
||||
// target. Only relative paths are honoured so a poisoned metadata
|
||||
// value can never point the SW at a foreign origin.
|
||||
url:
|
||||
!managedDelivery &&
|
||||
typeof payload.metadata?.url === "string" &&
|
||||
payload.metadata.url.startsWith("/")
|
||||
? payload.metadata.url
|
||||
|
||||
@@ -107,6 +107,30 @@ async function cleanupReminderTracking(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A managed-record reminder is tracked under the Guardian recipient so a
|
||||
* replacement can delete the prior message. Its callback must never mutate
|
||||
* the managed record, even if a crafted Telegram update supplies action data.
|
||||
*/
|
||||
async function isManagedReminderCallback(input: {
|
||||
recipientUserId: string;
|
||||
chatId: string;
|
||||
messageId: number;
|
||||
}): Promise<boolean> {
|
||||
const reminder = await prisma.telegramReminderMessage.findFirst({
|
||||
where: {
|
||||
recipientUserId: input.recipientUserId,
|
||||
chatId: input.chatId,
|
||||
messageId: input.messageId,
|
||||
},
|
||||
select: { medication: { select: { userId: true } } },
|
||||
});
|
||||
return (
|
||||
reminder?.medication?.userId !== undefined &&
|
||||
reminder.medication.userId !== input.recipientUserId
|
||||
);
|
||||
}
|
||||
|
||||
// v1.19.0 — the auto-delete window dropped from 1 h to ~30 min and moved to
|
||||
// the shared `scheduleTelegramAutoDelete` helper so the inbound (free-text)
|
||||
// and outbound (unanswered prompt) paths agree on the window.
|
||||
@@ -379,6 +403,23 @@ export async function handleCallback(update: TelegramUpdate) {
|
||||
const data = callback.data ?? "";
|
||||
const messageId = callback.message?.message_id;
|
||||
|
||||
if (
|
||||
messageId !== undefined &&
|
||||
/^(taken|snooze|skip|ack):/.test(data) &&
|
||||
(await isManagedReminderCallback({
|
||||
recipientUserId: user.id,
|
||||
chatId,
|
||||
messageId,
|
||||
}))
|
||||
) {
|
||||
await answerTelegramCallbackQuery(
|
||||
botToken,
|
||||
callback.id,
|
||||
t("telegram.errorInvalidAction"),
|
||||
);
|
||||
return;
|
||||
}
|
||||
|
||||
if (data.startsWith("taken:")) {
|
||||
const medicationId = data.slice("taken:".length).trim();
|
||||
if (!medicationId) {
|
||||
|
||||
@@ -1,6 +1,11 @@
|
||||
import { beforeEach, describe, expect, it, vi } from "vitest";
|
||||
|
||||
const sent = vi.hoisted(() => ({
|
||||
deliveries: [] as Array<{
|
||||
channel: "NTFY" | "APNS" | "WEB_PUSH" | "TELEGRAM";
|
||||
recordUserId: string;
|
||||
recipientUserId: string;
|
||||
}>,
|
||||
ntfy: [] as Array<{
|
||||
recipientUserId: string;
|
||||
title: string;
|
||||
@@ -9,29 +14,125 @@ const sent = vi.hoisted(() => ({
|
||||
apns: [] as Array<{ recipientUserId: string; recordUserId?: string }>,
|
||||
}));
|
||||
|
||||
vi.mock("@/lib/notifications/senders/ntfy", () => ({
|
||||
sendViaNtfy: async (
|
||||
_config: unknown,
|
||||
payload: { recipientUserId?: string; title: string; message: string },
|
||||
) => {
|
||||
sent.ntfy.push({
|
||||
recipientUserId: payload.recipientUserId ?? "missing",
|
||||
title: payload.title,
|
||||
message: payload.message,
|
||||
});
|
||||
return { ok: true };
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/notifications/senders/ntfy", async () => {
|
||||
const { recordPushAttemptForPayload } = await vi.importActual<
|
||||
typeof import("@/lib/notifications/senders/push-attempt-record")
|
||||
>("@/lib/notifications/senders/push-attempt-record");
|
||||
return {
|
||||
sendViaNtfy: async (
|
||||
_config: unknown,
|
||||
payload: {
|
||||
userId: string;
|
||||
recordUserId?: string;
|
||||
recipientUserId?: string;
|
||||
eventType: string;
|
||||
title: string;
|
||||
message: string;
|
||||
},
|
||||
) => {
|
||||
const recipientUserId = payload.recipientUserId ?? payload.userId;
|
||||
sent.ntfy.push({
|
||||
recipientUserId,
|
||||
title: payload.title,
|
||||
message: payload.message,
|
||||
});
|
||||
sent.deliveries.push({
|
||||
channel: "NTFY",
|
||||
recordUserId: payload.recordUserId ?? payload.userId,
|
||||
recipientUserId,
|
||||
});
|
||||
recordPushAttemptForPayload(payload, recipientUserId, {
|
||||
userId: payload.userId,
|
||||
channel: "NTFY",
|
||||
eventType: payload.eventType,
|
||||
result: "ok",
|
||||
});
|
||||
return { ok: true };
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@/lib/notifications/senders/apns", () => ({
|
||||
sendViaApns: async (
|
||||
recipientUserId: string,
|
||||
payload: { recordUserId?: string },
|
||||
) => {
|
||||
sent.apns.push({ recipientUserId, recordUserId: payload.recordUserId });
|
||||
return { ok: true };
|
||||
},
|
||||
}));
|
||||
vi.mock("@/lib/notifications/senders/apns", async () => {
|
||||
const { recordPushAttemptForPayload } = await vi.importActual<
|
||||
typeof import("@/lib/notifications/senders/push-attempt-record")
|
||||
>("@/lib/notifications/senders/push-attempt-record");
|
||||
return {
|
||||
sendViaApns: async (
|
||||
recipientUserId: string,
|
||||
payload: { userId: string; recordUserId?: string; eventType: string },
|
||||
) => {
|
||||
sent.apns.push({ recipientUserId, recordUserId: payload.recordUserId });
|
||||
sent.deliveries.push({
|
||||
channel: "APNS",
|
||||
recordUserId: payload.recordUserId ?? payload.userId,
|
||||
recipientUserId,
|
||||
});
|
||||
recordPushAttemptForPayload(payload, recipientUserId, {
|
||||
userId: payload.userId,
|
||||
channel: "APNS",
|
||||
eventType: payload.eventType,
|
||||
result: "ok",
|
||||
});
|
||||
return { ok: true };
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@/lib/notifications/senders/web-push", async () => {
|
||||
const { recordPushAttemptForPayload } = await vi.importActual<
|
||||
typeof import("@/lib/notifications/senders/push-attempt-record")
|
||||
>("@/lib/notifications/senders/push-attempt-record");
|
||||
return {
|
||||
sendViaWebPush: async (
|
||||
recipientUserId: string,
|
||||
payload: { userId: string; recordUserId?: string; eventType: string },
|
||||
) => {
|
||||
sent.deliveries.push({
|
||||
channel: "WEB_PUSH",
|
||||
recordUserId: payload.recordUserId ?? payload.userId,
|
||||
recipientUserId,
|
||||
});
|
||||
recordPushAttemptForPayload(payload, recipientUserId, {
|
||||
userId: payload.userId,
|
||||
channel: "WEB_PUSH",
|
||||
eventType: payload.eventType,
|
||||
result: "ok",
|
||||
});
|
||||
return { ok: true };
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
vi.mock("@/lib/notifications/senders/telegram", async () => {
|
||||
const { recordPushAttemptForPayload } = await vi.importActual<
|
||||
typeof import("@/lib/notifications/senders/push-attempt-record")
|
||||
>("@/lib/notifications/senders/push-attempt-record");
|
||||
return {
|
||||
sendViaTelegram: async (
|
||||
_config: unknown,
|
||||
payload: {
|
||||
userId: string;
|
||||
recordUserId?: string;
|
||||
recipientUserId?: string;
|
||||
eventType: string;
|
||||
},
|
||||
) => {
|
||||
const recipientUserId = payload.recipientUserId ?? payload.userId;
|
||||
sent.deliveries.push({
|
||||
channel: "TELEGRAM",
|
||||
recordUserId: payload.recordUserId ?? payload.userId,
|
||||
recipientUserId,
|
||||
});
|
||||
recordPushAttemptForPayload(payload, recipientUserId, {
|
||||
userId: payload.userId,
|
||||
channel: "TELEGRAM",
|
||||
eventType: payload.eventType,
|
||||
result: "ok",
|
||||
});
|
||||
return { ok: true };
|
||||
},
|
||||
};
|
||||
});
|
||||
|
||||
import { encrypt } from "@/lib/crypto";
|
||||
import type { Locale } from "@/lib/i18n/config";
|
||||
@@ -73,7 +174,7 @@ async function addGuardian(recordUserId: string, recipientUserId: string) {
|
||||
|
||||
async function addChannel(
|
||||
userId: string,
|
||||
type: "NTFY" | "APNS",
|
||||
type: "NTFY" | "APNS" | "WEB_PUSH" | "TELEGRAM",
|
||||
enabled = true,
|
||||
) {
|
||||
await getPrismaClient().notificationChannel.create({
|
||||
@@ -87,6 +188,7 @@ async function addChannel(
|
||||
}
|
||||
|
||||
beforeEach(async () => {
|
||||
sent.deliveries.length = 0;
|
||||
sent.ntfy.length = 0;
|
||||
sent.apns.length = 0;
|
||||
await truncateAllTables(getPrismaClient());
|
||||
@@ -199,4 +301,82 @@ describe("Guardian fan-out (real Postgres)", () => {
|
||||
]),
|
||||
);
|
||||
});
|
||||
|
||||
it("writes the complete two-record, two-Guardian, four-channel attribution matrix", async () => {
|
||||
const [recordA, recordB, guardianOne, guardianTwo, ordinaryRecord] =
|
||||
await Promise.all([
|
||||
createUser({ label: "record-a", locale: "en", managed: true }),
|
||||
createUser({ label: "record-b", locale: "de", managed: true }),
|
||||
createUser({ label: "guardian-one", locale: "en" }),
|
||||
createUser({ label: "guardian-two", locale: "de" }),
|
||||
createUser({ label: "ordinary-record", locale: "en" }),
|
||||
]);
|
||||
const guardianIds = [guardianOne.id, guardianTwo.id];
|
||||
const recordIds = [recordA.id, recordB.id];
|
||||
const channelTypes = ["NTFY", "APNS", "WEB_PUSH", "TELEGRAM"] as const;
|
||||
|
||||
await Promise.all([
|
||||
...recordIds.flatMap((recordUserId) =>
|
||||
guardianIds.map((recipientUserId) =>
|
||||
addGuardian(recordUserId, recipientUserId),
|
||||
),
|
||||
),
|
||||
// An ordinary adult's MANAGE grant must never cause fan-out.
|
||||
addGuardian(ordinaryRecord.id, guardianOne.id),
|
||||
...guardianIds.flatMap((userId) =>
|
||||
channelTypes.map((type) => addChannel(userId, type)),
|
||||
),
|
||||
]);
|
||||
|
||||
await Promise.all(
|
||||
recordIds.map((userId) =>
|
||||
dispatchNotification({
|
||||
eventType: "MEDICATION_REMINDER",
|
||||
userId,
|
||||
title: "record reminder",
|
||||
message: "record message",
|
||||
}),
|
||||
),
|
||||
);
|
||||
|
||||
const expected = recordIds.flatMap((recordUserId) =>
|
||||
guardianIds.flatMap((recipientUserId) =>
|
||||
channelTypes.map((channel) => ({
|
||||
channel,
|
||||
recordUserId,
|
||||
recipientUserId,
|
||||
})),
|
||||
),
|
||||
);
|
||||
expect(sent.deliveries).toHaveLength(expected.length);
|
||||
expect(sent.deliveries).toEqual(expect.arrayContaining(expected));
|
||||
expect(
|
||||
sent.deliveries.some((delivery) =>
|
||||
recordIds.includes(delivery.recipientUserId),
|
||||
),
|
||||
).toBe(false);
|
||||
|
||||
await vi.waitFor(async () => {
|
||||
const attempts = await getPrismaClient().pushAttempt.findMany({
|
||||
where: { eventType: "MEDICATION_REMINDER" },
|
||||
select: { channel: true, recordUserId: true, recipientUserId: true },
|
||||
});
|
||||
expect(attempts).toHaveLength(expected.length);
|
||||
expect(attempts).toEqual(expect.arrayContaining(expected));
|
||||
});
|
||||
|
||||
await dispatchNotification({
|
||||
eventType: "MEDICATION_REMINDER",
|
||||
userId: ordinaryRecord.id,
|
||||
title: "ordinary reminder",
|
||||
message: "ordinary message",
|
||||
});
|
||||
|
||||
expect(sent.deliveries).toHaveLength(expected.length);
|
||||
await expect(
|
||||
getPrismaClient().pushAttempt.count({
|
||||
where: { eventType: "MEDICATION_REMINDER" },
|
||||
}),
|
||||
).resolves.toBe(expected.length);
|
||||
});
|
||||
});
|
||||
Reference in new issue
Block a user