From d62407f431b8e34c228ecefb9338df325fbd97a3 Mon Sep 17 00:00:00 2001 From: Paul Nothaft <53005142+the-luap@users.noreply.github.com> Date: Sat, 29 Aug 2026 11:10:32 +0200 Subject: [PATCH] feat(email): webhook transport as an alternative to SMTP (#1225) (#1231) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Setting EMAIL_WEBHOOK_URL makes PicPeak stop sending mail itself and POST each composed message as JSON instead, for something downstream (n8n, Make, a self-hosted relay) to deliver. Unset, every SMTP path is unchanged. Settles the four things #1225 left open: - SSRF: the URL goes through the same DNS-resolving check the outbound webhook worker uses, before every send. Private receivers are opt-in. - Transport security: https is required for anything leaving the machine. The HMAC proves who sent the body, not who can read it, and these bodies carry password-reset links and guest recovery codes. The private-network opt-in doubles as the plaintext opt-in. - Authentication: EMAIL_WEBHOOK_SECRET is required and signs the body as X-PicPeak-Signature, the same scheme as gallery webhooks. A URL without a secret leaves the transport OFF and says so once. - Attachments: carried as base64, not dropped. Oversized ones fail and stay queued rather than arriving without the invoice. Configuration is environment-only on purpose: this redirects every outbound message including password resets, so it must not be changeable from a compromised admin session. Three wiring details decide whether it works at all: docker-compose.yml declares an explicit environment block, so the vars had to be forwarded there; a fresh webhook-only install has no email_configs row (migration 001 seeds it only when SMTP_HOST is set), so the From identity falls back to EMAIL_FROM; and processEmailQueue used to return early when SMTP could not initialise, which would have left the queue permanently unprocessed. guestRecoveryService and the admin test-email endpoint were bypassing the transport — the first dereferenced a null transporter, the second told webhook-only admins to go configure SMTP. emailIntakeService deliberately stays on SMTP: it round-trips a specific mailbox's own credentials. Response handling is streamed and read bounded by hand rather than capped via axios: maxContentLength throws while reading, so a receiver that delivered the mail and then echoed a large body would have been recorded as failed and the message sent again. Note: docker-compose.dev.yml is gitignored and local-only, so the equivalent entries there are not part of this change. docker-compose.production.yml needs none — it passes .env through with env_file. Three rounds of external review; 21 transport tests, 61 across the email suites. --- .env.example | 36 +++ .../services/emailWebhookTransport.test.js | 264 ++++++++++++++++ backend/server.js | 11 +- backend/src/routes/adminEmail.js | 49 ++- backend/src/services/emailProcessor.js | 106 +++++-- backend/src/services/emailWebhookTransport.js | 295 ++++++++++++++++++ backend/src/services/guestRecoveryService.js | 33 +- .../src/services/updateNotificationService.js | 9 +- docker-compose.yml | 7 + 9 files changed, 774 insertions(+), 36 deletions(-) create mode 100644 backend/__tests__/services/emailWebhookTransport.test.js create mode 100644 backend/src/services/emailWebhookTransport.js diff --git a/.env.example b/.env.example index 8dcf17c1..9b0050ff 100644 --- a/.env.example +++ b/.env.example @@ -99,6 +99,42 @@ DB_NAME=picpeak_prod #SMTP_PASS=your-app-specific-password #EMAIL_FROM=noreply@yourdomain.com +# Webhook email transport (#1225) — OPTIONAL, an alternative to SMTP entirely. +# When EMAIL_WEBHOOK_URL is set, PicPeak stops sending mail itself and POSTs +# each composed message as JSON to that URL instead; something downstream +# (n8n, Make, a self-hosted relay) delivers it. Useful when SMTP is the part +# you cannot get working — app passwords, blocked ports, a NAS with no +# outbound 25. +# +# Deliberately environment-only, not an admin setting: it redirects every +# outbound message including password resets, so it should not be changeable +# from a compromised admin session. +# +# EMAIL_WEBHOOK_SECRET is REQUIRED. The body is signed with it and sent as +# X-PicPeak-Signature (HMAC-SHA256, hex) — the same scheme as gallery +# webhooks, so a receiver verifies both the same way. Set the URL without a +# secret and the transport stays OFF and says so in the log, rather than +# posting unauthenticated mail to the internet. +# +# Payload: { from, to[], cc[], subject, html, text, attachments[] }, where each +# attachment is { filename, content_type, content_base64 }. Attachments are +# included rather than dropped; a message whose attachments exceed 10 MB fails +# and stays in the queue instead of arriving without its invoice. +# +# The receiver must be a public https:// address unless you opt in — a container or LAN +# address is refused by the SSRF check otherwise. Running n8n beside PicPeak is +# normal, so set EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS=true for that. +# +# A mail account with its own SMTP host (Settings -> Mail accounts) keeps +# sending through it; this replaces the global transport only. +# +# Set EMAIL_FROM above as well. A webhook-only install never gets an +# email_configs row (that is seeded only when SMTP_HOST is set), so EMAIL_FROM +# is where the sender address comes from. +#EMAIL_WEBHOOK_URL=https://n8n.example.com/webhook/picpeak-mail +#EMAIL_WEBHOOK_SECRET=generate-a-long-random-string +#EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS=false + # Application URLs — OPTIONAL. Leave unset for the normal install. # The public origin is captured by the setup wizard (it proposes the address # you opened the browser at) and stored as the `general_site_url` setting, so diff --git a/backend/__tests__/services/emailWebhookTransport.test.js b/backend/__tests__/services/emailWebhookTransport.test.js new file mode 100644 index 00000000..0b03543f --- /dev/null +++ b/backend/__tests__/services/emailWebhookTransport.test.js @@ -0,0 +1,264 @@ +/** + * Webhook email transport (#1225). + * + * Pins the four things the issue said had to be decided before this could + * ship, because each is a security or data-loss property rather than a + * preference: + * + * - it will not run unsigned (URL without a secret stays OFF) + * - the body is HMAC-signed with the same scheme as gallery webhooks + * - the URL goes through the DNS-resolving SSRF check + * - attachments are carried, and an oversized one FAILS rather than being + * dropped — an invoice email arriving without its invoice is worse than + * one that errors and stays in the queue + */ + +jest.mock('axios', () => ({ post: jest.fn() })); +jest.mock('../../src/utils/networkValidation', () => ({ + validateExternalUrlAsync: jest.fn(async () => ({ valid: true, reason: 'ok' })), +})); +jest.mock('../../src/utils/logger', () => ({ + info: jest.fn(), warn: jest.fn(), error: jest.fn(), debug: jest.fn(), +})); + +const path = require('path'); +const os = require('os'); +const fsSync = require('fs'); + +const axios = require('axios'); +const { validateExternalUrlAsync } = require('../../src/utils/networkValidation'); +const logger = require('../../src/utils/logger'); +const transport = require('../../src/services/emailWebhookTransport'); +const { verifySignature } = require('../../src/services/webhookService'); + +const URL = 'https://n8n.example.com/webhook/picpeak-mail'; +const SECRET = 'a-long-random-shared-secret'; + +const MAIL = { + from: 'PicPeak ', + to: 'client@example.com', + subject: 'Your gallery is ready', + html: '

hello

', + text: 'hello', +}; + +beforeEach(() => { + jest.clearAllMocks(); + process.env.EMAIL_WEBHOOK_URL = URL; + process.env.EMAIL_WEBHOOK_SECRET = SECRET; + transport.__testing.setAllowPrivateUrls(false); + transport.__testing.resetSecretWarning(); + validateExternalUrlAsync.mockResolvedValue({ valid: true, reason: 'ok' }); + axios.post.mockResolvedValue({ status: 200, data: streamOf('') }); +}); + +// The transport reads the response as a stream, so mocks must behave like one. +function streamOf(text) { + const { Readable } = require('stream'); + return Readable.from([Buffer.from(text, 'utf8')]); +} + +afterEach(() => { + delete process.env.EMAIL_WEBHOOK_URL; + delete process.env.EMAIL_WEBHOOK_SECRET; +}); + +describe('enablement', () => { + it('is off when no URL is configured', () => { + delete process.env.EMAIL_WEBHOOK_URL; + expect(transport.isEnabled()).toBe(false); + }); + + it('is ON with a URL and a secret', () => { + expect(transport.isEnabled()).toBe(true); + }); + + it('refuses to run unsigned: a URL without a secret stays OFF and says why', () => { + delete process.env.EMAIL_WEBHOOK_SECRET; + expect(transport.isEnabled()).toBe(false); + expect(logger.error).toHaveBeenCalledWith(expect.stringContaining('EMAIL_WEBHOOK_SECRET')); + }); + + it('logs that misconfiguration once, not per email', () => { + delete process.env.EMAIL_WEBHOOK_SECRET; + transport.isEnabled(); + transport.isEnabled(); + transport.isEnabled(); + expect(logger.error).toHaveBeenCalledTimes(1); + }); +}); + +describe('signing', () => { + it('signs the exact bytes sent, verifiable with the shared secret', async () => { + await transport.send(MAIL); + + const [url, body, opts] = axios.post.mock.calls[0]; + expect(url).toBe(URL); + const signature = opts.headers[transport.__testing.SIGNATURE_HEADER]; + // Same verifier a receiver would use for gallery webhooks. + expect(verifySignature(SECRET, body, signature)).toBe(true); + // And it must not verify against the wrong secret. + expect(verifySignature('not-the-secret', body, signature)).toBe(false); + }); + + it('sends the composed message, with recipients normalised to lists', async () => { + await transport.send({ ...MAIL, cc: 'a@example.com, b@example.com' }); + const payload = JSON.parse(axios.post.mock.calls[0][1]); + expect(payload.to).toEqual(['client@example.com']); + expect(payload.cc).toEqual(['a@example.com', 'b@example.com']); + expect(payload.subject).toBe('Your gallery is ready'); + expect(payload.html).toBe('

hello

'); + }); + + it('splits a combined address that arrives INSIDE an array', async () => { + // sendRawEmail wraps a string cc in an array before it reaches here, so + // "a@x, b@y" lands as one element. Left unsplit, the payload carries one + // combined address that a relay treating each element as a mailbox rejects. + await transport.send({ ...MAIL, cc: ['a@example.com, b@example.com'] }); + const payload = JSON.parse(axios.post.mock.calls[0][1]); + expect(payload.cc).toEqual(['a@example.com', 'b@example.com']); + }); +}); + +describe('transport security', () => { + it('refuses plaintext http:// — the HMAC signs, it does not conceal', async () => { + // These bodies carry password-reset links and guest recovery codes, which + // are usable by anyone on the path. + process.env.EMAIL_WEBHOOK_URL = 'http://n8n.example.com/webhook/picpeak-mail'; + await expect(transport.send(MAIL)).rejects.toThrow(/https/); + expect(axios.post).not.toHaveBeenCalled(); + }); + + it('allows http only under the private-network opt-in', async () => { + process.env.EMAIL_WEBHOOK_URL = 'http://n8n.internal:5678/webhook/mail'; + transport.__testing.setAllowPrivateUrls(true); + await expect(transport.send(MAIL)).resolves.toBeTruthy(); + }); +}); + +describe('SSRF preflight', () => { + it('refuses a URL that resolves to a private address', async () => { + validateExternalUrlAsync.mockResolvedValue({ + valid: false, error: 'URL points to a private or internal network address', reason: 'private', + }); + await expect(transport.send(MAIL)).rejects.toThrow(/rejected/); + expect(axios.post).not.toHaveBeenCalled(); + }); + + it('allows a private receiver only when explicitly opted in', async () => { + validateExternalUrlAsync.mockResolvedValue({ valid: false, error: 'private', reason: 'private' }); + transport.__testing.setAllowPrivateUrls(true); + await expect(transport.send(MAIL)).resolves.toBeTruthy(); + // The check is skipped entirely rather than its answer ignored. + expect(validateExternalUrlAsync).not.toHaveBeenCalled(); + }); +}); + +describe('attachments', () => { + const tmpDir = fsSync.mkdtempSync(path.join(os.tmpdir(), 'picpeak-webhook-mail-')); + const filePath = path.join(tmpDir, 'invoice.pdf'); + fsSync.writeFileSync(filePath, 'PDFBYTES'); + + it('carries a file from disk as base64 rather than dropping it', async () => { + await transport.send({ + ...MAIL, + attachments: [{ filename: 'invoice.pdf', path: filePath, contentType: 'application/pdf' }], + }); + const payload = JSON.parse(axios.post.mock.calls[0][1]); + expect(payload.attachments).toHaveLength(1); + expect(payload.attachments[0].filename).toBe('invoice.pdf'); + expect(payload.attachments[0].content_type).toBe('application/pdf'); + expect(Buffer.from(payload.attachments[0].content_base64, 'base64').toString()).toBe('PDFBYTES'); + }); + + it('carries an in-memory buffer too', async () => { + await transport.send({ + ...MAIL, + attachments: [{ filename: 'note.txt', content: Buffer.from('hi') }], + }); + const payload = JSON.parse(axios.post.mock.calls[0][1]); + expect(Buffer.from(payload.attachments[0].content_base64, 'base64').toString()).toBe('hi'); + }); + + it('FAILS on an oversized attachment instead of sending the mail without it', async () => { + const huge = Buffer.alloc(transport.__testing.MAX_ATTACHMENT_BYTES + 1); + await expect(transport.send({ + ...MAIL, + attachments: [{ filename: 'huge.bin', content: huge }], + })).rejects.toThrow(/exceed/); + expect(axios.post).not.toHaveBeenCalled(); + }); + + it('rejects an oversized FILE by its size, without reading it into memory', async () => { + // The cap has to be checked from stat, not after readFile — otherwise a + // file big enough to exhaust memory kills the process before the guard + // that exists to stop it ever fires. + const bigPath = path.join(tmpDir, 'big.bin'); + fsSync.writeFileSync(bigPath, Buffer.alloc(1024)); + const statSpy = jest.spyOn(require('fs').promises, 'stat') + .mockResolvedValue({ size: transport.__testing.MAX_ATTACHMENT_BYTES + 1 }); + const readSpy = jest.spyOn(require('fs').promises, 'readFile'); + + await expect(transport.send({ + ...MAIL, + attachments: [{ filename: 'big.bin', path: bigPath }], + })).rejects.toThrow(/exceed/); + expect(readSpy).not.toHaveBeenCalled(); + + statSpy.mockRestore(); + readSpy.mockRestore(); + }); +}); + +describe('response handling', () => { + it('caps how much of a receiver response it will buffer', async () => { + await transport.send(MAIL); + const opts = axios.post.mock.calls[0][2]; + // Only the status and an optional messageId are read; an unbounded body + // from a faulty or hostile receiver must not be buffered into memory. + expect(opts.responseType).toBe('stream'); + expect(opts.maxBodyLength).toEqual(expect.any(Number)); + }); + + it('sizes the request cap in UTF-8 bytes, so non-ASCII mail is not rejected', async () => { + // axios enforces maxBodyLength against the UTF-8 buffer it sends. Sizing it + // from String#length counts UTF-16 code units, so a German or Japanese + // message would exceed its own budget and never leave the process. + await transport.send({ ...MAIL, subject: 'Grüße', html: '

これはテストです。ありがとう。

' }); + const [, body, opts] = axios.post.mock.calls[0]; + expect(opts.maxBodyLength).toBeGreaterThanOrEqual(Buffer.byteLength(body, 'utf8')); + // And the gap is real: bytes genuinely exceed code units for this payload. + expect(Buffer.byteLength(body, 'utf8')).toBeGreaterThan(body.length); + }); +}); + +describe('delivery result', () => { + it('treats a non-2xx as a failure so the queue retries', async () => { + axios.post.mockResolvedValue({ status: 502, data: streamOf('') }); + await expect(transport.send(MAIL)).rejects.toThrow(/502/); + }); + + it('returns a messageId so the queue can record the send', async () => { + const result = await transport.send(MAIL); + expect(result.messageId).toEqual(expect.any(String)); + expect(result.messageId.length).toBeGreaterThan(0); + }); + + it('does NOT retry a delivered message just because the response was huge', async () => { + // A receiver that delivered the mail and then echoed a large body must not + // turn into a failure — the queue would resend and the recipient would get + // the same email twice. The status is the verdict; the body is optional. + const { Readable } = require('stream'); + const flood = Readable.from( + Array.from({ length: 40 }, () => Buffer.alloc(1024, 0x61)) + ); + axios.post.mockResolvedValue({ status: 200, data: flood }); + const result = await transport.send(MAIL); + expect(result.messageId).toEqual(expect.any(String)); + }); + + it('prefers a messageId the receiver reports', async () => { + axios.post.mockResolvedValue({ status: 200, data: streamOf(JSON.stringify({ messageId: 'from-n8n-123' })) }); + expect((await transport.send(MAIL)).messageId).toBe('from-n8n-123'); + }); +}); diff --git a/backend/server.js b/backend/server.js index 7e7ced7d..7f068b67 100644 --- a/backend/server.js +++ b/backend/server.js @@ -81,6 +81,7 @@ const { startDownloadJobCleanup } = require('./src/services/downloadJobCleanupSe const { startRevealScheduler } = require('./src/services/revealScheduler'); const { startInvoiceScheduler } = require('./src/services/invoiceSchedulerService'); const { initializeTransporter, startEmailQueueProcessor } = require('./src/services/emailProcessor'); +const emailWebhookTransport = require('./src/services/emailWebhookTransport'); const { startBackupService } = require('./src/services/backupService'); const { startScheduledBackups } = require('./src/services/databaseBackup'); const backgroundProcessor = require('./src/services/backgroundProcessor'); @@ -1069,8 +1070,14 @@ async function startServer() { // flag is OFF (the service short-circuits on empty result sets). startInvoiceScheduler(); - // Initialize email transporter and start queue processor - await initializeTransporter(); + // Initialize email transporter and start queue processor. + // Skipped under the webhook transport (#1225): an install that switched to + // it may still carry an old, now-unreachable SMTP row, and nodemailer's + // verify() would sit on a connection timeout here — delaying boot for a + // transport that will never send anything. + if (!emailWebhookTransport.isEnabled()) { + await initializeTransporter(); + } // Seed CRM / contract / event-reminder email templates and recover // any queue rows that exhausted retries because their template // didn't exist yet. Runs once per boot via module-level caches in diff --git a/backend/src/routes/adminEmail.js b/backend/src/routes/adminEmail.js index f23f1a69..de448785 100644 --- a/backend/src/routes/adminEmail.js +++ b/backend/src/routes/adminEmail.js @@ -8,7 +8,8 @@ const { requirePermission } = require('../middleware/permissions'); // /email mount — the pre-existing config/queue/received endpoints stay ungated). const { requireFeatureFlag } = require('../middleware/requireFeatureFlag'); const messagingGate = requireFeatureFlag('messaging'); -const { wrapEmailHtml, processEmailQueue } = require('../services/emailProcessor'); +const { wrapEmailHtml, processEmailQueue, resolveFromIdentity } = require('../services/emailProcessor'); +const emailWebhookTransport = require('../services/emailWebhookTransport'); const { errorResponse } = require('../utils/routeHelpers'); const logger = require('../utils/logger'); const router = express.Router(); @@ -369,8 +370,12 @@ router.get('/identities', adminAuth, messagingGate, requirePermission('email.vie const cust = await db('mail_accounts').where({ account_key: 'customers' }).first(); customers = cust?.imap_user || cust?.from_email || null; } catch (_) { customers = null; } + // A webhook-only install has no email_configs row (#1225), so reading the + // automated address from it alone shows "—" in the Messages sidebar for an + // instance that is sending perfectly well from EMAIL_FROM. + const identity = await resolveFromIdentity(); res.json({ - automated: cfg?.from_email || null, + automated: cfg?.from_email || identity?.fromEmail || null, accounting: cfg?.imap_user || null, customers, }); @@ -465,6 +470,46 @@ router.post('/test', adminAuth, requirePermission('email.send'), async (req, res return res.status(400).json({ error: 'Test email address is required' }); } + // Webhook transport (#1225): this endpoint is the "does email work" button, + // so it has to exercise the transport that actually sends. Left as-is it + // built a nodemailer transport from email_configs and told a webhook-only + // admin to go configure SMTP — settings their install does not use, on an + // instance whose mail is working fine. + if (emailWebhookTransport.isEnabled()) { + const identity = await resolveFromIdentity(); + if (!identity) { + return res.status(400).json({ + error: 'No sender address configured. Set EMAIL_FROM for the webhook transport.', + }); + } + const webhookSubject = 'Test Email - Photo Sharing Platform'; + const webhookHtml = await wrapEmailHtml( + '

Test Email Successful!

' + + '

This message was delivered through the configured email webhook, not SMTP.

', + webhookSubject + ); + try { + await emailWebhookTransport.send({ + from: `${identity.fromName} <${identity.fromEmail}>`, + to: test_email, + subject: webhookSubject, + html: webhookHtml, + text: 'Test Email Successful! Delivered through the configured email webhook.', + }); + } catch (webhookError) { + // Handled here, not by the outer catch: that one maps ECONNREFUSED and + // friends to "Failed to connect to SMTP server — check your SMTP + // settings", which on a webhook-only install points the admin at + // configuration this deploy does not use. + logger.error('Webhook test email failed:', webhookError); + return res.status(502).json({ + error: 'Failed to deliver through the email webhook', + details: webhookError.message, + }); + } + return res.json({ message: 'Test email sent successfully' }); + } + // Get email config const config = await db('email_configs').first(); diff --git a/backend/src/services/emailProcessor.js b/backend/src/services/emailProcessor.js index b91bcdec..8082bf06 100644 --- a/backend/src/services/emailProcessor.js +++ b/backend/src/services/emailProcessor.js @@ -7,6 +7,33 @@ const { normaliseSchedule, } = require('../utils/businessHours'); const { hasColumnCached } = require('../utils/schemaCache'); +const emailWebhookTransport = require('./emailWebhookTransport'); + +/** + * The From identity for an outbound message (#1225). + * + * `email_configs` holds it normally, but migration 001 seeds that row only when + * SMTP_HOST is set — so the install this feature exists for, a fresh one with + * no SMTP at all, has no row and every send would die on "Email configuration + * not found". Under the webhook transport the address therefore falls back to + * EMAIL_FROM, which already exists for config-as-code deploys. + * + * Returns null when nothing is configured, so callers keep their existing + * error. SMTP behaviour is unchanged: the fallback only applies in webhook mode. + */ +async function resolveFromIdentity() { + const config = await db('email_configs').first(); + if (config && config.from_email) { + return { fromEmail: config.from_email, fromName: config.from_name }; + } + if (emailWebhookTransport.isEnabled() && process.env.EMAIL_FROM) { + return { + fromEmail: process.env.EMAIL_FROM, + fromName: process.env.EMAIL_FROM_NAME || 'PicPeak', + }; + } + return null; +} let transporter = null; let lastConfigHash = null; @@ -105,6 +132,12 @@ async function getSupportEmail() { } catch (err) { logger.debug('getSupportEmail: email_configs lookup failed', { error: err.message }); } + // Same reason as resolveFromIdentity (#1225): a webhook-only install has no + // email_configs row, and returning '' here silently drops the support + // contact out of the archive and expiration templates that print it. + if (emailWebhookTransport.isEnabled() && process.env.EMAIL_FROM) { + return process.env.EMAIL_FROM; + } return ''; } @@ -705,10 +738,16 @@ async function processTemplate(template, variables, language = 'en') { // Send email using template async function sendTemplateEmail(to, templateKey, variables) { try { - // Always check for configuration changes before sending - transporter = await initializeTransporter(); - if (!transporter) { - throw new Error('Email service not configured'); + // Webhook transport (#1225) replaces SMTP entirely when configured, so an + // instance using it has no SMTP settings to initialise and must not be + // told it is "not configured". + const viaWebhook = emailWebhookTransport.isEnabled(); + if (!viaWebhook) { + // Always check for configuration changes before sending + transporter = await initializeTransporter(); + if (!transporter) { + throw new Error('Email service not configured'); + } } // Get email template @@ -720,10 +759,15 @@ async function sendTemplateEmail(to, templateKey, variables) { throw new Error(`Email template '${templateKey}' not found`); } - // Get email config for from address - const config = await db('email_configs').first(); - if (!config) { - throw new Error('Email configuration not found'); + // Get the From identity. Under the webhook transport this can come from + // EMAIL_FROM, because a webhook-only install has no email_configs row. + const identity = await resolveFromIdentity(); + if (!identity) { + throw new Error( + viaWebhook + ? 'No sender address configured — set EMAIL_FROM for the webhook transport' + : 'Email configuration not found' + ); } // Determine recipient language. An explicit `__language` in the email data @@ -755,15 +799,18 @@ async function sendTemplateEmail(to, templateKey, variables) { : undefined; // Send email - const info = await transporter.sendMail({ - from: `${config.from_name} <${config.from_email}>`, + const mail = { + from: `${identity.fromName} <${identity.fromEmail}>`, to: to, cc: ccList, subject: subject, html: htmlBody, text: textBody || htmlToText(htmlBody), attachments, - }); + }; + const info = viaWebhook + ? await emailWebhookTransport.send(mail) + : await transporter.sendMail(mail); logger.info(`Email sent successfully: ${info.messageId} (${language})`); // Return the rendered HTML so the queue processor can persist the ACTUAL @@ -804,13 +851,22 @@ async function sendRawEmail({ to, cc, subject, html, text, attachments, accountK fromName = acct.from_name || ''; } } + // Webhook transport (#1225) stands in for the GLOBAL transport only. A mail + // account with its own smtp_host above was configured deliberately for that + // identity, so it keeps sending through it rather than being silently + // redirected. + let viaWebhook = false; if (!tx) { - tx = await initializeTransporter(); - if (!tx) throw new Error('Email service not configured'); - const config = await db('email_configs').first(); - if (!config || !config.from_email) throw new Error('Email service not configured'); - fromEmail = config.from_email; - fromName = config.from_name; + const identity = await resolveFromIdentity(); + if (!identity) throw new Error('Email service not configured'); + fromEmail = identity.fromEmail; + fromName = identity.fromName; + if (emailWebhookTransport.isEnabled()) { + viaWebhook = true; + } else { + tx = await initializeTransporter(); + if (!tx) throw new Error('Email service not configured'); + } } const ccList = Array.isArray(cc) ? cc.filter(Boolean) : (cc ? [cc] : undefined); @@ -818,7 +874,7 @@ async function sendRawEmail({ to, cc, subject, html, text, attachments, accountK ? attachments.filter((a) => a && (a.contentPath || a.path || a.content)) .map((a) => ({ filename: a.filename, path: a.contentPath || a.path, content: a.content, contentType: a.contentType })) : undefined; - const info = await tx.sendMail({ + const mail = { from: `${fromName || 'picpeak'} <${fromEmail}>`, to, cc: ccList, @@ -826,7 +882,10 @@ async function sendRawEmail({ to, cc, subject, html, text, attachments, accountK html, text: text || htmlToText(html), attachments: atts, - }); + }; + const info = viaWebhook + ? await emailWebhookTransport.send(mail) + : await tx.sendMail(mail); logger.info(`Manual email sent: ${info.messageId}`); return { messageId: info.messageId, html }; } @@ -868,8 +927,12 @@ async function processEmailQueue({ ignoreSchedule = false, limit = 10, onlyId = const result = { processed: 0, sent: 0, failed: 0 }; try { - // Try to initialize transporter if it's null (in case it failed at startup) - if (!transporter) { + // Try to initialize transporter if it's null (in case it failed at startup). + // Skipped entirely under the webhook transport (#1225): that deploy has no + // SMTP settings to initialise, and this guard would otherwise return early + // and leave the queue permanently unprocessed — every email silently stuck + // pending, which is the whole feature dead rather than degraded. + if (!transporter && !emailWebhookTransport.isEnabled()) { logger.info('Transporter not initialized, attempting to initialize...'); transporter = await initializeTransporter(); if (!transporter) { @@ -1162,6 +1225,7 @@ function stopEmailQueueProcessor() { module.exports = { initializeTransporter, + resolveFromIdentity, startEmailQueueProcessor, sendTemplateEmail, sendRawEmail, diff --git a/backend/src/services/emailWebhookTransport.js b/backend/src/services/emailWebhookTransport.js new file mode 100644 index 00000000..8762edd4 --- /dev/null +++ b/backend/src/services/emailWebhookTransport.js @@ -0,0 +1,295 @@ +/** + * Webhook email transport (#1225). + * + * An alternative to SMTP: when EMAIL_WEBHOOK_URL is set, PicPeak POSTs the + * composed message to that URL instead of sending it, and something downstream + * (n8n, Make, a self-hosted relay) does the delivery. + * + * SMTP is the single most common thing people get stuck on in a self-hosted + * install — app passwords, 587 vs 465, providers that reject the sender, NAS + * boxes with no outbound 25. A webhook hands that problem to something the + * operator usually already runs. + * + * Configured by environment, NOT in the admin UI. That is deliberate: this + * setting redirects every outbound message, including password resets, so it + * should not be changeable by a compromised admin session. It also matches how + * the deploy that asked for this runs. + * + * Deliberately reuses the outbound-webhook primitives rather than growing a + * second set: the same HMAC scheme (signPayload / X-PicPeak-Signature) so a + * receiver verifies these exactly as it verifies gallery webhooks, and the same + * DNS-resolving SSRF preflight. + */ + +const fs = require('fs').promises; +const axios = require('axios'); +const logger = require('../utils/logger'); +const { signPayload } = require('./webhookService'); +const { validateExternalUrlAsync } = require('../utils/networkValidation'); + +const SIGNATURE_HEADER = 'X-PicPeak-Signature'; +const HTTP_TIMEOUT_MS = 15000; + +// Attachments are base64 in the JSON body, which inflates them by a third. +// Invoices and quotes are the real users of this and run to a few hundred KB; +// the cap exists so a pathological attachment cannot build a payload large +// enough to take the process down while serialising it. +const MAX_ATTACHMENT_BYTES = 10 * 1024 * 1024; + +// Same env-var shape as WEBHOOK_ALLOW_PRIVATE_URLS. Running n8n on the same +// Docker network or LAN is the normal case for a self-hosted install, and +// refusing private addresses outright would make this useless for exactly the +// people who asked for it — so it is opt-in rather than assumed. +let allowPrivateUrls = process.env.EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS === 'true'; + +// Logged once rather than per message: a misconfiguration is a property of the +// deploy, and one line per outbound email would bury it. +let warnedAboutMissingSecret = false; + +function config() { + return { + url: (process.env.EMAIL_WEBHOOK_URL || '').trim(), + secret: (process.env.EMAIL_WEBHOOK_SECRET || '').trim(), + }; +} + +/** + * Is the webhook transport configured and usable? + * + * A URL without a secret is treated as NOT enabled, and says so once. Sending + * unsigned would let anything that learns the URL feed the operator's + * automation — and every message PicPeak sends is one a receiver might act on. + * Failing back to SMTP here means a misconfigured deploy sends by its normal + * route rather than silently posting unauthenticated mail to the internet. + */ +function isEnabled() { + const { url, secret } = config(); + if (!url) return false; + if (!secret) { + if (!warnedAboutMissingSecret) { + warnedAboutMissingSecret = true; + logger.error( + '[email] EMAIL_WEBHOOK_URL is set but EMAIL_WEBHOOK_SECRET is not, so the ' + + 'webhook transport is disabled and mail will go over SMTP. Set a secret: ' + + 'the payload is signed with it (X-PicPeak-Signature), and without one ' + + 'anything that learns the URL could drive your automation.' + ); + } + return false; + } + return true; +} + +/** + * Turn nodemailer's attachment list into something a JSON body can carry. + * + * Callers pass `{ filename, path }` for a file already written to disk (quotes + * and invoices do this) or `{ filename, content }` for an in-memory buffer. + * Both become base64. + * + * Throws rather than dropping. The downstream implementation this was modelled + * on logged a warning and sent the body without its attachment, which turns + * "your invoice email failed" into "your customer received an empty invoice + * email" — a silent partial success is the worse outcome, and the email queue + * already surfaces and retries a throw. + */ +async function encodeAttachments(attachments) { + if (!Array.isArray(attachments) || attachments.length === 0) return []; + + const encoded = []; + let total = 0; + for (const att of attachments) { + if (!att) continue; + let buffer; + if (att.content) { + buffer = Buffer.isBuffer(att.content) ? att.content : Buffer.from(att.content); + } else if (att.path) { + // stat BEFORE reading. Checking the cap only after readFile means a file + // large enough to exhaust memory kills the process before the guard it is + // supposed to trip — the cap would exist and never fire. The post-read + // check below still applies, because the file can grow between the two. + const { size } = await fs.stat(att.path); + if (total + size > MAX_ATTACHMENT_BYTES) { + throw new Error( + `attachments exceed the ${Math.round(MAX_ATTACHMENT_BYTES / 1024 / 1024)}MB ` + + 'webhook payload limit; send this message over SMTP instead' + ); + } + buffer = await fs.readFile(att.path); + } else { + continue; + } + + total += buffer.length; + if (total > MAX_ATTACHMENT_BYTES) { + throw new Error( + `attachments exceed the ${Math.round(MAX_ATTACHMENT_BYTES / 1024 / 1024)}MB ` + + 'webhook payload limit; send this message over SMTP instead' + ); + } + + encoded.push({ + filename: att.filename, + content_type: att.contentType || 'application/octet-stream', + content_base64: buffer.toString('base64'), + }); + } + return encoded; +} + +/** + * Recipients as a flat list of single addresses. + * + * Splits inside array elements too, not just bare strings: sendRawEmail wraps a + * string cc in an array before it reaches here, so "a@x, b@y" arrives as ONE + * element. Passing that through would put a combined address in the payload, + * which a relay treating each element as one mailbox rejects or misaddresses. + */ +/** + * Read at most MAX_RESPONSE_BYTES from a response stream, then stop. + * + * Only a messageId is wanted, so the rest is dropped on the floor rather than + * buffered — a faulty or hostile receiver must not be able to grow this + * process's memory on every queue attempt. Any read failure resolves empty: + * the delivery verdict is the status code, which is already known by the time + * this runs, so a broken body must never turn a delivered message into a retry. + */ +function readBounded(stream) { + const MAX_RESPONSE_BYTES = 10 * 1024; + if (!stream || typeof stream.on !== 'function') return Promise.resolve(''); + return new Promise((resolve) => { + const chunks = []; + let size = 0; + const finish = () => resolve(Buffer.concat(chunks).toString('utf8')); + stream.on('data', (chunk) => { + size += chunk.length; + if (size <= MAX_RESPONSE_BYTES) { + chunks.push(chunk); + } else { + stream.destroy(); + finish(); + } + }); + stream.on('end', finish); + stream.on('error', () => resolve('')); + stream.on('close', finish); + }); +} + +function normalizeRecipients(value) { + if (!value) return []; + const parts = Array.isArray(value) ? value : [value]; + return parts + .filter(Boolean) + .flatMap((entry) => String(entry).split(/[,;]+/)) + .map((s) => s.trim()) + .filter(Boolean); +} + +/** + * POST a composed message to the configured webhook. + * + * @param {Object} mail nodemailer-shaped options (from/to/cc/subject/html/text/attachments) + * @returns {Promise<{ messageId: string }>} so callers match the sendMail contract + */ +async function send(mail) { + const { url, secret } = config(); + + // Vetted before every send, not once at startup: DNS answers change, and the + // check is what stops an operator-supplied URL becoming a request to link + // local metadata or a service on the host network. + if (!allowPrivateUrls) { + // https for anything leaving the machine. The HMAC proves who sent the + // body, not who can read it — and these bodies carry password-reset links + // and guest recovery codes, which are usable by anyone on the path. The + // private-network opt-in doubles as the plaintext opt-in, because http to + // a container on the same host is a different risk from http across the + // internet. + if (!/^https:\/\//i.test(url)) { + throw new Error( + 'EMAIL_WEBHOOK_URL must use https:// — the payload carries password-reset ' + + 'links and recovery codes, which plaintext exposes to anyone on the path. ' + + 'Set EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS=true only if the receiver is on a ' + + 'private network you trust.' + ); + } + const check = await validateExternalUrlAsync(url); + if (!check.valid) { + throw new Error( + `EMAIL_WEBHOOK_URL rejected: ${check.error}. Set ` + + 'EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS=true if the receiver really is on a ' + + 'private network (a container or LAN address).' + ); + } + } + + const payload = { + from: mail.from, + to: normalizeRecipients(mail.to), + cc: normalizeRecipients(mail.cc), + subject: mail.subject || '', + html: mail.html || '', + text: mail.text || '', + attachments: await encodeAttachments(mail.attachments), + }; + + // Signed over the exact bytes sent, so a receiver verifies what it received + // rather than a re-serialisation of it. + const rawBody = JSON.stringify(payload); + const signature = signPayload(secret, rawBody); + + const response = await axios.post(url, rawBody, { + headers: { + 'Content-Type': 'application/json', + [SIGNATURE_HEADER]: signature, + }, + timeout: HTTP_TIMEOUT_MS, + // Resolve on any status so a 4xx/5xx becomes our error message rather than + // axios's, which does not say which webhook failed. + validateStatus: () => true, + maxRedirects: 0, + // Streamed, NOT buffered with maxContentLength. axios enforces that limit + // while reading, so a receiver that delivered the mail and then echoed a + // large body would make this throw AFTER a successful delivery — the queue + // would retry and the recipient would get the same email again. Reading it + // ourselves means an oversized response costs us the messageId, never a + // duplicate send. + responseType: 'stream', + // Byte length, not String#length. axios enforces this against the UTF-8 + // buffer it sends, while rawBody.length counts UTF-16 code units — every + // umlaut is 2 bytes and every CJK character 3, so a German or Japanese + // message would blow past a code-unit budget and axios would reject it + // before posting. With base64 attachments in the body the gap is easily + // more than the slack. + maxBodyLength: Buffer.byteLength(rawBody, 'utf8') + 1024, + }); + + // Status first: it is the delivery verdict, and it is known before a single + // byte of the body is read. + const delivered = response.status >= 200 && response.status < 300; + const body = await readBounded(response.data); + if (!delivered) { + throw new Error(`email webhook returned ${response.status}`); + } + + // The queue stores a messageId for the record. There is no SMTP id here, so + // synthesise one that is obviously not from a mail server. + let reported = null; + try { + reported = body ? JSON.parse(body).messageId : null; + } catch { /* a receiver is not obliged to answer JSON */ } + return { messageId: reported || `webhook-${signature.slice(0, 16)}` }; +} + +module.exports = { + isEnabled, + send, + // Test seams, mirroring webhookDeliveryWorker's. + __testing: { + SIGNATURE_HEADER, + MAX_ATTACHMENT_BYTES, + encodeAttachments, + setAllowPrivateUrls(value) { allowPrivateUrls = !!value; }, + resetSecretWarning() { warnedAboutMissingSecret = false; }, + }, +}; diff --git a/backend/src/services/guestRecoveryService.js b/backend/src/services/guestRecoveryService.js index 971783ce..b7a07c9d 100644 --- a/backend/src/services/guestRecoveryService.js +++ b/backend/src/services/guestRecoveryService.js @@ -13,7 +13,8 @@ const crypto = require('crypto'); const bcrypt = require('bcrypt'); const { db } = require('../database/db'); const logger = require('../utils/logger'); -const { initializeTransporter, wrapEmailHtml } = require('./emailProcessor'); +const { initializeTransporter, wrapEmailHtml, resolveFromIdentity } = require('./emailProcessor'); +const emailWebhookTransport = require('./emailWebhookTransport'); const CODE_TTL_MS = 15 * 60 * 1000; const MAX_ATTEMPTS = 5; @@ -45,13 +46,22 @@ async function createCode(eventId, email) { } async function sendRecoveryEmail(toEmail, code, eventName = 'your gallery') { - const transporter = await initializeTransporter(); - if (!transporter) { - throw new Error('Email service not configured'); + // This composes its own message rather than going through a template, so it + // has to select the transport itself (#1225). Without this it called + // initializeTransporter() unconditionally and dereferenced the null it + // returns on a webhook-only install — recovery codes failed outright on + // exactly the deploys the webhook transport exists for. + const viaWebhook = emailWebhookTransport.isEnabled(); + let transporter = null; + if (!viaWebhook) { + transporter = await initializeTransporter(); + if (!transporter) { + throw new Error('Email service not configured'); + } } - const config = await db('email_configs').first(); - if (!config) { + const identity = await resolveFromIdentity(); + if (!identity) { throw new Error('Email configuration not found'); } @@ -70,13 +80,18 @@ async function sendRecoveryEmail(toEmail, code, eventName = 'your gallery') { `; const styledHtml = await wrapEmailHtml(htmlBody, subject, 'en'); - await transporter.sendMail({ - from: `${config.from_name} <${config.from_email}>`, + const mail = { + from: `${identity.fromName} <${identity.fromEmail}>`, to: toEmail, subject, html: styledHtml, text: `Your verification code is ${code}. It expires in 15 minutes.`, - }); + }; + if (viaWebhook) { + await emailWebhookTransport.send(mail); + } else { + await transporter.sendMail(mail); + } logger.info('Guest recovery code sent', { email: toEmail }); } diff --git a/backend/src/services/updateNotificationService.js b/backend/src/services/updateNotificationService.js index 30b45dec..053d3b9f 100644 --- a/backend/src/services/updateNotificationService.js +++ b/backend/src/services/updateNotificationService.js @@ -6,6 +6,7 @@ const { db } = require('../database/db'); const { checkForUpdates } = require('./updateCheckService'); const { sendTemplateEmail, initializeTransporter } = require('./emailProcessor'); +const emailWebhookTransport = require('./emailWebhookTransport'); const logger = require('../utils/logger'); const { getAbsoluteFrontendUrl } = require('../utils/frontendUrl'); @@ -126,7 +127,9 @@ async function checkAndNotifyUpdates() { } // Ensure email transporter is initialized - await initializeTransporter(); + // Skipped under the webhook transport (#1225): there is no SMTP to warm, + // and a stale unreachable config would sit on nodemailer's connect timeout. + if (!emailWebhookTransport.isEnabled()) await initializeTransporter(); // Send email to each recipient const frontendUrl = await getAbsoluteFrontendUrl(); @@ -215,7 +218,9 @@ async function sendTestUpdateNotification() { const channelLabel = updateInfo.channel === 'beta' ? 'Beta' : 'Stable'; - await initializeTransporter(); + // Skipped under the webhook transport (#1225): there is no SMTP to warm, + // and a stale unreachable config would sit on nodemailer's connect timeout. + if (!emailWebhookTransport.isEnabled()) await initializeTransporter(); let successCount = 0; let errorCount = 0; diff --git a/docker-compose.yml b/docker-compose.yml index aaa8e699..4ebb703a 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -63,6 +63,13 @@ services: - SMTP_USER=${SMTP_USER} - SMTP_PASS=${SMTP_PASS} - EMAIL_FROM=${EMAIL_FROM:-noreply@picpeak.local} + # Webhook email transport (#1225). Listed here because this service + # declares an explicit `environment:` block — a variable only present in + # .env is NOT passed through, so without these three the documented + # "uncomment in .env and restart" flow silently leaves the transport off. + - EMAIL_WEBHOOK_URL=${EMAIL_WEBHOOK_URL:-} + - EMAIL_WEBHOOK_SECRET=${EMAIL_WEBHOOK_SECRET:-} + - EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS=${EMAIL_WEBHOOK_ALLOW_PRIVATE_URLS:-false} # Unset by default (#705): an injected value would always win over the # `general_site_url` admin setting, so the setup wizard could never # take effect. Set this only to pin the origin from config-as-code.