Say which rule a rejected Web Push subscription broke

`POST` and `DELETE /api/notifications/web-push` both answered a flat
`apiError("Invalid data", 422)` and discarded the Zod issues. A non-HTTPS
endpoint, an endpoint aimed at an internal host, an over-long one and a missing
subscription key produced byte-identical bodies, so a browser whose
subscription was refused could not tell a fixable client bug from a policy this
instance will never satisfy. Both now answer the multi-issue envelope every
other body-parsing route in the tree uses.

Echoing the issues is safe here and the route says why, because it is the kind
of thing that gets re-derived wrongly later: the endpoint is a subscription's
routing secret and `keys` is its crypto material, but `sanitiseZodIssues` emits
`path`, `code` and `message` only — the rejected value lives in `issue.params`
and stays server-side — and every message this schema can produce is a fixed
string or a length/format default rather than an interpolation. A test pins
that none of the four secrets reaches the wire, so a future schema change that
adds an interpolating message fails here instead of leaking quietly.

The tests assert on content rather than status: a status-only check passes
against the flat error it replaces, which is how this survived. Two of them
compare whole bodies between two different refusals and require them to differ.

The two `/test` routes were checked in the same pass and need nothing. Neither
accepts a body — they take no parameters at all — so neither runs a Zod parse
to discard. Their 422s already carry `not_configured`, `vapid_not_configured`
and `rate_limited_self` as machine-readable codes.

src/app/api/notifications/web-push/__tests__/route.test.ts

