fix(notifications): dial Web Push through the pinned dispatcher

The push endpoint host is user-controlled — the browser supplies it at
subscribe time. `isPublicUrl` guards it at subscribe time and again before each
dial, but it is a literal input-time check and cannot see a DNS rebind between
the accept and the connect; with a short-TTL record the dial reaches the pod's
private network.

`web-push`'s `sendNotification` dials with its own internal `https.request`,
which bypassed `safeFetch` and therefore the connect-time pin entirely. Both
senders now sign and encrypt through `generateRequestDetails` — identical VAPID
handling and payload encryption — and dial through `safeFetch` with
`requirePublicHost`. Non-2xx is re-shaped as the `{ statusCode }` rejection the
existing classification already understands, so the 410/404 expiry reap and the
transient/permanent split are unchanged.

This is the last user-controlled-host egress that was not behind the pinned
dispatcher (issue #217).
This commit is contained in:
Marc-André Bombeck committed 2026-07-19 14:31:25 +02:00
1 parent d103699099
commit 83da7c57c8
6 files changed
+316 -32

No files matched your search

@@ -11,9 +11,24 @@
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const { sendNotificationMock, findManyMock, deleteManyMock } = vi.hoisted(
// safeFetch's requirePublicHost path drives undici's OWN fetch (version-locked
// with its dispatcher). Delegate it to the global fetch these tests stub so the
// existing interception still applies.
vi.mock("undici", async (importOriginal) => {
const actual = await importOriginal<typeof import("undici")>();
return {
...actual,
fetch: (input: unknown, init?: unknown) =>
(globalThis.fetch as unknown as (i: unknown, n?: unknown) => unknown)(
input,
init,
),
};
});
const { generateRequestDetailsMock, findManyMock, deleteManyMock } = vi.hoisted(
() => ({
sendNotificationMock: vi.fn(),
generateRequestDetailsMock: vi.fn(),
findManyMock: vi.fn(),
deleteManyMock: vi.fn(),
}),
@@ -61,7 +76,17 @@ vi.mock("@/lib/validations/notifications", () => ({
vi.mock("web-push", () => ({
setVapidDetails: vi.fn(),
sendNotification: (...args: unknown[]) => sendNotificationMock(...args),
// The sender signs + encrypts here and dials through `safeFetch`; the
// payload assertions below read the same second argument as before.
generateRequestDetails: (...args: unknown[]) => {
generateRequestDetailsMock(...args);
return {
endpoint: "https://push.example.com/x",
method: "POST",
headers: {},
body: null,
};
},
}));
import { sendViaWebPush } from "../senders/web-push";
@@ -76,7 +101,7 @@ const SUB = {
};
beforeEach(() => {
sendNotificationMock.mockReset();
generateRequestDetailsMock.mockReset();
findManyMock.mockReset();
deleteManyMock.mockReset();
deleteManyMock.mockResolvedValue({ count: 0 });
@@ -96,7 +121,9 @@ describe("medicationDoseTag", () => {
describe("web-push reminder push — stable tag + badge", () => {
it("uses metadata.webPushTag as the notification tag and carries the badge", async () => {
findManyMock.mockResolvedValue([SUB]);
sendNotificationMock.mockResolvedValue(undefined);
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 201 }),
);
const tag = medicationDoseTag("med-1", "2026-06-18T07:00:00.000Z");
const res = await sendViaWebPush("user-1", {
@@ -108,7 +135,9 @@ describe("web-push reminder push — stable tag + badge", () => {
});
expect(res.ok).toBe(true);
const body = JSON.parse(sendNotificationMock.mock.calls[0][1] as string);
const body = JSON.parse(
generateRequestDetailsMock.mock.calls[0][1] as string,
);
expect(body.tag).toBe(tag);
expect(body.badge).toBe(2);
expect(body.url).toBe("/medications/med-1");
@@ -116,7 +145,9 @@ describe("web-push reminder push — stable tag + badge", () => {
it("discreet mode still wins over a stable tag (no event name leak)", async () => {
findManyMock.mockResolvedValue([SUB]);
sendNotificationMock.mockResolvedValue(undefined);
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 201 }),
);
await sendViaWebPush("user-1", {
eventType: "CYCLE_PERIOD_SOON",
@@ -127,7 +158,9 @@ describe("web-push reminder push — stable tag + badge", () => {
metadata: { webPushTag: "med:should-not-leak" },
});
const body = JSON.parse(sendNotificationMock.mock.calls[0][1] as string);
const body = JSON.parse(
generateRequestDetailsMock.mock.calls[0][1] as string,
);
expect(body.tag).toBe("REMINDER");
});
});
@@ -135,7 +168,9 @@ describe("web-push reminder push — stable tag + badge", () => {
describe("clear-on-taken push", () => {
it("emits type:clear with the SAME slot tag as the reminder + badge", async () => {
findManyMock.mockResolvedValue([SUB]);
sendNotificationMock.mockResolvedValue(undefined);
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 201 }),
);
await dispatchMedicationIntakeWebClear({
userId: "user-1",
@@ -144,8 +179,10 @@ describe("clear-on-taken push", () => {
badgeCount: 1,
});
expect(sendNotificationMock).toHaveBeenCalledTimes(1);
const body = JSON.parse(sendNotificationMock.mock.calls[0][1] as string);
expect(generateRequestDetailsMock).toHaveBeenCalledTimes(1);
const body = JSON.parse(
generateRequestDetailsMock.mock.calls[0][1] as string,
);
expect(body.type).toBe("clear");
expect(body.tag).toBe(
medicationDoseTag("med-1", "2026-06-18T07:00:00.000Z"),
@@ -155,7 +192,9 @@ describe("clear-on-taken push", () => {
it("a count of 0 clears the badge (badge:0 on the wire)", async () => {
findManyMock.mockResolvedValue([SUB]);
sendNotificationMock.mockResolvedValue(undefined);
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 201 }),
);
await dispatchMedicationIntakeWebClear({
userId: "user-1",
@@ -164,7 +203,9 @@ describe("clear-on-taken push", () => {
badgeCount: 0,
});
const body = JSON.parse(sendNotificationMock.mock.calls[0][1] as string);
const body = JSON.parse(
generateRequestDetailsMock.mock.calls[0][1] as string,
);
expect(body.badge).toBe(0);
});
@@ -177,12 +218,14 @@ describe("clear-on-taken push", () => {
scheduledFor: "2026-06-18T07:00:00.000Z",
}),
).resolves.toBeUndefined();
expect(sendNotificationMock).not.toHaveBeenCalled();
expect(generateRequestDetailsMock).not.toHaveBeenCalled();
});
it("reaps expired (410) subscriptions", async () => {
findManyMock.mockResolvedValue([SUB]);
sendNotificationMock.mockRejectedValue({ statusCode: 410 });
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 410 }),
);
await dispatchMedicationIntakeWebClear({
userId: "user-1",
@@ -0,0 +1,147 @@
/**
* Web Push egress must run through the pinned dispatcher.
*
* The push endpoint host is USER-CONTROLLED — the browser hands it over at
* subscribe time and the server stores it. `isPublicUrl` is checked both at
* subscribe time and again before each dial, but it is a literal, input-time
* check: it cannot see a DNS rebind between the accept and the connect. With a
* short-TTL record an attacker flips the host to `169.254.169.254` or an
* internal address after the check passes, and the dial reaches the pod's
* private network (issue #217).
*
* `web-push`'s `sendNotification` dials with its own internal `https.request`,
* which bypasses `safeFetch` entirely and therefore bypasses the pinned
* dispatcher. Both senders now sign and encrypt via `generateRequestDetails`
* — identical VAPID + payload handling — and dial through `safeFetch` with
* `requirePublicHost: true`.
*/
import { beforeEach, describe, expect, it, vi } from "vitest";
const { safeFetchMock, findManyMock, deleteManyMock } = vi.hoisted(() => ({
safeFetchMock: vi.fn(),
findManyMock: vi.fn(),
deleteManyMock: vi.fn(),
}));
vi.mock("@/lib/safe-fetch", () => ({
safeFetch: (...args: unknown[]) => safeFetchMock(...args),
}));
vi.mock("@/lib/logging/context", () => ({
getEvent: () => ({
addWarning: vi.fn(),
addExternalCall: vi.fn(),
setError: vi.fn(),
addMeta: vi.fn(),
}),
}));
vi.mock("@/lib/notifications/senders/push-attempt-record", () => ({
recordPushAttempt: vi.fn(),
}));
vi.mock("@/lib/db", () => ({
prisma: {
pushSubscription: { findMany: findManyMock, deleteMany: deleteManyMock },
},
}));
vi.mock("@/lib/crypto", () => ({
decrypt: (v: string) => v,
encrypt: (v: string) => v,
}));
vi.mock("@/lib/notifications/vapid-config", () => ({
getVapidConfig: () =>
Promise.resolve({
subject: "mailto:a@b.c",
publicKey: "pub",
privateKey: "priv",
}),
}));
vi.mock("@/lib/validations/notifications", () => ({
isPublicUrl: () => true,
}));
vi.mock("web-push", () => ({
setVapidDetails: vi.fn(),
generateRequestDetails: () => ({
endpoint: "https://push.example.com/x",
method: "POST",
headers: { TTL: "60" },
body: null,
}),
}));
import { sendViaWebPush } from "../senders/web-push";
import { dispatchMedicationIntakeWebClear } from "../web-push-clear";
const SUB = {
id: "s1",
endpoint: "https://push.example.com/x",
p256dh: "a",
auth: "b",
};
beforeEach(() => {
safeFetchMock.mockReset();
findManyMock.mockReset();
deleteManyMock.mockReset();
deleteManyMock.mockResolvedValue({ count: 0 });
findManyMock.mockResolvedValue([SUB]);
safeFetchMock.mockResolvedValue({ ok: true, status: 201 });
});
function optsOfFirstCall() {
return safeFetchMock.mock.calls[0]?.[2] as
{ requirePublicHost?: boolean } | undefined;
}
describe("sendViaWebPush egress", () => {
it("dials through safeFetch, not web-push's own transport", async () => {
const res = await sendViaWebPush("user-1", {
eventType: "MEDICATION_REMINDER",
userId: "user-1",
title: "T",
message: "M",
});
expect(res.ok).toBe(true);
expect(safeFetchMock).toHaveBeenCalledTimes(1);
});
it("pins the connect-time host check with requirePublicHost", async () => {
await sendViaWebPush("user-1", {
eventType: "MEDICATION_REMINDER",
userId: "user-1",
title: "T",
message: "M",
});
expect(optsOfFirstCall()?.requirePublicHost).toBe(true);
});
it("maps a 410 onto the expiry-reap path", async () => {
safeFetchMock.mockResolvedValue({ ok: false, status: 410 });
await sendViaWebPush("user-1", {
eventType: "MEDICATION_REMINDER",
userId: "user-1",
title: "T",
message: "M",
});
expect(deleteManyMock).toHaveBeenCalledWith({
where: { id: { in: ["s1"] } },
});
});
});
describe("dispatchMedicationIntakeWebClear egress", () => {
it("dials through safeFetch with requirePublicHost too", async () => {
await dispatchMedicationIntakeWebClear({
userId: "user-1",
medicationId: "med-1",
scheduledFor: "2026-06-18T07:00:00.000Z",
});
expect(safeFetchMock).toHaveBeenCalledTimes(1);
expect(optsOfFirstCall()?.requirePublicHost).toBe(true);
});
});
@@ -38,7 +38,7 @@ const hoisted = vi.hoisted(() => ({
apnsSendMock: vi.fn(),
apnsShutdownMock: vi.fn(),
webPushSetVapidDetailsMock: vi.fn(),
webPushSendNotificationMock: vi.fn(),
webPushGenerateRequestDetailsMock: vi.fn(),
sendTelegramMessageMock: vi.fn(),
deleteTelegramMessageMock: vi.fn(),
}));
@@ -111,13 +111,23 @@ vi.mock("@/lib/notifications/vapid-config", () => ({
}),
}));
// The sender signs + encrypts via `generateRequestDetails` and dials through
// `safeFetch` (pinned dispatcher) rather than letting web-push run its own
// `https.request`, so the dial is interceptable by the global fetch stub.
const webPushRequestDetails = () => ({
endpoint: "https://example.test/push",
method: "POST",
headers: {},
body: null,
});
vi.mock("web-push", () => ({
default: {
setVapidDetails: hoisted.webPushSetVapidDetailsMock,
sendNotification: hoisted.webPushSendNotificationMock,
generateRequestDetails: hoisted.webPushGenerateRequestDetailsMock,
},
setVapidDetails: hoisted.webPushSetVapidDetailsMock,
sendNotification: hoisted.webPushSendNotificationMock,
generateRequestDetails: hoisted.webPushGenerateRequestDetailsMock,
}));
vi.mock("@/lib/telegram", () => ({
@@ -330,7 +340,12 @@ describe("recordPushAttempt — WEB_PUSH sender", () => {
auth: "enc(a)",
},
] as never);
hoisted.webPushSendNotificationMock.mockResolvedValue(undefined);
hoisted.webPushGenerateRequestDetailsMock.mockReturnValue(
webPushRequestDetails(),
);
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 201 }),
);
const outcome = await sendViaWebPush("user-1", {
userId: "user-1",
@@ -379,7 +394,12 @@ describe("recordPushAttempt — WEB_PUSH sender", () => {
auth: "enc(a)",
},
] as never);
hoisted.webPushSendNotificationMock.mockResolvedValue(undefined);
hoisted.webPushGenerateRequestDetailsMock.mockReturnValue(
webPushRequestDetails(),
);
vi.spyOn(global, "fetch").mockResolvedValue(
new Response(null, { status: 201 }),
);
vi.mocked(prisma.pushAttempt.create).mockRejectedValue(
new Error("connection refused"),
);
@@ -11,12 +11,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const {
safeFetchMock,
recordPushAttemptMock,
sendNotificationMock,
generateRequestDetailsMock,
findManyMock,
} = vi.hoisted(() => ({
safeFetchMock: vi.fn(),
recordPushAttemptMock: vi.fn(),
sendNotificationMock: vi.fn(),
generateRequestDetailsMock: vi.fn(),
findManyMock: vi.fn(),
}));
@@ -61,7 +61,19 @@ vi.mock("@/lib/validations/notifications", () => ({
vi.mock("web-push", () => ({
setVapidDetails: vi.fn(),
sendNotification: (...args: unknown[]) => sendNotificationMock(...args),
// The sender now signs + encrypts via `generateRequestDetails` and dials
// through `safeFetch` (pinned dispatcher) instead of letting web-push run
// its own `https.request`. Same three arguments, so the urgency assertions
// below are unchanged.
generateRequestDetails: (...args: unknown[]) => {
generateRequestDetailsMock(...args);
return {
endpoint: "https://push.example.com/x",
method: "POST",
headers: {},
body: null,
};
},
}));
import { sendViaNtfy } from "../ntfy";
@@ -81,7 +93,7 @@ function payload(over?: Record<string, unknown>) {
beforeEach(() => {
safeFetchMock.mockReset();
recordPushAttemptMock.mockReset();
sendNotificationMock.mockReset();
generateRequestDetailsMock.mockReset();
findManyMock.mockReset();
});
@@ -164,9 +176,9 @@ describe("web-push urgent mapping", () => {
auth: "b",
},
]);
sendNotificationMock.mockResolvedValue(undefined);
safeFetchMock.mockResolvedValue({ ok: true, status: 201 });
await sendViaWebPush("user-1", payload({ urgent: true }));
const [, body, options] = sendNotificationMock.mock.calls[0];
const [, body, options] = generateRequestDetailsMock.mock.calls[0];
expect((options as { urgency?: string }).urgency).toBe("high");
expect(JSON.parse(body as string).requireInteraction).toBe(true);
});
@@ -180,9 +192,9 @@ describe("web-push urgent mapping", () => {
auth: "b",
},
]);
sendNotificationMock.mockResolvedValue(undefined);
safeFetchMock.mockResolvedValue({ ok: true, status: 201 });
await sendViaWebPush("user-1", payload());
const [, body, options] = sendNotificationMock.mock.calls[0];
const [, body, options] = generateRequestDetailsMock.mock.calls[0];
expect(options).toBeUndefined();
expect(JSON.parse(body as string).requireInteraction).toBe(false);
});
@@ -201,7 +213,6 @@ describe("APNs-less instance degrades gracefully", () => {
auth: "b",
},
]);
sendNotificationMock.mockResolvedValue(undefined);
const ntfy = await sendViaNtfy(
{ serverUrl: "https://ntfy.example.com", topic: "t" },
@@ -216,7 +227,8 @@ describe("APNs-less instance degrades gracefully", () => {
.headers.Priority,
).toBe("5");
expect(
(sendNotificationMock.mock.calls[0][2] as { urgency?: string }).urgency,
(generateRequestDetailsMock.mock.calls[0][2] as { urgency?: string })
.urgency,
).toBe("high");
});
});
+37 -1
View File
@@ -7,6 +7,7 @@ import { getVapidConfig } from "@/lib/notifications/vapid-config";
import { getEvent } from "@/lib/logging/context";
import { recordPushAttempt } from "@/lib/notifications/senders/push-attempt-record";
import { isPublicUrl } from "@/lib/validations/notifications";
import { safeFetch } from "@/lib/safe-fetch";
import { plainPushText } from "@/lib/notifications/strip-emoji";
/**
@@ -154,14 +155,49 @@ export async function sendViaWebPush(
const p256dh = decrypt(sub.p256dh);
const auth = decrypt(sub.auth);
await webpush.sendNotification(
// The dial itself goes through `safeFetch` with `requirePublicHost`,
// not through `web-push`'s own internal `https.request`. The literal
// `isPublicUrl` check above is an input-time floor and cannot see a
// DNS rebind between the check and the connect; the pinned dispatcher
// vets every address the resolver actually returns (issue #217).
// `generateRequestDetails` performs the identical VAPID signing and
// payload encryption `sendNotification` would have — only the
// transport changes.
const details = webpush.generateRequestDetails(
{
endpoint: sub.endpoint,
keys: { p256dh, auth },
},
pushPayload,
sendOptions,
) as {
endpoint: string;
method: string;
headers: Record<string, string>;
body: Buffer | string | null;
};
const pushRes = await safeFetch(
details.endpoint,
{
method: details.method,
headers: details.headers,
// `generateRequestDetails` hands back a Node Buffer for an
// encrypted payload; widen it to the BodyInit the fetch API takes.
body: (details.body ?? undefined) as BodyInit | undefined,
},
{ requirePublicHost: true },
);
if (pushRes.status < 200 || pushRes.status >= 300) {
// Re-shape as the `{ statusCode }` rejection the classification
// below already understands, so the 410/404 expiry path and the
// transient/permanent split stay byte-for-byte unchanged.
throw Object.assign(
new Error(`Web Push responded ${pushRes.status}`),
{ statusCode: pushRes.status },
);
}
anySuccess = true;
allPermanentReject = false;
} catch (err: unknown) {
+27 -1
View File
@@ -22,6 +22,7 @@ import { decrypt } from "@/lib/crypto";
import { getVapidConfig } from "@/lib/notifications/vapid-config";
import { getEvent } from "@/lib/logging/context";
import { isPublicUrl } from "@/lib/validations/notifications";
import { safeFetch } from "@/lib/safe-fetch";
import { medicationDoseTag } from "@/lib/notifications/dose-tag";
export interface WebClearInput {
@@ -80,10 +81,35 @@ export async function dispatchMedicationIntakeWebClear(
}
const p256dh = decrypt(sub.p256dh);
const auth = decrypt(sub.auth);
await webpush.sendNotification(
// Same transport swap as the main sender: sign + encrypt via
// `generateRequestDetails`, then dial through `safeFetch` with the
// pinned dispatcher so a DNS rebind between the literal
// `isPublicUrl` check above and the connect cannot reach a private
// address (issue #217).
const details = webpush.generateRequestDetails(
{ endpoint: sub.endpoint, keys: { p256dh, auth } },
clearPayload,
) as {
endpoint: string;
method: string;
headers: Record<string, string>;
body: Buffer | string | null;
};
const clearRes = await safeFetch(
details.endpoint,
{
method: details.method,
headers: details.headers,
body: (details.body ?? undefined) as BodyInit | undefined,
},
{ requirePublicHost: true },
);
if (clearRes.status < 200 || clearRes.status >= 300) {
throw Object.assign(
new Error(`Web Push clear responded ${clearRes.status}`),
{ statusCode: clearRes.status },
);
}
} catch (err: unknown) {
const status = (err as { statusCode?: number }).statusCode;
if (status === 410 || status === 404) expiredIds.push(sub.id);