diff --git a/addons/smtp/action.ts b/addons/smtp/action.ts index 20c258f..f8b8abb 100644 --- a/addons/smtp/action.ts +++ b/addons/smtp/action.ts @@ -8,10 +8,10 @@ import { import { CLI_BIN, PANEL_DB, STATE_DIR } from "../../cli/paths"; import { writeFileAtomic } from "../../lib/atomic-write"; import { - emptySmtpPolicy, parseRelay, parseRule, senderFor, senderGrants, smtpAddress, smtpDomain, + emptySmtpPolicy, parseRelay, parseRule, senderGrants, smtpAddress, smtpDomain, type SmtpPolicy, type SmtpRelay, type SmtpSiteRule, type SmtpSubmissionPolicy, } from "./config"; -import { SUBMISSION_POLICY_PATH } from "./submit"; +import { prepareSubmission, SUBMISSION_POLICY_PATH } from "./submit"; const MANAGED_POOL_LINE = `php_admin_value[sendmail_path] = ${CLI_BIN} smtp-submit -t -i`; const MANAGED_POOL_MARKER = "; clp-addons smtp relay"; @@ -49,7 +49,7 @@ export interface SmtpPaths { lockFile: string; submissionFile: string; postfixDir: string; - sendmail: string; + runuser: string; rootUid: number; } export interface SmtpActionOptions { @@ -69,7 +69,7 @@ export const DEFAULT_SMTP_PATHS: SmtpPaths = { lockFile: "/run/lock/clp-addons/smtp.lock", submissionFile: SUBMISSION_POLICY_PATH, postfixDir: "/etc/postfix", - sendmail: "/usr/sbin/sendmail", + runuser: "/usr/sbin/runuser", rootUid: 0, }; @@ -176,7 +176,7 @@ function stateOf(policy: SmtpPolicy, sites: SiteRow[]): SmtpState { return { domain: site.domain, user: site.user, phpVersion: site.phpVersion, rule, overridden: Boolean(policy.siteRules[site.domain]), - senderPreview: senderFor(rule.sender, site.domain), + senderPreview: rule.sender.replaceAll("{site}", site.domain), }; }), }; @@ -242,9 +242,7 @@ export function postfixMaps(policy: SmtpPolicy): { credentials: string; routes: function localSenderMap(policy: SmtpPolicy, sites: SiteRow[]): string { const entries = ["root *", "postfix *", "clp *"]; for (const site of sites) { - const rule = effectiveRule(policy, site.domain); - const grants = senderGrants({ domain: site.domain, rule }); - entries.push(`${site.user} ${[...grants.addresses, ...grants.domains.map((domain) => `@${domain}`)].join(" ")}`); + entries.push(`${site.user} ${senderGrants({ domain: site.domain, rule: effectiveRule(policy, site.domain) }).join(" ")}`); } return entries.join("\n") + "\n"; } @@ -479,24 +477,32 @@ async function inputBody(options: SmtpActionOptions): Promise; } -function testSubmission(policy: SmtpPolicy, sites: SiteRow[], body: Record): { domain: string; sender: string; recipient: string } { +export interface SmtpTestResult { queued: true; requested: string; sender: string; replyTo: string | null; recipient: string } + +function testSubmission(policy: SmtpPolicy, sites: SiteRow[], body: Record): { site: SiteRow; message: Buffer; result: SmtpTestResult } { if (!policy.relay) failAction("configure the global SMTP relay first"); const domain = smtpDomain(body.domain); const site = sites.find((item) => item.domain === domain); if (!site) failAction(`CloudPanel has no PHP site ${domain}`); const recipient = smtpAddress(body.recipient); - const sender = senderFor(effectiveRule(policy, domain).sender, domain); - return { domain, sender, recipient }; -} - -function sendTest(paths: SmtpPaths, policy: SmtpPolicy, sites: SiteRow[], body: Record): { queued: true; sender: string; recipient: string } { - const { domain, sender, recipient } = testSubmission(policy, sites, body); - const message = `To: ${recipient}\nFrom: ${sender}\nSubject: CloudPanel SMTP relay test for ${domain}\n\nThis message was submitted through the CloudPanel Addons Postfix relay.\n`; - const result = Bun.spawnSync([paths.sendmail, "-t", "-i", "-f", sender], { - stdin: Buffer.from(message), stdout: "pipe", stderr: "pipe", maxBuffer: 64 * 1024, + const requested = body.from === undefined || body.from === "" ? `wordpress@${domain}` : smtpAddress(body.from); + const message = Buffer.from(`To: ${recipient}\nFrom: ${requested}\nSubject: CloudPanel SMTP relay test for ${domain}\n\nThis message was sent through PHP's mail path for ${domain} and the CloudPanel Addons Postfix relay.\n`); + const prepared = prepareSubmission(message, { domain, uid: site.uid, user: site.user, rule: effectiveRule(policy, domain) }); + const replyTo = Buffer.from(prepared.message).toString("utf8").match(/^Reply-To: (.+)$/m)?.[1] ?? null; + return { site, message, result: { queued: true, requested, sender: prepared.sender, replyTo, recipient } }; +} + +/** Submits as the site's Unix account through the same command its PHP pool uses. */ +function sendTest(paths: SmtpPaths, policy: SmtpPolicy, sites: SiteRow[], body: Record): SmtpTestResult { + const { site, message, result } = testSubmission(policy, sites, body); + const submit = Bun.spawnSync([paths.runuser, "-u", site.user, "--", CLI_BIN, "smtp-submit", "-t", "-i"], { + stdin: message, stdout: "pipe", stderr: "pipe", maxBuffer: 64 * 1024, timeout: 30_000, }); - if (!result.success) failAction(`Postfix did not queue the test: ${Buffer.from(result.stderr).toString("utf8").trim() || "sendmail failed"}`); - return { queued: true, sender, recipient }; + if (!submit.success) { + const detail = Buffer.from(submit.stderr).toString("utf8").trim().replace(/^\[smtp\] /, ""); + failAction(`${site.domain}'s PHP mail path did not queue the test: ${detail || "submission failed"}`); + } + return result; } export async function executeSmtpAction(argv: string[], options: SmtpActionOptions = {}): Promise { diff --git a/addons/smtp/app/index.ts b/addons/smtp/app/index.ts index 0a028cf..a13a7e8 100644 --- a/addons/smtp/app/index.ts +++ b/addons/smtp/app/index.ts @@ -1,6 +1,6 @@ import { bodyErrorResponse, guardMutation, htmlResponse, jsonResponse, newCsrfToken, readJsonObject } from "../../../lib/app-http"; import { smtpService } from "./service"; -import { dashboardView, layout } from "./views"; +import { dashboardContent, dashboardView, layout } from "./views"; export async function handle(req: Request, path: string, notice?: { current: string; latest: string } | null): Promise { if (req.method === "GET" && path === "/") { @@ -20,6 +20,10 @@ export async function handle(req: Request, path: string, notice?: { current: str if (denied) return denied; let body: Record; try { body = await readJsonObject(req); } catch (error) { return bodyErrorResponse(error); } + if (path === "/api/test") { + const result = await smtpService.test(body); + return jsonResponse(result, { status: result.ok ? 200 : 400 }); + } const result = path === "/api/setup" ? await smtpService.saveSetup(body) : path === "/api/relay" ? await smtpService.saveRelay(body) : path === "/api/default" ? await smtpService.saveDefault(body) @@ -27,9 +31,9 @@ export async function handle(req: Request, path: string, notice?: { current: str : path === "/api/site/clear" ? await smtpService.clearSite(body) : path === "/api/domain-relay" ? await smtpService.saveDomainRelay(body) : path === "/api/domain-relay/clear" ? await smtpService.clearDomainRelay(body) - : path === "/api/test" ? await smtpService.test(body) : null; - if (result) return jsonResponse(result, { status: result.ok ? 200 : 400 }); + if (result?.ok && result.data) return jsonResponse({ ...result, html: dashboardContent(result.data) }); + if (result) return jsonResponse(result, { status: 400 }); } return jsonResponse({ ok: false, error: "not found" }, { status: 404 }); } diff --git a/addons/smtp/app/service.ts b/addons/smtp/app/service.ts index ff4c179..d73208e 100644 --- a/addons/smtp/app/service.ts +++ b/addons/smtp/app/service.ts @@ -1,5 +1,5 @@ import { callGatewayAction, type ActionResult } from "../../../lib/gateway-client"; -import type { SmtpState } from "../action"; +import type { SmtpState, SmtpTestResult } from "../action"; const call = (verb: string, body?: unknown): Promise> => callGatewayAction("smtp", verb, [], body === undefined ? undefined : JSON.stringify(body)); @@ -13,6 +13,6 @@ export const smtpService = { clearSite: (body: unknown) => call("clear-site", body), saveDomainRelay: (body: unknown) => call("save-domain-relay", body), clearDomainRelay: (body: unknown) => call("clear-domain-relay", body), - test: (body: unknown): Promise> => + test: (body: unknown): Promise> => callGatewayAction("smtp", "test", [], JSON.stringify(body)), }; diff --git a/addons/smtp/app/views.client.js b/addons/smtp/app/views.client.js index 99669c7..6062f44 100644 --- a/addons/smtp/app/views.client.js +++ b/addons/smtp/app/views.client.js @@ -1,18 +1,21 @@ -const smtpState = JSON.parse(document.getElementById('smtp-state')?.textContent || '{}'); +let smtpState = JSON.parse(document.getElementById('smtp-state')?.textContent || '{}'); function smtpFields(form) { return Object.fromEntries(new FormData(form).entries()); } -async function smtpPost(path, body) { +async function smtpPost(path, body, done) { busy(true); try { - await call(path, { + const reply = await call(path, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify(body), }); - location.reload(); + for (const dialog of document.querySelectorAll('dialog[open]')) dialog.close(); + document.getElementById('smtp-root').innerHTML = reply.html; + smtpState = reply.data; + notify(done, 'ok'); } catch (error) { notify(error.message, 'error'); } finally { @@ -29,9 +32,8 @@ function smtpSaveSetup(event) { const fields = smtpFields(event.currentTarget); smtpPost('/api/setup', { relay: smtpRelayPayload(fields), - rule: { mode: fields.mode, sender: fields.sender, - domains: smtpState.defaultRule.domains, addresses: smtpState.defaultRule.addresses }, - }); + rule: { sender: fields.sender, domains: [] }, + }, 'Saved the relay and default From.'); } async function smtpSendTest(event) { @@ -41,9 +43,11 @@ async function smtpSendTest(event) { try { const result = await call('/api/test', { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ domain: fields.domain, recipient: fields.recipient }), + body: JSON.stringify({ domain: fields.domain, recipient: fields.recipient, from: fields.from }), }); - notify('Postfix queued a test from ' + result.data.sender + ' to ' + result.data.recipient + '.', 'ok'); + const data = result.data; + notify('Postfix queued the test to ' + data.recipient + '. The app asked for ' + data.requested + + ' and it was sent as ' + data.sender + (data.replyTo ? ', with Reply-To ' + data.replyTo : '') + '.', 'ok'); } catch (error) { notify(error.message, 'error'); } finally { @@ -60,39 +64,32 @@ function smtpEditSite(domain) { if (!site) return; const form = document.getElementById('smtp-site-form'); form.elements.namedItem('domain').value = domain; - form.elements.namedItem('mode').value = site.rule.mode; form.elements.namedItem('sender').value = site.rule.sender; form.elements.namedItem('domains').value = site.rule.domains.join('\n'); - form.elements.namedItem('addresses').value = site.rule.addresses.join('\n'); - smtpSyncSiteMode(); - document.getElementById('smtp-site-heading').textContent = 'Sender policy for ' + domain; + smtpSyncSiteDomains(); + document.getElementById('smtp-site-heading').textContent = 'From for ' + domain; document.getElementById('smtp-clear-site').hidden = !site.overridden; document.getElementById('smtp-site-dialog').showModal(); } -function smtpSyncSiteMode() { +function smtpSyncSiteDomains() { const form = document.getElementById('smtp-site-form'); - const allow = form.elements.namedItem('mode').value === 'allow'; - document.getElementById('smtp-site-allow-fields').hidden = !allow; - for (const name of ['domains', 'addresses']) { - const field = form.elements.namedItem(name); - if (!allow) field.value = ''; - field.disabled = !allow; - } + const keepsDomain = form.elements.namedItem('sender').value.includes('{from.domain}'); + document.getElementById('smtp-site-domains').hidden = !keepsDomain; + form.elements.namedItem('domains').disabled = !keepsDomain; } function smtpSaveSite(event) { event.preventDefault(); const fields = smtpFields(event.currentTarget); smtpPost('/api/site', { domain: fields.domain, rule: { - mode: fields.mode, sender: fields.sender, - domains: smtpList(fields.domains), addresses: smtpList(fields.addresses), - } }); + sender: fields.sender, domains: smtpList(fields.domains), + } }, 'Saved the From for ' + fields.domain + '.'); } function smtpClearSite() { const domain = document.getElementById('smtp-site-form').elements.namedItem('domain').value; - smtpPost('/api/site/clear', { domain: domain }); + smtpPost('/api/site/clear', { domain: domain }, domain + ' uses the default From again.'); } function smtpEditDomain(domain) { @@ -112,10 +109,10 @@ function smtpEditDomain(domain) { function smtpSaveDomain(event) { event.preventDefault(); const fields = smtpFields(event.currentTarget); - smtpPost('/api/domain-relay', { domain: fields.domain, relay: smtpRelayPayload(fields) }); + smtpPost('/api/domain-relay', { domain: fields.domain, relay: smtpRelayPayload(fields) }, 'Saved the relay for ' + fields.domain + '.'); } async function smtpClearDomain(domain) { if (!await confirmAction({ title: 'Remove SMTP override?', text: domain + ' will use the global relay credential again.', confirmLabel: 'Remove' })) return; - smtpPost('/api/domain-relay/clear', { domain: domain }); + smtpPost('/api/domain-relay/clear', { domain: domain }, domain + ' uses the global relay again.'); } diff --git a/addons/smtp/app/views.ts b/addons/smtp/app/views.ts index ff980b8..c389a30 100644 --- a/addons/smtp/app/views.ts +++ b/addons/smtp/app/views.ts @@ -11,17 +11,19 @@ export function layout(title: string, content: string, notice?: { current: strin return renderLayout(title, content, { brand: "SMTP Relay", base: BASE, nav: [], css: CSS, script: CLIENT, updateNotice: notice }); } -function modeOptions(mode: string): string { - return ``; -} +const TEMPLATE_HINT = "{site} is the site's domain. {from.local} and {from.domain} are the name and domain of the From the app asked for, when it is on the site's domain, so {from.local}@{site} keeps wordpress@ or orders@ on the site's domain."; export function dashboardView(state: SmtpState): string { + return `
${dashboardContent(state)}
`; +} + +/** Everything a save can change, repainted in place from the action's reply. */ +export function dashboardContent(state: SmtpState): string { const data = JSON.stringify(state).replaceAll("<", "\\u003c"); const relay = state.relay; const sites = state.sites.map((site) => ` ${esc(site.domain)}${esc(site.user)} - ${site.rule.mode === "force" ? "Force" : "Allow listed"}${site.overridden ? ' Override' : ""} - ${esc(site.senderPreview)}${site.rule.mode === "allow" ? `Also allowed: ${[site.domain, ...site.rule.domains].map((domain) => `@${esc(domain)}`).concat(site.rule.addresses.map(esc)).join(", ")}` : ""} + ${esc(site.senderPreview)}${site.overridden ? ' Override' : ""}${site.rule.domains.length ? `{from.domain} may also be ${site.rule.domains.map((domain) => esc(domain)).join(", ")}` : ""} `).join(""); const overrides = Object.entries(state.relayOverrides).map(([domain, item]) => ` @@ -29,39 +31,38 @@ export function dashboardView(state: SmtpState): string { `).join(""); return ` -

SMTP relay

Send WordPress and other PHP mail through Postfix, with a sender policy for each site.

+

SMTP relay

Send WordPress and other PHP mail through Postfix, with a From rule for each site.

-

Relay and default sender

Set the fallback SMTP account and sender rule for every PHP site in one save.

${relay ? "Configured" : "Setup needed"}
+

Relay and default sender

Set the fallback SMTP account and the From every PHP site uses unless it has its own.

${relay ? "Configured" : "Setup needed"}

Global SMTP relay

Used for sending domains without a relay override. Postfix queues messages and retries temporary failures.

-

Default sender policy

{domain} is each CloudPanel site's domain. Force replaces the requested From address; allow listed preserves addresses on that site's approved domains.

-
-
+

Default From

${TEMPLATE_HINT}

+
-

Send a test

Submits directly to Postfix as root using the selected site's configured sender. It does not test PHP mail or that site's sender permissions. A successful result means Postfix queued the message; check the inbox for delivery.

+

Send a test

Sends as the site's own account through the same path as PHP mail(), so the site's From rule applies. A successful result means Postfix queued the message; check the inbox for delivery.

+
-

PHP sites

Edit a site to add sending domains or use a different address.

- ${sites ? `${sites}
SiteModeFrom policyActions
` : '
No PHP sites found.
'} +

PHP sites

Edit a site to give it its own From.

+ ${sites ? `${sites}
SiteFromActions
` : '
No PHP sites found.
'}

Sending domain relays

Use a different SMTP account for a domain whose provider does not allow the global credential to send as it.

${overrides ? `${overrides}
Sending domainSMTP hostUsernameActions
` : '
All sending domains use the global relay.
'}

Site sender

- - - + +

${TEMPLATE_HINT}

+
diff --git a/addons/smtp/config.ts b/addons/smtp/config.ts index 5696386..4a73521 100644 --- a/addons/smtp/config.ts +++ b/addons/smtp/config.ts @@ -1,12 +1,9 @@ /** The public sender policy is separate from the root-only SMTP credentials. */ export interface SmtpSiteRule { - mode: "force" | "allow"; - /** Used by force mode, and as the fallback when an allowed message has no From. */ + /** From template: {from.local} and {from.domain} come from the app's From, {site} is the site's domain. */ sender: string; - /** Extra sending domains explicitly granted to this CloudPanel site. */ + /** Domains {from.domain} may keep besides the site's own. */ domains: string[]; - /** Exact additional addresses, for providers that authorize individual senders. */ - addresses: string[]; } export interface SmtpRelay { @@ -40,9 +37,8 @@ export interface SmtpSubmissionPolicy { const DOMAIN = /^[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)+$/; const ADDRESS = /^[A-Za-z0-9._%+-]+@([a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?(?:\.[a-z0-9](?:[a-z0-9-]{0,61}[a-z0-9])?)+)$/i; -export const DEFAULT_RULE: SmtpSiteRule = { - mode: "force", sender: "noreply@{domain}", domains: [], addresses: [], -}; +export const DEFAULT_RULE: SmtpSiteRule = { sender: "noreply@{site}", domains: [] }; +export const FALLBACK_LOCAL = "noreply"; export function emptySmtpPolicy(): SmtpPolicy { return { version: 1, relay: null, relayOverrides: {}, defaultRule: { ...DEFAULT_RULE }, siteRules: {} }; @@ -62,28 +58,39 @@ export function smtpAddress(value: unknown): string { return value.toLowerCase(); } -export function senderFor(template: string, domain: string): string { - if (typeof template !== "string" || template.length > 254 || - template.replaceAll("{domain}", "").includes("{")) { - throw new Error("sender template may contain only {domain}"); +export interface RequestedSender { local: string; domain: string } + +export function senderFor(template: string, site: string, from?: RequestedSender | null): string { + return smtpAddress(template + .replaceAll("{from.local}", from?.local ?? FALLBACK_LOCAL) + .replaceAll("{from.domain}", from?.domain ?? site) + .replaceAll("{site}", site)); +} + +export function parseSenderTemplate(value: unknown): string { + if (typeof value !== "string" || value.length > 254) throw new Error("From address is required"); + const template = value.trim().toLowerCase(); + const at = template.lastIndexOf("@"); + const local = template.slice(0, at); + const domain = template.slice(at + 1); + if (["{from.local}", "{from.domain}", "{site}"].reduce((rest, token) => rest.replaceAll(token, ""), template).match(/[{}]/)) { + throw new Error("From may use only {from.local}, {from.domain} and {site}"); + } + if (at < 1 || local.includes("{from.domain}") || domain.includes("{from.local}") || + (domain.includes("{from.domain}") && domain !== "{from.domain}")) { + throw new Error("From must be name@domain, with {from.local} before the @ and {from.domain} as the whole domain"); } - return smtpAddress(template.replaceAll("{domain}", domain)); + senderFor(template, "example.com", { local: "wordpress", domain: "example.com" }); + return template; } export function parseRule(value: unknown): SmtpSiteRule { if (!value || typeof value !== "object" || Array.isArray(value)) throw new Error("sender rule must be an object"); const rule = value as Record; - if (rule.mode !== "force" && rule.mode !== "allow") throw new Error("sender mode must be force or allow"); - if (typeof rule.sender !== "string") throw new Error("sender template is required"); - senderFor(rule.sender, "example.com"); - if (!Array.isArray(rule.domains) || !Array.isArray(rule.addresses) || - rule.domains.length > 30 || rule.addresses.length > 100) throw new Error("too many allowed senders"); - return { - mode: rule.mode, - sender: rule.sender.toLowerCase(), - domains: rule.mode === "allow" ? [...new Set(rule.domains.map(smtpDomain))].sort() : [], - addresses: rule.mode === "allow" ? [...new Set(rule.addresses.map(smtpAddress))].sort() : [], - }; + const sender = parseSenderTemplate(rule.sender); + const domains = rule.domains ?? []; + if (!Array.isArray(domains) || domains.length > 30) throw new Error("too many allowed domains"); + return { sender, domains: sender.includes("{from.domain}") ? [...new Set(domains.map(smtpDomain))].sort() : [] }; } export function parseRelay(value: unknown): SmtpRelay { @@ -102,18 +109,17 @@ export function parseRelay(value: unknown): SmtpRelay { return { host, port: Number(relay.port), username: relay.username, password: relay.password }; } -export function senderGrants(site: Pick): { addresses: string[]; domains: string[] } { - const addresses = [senderFor(site.rule.sender, site.domain)]; - if (site.rule.mode === "force") return { addresses, domains: [] }; - return { - addresses: [...new Set([...addresses, ...site.rule.addresses])], - domains: [...new Set([site.domain, ...site.rule.domains])], - }; +/** Domains the app's own From domain may keep; anything else is foreign. */ +export function allowedDomains(site: Pick): string[] { + return [...new Set([site.domain, ...site.rule.domains])]; } -export function permittedSender(site: SmtpSubmissionSite, address: string): boolean { - const sender = smtpAddress(address); - const domain = sender.slice(sender.lastIndexOf("@") + 1); - const grants = senderGrants(site); - return grants.addresses.includes(sender) || grants.domains.includes(domain); +/** Envelope senders a site's Unix account may use, as Postfix sender map entries. */ +export function senderGrants(site: Pick): string[] { + const { sender } = site.rule; + const domains = sender.includes("{from.domain}") ? allowedDomains(site) : [site.domain]; + return [...new Set(domains.map((domain) => { + const address = senderFor(sender, site.domain, { local: FALLBACK_LOCAL, domain }); + return sender.includes("{from.local}") ? address.slice(address.lastIndexOf("@")) : address; + }))]; } diff --git a/addons/smtp/submit.ts b/addons/smtp/submit.ts index fb639aa..263a229 100644 --- a/addons/smtp/submit.ts +++ b/addons/smtp/submit.ts @@ -1,6 +1,6 @@ import { lstatSync, readFileSync } from "node:fs"; import { CONFIG_DIR } from "../../cli/paths"; -import { permittedSender, senderFor, smtpAddress, type SmtpSubmissionPolicy, type SmtpSubmissionSite } from "./config"; +import { allowedDomains, senderFor, smtpAddress, type SmtpSubmissionPolicy, type SmtpSubmissionSite } from "./config"; export const SUBMISSION_POLICY_PATH = `${CONFIG_DIR}/smtp-submission.json`; export const MAX_MESSAGE_BYTES = 25 * 1024 * 1024; @@ -17,12 +17,18 @@ function trustedPolicy(path: string): SmtpSubmissionPolicy { return value as SmtpSubmissionPolicy; } -function senderFromHeader(value: string): string { - const unfolded = value.replace(/\r?\n[ \t]+/g, " ").trim(); - const bracketed = unfolded.match(/<([^<>]+)>$/); - const address = bracketed ? bracketed[1] : unfolded; - if (!address || address.includes(",") || /[\r\n]/.test(address)) throw new Error("message has an invalid From address"); - return smtpAddress(address); +/** The app's requested From, or null when it is missing or not one plain address. */ +function requestedFrom(raw: string | null): { display: string; address: string } | null { + if (raw === null) return null; + const unfolded = raw.replace(/\r?\n[ \t]+/g, " ").trim(); + const bracketed = unfolded.match(/^([^<>]*)<([^<>]+)>$/); + try { + return bracketed + ? { display: bracketed[1]!.trim(), address: smtpAddress(bracketed[2]!.trim()) } + : { display: "", address: smtpAddress(unfolded) }; + } catch { + return null; + } } /** Stops reading stdin at the first chunk that crosses the submission limit. */ @@ -63,6 +69,7 @@ export function prepareSubmission(message: Uint8Array, site: SmtpSubmissionSite) let fromRaw: string | null = null; let lastWasFrom = false; let lastWasIgnored = false; + let hasReplyTo = false; for (let i = 0; i < headers.length; i++) { const line = headers[i]!; if (/^[ \t]/.test(line)) { @@ -77,6 +84,7 @@ export function prepareSubmission(message: Uint8Array, site: SmtpSubmissionSite) const name = line.slice(0, colon).toLowerCase(); lastWasFrom = name === "from"; lastWasIgnored = name === "sender" || name === "return-path"; + if (name === "reply-to") hasReplyTo = true; if (name === "from") { if (fromRaw !== null) throw new Error("message has multiple From headers"); fromRaw = line.slice(colon + 1); @@ -84,13 +92,22 @@ export function prepareSubmission(message: Uint8Array, site: SmtpSubmissionSite) kept.push(line); } } - const configured = senderFor(site.rule.sender, site.domain); - const from = site.rule.mode === "allow" && fromRaw !== null ? senderFromHeader(fromRaw) : null; - const sender = site.rule.mode === "force" ? configured : (from ?? configured); - if (site.rule.mode === "allow" && !permittedSender(site, sender)) { - throw new Error(`sender ${sender} is not allowed for ${site.domain}`); + const from = requestedFrom(fromRaw); + const at = from ? from.address.lastIndexOf("@") : -1; + const local = from ? from.address.slice(0, at) : ""; + const domain = from ? from.address.slice(at + 1) : ""; + const allowed = allowedDomains(site); + let sender: string; + try { + sender = senderFor(site.rule.sender, site.domain, from && allowed.includes(domain) ? { local, domain } : null); + } catch { + sender = senderFor(site.rule.sender, site.domain); + } + // A contact form's visitor address cannot be the From, but replies should still reach them. + if (from && !hasReplyTo && !allowed.includes(domain)) { + kept.unshift(`Reply-To: ${from.address}`); } - kept.unshift(`From: ${sender}`); + kept.unshift(`From: ${from?.display ? `${from.display} <${sender}>` : sender}`); const head = Buffer.from(kept.join(newline) + newline + newline, "utf8"); return { message: Buffer.concat([head, input.subarray(boundary + separatorLength)]), sender }; } diff --git a/docs/decisions/smtp.md b/docs/decisions/smtp.md index ff0c35d..0005b4f 100644 --- a/docs/decisions/smtp.md +++ b/docs/decisions/smtp.md @@ -11,10 +11,17 @@ envelope sender. This covers default WordPress PHPMailer behavior and other PHP `mail()` callers without changing application files. Applications with their own SMTP transports remain outside this path. -The default rule forces `noreply@{domain}`. A site may instead allow senders on -its own domain, additional administrator-approved domains, or exact addresses. -Forcing an address supports a relay account allowed to send as many domains; -allowing senders supports applications that need their own From identities. +Each site's From is a template, `noreply@{site}` by default. `{site}` is the +site's domain; `{from.local}` and `{from.domain}` are the two halves of the From +the application requested, used only when its domain is the site's own or one +an administrator granted that site. `{from.domain}` must be the whole domain. +For any other, missing, unparsable, or overlong requested From, the tokens +resolve to `noreply` and the site's domain, so a contact form's visitor never +appears as a mailbox on the site's domain. The template is the whole policy, so +no submission is rejected for its sender. A requested address on a domain the +site may not use moves to `Reply-To` unless the message already has one. The +requested display name is kept. + The global SMTP credential is the fallback. Optional relay overrides are keyed by *sending* domain, so a site can use a separate provider for mail it is authorized to send from that domain. @@ -45,8 +52,9 @@ The global and per-domain credentials are stored in `regexp:` credential and route maps under `/etc/postfix`; the credential map is `0600`. Local Unix sender restrictions use a `hash:` map. The trusted `root`, `postfix`, and CloudPanel `clp` accounts retain unrestricted local envelope -senders; site accounts get only their configured senders. Other local Unix -accounts are not granted Postfix sendmail access by this addon. The public +senders; site accounts get only the envelope senders their template can +produce: an exact address, or a whole domain when the template keeps +`{from.local}`. Other local Unix accounts are not granted Postfix sendmail access by this addon. The public submission policy lives at `/etc/clp-addons/smtp-submission.json` and contains no SMTP passwords. The manager receives only relay host, port, username, and a has-password flag, never the saved password. @@ -64,7 +72,7 @@ saved addon policy remains for a later enable. The root gateway accepts only named SMTP verbs. The action validates a sending domain against CloudPanel's PHP sites before changing a site rule, and validates -every relay host, address, template, and allowlist entry. It rejects multiple +every relay host, address, template, and granted domain. It rejects multiple sites sharing one Unix UID because their submissions could not be distinguished. Request data is checked before taking the SMTP lock and checked again against current site and policy state under the lock. @@ -77,8 +85,10 @@ pool. Direct Postfix submission from a site account is restricted at the envelope only; a process with shell access can still write an arbitrary From header, and another local SMTP listener can bypass the Unix account check. This addon does not promise isolation against a hostile tenant with direct -process or network access. The test-mail action queues a message through -Postfix as root to test relay delivery; it does not exercise the PHP wrapper. +process or network access. The test-mail action submits as the site's Unix +account with `runuser` through the same `smtp-submit` command the pool uses, so +it exercises the template, the submission policy, and Postfix's local sender +check. The regular repair timer reapplies the policy to new or recreated PHP pools every 15 minutes. Until then, a new site's mail may fail the Postfix local diff --git a/docs/smtp-relay.md b/docs/smtp-relay.md index 448c3cf..45328f4 100644 --- a/docs/smtp-relay.md +++ b/docs/smtp-relay.md @@ -13,13 +13,21 @@ there is no WordPress plugin to install. An existing Postfix must be version 3.6 or newer with Cyrus SASL client support. 2. Open **SMTP Relay** and enter the SMTP hostname, STARTTLS submission port - (normally 587), username, and password. Choose a default sender in the same - form, then save both settings together. `noreply@{domain}` becomes - `noreply@example.com` for the `example.com` site. **Force one address** - replaces an application's requested From address. **Allow site domains** - preserves From addresses on that site's domain and on any domains or exact - addresses explicitly granted in the site's editor. Switching a site to - Force clears its additional grants. + (normally 587), username, and password. Choose the default From in the same + form, then save both together. In the From template, `{site}` is the site's + domain, and `{from.local}` and `{from.domain}` are the name and domain of the + From the application asked for: + + | From template | WordPress asks for `wordpress@example.com` | A form asks for `jane@gmail.com` | + |---|---|---| + | `noreply@{site}` | `noreply@example.com` | `noreply@example.com` | + | `{from.local}@{site}` | `wordpress@example.com` | `noreply@example.com` | + | `{from.local}@{from.domain}` | `wordpress@example.com` | `noreply@example.com` | + + The requested From is used only when its domain is the site's own or one + granted in the site's editor. Otherwise, or when the application asks for no + From, `{from.local}` is `noreply`, `{from.domain}` is the site's domain, and + a requested address is added as `Reply-To` so replies still reach it. If saving reports a Postfix routing conflict, resolve the named `transport_maps`, `sender_dependent_default_transport_maps`, `default_transport`, or `relay_transport` setting first. Those settings can @@ -28,11 +36,12 @@ there is no WordPress plugin to install. relay**. All other senders use the global account. The relay's SMTP provider must allow the resulting From address; configuring the addon does not create mailboxes, authorize senders at the provider, or set DNS records. -4. Send a test to an inbox you control. The test submits directly to Postfix - as root with the selected site's configured sender; it does not exercise - the site's PHP path. The page confirms that Postfix queued the message; - check the inbox and, if needed, `/var/log/mail.log` and - `postqueue -p` for the delivery result. +4. Send a test to an inbox you control. The test sends as the site's own + account through the same path as PHP `mail()`, with `wordpress@` the site's + domain as the requested From unless you enter another. The page shows the + From it was sent as and confirms that Postfix queued it; check the inbox + and, if needed, `/var/log/mail.log` and `postqueue -p` for the delivery + result. For Mailcow, one mailbox credential can be used as the global relay when that mailbox is explicitly permitted to send as all intended domains. Otherwise use @@ -41,18 +50,16 @@ receive mail; the addon does not replace them. ## Behavior and limits -The sender rule applies to PHP `mail()` in CloudPanel's PHP-FPM site pools. In -force mode, the addon sets both the visible From and envelope sender to the -configured address. In allow mode, it rejects a requested From outside the -site's own domain and its approved senders. Only CloudPanel administrators can -grant additional domains or addresses. A site cannot use another site's domain -through this PHP mail path unless an administrator grants it. +The From template applies to PHP `mail()` in CloudPanel's PHP-FPM site pools +and sets both the visible From and the envelope sender. Only CloudPanel +administrators can grant a site additional domains. A site cannot use another +site's domain through this PHP mail path unless an administrator grants it. The addon does not alter applications that open their own SMTP connection. It also does not filter arbitrary mail submitted directly to Postfix or another local SMTP listener. Postfix restricts local envelope senders for known site Unix accounts, but that check does not validate the message's visible From -header. Treat the site sender rule as a policy for PHP `mail()` and configure +header. Treat the site's From template as a policy for PHP `mail()` and configure untrusted shell or SMTP access separately. Local submissions from other Unix accounts are restricted, except for Postfix, root, and CloudPanel's `clp` account. diff --git a/tests/test-smtp.test.ts b/tests/test-smtp.test.ts index d2dfa5f..74844d4 100644 --- a/tests/test-smtp.test.ts +++ b/tests/test-smtp.test.ts @@ -3,7 +3,7 @@ import { chmodSync, existsSync, mkdtempSync, mkdirSync, readFileSync, rmSync, st import { tmpdir } from "node:os"; import { join } from "node:path"; import { executeSmtpAction, postfixMaps, type SmtpActionOptions, type SmtpState } from "../addons/smtp/action"; -import { emptySmtpPolicy, parseRule, type SmtpSubmissionSite } from "../addons/smtp/config"; +import { emptySmtpPolicy, parseRule, senderGrants, type SmtpSubmissionSite } from "../addons/smtp/config"; import { MAX_MESSAGE_BYTES, prepareSubmission, readBoundedSubmission } from "../addons/smtp/submit"; import { dashboardView } from "../addons/smtp/app/views"; @@ -12,37 +12,73 @@ afterEach(() => { for (const dir of dirs.splice(0)) rmSync(dir, { recursive: tru const site: SmtpSubmissionSite = { domain: "example.com", user: "example", uid: 1234, - rule: { mode: "force", sender: "noreply@{domain}", domains: [], addresses: [] }, + rule: { sender: "noreply@{site}", domains: [] }, }; -test("forced sender replaces a foreign From and sets the envelope identity", () => { - const raw = Buffer.from("To: user@recipient.test\r\nFrom: noreply@cool.com\r\nSubject: Reset\r\nReturn-Path: forged@cool.com\r\n\r\nHello", "utf8"); +const submit = (rule: SmtpSubmissionSite["rule"], headers: string) => { + const result = prepareSubmission(Buffer.from(`To: user@recipient.test\n${headers}\n\nHi`), { ...site, rule }); + return { sender: result.sender, text: Buffer.from(result.message).toString("utf8") }; +}; + +test("a fixed From replaces the app's and keeps a foreign address reachable", () => { + const raw = Buffer.from("To: user@recipient.test\r\nFrom: Visitor \r\nSubject: Reset\r\nReturn-Path: forged@cool.com\r\n\r\nHello", "utf8"); const result = prepareSubmission(raw, site); const output = Buffer.from(result.message).toString("utf8"); expect(result.sender).toBe("noreply@example.com"); - expect(output).toContain("From: noreply@example.com\r\n"); - expect(output).not.toContain("cool.com"); + expect(output).toContain("From: Visitor \r\nReply-To: visitor@cool.com\r\n"); + expect(output).not.toContain("forged@cool.com"); expect(output.endsWith("\r\n\r\nHello")).toBe(true); - const invalidRequested = prepareSubmission(Buffer.from("To: user@recipient.test\nFrom: wordpress@example.com (WordPress)\n\nHello"), site); - expect(invalidRequested.sender).toBe("noreply@example.com"); - expect(Buffer.from(invalidRequested.message).toString()).not.toContain("(WordPress)"); + const unparsable = submit(site.rule, "From: wordpress@example.com (WordPress)"); + expect(unparsable.sender).toBe("noreply@example.com"); + expect(unparsable.text).not.toContain("(WordPress)"); + expect(unparsable.text).not.toContain("Reply-To"); + expect(submit(site.rule, "From: wordpress@example.com").text).not.toContain("Reply-To"); + expect(submit(site.rule, "From: a@cool.com\nReply-To: desk@cool.com").text.match(/Reply-To/g)).toHaveLength(1); +}); + +test("{from.local} keeps the app's name on the template's domain", () => { + const rule = parseRule({ sender: "{from.local}@{site}" }); + const wordpress = submit(rule, "From: WordPress "); + expect(wordpress.sender).toBe("wordpress@example.com"); + expect(wordpress.text).toContain("From: WordPress \n"); + expect(wordpress.text).not.toContain("Reply-To"); + const visitor = submit(rule, "From: Jane "); + expect(visitor.text).toContain("From: Jane \nReply-To: jane@cool.com\n"); + expect(submit(rule, "Subject: none").sender).toBe("noreply@example.com"); + expect(submit(parseRule({ sender: "{from.local}@mail.{site}" }), `From: ${"a".repeat(240)}@example.com`).sender).toBe("noreply@mail.example.com"); }); -test("allow-listed mode preserves own and approved domains but refuses another site", () => { - const allowed = { ...site, rule: parseRule({ mode: "allow", sender: "noreply@{domain}", domains: ["news.example.com"], addresses: ["billing@partner.test"] }) }; - for (const sender of ["wordpress@example.com", "edition@news.example.com", "billing@partner.test"]) { - const result = prepareSubmission(Buffer.from(`To: test@recipient.test\nFrom: ${sender}\n\nHi`), allowed); - expect(result.sender).toBe(sender); +test("{from.domain} keeps only the site's own and granted domains", () => { + const rule = parseRule({ sender: "{from.local}@{from.domain}", domains: ["news.example.com"] }); + for (const sender of ["wordpress@example.com", "edition@news.example.com"]) { + expect(submit(rule, `From: ${sender}`).sender).toBe(sender); } - expect(() => prepareSubmission(Buffer.from("To: test@recipient.test\nFrom: noreply@cool.com\n\nHi"), allowed)).toThrow("not allowed"); - expect(() => prepareSubmission(Buffer.from("From: a@example.com\nFrom: b@example.com\nTo: test@recipient.test\n\nHi"), allowed)).toThrow("multiple From"); + const foreign = submit(rule, "From: noreply@othersite.test"); + expect(foreign.sender).toBe("noreply@example.com"); + expect(foreign.text).toContain("Reply-To: noreply@othersite.test"); + expect(() => submit(rule, "From: a@example.com\nFrom: b@example.com")).toThrow("multiple From"); }); -test("folded From is checked and ignored Sender fields cannot change it", () => { - const allowed = { ...site, rule: { ...site.rule, mode: "allow" as const } }; - const result = prepareSubmission(Buffer.from("To: test@recipient.test\nFrom: Example\n \nSender: forged@cool.com\n x: bogus\n\nHi"), allowed); +test("folded From is read and ignored Sender fields cannot change it", () => { + const rule = parseRule({ sender: "{from.local}@{from.domain}" }); + const result = submit(rule, "From: Example\n \nSender: forged@cool.com\n x: bogus"); expect(result.sender).toBe("wordpress@example.com"); - expect(Buffer.from(result.message).toString()).not.toContain("forged@cool.com"); + expect(result.text).not.toContain("forged@cool.com"); +}); + +test("From templates accept only the three tokens in their own halves", () => { + for (const sender of ["noreply@{domain}", "{from.domain}@example.com", "noreply@mail.{from.domain}", "{from.local}", "a@b@{site}"]) { + expect(() => parseRule({ sender })).toThrow(); + } + expect(parseRule({ sender: "NoReply@{site}", domains: ["other.test"] })).toEqual(site.rule); +}); + +test("Postfix envelope grants follow what the template can produce", () => { + const grants = (sender: string, domains: string[] = []) => senderGrants({ domain: "example.com", rule: parseRule({ sender, domains }) }); + expect(grants("noreply@{site}")).toEqual(["noreply@example.com"]); + expect(grants("{from.local}@{site}")).toEqual(["@example.com"]); + expect(grants("{from.local}@{from.domain}", ["news.example.com"])).toEqual(["@example.com", "@news.example.com"]); + expect(grants("noreply@{from.domain}", ["news.example.com"])).toEqual(["noreply@example.com", "noreply@news.example.com"]); }); test("stdin accepts exactly 25 MiB and cancels at the first byte over the limit", async () => { @@ -122,7 +158,7 @@ test("configured relay applies to Postfix and site pool, then deactivates cleanl run, }; const body = { relay: { host: "mail.example.com", port: 587, username: "relay@example.com", password: "secret" } }; - await expect(executeSmtpAction(["save-setup"], { ...options, input: JSON.stringify({ ...body, rule: { mode: "invalid" } }) })).rejects.toThrow("sender mode"); + await expect(executeSmtpAction(["save-setup"], { ...options, input: JSON.stringify({ ...body, rule: { sender: "noreply@{domain}" } }) })).rejects.toThrow("{from.local}, {from.domain} and {site}"); expect(existsSync(join(dir, "smtp.lock"))).toBe(false); expect((await executeSmtpAction(["list"], options) as SmtpState).configured).toBe(false); const submissionPath = join(dir, "submission.json"); @@ -144,7 +180,7 @@ test("configured relay applies to Postfix and site pool, then deactivates cleanl settings.delete("transport_maps"); await executeSmtpAction(["save-setup"], { ...options, input: JSON.stringify({ ...body, rule: site.rule }) }); expect(readFileSync(pool, "utf8")).toContain("smtp-submit -t -i"); - expect(readFileSync(join(dir, "submission.json"), "utf8")).toContain("noreply@{domain}"); + expect(readFileSync(join(dir, "submission.json"), "utf8")).toContain("noreply@{site}"); expect(settings.get("local_login_sender_maps")).toContain("clp-addons-local-senders"); expect(readFileSync(join(postfixDir, "clp-addons-local-senders"), "utf8")).toContain("clp *\n"); expect(settings.get("smtp_tls_security_level")).toBe("secure"); @@ -207,12 +243,12 @@ test("configured relay applies to Postfix and site pool, then deactivates cleanl const html = dashboardView(publicState as SmtpState); expect(JSON.stringify(publicState)).not.toContain("secret"); expect(html).not.toContain("secret"); - expect(html).toContain('data-label="Forced From"'); + expect(html).toContain('data-label="From"'); writeFileSync(pool, readFileSync(pool, "utf8") + "php_admin_value[sendmail_path] = /other/sendmail\n"); await expect(executeSmtpAction(["save-default"], { ...options, input: JSON.stringify({ - rule: { mode: "allow", sender: "noreply@{domain}", domains: [], addresses: [] }, + rule: { sender: "{from.local}@{site}", domains: [] }, }) })).rejects.toThrow("already configures sendmail_path"); - expect((await executeSmtpAction(["list"], options) as SmtpState).defaultRule.mode).toBe("force"); + expect((await executeSmtpAction(["list"], options) as SmtpState).defaultRule.sender).toBe("noreply@{site}"); writeFileSync(pool, readFileSync(pool, "utf8").replace("php_admin_value[sendmail_path] = /other/sendmail\n", "")); await executeSmtpAction(["deactivate"], options); expect(readFileSync(pool, "utf8")).not.toContain("smtp-submit"); @@ -223,19 +259,14 @@ test("configured relay applies to Postfix and site pool, then deactivates cleanl expect(existsSync(join(postfixDir, "clp-addons-tls-policy"))).toBe(false); }); -test("force mode drops dormant allow-list grants", () => { - expect(parseRule({ mode: "force", sender: "noreply@{domain}", domains: ["other.test"], addresses: ["a@other.test"] })).toEqual(site.rule); -}); - -test("site table distinguishes an allow-mode fallback from its full grants", () => { +test("site table shows each site's From and its granted domains", () => { const policy = emptySmtpPolicy(); - const rule = parseRule({ mode: "allow", sender: "noreply@{domain}", domains: ["news.example.com"], addresses: ["billing@partner.test"] }); + const rule = parseRule({ sender: "{from.local}@{from.domain}", domains: ["news.example.com"] }); const html = dashboardView({ configured: false, relay: null, relayOverrides: {}, defaultRule: policy.defaultRule, - sites: [{ domain: "example.com", user: "example", phpVersion: "8.2", rule, overridden: true, senderPreview: "noreply@example.com" }], + sites: [{ domain: "example.com", user: "example", phpVersion: "8.2", rule, overridden: true, senderPreview: rule.sender }], }); - expect(html).toContain('data-label="Fallback From"'); - expect(html).toContain("@news.example.com"); - expect(html).toContain("billing@partner.test"); + expect(html).toContain("{from.local}@{from.domain}"); + expect(html).toContain("news.example.com"); expect(html).toContain('data-label="Actions"'); }); diff --git a/tools/preview-ui.ts b/tools/preview-ui.ts index 6ec5d3f..1a23aa5 100644 --- a/tools/preview-ui.ts +++ b/tools/preview-ui.ts @@ -55,12 +55,14 @@ function smtpPreviewState(url: URL): SmtpState { configured, relay: configured ? { host: "mail.example.com", port: 587, username: "cloudpanel-relay@example.com", hasPassword: true } : null, relayOverrides: configured ? { "shop.example.com": { host: "smtp.provider.test", port: 587, username: "shop@example.com", hasPassword: true } } : {}, - defaultRule: { mode: "force", sender: "noreply@{domain}", domains: [], addresses: [] }, + defaultRule: { sender: "noreply@{site}", domains: [] }, sites: url.searchParams.has("empty") ? [] : [ { domain: "www.example.com", user: "example", phpVersion: "8.3", overridden: false, - rule: { mode: "force", sender: "noreply@{domain}", domains: [], addresses: [] }, senderPreview: "noreply@www.example.com" }, + rule: { sender: "noreply@{site}", domains: [] }, senderPreview: "noreply@www.example.com" }, { domain: "shop.example.com", user: "shop", phpVersion: "8.3", overridden: true, - rule: { mode: "allow", sender: "noreply@{domain}", domains: ["news.shop.example.com"], addresses: [] }, senderPreview: "noreply@shop.example.com" }, + rule: { sender: "{from.local}@{from.domain}", domains: ["news.shop.example.com"] }, senderPreview: "{from.local}@{from.domain}" }, + { domain: "blog.example.com", user: "blog", phpVersion: "8.2", overridden: true, + rule: { sender: "{from.local}@{site}", domains: [] }, senderPreview: "{from.local}@blog.example.com" }, ], }; } diff --git a/tools/ui-shot.ts b/tools/ui-shot.ts index 1deadfc..12254a4 100644 --- a/tools/ui-shot.ts +++ b/tools/ui-shot.ts @@ -66,7 +66,7 @@ function findChrome(): string | null { async function chromeBinary(): Promise { const existing = findChrome(); if (existing) return existing; - await run(["bunx", "playwright", "install", "chromium-headless-shell"]); + await run(["bunx", "--bun", "playwright", "install", "chromium-headless-shell"]); const installed = findChrome(); if (!installed) throw new Error("no headless chromium found after installing it"); return installed;