@@ -0,0 +1,174 @@
+/**
+ * `/api/notifications/web-push` — the refusal has to say which rule was broken.
+ *
+ * Both handlers used to answer a flat `apiError("Invalid data", 422)` and throw
+ * the Zod issues away. A non-HTTPS endpoint, an endpoint aimed at an internal
+ * host, an over-long one and a missing subscription key all produced the same
+ * three words, so a browser whose subscription was refused could not tell a
+ * fixable mistake from a policy it would never satisfy.
+ *
+ * The cases below assert on CONTENT rather than status. A status-only test
+ * passes just as happily against the flat error it replaces, which is the whole
+ * reason the defect survived this long.
+ *
+ * The other half is what must NOT come back. The endpoint is the routing secret
+ * for a subscription and `keys` is crypto material, so the last test pins that
+ * neither reaches the wire: `sanitiseZodIssues` emits `path`, `code` and
+ * `message` only, and every message this schema can produce is a fixed string
+ * or a length/format default rather than an interpolation of the value.
+ */
+import { describe, it, expect, vi, beforeEach } from "vitest";
+
+vi.mock("@/lib/api-handler", () => ({
+  apiHandler: (fn: unknown) => fn,
+  requireAuth: vi.fn(),
+}));
+
+vi.mock("@/lib/db", () => ({
+  prisma: {
+    pushSubscription: { upsert: vi.fn(), deleteMany: vi.fn() },
+    notificationChannel: { findFirst: vi.fn(), create: vi.fn() },
+  },
+}));
+
+vi.mock("@/lib/logging/context", () => ({ annotate: vi.fn() }));
+
+vi.mock("@/lib/crypto", () => ({ encrypt: (v: string) => `enc(${v})` }));
+
+import { POST, DELETE } from "../route";
+import { requireAuth } from "@/lib/api-handler";
+import { prisma } from "@/lib/db";
+
+const USER = { id: "user-1", username: "u" };
+
+/** A well-formed subscription, for the cases that should succeed. */
+const GOOD = {
+  endpoint: "https://fcm.googleapis.com/fcm/send/abcdef-routing-secret",
+  keys: { p256dh: "BPkey", auth: "authsecret" },
+};
+
+type IssueEnvelope = {
+  error: string;
+  details?: {
+    issues?: Array<{ path?: string; code?: string; message?: string }>;
+  };
+};
+
+function req(body: unknown): Request {
+  return new Request("http://localhost/api/notifications/web-push", {
+    method: "POST",
+    body: JSON.stringify(body),
+    headers: { "Content-Type": "application/json" },
+  });
+}
+
+type Handler = (r: Request) => Promise<Response>;
+
+beforeEach(() => {
+  vi.resetAllMocks();
+  vi.mocked(requireAuth).mockResolvedValue({ user: USER } as never);
+});
+
+describe("POST /api/notifications/web-push — the subscribe refusal", () => {
+  it("names the offending field for a plain-http endpoint", async () => {
+    const res = await (POST as unknown as Handler)(
+      req({ ...GOOD, endpoint: "http://fcm.googleapis.com/fcm/send/x" }),
+    );
+
+    expect(res.status).toBe(422);
+    const env = (await res.json()) as IssueEnvelope;
+    expect(env.error).toBe("Validation failed");
+    expect(env.details?.issues?.length).toBeGreaterThan(0);
+    expect(env.details?.issues?.some((i) => i.path === "endpoint")).toBe(true);
+    expect(prisma.pushSubscription.upsert).not.toHaveBeenCalled();
+  });
+
+  it("names the offending field for a missing subscription key", async () => {
+    const res = await (POST as unknown as Handler)(
+      req({ endpoint: GOOD.endpoint, keys: { p256dh: "BPkey" } }),
+    );
+
+    expect(res.status).toBe(422);
+    const env = (await res.json()) as IssueEnvelope;
+    expect(env.details?.issues?.some((i) => i.path === "keys.auth")).toBe(true);
+    expect(prisma.pushSubscription.upsert).not.toHaveBeenCalled();
+  });
+
+  it("tells an internal-host endpoint apart from an http one", async () => {
+    // The two are different problems with different fixes — one is a client
+    // bug, the other is a subscription this instance will never accept. Under
+    // the flat error the bodies were byte-identical.
+    const insecure = await (POST as unknown as Handler)(
+      req({ ...GOOD, endpoint: "http://fcm.googleapis.com/fcm/send/x" }),
+    );
+    const internal = await (POST as unknown as Handler)(
+      req({ ...GOOD, endpoint: "https://192.168.1.10/push/x" }),
+    );
+
+    expect(insecure.status).toBe(422);
+    expect(internal.status).toBe(422);
+    expect(JSON.stringify(await insecure.json())).not.toBe(
+      JSON.stringify(await internal.json()),
+    );
+  });
+
+  it("still stores a well-formed subscription", async () => {
+    vi.mocked(prisma.notificationChannel.findFirst).mockResolvedValue(
+      null as never,
+    );
+    vi.mocked(prisma.pushSubscription.upsert).mockResolvedValue({} as never);
+    vi.mocked(prisma.notificationChannel.create).mockResolvedValue({} as never);
+
+    const res = await (POST as unknown as Handler)(req(GOOD));
+
+    expect(res.status).toBe(200);
+    expect(prisma.pushSubscription.upsert).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe("DELETE /api/notifications/web-push — the unsubscribe refusal", () => {
+  it("names the offending field rather than answering flat", async () => {
+    const res = await (DELETE as unknown as Handler)(
+      req({ endpoint: "not-a-url" }),
+    );
+
+    expect(res.status).toBe(422);
+    const env = (await res.json()) as IssueEnvelope;
+    expect(env.error).toBe("Validation failed");
+    expect(env.details?.issues?.some((i) => i.path === "endpoint")).toBe(true);
+    expect(prisma.pushSubscription.deleteMany).not.toHaveBeenCalled();
+  });
+
+  it("distinguishes a missing endpoint from a malformed one", async () => {
+    const missing = await (DELETE as unknown as Handler)(req({}));
+    const malformed = await (DELETE as unknown as Handler)(
+      req({ endpoint: "not-a-url" }),
+    );
+
+    expect(JSON.stringify(await missing.json())).not.toBe(
+      JSON.stringify(await malformed.json()),
+    );
+  });
+});
+
+describe("what the refusal must never carry", () => {
+  it("echoes neither the endpoint nor the subscription keys", async () => {
+    // The endpoint path is the routing secret for a push subscription and the
+    // keys are its crypto material. Publishing the issue list is only safe
+    // while this holds; if a future schema change adds a message that
+    // interpolates the value, this fails rather than leaking quietly.
+    const res = await (POST as unknown as Handler)(
+      req({
+        endpoint: "http://attacker.example/fcm/send/ROUTING-SECRET-abc123",
+        keys: { p256dh: "P256-SECRET-xyz", auth: "AUTH-SECRET-789" },
+      }),
+    );
+
+    expect(res.status).toBe(422);
+    const raw = await res.text();
+    expect(raw).not.toContain("ROUTING-SECRET-abc123");
+    expect(raw).not.toContain("P256-SECRET-xyz");
+    expect(raw).not.toContain("AUTH-SECRET-789");
+    expect(raw).not.toContain("attacker.example");
+  });
+});

src/app/api/notifications/web-push/route.ts

@@ -2,7 +2,7 @@ import { NextRequest } from "next/server";
 import { prisma } from "@/lib/db";
 import { apiHandler, requireAuth } from "@/lib/api-handler";
 import { annotate } from "@/lib/logging/context";
-import { apiSuccess, apiError } from "@/lib/api-response";
+import { apiSuccess, apiError, returnAllZodIssues } from "@/lib/api-response";
 import { encrypt } from "@/lib/crypto";
 import { webPushSubscriptionSchema } from "@/lib/validations/notifications";
 import { z } from "zod/v4";
@@ -18,6 +18,23 @@ const unsubscribeSchema = z.object({
   endpoint: z.string().url(),
 });
 
+/*
+ * Both parses below answer with the multi-issue envelope, like every other
+ * body-parsing route in the tree. They used to answer a flat
+ * `apiError("Invalid data", 422)`, which made a non-HTTPS endpoint, an
+ * endpoint pointing at an internal host, an over-long one and a missing
+ * subscription key byte-identical to each other — a client could see that its
+ * subscription was refused and had no way to learn which rule it broke.
+ *
+ * Echoing the issues is safe here, and it is worth writing down why rather
+ * than leaving it to be re-derived. The endpoint is a routing secret and the
+ * keys are subscription crypto material, so the refusal must not carry them
+ * back. It does not: `sanitiseZodIssues` emits `path`, `code` and `message`
+ * only — `issue.params`, which is where Zod keeps a rejected value, stays
+ * server-side — and every message this schema can produce is a fixed string
+ * or a length/format default. None of them interpolates the value.
+ */
+
 /**
  * POST /api/notifications/web-push
  * Save a Web Push subscription for the current user.
@@ -38,7 +55,7 @@ export const POST = apiHandler(async (request: NextRequest) => {
   }
 
   const parsed = subscribeSchema.safeParse(body);
-  if (!parsed.success) return apiError("Invalid data", 422);
+  if (!parsed.success) return returnAllZodIssues(parsed.error, 422);
 
   const { endpoint, keys } = parsed.data;
   const userAgent = request.headers.get("user-agent") ?? undefined;
@@ -104,7 +121,7 @@ export const DELETE = apiHandler(async (request: NextRequest) => {
   }
 
   const parsed = unsubscribeSchema.safeParse(body);
-  if (!parsed.success) return apiError("Invalid data", 422);
+  if (!parsed.success) return returnAllZodIssues(parsed.error, 422);
 
   await prisma.pushSubscription.deleteMany({
     where: {

src/lib/openapi/routes/notifications-transport.ts

@@ -130,7 +130,7 @@ export const notificationTransportPaths: NonNullable<
       description:
         "Stores a browser Push subscription for the caller and, on the first one, creates the account's Web Push notification channel enabled. Upsert by endpoint: re-posting the same endpoint refreshes its keys rather than duplicating the row, so a client can send this on every page load without checking first.\n\n" +
         "The subscription keys are encrypted at rest. The endpoint is validated as HTTPS and as a public host before anything is written — the push library dials it later through its own fetch, outside the shared egress wrapper, so input time is where that has to be caught.\n\n" +
-        "Body capped at 64 KB. A malformed body and a rejected endpoint both answer 422 with one flat message; the Zod issues are not surfaced, so a client cannot tell an invalid endpoint from a missing key.",
+        "Body capped at 64 KB. A rejected subscription answers the standard multi-issue 422, so a client can tell a non-HTTPS endpoint from an internal-host one from a missing key. The issue list carries `path`, `code` and `message` and never the rejected value — neither the endpoint nor the subscription keys are echoed back.",
       requestBody: {
         required: true,
         content: { "application/json": { schema: webPushSubscribeRequest } },
@@ -159,7 +159,7 @@ export const notificationTransportPaths: NonNullable<
       summary: "Forget a Web Push subscription",
       description:
         "Removes the caller's subscription for the given endpoint. Idempotent and silent about the outcome: an endpoint that was never stored, and one belonging to another account, both answer the same success — the response says nothing about how many rows matched, so it cannot be used to probe whether an endpoint exists.\n\n" +
-        "The Web Push channel row is left in place; this removes a destination, not the channel. Body capped at 64 KB.",
+        "The Web Push channel row is left in place; this removes a destination, not the channel. Body capped at 64 KB; a malformed `endpoint` answers the standard multi-issue 422, which does not echo the value.",
       requestBody: {
         required: true,
         content: { "application/json": { schema: webPushUnsubscribeRequest } },