Harden transactional email envelopes
This commit is contained in:
parent
6ed3ea4ab7
commit
6b4d238e51
|
|
@ -133,6 +133,7 @@ RUN apt-get update \
|
|||
&& apt-get install -y --no-install-recommends \
|
||||
build-essential=12.12 \
|
||||
ca-certificates=20250419 \
|
||||
cmake=3.31.6-2 \
|
||||
git=1:2.47.3-0+deb13u1 \
|
||||
&& rm -rf /var/lib/apt/lists/*
|
||||
|
||||
|
|
|
|||
|
|
@ -727,9 +727,16 @@ if config_env() == :prod do
|
|||
|
||||
config :who_need_help, :handover_secret, handover_secret
|
||||
|
||||
email_from_name = System.get_env("EMAIL_FROM_NAME", "Who Need Help")
|
||||
email_from_address = System.get_env("EMAIL_FROM_ADDRESS", "contact@example.com")
|
||||
|
||||
unless WhoNeedHelp.EmailAddress.valid?(email_from_address) do
|
||||
raise "EMAIL_FROM_ADDRESS must be a single SMTP-safe mailbox address."
|
||||
end
|
||||
|
||||
config :who_need_help, :mailer_from,
|
||||
name: System.get_env("EMAIL_FROM_NAME", "Who Need Help"),
|
||||
address: System.get_env("EMAIL_FROM_ADDRESS", "contact@example.com")
|
||||
name: email_from_name,
|
||||
address: email_from_address
|
||||
|
||||
support_inbox_address =
|
||||
case System.get_env("SUPPORT_INBOX_ADDRESS") do
|
||||
|
|
@ -737,9 +744,8 @@ if config_env() == :prod do
|
|||
value -> value
|
||||
end
|
||||
|
||||
if support_inbox_address &&
|
||||
not Regex.match?(~r/^[^@,;\s]+@[^@,;\s]+$/, support_inbox_address) do
|
||||
raise "SUPPORT_INBOX_ADDRESS must be a single email address without spaces."
|
||||
if support_inbox_address && not WhoNeedHelp.EmailAddress.valid?(support_inbox_address) do
|
||||
raise "SUPPORT_INBOX_ADDRESS must be a single SMTP-safe mailbox address."
|
||||
end
|
||||
|
||||
config :who_need_help, :support_inbox_address, support_inbox_address
|
||||
|
|
|
|||
|
|
@ -5,6 +5,16 @@ results from product limits and unknown production properties.
|
|||
|
||||
## Current launch-boundary and transactional-email proof on 2026-08-12
|
||||
|
||||
- A follow-up SMTP-envelope regression run completed successfully in isolated
|
||||
user-systemd unit
|
||||
`codex-heavy-wnh-email-full-final-20260812-213848-711438.service`.
|
||||
ExUnit reported 468 passing tests with seed `954756`. The shared mailbox
|
||||
validator now covers model input, production/test environment preflight, and
|
||||
every outbound envelope; malformed historical recipients cancel their exact
|
||||
Oban mail job instead of reaching the SMTP adapter or retrying indefinitely.
|
||||
A separately built production image loaded the same runtime validation
|
||||
successfully in unit
|
||||
`codex-heavy-wnh-release-runtime-email-20260812-213723-667643.service`.
|
||||
- The final email-boundary quality rerun completed successfully in isolated
|
||||
user-systemd unit
|
||||
`codex-heavy-wnh-email-quality-final-20260812-184719-3589051.service`.
|
||||
|
|
@ -77,6 +87,12 @@ results from product limits and unknown production properties.
|
|||
- `SUPPORT_OPERATOR_EMAIL_MODE` was absent in the production environment and
|
||||
the deployed default was `disabled`; verified support requests therefore did
|
||||
not generate an operator-inbox alert in that observed configuration.
|
||||
- Four legacy SMTP exceptions were reproduced from stored recipients whose
|
||||
local part began or ended with a dot or contained consecutive dots. The
|
||||
deployed revision accepted those forms before the SMTP adapter rejected
|
||||
them. This establishes the failure class, but the legacy aggregate lacks
|
||||
recipient and purpose labels, so it does not prove which historical rows
|
||||
produced each of the four process-level exceptions.
|
||||
- The workstation-scheduled encrypted off-site backup completed successfully
|
||||
at 2026-08-12 00:04:48 EEST, including its isolated restore drill. The new
|
||||
restore-verified heartbeat implementation was committed later that day, so
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ defmodule WhoNeedHelp.EmailDelivery do
|
|||
subjects, bodies, references, and other personal data are never attached.
|
||||
"""
|
||||
|
||||
alias WhoNeedHelp.EmailAddress
|
||||
alias WhoNeedHelp.Mailer
|
||||
|
||||
@kinds [
|
||||
|
|
@ -26,6 +27,7 @@ defmodule WhoNeedHelp.EmailDelivery do
|
|||
def deliver(%Swoosh.Email{} = email, kind) when kind in @kinds do
|
||||
started_at = System.monotonic_time()
|
||||
|
||||
if valid_envelope?(email) do
|
||||
try do
|
||||
result = Mailer.deliver(email)
|
||||
emit(started_at, kind, result_status(result))
|
||||
|
|
@ -39,6 +41,10 @@ defmodule WhoNeedHelp.EmailDelivery do
|
|||
emit(started_at, kind, :exception)
|
||||
:erlang.raise(class, reason, __STACKTRACE__)
|
||||
end
|
||||
else
|
||||
emit(started_at, kind, :error)
|
||||
{:error, :invalid_email_address}
|
||||
end
|
||||
end
|
||||
|
||||
def kinds, do: @kinds
|
||||
|
|
@ -46,6 +52,16 @@ defmodule WhoNeedHelp.EmailDelivery do
|
|||
defp result_status({:ok, _metadata}), do: :ok
|
||||
defp result_status({:error, _reason}), do: :error
|
||||
|
||||
defp valid_envelope?(email) do
|
||||
[email.from, email.reply_to | email.to ++ email.cc ++ email.bcc]
|
||||
|> List.flatten()
|
||||
|> Enum.reject(&is_nil/1)
|
||||
|> Enum.all?(fn
|
||||
{_name, address} -> EmailAddress.valid?(address)
|
||||
_invalid_recipient -> false
|
||||
end)
|
||||
end
|
||||
|
||||
defp emit(started_at, kind, status) do
|
||||
:telemetry.execute(
|
||||
[:who_need_help, :email, :delivery],
|
||||
|
|
|
|||
|
|
@ -54,6 +54,7 @@ defmodule WhoNeedHelp.Mail.ContentRemovalEmailWorker do
|
|||
defp deliver(%Notice{} = notice, "operator") do
|
||||
case Notifier.deliver_operator_alert(notice) do
|
||||
{:ok, _metadata} -> :ok
|
||||
{:error, :invalid_email_address} -> {:cancel, :invalid_email_address}
|
||||
{:error, reason} -> {:error, reason}
|
||||
end
|
||||
end
|
||||
|
|
@ -70,5 +71,8 @@ defmodule WhoNeedHelp.Mail.ContentRemovalEmailWorker do
|
|||
end
|
||||
end
|
||||
|
||||
defp mark_sent({:error, :invalid_email_address}, _notice, _field),
|
||||
do: {:cancel, :invalid_email_address}
|
||||
|
||||
defp mark_sent({:error, reason}, _notice, _field), do: {:error, reason}
|
||||
end
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ defmodule WhoNeedHelp.Mail.SupportConfirmationWorker do
|
|||
%SupportRequest{} = request ->
|
||||
case Notifier.deliver_confirmation(request, Support.confirmation_url(request)) do
|
||||
{:ok, _metadata} -> :ok
|
||||
{:error, :invalid_email_address} -> {:cancel, :invalid_email_address}
|
||||
{:error, reason} -> {:error, reason}
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -24,7 +24,10 @@ defmodule WhoNeedHelp.Mail.SupportOperatorAlertWorker do
|
|||
{:cancel, :contact_not_verified}
|
||||
|
||||
%SupportRequest{} = request ->
|
||||
Notifier.deliver_operator_alert(request)
|
||||
case Notifier.deliver_operator_alert(request) do
|
||||
{:error, :invalid_email_address} -> {:cancel, :invalid_email_address}
|
||||
result -> result
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -27,6 +27,7 @@ defmodule WhoNeedHelp.Mail.SupportUpdateWorker do
|
|||
%SupportRequest{} = request ->
|
||||
case Notifier.deliver_decision(request, Support.status_url(request)) do
|
||||
{:ok, _metadata} -> mark_sent(request)
|
||||
{:error, :invalid_email_address} -> {:cancel, :invalid_email_address}
|
||||
{:error, reason} -> {:error, reason}
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -92,6 +92,27 @@ valid_nonempty_csv() {
|
|||
done
|
||||
}
|
||||
|
||||
valid_mailbox_address() {
|
||||
local address=$1 local_part domain label
|
||||
local -a labels
|
||||
|
||||
[[ "$address" =~ ^[A-Za-z0-9.!#\$%\&\'*+/=\?\^_\`\{\|\}~-]+@[A-Za-z0-9.-]+$ ]] ||
|
||||
return 1
|
||||
|
||||
local_part=${address%@*}
|
||||
domain=${address#*@}
|
||||
[[ -n "$local_part" && "$local_part" != .* && "$local_part" != *. &&
|
||||
"$local_part" != *..* ]] || return 1
|
||||
|
||||
IFS='.' read -r -a labels <<<"$domain"
|
||||
[[ ${#labels[@]} -ge 2 ]] || return 1
|
||||
|
||||
for label in "${labels[@]}"; do
|
||||
[[ -n "$label" && "$label" =~ ^[A-Za-z0-9-]+$ &&
|
||||
"$label" != -* && "$label" != *- ]] || return 1
|
||||
done
|
||||
}
|
||||
|
||||
valid_public_rate_limit_policy() {
|
||||
local json=$1
|
||||
|
||||
|
|
@ -397,13 +418,13 @@ case "$email_delivery_provider" in
|
|||
exit 1
|
||||
;;
|
||||
esac
|
||||
[[ "$email_from_address" == *@* ]] || {
|
||||
echo "EMAIL_FROM_ADDRESS is not an email address." >&2
|
||||
valid_mailbox_address "$email_from_address" || {
|
||||
echo "EMAIL_FROM_ADDRESS must be a single SMTP-safe mailbox address." >&2
|
||||
exit 1
|
||||
}
|
||||
if [[ -n "$support_inbox_address" &&
|
||||
! "$support_inbox_address" =~ ^[^@,\;[:space:]]+@[^@,\;[:space:]]+$ ]]; then
|
||||
echo "SUPPORT_INBOX_ADDRESS is not an email address." >&2
|
||||
if [[ -n "$support_inbox_address" ]] &&
|
||||
! valid_mailbox_address "$support_inbox_address"; then
|
||||
echo "SUPPORT_INBOX_ADDRESS must be a single SMTP-safe mailbox address." >&2
|
||||
exit 1
|
||||
fi
|
||||
|
||||
|
|
|
|||
|
|
@ -63,6 +63,27 @@ valid_nonempty_csv() {
|
|||
done
|
||||
}
|
||||
|
||||
valid_mailbox_address() {
|
||||
local address=$1 local_part domain label
|
||||
local -a labels
|
||||
|
||||
[[ "$address" =~ ^[A-Za-z0-9.!#\$%\&\'*+/=\?\^_\`\{\|\}~-]+@[A-Za-z0-9.-]+$ ]] ||
|
||||
return 1
|
||||
|
||||
local_part=${address%@*}
|
||||
domain=${address#*@}
|
||||
[[ -n "$local_part" && "$local_part" != .* && "$local_part" != *. &&
|
||||
"$local_part" != *..* ]] || return 1
|
||||
|
||||
IFS='.' read -r -a labels <<<"$domain"
|
||||
[[ ${#labels[@]} -ge 2 ]] || return 1
|
||||
|
||||
for label in "${labels[@]}"; do
|
||||
[[ -n "$label" && "$label" =~ ^[A-Za-z0-9-]+$ &&
|
||||
"$label" != -* && "$label" != *- ]] || return 1
|
||||
done
|
||||
}
|
||||
|
||||
[[ "$(require_value DEPLOYMENT_ENV)" == test ]] || {
|
||||
echo "Test validation requires DEPLOYMENT_ENV=test." >&2
|
||||
exit 1
|
||||
|
|
@ -126,7 +147,11 @@ case "$email_delivery_provider" in
|
|||
exit 1
|
||||
;;
|
||||
esac
|
||||
require_value EMAIL_FROM_ADDRESS >/dev/null
|
||||
email_from_address=$(require_value EMAIL_FROM_ADDRESS)
|
||||
valid_mailbox_address "$email_from_address" || {
|
||||
echo "EMAIL_FROM_ADDRESS must be a single SMTP-safe mailbox address." >&2
|
||||
exit 1
|
||||
}
|
||||
[[ "$(require_value PHX_HOST)" == "$expected_domain" &&
|
||||
"$(require_value WNH_BASE_URL)" == "https://$expected_domain" ]] || {
|
||||
echo "The test public origin does not match EXPECTED_DOMAIN." >&2
|
||||
|
|
|
|||
|
|
@ -1,7 +1,11 @@
|
|||
defmodule WhoNeedHelp.EmailAddressTest do
|
||||
use ExUnit.Case, async: true
|
||||
|
||||
import Swoosh.Email
|
||||
|
||||
alias WhoNeedHelp.EmailAddress
|
||||
alias WhoNeedHelp.EmailDelivery
|
||||
alias Swoosh.Adapters.SMTP.Helpers
|
||||
|
||||
test "accepts ordinary mailbox addresses used by transactional email" do
|
||||
assert EmailAddress.valid?("person@example.com")
|
||||
|
|
@ -9,6 +13,8 @@ defmodule WhoNeedHelp.EmailAddressTest do
|
|||
end
|
||||
|
||||
test "rejects addresses that the SMTP adapter cannot safely parse" do
|
||||
refute EmailAddress.valid?(".person@example.com")
|
||||
refute EmailAddress.valid?("person.@example.com")
|
||||
refute EmailAddress.valid?("two..dots@example.com")
|
||||
refute EmailAddress.valid?("person@example")
|
||||
refute EmailAddress.valid?("person@-example.com")
|
||||
|
|
@ -16,4 +22,31 @@ defmodule WhoNeedHelp.EmailAddressTest do
|
|||
refute EmailAddress.valid?("person@exam ple.com")
|
||||
refute EmailAddress.valid?("first@example.com,second@example.com")
|
||||
end
|
||||
|
||||
test "every accepted address can be encoded by the configured SMTP implementation" do
|
||||
for address <- ["person@example.com", "person+alerts@example.co.uk"] do
|
||||
encoded =
|
||||
new()
|
||||
|> from({"Who Need Help", "contact@example.com"})
|
||||
|> to(address)
|
||||
|> reply_to("support@example.com")
|
||||
|> subject("Email address compatibility")
|
||||
|> text_body("Compatibility check")
|
||||
|> Helpers.body([])
|
||||
|
||||
assert is_binary(encoded)
|
||||
end
|
||||
end
|
||||
|
||||
test "delivery rejects an invalid envelope before invoking the mail adapter" do
|
||||
email =
|
||||
new()
|
||||
|> from({"Who Need Help", "contact@example.com"})
|
||||
|> to(".person@example.com")
|
||||
|> subject("Invalid envelope check")
|
||||
|> text_body("This message must not be delivered")
|
||||
|
||||
assert {:error, :invalid_email_address} =
|
||||
EmailDelivery.deliver(email, :auth_login)
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -98,6 +98,28 @@ defmodule WhoNeedHelp.SupportAndContentRemovalTest do
|
|||
refute_receive {:email, _duplicate_received_email}, 50
|
||||
end
|
||||
|
||||
test "legacy support jobs with an invalid stored mailbox are cancelled without delivery" do
|
||||
assert {:ok, request} =
|
||||
Support.create_request(nil, %{
|
||||
"kind" => "technical_issue",
|
||||
"contact_email" => "legacy@example.com",
|
||||
"subject" => "Legacy queued support message",
|
||||
"details" =>
|
||||
"This request simulates malformed historical data already stored before validation."
|
||||
})
|
||||
|
||||
{1, nil} =
|
||||
Repo.update_all(
|
||||
from(stored_request in SupportRequest, where: stored_request.id == ^request.id),
|
||||
set: [contact_email: ".legacy@example.com"]
|
||||
)
|
||||
|
||||
assert {:cancel, :invalid_email_address} =
|
||||
perform_job(SupportConfirmationWorker, %{"request_id" => request.id})
|
||||
|
||||
refute_receive {:email, _email}, 50
|
||||
end
|
||||
|
||||
test "operator alert is sent only after public contact verification" do
|
||||
previous = Application.get_env(:who_need_help, :support_inbox_address)
|
||||
previous_mode = Application.get_env(:who_need_help, :support_operator_email_mode)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user