fix(notifications): preserve existing destinations with target allowlist

This commit is contained in:
Nicolas Meienberger
2026-05-04 19:48:55 +02:00
parent 497a0e8bee
commit b1ae85e2c1
5 changed files with 31 additions and 42 deletions

View File

@@ -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

View File

@@ -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", () => {

View File

@@ -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" });
});
});
});

View File

@@ -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");
}

View File

@@ -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":