From f42f9cfbb5111a77b368e0c01ddeb6897f77461c Mon Sep 17 00:00:00 2001 From: Nicolas Meienberger Date: Thu, 30 Apr 2026 21:58:45 +0200 Subject: [PATCH] fix: pr feedbacks --- app/server/core/__tests__/config.test.ts | 1 + .../src/backup-hooks/__tests__/hooks.test.ts | 31 ++++++++++++++++++- packages/core/src/backup-hooks/index.ts | 17 +++++----- 3 files changed, 39 insertions(+), 10 deletions(-) diff --git a/app/server/core/__tests__/config.test.ts b/app/server/core/__tests__/config.test.ts index e60dadac..43a56c8b 100644 --- a/app/server/core/__tests__/config.test.ts +++ b/app/server/core/__tests__/config.test.ts @@ -73,6 +73,7 @@ describe("parseConfig", () => { }, provisioningPath: "/tmp/provisioning", allowedHosts: ["example.com", "admin.example.com", "localhost:3000"], + webhookAllowedOrigins: [], }); }); diff --git a/packages/core/src/backup-hooks/__tests__/hooks.test.ts b/packages/core/src/backup-hooks/__tests__/hooks.test.ts index f7b9e2a5..54b3870f 100644 --- a/packages/core/src/backup-hooks/__tests__/hooks.test.ts +++ b/packages/core/src/backup-hooks/__tests__/hooks.test.ts @@ -351,7 +351,36 @@ test("rejects webhook URLs outside the configured allowed origins", async () => }); expect(backupRan).toBe(false); - expect(result).toEqual({ status: "failed", error: "pre webhook URL origin is not allowed" }); + expect(result).toEqual({ + status: "failed", + error: "pre webhook URL origin is not allowed. Add http://127.0.0.1:8080 to WEBHOOK_ALLOWED_ORIGINS.", + }); +}); + +test("matches configured webhook origins with trailing slashes or paths", async () => { + let backupRan = false; + + server.use( + http.post("http://localhost:8080/pre", () => { + return new HttpResponse(null, { status: 204 }); + }), + ); + + const result = await runWithHooks({ + webhookAllowedOrigins: ["http://localhost:8080/", "http://example.com/webhook"], + webhooks: { + pre: { url: "http://localhost:8080/pre" }, + post: null, + }, + runBackup: () => + Effect.sync(() => { + backupRan = true; + return { exitCode: 0, result: null, warningDetails: null }; + }), + }); + + expect(backupRan).toBe(true); + expect(result).toEqual({ status: "completed", exitCode: 0, result: null, warningDetails: null }); }); test("does not follow webhook redirects", async () => { diff --git a/packages/core/src/backup-hooks/index.ts b/packages/core/src/backup-hooks/index.ts index c491e13f..4533ec47 100644 --- a/packages/core/src/backup-hooks/index.ts +++ b/packages/core/src/backup-hooks/index.ts @@ -9,15 +9,11 @@ const MAX_BACKUP_WEBHOOK_HEADERS = 32; const MAX_BACKUP_WEBHOOK_HEADER_BYTES = 8 * 1024; const getByteLength = (value: string) => new TextEncoder().encode(value).byteLength; +const getUrlOrigin = (url: string) => (URL.canParse(url) ? new URL(url).origin : null); -const isAllowedWebhookUrl = (url: string, allowedOrigins: readonly string[]) => { - try { - const parsedUrl = new URL(url); - - return allowedOrigins.includes(parsedUrl.origin); - } catch { - return false; - } +export const isAllowedWebhookUrl = (url: string, allowedOrigins: readonly string[]) => { + const webhookOrigin = getUrlOrigin(url); + return webhookOrigin !== null && allowedOrigins.some((origin) => getUrlOrigin(origin) === webhookOrigin); }; export const backupWebhookConfigSchema = z.object({ @@ -204,9 +200,12 @@ const runBackupWebhook = ( return Effect.tryPromise({ try: async () => { if (!isAllowedWebhookUrl(config.url, options.allowedOrigins)) { + const webhookOrigin = getUrlOrigin(config.url); throw new BackupWebhookError({ cause: new Error("Webhook URL origin is not allowed"), - message: `${context.phase} webhook URL origin is not allowed`, + message: `${context.phase} webhook URL origin is not allowed. Add ${ + webhookOrigin ?? config.url + } to WEBHOOK_ALLOWED_ORIGINS.`, }); }