diff --git a/src/selfhost/discord-notify.ts b/src/selfhost/discord-notify.ts index e72ed7b804..ecae1d0a8e 100644 --- a/src/selfhost/discord-notify.ts +++ b/src/selfhost/discord-notify.ts @@ -3,6 +3,26 @@ // the environment — self-host only, and a no-op when neither is set or in a runtime without process.env. // Best-effort: an absent or failing webhook never affects the review (all errors are swallowed). +const ALLOWED_DISCORD_HOSTS = new Set([ + "discord.com", + "discordapp.com", + "canary.discord.com", + "ptb.discord.com", +]); + +function isValidDiscordWebhook(url: string): boolean { + try { + const parsed = new URL(url); + return ( + parsed.protocol === "https:" && + ALLOWED_DISCORD_HOSTS.has(parsed.hostname.toLowerCase()) && + parsed.pathname.startsWith("/api/webhooks/") + ); + } catch { + return false; + } +} + function readConfig(): { map: Record; global: string | null } { /* v8 ignore next */ // process is always defined in the self-host (node) runtime; the guard is for the Worker bundle const env: Record = @@ -24,11 +44,9 @@ function readConfig(): { map: Record; global: string | null } { export function resolveDiscordWebhook(repoFullName: string): string | null { const { map, global } = readConfig(); const repoUrl = map[repoFullName]; - return typeof repoUrl === "string" && repoUrl.length > 0 - ? repoUrl - : global && global.length > 0 - ? global - : null; + if (typeof repoUrl === "string" && isValidDiscordWebhook(repoUrl)) + return repoUrl; + return global && isValidDiscordWebhook(global) ? global : null; } // Embed accent colour by outcome — green = good, amber = caution, red = blocked/closed (matches the hosted look). diff --git a/test/unit/selfhost-discord-notify.test.ts b/test/unit/selfhost-discord-notify.test.ts index 15be8d1591..ef9411b784 100644 --- a/test/unit/selfhost-discord-notify.test.ts +++ b/test/unit/selfhost-discord-notify.test.ts @@ -21,34 +21,71 @@ afterEach(() => { describe("resolveDiscordWebhook", () => { it("returns the repo-specific webhook when configured", () => { - setEnv(JSON.stringify({ "o/a": "https://discord/a" }), undefined); - expect(resolveDiscordWebhook("o/a")).toBe("https://discord/a"); + setEnv( + JSON.stringify({ "o/a": "https://discord.com/api/webhooks/a/token" }), + undefined, + ); + expect(resolveDiscordWebhook("o/a")).toBe( + "https://discord.com/api/webhooks/a/token", + ); }); it("falls back to the global webhook for an unmapped repo", () => { setEnv( - JSON.stringify({ "o/a": "https://discord/a" }), - "https://discord/global", + JSON.stringify({ "o/a": "https://discord.com/api/webhooks/a/token" }), + "https://discord.com/api/webhooks/global/token", + ); + expect(resolveDiscordWebhook("o/b")).toBe( + "https://discord.com/api/webhooks/global/token", ); - expect(resolveDiscordWebhook("o/b")).toBe("https://discord/global"); }); it("returns null when nothing is configured", () => { setEnv(undefined, undefined); expect(resolveDiscordWebhook("o/a")).toBeNull(); }); it("ignores malformed JSON and uses the global", () => { - setEnv("{not json", "https://discord/global"); - expect(resolveDiscordWebhook("o/a")).toBe("https://discord/global"); + setEnv("{not json", "https://discord.com/api/webhooks/global/token"); + expect(resolveDiscordWebhook("o/a")).toBe( + "https://discord.com/api/webhooks/global/token", + ); }); it("ignores a non-object map value and uses the global", () => { - setEnv("123", "https://discord/global"); - expect(resolveDiscordWebhook("o/a")).toBe("https://discord/global"); + setEnv("123", "https://discord.com/api/webhooks/global/token"); + expect(resolveDiscordWebhook("o/a")).toBe( + "https://discord.com/api/webhooks/global/token", + ); + }); + it("ignores non-Discord repo webhooks and uses a valid global fallback", () => { + setEnv( + JSON.stringify({ "o/a": "http://127.0.0.1:9999/not-discord" }), + "https://discord.com/api/webhooks/global/token", + ); + expect(resolveDiscordWebhook("o/a")).toBe( + "https://discord.com/api/webhooks/global/token", + ); + }); + it("returns null for non-Discord global webhooks", () => { + setEnv(undefined, "http://127.0.0.1:9999/not-discord"); + expect(resolveDiscordWebhook("o/a")).toBeNull(); + }); + it("rejects HTTPS webhooks on non-Discord hosts", () => { + setEnv(undefined, "https://example.com/api/webhooks/a/token"); + expect(resolveDiscordWebhook("o/a")).toBeNull(); + }); + it("rejects malformed and non-webhook Discord URLs", () => { + setEnv( + JSON.stringify({ "o/a": "not a url" }), + "https://discord.com/channels/1", + ); + expect(resolveDiscordWebhook("o/a")).toBeNull(); }); }); describe("notifyDiscordReview", () => { it("posts a rich embed (title repo#pr·outcome, reason, Outcome/PR/Submitter fields, footer) — closed → red", async () => { setEnv( - JSON.stringify({ "JSONbored/gittensory": "https://discord/gt" }), + JSON.stringify({ + "JSONbored/gittensory": "https://discord.com/api/webhooks/gt/token", + }), undefined, ); let posted: { @@ -77,7 +114,7 @@ describe("notifyDiscordReview", () => { url: "https://gh/JSONbored/gittensory/pull/1171", }); const e = posted!.body.embeds[0]!; - expect(posted!.url).toBe("https://discord/gt"); + expect(posted!.url).toBe("https://discord.com/api/webhooks/gt/token"); expect(e.title).toBe("JSONbored/gittensory#1171 · closed"); expect(e.url).toBe("https://gh/JSONbored/gittensory/pull/1171"); expect(e.description).toBe( @@ -90,6 +127,20 @@ describe("notifyDiscordReview", () => { expect(e.fields[2]!.value).toBe("@jaso0n0818"); expect(e.footer.text).toBe("Gittensory · JSONbored/gittensory"); }); + it("no-ops (no fetch) when a configured webhook is not a Discord webhook", async () => { + setEnv(undefined, "http://127.0.0.1:9999/not-discord"); + const fetchSpy = vi.fn(async () => new Response(null, { status: 204 })); + vi.stubGlobal("fetch", fetchSpy); + await notifyDiscordReview({ + repoFullName: "o/a", + prNumber: 1, + author: "a", + outcome: "reviewed", + reason: "x", + url: "u", + }); + expect(fetchSpy).not.toHaveBeenCalled(); + }); it("no-ops (no fetch) when no webhook is configured", async () => { setEnv(undefined, undefined); const fetchSpy = vi.fn(async () => new Response(null, { status: 204 })); @@ -105,7 +156,7 @@ describe("notifyDiscordReview", () => { expect(fetchSpy).not.toHaveBeenCalled(); }); it("swallows fetch errors (best-effort)", async () => { - setEnv(undefined, "https://discord/global"); + setEnv(undefined, "https://discord.com/api/webhooks/global/token"); vi.stubGlobal("fetch", async () => { throw new Error("network down"); }); @@ -121,7 +172,7 @@ describe("notifyDiscordReview", () => { ).resolves.toBeUndefined(); }); it("uses the default colour for an unknown outcome", async () => { - setEnv(undefined, "https://discord/global"); + setEnv(undefined, "https://discord.com/api/webhooks/global/token"); let color = -1; vi.stubGlobal("fetch", async (_u: string, init: { body: string }) => { color = JSON.parse(init.body).embeds[0].color;