From 0f20ec8ab85de4170ba542b7b24cab11ae1287d9 Mon Sep 17 00:00:00 2001 From: Phuc Nguyen Date: Sun, 2 Aug 2026 08:28:21 +0700 Subject: [PATCH] feat(webhooks): add project event filters --- app/api/projects/[id]/webhooks/route.ts | 7 ++-- app/api/projects/[id]/webhooks/test/route.ts | 7 +++- lib/webhook-events.ts | 30 ++++++++++++++++ lib/webhooks.ts | 11 ++++-- tests/project-webhook-management.test.mjs | 32 +++++++++++++++++ tests/webhook-events.test.mjs | 37 ++++++++++++++++++++ 6 files changed, 118 insertions(+), 6 deletions(-) diff --git a/app/api/projects/[id]/webhooks/route.ts b/app/api/projects/[id]/webhooks/route.ts index 6a20316..2ce7db0 100644 --- a/app/api/projects/[id]/webhooks/route.ts +++ b/app/api/projects/[id]/webhooks/route.ts @@ -3,6 +3,7 @@ import { db } from "@/lib/db"; import { requireUser, unauthorized, bad } from "@/lib/api"; import { authorizeProject } from "@/lib/authz"; import { newSecret, isInternalUrl } from "@/lib/webhooks"; +import { normalizeWebhookEventSubscriptions } from "@/lib/webhook-events"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -33,8 +34,10 @@ export async function POST(req: NextRequest, ctx: { params: Promise<{ id: string const url = String(body?.url || "").trim(); if (!/^https?:\/\//.test(url)) return bad("a valid http(s) url is required"); if (isInternalUrl(url)) return bad("that url points at an internal/blocked address"); - let events: string[] = ["*"]; - if (Array.isArray(body?.events) && body.events.length) events = body.events.map((e: any) => String(e)); + const events = normalizeWebhookEventSubscriptions(body?.events); + if (!events) { + return bad("events must be an array of names using letters, digits, dot, underscore, colon, or dash"); + } const secret = newSecret(); const res = await db().execute({ diff --git a/app/api/projects/[id]/webhooks/test/route.ts b/app/api/projects/[id]/webhooks/test/route.ts index 7a7343d..3324684 100644 --- a/app/api/projects/[id]/webhooks/test/route.ts +++ b/app/api/projects/[id]/webhooks/test/route.ts @@ -14,6 +14,11 @@ export async function POST(req: NextRequest, ctx: { params: Promise<{ id: string const az = await authorizeProject(u.sub, projectId, "webhook.manage"); if (!az.ok) return bad(az.error, az.status); - const results = await dispatchEvent(projectId, "ping", { message: "🤘 moshcoding test event", at: new Date().toISOString() }); + const results = await dispatchEvent( + projectId, + "ping", + { message: "🤘 moshcoding test event", at: new Date().toISOString() }, + { bypassEventFilter: true }, + ); return NextResponse.json({ dispatched: results.length, results }); } diff --git a/lib/webhook-events.ts b/lib/webhook-events.ts index 83db858..781cc16 100644 --- a/lib/webhook-events.ts +++ b/lib/webhook-events.ts @@ -13,6 +13,36 @@ export function normalizeInboundEventType(value: unknown): string | null { return eventType; } +/** + * Canonical event subscriptions for one outbound project webhook. + * + * Blank lists mean "all events" so the creation form can leave the field + * empty. Specific names use the same compact grammar as inbound event types; + * duplicates are removed without changing the order shown back to the user. + * A wildcard makes specific entries redundant, but every entry is still + * validated before the list is collapsed to `["*"]`. + */ +export function normalizeWebhookEventSubscriptions(value: unknown): string[] | null { + if (value === undefined || value === null) return ["*"]; + if (!Array.isArray(value)) return null; + + const events: string[] = []; + const seen = new Set(); + for (const item of value) { + if (typeof item !== "string") return null; + const eventType = item.trim(); + if (!eventType) continue; + if (eventType !== "*" && !EVENT_TYPE_RE.test(eventType)) return null; + if (!seen.has(eventType)) { + seen.add(eventType); + events.push(eventType); + } + } + + if (!events.length || seen.has("*")) return ["*"]; + return events; +} + /** * Event type used when relaying an inbound event to a domain's outbound targets. * The `inbound.` prefix is applied at most once: a target pointed back at our own diff --git a/lib/webhooks.ts b/lib/webhooks.ts index 4ec711e..4e978fd 100644 --- a/lib/webhooks.ts +++ b/lib/webhooks.ts @@ -50,8 +50,13 @@ export function eventEnvelope(type: string, data: unknown, projectId: string) { return { id, type, data, created_at: new Date().toISOString(), project_id: projectId }; } -/** Dispatch an event to every active endpoint of a project that subscribes to it. */ -export async function dispatchEvent(projectId: string, type: string, data: unknown) { +/** Dispatch an event to active project endpoints, normally honoring subscriptions. */ +export async function dispatchEvent( + projectId: string, + type: string, + data: unknown, + opts: { bypassEventFilter?: boolean } = {}, +) { const { rows } = await db().execute({ sql: `SELECT id, url, secret, events FROM webhook_endpoints WHERE project_id = ? AND active = 1`, args: [projectId], @@ -60,7 +65,7 @@ export async function dispatchEvent(projectId: string, type: string, data: unkno for (const ep of rows as any[]) { let events: string[] = ["*"]; try { events = JSON.parse(ep.events); } catch { /* default */ } - if (!events.includes("*") && !events.includes(type)) continue; + if (!opts.bypassEventFilter && !events.includes("*") && !events.includes(type)) continue; results.push(await deliverToEndpoint(String(ep.id), String(ep.url), String(ep.secret), type, data, projectId)); } return results; diff --git a/tests/project-webhook-management.test.mjs b/tests/project-webhook-management.test.mjs index 7148d57..9fbcf7a 100644 --- a/tests/project-webhook-management.test.mjs +++ b/tests/project-webhook-management.test.mjs @@ -64,6 +64,38 @@ test("project webhook targets pause and resume without losing configuration", as } }); +test("test deliveries reach active endpoints regardless of event subscriptions", async () => { + const id = "endpoint-subscription-test"; + await db().execute({ + sql: `INSERT INTO webhook_endpoints (id, project_id, url, secret, events) + VALUES (?, ?, ?, ?, ?)`, + args: [id, "project-subscription-test", "https://hooks.example.test/subscription-test", "whsec_subscription_test", '["build.finished"]'], + }); + const originalFetch = globalThis.fetch; + const calls = []; + globalThis.fetch = async (...args) => { + calls.push(args); + return new Response(null, { status: 204 }); + }; + + try { + const filtered = await dispatchEvent("project-subscription-test", "ping", { ok: true }); + assert.equal(filtered.length, 0); + assert.equal(calls.length, 0); + + const testDeliveries = await dispatchEvent( + "project-subscription-test", + "ping", + { ok: true }, + { bypassEventFilter: true }, + ); + assert.equal(testDeliveries.length, 1); + assert.equal(calls.length, 1); + } finally { + globalThis.fetch = originalFetch; + } +}); + test("project scoping prevents cross-project changes", async () => { const id = await insertEndpoint("project-owner", "scoped"); diff --git a/tests/webhook-events.test.mjs b/tests/webhook-events.test.mjs index 863c07b..72d8710 100644 --- a/tests/webhook-events.test.mjs +++ b/tests/webhook-events.test.mjs @@ -4,6 +4,7 @@ import test from "node:test"; import { MAX_RELAY_HOPS, normalizeInboundEventType, + normalizeWebhookEventSubscriptions, parseRelayHop, relayEventType, } from "../lib/webhook-events.ts"; @@ -23,6 +24,42 @@ test("inbound webhook event types reject non-string and unsafe values", () => { assert.equal(normalizeInboundEventType("x".repeat(81)), null); }); +test("outbound subscriptions trim and deduplicate event names", () => { + assert.deepEqual( + normalizeWebhookEventSubscriptions([ + " build.finished ", + "deploy.failed", + "build.finished", + "", + ]), + ["build.finished", "deploy.failed"], + ); +}); + +test("blank outbound subscriptions mean all events", () => { + for (const value of [undefined, null, [], [""], [" ", ""]]) { + assert.deepEqual(normalizeWebhookEventSubscriptions(value), ["*"]); + } + assert.deepEqual( + normalizeWebhookEventSubscriptions(["build.finished", "*", "deploy.failed", "*"]), + ["*"], + ); +}); + +test("outbound subscriptions reject malformed arrays and event names", () => { + for (const value of [ + "build.finished", + { event: "build.finished" }, + ["build finished"], + [".hidden"], + ["x".repeat(81)], + ["build.finished", 42], + ["*", "not valid"], + ]) { + assert.equal(normalizeWebhookEventSubscriptions(value), null); + } +}); + test("relayed event types are prefixed exactly once", () => { assert.equal(relayEventType("payment.succeeded"), "inbound.payment.succeeded"); assert.equal(relayEventType(null), "inbound.event");