diff --git a/lib/email.ts b/lib/email.ts index 3c9ab41..082ffba 100644 --- a/lib/email.ts +++ b/lib/email.ts @@ -6,6 +6,21 @@ const RESEND_ENDPOINT = "https://api.resend.com/emails"; +/** + * Escape text before interpolating it into an HTML email body. Titles, brand + * names and actors can come from external/untrusted sources (e.g. a GitHub + * issue title), so raw interpolation would let them inject markup into the + * rendered email. Plain-text bodies don't need this. + */ +function escapeHtml(value: string | number): string { + return String(value) + .replace(/&/g, "&") + .replace(//g, ">") + .replace(/"/g, """) + .replace(/'/g, "'"); +} + export function isEmailConfigured(): boolean { return Boolean(process.env.RESEND_API_KEY); } @@ -84,23 +99,25 @@ export function sendWaitlistVerification(opts: { `You're almost in. Confirm your email to lock in your spot on the ${brand} waitlist:\n\n` + `${url}\n\n` + `If you didn't request this, just ignore it — no account is created.`; + const safeBrand = escapeHtml(brand); + const safeUrl = escapeHtml(url); const html = `
-

${brand} 🤘

+

${safeBrand} 🤘

You're almost in. Confirm your email to lock in your spot in the pit.

- + Confirm my spot

- Or paste this link:
${url} + Or paste this link:
${safeUrl}

Didn't sign up? Ignore this — nothing happens. @@ -129,17 +146,21 @@ export function sendGithubClosedNotification(opts: { const text = `${opts.actor} ${verb} ${opts.kind} #${opts.number} in ${opts.repo}\n\n` + `${opts.title}\n${opts.url}\n`; + const safeRepo = escapeHtml(opts.repo); + const safeTitle = escapeHtml(opts.title); + const safeActor = escapeHtml(opts.actor); + const safeUrl = escapeHtml(opts.url); const html = ` @@ -161,23 +182,25 @@ export function sendPasswordReset(opts: { `Someone asked to reset the password for your ${brand} account.\n\n` + `Set a new one here (link expires in 1 hour):\n${url}\n\n` + `If this wasn't you, ignore this email — your password stays unchanged.`; + const safeBrand = escapeHtml(brand); + const safeUrl = escapeHtml(url); const html = `
-

${opts.repo}

+

${safeRepo}

${opts.kind} #${opts.number} ${verb} 🤘

-

${opts.title}

-

by ${opts.actor}

- View on GitHub +

${safeTitle}

+

by ${safeActor}

+ View on GitHub
-

${brand} 🤘

+

${safeBrand} 🤘

Reset your password. This link expires in 1 hour.

- + Set a new password

- Or paste this link:
${url} + Or paste this link:
${safeUrl}

Didn't ask for this? Ignore it — nothing changes. diff --git a/tests/email-escape.test.mjs b/tests/email-escape.test.mjs new file mode 100644 index 0000000..e84dcd5 --- /dev/null +++ b/tests/email-escape.test.mjs @@ -0,0 +1,74 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + sendGithubClosedNotification, + sendWaitlistVerification, + sendPasswordReset, +} from "../lib/email.ts"; + +/** + * These emails interpolate external/tenant-controlled values (a GitHub issue + * title, the actor, the tenant brand) into an HTML body. Raw interpolation + * let those values inject markup into the rendered email. Capture the outgoing + * Resend payload and assert the HTML body is escaped. + */ +async function capturePayload(run) { + const prevKey = process.env.RESEND_API_KEY; + const prevFetch = globalThis.fetch; + process.env.RESEND_API_KEY = "test_key"; // force the real send() path + let captured = null; + globalThis.fetch = async (_url, opts) => { + captured = JSON.parse(opts.body); + return { ok: true, json: async () => ({ id: "x" }), text: async () => "" }; + }; + try { + await run(); + } finally { + globalThis.fetch = prevFetch; + if (prevKey === undefined) delete process.env.RESEND_API_KEY; + else process.env.RESEND_API_KEY = prevKey; + } + return captured; +} + +test("github notification escapes an attacker-controlled issue title in the HTML body", async () => { + const payload = await capturePayload(() => + sendGithubClosedNotification({ + to: "owner@example.com", + kind: "issue", + repo: "moshcoder/moshcode", + number: 1, + title: "", + url: "https://github.com/x", + actor: 'evil">pwn', + merged: false, + }), + ); + assert.ok(!payload.html.includes("