fix: pr feedbacks
This commit is contained in:
parent
a47877de4d
commit
f42f9cfbb5
3 changed files with 39 additions and 10 deletions
|
|
@ -73,6 +73,7 @@ describe("parseConfig", () => {
|
||||||
},
|
},
|
||||||
provisioningPath: "/tmp/provisioning",
|
provisioningPath: "/tmp/provisioning",
|
||||||
allowedHosts: ["example.com", "admin.example.com", "localhost:3000"],
|
allowedHosts: ["example.com", "admin.example.com", "localhost:3000"],
|
||||||
|
webhookAllowedOrigins: [],
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -351,7 +351,36 @@ test("rejects webhook URLs outside the configured allowed origins", async () =>
|
||||||
});
|
});
|
||||||
|
|
||||||
expect(backupRan).toBe(false);
|
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 () => {
|
test("does not follow webhook redirects", async () => {
|
||||||
|
|
|
||||||
|
|
@ -9,15 +9,11 @@ const MAX_BACKUP_WEBHOOK_HEADERS = 32;
|
||||||
const MAX_BACKUP_WEBHOOK_HEADER_BYTES = 8 * 1024;
|
const MAX_BACKUP_WEBHOOK_HEADER_BYTES = 8 * 1024;
|
||||||
|
|
||||||
const getByteLength = (value: string) => new TextEncoder().encode(value).byteLength;
|
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[]) => {
|
export const isAllowedWebhookUrl = (url: string, allowedOrigins: readonly string[]) => {
|
||||||
try {
|
const webhookOrigin = getUrlOrigin(url);
|
||||||
const parsedUrl = new URL(url);
|
return webhookOrigin !== null && allowedOrigins.some((origin) => getUrlOrigin(origin) === webhookOrigin);
|
||||||
|
|
||||||
return allowedOrigins.includes(parsedUrl.origin);
|
|
||||||
} catch {
|
|
||||||
return false;
|
|
||||||
}
|
|
||||||
};
|
};
|
||||||
|
|
||||||
export const backupWebhookConfigSchema = z.object({
|
export const backupWebhookConfigSchema = z.object({
|
||||||
|
|
@ -204,9 +200,12 @@ const runBackupWebhook = (
|
||||||
return Effect.tryPromise({
|
return Effect.tryPromise({
|
||||||
try: async () => {
|
try: async () => {
|
||||||
if (!isAllowedWebhookUrl(config.url, options.allowedOrigins)) {
|
if (!isAllowedWebhookUrl(config.url, options.allowedOrigins)) {
|
||||||
|
const webhookOrigin = getUrlOrigin(config.url);
|
||||||
throw new BackupWebhookError({
|
throw new BackupWebhookError({
|
||||||
cause: new Error("Webhook URL origin is not allowed"),
|
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.`,
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue