feat: make webhook calls trusted only (#848)
* feat: make webhook calls trusted only * fix: pr feedbacks
This commit is contained in:
parent
1cf96c5292
commit
220e2d2801
10 changed files with 176 additions and 10 deletions
|
|
@ -73,6 +73,7 @@ describe("parseConfig", () => {
|
|||
},
|
||||
provisioningPath: "/tmp/provisioning",
|
||||
allowedHosts: ["example.com", "admin.example.com", "localhost:3000"],
|
||||
webhookAllowedOrigins: [],
|
||||
});
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -44,11 +44,13 @@ const envSchema = z
|
|||
BASE_URL: z.string(),
|
||||
ENABLE_DEV_PANEL: z.string().default("false"),
|
||||
ENABLE_LOCAL_AGENT: z.string().default("false"),
|
||||
WEBHOOK_ALLOWED_ORIGINS: z.string().optional(),
|
||||
PROVISIONING_PATH: z.string().optional(),
|
||||
})
|
||||
.transform((s, ctx) => {
|
||||
const baseUrl = unquote(s.BASE_URL);
|
||||
const trustedOrigins = s.TRUSTED_ORIGINS?.split(",").map(unquote).filter(Boolean).concat(baseUrl) ?? [baseUrl];
|
||||
const webhookAllowedOrigins = s.WEBHOOK_ALLOWED_ORIGINS?.split(",").map(unquote).filter(Boolean) ?? [];
|
||||
const authOrigins = [baseUrl, ...trustedOrigins];
|
||||
const { allowedHosts, invalidOrigins } = buildAllowedHosts(authOrigins);
|
||||
let appSecret = s.APP_SECRET;
|
||||
|
|
@ -136,6 +138,7 @@ const envSchema = z
|
|||
},
|
||||
provisioningPath: s.PROVISIONING_PATH,
|
||||
allowedHosts,
|
||||
webhookAllowedOrigins,
|
||||
};
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -145,6 +145,7 @@ test("close emits a synthetic backup.cancelled for a queued backup", () => {
|
|||
rcloneConfigFile: "/tmp/rclone.conf",
|
||||
},
|
||||
webhooks: { pre: null, post: null },
|
||||
webhookAllowedOrigins: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -56,6 +56,7 @@ const createBackupRunPayload = async ({
|
|||
hostname: resticDeps.hostname,
|
||||
},
|
||||
webhooks: schedule.backupWebhooks ?? { pre: null, post: null },
|
||||
webhookAllowedOrigins: config.webhookAllowedOrigins,
|
||||
};
|
||||
};
|
||||
|
||||
|
|
@ -73,6 +74,7 @@ const executeBackupWithoutAgent = async (
|
|||
organizationId: payload.organizationId,
|
||||
options: payload.options,
|
||||
webhooks: payload.webhooks,
|
||||
webhookAllowedOrigins: payload.webhookAllowedOrigins,
|
||||
signal,
|
||||
onProgress,
|
||||
formatError: toErrorDetails,
|
||||
|
|
|
|||
|
|
@ -47,6 +47,7 @@ test("emits backup.failed when a backup command hits a restic error", async () =
|
|||
rcloneConfigFile: "/root/.config/rclone/rclone.conf",
|
||||
},
|
||||
webhooks: { pre: null, post: null },
|
||||
webhookAllowedOrigins: [],
|
||||
}),
|
||||
);
|
||||
|
||||
|
|
|
|||
|
|
@ -53,6 +53,7 @@ const createRunPayload = (overrides: Partial<BackupRunPayload> = {}) =>
|
|||
rcloneConfigFile: "/tmp/rclone.conf",
|
||||
},
|
||||
webhooks: { pre: null, post: null },
|
||||
webhookAllowedOrigins: ["http://localhost:8080"],
|
||||
...overrides,
|
||||
});
|
||||
|
||||
|
|
@ -272,7 +273,8 @@ test("includes post-backup webhook failure details when a backup is cancelled",
|
|||
const cancelled = messages.find((message) => message?.success && message.data.type === "backup.cancelled");
|
||||
expect(cancelled?.success).toBe(true);
|
||||
if (cancelled?.success && cancelled.data.type === "backup.cancelled") {
|
||||
expect(cancelled.data.payload.message).toContain("post webhook returned HTTP 500: start failed");
|
||||
expect(cancelled.data.payload.message).toContain("post webhook returned HTTP 500");
|
||||
expect(cancelled.data.payload.message).not.toContain("start failed");
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -334,6 +336,7 @@ test("waits for running-job registration before returning to the processor loop"
|
|||
defaultExcludes: [],
|
||||
},
|
||||
webhooks: { pre: null, post: null },
|
||||
webhookAllowedOrigins: [],
|
||||
});
|
||||
const cancelPayload = fromPartial<BackupCancelPayload>({
|
||||
jobId: "job-1",
|
||||
|
|
|
|||
|
|
@ -66,6 +66,7 @@ export const handleBackupRunCommand = (context: ControllerCommandContext, payloa
|
|||
organizationId: payload.organizationId,
|
||||
options: payload.options,
|
||||
webhooks: payload.webhooks,
|
||||
webhookAllowedOrigins: payload.webhookAllowedOrigins,
|
||||
signal: abortController.signal,
|
||||
onProgress: (progress) => {
|
||||
void Runtime.runPromise(
|
||||
|
|
|
|||
|
|
@ -41,6 +41,7 @@ const backupRunSchema = z.object({
|
|||
options: backupExecutionOptionsSchema,
|
||||
runtime: backupRuntimeSchema,
|
||||
webhooks: backupWebhooksSchema,
|
||||
webhookAllowedOrigins: z.array(z.string()),
|
||||
}),
|
||||
});
|
||||
|
||||
|
|
|
|||
|
|
@ -44,6 +44,7 @@ const runWithHooks = <TResult>(
|
|||
repositoryConfig: { backend: "local", path: "/tmp/repository" },
|
||||
options: {},
|
||||
webhooks: { pre: null, post: null },
|
||||
webhookAllowedOrigins: ["http://localhost:8080"],
|
||||
signal: defaultSignal(),
|
||||
...options,
|
||||
}),
|
||||
|
|
@ -176,7 +177,8 @@ test("fails without running the backup or post webhook when the pre-backup webho
|
|||
expect(postRan).toBe(false);
|
||||
expect(result.status).toBe("failed");
|
||||
if (result.status === "failed") {
|
||||
expect(result.error).toContain("pre webhook returned HTTP 500: stop failed");
|
||||
expect(result.error).toContain("pre webhook returned HTTP 500");
|
||||
expect(result.error).not.toContain("stop failed");
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -295,7 +297,8 @@ test("includes post-backup webhook failure details after cancellation", async ()
|
|||
expect(result.status).toBe("cancelled");
|
||||
if (result.status === "cancelled") {
|
||||
expect(result.message).toContain("Backup was cancelled");
|
||||
expect(result.message).toContain("post webhook returned HTTP 500: start failed");
|
||||
expect(result.message).toContain("post webhook returned HTTP 500");
|
||||
expect(result.message).not.toContain("start failed");
|
||||
}
|
||||
});
|
||||
|
||||
|
|
@ -327,10 +330,106 @@ test("includes post-backup webhook failure details after completed cancellation"
|
|||
expect(result.status).toBe("cancelled");
|
||||
if (result.status === "cancelled") {
|
||||
expect(result.message).toContain("Backup was cancelled");
|
||||
expect(result.message).toContain("post webhook returned HTTP 500: cleanup failed");
|
||||
expect(result.message).toContain("post webhook returned HTTP 500");
|
||||
expect(result.message).not.toContain("cleanup failed");
|
||||
}
|
||||
});
|
||||
|
||||
test("rejects webhook URLs outside the configured allowed origins", async () => {
|
||||
let backupRan = false;
|
||||
|
||||
const result = await runWithHooks({
|
||||
webhooks: {
|
||||
pre: { url: "http://127.0.0.1:8080/pre" },
|
||||
post: null,
|
||||
},
|
||||
runBackup: () =>
|
||||
Effect.sync(() => {
|
||||
backupRan = true;
|
||||
return { exitCode: 0, result: null, warningDetails: null };
|
||||
}),
|
||||
});
|
||||
|
||||
expect(backupRan).toBe(false);
|
||||
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 () => {
|
||||
let redirectedTargetCalled = false;
|
||||
|
||||
server.use(
|
||||
http.post("http://localhost:8080/redirect", () => {
|
||||
return new HttpResponse(null, { status: 302, headers: { location: "http://localhost:8080/target" } });
|
||||
}),
|
||||
http.post("http://localhost:8080/target", () => {
|
||||
redirectedTargetCalled = true;
|
||||
return new HttpResponse(null, { status: 204 });
|
||||
}),
|
||||
);
|
||||
|
||||
const result = await runWithHooks({
|
||||
webhooks: {
|
||||
pre: { url: "http://localhost:8080/redirect" },
|
||||
post: null,
|
||||
},
|
||||
runBackup: () => completedBackup(null),
|
||||
});
|
||||
|
||||
expect(redirectedTargetCalled).toBe(false);
|
||||
expect(result).toEqual({ status: "failed", error: "pre webhook returned HTTP 302" });
|
||||
});
|
||||
|
||||
test("rejects oversized webhook request bodies and headers", async () => {
|
||||
const bodyResult = await runWithHooks({
|
||||
webhooks: {
|
||||
pre: { url: "http://localhost:8080/pre", body: "a".repeat(64 * 1024 + 1) },
|
||||
post: null,
|
||||
},
|
||||
runBackup: () => completedBackup(null),
|
||||
});
|
||||
|
||||
expect(bodyResult).toEqual({ status: "failed", error: "Webhook request body exceeds 65536 bytes" });
|
||||
|
||||
const headersResult = await runWithHooks({
|
||||
webhooks: {
|
||||
pre: { url: "http://localhost:8080/pre", headers: [`x-large: ${"a".repeat(8 * 1024)}`] },
|
||||
post: null,
|
||||
},
|
||||
runBackup: () => completedBackup(null),
|
||||
});
|
||||
|
||||
expect(headersResult).toEqual({ status: "failed", error: "Webhook request headers exceed 8192 bytes" });
|
||||
});
|
||||
|
||||
test("cancels before the pre-backup webhook without running the backup", async () => {
|
||||
const abortController = new AbortController();
|
||||
let backupRan = false;
|
||||
|
|
|
|||
|
|
@ -3,7 +3,18 @@ import { z } from "zod";
|
|||
import type { CompressionMode, RepositoryConfig, ResticBackupProgressDto } from "../restic/index.js";
|
||||
import { toErrorDetails, toMessage } from "../utils/index.js";
|
||||
|
||||
const DEFAULT_BACKUP_WEBHOOK_TIMEOUT_MS = 60_000;
|
||||
const DEFAULT_BACKUP_WEBHOOK_TIMEOUT_MS = 10_000;
|
||||
const MAX_BACKUP_WEBHOOK_BODY_BYTES = 64 * 1024;
|
||||
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);
|
||||
|
||||
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({
|
||||
url: z.url(),
|
||||
|
|
@ -70,6 +81,7 @@ type BackupLifecycleOptions<TResult> = {
|
|||
repositoryConfig: RepositoryConfig;
|
||||
options: BackupOptions;
|
||||
webhooks: BackupWebhooks;
|
||||
webhookAllowedOrigins: readonly string[];
|
||||
signal: AbortSignal;
|
||||
onProgress?: (progress: ResticBackupProgressDto) => void;
|
||||
formatError?: (error: unknown) => string;
|
||||
|
|
@ -121,10 +133,24 @@ const createRequestInit = (config: BackupWebhookConfig, context: BackupWebhookCo
|
|||
const headers = new Headers();
|
||||
const body = config.body ?? JSON.stringify(context);
|
||||
|
||||
if (getByteLength(body) > MAX_BACKUP_WEBHOOK_BODY_BYTES) {
|
||||
throw new BackupWebhookError({
|
||||
cause: new Error("Webhook request body is too large"),
|
||||
message: `Webhook request body exceeds ${MAX_BACKUP_WEBHOOK_BODY_BYTES} bytes`,
|
||||
});
|
||||
}
|
||||
|
||||
if (config.body === undefined) {
|
||||
headers.set("content-type", "application/json");
|
||||
}
|
||||
|
||||
if ((config.headers?.length ?? 0) > MAX_BACKUP_WEBHOOK_HEADERS) {
|
||||
throw new BackupWebhookError({
|
||||
cause: new Error("Webhook request has too many headers"),
|
||||
message: `Webhook request exceeds ${MAX_BACKUP_WEBHOOK_HEADERS} custom headers`,
|
||||
});
|
||||
}
|
||||
|
||||
for (const header of config.headers ?? []) {
|
||||
const [name, ...valueParts] = header.split(":");
|
||||
|
||||
|
|
@ -133,7 +159,24 @@ const createRequestInit = (config: BackupWebhookConfig, context: BackupWebhookCo
|
|||
}
|
||||
}
|
||||
|
||||
return { method: "POST", headers, body };
|
||||
const headerBytes = [...headers.entries()].reduce(
|
||||
(total, [name, value]) => total + getByteLength(name) + getByteLength(value),
|
||||
0,
|
||||
);
|
||||
|
||||
if (headerBytes > MAX_BACKUP_WEBHOOK_HEADER_BYTES) {
|
||||
throw new BackupWebhookError({
|
||||
cause: new Error("Webhook request headers are too large"),
|
||||
message: `Webhook request headers exceed ${MAX_BACKUP_WEBHOOK_HEADER_BYTES} bytes`,
|
||||
});
|
||||
}
|
||||
|
||||
return {
|
||||
method: "POST",
|
||||
headers,
|
||||
body: config.body === undefined ? body : new TextEncoder().encode(body),
|
||||
redirect: "manual",
|
||||
};
|
||||
};
|
||||
|
||||
const runBackupWebhook = (
|
||||
|
|
@ -141,6 +184,7 @@ const runBackupWebhook = (
|
|||
context: BackupWebhookContext,
|
||||
options: {
|
||||
formatError: (error: unknown) => string;
|
||||
allowedOrigins: readonly string[];
|
||||
signal?: AbortSignal;
|
||||
timeoutMs?: number;
|
||||
},
|
||||
|
|
@ -155,14 +199,22 @@ 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. Add ${
|
||||
webhookOrigin ?? config.url
|
||||
} to WEBHOOK_ALLOWED_ORIGINS.`,
|
||||
});
|
||||
}
|
||||
|
||||
const response = await fetch(config.url, { ...createRequestInit(config, context), signal: controller.signal });
|
||||
|
||||
if (!response.ok) {
|
||||
const responseText = await response.text().catch(() => "");
|
||||
const details = responseText.trim().slice(0, 500);
|
||||
throw new BackupWebhookError({
|
||||
cause: new Error(`${context.phase} webhook returned HTTP ${response.status}`),
|
||||
message: `${context.phase} webhook returned HTTP ${response.status}${details ? `: ${details}` : ""}`,
|
||||
message: `${context.phase} webhook returned HTTP ${response.status}`,
|
||||
});
|
||||
}
|
||||
},
|
||||
|
|
@ -199,6 +251,7 @@ export const runBackupLifecycle = <TResult>({
|
|||
repositoryConfig,
|
||||
options,
|
||||
webhooks,
|
||||
webhookAllowedOrigins,
|
||||
signal,
|
||||
onProgress,
|
||||
formatError = toErrorDetails,
|
||||
|
|
@ -210,6 +263,7 @@ export const runBackupLifecycle = <TResult>({
|
|||
{ ...context, phase: "pre", event: "backup.pre" },
|
||||
{
|
||||
formatError,
|
||||
allowedOrigins: webhookAllowedOrigins,
|
||||
signal,
|
||||
},
|
||||
);
|
||||
|
|
@ -254,7 +308,7 @@ export const runBackupLifecycle = <TResult>({
|
|||
status: backupResult.hookStatus,
|
||||
error: backupResult.hookError,
|
||||
},
|
||||
{ formatError },
|
||||
{ formatError, allowedOrigins: webhookAllowedOrigins },
|
||||
);
|
||||
|
||||
if (signal.aborted) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue