fix: avoid unnecessary webhook allowlist checks on notification edits

This commit is contained in:
Nicolas Meienberger
2026-05-04 20:18:46 +02:00
parent 6a47af58c7
commit ade31e7e95
4 changed files with 59 additions and 12 deletions

View File

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

View File

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

View File

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

View File

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