From b1ae85e2c187e19a93f8224c6a591e97cd879e5f Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Mon, 4 May 2026 19:48:55 +0200 Subject: [PATCH] fix(notifications): preserve existing destinations with target allowlist --- .github/workflows/release.yml | 28 ------------------- .../notification-target-policy.test.ts | 9 ++---- .../__tests__/notifications.service.test.ts | 26 +++++++++++++++++ .../notifications/notifications.service.ts | 3 +- .../utils/notification-target-policy.ts | 7 +---- 5 files changed, 31 insertions(+), 42 deletions(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5c7c0d53..5c69f290 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -79,34 +79,6 @@ jobs: username: ${{ github.repository_owner }} password: ${{ secrets.GITHUB_TOKEN }} - - name: Build docker image - uses: docker/build-push-action@d08e5c354a6adb9ed34480a06d141179aa583294 # v7 - with: - context: . - target: production - platforms: linux/amd64 - push: false - load: true - tags: local/zerobyte:ci - build-args: | - APP_VERSION=${{ needs.determine-release-type.outputs.tagname }} - - - name: Scan new image for vulnerabilities - if: needs.determine-release-type.outputs.release_type == 'release' - uses: anchore/scan-action@e1165082ffb1fe366ebaf02d8526e7c4989ea9d2 # v7 - id: scan - with: - image: local/zerobyte:ci - fail-build: false - only-fixed: true - severity-cutoff: critical - - - name: upload Anchore scan report - if: needs.determine-release-type.outputs.release_type == 'release' - uses: github/codeql-action/upload-sarif@c10b8064de6f491fea524254123dbe5e09572f13 # v4 - with: - sarif_file: ${{ steps.scan.outputs.sarif }} - - name: Docker meta id: meta uses: docker/metadata-action@030e881283bb7a6894de51c315a6bfe6a94e05cf # v6 diff --git a/app/server/modules/notifications/__tests__/notification-target-policy.test.ts b/app/server/modules/notifications/__tests__/notification-target-policy.test.ts index 59fb7ac9..bd792e7b 100644 --- a/app/server/modules/notifications/__tests__/notification-target-policy.test.ts +++ b/app/server/modules/notifications/__tests__/notification-target-policy.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "vitest"; import { assertNotificationTargetAllowed } from "../utils/notification-target-policy"; describe("assertNotificationTargetAllowed", () => { - test("requires email SMTP targets to match the allowlist", () => { + test("allows built-in email SMTP targets without the webhook allowlist", () => { const notificationConfig = { type: "email" as const, smtpHost: "smtp.example.com", @@ -12,12 +12,7 @@ describe("assertNotificationTargetAllowed", () => { useTLS: true, }; - expect(() => assertNotificationTargetAllowed(notificationConfig, [])).toThrow( - "Add smtp://smtp.example.com:587 to WEBHOOK_ALLOWED_ORIGINS", - ); - - expect(() => assertNotificationTargetAllowed(notificationConfig, ["smtp://smtp.example.com:587"])).not.toThrow(); - expect(() => assertNotificationTargetAllowed(notificationConfig, ["smtp://smtp.example.com:25"])).toThrow(); + expect(() => assertNotificationTargetAllowed(notificationConfig, [])).not.toThrow(); }); test("rejects notification types that are not classified by the SSRF policy", () => { diff --git a/app/server/modules/notifications/__tests__/notifications.service.test.ts b/app/server/modules/notifications/__tests__/notifications.service.test.ts index 2c8f2da3..06ad3090 100644 --- a/app/server/modules/notifications/__tests__/notifications.service.test.ts +++ b/app/server/modules/notifications/__tests__/notifications.service.test.ts @@ -6,6 +6,7 @@ import { createTestSession } from "~/test/helpers/auth"; import * as shoutrrr from "~/server/utils/shoutrrr"; import { notificationsService } from "../notifications.service"; import { serverEvents } from "~/server/core/events"; +import { cryptoUtils } from "~/server/utils/crypto"; afterEach(() => { vi.restoreAllMocks(); @@ -84,3 +85,28 @@ describe("notificationsService.testDestination", () => { }); }); }); + +describe("notificationsService.updateDestination", () => { + test("updates metadata for an existing encrypted custom destination without revalidating the encrypted URL", async () => { + const { organizationId, user } = await createTestSession(); + + await withContext({ organizationId, userId: user.id }, async () => { + vi.spyOn(cryptoUtils, "resolveSecret").mockResolvedValue("discord://token@webhookid"); + + const [destination] = await db + .insert(notificationDestinationsTable) + .values({ + name: "Custom webhook", + type: "custom", + config: { type: "custom", shoutrrrUrl: "encv1:stored-secret" }, + organizationId, + }) + .returning(); + + const updated = await notificationsService.updateDestination(destination.id, { name: "Renamed webhook" }); + + expect(updated.name).toBe("Renamed webhook"); + expect(updated.config).toMatchObject({ type: "custom", shoutrrrUrl: "discord://token@webhookid" }); + }); + }); +}); diff --git a/app/server/modules/notifications/notifications.service.ts b/app/server/modules/notifications/notifications.service.ts index 3a9138e9..9fe0d8ac 100644 --- a/app/server/modules/notifications/notifications.service.ts +++ b/app/server/modules/notifications/notifications.service.ts @@ -104,7 +104,8 @@ const updateDestination = async ( updateData.enabled = updates.enabled; } - const newConfigResult = notificationConfigSchema.safeParse(updates.config || existing.config); + const configToValidate = updates.config ?? (await decryptNotificationConfig(existing.config)); + const newConfigResult = notificationConfigSchema.safeParse(configToValidate); if (!newConfigResult.success) { throw new BadRequestError("Invalid notification configuration"); } diff --git a/app/server/modules/notifications/utils/notification-target-policy.ts b/app/server/modules/notifications/utils/notification-target-policy.ts index c79137e0..75ab34c5 100644 --- a/app/server/modules/notifications/utils/notification-target-policy.ts +++ b/app/server/modules/notifications/utils/notification-target-policy.ts @@ -115,12 +115,7 @@ const getCustomShoutrrrTarget = (shoutrrrUrl: string) => { const getNotificationTarget = (notificationConfig: NotificationConfig) => { switch (notificationConfig.type) { - case "email": { - const smtpTarget = new URL("smtp://placeholder"); - smtpTarget.hostname = notificationConfig.smtpHost; - smtpTarget.port = String(notificationConfig.smtpPort); - return `${smtpTarget.protocol}//${smtpTarget.host}`; - } + case "email": case "slack": case "discord": case "pushover":