From 8d2753f942cd7af83113eb89ccd2fde0405eb734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 02:02:04 +0200 Subject: [PATCH 1/5] fix(notifications): apply the global channel switches at dispatch `AppSettings.telegramGlobal`, `.ntfyGlobal` and `.webPushGlobal` were read in two places, and neither of them was the one that matters: the setup-card visibility on Settings -> Integrations and an echo in the preferences payload. The dispatcher never looked at them. An operator who switched a channel off instance-wide lost the setup card and kept every existing delivery on that channel, indefinitely. Gate each of the three in the dispatcher, in the shape `isApiGloballyEnabled()` already uses for the twelve API routes. The read sits above the per-user channel row and below the per-event preference, it happens at most once per dispatch, and only when a switchable channel is actually in the cascade. A skip writes a `globally_disabled` row to the push-attempt ledger so the admin diagnostics name the cause, and does not count as an attempt, so `dispatched` keeps telling the truth. The account's own channel row is left enabled: the switch is the operator's, and flipping it back has to restore delivery without every user re-enabling anything. APNs, the generic webhook and email carry no instance-wide switch and are not gated here; they are turned off at their source instead. The unit test drives the real dispatcher over a stubbed settings row; the integration test writes the actual singleton and reads the ledger back out of Postgres, which is the layer that proves the column the admin route writes is the column the dispatcher reads. --- src/lib/app-settings.ts | 50 +++++ .../dispatcher-global-channel-switch.test.ts | 201 ++++++++++++++++++ src/lib/notifications/dispatcher.ts | 56 ++++- ...notification-global-channel-switch.test.ts | 138 ++++++++++++ 4 files changed, 441 insertions(+), 4 deletions(-) create mode 100644 src/lib/notifications/__tests__/dispatcher-global-channel-switch.test.ts create mode 100644 tests/integration/notification-global-channel-switch.test.ts diff --git a/src/lib/app-settings.ts b/src/lib/app-settings.ts index b210675f8..0c8752e5a 100644 --- a/src/lib/app-settings.ts +++ b/src/lib/app-settings.ts @@ -42,6 +42,56 @@ export async function isApiGloballyEnabled(): Promise { return settings.apiGlobal; } +/** + * The notification channels an operator can switch off instance-wide, mapped + * to the `AppSettings` column that carries the switch. A channel absent from + * this map (APNs, generic webhook, email) has no instance-wide switch and is + * never gated by it — the operator turns those off at their source (the APNs + * credentials, the SMTP env) instead. + */ +const GLOBAL_CHANNEL_SWITCHES = { + TELEGRAM: "telegramGlobal", + NTFY: "ntfyGlobal", + WEB_PUSH: "webPushGlobal", +} as const; + +export type GloballySwitchableChannel = keyof typeof GLOBAL_CHANNEL_SWITCHES; + +export function hasGlobalChannelSwitch( + channelType: string, +): channelType is GloballySwitchableChannel { + return Object.hasOwn(GLOBAL_CHANNEL_SWITCHES, channelType); +} + +/** + * Resolve one channel's instance-wide switch out of an availability snapshot + * the caller already holds. Split from the async form so a dispatch that + * walks several channels reads the settings row once. + */ +export function resolveChannelGloballyEnabled( + availability: GlobalServiceAvailability, + channelType: string, +): boolean { + if (!hasGlobalChannelSwitch(channelType)) return true; + return availability[GLOBAL_CHANNEL_SWITCHES[channelType]]; +} + +/** + * Whether a notification channel may deliver on this instance. Same shape and + * same posture as `isApiGloballyEnabled()`: the switch is the operator's and + * sits above every per-user channel setting, and an unreadable settings row + * resolves to "enabled" so a storage blip never silently mutes an instance. + */ +export async function isChannelGloballyEnabled( + channelType: string, +): Promise { + if (!hasGlobalChannelSwitch(channelType)) return true; + return resolveChannelGloballyEnabled( + await getGlobalServiceAvailability(), + channelType, + ); +} + export interface ReminderThresholds { lateMinutes: number; missedMinutes: number; diff --git a/src/lib/notifications/__tests__/dispatcher-global-channel-switch.test.ts b/src/lib/notifications/__tests__/dispatcher-global-channel-switch.test.ts new file mode 100644 index 000000000..5dbf4434c --- /dev/null +++ b/src/lib/notifications/__tests__/dispatcher-global-channel-switch.test.ts @@ -0,0 +1,201 @@ +/** + * The operator's instance-wide channel switches, asserted where they have to + * hold: at dispatch. + * + * `AppSettings.telegramGlobal` / `.ntfyGlobal` / `.webPushGlobal` used to + * decide only whether the setup card rendered under Settings → Integrations. + * A channel a self-hoster had switched off kept delivering to every account + * that had already configured it. These tests drive the real dispatcher over + * a stubbed settings row and assert the user-visible outcome: nothing is + * sent, and the push-attempt ledger says why. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + appSettings: { findUnique: vi.fn() }, + notificationChannel: { findMany: vi.fn(), update: vi.fn() }, + pushAttempt: { create: vi.fn() }, + user: { findUnique: vi.fn() }, + }, +})); + +vi.mock("@/lib/auth/audit", () => ({ auditLog: vi.fn(async () => undefined) })); + +vi.mock("@/lib/crypto", () => ({ + encrypt: (s: string) => s, + decrypt: (s: string) => s, +})); + +vi.mock("@/lib/logging/context", () => ({ + getEvent: () => ({ + addWarning: vi.fn(), + addMeta: vi.fn(), + addExternalCall: vi.fn(), + setError: vi.fn(), + }), +})); + +const sendViaTelegramMock = vi.fn(); +const sendViaNtfyMock = vi.fn(); +const sendViaWebPushMock = vi.fn(); +const sendViaEmailMock = vi.fn(); + +vi.mock("@/lib/notifications/senders/email", () => ({ + sendViaEmail: (...args: unknown[]) => sendViaEmailMock(...args), +})); + +vi.mock("@/lib/notifications/senders/telegram", () => ({ + sendViaTelegram: (...args: unknown[]) => sendViaTelegramMock(...args), +})); +vi.mock("@/lib/notifications/senders/ntfy", () => ({ + sendViaNtfy: (...args: unknown[]) => sendViaNtfyMock(...args), +})); +vi.mock("@/lib/notifications/senders/web-push", () => ({ + sendViaWebPush: (...args: unknown[]) => sendViaWebPushMock(...args), +})); + +import { prisma } from "@/lib/db"; +import { dispatchNotification } from "@/lib/notifications/dispatcher"; + +type ChannelKind = "TELEGRAM" | "NTFY" | "WEB_PUSH" | "EMAIL"; + +function makeChannel(type: ChannelKind) { + return { + id: `ch-${type}`, + userId: "u-1", + type, + enabled: true, + config: JSON.stringify({ serverUrl: "https://ntfy.example", topic: "t" }), + consecutiveFailures: 0, + nextRetryAt: null, + preferences: [], + }; +} + +function stubSettings(over: Record = {}) { + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + telegramGlobal: true, + ntfyGlobal: true, + webPushGlobal: true, + apiGlobal: true, + ...over, + } as never); +} + +const SENDERS: Record> = { + TELEGRAM: sendViaTelegramMock, + NTFY: sendViaNtfyMock, + WEB_PUSH: sendViaWebPushMock, + EMAIL: sendViaEmailMock, +}; + +const SWITCH_COLUMN = { + TELEGRAM: "telegramGlobal", + NTFY: "ntfyGlobal", + WEB_PUSH: "webPushGlobal", +} as const; + +beforeEach(() => { + vi.resetAllMocks(); + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + managedProfileAt: null, + } as never); + vi.mocked(prisma.notificationChannel.update).mockResolvedValue({ + consecutiveFailures: 0, + } as never); + vi.mocked(prisma.pushAttempt.create).mockResolvedValue({} as never); + stubSettings(); +}); + +describe.each(["TELEGRAM", "NTFY", "WEB_PUSH"] as const)( + "dispatchNotification — %s instance-wide switch", + (type) => { + it("sends nothing and writes a globally_disabled ledger row when the operator switched the channel off", async () => { + stubSettings({ [SWITCH_COLUMN[type]]: false }); + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValue([ + makeChannel(type), + ] as never); + + const outcome = await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + expect(SENDERS[type]).not.toHaveBeenCalled(); + expect(outcome).toEqual({ + dispatched: false, + channelsAttempted: 0, + channelsSucceeded: 0, + }); + expect(prisma.pushAttempt.create).toHaveBeenCalledTimes(1); + expect( + vi.mocked(prisma.pushAttempt.create).mock.calls[0][0], + ).toMatchObject({ + data: { + channel: type, + eventType: "SYSTEM_ALERT", + result: "skipped", + reason: "globally_disabled", + }, + }); + }); + + it("sends normally when the switch is on", async () => { + stubSettings({ [SWITCH_COLUMN[type]]: true }); + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValue([ + makeChannel(type), + ] as never); + SENDERS[type].mockResolvedValue({ ok: true }); + + const outcome = await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + expect(SENDERS[type]).toHaveBeenCalledTimes(1); + expect(outcome.dispatched).toBe(true); + expect(outcome.channelsSucceeded).toBe(1); + }); + }, +); + +describe("dispatchNotification — switch scope", () => { + it("reads the settings row once for a cascade that carries several switchable channels", async () => { + stubSettings({ telegramGlobal: false, ntfyGlobal: false }); + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValue([ + makeChannel("TELEGRAM"), + makeChannel("NTFY"), + ] as never); + + await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + expect(sendViaTelegramMock).not.toHaveBeenCalled(); + expect(sendViaNtfyMock).not.toHaveBeenCalled(); + expect(prisma.appSettings.findUnique).toHaveBeenCalledTimes(1); + }); + + it("never reads the settings row for a cascade with no switchable channel", async () => { + vi.mocked(prisma.notificationChannel.findMany).mockResolvedValue([ + makeChannel("EMAIL"), + ] as never); + + await dispatchNotification({ + eventType: "SYSTEM_ALERT", + userId: "u-1", + title: "t", + message: "m", + }); + + expect(prisma.appSettings.findUnique).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/notifications/dispatcher.ts b/src/lib/notifications/dispatcher.ts index 4bc30dff2..2d8f9d793 100644 --- a/src/lib/notifications/dispatcher.ts +++ b/src/lib/notifications/dispatcher.ts @@ -25,6 +25,12 @@ import { recordChannelTransientFailure, } from "@/lib/notifications/channel-state"; import { getEvent } from "@/lib/logging/context"; +import { + getGlobalServiceAvailability, + hasGlobalChannelSwitch, + resolveChannelGloballyEnabled, + type GlobalServiceAvailability, +} from "@/lib/app-settings"; import { CLIENT_MANAGED_APNS_EVENTS, hasClientManagedApnsGate, @@ -43,11 +49,12 @@ import { * * For each channel: * 1. Skip if `enabled=false` (manually or auto-disabled). - * 2. Skip if currently in retry-cooldown (`nextRetryAt > now`). - * 3. Check the per-channel preference for this eventType (default: enabled). - * 4. Skip the APNs channel — and only that one — when the user's iOS + * 2. Check the per-channel preference for this eventType (default: enabled). + * 3. Skip if the operator switched the channel off instance-wide. + * 4. Skip if currently in retry-cooldown (`nextRetryAt > now`). + * 5. Skip the APNs channel — and only that one — when the user's iOS * client owns the local reminder for this event type. - * 5. Call the appropriate sender — which now returns a `SendOutcome` so + * 6. Call the appropriate sender — which now returns a `SendOutcome` so * the dispatcher can classify hard rejects (410, blocked-by-user) vs * soft errors (5xx, 429, network) and update channel state accordingly. * @@ -231,6 +238,16 @@ export async function dispatchNotification( }); defaultEnabled = user?.moodReminderEnabled === true; } + // Operator kill switches for the three channels that carry one, read at + // most once per dispatch and only when such a channel is actually in the + // cascade — a cascade of APNs + webhook + email pays nothing for it. + let globalAvailability: GlobalServiceAvailability | null = null; + const resolveGlobalAvailability = + async (): Promise => { + globalAvailability ??= await getGlobalServiceAvailability(); + return globalAvailability; + }; + // Client-managed APNs suppression, resolved at most once per dispatch // and only when an APNs channel is actually in the cascade. `null` // means "not applicable"; the read is lazy so the common event types @@ -259,6 +276,37 @@ export async function dispatchNotification( continue; } + // The operator's instance-wide switch for this channel + // (`AppSettings.{telegram,ntfy,webPush}Global`). It sits above every + // per-user setting: a channel switched off here delivers nothing, no + // matter what the account has configured. Not counted as an attempt — + // nothing was sent — but the ledger carries a `globally_disabled` row + // so the operator reads a reason instead of inferring silence. + const globalSwitch = hasGlobalChannelSwitch(channel.type) + ? channel.type + : null; + if ( + globalSwitch && + !resolveChannelGloballyEnabled( + await resolveGlobalAvailability(), + globalSwitch, + ) + ) { + getEvent()?.addMeta( + `notification_skip_${globalSwitch.toLowerCase()}_globally_disabled`, + recipientPayload.eventType, + ); + recordPushAttempt({ + recordUserId: delivery.recordUserId, + recipientUserId: delivery.recipientUserId, + channel: globalSwitch, + eventType: recipientPayload.eventType, + result: "skipped", + reason: "globally_disabled", + }); + continue; + } + // Backoff cooldown — skip the channel until `nextRetryAt`. Without // this guard, a flapping upstream would burn API quota every // reminder-tick (every 60s by default), defeating the whole point diff --git a/tests/integration/notification-global-channel-switch.test.ts b/tests/integration/notification-global-channel-switch.test.ts new file mode 100644 index 000000000..43b01fae7 --- /dev/null +++ b/tests/integration/notification-global-channel-switch.test.ts @@ -0,0 +1,138 @@ +/** + * The operator's instance-wide channel switches, against a real settings row. + * + * The unit suite stubs `prisma.appSettings.findUnique`; this one writes the + * actual `app_settings` singleton, dispatches through the real cascade, and + * reads the `push_attempts` ledger back out of Postgres. It is the layer that + * proves the column the admin route writes is the column the dispatcher + * reads — a unit mock would agree with itself even if those two drifted apart. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +process.env.ENCRYPTION_KEY ??= + "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"; + +const ntfySendMock = vi.fn(); + +vi.mock("@/lib/notifications/senders/ntfy", () => ({ + sendViaNtfy: (...args: unknown[]) => ntfySendMock(...args), +})); + +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); + +import { getPrismaClient, truncateAllTables } from "./setup"; + +const TEST_USER_ID = "user-global-channel-switch"; + +async function seedNtfyChannel(): Promise { + const { encrypt } = await import("@/lib/crypto"); + await getPrismaClient().notificationChannel.create({ + data: { + userId: TEST_USER_ID, + type: "NTFY", + enabled: true, + config: encrypt( + JSON.stringify({ serverUrl: "https://ntfy.example", topic: "health" }), + ), + }, + }); +} + +async function setNtfyGlobal(ntfyGlobal: boolean): Promise { + await getPrismaClient().appSettings.upsert({ + where: { id: "singleton" }, + create: { id: "singleton", ntfyGlobal }, + update: { ntfyGlobal }, + }); +} + +async function dispatch() { + const { dispatchNotification } = + await import("@/lib/notifications/dispatcher"); + return dispatchNotification({ + userId: TEST_USER_ID, + eventType: "MEASUREMENT_REMINDER", + title: "Time to weigh in", + message: "Your morning reading is due", + }); +} + +/** The ledger write is fire-and-forget; let its microtask land. */ +async function settleLedger(): Promise { + await new Promise((resolve) => setTimeout(resolve, 50)); +} + +beforeEach(async () => { + vi.clearAllMocks(); + await truncateAllTables(getPrismaClient()); + await getPrismaClient().user.create({ + data: { + id: TEST_USER_ID, + username: "global-channel-switch", + email: "global-channel-switch@example.test", + }, + }); +}); + +describe("instance-wide notification channel switch", () => { + it("delivers nothing and records the skip when the operator switched ntfy off", async () => { + await seedNtfyChannel(); + await setNtfyGlobal(false); + + const outcome = await dispatch(); + await settleLedger(); + + expect(ntfySendMock).not.toHaveBeenCalled(); + expect(outcome).toEqual({ + dispatched: false, + channelsAttempted: 0, + channelsSucceeded: 0, + }); + + const attempts = await getPrismaClient().pushAttempt.findMany({ + where: { userId: TEST_USER_ID }, + }); + expect(attempts).toHaveLength(1); + expect(attempts[0]).toMatchObject({ + channel: "NTFY", + eventType: "MEASUREMENT_REMINDER", + result: "skipped", + reason: "globally_disabled", + }); + + // The switch is the operator's, not the user's: the account's own + // channel row is left enabled so flipping the switch back restores + // delivery without every user having to re-enable anything. + const channel = await getPrismaClient().notificationChannel.findFirst({ + where: { userId: TEST_USER_ID, type: "NTFY" }, + }); + expect(channel?.enabled).toBe(true); + }); + + it("delivers on the same channel once the switch is back on", async () => { + await seedNtfyChannel(); + await setNtfyGlobal(true); + ntfySendMock.mockResolvedValueOnce({ ok: true }); + + const outcome = await dispatch(); + + expect(ntfySendMock).toHaveBeenCalledTimes(1); + expect(outcome).toEqual({ + dispatched: true, + channelsAttempted: 1, + channelsSucceeded: 1, + }); + }); + + it("delivers when no settings row exists at all", async () => { + await seedNtfyChannel(); + ntfySendMock.mockResolvedValueOnce({ ok: true }); + + const outcome = await dispatch(); + + expect(ntfySendMock).toHaveBeenCalledTimes(1); + expect(outcome.dispatched).toBe(true); + }); +}); From de0876b2159f38fc35f84478e419e645ffd474a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 02:02:15 +0200 Subject: [PATCH 2/5] fix(settings): refuse enabling a channel the operator switched off MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the switches now enforced at dispatch, a user could still flip a channel on, see the toggle stick, and get nothing. Refuse the enable instead, with 403 and a message that names the reason: the ntfy and Telegram config endpoints on both accepted body shapes, and the Web Push subscribe endpoint, which is the enable step for that channel because it creates the channel row. Telegram is checked before the webhook registration, so a refused enable makes no outbound call and stores nothing. Disabling a channel keeps working while the switch is off — a user must always be able to turn a channel off, whatever the operator has done. The three refusals are published in the contract; `/api/settings/global-services` already exposes the switches so a client can hide the control instead of provoking the refusal. --- docs/api/openapi.yaml | 51 +++- .../web-push/__tests__/route.test.ts | 15 +- src/app/api/notifications/web-push/route.ts | 13 + .../channel-global-switch-refusal.test.ts | 226 ++++++++++++++++++ src/app/api/settings/ntfy/route.ts | 25 ++ src/app/api/settings/telegram/route.ts | 25 ++ .../openapi/routes/notifications-transport.ts | 8 +- src/lib/openapi/routes/settings.ts | 16 +- 8 files changed, 368 insertions(+), 11 deletions(-) create mode 100644 src/app/api/settings/__tests__/channel-global-switch-refusal.test.ts diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 935b12cad..2e84394bf 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -11763,9 +11763,16 @@ paths: tags: - Notifications summary: Update the caller's ntfy channel config - description: "Upserts the channel. Accepts either the toggle-only body or the full config. The response is `{ saved: - true }` and carries no config echo, so re-read the GET if the client needs the resolved state. Auth via cookie - or Bearer." + description: >- + Upserts the channel. Accepts either the toggle-only body or the full config. The response is `{ saved: true }` + and carries no config echo, so re-read the GET if the client needs the resolved state. Auth via cookie or + Bearer. + + + An operator can switch ntfy off for the whole instance. While that switch is off, a body that sets `enabled` to + true is refused with 403 and nothing is stored — the channel would not deliver, so the toggle does not pretend + otherwise. Disabling the channel keeps working either way. Read the switch from `GET + /api/settings/global-services`. requestBody: required: true content: @@ -11786,6 +11793,12 @@ paths: schema: $ref: "#/components/schemas/ErrorEnvelope" "401": *a1 + "403": + description: The operator switched ntfy off for this instance and the body asked to enable it. Nothing was stored. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "413": &a16 description: Body exceeds 64 KiB. content: @@ -11827,10 +11840,16 @@ paths: tags: - Notifications summary: Update the caller's Telegram channel config - description: "Accepts either the toggle-only body or the full config, then registers (on enable) or deletes (on disable) - the Telegram webhook for the bot before persisting. Webhook deletion is best-effort and its failure does not - fail the write; webhook REGISTRATION is not — a rejected registration aborts the whole call and nothing is - stored. The response is `{ updated: true }` with no config echo. Auth via cookie or Bearer." + description: >- + Accepts either the toggle-only body or the full config, then registers (on enable) or deletes (on disable) the + Telegram webhook for the bot before persisting. Webhook deletion is best-effort and its failure does not fail + the write; webhook REGISTRATION is not — a rejected registration aborts the whole call and nothing is stored. + The response is `{ updated: true }` with no config echo. Auth via cookie or Bearer. + + + An operator can switch Telegram off for the whole instance. While that switch is off, a body that sets `enabled` + to true is refused with 403 before any webhook call and nothing is stored. Disabling the channel keeps working + either way. Read the switch from `GET /api/settings/global-services`. requestBody: required: true content: @@ -11846,6 +11865,13 @@ paths: $ref: "#/components/schemas/PutTelegramSettingsResponse" "400": *a15 "401": *a1 + "403": + description: The operator switched Telegram off for this instance and the body asked to enable it. No webhook was + registered and nothing was stored. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "404": description: The session's user row no longer exists. content: @@ -18649,6 +18675,11 @@ paths: 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. + + + An operator can switch Web Push off for the whole instance. While that switch is off this answers 403 before it + reads the body, and no subscription and no channel row are created. Read the switch from `GET + /api/settings/global-services` and hide the subscribe control rather than retrying. requestBody: required: true content: @@ -18663,6 +18694,12 @@ paths: schema: $ref: "#/components/schemas/WebPushSubscribeEnvelope" "401": *a1 + "403": + description: The operator switched Web Push off for this instance. Nothing was stored. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorEnvelope" "413": description: Request body exceeds 65536 bytes. Nothing was stored. content: diff --git a/src/app/api/notifications/web-push/__tests__/route.test.ts b/src/app/api/notifications/web-push/__tests__/route.test.ts index 516cd7838..237e76856 100644 --- a/src/app/api/notifications/web-push/__tests__/route.test.ts +++ b/src/app/api/notifications/web-push/__tests__/route.test.ts @@ -26,12 +26,19 @@ vi.mock("@/lib/api-handler", () => ({ vi.mock("@/lib/db", () => ({ prisma: { + // The route reads the operator's instance-wide Web Push switch before it + // touches the body; the default stub leaves it on so these cases keep + // asserting the validation refusals they were written for. + appSettings: { findUnique: vi.fn() }, 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/logging/context", () => ({ + annotate: vi.fn(), + getEvent: vi.fn(() => null), +})); vi.mock("@/lib/crypto", () => ({ encrypt: (v: string) => `enc(${v})` })); @@ -67,6 +74,12 @@ type Handler = (r: Request) => Promise; beforeEach(() => { vi.resetAllMocks(); vi.mocked(requireAuth).mockResolvedValue({ user: USER } as never); + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + telegramGlobal: true, + ntfyGlobal: true, + webPushGlobal: true, + apiGlobal: true, + } as never); }); describe("POST /api/notifications/web-push — the subscribe refusal", () => { diff --git a/src/app/api/notifications/web-push/route.ts b/src/app/api/notifications/web-push/route.ts index 5e64b11cf..87447ea13 100644 --- a/src/app/api/notifications/web-push/route.ts +++ b/src/app/api/notifications/web-push/route.ts @@ -1,6 +1,7 @@ import { NextRequest } from "next/server"; import { prisma } from "@/lib/db"; import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { isChannelGloballyEnabled } from "@/lib/app-settings"; import { annotate } from "@/lib/logging/context"; import { apiSuccess, apiError, returnAllZodIssues } from "@/lib/api-response"; import { encrypt } from "@/lib/crypto"; @@ -43,6 +44,18 @@ export const POST = apiHandler(async (request: NextRequest) => { const { user } = await requireAuth(); annotate({ action: { name: "notifications.web-push.subscribe" } }); + // Subscribing IS the enable step for this channel — it creates the + // WEB_PUSH `NotificationChannel` row. An operator who switched Web Push + // off instance-wide (`AppSettings.webPushGlobal`) gets a refusal that says + // so rather than a subscription that stores fine and delivers nothing. + if (!(await isChannelGloballyEnabled("WEB_PUSH"))) { + annotate({ meta: { refused: "globally_disabled" } }); + return apiError( + "Web Push is disabled on this instance by the operator", + 403, + ); + } + let body: unknown; try { const raw = await request.text(); diff --git a/src/app/api/settings/__tests__/channel-global-switch-refusal.test.ts b/src/app/api/settings/__tests__/channel-global-switch-refusal.test.ts new file mode 100644 index 000000000..124a4a6cb --- /dev/null +++ b/src/app/api/settings/__tests__/channel-global-switch-refusal.test.ts @@ -0,0 +1,226 @@ +/** + * Enabling a channel the operator switched off instance-wide is refused. + * + * The three switches (`AppSettings.telegramGlobal` / `.ntfyGlobal` / + * `.webPushGlobal`) now stop delivery in the dispatcher. Without a matching + * refusal on the way in, a user would still see the toggle flip, the row + * would still say `enabled = true`, and nothing would ever arrive. These + * tests pin the refusal and the absence of any write behind it. + */ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + appSettings: { findUnique: vi.fn() }, + notificationChannel: { + findUnique: vi.fn(), + findFirst: vi.fn(), + create: vi.fn(), + updateMany: vi.fn(), + upsert: vi.fn(), + deleteMany: vi.fn(), + }, + pushSubscription: { upsert: vi.fn() }, + user: { findUnique: vi.fn(), update: vi.fn() }, + }, +})); + +vi.mock("@/lib/crypto", () => ({ + encrypt: vi.fn((value: string) => `encrypted:${value}`), + decrypt: vi.fn((value: string) => value.replace(/^encrypted:/, "")), +})); + +const setTelegramWebhookMock = vi.fn().mockResolvedValue(true); +vi.mock("@/lib/telegram", () => ({ + setTelegramWebhook: (...args: unknown[]) => setTelegramWebhookMock(...args), + deleteTelegramWebhook: vi.fn().mockResolvedValue(true), +})); + +vi.mock("@/lib/auth/session", () => ({ getSession: vi.fn() })); +vi.mock("@/lib/auth/audit", () => ({ + auditLog: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/db-compat", () => ({ + ensureDbCompatibility: vi.fn().mockResolvedValue(undefined), +})); +vi.mock("@/lib/rate-limit", () => ({ + checkRateLimit: vi.fn().mockResolvedValue({ + allowed: true, + remaining: 59, + resetAt: Date.now() + 60_000, + }), + rateLimitHeaders: () => ({}), +})); +vi.mock("next/headers", () => ({ + headers: vi.fn(async () => ({ get: () => null })), + cookies: vi.fn(async () => ({ + get: () => undefined, + set: () => {}, + delete: () => {}, + })), +})); + +import { PUT as putNtfy } from "../ntfy/route"; +import { PUT as putTelegram } from "../telegram/route"; +import { POST as postWebPush } from "../../notifications/web-push/route"; +import { prisma } from "@/lib/db"; +import { getSession } from "@/lib/auth/session"; + +const SESSION_OK = { + session: { id: "sess-1", expiresAt: new Date(Date.now() + 3_600_000) }, + user: { id: "user-1", username: "testuser", role: "USER" as const }, +}; + +const WEB_PUSH_SUBSCRIPTION = { + endpoint: "https://push.example.com/subscription/abc", + keys: { p256dh: "a".repeat(87), auth: "b".repeat(22) }, +}; + +function stubSettings(over: Record) { + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + telegramGlobal: true, + ntfyGlobal: true, + webPushGlobal: true, + apiGlobal: true, + ...over, + } as never); +} + +function request(path: string, method: string, body: unknown) { + return new Request(`http://localhost${path}`, { + method, + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); +} + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(getSession).mockResolvedValue(SESSION_OK as never); + vi.mocked(prisma.notificationChannel.updateMany).mockResolvedValue({ + count: 1, + } as never); + vi.mocked(prisma.notificationChannel.upsert).mockResolvedValue({} as never); + vi.mocked(prisma.notificationChannel.findUnique).mockResolvedValue({ + config: 'encrypted:{"serverUrl":"https://ntfy.sh","topic":"saved-topic"}', + } as never); + vi.mocked(prisma.user.findUnique).mockResolvedValue({ + telegramBotToken: "encrypted:saved-token", + telegramChatId: "saved-chat", + telegramEnabled: false, + } as never); + vi.mocked(prisma.user.update).mockResolvedValue({} as never); + vi.mocked(prisma.pushSubscription.upsert).mockResolvedValue({} as never); + vi.mocked(prisma.notificationChannel.findFirst).mockResolvedValue(null); +}); + +describe("ntfy enable against the instance-wide switch", () => { + it("refuses with 403 and writes nothing when the operator switched ntfy off", async () => { + stubSettings({ ntfyGlobal: false }); + + const response = await putNtfy( + request("/api/settings/ntfy", "PUT", { enabled: true }) as never, + ); + + expect(response.status).toBe(403); + await expect(response.json()).resolves.toMatchObject({ + data: null, + error: expect.stringContaining("disabled on this instance"), + }); + expect(prisma.notificationChannel.updateMany).not.toHaveBeenCalled(); + expect(prisma.notificationChannel.upsert).not.toHaveBeenCalled(); + }); + + it("still allows disabling the channel while the switch is off", async () => { + stubSettings({ ntfyGlobal: false }); + + const response = await putNtfy( + request("/api/settings/ntfy", "PUT", { enabled: false }) as never, + ); + + expect(response.status).toBe(200); + expect(prisma.notificationChannel.updateMany).toHaveBeenCalledWith({ + where: { userId: "user-1", type: "NTFY" }, + data: { enabled: false }, + }); + }); + + it("allows enabling while the switch is on", async () => { + stubSettings({ ntfyGlobal: true }); + + const response = await putNtfy( + request("/api/settings/ntfy", "PUT", { enabled: true }) as never, + ); + + expect(response.status).toBe(200); + expect(prisma.notificationChannel.updateMany).toHaveBeenCalledWith({ + where: { userId: "user-1", type: "NTFY" }, + data: { enabled: true }, + }); + }); +}); + +describe("Telegram enable against the instance-wide switch", () => { + it("refuses with 403, registers no webhook, and writes nothing", async () => { + stubSettings({ telegramGlobal: false }); + + const response = await putTelegram( + request("/api/settings/telegram", "PUT", { enabled: true }) as never, + ); + + expect(response.status).toBe(403); + expect(setTelegramWebhookMock).not.toHaveBeenCalled(); + expect(prisma.user.update).not.toHaveBeenCalled(); + expect(prisma.notificationChannel.updateMany).not.toHaveBeenCalled(); + }); + + it("refuses a full-config save that enables the channel", async () => { + stubSettings({ telegramGlobal: false }); + + const response = await putTelegram( + request("/api/settings/telegram", "PUT", { + botToken: "123456:AAaaBBbbCCccDDddEEeeFFffGGgg-HHhhIIii", + chatId: "9876543", + enabled: true, + }) as never, + ); + + expect(response.status).toBe(403); + expect(setTelegramWebhookMock).not.toHaveBeenCalled(); + expect(prisma.user.update).not.toHaveBeenCalled(); + }); +}); + +describe("Web Push subscribe against the instance-wide switch", () => { + it("refuses with 403 and stores no subscription", async () => { + stubSettings({ webPushGlobal: false }); + + const response = await postWebPush( + request( + "/api/notifications/web-push", + "POST", + WEB_PUSH_SUBSCRIPTION, + ) as never, + ); + + expect(response.status).toBe(403); + expect(prisma.pushSubscription.upsert).not.toHaveBeenCalled(); + expect(prisma.notificationChannel.create).not.toHaveBeenCalled(); + }); + + it("stores the subscription while the switch is on", async () => { + stubSettings({ webPushGlobal: true }); + + const response = await postWebPush( + request( + "/api/notifications/web-push", + "POST", + WEB_PUSH_SUBSCRIPTION, + ) as never, + ); + + expect(response.status).toBe(200); + expect(prisma.pushSubscription.upsert).toHaveBeenCalledTimes(1); + }); +}); diff --git a/src/app/api/settings/ntfy/route.ts b/src/app/api/settings/ntfy/route.ts index f9dc4f043..ea54e4c0f 100644 --- a/src/app/api/settings/ntfy/route.ts +++ b/src/app/api/settings/ntfy/route.ts @@ -7,8 +7,17 @@ import { import { encrypt, decrypt } from "@/lib/crypto"; import { NextRequest } from "next/server"; import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { isChannelGloballyEnabled } from "@/lib/app-settings"; import { annotate } from "@/lib/logging/context"; +/** + * Refusal for an enable attempt on a channel the operator switched off + * instance-wide (`AppSettings.ntfyGlobal`). A toggle that appeared to work + * and then delivered nothing is worse than a refusal that says why. + */ +const NTFY_GLOBALLY_DISABLED = + "ntfy is disabled on this instance by the operator"; + /** * GET: Return current ntfy config (without auth token). * PUT: Update ntfy config. @@ -60,6 +69,14 @@ export const PUT = apiHandler(async (request: NextRequest) => { if (enabledOnly.success) { const { enabled } = enabledOnly.data; + if (enabled && !(await isChannelGloballyEnabled("NTFY"))) { + annotate({ + action: { name: "settings.ntfy.update" }, + meta: { refused: "globally_disabled" }, + }); + return apiError(NTFY_GLOBALLY_DISABLED, 403); + } + if (enabled) { const existing = await prisma.notificationChannel.findUnique({ where: { userId_type: { userId: user.id, type: "NTFY" } }, @@ -106,6 +123,14 @@ export const PUT = apiHandler(async (request: NextRequest) => { const { serverUrl, topic, authToken, enabled } = parsed.data; + if (enabled && !(await isChannelGloballyEnabled("NTFY"))) { + annotate({ + action: { name: "settings.ntfy.update" }, + meta: { refused: "globally_disabled" }, + }); + return apiError(NTFY_GLOBALLY_DISABLED, 403); + } + if (enabled && (!serverUrl || !topic)) { return apiError( "Server URL and topic are required when ntfy is enabled", diff --git a/src/app/api/settings/telegram/route.ts b/src/app/api/settings/telegram/route.ts index 2fb9f9085..c837ce010 100644 --- a/src/app/api/settings/telegram/route.ts +++ b/src/app/api/settings/telegram/route.ts @@ -7,8 +7,17 @@ import { notificationChannelEnabledSchema } from "@/lib/validations/notification import { NextRequest } from "next/server"; import { z } from "zod/v4"; import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { isChannelGloballyEnabled } from "@/lib/app-settings"; import { annotate } from "@/lib/logging/context"; +/** + * Refusal for an enable attempt on a channel the operator switched off + * instance-wide (`AppSettings.telegramGlobal`). A toggle that appeared to + * work and then delivered nothing is worse than a refusal that says why. + */ +const TELEGRAM_GLOBALLY_DISABLED = + "Telegram is disabled on this instance by the operator"; + /** * Get Telegram notification settings for the current user. * Never returns the bot token in plaintext. @@ -117,6 +126,14 @@ export const PUT = apiHandler(async (request: NextRequest) => { const enabledOnly = notificationChannelEnabledSchema.safeParse(body); if (enabledOnly.success) { const { enabled } = enabledOnly.data; + if (enabled && !(await isChannelGloballyEnabled("TELEGRAM"))) { + annotate({ + action: { name: "settings.telegram.update" }, + meta: { refused: "globally_disabled" }, + }); + return apiError(TELEGRAM_GLOBALLY_DISABLED, 403); + } + if (enabled && (!current.telegramBotToken || !current.telegramChatId)) { return apiError( "Bot token and chat ID are required when Telegram is enabled", @@ -166,6 +183,14 @@ export const PUT = apiHandler(async (request: NextRequest) => { const hasChatIdAfter = chatId !== undefined ? !!trimmedChatId : !!current.telegramChatId; + if (enabled && !(await isChannelGloballyEnabled("TELEGRAM"))) { + annotate({ + action: { name: "settings.telegram.update" }, + meta: { refused: "globally_disabled" }, + }); + return apiError(TELEGRAM_GLOBALLY_DISABLED, 403); + } + if (enabled && (!hasTokenAfter || !hasChatIdAfter)) { return apiError( "Bot token and chat ID are required when Telegram is enabled", diff --git a/src/lib/openapi/routes/notifications-transport.ts b/src/lib/openapi/routes/notifications-transport.ts index 238b4bf30..9eb27b5b4 100644 --- a/src/lib/openapi/routes/notifications-transport.ts +++ b/src/lib/openapi/routes/notifications-transport.ts @@ -136,7 +136,8 @@ 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 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.", + "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.\n\n" + + "An operator can switch Web Push off for the whole instance. While that switch is off this answers 403 before it reads the body, and no subscription and no channel row are created. Read the switch from `GET /api/settings/global-services` and hide the subscribe control rather than retrying.", requestBody: { required: true, content: { "application/json": { schema: webPushSubscribeRequest } }, @@ -153,6 +154,11 @@ export const notificationTransportPaths: NonNullable< }, }, }, + "403": { + description: + "The operator switched Web Push off for this instance. Nothing was stored.", + content: { "application/json": { schema: errorEnvelope } }, + }, "413": { description: "Request body exceeds 65536 bytes. Nothing was stored.", content: { "application/json": { schema: errorEnvelope } }, diff --git a/src/lib/openapi/routes/settings.ts b/src/lib/openapi/routes/settings.ts index 085fee9ab..5edde619c 100644 --- a/src/lib/openapi/routes/settings.ts +++ b/src/lib/openapi/routes/settings.ts @@ -375,7 +375,8 @@ export const settingsPaths: NonNullable = { tags: ["Notifications"], summary: "Update the caller's ntfy channel config", description: - "Upserts the channel. Accepts either the toggle-only body or the full config. The response is `{ saved: true }` and carries no config echo, so re-read the GET if the client needs the resolved state. Auth via cookie or Bearer.", + "Upserts the channel. Accepts either the toggle-only body or the full config. The response is `{ saved: true }` and carries no config echo, so re-read the GET if the client needs the resolved state. Auth via cookie or Bearer.\n\n" + + "An operator can switch ntfy off for the whole instance. While that switch is off, a body that sets `enabled` to true is refused with 403 and nothing is stored — the channel would not deliver, so the toggle does not pretend otherwise. Disabling the channel keeps working either way. Read the switch from `GET /api/settings/global-services`.", requestBody: { required: true, content: { "application/json": { schema: ntfyPutRequest } }, @@ -389,6 +390,11 @@ export const settingsPaths: NonNullable = { }, }, }, + "403": { + description: + "The operator switched ntfy off for this instance and the body asked to enable it. Nothing was stored.", + content: { "application/json": { schema: errorEnvelope } }, + }, ...jsonBodyRefusals, ...stdResponses, "422": { @@ -424,7 +430,8 @@ export const settingsPaths: NonNullable = { tags: ["Notifications"], summary: "Update the caller's Telegram channel config", description: - "Accepts either the toggle-only body or the full config, then registers (on enable) or deletes (on disable) the Telegram webhook for the bot before persisting. Webhook deletion is best-effort and its failure does not fail the write; webhook REGISTRATION is not — a rejected registration aborts the whole call and nothing is stored. The response is `{ updated: true }` with no config echo. Auth via cookie or Bearer.", + "Accepts either the toggle-only body or the full config, then registers (on enable) or deletes (on disable) the Telegram webhook for the bot before persisting. Webhook deletion is best-effort and its failure does not fail the write; webhook REGISTRATION is not — a rejected registration aborts the whole call and nothing is stored. The response is `{ updated: true }` with no config echo. Auth via cookie or Bearer.\n\n" + + "An operator can switch Telegram off for the whole instance. While that switch is off, a body that sets `enabled` to true is refused with 403 before any webhook call and nothing is stored. Disabling the channel keeps working either way. Read the switch from `GET /api/settings/global-services`.", requestBody: { required: true, content: { "application/json": { schema: telegramPutRequest } }, @@ -441,6 +448,11 @@ export const settingsPaths: NonNullable = { }, }, }, + "403": { + description: + "The operator switched Telegram off for this instance and the body asked to enable it. No webhook was registered and nothing was stored.", + content: { "application/json": { schema: errorEnvelope } }, + }, "404": { description: "The session's user row no longer exists.", content: { "application/json": { schema: errorEnvelope } }, From 4dbad48d89b254f91092236f0b30b6e495378929 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 02:02:22 +0200 Subject: [PATCH 3/5] fix(admin): persist the assistant flags sent to the settings route MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `adminSettingsSchema` validates five `assistant*` booleans and the strict schema refuses anything else, but the route's `booleanFields` list never carried them. Sending one alone answered 422 "No valid fields"; sending one alongside a real field answered 200 and dropped it. The schema comment says the entry exists so an operator scripting their settings over the single route can carry the values, so the route is brought up to the contract rather than the contract cut down to the route. The flags are echoed in the GET and PUT payloads as raw column values — the master-over-sub resolution stays with `/settings/assistant-flags`, which owns that shape and was never affected. --- .../admin/settings/__tests__/route.test.ts | 131 ++++++++++++++++++ src/app/api/admin/settings/route.ts | 28 +++- 2 files changed, 158 insertions(+), 1 deletion(-) diff --git a/src/app/api/admin/settings/__tests__/route.test.ts b/src/app/api/admin/settings/__tests__/route.test.ts index deaae0f41..56d33df68 100644 --- a/src/app/api/admin/settings/__tests__/route.test.ts +++ b/src/app/api/admin/settings/__tests__/route.test.ts @@ -482,3 +482,134 @@ describe("PUT /api/admin/settings", () => { }); }); }); + +/** + * The five assistant flags were accepted by `adminSettingsSchema` and then + * dropped on the floor: sending one alone answered 422 ("No valid fields"), + * sending it alongside a real field answered 200 and stored nothing. The + * schema comment promised the opposite, so the route was brought up to the + * contract rather than the contract down to the route. + */ +describe("PUT /api/admin/settings — assistant flags", () => { + const STORED = { + registrationEnabled: true, + mfaRequired: false, + defaultLocale: "de", + telegramGlobal: true, + ntfyGlobal: true, + webPushGlobal: true, + webPushVapidPublicKey: null, + webPushVapidPrivateKeyEncrypted: null, + webPushVapidSubject: null, + apiGlobal: true, + umamiEnabled: false, + umamiScriptUrl: null, + umamiWebsiteId: null, + glitchtipEnabled: false, + glitchtipDsn: null, + glitchtipEnvironment: null, + reminderLateMinutes: 120, + reminderMissedMinutes: 240, + documentMaxFileBytes: 26_214_400, + documentQuotaBytes: BigInt(1_073_741_824), + defaultUserTimezone: null, + assistantEnabled: true, + assistantCoachEnabled: false, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantCorrelationsEnabled: true, + }; + + beforeEach(() => { + vi.mocked(prisma.appSettings.upsert).mockResolvedValue(STORED as never); + }); + + it("persists a lone assistant flag instead of answering 'No valid fields'", async () => { + const res = await PUT(jsonReq({ assistantCoachEnabled: false })); + + expect(res.status).toBe(200); + expect(prisma.appSettings.upsert).toHaveBeenCalledTimes(1); + const args = vi.mocked(prisma.appSettings.upsert).mock.calls[0]?.[0] as { + update: Record; + }; + expect(args.update).toEqual({ assistantCoachEnabled: false }); + }); + + it("persists an assistant flag sent alongside an unrelated field", async () => { + const res = await PUT( + jsonReq({ registrationEnabled: false, assistantCoachEnabled: false }), + ); + + expect(res.status).toBe(200); + const args = vi.mocked(prisma.appSettings.upsert).mock.calls[0]?.[0] as { + update: Record; + }; + expect(args.update).toEqual({ + registrationEnabled: false, + assistantCoachEnabled: false, + }); + }); + + it("echoes the stored flags so a scripted write reads back", async () => { + const res = await PUT(jsonReq({ assistantCoachEnabled: false })); + const body = (await res.json()) as { data: Record }; + + expect(body.data).toMatchObject({ + assistantEnabled: true, + assistantCoachEnabled: false, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantCorrelationsEnabled: true, + }); + }); + + it("audits the flag it changed", async () => { + await PUT(jsonReq({ assistantCorrelationsEnabled: false })); + + expect(auditLog).toHaveBeenCalledWith( + "admin.settings.update", + expect.objectContaining({ + details: expect.objectContaining({ + assistantCorrelationsEnabled: false, + }), + }), + ); + }); + + it("still refuses an unknown flag-shaped key", async () => { + const res = await PUT(jsonReq({ assistantNotAThing: false })); + expect(res.status).toBe(422); + expect(prisma.appSettings.upsert).not.toHaveBeenCalled(); + }); +}); + +describe("GET /api/admin/settings — assistant flags", () => { + it("defaults every flag to enabled when the row is absent", async () => { + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue(null); + const res = await GET(); + const body = (await res.json()) as { data: Record }; + + expect(body.data).toMatchObject({ + assistantEnabled: true, + assistantCoachEnabled: true, + assistantBriefingEnabled: true, + assistantInsightStatusEnabled: true, + assistantCorrelationsEnabled: true, + }); + }); + + it("surfaces the stored flags", async () => { + vi.mocked(prisma.appSettings.findUnique).mockResolvedValue({ + assistantEnabled: false, + assistantCoachEnabled: false, + } as never); + const res = await GET(); + const body = (await res.json()) as { data: Record }; + + expect(body.data).toMatchObject({ + assistantEnabled: false, + assistantCoachEnabled: false, + assistantBriefingEnabled: true, + }); + }); +}); diff --git a/src/app/api/admin/settings/route.ts b/src/app/api/admin/settings/route.ts index cfb4d787b..bfbaf9b89 100644 --- a/src/app/api/admin/settings/route.ts +++ b/src/app/api/admin/settings/route.ts @@ -61,6 +61,16 @@ export const GET = apiHandler(async () => { // resolver"; surfacing the raw value lets the admin UI render // an empty picker placeholder until they opt in. defaultUserTimezone: settings?.defaultUserTimezone ?? null, + // Raw column values, not the resolved matrix — the master flag is + // applied to the sub-flags by `/settings/assistant-flags`, which owns + // that shape. Echoed here so a write over this route reads back. + assistantEnabled: settings?.assistantEnabled ?? true, + assistantCoachEnabled: settings?.assistantCoachEnabled ?? true, + assistantBriefingEnabled: settings?.assistantBriefingEnabled ?? true, + assistantInsightStatusEnabled: + settings?.assistantInsightStatusEnabled ?? true, + assistantCorrelationsEnabled: + settings?.assistantCorrelationsEnabled ?? true, }); }); @@ -83,7 +93,13 @@ export const PUT = apiHandler(async (request: NextRequest) => { const updates: Record = {}; const auditDetails: Record = {}; - // Boolean fields — direct mapping + // Boolean fields — direct mapping. + // + // The five assistant flags belong here and not only on the dedicated + // `/settings/assistant-flags` endpoint: `adminSettingsSchema` accepts them + // so an operator scripting their settings over a single route can carry + // them, and a schema that accepts a field and then drops it is worse than + // one that refuses it. const booleanFields = [ "registrationEnabled", "mfaRequired", @@ -93,6 +109,11 @@ export const PUT = apiHandler(async (request: NextRequest) => { "apiGlobal", "umamiEnabled", "glitchtipEnabled", + "assistantEnabled", + "assistantCoachEnabled", + "assistantBriefingEnabled", + "assistantInsightStatusEnabled", + "assistantCorrelationsEnabled", ] as const; for (const field of booleanFields) { if (data[field] !== undefined) { @@ -258,5 +279,10 @@ export const PUT = apiHandler(async (request: NextRequest) => { documentMaxFileBytes: settings.documentMaxFileBytes, documentQuotaBytes: Number(settings.documentQuotaBytes), defaultUserTimezone: settings.defaultUserTimezone, + assistantEnabled: settings.assistantEnabled, + assistantCoachEnabled: settings.assistantCoachEnabled, + assistantBriefingEnabled: settings.assistantBriefingEnabled, + assistantInsightStatusEnabled: settings.assistantInsightStatusEnabled, + assistantCorrelationsEnabled: settings.assistantCorrelationsEnabled, }); }); From f39f2f32ceff650e9a858da6778e8b9ba492a1f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 02:02:30 +0200 Subject: [PATCH 4/5] fix(insights): read the dismissed evidence hash in the resurface gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dismissing a correlation pattern stores `dismissedEvidenceHash` alongside the effect size and sample size, and the gate that decides whether to raise the finding again read the two numbers and never the hash. The hash is the only part of the snapshot that covers the whole evidence tuple, and a snapshot that carries it without the two numbers — what a partial restore leaves behind — could not be released by anything: the gate had no value to compare and fell through to `false` on every future run. Move the decision into `shouldReleaseDismissal`, which reads the hash twice and for two different reasons. Evidence byte-identical to what was dismissed settles immediately, without evaluating a threshold. Evidence whose hash no longer matches goes to the existing thresholds when the numbers are there, and releases the dismissal when they are not, because a changed hash is still proof the evidence moved. The thresholds themselves are untouched. A dismissal still has to survive ordinary sampling noise, which is what they are for; a hash that changes on every recomputation would otherwise undo a deliberate dismissal on the next nightly run. --- .../__tests__/correlation-patterns.test.ts | 135 ++++++++++++++++++ src/lib/insights/correlation-patterns.ts | 64 +++++++-- 2 files changed, 189 insertions(+), 10 deletions(-) diff --git a/src/lib/insights/__tests__/correlation-patterns.test.ts b/src/lib/insights/__tests__/correlation-patterns.test.ts index 8308a9607..0d0c902c3 100644 --- a/src/lib/insights/__tests__/correlation-patterns.test.ts +++ b/src/lib/insights/__tests__/correlation-patterns.test.ts @@ -109,6 +109,7 @@ import { canonicalPatternKey, isMaterialEvidenceChange, PATTERN_FAMILIES, + shouldReleaseDismissal, syncAcceptedPatterns, } from "@/lib/insights/correlation-patterns"; @@ -201,3 +202,137 @@ describe("correlation pattern identity and dismissal", () => { ).toBe(true); }); }); + +/** + * `dismissedEvidenceHash` was written on every dismissal and read by nothing: + * the gate compared effect size and sample size only. These tests pin it as a + * reader, in both directions. + */ +describe("dismissal release against the stored evidence hash", () => { + const DISMISSED_AT = new Date("2026-07-02T00:00:00.000Z"); + + it("holds a dismissal whose evidence has not moved at all", () => { + expect( + shouldReleaseDismissal({ + dismissedAt: DISMISSED_AT, + dismissedEvidenceHash: "a".repeat(64), + dismissedEffectSize: 0.3, + dismissedSampleSize: 40, + currentEvidenceHash: "a".repeat(64), + currentEffectSize: 0.3, + currentSampleSize: 40, + }), + ).toBe(false); + }); + + it("releases a dismissal whose evidence moved materially", () => { + expect( + shouldReleaseDismissal({ + dismissedAt: DISMISSED_AT, + dismissedEvidenceHash: "a".repeat(64), + dismissedEffectSize: 0.3, + dismissedSampleSize: 40, + currentEvidenceHash: "b".repeat(64), + currentEffectSize: -0.2, + currentSampleSize: 40, + }), + ).toBe(true); + }); + + it("holds a dismissal whose evidence moved only within the noise band", () => { + expect( + shouldReleaseDismissal({ + dismissedAt: DISMISSED_AT, + dismissedEvidenceHash: "a".repeat(64), + dismissedEffectSize: 0.3, + dismissedSampleSize: 40, + currentEvidenceHash: "b".repeat(64), + currentEffectSize: 0.35, + currentSampleSize: 45, + }), + ).toBe(false); + }); + + it("releases a snapshot that has only a hash once that hash no longer matches", () => { + expect( + shouldReleaseDismissal({ + dismissedAt: DISMISSED_AT, + dismissedEvidenceHash: "a".repeat(64), + dismissedEffectSize: null, + dismissedSampleSize: null, + currentEvidenceHash: "b".repeat(64), + currentEffectSize: 0.3, + currentSampleSize: 40, + }), + ).toBe(true); + }); + + it("holds a snapshot that has only a hash while that hash still matches", () => { + expect( + shouldReleaseDismissal({ + dismissedAt: DISMISSED_AT, + dismissedEvidenceHash: "a".repeat(64), + dismissedEffectSize: null, + dismissedSampleSize: null, + currentEvidenceHash: "a".repeat(64), + currentEffectSize: 0.3, + currentSampleSize: 40, + }), + ).toBe(false); + }); + + it("holds a pattern that was never dismissed", () => { + expect( + shouldReleaseDismissal({ + dismissedAt: null, + dismissedEvidenceHash: null, + dismissedEffectSize: null, + dismissedSampleSize: null, + currentEvidenceHash: "b".repeat(64), + currentEffectSize: 0.9, + currentSampleSize: 400, + }), + ).toBe(false); + }); +}); + +describe("syncAcceptedPatterns honours the stored hash", () => { + beforeEach(() => { + rows.clear(); + vi.clearAllMocks(); + }); + + it("resurfaces a hash-only dismissal on the next run and leaves an unchanged one dismissed", async () => { + const first = await syncAcceptedPatterns({ + userId: "owner-2", + family: PATTERN_FAMILIES.moodTagCrosstab, + accepted: [baseEvidence], + computedAt: new Date("2026-07-01T00:00:00.000Z"), + }); + const canonicalKey = [...first.values()][0].canonicalKey; + const row = [...rows.values()][0]; + + // A dismissal snapshot carrying the hash and nothing else — the shape a + // partial restore leaves behind. It used to be frozen dismissed forever. + row.dismissedAt = new Date("2026-07-02T00:00:00.000Z"); + row.dismissedEvidenceHash = row.evidenceHash; + row.dismissedEffectSize = null; + row.dismissedSampleSize = null; + + const unchanged = await syncAcceptedPatterns({ + userId: "owner-2", + family: PATTERN_FAMILIES.moodTagCrosstab, + accepted: [baseEvidence], + }); + expect(unchanged.get(canonicalKey)?.dismissed).toBe(true); + + const moved = await syncAcceptedPatterns({ + userId: "owner-2", + family: PATTERN_FAMILIES.moodTagCrosstab, + accepted: [{ ...baseEvidence, pValue: 0.002 }], + }); + expect(moved.get(canonicalKey)?.dismissed).toBe(false); + expect(row.dismissedAt).toBeNull(); + expect(row.dismissedEvidenceHash).toBeNull(); + }); +}); diff --git a/src/lib/insights/correlation-patterns.ts b/src/lib/insights/correlation-patterns.ts index 98d22bf65..07c79f698 100644 --- a/src/lib/insights/correlation-patterns.ts +++ b/src/lib/insights/correlation-patterns.ts @@ -73,6 +73,51 @@ export function isMaterialEvidenceChange(args: { return signChanged || effectMoved || sampleDeepened; } +/** + * Whether this run releases a dismissal. + * + * Two things have to hold. The evidence must have MOVED since the dismissal, + * which is what `dismissedEvidenceHash` records — the only part of the + * snapshot that covers the whole evidence tuple rather than two of its + * numbers — and the move must be material by the thresholds above, so an + * ordinary sampling wobble does not undo a dismissal the user made on + * purpose. + * + * The hash also answers the case the two scalars cannot. A snapshot that + * carries a hash but no effect size and no sample size — a restore from a + * partial backup — used to be frozen dismissed forever, because the gate had + * no number to compare and fell through to `false` on every future run. A + * hash that no longer matches is still proof the evidence moved, so it + * releases the dismissal rather than holding it against a snapshot nobody + * can evaluate. + */ +export function shouldReleaseDismissal(args: { + dismissedAt: Date | null; + dismissedEvidenceHash: string | null; + dismissedEffectSize: number | null; + dismissedSampleSize: number | null; + currentEvidenceHash: string; + currentEffectSize: number; + currentSampleSize: number; +}): boolean { + if (args.dismissedAt == null) return false; + + // Byte-identical to what was dismissed: nothing has changed, so there is + // nothing to reconsider and no threshold worth evaluating. + if (args.dismissedEvidenceHash === args.currentEvidenceHash) return false; + + if (args.dismissedEffectSize != null && args.dismissedSampleSize != null) { + return isMaterialEvidenceChange({ + currentEffectSize: args.currentEffectSize, + currentSampleSize: args.currentSampleSize, + dismissedEffectSize: args.dismissedEffectSize, + dismissedSampleSize: args.dismissedSampleSize, + }); + } + + return args.dismissedEvidenceHash != null; +} + /** * Persist one accepted family run, retire findings no longer accepted, and * return the server's presentation decision for every accepted finding. @@ -123,16 +168,15 @@ export async function syncAcceptedPatterns(args: { const decisions = new Map(); for (const item of prepared) { const prior = existingByKey.get(item.canonicalKey); - const shouldResurface = - prior?.dismissedAt != null && - prior.dismissedEffectSize != null && - prior.dismissedSampleSize != null && - isMaterialEvidenceChange({ - currentEffectSize: item.evidence.effectSize, - currentSampleSize: item.evidence.sampleSize, - dismissedEffectSize: prior.dismissedEffectSize, - dismissedSampleSize: prior.dismissedSampleSize, - }); + const shouldResurface = shouldReleaseDismissal({ + dismissedAt: prior?.dismissedAt ?? null, + dismissedEvidenceHash: prior?.dismissedEvidenceHash ?? null, + dismissedEffectSize: prior?.dismissedEffectSize ?? null, + dismissedSampleSize: prior?.dismissedSampleSize ?? null, + currentEvidenceHash: item.evidenceHash, + currentEffectSize: item.evidence.effectSize, + currentSampleSize: item.evidence.sampleSize, + }); const row = await tx.correlationPattern.upsert({ where: { From 8409d7a0a081d94025467c752f8afcb77c744ebe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Thu, 3 Sep 2026 02:02:33 +0200 Subject: [PATCH 5/5] docs(changelog): record the three settings that did nothing --- CHANGELOG.md | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48b6ec02c..b99c56b6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,42 @@ # Changelog +## [Unreleased] + +### Fixed + +- **A dismissed correlation finding comes back when its evidence has really + moved.** Dismissing a finding stores a fingerprint of the evidence behind it, + and the check that decides whether to raise it again never read that + fingerprint; it compared two of the numbers instead. A dismissal restored + from a backup that carried the fingerprint but not those numbers could not be + released by anything and stayed hidden for good. The fingerprint is read now: + evidence that has not moved at all is settled without going further, and a + dismissal with no numbers to measure against is released once the fingerprint + stops matching. A dismissal still survives ordinary sampling noise, which is + the whole point of it. +- **The five assistant switches save when they are sent to the general admin + settings endpoint.** That endpoint accepted them, answered 200 and stored + nothing; sent on their own they came back as "no valid fields". An operator + scripting their settings over the one endpoint had no way to tell the + difference between a switch that was written and one that was dropped. They + are written now, and echoed in the response so the write reads back. The + dedicated endpoint the admin console uses was never affected. + +### Security + +- **A notification channel the operator switched off stops delivering.** + Telegram, ntfy and Web Push each carry an instance-wide switch in admin + settings. Switching one off only hid its setup card: every account that had + already configured the channel kept receiving on it, for as long as the + channel was configured. The switch now holds where it has to, at delivery. A + dispatch on a channel that is off sends nothing and writes the skip to the + delivery ledger with a reason, so the admin notification diagnostics name the + cause instead of showing a gap. Accounts keep their own channel settings + untouched, so switching a channel back on restores delivery without anyone + re-enabling anything. Turning such a channel on from the user side is now + refused with a message that says why, rather than a toggle that flips and + then delivers nothing. + ## [1.38.4] — 2026-09-02 The shared AI key reaches the provider its address names, and the image