From cc507deafaf790b5ecd7731284debe6cf2ab5fbc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Wed, 14 Oct 2026 18:57:06 +0200 Subject: [PATCH] fix(notifications): log every refused notification answer A quick answer whose request never reached the route left no trace: a redirect in front of the app turns ntfy's POST into a GET, which Next answered 405 before any wide event, and the worker-only and demo-mode refusals in the proxy wrote nothing either. The proxy now answers a non-POST on the action path itself with 405 and Allow: POST, and every refusal it makes there writes one warn line with a reason and the path without its token. The route's rate-limit refusal carries a reason too. The regression test sends the action ntfy publishes, in the shape ntfy sends it, through the real proxy and route against Postgres. Refs #1203 --- .../proxy-notification-action.test.ts | 123 ++++++++++++++++++ .../api/notifications/action/[token]/route.ts | 4 + src/proxy.ts | 65 +++++++++ .../notification-action-mood.test.ts | 83 ++++++++++++ 4 files changed, 275 insertions(+) create mode 100644 src/__tests__/proxy-notification-action.test.ts diff --git a/src/__tests__/proxy-notification-action.test.ts b/src/__tests__/proxy-notification-action.test.ts new file mode 100644 index 000000000..c0ec388e2 --- /dev/null +++ b/src/__tests__/proxy-notification-action.test.ts @@ -0,0 +1,123 @@ +/** + * The proxy's side of a notification quick answer (#1203). + * + * ntfy's `http` action POSTs to `/api/notifications/action/` with no + * cookie, no Origin and an empty body. That request must pass the proxy + * untouched. Every request the proxy refuses on this path must leave one log + * line with a reason, because the route's own wide event never runs for it + * and the server log is the operator's only evidence. The token must never be + * in that line. + */ +import { NextRequest } from "next/server"; +import { + afterEach, + beforeEach, + describe, + expect, + it, + vi, + type MockInstance, +} from "vitest"; + +const processType = vi.hoisted(() => ({ web: true })); +vi.mock("@/lib/process-type", () => ({ + shouldRunWeb: () => processType.web, +})); + +import { proxy } from "../proxy"; + +const TOKEN = + "na1.cuserid0000000000000000.mood-good.1792017927608.ACWxZHsEVXyh-Yghadj-2Q.PRmiFi7-C4ZkQ45E9YHl95GjSXoeV3xZMwLJ90Ts4QY"; +const URL_ = `https://health.example.test/api/notifications/action/${TOKEN}`; + +/** The request ntfy's Android app sends for an `http` action. */ +function ntfyRequest(method = "POST"): NextRequest { + return new NextRequest(URL_, { + method, + headers: { "user-agent": "ntfy/1.16.0 (okhttp)", "content-length": "0" }, + ...(method === "POST" ? { body: "" } : {}), + }); +} + +let warn: MockInstance<(...args: unknown[]) => void>; + +beforeEach(() => { + processType.web = true; + warn = vi.spyOn(console, "warn").mockImplementation(() => {}); +}); + +afterEach(() => { + warn.mockRestore(); + vi.unstubAllEnvs(); +}); + +function loggedLines(): Array> { + return warn.mock.calls.map((args: unknown[]) => JSON.parse(String(args[0]))); +} + +describe("proxy on /api/notifications/action/", () => { + it("passes ntfy's POST through to the route without a refusal", () => { + const response = proxy(ntfyRequest()); + + expect(response.status).toBe(200); + expect(response.headers.get("x-middleware-next")).toBe("1"); + expect(warn).not.toHaveBeenCalled(); + }); + + it.each(["GET", "HEAD", "PUT", "DELETE"])( + "refuses %s with 405 and logs why, without the token", + (method) => { + const response = proxy(ntfyRequest(method)); + + expect(response.status).toBe(405); + expect(response.headers.get("allow")).toBe("POST"); + const lines = loggedLines(); + expect(lines).toHaveLength(1); + expect(lines[0]).toMatchObject({ + level: "warn", + action: { name: "notification.action.refused" }, + meta: { reason: "method_not_allowed" }, + http: { + method, + status: 405, + path: "/api/notifications/action/[REDACTED]", + }, + }); + expect(String(warn.mock.calls[0][0])).not.toContain(TOKEN); + expect(String(warn.mock.calls[0][0])).not.toContain("PRmiFi7"); + }, + ); + + it("logs the demo-mode refusal", () => { + vi.stubEnv("DEMO_MODE", "true"); + const response = proxy(ntfyRequest()); + + expect(response.status).toBe(403); + expect(loggedLines()[0]).toMatchObject({ + meta: { reason: "demo_mode" }, + http: { status: 403 }, + }); + }); + + it("logs the worker-only refusal", () => { + processType.web = false; + const response = proxy(ntfyRequest()); + + expect(response.status).toBe(503); + expect(loggedLines()[0]).toMatchObject({ + meta: { reason: "worker_only" }, + http: { status: 503 }, + }); + }); + + it("logs nothing for a refusal on any other path", () => { + vi.stubEnv("DEMO_MODE", "true"); + proxy( + new NextRequest("https://health.example.test/api/measurements", { + method: "POST", + body: "{}", + }), + ); + expect(warn).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/notifications/action/[token]/route.ts b/src/app/api/notifications/action/[token]/route.ts index cbf074576..d74b5fc1f 100644 --- a/src/app/api/notifications/action/[token]/route.ts +++ b/src/app/api/notifications/action/[token]/route.ts @@ -71,6 +71,10 @@ export const POST = apiHandler( RATE_WINDOW_MS, ); if (!rl.allowed) { + annotate({ + action: { name: "notification.action.refused" }, + meta: { reason: "rate_limited" }, + }); const response = apiError("Too many requests", 429); for (const [k, v] of Object.entries(rateLimitHeaders(rl))) { response.headers.set(k, v); diff --git a/src/proxy.ts b/src/proxy.ts index b973d8c2f..3389400b3 100644 --- a/src/proxy.ts +++ b/src/proxy.ts @@ -310,6 +310,46 @@ function applyBaselineSecurityHeaders(response: NextResponse): NextResponse { return response; } +/** + * The notification quick-answer route (`/api/notifications/action/`). + * Its caller is a notification app on a phone, not a browser, so when a tap + * does nothing the operator's only evidence is the server log. Every refusal + * this proxy makes on that path therefore writes one line with a reason, + * because the route's own wide event never runs for them. The token is never + * written: the path is logged as the prefix plus a placeholder. + */ +const NOTIFICATION_ACTION_PREFIX = "/api/notifications/action/"; + +type NotificationActionRefusal = + "method_not_allowed" | "worker_only" | "demo_mode"; + +function logNotificationActionRefusal( + request: NextRequest, + reason: NotificationActionRefusal, + status: number, +): void { + if (!request.nextUrl.pathname.startsWith(NOTIFICATION_ACTION_PREFIX)) return; + console.warn( + JSON.stringify({ + timestamp: new Date().toISOString(), + level: "warn", + kind: "http", + message: + reason === "method_not_allowed" + ? "Notification answer refused: the route takes POST only. A redirect in front of the app (http to https, or a sign-in gateway) turns the POST into a GET." + : "Notification answer refused before the route ran.", + action: { name: "notification.action.refused" }, + meta: { reason }, + http: { + method: request.method, + path: `${NOTIFICATION_ACTION_PREFIX}[REDACTED]`, + status, + user_agent: request.headers.get("user-agent") ?? undefined, + }, + }), + ); +} + export function proxy(request: NextRequest) { const { pathname } = request.nextUrl; @@ -317,6 +357,7 @@ export function proxy(request: NextRequest) { // serving requests that would either crash (no DB pool ready for writes from // the worker) or duplicate work that the dedicated web container is doing. if (!shouldRunWeb() && pathname !== "/api/health") { + logNotificationActionRefusal(request, "worker_only", 503); return applyBaselineSecurityHeaders( NextResponse.json( { @@ -364,6 +405,29 @@ export function proxy(request: NextRequest) { ); } + // A notification quick answer is a POST and nothing else. ntfy's `http` + // action sends one; anything else here is a request a redirect rewrote (an + // HTTP client follows a 301/302 with a GET) or a stray probe. Next would + // answer 405 on its own, but before any wide event, so the refusal is made + // here where it can be logged. `OPTIONS` stays with Next. + if ( + pathname.startsWith(NOTIFICATION_ACTION_PREFIX) && + request.method !== "POST" && + request.method !== "OPTIONS" + ) { + logNotificationActionRefusal(request, "method_not_allowed", 405); + return applyBaselineSecurityHeaders( + NextResponse.json( + { + data: null, + error: "Method not allowed", + meta: { errorCode: "notification.action.method_not_allowed" }, + }, + { status: 405, headers: { Allow: "POST" } }, + ), + ); + } + // Demo mode: block all mutations except login if (process.env.DEMO_MODE === "true") { const method = request.method.toUpperCase(); @@ -377,6 +441,7 @@ export function proxy(request: NextRequest) { (entry) => pathname === entry.path && method === entry.method, ) ) { + logNotificationActionRefusal(request, "demo_mode", 403); return applyBaselineSecurityHeaders( NextResponse.json(DEMO_REFUSAL_BODY, { status: 403 }), ); diff --git a/tests/integration/notification-action-mood.test.ts b/tests/integration/notification-action-mood.test.ts index 0806e8390..59fd707f7 100644 --- a/tests/integration/notification-action-mood.test.ts +++ b/tests/integration/notification-action-mood.test.ts @@ -116,3 +116,86 @@ describe("POST /api/notifications/action/{token}", () => { ).toBe(0); }); }); + +/** + * #1203 — the request in ntfy's own shape, from the published message to the + * row. The action is taken from the JSON body `buildNtfyMessage` publishes, + * and sent the way ntfy's Android app sends an `http` action: POST, empty + * body, no cookie, no Origin, an okhttp user agent. It goes through the real + * proxy first and then the real route, with the route param taken from the + * URL path the way Next derives it. + */ +describe("a ntfy http action through the proxy and the route", () => { + async function ntfyAction() { + const { buildMoodQuickActions } = + await import("@/lib/notifications/action-token"); + const { buildNtfyMessage } = + await import("@/lib/notifications/senders/ntfy"); + const quickActions = buildMoodQuickActions({ + userId: USER, + origin: "https://health.example.test", + labels: { "mood-good": "Good", "mood-okay": "Okay", "mood-bad": "Bad" }, + }); + const message = buildNtfyMessage("topic", { + userId: USER, + eventType: "MOOD_REMINDER", + title: "Mood", + message: "How are you?", + quickActions, + } as Parameters[1]); + // Round-trip through JSON: what the phone holds is the parsed publish body. + const published = JSON.parse(JSON.stringify(message)) as typeof message; + const okay = published.actions?.find((a) => a.label === "Okay"); + if (!okay) throw new Error("no Okay action in the published message"); + return okay; + } + + async function send(method: string, url: string) { + const { proxy } = await import("@/proxy"); + const request = new NextRequest(url, { + method, + headers: { "user-agent": "okhttp/4.12.0", "content-length": "0" }, + ...(method === "POST" ? { body: "" } : {}), + }); + const edge = proxy(request); + if (edge.headers.get("x-middleware-next") !== "1") return edge; + const token = decodeURIComponent( + new URL(url).pathname.split("/").pop() ?? "", + ); + const { POST } = + await import("@/app/api/notifications/action/[token]/route"); + return POST(request, { params: Promise.resolve({ token }) }); + } + + it("logs the mood when the phone POSTs the published action", async () => { + const action = await ntfyAction(); + expect(action).toMatchObject({ action: "http", method: "POST" }); + + const res = await send(action.method, action.url); + expect(res.status).toBe(200); + expect(await res.json()).toMatchObject({ + data: { logged: true, action: "mood-okay" }, + }); + const rows = await getPrismaClient().moodEntry.findMany({ + where: { userId: USER }, + select: { source: true, score: true }, + }); + expect(rows).toEqual([{ source: "NOTIFICATION", score: 3 }]); + }); + + it("refuses a GET (a POST a redirect rewrote) at the proxy and writes nothing", async () => { + const action = await ntfyAction(); + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + try { + const res = await send("GET", action.url); + expect(res.status).toBe(405); + expect(warn).toHaveBeenCalledTimes(1); + expect(String(warn.mock.calls[0][0])).toContain("method_not_allowed"); + } finally { + warn.mockRestore(); + } + expect( + await getPrismaClient().moodEntry.count({ where: { userId: USER } }), + ).toBe(0); + }); +});