diff --git a/app/server/modules/backups/__tests__/backups.service.execution.test.ts b/app/server/modules/backups/__tests__/backups.service.execution.test.ts index 88e64757..7ca0519b 100644 --- a/app/server/modules/backups/__tests__/backups.service.execution.test.ts +++ b/app/server/modules/backups/__tests__/backups.service.execution.test.ts @@ -424,6 +424,7 @@ describe("backup execution - validation failures", () => { describe("stop backup", () => { test("should keep restic warning details when backup completes with read errors", async () => { const { resticBackupMock } = setup(); + const notificationSpy = vi.spyOn(notificationsService, "sendBackupNotification").mockResolvedValue(); const volume = await createTestVolume(); const repository = await createTestRepository(); const schedule = await createTestBackupSchedule({ @@ -446,6 +447,11 @@ describe("stop backup", () => { const updatedSchedule = await getScheduleByIdOrShortId(schedule.id); expect(updatedSchedule.lastBackupStatus).toBe("warning"); expect(updatedSchedule.lastBackupError).toBe("error: open /mnt/data/private.db: permission denied"); + expect(notificationSpy).toHaveBeenLastCalledWith( + schedule.id, + "warning", + expect.objectContaining({ error: "error: open /mnt/data/private.db: permission denied" }), + ); }); test("should store restic diagnostic details instead of the generic summary on hard failure", async () => { diff --git a/app/server/modules/backups/helpers/backup-lifecycle.ts b/app/server/modules/backups/helpers/backup-lifecycle.ts index ba89e80a..141ce142 100644 --- a/app/server/modules/backups/helpers/backup-lifecycle.ts +++ b/app/server/modules/backups/helpers/backup-lifecycle.ts @@ -201,6 +201,7 @@ export async function finalizeSuccessfulBackup( volumeName: ctx.volume.name, repositoryName: ctx.repository.name, scheduleName: ctx.schedule.name, + error: finalStatus === "warning" ? (warningDetails ?? undefined) : undefined, summary: result ?? undefined, }) .catch((error) => { diff --git a/app/server/modules/notifications/__tests__/notifications.service.test.ts b/app/server/modules/notifications/__tests__/notifications.service.test.ts index 06ad3090..a2ef02d8 100644 --- a/app/server/modules/notifications/__tests__/notifications.service.test.ts +++ b/app/server/modules/notifications/__tests__/notifications.service.test.ts @@ -91,7 +91,7 @@ describe("notificationsService.updateDestination", () => { const { organizationId, user } = await createTestSession(); await withContext({ organizationId, userId: user.id }, async () => { - vi.spyOn(cryptoUtils, "resolveSecret").mockResolvedValue("discord://token@webhookid"); + const resolveSecretSpy = vi.spyOn(cryptoUtils, "resolveSecret").mockResolvedValue("discord://token@webhookid"); const [destination] = await db .insert(notificationDestinationsTable) @@ -106,7 +106,41 @@ describe("notificationsService.updateDestination", () => { 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" }); + expect(updated.config).toMatchObject({ type: "custom", shoutrrrUrl: "encv1:stored-secret" }); + expect(resolveSecretSpy).not.toHaveBeenCalled(); + }); + }); + + test("updates metadata for an existing destination without requiring webhook allowlist when config is unchanged", async () => { + const { organizationId, user } = await createTestSession(); + + await withContext({ organizationId, userId: user.id }, async () => { + const existingConfig = { + type: "generic" as const, + url: "https://hooks.example.com/backup", + method: "POST" as const, + }; + const [destination] = await db + .insert(notificationDestinationsTable) + .values({ + name: "Generic webhook", + type: "generic", + config: existingConfig, + organizationId, + }) + .returning(); + + const updated = await notificationsService.updateDestination(destination.id, { + name: "Renamed webhook", + config: existingConfig, + }); + + expect(updated.name).toBe("Renamed webhook"); + expect(updated.config).toMatchObject({ + type: "generic", + url: "https://hooks.example.com/backup", + method: "POST", + }); }); }); }); diff --git a/app/server/modules/notifications/notifications.service.ts b/app/server/modules/notifications/notifications.service.ts index 9fe0d8ac..6f3f905b 100644 --- a/app/server/modules/notifications/notifications.service.ts +++ b/app/server/modules/notifications/notifications.service.ts @@ -104,17 +104,23 @@ const updateDestination = async ( updateData.enabled = updates.enabled; } - const configToValidate = updates.config ?? (await decryptNotificationConfig(existing.config)); - const newConfigResult = notificationConfigSchema.safeParse(configToValidate); - if (!newConfigResult.success) { - throw new BadRequestError("Invalid notification configuration"); - } - const newConfig = newConfigResult.data; - assertNotificationTargetAllowed(newConfig, serverConfig.webhookAllowedOrigins); + if (updates.config !== undefined) { + const newConfigResult = notificationConfigSchema.safeParse(updates.config); + if (!newConfigResult.success) { + throw new BadRequestError("Invalid notification configuration"); + } + const newConfig = newConfigResult.data; + const resolvedConfig = await decryptNotificationConfig(newConfig); + const existingConfig = await decryptNotificationConfig(existing.config); - const encryptedConfig = await encryptNotificationConfig(newConfig); - updateData.config = encryptedConfig; - updateData.type = newConfig.type; + if (JSON.stringify(resolvedConfig) !== JSON.stringify(existingConfig)) { + assertNotificationTargetAllowed(resolvedConfig, serverConfig.webhookAllowedOrigins); + + const encryptedConfig = await encryptNotificationConfig(newConfig); + updateData.config = encryptedConfig; + updateData.type = newConfig.type; + } + } const [updated] = await db .update(notificationDestinationsTable)