This is an automated email from the ASF dual-hosted git repository. sbp pushed a commit to branch sbp in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
commit 8f86fe3b13b855d0fe34e176823a97136a563cb9 Author: Sean B. Palmer <[email protected]> AuthorDate: Tue Mar 24 16:27:44 2026 +0000 Allow the use of CC and BCC for voting and announcing --- atr/api/__init__.py | 4 +- atr/db/interaction.py | 4 +- atr/get/announce.py | 67 ++--- atr/get/voting.py | 22 +- atr/mail.py | 77 +++-- atr/models/results.py | 2 +- atr/post/announce.py | 18 +- atr/post/vote.py | 6 +- atr/post/voting.py | 20 +- atr/render.py | 55 ++++ atr/shared/announce.py | 15 +- atr/shared/voting.py | 19 +- atr/static/css/atr.css | 5 + atr/storage/writers/announce.py | 20 +- atr/storage/writers/mail.py | 4 +- atr/storage/writers/tokens.py | 4 +- atr/storage/writers/vote.py | 44 ++- atr/tasks/message.py | 24 +- atr/tasks/vote.py | 30 +- atr/util.py | 33 ++- tests/unit/test_mail.py | 601 +++++++++++++++++++++++----------------- tests/unit/test_message.py | 6 +- 22 files changed, 657 insertions(+), 423 deletions(-) diff --git a/atr/api/__init__.py b/atr/api/__init__.py index 0c348c6e..206830d5 100644 --- a/atr/api/__init__.py +++ b/atr/api/__init__.py @@ -889,7 +889,7 @@ async def publisher_release_announce( project_key=project.safe_key, version_key=data.version, preview_revision_number=data.revision, - recipient=data.email_to, + email_to=data.email_to, body=data.body, download_path_suffix=data.path_suffix, asf_uid=asf_uid, @@ -995,7 +995,7 @@ async def release_announce( project_key=data.project, version_key=data.version, preview_revision_number=data.revision, - recipient=data.email_to, + email_to=data.email_to, body=data.body, download_path_suffix=data.path_suffix, asf_uid=asf_uid, diff --git a/atr/db/interaction.py b/atr/db/interaction.py index f18a1a93..006e1d8d 100644 --- a/atr/db/interaction.py +++ b/atr/db/interaction.py @@ -399,7 +399,9 @@ def task_recipient_get(latest_vote_task: sql.Task) -> str | None: result = latest_vote_task.result if not isinstance(result, results.VoteInitiate): return None - return result.email_to + if not result.email_to: + return None + return result.email_to[0] async def tasks_ongoing( diff --git a/atr/get/announce.py b/atr/get/announce.py index 5a8055a0..32e765c9 100644 --- a/atr/get/announce.py +++ b/atr/get/announce.py @@ -90,11 +90,10 @@ async def selected( description_download_prefix += f"/{committee.key}" permitted_recipients = util.permitted_announce_recipients(session.uid) - mailing_list_choices = sorted([(recipient, recipient) for recipient in permitted_recipients]) content = await _render_page( release=release, - mailing_list_choices=mailing_list_choices, + permitted_recipients=permitted_recipients, default_subject=default_subject, subject_template_hash=subject_template_hash, default_body=default_body, @@ -157,48 +156,9 @@ def _render_download_path_field(default_value: str, description: str) -> htm.Ele ] -def _render_mailing_list_with_warning(choices: list[tuple[str, str]], default_value: str) -> htm.Element: - """Render the mailing list radio buttons with a warning card.""" - container = htm.Block(htm.div) - - # Radio buttons - radio_container = htm.div(".d-flex.flex-wrap.gap-2.mb-3") - radio_buttons = [] - for value, label in choices: - radio_id = f"mailing_list_{value}" - radio_attrs = { - "type": "radio", - "name": "mailing_list", - "value": value, - } - if value == default_value: - radio_attrs["checked"] = "" - - radio_buttons.append( - htm.div(".form-check")[ - htpy.input(f"#{radio_id}.form-check-input", **radio_attrs), - htpy.label(".form-check-label", for_=radio_id)[label], - ] - ) - container.append(radio_container[radio_buttons]) - - # Warning card - warning_card = htm.div(".card.bg-warning-subtle.mb-3")[ - htm.span(".card-body.p-3")[ - htpy.i(".bi.bi-exclamation-triangle.me-1"), - htm.strong["TODO: "], - "The limited options above are provided for testing purposes. In the finished version of ATR, " - "you will be able to send to your own specified mailing lists.", - ] - ] - container.append(warning_card) - - return container.collect() - - async def _render_page( release: sql.Release, - mailing_list_choices: list[tuple[str, str]], + permitted_recipients: list[str], default_subject: str, subject_template_hash: str, default_body: str, @@ -246,11 +206,27 @@ async def _render_page( }" if not announce_msg: - page.p[f"This form will send an announcement to the ASF {util.USER_TESTS_ADDRESS} mailing list."] + page.p["This form will send an announcement to the selected recipients."] custom_subject_widget = _render_subject_field(default_subject, release.project.key) custom_body_widget = _render_body_field(default_body, release.project.key) - custom_mailing_list_widget = _render_mailing_list_with_warning(mailing_list_choices, util.USER_TESTS_ADDRESS) + default_to = permitted_recipients[0] if permitted_recipients else None + to_radios = htm.div[ + render.html_recipients_to_radios( + permitted_recipients, + default_to=default_to, + documentation=( + "Note: The options to send to the user-tests " + "mailing list and yourself are provided for " + "testing purposes only, and will not be " + "available in the finished version of ATR." + ), + ), + htpy.details(".mt-2")[ + htpy.summary["Select CC and BCC recipients"], + render.html_recipients_cc_bcc_table(permitted_recipients), + ], + ] # Custom widget for download_path_suffix with custom documentation download_path_widget = _render_download_path_field(default_download_path_suffix, download_path_description) @@ -268,11 +244,12 @@ async def _render_page( submit_label="Send announcement email", defaults=defaults_dict, custom={ + "email_to": to_radios, "subject": custom_subject_widget, "body": custom_body_widget, - "mailing_list": custom_mailing_list_widget, "download_path_suffix": download_path_widget, }, + skip=["email_cc", "email_bcc"], form_classes=".atr-canary.py-4.px-5.mb-4.border.rounded", border=True, wider_widgets=True, diff --git a/atr/get/voting.py b/atr/get/voting.py index f742252c..22c4ca3a 100644 --- a/atr/get/voting.py +++ b/atr/get/voting.py @@ -192,21 +192,41 @@ async def _render_page( custom_subject_widget = _render_subject_field(default_subject, release.project.key) custom_body_widget = _render_body_field(default_body, release.project.key) + default_to = permitted_recipients[0] if permitted_recipients else None + to_radios = htm.div[ + render.html_recipients_to_radios( + permitted_recipients, + default_to=default_to, + documentation=( + "Note: The options to send to the user-tests " + "mailing list and yourself are provided for " + "testing purposes only, and will not be " + "available in the finished version of ATR. " + "If the option you pick is not a mailing list, " + "you will not be able to use vote tabulation." + ), + ), + htpy.details(".mt-2")[ + htpy.summary["Select CC and BCC recipients"], + render.html_recipients_cc_bcc_table(permitted_recipients), + ], + ] vote_form = form.render( model_cls=shared.voting.StartVotingForm, submit_label="Send vote email", cancel_url=cancel_url, defaults={ - "mailing_list": permitted_recipients, "vote_duration": min_hours, "subject_template_hash": subject_template_hash, "body": default_body, }, custom={ + "email_to": to_radios, "subject": custom_subject_widget, "body": custom_body_widget, }, + skip=["email_cc", "email_bcc"], ) page.append(vote_form) diff --git a/atr/mail.py b/atr/mail.py index 7d59d33c..4cd9e00e 100644 --- a/atr/mail.py +++ b/atr/mail.py @@ -30,13 +30,13 @@ import aiosmtplib # import dkim import atr.log as log +import atr.util as util # TODO: We should choose a pattern for globals # We could e.g. use uppercase instead of global_ # It's not always worth identifying globals as globals # But in many cases we should do so -global_domain: str = "apache.org" - +_APACHE_DOMAIN: Final[str] = "apache.org" _MAIL_RELAY: Final[str] = "mail-relay.apache.org" _SMTP_PORT: Final[int] = 587 _SMTP_TIMEOUT: Final[int] = 30 @@ -51,12 +51,12 @@ class MailFooterCategory(enum.StrEnum): @dataclasses.dataclass class Message: email_sender: str - email_recipient: str + email_to: str subject: str body: str - email_cc: str = "" - email_bcc: str = "" in_reply_to: str | None = None + email_cc: list[str] = dataclasses.field(default_factory=list) + email_bcc: list[str] = dataclasses.field(default_factory=list) async def send(msg_data: Message, category: MailFooterCategory) -> tuple[str, list[str]]: @@ -64,42 +64,36 @@ async def send(msg_data: Message, category: MailFooterCategory) -> tuple[str, li log.info(f"Sending email for event: {msg_data}") _reject_null_bytes( msg_data.email_sender, - msg_data.email_recipient, - msg_data.email_cc, - msg_data.email_bcc, + msg_data.email_to, + *msg_data.email_cc, + *msg_data.email_bcc, msg_data.subject, msg_data.body, msg_data.in_reply_to, ) from_addr = msg_data.email_sender - if not from_addr.endswith(f"@{global_domain}"): - raise ValueError(f"from_addr must end with @{global_domain}, got {from_addr}") - to_addrs = _validate_recipients(msg_data.email_recipient) - if len(to_addrs) < 1: - raise ValueError("email_recipient must contain at least one email address") - cc_addrs = _validate_recipients(msg_data.email_cc) - bcc_addrs = _validate_recipients(msg_data.email_bcc) + if not from_addr.endswith(f"@{_APACHE_DOMAIN}"): + raise ValueError(f"from_addr must end with @{_APACHE_DOMAIN}, got {from_addr}") + util.validate_email_recipients(msg_data) + # BCC recipients go in the SMTP envelope only, never in message headers + all_envelope_recipients = [msg_data.email_to, *msg_data.email_cc, *msg_data.email_bcc] + for addr in all_envelope_recipients: + _validate_recipient(addr) # UUID4 is entirely random, with no timestamp nor namespace # It does have 6 version and variant bits, so only 122 bits are random - mid = f"{uuid.uuid4()}@{global_domain}" + mid = f"{uuid.uuid4()}@{_APACHE_DOMAIN}" # Use EmailMessage with Address objects for CRLF injection protection msg = message.EmailMessage(policy=policy.SMTPUTF8) try: from_local, from_domain = _split_address(from_addr) - msg["From"] = headerregistry.Address(username=from_local, domain=from_domain) - msg["To"] = tuple( - headerregistry.Address(username=local, domain=domain) - for local, domain in [_split_address(addr) for addr in to_addrs] - ) - if cc_addrs: - msg["Cc"] = tuple( - headerregistry.Address(username=local, domain=domain) - for local, domain in [_split_address(addr) for addr in cc_addrs] - ) + to_local, to_domain = _split_address(msg_data.email_to) + msg["To"] = headerregistry.Address(username=to_local, domain=to_domain) + if msg_data.email_cc: + msg["Cc"] = _address_header(msg_data.email_cc) msg["Subject"] = msg_data.subject msg["Date"] = utils.formatdate(usegmt=True) msg["Message-ID"] = f"<{mid}>" @@ -120,8 +114,6 @@ async def send(msg_data: Message, category: MailFooterCategory) -> tuple[str, li msg_text = msg.as_string() log.info(f"sending message: {msg_text}") - # BCC recipients go in the SMTP envelope only, never in message headers - all_envelope_recipients = to_addrs + cc_addrs + bcc_addrs errors = await _send_many(from_addr, all_envelope_recipients, msg_text) if not errors: @@ -135,6 +127,11 @@ async def send(msg_data: Message, category: MailFooterCategory) -> tuple[str, li return mid, errors +def _address_header(addresses: list[str]) -> str: + parts = [headerregistry.Address(username=local, domain=domain) for local, domain in map(_split_address, addresses)] + return ", ".join(str(a) for a in parts) + + def _body_with_footer(body: str, category: MailFooterCategory, from_addr: str) -> str: # TODO: AM need to get the domain but we don't have that apart from in a request context currently. match category: @@ -203,19 +200,11 @@ def _split_address(addr: str) -> tuple[str, str]: return parts[0], parts[1] -def _validate_recipients(to_addr: str) -> list[str]: - # Ensure recipient is @apache.org or @tooling.apache.org - if not to_addr.strip(): - return [] - emails: list[str] = [] - for mail in to_addr.split(","): - mail = mail.strip() - _, domain = _split_address(mail) - domain_is_apache = domain == "apache.org" - domain_is_subdomain = domain.endswith(".apache.org") - if not (domain_is_apache or domain_is_subdomain): - error_msg = f"Email recipient must be @apache.org or @*.apache.org, got {mail}" - log.error(error_msg) - raise ValueError(error_msg) - emails.append(mail) - return emails +def _validate_recipient(addr: str) -> None: + _, domain = _split_address(addr) + domain_is_apache = domain == "apache.org" + domain_is_subdomain = domain.endswith(".apache.org") + if not (domain_is_apache or domain_is_subdomain): + error_msg = f"Email recipient must be @apache.org or @*.apache.org, got {addr}" + log.error(error_msg) + raise ValueError(error_msg) diff --git a/atr/models/results.py b/atr/models/results.py index b3f14a7f..acdf0078 100644 --- a/atr/models/results.py +++ b/atr/models/results.py @@ -218,7 +218,7 @@ class VoteInitiate(schema.Strict): kind: Literal["vote_initiate"] = schema.Field(alias="kind") message: str = schema.description("The message from the vote initiation") - email_to: str = schema.description("The email address the vote was sent to") + email_to: str = schema.description("The email To address the vote was sent to") vote_end: str = schema.description("The date and time the vote ends") subject: str = schema.description("The subject of the vote email") mid: str | None = schema.description("The message ID of the vote email") diff --git a/atr/post/announce.py b/atr/post/announce.py index 4c625d91..5bafcc59 100644 --- a/atr/post/announce.py +++ b/atr/post/announce.py @@ -44,12 +44,14 @@ async def selected( """ permitted_recipients = util.permitted_announce_recipients(session.uid) - # Validate that the recipient is permitted - if announce_form.mailing_list not in permitted_recipients: - return await session.form_error( - "mailing_list", - f"You are not permitted to send announcements to {announce_form.mailing_list}", - ) + # Validate that the recipients are permitted + all_addrs = [announce_form.email_to, *announce_form.email_cc, *announce_form.email_bcc] + for addr in all_addrs: + if addr not in permitted_recipients: + return await session.form_error( + "email_to", + f"You are not permitted to send announcements to {addr}", + ) # Get the release to find the revision number release = await session.release( @@ -106,12 +108,14 @@ async def selected( project_key=project_key, version_key=version_key, preview_revision_number=preview_revision_number, - recipient=announce_form.mailing_list, + email_to=announce_form.email_to, body=announce_form.body, download_path_suffix=announce_form.download_path_suffix, asf_uid=session.uid, fullname=session.fullname, subject_template_hash=announce_form.subject_template_hash, + email_cc=announce_form.email_cc, + email_bcc=announce_form.email_bcc, ) except storage.AccessError as e: return await session.redirect( diff --git a/atr/post/vote.py b/atr/post/vote.py index 1812befd..06dff30d 100644 --- a/atr/post/vote.py +++ b/atr/post/vote.py @@ -52,14 +52,12 @@ async def selected_post( is_binding, _binding_committee = await shared.vote.is_binding(release.committee, is_pmc_member) async with storage.write_as_committee_participant(release.committee.key) as wacm: - email_recipient, error_message = await wacm.vote.send_user_vote( - release, vote, comment, session.fullname, is_binding - ) + email_to, error_message = await wacm.vote.send_user_vote(release, vote, comment, session.fullname, is_binding) if error_message: await quart.flash(error_message, "error") return await session.redirect(get.vote.selected, project_key=str(project_key), version_key=str(version_key)) - success_message = f"Sending your vote to {email_recipient}." + success_message = f"Sending your vote to {email_to}." await quart.flash(success_message, "success") return await session.redirect(get.vote.selected, project_key=str(project_key), version_key=str(version_key)) diff --git a/atr/post/voting.py b/atr/post/voting.py index 17f4e102..d7bbd818 100644 --- a/atr/post/voting.py +++ b/atr/post/voting.py @@ -95,11 +95,13 @@ async def selected_revision( pass permitted_recipients = util.permitted_voting_recipients(session.uid, committee.key) - if start_voting_form.mailing_list not in permitted_recipients: - return await session.form_error( - "mailing_list", - f"Invalid mailing list selection: {start_voting_form.mailing_list}", - ) + all_addrs = [start_voting_form.email_to, *start_voting_form.email_cc, *start_voting_form.email_bcc] + for addr in all_addrs: + if addr not in permitted_recipients: + return await session.form_error( + "email_to", + f"Invalid recipient selection: {addr}", + ) subject_template = await construct.start_vote_subject_default(project_key) current_hash = construct.template_hash(subject_template) @@ -122,7 +124,7 @@ async def selected_revision( async with storage.write_as_committee_participant(committee.key) as wacp: _task = await wacp.vote.start( - start_voting_form.mailing_list, + start_voting_form.email_to, project_key, version_key, revision, @@ -134,12 +136,14 @@ async def selected_revision( release=release, promote=True, permitted_recipients=permitted_recipients, + email_cc=start_voting_form.email_cc, + email_bcc=start_voting_form.email_bcc, ) - log.info(f"Vote email will be sent to: {start_voting_form.mailing_list}") + log.info(f"Vote email will be sent to: {all_addrs}") return await session.redirect( get.vote.selected, - success=f"The vote announcement email will soon be sent to {start_voting_form.mailing_list}.", + success=f"The vote announcement email will soon be sent to {start_voting_form.email_to}.", project_key=str(project_key), version_key=str(version_key), ) diff --git a/atr/render.py b/atr/render.py index 8c8a1eda..4b96dd67 100644 --- a/atr/render.py +++ b/atr/render.py @@ -17,6 +17,8 @@ from typing import Literal +import htpy + import atr.get as get import atr.htm as htm import atr.util as util @@ -69,3 +71,56 @@ def html_nav_phase(block: htm.Block, project: str, version: str, staging: bool) back_anchor=f"{label.title()} {project} {version}", phase=label, ) + + +def html_recipients_cc_bcc_table(recipients: list[str]) -> htm.Element: + header = htpy.thead[ + htpy.tr[ + htpy.th(".text-center.atr-checkbox-col")["CC"], + htpy.th(".text-center.atr-checkbox-col")["BCC"], + htpy.th["Recipient"], + ] + ] + rows = [] + for recipient in recipients: + rows.append( + htpy.tr[ + htpy.td(".text-center.atr-checkbox-col")[ + htpy.input(type="checkbox", name="email_cc", value=recipient, class_="form-check-input") + ], + htpy.td(".text-center.atr-checkbox-col")[ + htpy.input(type="checkbox", name="email_bcc", value=recipient, class_="form-check-input") + ], + htpy.td[recipient], + ] + ) + return htpy.table(".table.table-bordered.mb-0")[header, htpy.tbody[rows]] + + +def html_recipients_to_radios( + recipients: list[str], + default_to: str | None = None, + documentation: str | None = None, +) -> htm.Element: + radios = [] + for recipient in recipients: + radio_id = f"email_to_{recipient.replace('@', '_').replace('.', '_')}" + radio_attrs: dict[str, str] = { + "type": "radio", + "name": "email_to", + "id": radio_id, + "value": recipient, + "class_": "form-check-input", + } + if recipient == default_to: + radio_attrs["checked"] = "" + radios.append( + htpy.div(".form-check")[ + htpy.input(**radio_attrs), + htpy.label(".form-check-label", for_=radio_id)[recipient], + ] + ) + container = htm.div[radios] + if documentation is None: + return container + return htm.div[container, htm.div(".text-muted.mt-1.form-text")[documentation]] diff --git a/atr/shared/announce.py b/atr/shared/announce.py index e156cdbe..0490e17f 100644 --- a/atr/shared/announce.py +++ b/atr/shared/announce.py @@ -17,18 +17,20 @@ from typing import Literal +import pydantic + import atr.form as form import atr.models.safe as safe +import atr.util as util class AnnounceForm(form.Form): """Form for announcing a release preview.""" revision_number: safe.RevisionNumber = form.label("Revision number", widget=form.Widget.HIDDEN) - mailing_list: str = form.label( - "Send vote email to", - widget=form.Widget.CUSTOM, - ) + email_to: str = form.label("To", widget=form.Widget.CUSTOM) + email_cc: form.StrList = form.label("CC") + email_bcc: form.StrList = form.label("BCC") subject: str = form.label("Subject", widget=form.Widget.CUSTOM) subject_template_hash: str = form.label("Subject template hash", widget=form.Widget.HIDDEN) body: str = form.label("Body", widget=form.Widget.CUSTOM) @@ -37,3 +39,8 @@ class AnnounceForm(form.Form): "Confirm", "Type CONFIRM (in capitals) to enable the submit button.", ) + + @pydantic.model_validator(mode="after") + def _validate_recipients(self) -> "AnnounceForm": + util.validate_email_recipients(self) + return self diff --git a/atr/shared/voting.py b/atr/shared/voting.py index eb14347d..dd44e20d 100644 --- a/atr/shared/voting.py +++ b/atr/shared/voting.py @@ -15,18 +15,16 @@ # specific language governing permissions and limitations # under the License. +import pydantic + import atr.form as form +import atr.util as util class StartVotingForm(form.Form): - mailing_list: str = form.label( - "Send vote email to", - "Note: The options to send to the user-tests " - "mailing list and yourself are provided for " - "testing purposes only, and will not be " - "available in the finished version of ATR.", - widget=form.Widget.RADIO, - ) + email_to: str = form.label("To (mailing list)", widget=form.Widget.CUSTOM) + email_cc: form.StrList = form.label("CC") + email_bcc: form.StrList = form.label("BCC") vote_duration: form.Int = form.label( "Minimum vote duration", "Minimum number of hours the vote will be open for.", @@ -35,3 +33,8 @@ class StartVotingForm(form.Form): subject: str = form.label("Subject", widget=form.Widget.CUSTOM) subject_template_hash: str = form.label("Subject template hash", widget=form.Widget.HIDDEN) body: str = form.label("Body", widget=form.Widget.CUSTOM) + + @pydantic.model_validator(mode="after") + def _validate_recipients(self) -> "StartVotingForm": + util.validate_email_recipients(self) + return self diff --git a/atr/static/css/atr.css b/atr/static/css/atr.css index 1d3313d9..f0ac4417 100644 --- a/atr/static/css/atr.css +++ b/atr/static/css/atr.css @@ -569,6 +569,11 @@ td.atr-shrink { white-space: nowrap; } +th.atr-checkbox-col, +td.atr-checkbox-col { + max-width: 25px; +} + .atr-stripe-odd { background-color: #f8f9fa; } diff --git a/atr/storage/writers/announce.py b/atr/storage/writers/announce.py index b410c774..31345604 100644 --- a/atr/storage/writers/announce.py +++ b/atr/storage/writers/announce.py @@ -30,6 +30,7 @@ import sqlmodel import atr.construct as construct import atr.db as db import atr.mail as mail +import atr.models.basic as basic import atr.models.safe as safe import atr.models.sql as sql import atr.paths as paths @@ -108,15 +109,20 @@ class CommitteeMember(CommitteeParticipant): project_key: safe.ProjectKey, version_key: safe.VersionKey, preview_revision_number: safe.RevisionNumber, - recipient: str, + email_to: str, body: str, download_path_suffix: safe.RelPath | None, asf_uid: str, fullname: str, subject_template_hash: str | None = None, + email_cc: list[str] | None = None, + email_bcc: list[str] | None = None, ) -> None: - if recipient not in util.permitted_announce_recipients(asf_uid): - raise storage.AccessError(f"You are not permitted to send announcements to {recipient}") + permitted = util.permitted_announce_recipients(asf_uid) + all_addrs = [email_to] + (email_cc or []) + (email_bcc or []) + for addr in all_addrs: + if addr not in permitted: + raise storage.AccessError(f"You are not permitted to send announcements to {addr}") unfinished_dir: str = "" finished_dir: str = "" @@ -203,7 +209,9 @@ class CommitteeMember(CommitteeParticipant): revision_number=str(preview_revision_number), source_directory=unfinished_dir, target_directory=finished_dir, - email_recipient=recipient, + email_to=email_to, + email_cc=basic.as_json(email_cc or []), + email_bcc=basic.as_json(email_bcc or []), ) if unfinished_revisions_path: # This removes all of the prior revisions @@ -231,10 +239,12 @@ class CommitteeMember(CommitteeParticipant): task_type=sql.TaskType.MESSAGE_SEND, task_args=message.Send( email_sender=f"{asf_uid}@apache.org", - email_recipient=recipient, + email_to=email_to, subject=subject, body=body, in_reply_to=None, + email_cc=email_cc or [], + email_bcc=email_bcc or [], footer_category=mail.MailFooterCategory.NONE, ).model_dump(), asf_uid=asf_uid, diff --git a/atr/storage/writers/mail.py b/atr/storage/writers/mail.py index 78b98033..729d1276 100644 --- a/atr/storage/writers/mail.py +++ b/atr/storage/writers/mail.py @@ -56,7 +56,7 @@ class FoundationCommitter(GeneralPublic): is_dev = util.is_dev_environment() if is_dev: - log.info(f"Dev environment detected, not sending email to {message.email_recipient}") + log.info(f"Dev environment detected, not sending email to {message.email_to}") mid = util.DEV_TEST_MID errors: list[str] = [] else: @@ -65,7 +65,7 @@ class FoundationCommitter(GeneralPublic): self.__write_as.append_to_audit_log( sent=not is_dev, email_sender=message.email_sender, - email_recipient=message.email_recipient, + email_to=message.email_to, subject=message.subject, mid=mid, in_reply_to=message.in_reply_to, diff --git a/atr/storage/writers/tokens.py b/atr/storage/writers/tokens.py index 0a211bf6..d835325a 100644 --- a/atr/storage/writers/tokens.py +++ b/atr/storage/writers/tokens.py @@ -78,7 +78,7 @@ class FoundationCommitter(GeneralPublic): await self.__data.commit() message = mail.Message( email_sender=NOREPLY_EMAIL_ADDRESS, - email_recipient=f"{self.__asf_uid}@apache.org", + email_to=f"{self.__asf_uid}@apache.org", subject="ATR - New API Token Created", body=f"In ATR a new API token called '{label}' was created for your account. " "If you did not create this token, please revoke it immediately.", @@ -103,7 +103,7 @@ class FoundationCommitter(GeneralPublic): label = pat.label or "[unlabeled]" message = mail.Message( email_sender=NOREPLY_EMAIL_ADDRESS, - email_recipient=f"{self.__asf_uid}@apache.org", + email_to=f"{self.__asf_uid}@apache.org", subject="ATR - Deleted API Token", body=f"In ATR an API token called '{label}' was deleted from your account. " "If you did not delete this token, please check your account immediately.", diff --git a/atr/storage/writers/vote.py b/atr/storage/writers/vote.py index ae5f69da..5eaf8418 100644 --- a/atr/storage/writers/vote.py +++ b/atr/storage/writers/vote.py @@ -85,25 +85,27 @@ class CommitteeParticipant(FoundationCommitter): comment: str, fullname: str, is_binding: bool = False, - ) -> tuple[str, str]: + ) -> tuple[list[str], str]: # Get the email thread latest_vote_task = await interaction.release_latest_vote_task(release) if latest_vote_task is None: - return "", "No vote task found." + return [], "No vote task found." vote_thread_mid = interaction.task_mid_get(latest_vote_task) if vote_thread_mid is None: - return "", "No vote thread found." + return [], "No vote thread found." # Construct the reply email vote_result = latest_vote_task.result if vote_result is None: - return "", "Vote task has not completed yet." + return [], "Vote task has not completed yet." if not isinstance(vote_result, results.VoteInitiate): - return "", "Vote task result is not a VoteInitiate result." + return [], "Vote task result is not a VoteInitiate result." original_subject = vote_result.subject # Arguments for the task to cast a vote - email_recipient = latest_vote_task.task_args["email_to"] + email_to: str = latest_vote_task.task_args["email_to"] + email_cc: list[str] = latest_vote_task.task_args.get("email_cc", []) + email_bcc: list[str] = latest_vote_task.task_args.get("email_bcc", []) email_sender = f"{self.__asf_uid}@apache.org" subject = f"Re: {original_subject}" body_text = format_vote_email_body( @@ -120,10 +122,12 @@ class CommitteeParticipant(FoundationCommitter): task_type=sql.TaskType.MESSAGE_SEND, task_args=message.Send( email_sender=email_sender, - email_recipient=email_recipient, + email_to=email_to, subject=subject, body=body_text, in_reply_to=in_reply_to, + email_cc=email_cc, + email_bcc=email_bcc, footer_category=mail.MailFooterCategory.USER, ).model_dump(), asf_uid=self.__asf_uid, @@ -134,7 +138,7 @@ class CommitteeParticipant(FoundationCommitter): await self.__data.flush() await self.__data.commit() - return email_recipient, "" + return [email_to], "" async def start( self, @@ -150,6 +154,8 @@ class CommitteeParticipant(FoundationCommitter): release: sql.Release | None = None, promote: bool = True, permitted_recipients: list[str] | None = None, + email_cc: list[str] | None = None, + email_bcc: list[str] | None = None, ) -> sql.Task: if release is None: release = await self.__data.release( @@ -160,10 +166,12 @@ class CommitteeParticipant(FoundationCommitter): ).demand(storage.AccessError("Release not found")) if permitted_recipients is None: permitted_recipients = util.permitted_voting_recipients(asf_uid, self.__committee_key) - if email_to not in permitted_recipients: - # This will be checked again by tasks/vote.py for extra safety - log.info(f"Invalid mailing list choice: {email_to} not in {permitted_recipients}") - raise storage.AccessError("Invalid mailing list choice") + all_addrs = [email_to] + (email_cc or []) + (email_bcc or []) + for addr in all_addrs: + if addr not in permitted_recipients: + # This will be checked again by tasks/vote.py for extra safety + log.info(f"Invalid mailing list choice: {addr} not in {permitted_recipients}") + raise storage.AccessError("Invalid mailing list choice") if await interaction.has_blocker_checks(release, selected_revision_number, caller_data=self.__data): raise storage.AccessError( @@ -197,6 +205,8 @@ class CommitteeParticipant(FoundationCommitter): initiator_fullname=asf_fullname, subject=subject, body=body_data, + email_cc=email_cc or [], + email_bcc=email_bcc or [], ).model_dump(), asf_uid=asf_uid, project_key=str(project_key), @@ -443,7 +453,9 @@ class CommitteeMember(CommitteeParticipant): # original_subject = latest_vote_task.task_args["subject"] # Arguments for the task to cast a vote - email_recipient = latest_vote_task.task_args["email_to"] + email_to: str = latest_vote_task.task_args["email_to"] + email_cc: list[str] = latest_vote_task.task_args.get("email_cc", []) + email_bcc: list[str] = latest_vote_task.task_args.get("email_bcc", []) email_sender = f"{asf_uid}@apache.org" subject = f"[VOTE] [RESULT] Release {release.project.display_name} {release.version} {resolution.upper()}" # TODO: This duplicates atr/tabulate.py code @@ -465,10 +477,12 @@ class CommitteeMember(CommitteeParticipant): task_type=sql.TaskType.MESSAGE_SEND, task_args=message.Send( email_sender=email_sender, - email_recipient=email_recipient, + email_to=email_to, subject=subject, body=body, in_reply_to=in_reply_to, + email_cc=email_cc, + email_bcc=email_bcc, footer_category=mail.MailFooterCategory.USER, ).model_dump(), asf_uid=asf_uid, @@ -482,7 +496,7 @@ class CommitteeMember(CommitteeParticipant): task_type=sql.TaskType.MESSAGE_SEND, task_args=message.Send( email_sender=email_sender, - email_recipient=extra_destination[0], + email_to=extra_destination[0], subject=subject, body=body, in_reply_to=extra_destination[1], diff --git a/atr/tasks/message.py b/atr/tasks/message.py index 1fb716e6..7e3f59d9 100644 --- a/atr/tasks/message.py +++ b/atr/tasks/message.py @@ -40,10 +40,12 @@ class Send(schema.Strict): """Arguments for the task to send an email.""" email_sender: pydantic.EmailStr = schema.description("The email address of the sender") - email_recipient: pydantic.EmailStr = schema.description("The email address of the recipient") + email_to: pydantic.EmailStr = schema.description("The email To address") subject: str = schema.description("The subject of the email") body: str = schema.description("The body of the email") in_reply_to: str | None = schema.description("The message ID of the email to reply to") + email_cc: list[pydantic.EmailStr] = schema.factory(list) + email_bcc: list[pydantic.EmailStr] = schema.factory(list) footer_category: Annotated[mail.MailFooterCategory, pydantic.BeforeValidator(_ensure_footer_enum)] = ( schema.description("The category of email footer to include") ) @@ -69,19 +71,23 @@ async def send(args: Send) -> results.Results | None: if ldap.is_banned(sender_account): raise SendError(f"Email account {args.email_sender} is banned") - recipient_domain = args.email_recipient.split("@")[-1] - sending_to_self = args.email_recipient == f"{sender_asf_uid}@apache.org" - # audit_guidance this application intentionally allows users to send messages to committees they are not a part of - sending_to_committee = recipient_domain.endswith(".apache.org") - if not (sending_to_self or sending_to_committee): - raise SendError(f"You are not permitted to send emails to {args.email_recipient}") + all_recipients = [args.email_to, *args.email_cc, *args.email_bcc] + for addr in all_recipients: + recipient_domain = addr.split("@")[-1] + sending_to_self = addr == f"{sender_asf_uid}@apache.org" + # audit_guidance we intentionally allow users to send messages to committees they are not a part of + sending_to_committee = recipient_domain.endswith(".apache.org") + if not (sending_to_self or sending_to_committee): + raise SendError(f"You are not permitted to send emails to {addr}") message = mail.Message( email_sender=args.email_sender, - email_recipient=args.email_recipient, + email_to=args.email_to, subject=args.subject, body=args.body, in_reply_to=args.in_reply_to, + email_cc=args.email_cc, + email_bcc=args.email_bcc, ) footer_category = mail.MailFooterCategory(args.footer_category) @@ -91,7 +97,7 @@ async def send(args: Send) -> results.Results | None: mid, mail_errors = await wafc.mail.send(message, footer_category) if mail_errors: - log.warning(f"Mail sending to {args.email_recipient} for subject '{args.subject}' encountered errors:") + log.warning(f"Mail sending to {args.email_to} for subject '{args.subject}' encountered errors:") for error in mail_errors: log.warning(f"- {error}") diff --git a/atr/tasks/vote.py b/atr/tasks/vote.py index d126135a..c4b93419 100644 --- a/atr/tasks/vote.py +++ b/atr/tasks/vote.py @@ -17,6 +17,8 @@ import datetime +import pydantic + import atr.db as db import atr.db.interaction as interaction import atr.log as log @@ -33,12 +35,14 @@ class Initiate(schema.Strict): """Arguments for the task to start a vote.""" release_key: str = schema.description("The name of the release to vote on") - email_to: str = schema.description("The mailing list address to send the vote email to") + email_to: pydantic.EmailStr = schema.description("The mailing list To address") vote_duration: int = schema.description("Duration of the vote in hours") initiator_id: str = schema.description("ASF ID of the vote initiator") initiator_fullname: str = schema.description("Full name of the vote initiator") subject: str = schema.description("Subject line for the vote email") body: str = schema.description("Body content for the vote email") + email_cc: list[pydantic.EmailStr] = schema.factory(list) + email_bcc: list[pydantic.EmailStr] = schema.factory(list) class VoteInitiationError(Exception): @@ -65,9 +69,11 @@ async def _initiate_core_logic(args: Initiate) -> results.Results | None: safe.ReleaseKey(args.release_key) # Validate arguments - if not (args.email_to.endswith("@apache.org") or args.email_to.endswith(".apache.org")): - log.error(f"Invalid destination email address: {args.email_to}") - raise VoteInitiationError("Invalid destination email address") + all_addrs = [args.email_to, *args.email_cc, *args.email_bcc] + for addr in all_addrs: + if not (addr.endswith("@apache.org") or addr.endswith(".apache.org")): + log.error(f"Invalid destination email address: {addr}") + raise VoteInitiationError(f"Invalid destination email address: {addr}") async with db.session() as data: release = await data.release(key=args.release_key, _project=True, _committee=True).demand( @@ -110,17 +116,20 @@ async def _initiate_core_logic(args: Initiate) -> results.Results | None: body = args.body permitted_recipients = util.permitted_voting_recipients(args.initiator_id, release.committee.key) - if args.email_to not in permitted_recipients: - log.error(f"Invalid mailing list choice: {args.email_to} not in {permitted_recipients}") - raise VoteInitiationError("Invalid mailing list choice") + for addr in all_addrs: + if addr not in permitted_recipients: + log.error(f"Invalid mailing list choice: {addr} not in {permitted_recipients}") + raise VoteInitiationError("Invalid mailing list choice") # Create mail message log.info(f"Creating mail message for {args.email_to}") message = mail.Message( email_sender=f"{args.initiator_id}@apache.org", - email_recipient=args.email_to, + email_to=args.email_to, subject=subject, body=body, + email_cc=args.email_cc, + email_bcc=args.email_bcc, ) async with storage.write(args.initiator_id) as write: @@ -128,6 +137,7 @@ async def _initiate_core_logic(args: Initiate) -> results.Results | None: mid, mail_errors = await wafc.mail.send(message, mail.MailFooterCategory.USER) # Original success message structure + all_destinations = [args.email_to, *args.email_cc, *args.email_bcc] result = results.VoteInitiate( kind="vote_initiate", message="Vote announcement email sent successfully", @@ -139,7 +149,7 @@ async def _initiate_core_logic(args: Initiate) -> results.Results | None: ) if mail_errors: - log.warning(f"Start vote for {args.release_key}: sending to {args.email_to} gave errors: {mail_errors}") + log.warning(f"Start vote for {args.release_key}: sending to {all_destinations} gave errors: {mail_errors}") else: - log.info(f"Vote email sent successfully to {args.email_to}") + log.info(f"Vote email sent successfully to {all_destinations}") return result diff --git a/atr/util.py b/atr/util.py index fcbb1248..bcaf24a0 100644 --- a/atr/util.py +++ b/atr/util.py @@ -35,7 +35,7 @@ import urllib.parse import uuid import zipfile from collections.abc import AsyncGenerator, Callable, Iterable, Sequence -from typing import Any, Final +from typing import Any, Final, Protocol import aiofiles.os import aiohttp @@ -82,6 +82,12 @@ USER_TESTS_ADDRESS: Final[str] = "[email protected]" NoneType: Final[type[None]] = type(None) +class EmailRecipients(Protocol): + email_to: str + email_cc: list[str] + email_bcc: list[str] + + @dataclasses.dataclass(frozen=True) class NpmPackInfo: name: str @@ -247,6 +253,20 @@ def committee_is_standing(committee_key: str) -> bool: return committee_key in registry.STANDING_COMMITTEES +def conjunction(items: Sequence[str], empty: str | None = None) -> str: + match len(items): + case 0: + if empty is None: + raise ValueError("No items to join") + return empty + case 1: + return items[0] + case 2: + return f"{items[0]} and {items[1]}" + case _: + return ", ".join(items[:-1]) + f", and {items[-1]}" + + async def content_list( phase_subdir: pathlib.Path, project_key: str, version_key: str, revision_name: str | None = None ) -> AsyncGenerator[FileStat]: @@ -1128,6 +1148,17 @@ def validate_as_type[T](value: Any, t: type[T]) -> T: return value +def validate_email_recipients(recipients: EmailRecipients) -> None: + if not recipients.email_to: + raise ValueError("At least one To recipient is required") + seen: set[str] = set() + for addr in [recipients.email_to, *recipients.email_cc, *recipients.email_bcc]: + lower = addr.lower() + if lower in seen: + raise ValueError(f"Duplicate recipient: {addr}") + seen.add(lower) + + def validate_filename(filename: str) -> str: return validate_path_segment(filename, "Filename") diff --git a/tests/unit/test_mail.py b/tests/unit/test_mail.py index 5957527e..dd3b59de 100644 --- a/tests/unit/test_mail.py +++ b/tests/unit/test_mail.py @@ -38,7 +38,7 @@ async def test_address_objects_used_for_from_to_headers(monkeypatch: "MonkeyPatc legitimate_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test Subject", body="Test body", ) @@ -58,6 +58,46 @@ async def test_address_objects_used_for_from_to_headers(monkeypatch: "MonkeyPatc assert "To: [email protected]" in msg_text [email protected] +async def test_footer_auto_appended_to_body(monkeypatch: "MonkeyPatch") -> None: + """Test that AUTO category appends an automation footer without a user attribution.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Footer test", + body="Hello.", + ) + + _, errors = await mail.send(msg, mail.MailFooterCategory.AUTO) + + assert len(errors) == 0 + msg_text = mock_send_many.call_args[0][2] + assert "This email was sent from automation on the Apache Trusted Releases platform" in msg_text + + [email protected] +async def test_footer_user_appended_to_body(monkeypatch: "MonkeyPatch") -> None: + """Test that USER category appends a footer attributing the sending user.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Footer test", + body="Hello.", + ) + + _, errors = await mail.send(msg, mail.MailFooterCategory.USER) + + assert len(errors) == 0 + msg_text = mock_send_many.call_args[0][2] + assert "This email was sent by [email protected] on the Apache Trusted Releases platform" in msg_text + + @pytest.mark.asyncio async def test_send_accepts_legitimate_message(monkeypatch: "MonkeyPatch") -> None: """Test that a legitimate message without CRLF is accepted.""" @@ -67,7 +107,7 @@ async def test_send_accepts_legitimate_message(monkeypatch: "MonkeyPatch") -> No # Create a legitimate message without any CRLF injection attempts legitimate_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Legitimate Subject", body="This is a legitimate test message with no injection attempts.", ) @@ -100,7 +140,7 @@ async def test_send_accepts_message_with_reply_to(monkeypatch: "MonkeyPatch") -> # Create a legitimate message with a valid in_reply_to legitimate_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Re: Previous Subject", body="This is a reply message.", in_reply_to="[email protected]", @@ -119,6 +159,87 @@ async def test_send_accepts_message_with_reply_to(monkeypatch: "MonkeyPatch") -> assert "@apache.org" in mid [email protected] +async def test_send_bcc_in_envelope_not_in_headers(monkeypatch: "MonkeyPatch") -> None: + """Test that BCC addresses are in the SMTP envelope but absent from all message headers.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="BCC test", + body="Hello.", + email_bcc=["[email protected]"], + ) + + _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) + + assert len(errors) == 0 + call_args = mock_send_many.call_args + envelope_recipients = call_args[0][1] + msg_text = call_args[0][2] + + # BCC must be in the SMTP envelope + assert "[email protected]" in envelope_recipients + + # BCC must not appear anywhere in the message headers + assert "[email protected]" not in msg_text + + [email protected] +async def test_send_cc_appears_in_header_and_envelope(monkeypatch: "MonkeyPatch") -> None: + """Test that CC addresses appear in the Cc header and the SMTP envelope.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="CC test", + body="Hello.", + email_cc=["[email protected]"], + ) + + _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) + + assert len(errors) == 0 + call_args = mock_send_many.call_args + envelope_recipients = call_args[0][1] + msg_text = call_args[0][2] + + # CC must be in the SMTP envelope + assert "[email protected]" in envelope_recipients + + # CC must appear in a Cc header, not only To + assert "Cc: [email protected]" in msg_text + + [email protected] +async def test_send_empty_cc_bcc_omits_cc_header(monkeypatch: "MonkeyPatch") -> None: + """Test that omitting CC or BCC produces no Cc header and only To recipients in envelope.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="No CC or BCC test", + body="Hello.", + ) + + _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) + + assert len(errors) == 0 + call_args = mock_send_many.call_args + envelope_recipients = call_args[0][1] + msg_text = call_args[0][2] + + assert envelope_recipients == ["[email protected]"] + assert "Cc:" not in msg_text + assert "Bcc:" not in msg_text + + @pytest.mark.asyncio async def test_send_handles_non_ascii_headers(monkeypatch: "MonkeyPatch") -> None: """Test that non-ASCII characters in headers are handled correctly.""" @@ -128,7 +249,7 @@ async def test_send_handles_non_ascii_headers(monkeypatch: "MonkeyPatch") -> Non # Create a message with non-ASCII characters in the subject message_with_unicode = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test avec Accént", body="This message has non-ASCII characters in the subject.", ) @@ -149,6 +270,34 @@ async def test_send_handles_non_ascii_headers(monkeypatch: "MonkeyPatch") -> Non assert "Subject: Test avec Accént" in msg_text [email protected] +async def test_send_to_with_cc_in_envelope(monkeypatch: "MonkeyPatch") -> None: + """Test that To and CC addresses both appear in the SMTP envelope.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Multi-recipient test", + body="Hello both.", + email_cc=["[email protected]"], + ) + + _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) + + assert len(errors) == 0 + call_args = mock_send_many.call_args + envelope_recipients = call_args[0][1] + msg_text = call_args[0][2] + + assert "[email protected]" in envelope_recipients + assert "[email protected]" in envelope_recipients + + assert "[email protected]" in msg_text + assert "[email protected]" in msg_text + + @pytest.mark.asyncio async def test_send_rejects_bcc_header_injection(monkeypatch: "MonkeyPatch") -> None: """Test a realistic Bcc header injection attack scenario.""" @@ -158,7 +307,7 @@ async def test_send_rejects_bcc_header_injection(monkeypatch: "MonkeyPatch") -> # Create a malicious message attempting to inject a Bcc header malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Important Notice\r\nBcc: [email protected]\r\nX-Priority: 1", body="This message attempts to secretly copy an attacker.", ) @@ -174,6 +323,26 @@ async def test_send_rejects_bcc_header_injection(monkeypatch: "MonkeyPatch") -> mock_send_many.assert_not_called() [email protected] +async def test_send_rejects_case_insensitive_duplicate(monkeypatch: "MonkeyPatch") -> None: + """Test that duplicate detection is case-insensitive.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Dup test", + body="Hello.", + email_bcc=["[email protected]"], + ) + + with pytest.raises(ValueError, match=r"Duplicate recipient"): + await mail.send(msg, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + @pytest.mark.asyncio async def test_send_rejects_content_type_injection(monkeypatch: "MonkeyPatch") -> None: """Test injection attempting to override Content-Type header.""" @@ -183,7 +352,7 @@ async def test_send_rejects_content_type_injection(monkeypatch: "MonkeyPatch") - # Create a malicious message attempting to inject Content-Type malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test\r\nContent-Type: text/html\r\n\r\n<html><script>alert('XSS')</script></html>", body="Normal body", ) @@ -208,7 +377,7 @@ async def test_send_rejects_cr_only_injection(monkeypatch: "MonkeyPatch") -> Non # Create a malicious message with just CR (no LF) malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Legitimate Subject\rBcc: [email protected]", body="This is a test message", ) @@ -224,6 +393,50 @@ async def test_send_rejects_cr_only_injection(monkeypatch: "MonkeyPatch") -> Non mock_send_many.assert_not_called() [email protected] +async def test_send_rejects_crlf_in_bcc_address(monkeypatch: "MonkeyPatch") -> None: + """Test that CRLF injection in the BCC address field is rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + malicious_message = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Test Subject", + body="This is a test message", + email_bcc=["[email protected]\r\nTo: [email protected]"], + ) + + with pytest.raises(ValueError, match=r"CR/LF"): + await mail.send(malicious_message, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + [email protected] +async def test_send_rejects_crlf_in_cc_address(monkeypatch: "MonkeyPatch") -> None: + """Test that CRLF injection in the CC address field is rejected. + + An attacker supplying CC addresses controls what goes into the Cc header, + so CR/LF must be caught before address objects are constructed. + """ + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + malicious_message = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Test Subject", + body="This is a test message", + email_cc=["[email protected]\r\nBcc: [email protected]"], + ) + + with pytest.raises(ValueError, match=r"CR/LF"): + await mail.send(malicious_message, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + @pytest.mark.asyncio async def test_send_rejects_crlf_in_from_address(monkeypatch: "MonkeyPatch") -> None: """Test that CRLF injection in from address field is rejected. @@ -237,7 +450,7 @@ async def test_send_rejects_crlf_in_from_address(monkeypatch: "MonkeyPatch") -> # Create a malicious message with CRLF in the from address malicious_message = mail.Message( email_sender="[email protected]\r\nBcc: [email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test Subject", body="This is a test message", ) @@ -259,7 +472,7 @@ async def test_send_rejects_crlf_in_reply_to(monkeypatch: "MonkeyPatch") -> None # Create a malicious message with CRLF in the in_reply_to field malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test Subject", body="This is a test message", in_reply_to="[email protected]\r\nBcc: [email protected]", @@ -286,7 +499,7 @@ async def test_send_rejects_crlf_in_subject(monkeypatch: "MonkeyPatch") -> None: # Create a malicious message with CRLF in the subject malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Legitimate Subject\r\nBcc: [email protected]", body="This is a test message", ) @@ -315,7 +528,7 @@ async def test_send_rejects_crlf_in_to_address(monkeypatch: "MonkeyPatch") -> No # Create a malicious message with CRLF in the to address malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]\r\nBcc: [email protected]", + email_to="[email protected]\r\nBcc: [email protected]", subject="Test Subject", body="This is a test message", ) @@ -328,6 +541,86 @@ async def test_send_rejects_crlf_in_to_address(monkeypatch: "MonkeyPatch") -> No mock_send_many.assert_not_called() [email protected] +async def test_send_rejects_duplicate_across_cc_and_bcc(monkeypatch: "MonkeyPatch") -> None: + """Test that the same address in CC and BCC is rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Dup test", + body="Hello.", + email_cc=["[email protected]"], + email_bcc=["[email protected]"], + ) + + with pytest.raises(ValueError, match=r"Duplicate recipient"): + await mail.send(msg, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + [email protected] +async def test_send_rejects_duplicate_across_to_and_cc(monkeypatch: "MonkeyPatch") -> None: + """Test that the same address in To and CC is rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Dup test", + body="Hello.", + email_cc=["[email protected]"], + ) + + with pytest.raises(ValueError, match=r"Duplicate recipient"): + await mail.send(msg, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + [email protected] +async def test_send_rejects_duplicate_across_to_and_bcc(monkeypatch: "MonkeyPatch") -> None: + """Test that the same address in To and BCC is rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Dup test", + body="Hello.", + email_bcc=["[email protected]"], + ) + + with pytest.raises(ValueError, match=r"Duplicate recipient"): + await mail.send(msg, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + [email protected] +async def test_send_rejects_empty_to(monkeypatch: "MonkeyPatch") -> None: + """Test that an empty To list is rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + msg = mail.Message( + email_sender="[email protected]", + email_to="", + subject="Empty To test", + body="Hello.", + ) + + with pytest.raises(ValueError, match=r"At least one To recipient is required"): + await mail.send(msg, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + @pytest.mark.asyncio async def test_send_rejects_lf_only_injection(monkeypatch: "MonkeyPatch") -> None: """Test that injection with LF only (\\n) is also rejected.""" @@ -337,7 +630,7 @@ async def test_send_rejects_lf_only_injection(monkeypatch: "MonkeyPatch") -> Non # Create a malicious message with just LF (no CR) malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Legitimate Subject\nBcc: [email protected]", body="This is a test message", ) @@ -353,6 +646,26 @@ async def test_send_rejects_lf_only_injection(monkeypatch: "MonkeyPatch") -> Non mock_send_many.assert_not_called() [email protected] +async def test_send_rejects_null_byte_in_bcc(monkeypatch: "MonkeyPatch") -> None: + """Test that null bytes in the BCC field are rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + malicious_message = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Test Subject", + body="This is a test message", + email_bcc=["bcc\[email protected]"], + ) + + with pytest.raises(ValueError, match=r"null bytes"): + await mail.send(malicious_message, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + @pytest.mark.asyncio async def test_send_rejects_null_byte_in_body(monkeypatch: "MonkeyPatch") -> None: """Test that null bytes in body field are rejected.""" @@ -361,7 +674,7 @@ async def test_send_rejects_null_byte_in_body(monkeypatch: "MonkeyPatch") -> Non malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test Subject", body="Normal start\x00injected content", ) @@ -372,6 +685,26 @@ async def test_send_rejects_null_byte_in_body(monkeypatch: "MonkeyPatch") -> Non mock_send_many.assert_not_called() [email protected] +async def test_send_rejects_null_byte_in_cc(monkeypatch: "MonkeyPatch") -> None: + """Test that null bytes in the CC field are rejected.""" + mock_send_many = mock.AsyncMock(return_value=[]) + monkeypatch.setattr("atr.mail._send_many", mock_send_many) + + malicious_message = mail.Message( + email_sender="[email protected]", + email_to="[email protected]", + subject="Test Subject", + body="This is a test message", + email_cc=["cc\[email protected]"], + ) + + with pytest.raises(ValueError, match=r"null bytes"): + await mail.send(malicious_message, mail.MailFooterCategory.NONE) + + mock_send_many.assert_not_called() + + @pytest.mark.asyncio async def test_send_rejects_null_byte_in_from_address(monkeypatch: "MonkeyPatch") -> None: """Test that null bytes in from address field are rejected.""" @@ -380,7 +713,7 @@ async def test_send_rejects_null_byte_in_from_address(monkeypatch: "MonkeyPatch" malicious_message = mail.Message( email_sender="sender\[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test Subject", body="This is a test message", ) @@ -399,7 +732,7 @@ async def test_send_rejects_null_byte_in_reply_to(monkeypatch: "MonkeyPatch") -> malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Test Subject", body="This is a test message", in_reply_to="valid-id\[email protected]", @@ -419,7 +752,7 @@ async def test_send_rejects_null_byte_in_subject(monkeypatch: "MonkeyPatch") -> malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="[email protected]", + email_to="[email protected]", subject="Legitimate Subject\x00Bcc: [email protected]", body="This is a test message", ) @@ -438,7 +771,7 @@ async def test_send_rejects_null_byte_in_to_address(monkeypatch: "MonkeyPatch") malicious_message = mail.Message( email_sender="[email protected]", - email_recipient="recipient\[email protected]", + email_to="recipient\[email protected]", subject="Test Subject", body="This is a test message", ) @@ -497,237 +830,3 @@ def test_split_address_rejects_null_byte() -> None: """Test that _split_address rejects addresses containing null bytes.""" with pytest.raises(ValueError, match=r"null bytes"): mail._split_address("user\[email protected]") - - [email protected] -async def test_send_rejects_null_byte_in_cc(monkeypatch: "MonkeyPatch") -> None: - """Test that null bytes in the CC field are rejected.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - malicious_message = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="Test Subject", - body="This is a test message", - email_cc="cc\[email protected]", - ) - - with pytest.raises(ValueError, match=r"null bytes"): - await mail.send(malicious_message, mail.MailFooterCategory.NONE) - - mock_send_many.assert_not_called() - - [email protected] -async def test_send_rejects_null_byte_in_bcc(monkeypatch: "MonkeyPatch") -> None: - """Test that null bytes in the BCC field are rejected.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - malicious_message = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="Test Subject", - body="This is a test message", - email_bcc="bcc\[email protected]", - ) - - with pytest.raises(ValueError, match=r"null bytes"): - await mail.send(malicious_message, mail.MailFooterCategory.NONE) - - mock_send_many.assert_not_called() - - [email protected] -async def test_send_rejects_crlf_in_cc_address(monkeypatch: "MonkeyPatch") -> None: - """Test that CRLF injection in the CC address field is rejected. - - An attacker supplying CC addresses controls what goes into the Cc header, - so CR/LF must be caught before address objects are constructed. - """ - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - malicious_message = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="Test Subject", - body="This is a test message", - email_cc="[email protected]\r\nBcc: [email protected]", - ) - - with pytest.raises(ValueError, match=r"CR/LF"): - await mail.send(malicious_message, mail.MailFooterCategory.NONE) - - mock_send_many.assert_not_called() - - [email protected] -async def test_send_rejects_crlf_in_bcc_address(monkeypatch: "MonkeyPatch") -> None: - """Test that CRLF injection in the BCC address field is rejected.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - malicious_message = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="Test Subject", - body="This is a test message", - email_bcc="[email protected]\r\nTo: [email protected]", - ) - - with pytest.raises(ValueError, match=r"CR/LF"): - await mail.send(malicious_message, mail.MailFooterCategory.NONE) - - mock_send_many.assert_not_called() - - [email protected] -async def test_send_multiple_to_addresses(monkeypatch: "MonkeyPatch") -> None: - """Test that multiple comma-separated To addresses all appear in the header and envelope.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - msg = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected], [email protected]", - subject="Multi-recipient test", - body="Hello both.", - ) - - _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) - - assert len(errors) == 0 - call_args = mock_send_many.call_args - envelope_recipients = call_args[0][1] - msg_text = call_args[0][2] - - # Both addresses must be in the SMTP envelope - assert "[email protected]" in envelope_recipients - assert "[email protected]" in envelope_recipients - - # Both addresses must appear in the To header - assert "[email protected]" in msg_text - assert "[email protected]" in msg_text - - [email protected] -async def test_send_cc_appears_in_header_and_envelope(monkeypatch: "MonkeyPatch") -> None: - """Test that CC addresses appear in the Cc header and the SMTP envelope.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - msg = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="CC test", - body="Hello.", - email_cc="[email protected]", - ) - - _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) - - assert len(errors) == 0 - call_args = mock_send_many.call_args - envelope_recipients = call_args[0][1] - msg_text = call_args[0][2] - - # CC must be in the SMTP envelope - assert "[email protected]" in envelope_recipients - - # CC must appear in a Cc header, not only To - assert "Cc: [email protected]" in msg_text - - [email protected] -async def test_send_bcc_in_envelope_not_in_headers(monkeypatch: "MonkeyPatch") -> None: - """Test that BCC addresses are in the SMTP envelope but absent from all message headers.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - msg = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="BCC test", - body="Hello.", - email_bcc="[email protected]", - ) - - _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) - - assert len(errors) == 0 - call_args = mock_send_many.call_args - envelope_recipients = call_args[0][1] - msg_text = call_args[0][2] - - # BCC must be in the SMTP envelope - assert "[email protected]" in envelope_recipients - - # BCC must not appear anywhere in the message headers - assert "[email protected]" not in msg_text - - [email protected] -async def test_send_empty_cc_bcc_omits_cc_header(monkeypatch: "MonkeyPatch") -> None: - """Test that omitting CC/BCC produces no Cc header and only To recipients in envelope.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - msg = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="No CC/BCC test", - body="Hello.", - ) - - _, errors = await mail.send(msg, mail.MailFooterCategory.NONE) - - assert len(errors) == 0 - call_args = mock_send_many.call_args - envelope_recipients = call_args[0][1] - msg_text = call_args[0][2] - - assert envelope_recipients == ["[email protected]"] - assert "Cc:" not in msg_text - assert "Bcc:" not in msg_text - - [email protected] -async def test_footer_user_appended_to_body(monkeypatch: "MonkeyPatch") -> None: - """Test that USER category appends a footer attributing the sending user.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - msg = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="Footer test", - body="Hello.", - ) - - _, errors = await mail.send(msg, mail.MailFooterCategory.USER) - - assert len(errors) == 0 - msg_text = mock_send_many.call_args[0][2] - assert "This email was sent by [email protected] on the Apache Trusted Releases platform" in msg_text - - [email protected] -async def test_footer_auto_appended_to_body(monkeypatch: "MonkeyPatch") -> None: - """Test that AUTO category appends an automation footer without a user attribution.""" - mock_send_many = mock.AsyncMock(return_value=[]) - monkeypatch.setattr("atr.mail._send_many", mock_send_many) - - msg = mail.Message( - email_sender="[email protected]", - email_recipient="[email protected]", - subject="Footer test", - body="Hello.", - ) - - _, errors = await mail.send(msg, mail.MailFooterCategory.AUTO) - - assert len(errors) == 0 - msg_text = mock_send_many.call_args[0][2] - assert "This email was sent from automation on the Apache Trusted Releases platform" in msg_text diff --git a/tests/unit/test_message.py b/tests/unit/test_message.py index 4abbd64a..7bb05be1 100644 --- a/tests/unit/test_message.py +++ b/tests/unit/test_message.py @@ -113,14 +113,14 @@ async def test_send_succeeds_with_valid_asf_id(monkeypatch: "MonkeyPatch") -> No def _send_args( email_sender: str = "[email protected]", - email_recipient: str = "[email protected]", + email_to: str = "[email protected]", subject: str = "Test Subject", body: str = "Test body", in_reply_to: str | None = None, -) -> dict[str, str | None]: +) -> dict[str, object]: return { "email_sender": email_sender, - "email_recipient": email_recipient, + "email_to": email_to, "subject": subject, "body": body, "in_reply_to": in_reply_to, --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
