From c46662561ed0c0498573df59e5427fdecbcb3b1d Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Fri, 28 Aug 2026 21:19:35 +0200 Subject: [PATCH] Reject unbalanced repeat markers in mail templates validate_mail_template() only looked at placeholder names, and "{{#claims}}" is not a placeholder -- so an unclosed block or a stray "{{/claims}}" passed the check and rendered as itself: the member would read the marker in their mail. Repeat markers are now checked structurally: every one names a block the template actually has, openers and closers pair up in order, blocks do not nest (the renderer does not support it either), and the subject takes no markers at all. Each case explains what is wrong and what is missing. The same check runs before sending, not just before saving: the templates are plain files in the store and can be edited outside CCMA, where refusing to send beats mailing a marker. Saving several edited templates now validates all of them before writing the first, so a mistake in one no longer leaves the others half-saved. Co-Authored-By: Claude Opus 5 --- src/ccma/domain/mail_templates.py | 48 +++++++++++++++++++++ src/ccma/services/mail_composition.py | 19 +++++++-- src/ccma/ui/options_dialog.py | 17 ++++++-- tests/test_mail_templates.py | 61 +++++++++++++++++++++++++++ tests/test_options_dialog_ui.py | 41 +++++++++++++++++- tests/test_welcome_mail.py | 26 ++++++++++++ 6 files changed, 205 insertions(+), 7 deletions(-) diff --git a/src/ccma/domain/mail_templates.py b/src/ccma/domain/mail_templates.py index 39b27bd..b1c2b14 100644 --- a/src/ccma/domain/mail_templates.py +++ b/src/ccma/domain/mail_templates.py @@ -10,6 +10,10 @@ PLACEHOLDER_PATTERN = re.compile(r"\{\{\s*([a-z][a-z0-9_.]*)\s*\}\}", re.IGNOREC # A repeat block either wraps a single line ("{{#claims}}- {{claim.title}}{{/claims}}") # or spans several lines with the markers on their own lines; both render one copy of # the block per entry. +# Matches a single repeat marker, opening or closing. The placeholder pattern +# deliberately does not, so unbalanced markers need a check of their own -- they +# would otherwise survive validation and end up verbatim in a sent mail. +MARKER_PATTERN = re.compile(r"\{\{\s*([#/])\s*([a-z][a-z0-9_.]*)\s*\}\}", re.IGNORECASE) BLOCK_PATTERN = re.compile( r"\{\{\s*#\s*([a-z][a-z0-9_.]*)\s*\}\}[ \t]*\n?(.*?)\{\{\s*/\s*\1\s*\}\}[ \t]*(\n?)", re.IGNORECASE | re.DOTALL, @@ -254,10 +258,54 @@ def render_mail_template( return MailTemplate(subject.strip(), _render_text(body, values)) +def validate_block_markers(key: str, subject: str, body: str) -> None: + """Checks that every {{#block}} is closed by its own {{/block}} and that no + stray marker is left over. An unbalanced marker is not a placeholder, so it + renders as itself -- the recipient would read "{{#claims}}" in their mail.""" + spec = template_spec(key) + known = {block.name.casefold(): block.name for block in spec.blocks} + for match in MARKER_PATTERN.finditer(subject): + raise MailTemplateError( + f"Wiederholungsblöcke sind im Betreff nicht möglich: {match.group(0)}" + ) + open_marker: tuple[str, str] | None = None + for match in MARKER_PATTERN.finditer(body): + kind, name = match.group(1), match.group(2) + if name.casefold() not in known: + raise MailTemplateError( + f"Unbekannter Wiederholungsblock: {match.group(0)}. Verfügbar: " + + (", ".join(f"{{{{#{value}}}}}" for value in known.values()) or "keine") + ) + if kind == "#": + if open_marker is not None: + raise MailTemplateError( + f"Wiederholungsblöcke können nicht ineinander verschachtelt werden: " + f"{{{{#{open_marker[1]}}}}} ist noch offen, als {match.group(0)} beginnt." + ) + open_marker = (kind, name) + continue + if open_marker is None: + raise MailTemplateError( + f"{match.group(0)} steht ohne zugehöriges {{{{#{name}}}}} in der Vorlage." + ) + if open_marker[1].casefold() != name.casefold(): + raise MailTemplateError( + f"{{{{#{open_marker[1]}}}}} wird durch {match.group(0)} geschlossen. " + f"Erwartet wird {{{{/{open_marker[1]}}}}}." + ) + open_marker = None + if open_marker is not None: + raise MailTemplateError( + f"{{{{#{open_marker[1]}}}}} wird nicht geschlossen. Es fehlt " + f"{{{{/{open_marker[1]}}}}}." + ) + + def validate_mail_template(key: str, subject: str, body: str) -> None: """Rejects placeholders the mail service will never supply, so a typo surfaces while editing the template instead of during a send run.""" spec = template_spec(key) + validate_block_markers(key, subject, body) known = {name for name, _help in spec.placeholders} block_names = {block.name for block in spec.blocks} unknown: set[str] = set() diff --git a/src/ccma/services/mail_composition.py b/src/ccma/services/mail_composition.py index 3c6b0ca..0ca7395 100644 --- a/src/ccma/services/mail_composition.py +++ b/src/ccma/services/mail_composition.py @@ -5,7 +5,13 @@ from email.message import EmailMessage from email.policy import SMTP from email.utils import format_datetime, make_msgid -from ccma.domain.mail_templates import MailTemplate, MailTemplateError, render_mail_template +from ccma.domain.mail_templates import ( + MailTemplate, + MailTemplateError, + render_mail_template, + template_spec, + validate_block_markers, +) from ccma.domain.models import Member from ccma.domain.placeholders import base_placeholder_values from ccma.storage.repository import MemberRepository, RepositoryError @@ -61,10 +67,17 @@ def render_template( values: dict[str, str], repeats: dict[str, list[dict[str, str]]] | None = None, ) -> MailTemplate: + template = repository.get_mail_template(key) try: - return render_mail_template(repository.get_mail_template(key), values, repeats) + # The templates are plain files in the store and may have been edited outside + # CCMA, so the structural check runs again here: refusing to send beats + # sending a mail with "{{#claims}}" in its text. + validate_block_markers(key, template.subject, template.body) + return render_mail_template(template, values, repeats) except MailTemplateError as exc: - raise RepositoryError(str(exc)) from exc + raise RepositoryError( + f"Die E-Mail-Vorlage „{template_spec(key).label}“ ist fehlerhaft: {exc}" + ) from exc def compose_mail( diff --git a/src/ccma/ui/options_dialog.py b/src/ccma/ui/options_dialog.py index 215a434..105cebf 100644 --- a/src/ccma/ui/options_dialog.py +++ b/src/ccma/ui/options_dialog.py @@ -13,6 +13,7 @@ from ccma.domain.mail_templates import ( default_mail_template, placeholder_entries, template_spec, + validate_mail_template, ) from ccma.domain.models import HOUSEKEEPER_MEMBER_FIELD_LABELS from ccma.services.intervals import ( @@ -1095,9 +1096,19 @@ class OptionsDialog(tk.Toplevel): def _save_mail_templates(self) -> None: self._capture_mail_template() - for key, (subject, body) in self.mail_template_edits.items(): - if self.mail_template_original.get(key) == (subject, body): - continue + pending = { + key: value + for key, value in self.mail_template_edits.items() + if self.mail_template_original.get(key) != value + } + # Check every edited template before writing the first one: a mistake in one + # must not leave the others half-saved. + for key, (subject, body) in pending.items(): + try: + validate_mail_template(key, subject, body) + except MailTemplateError as exc: + raise RepositoryError(f"{template_spec(key).label}: {exc}") from exc + for key, (subject, body) in pending.items(): self.repository.save_mail_template(key, subject=subject, body=body) def _center_on_parent(self) -> None: diff --git a/tests/test_mail_templates.py b/tests/test_mail_templates.py index aae0ba7..f266f4f 100644 --- a/tests/test_mail_templates.py +++ b/tests/test_mail_templates.py @@ -3,12 +3,15 @@ import pytest from ccma.domain.mail_templates import ( MAIL_TEMPLATES, MailTemplate, + MailTemplateBlock, MailTemplateError, + MailTemplateSpec, default_mail_template, parse_mail_template, placeholder_entries, render_mail_template, serialize_mail_template, + validate_block_markers, validate_mail_template, ) from ccma.storage.repository import MemberRepository, RepositoryError @@ -154,3 +157,61 @@ def test_every_block_snippet_is_valid_in_its_own_template() -> None: for spec in MAIL_TEMPLATES: for block in spec.blocks: validate_mail_template(spec.key, "Betreff", block.snippet) + + +@pytest.mark.parametrize( + ("body", "expected"), + [ + ("Hallo\n{{#claims}}\n{{claim.title}}", "wird nicht geschlossen"), + ("Hallo\n{{/claims}}\nGruß", "ohne zugehöriges"), + ("{{#claims}}\n{{claim.title}}\n{{/claims}}\n{{/claims}}", "ohne zugehöriges"), + ("{{#claims}}\n{{#claims}}\nx\n{{/claims}}\n{{/claims}}", "verschachtelt"), + ("{{#members}}\nx\n{{/members}}", "Unbekannter Wiederholungsblock"), + ("{{ # claims }}\nx", "wird nicht geschlossen"), + ], +) +def test_unbalanced_repeat_markers_are_rejected(body, expected) -> None: + with pytest.raises(MailTemplateError, match=expected): + validate_mail_template("welcome", "Betreff", body) + + +def test_repeat_markers_in_the_subject_are_rejected() -> None: + with pytest.raises(MailTemplateError, match="Betreff"): + validate_mail_template("welcome", "Beitrag {{#claims}}", "Hallo") + + +def test_a_template_without_blocks_rejects_every_marker() -> None: + with pytest.raises(MailTemplateError, match="Verfügbar: keine"): + validate_mail_template("sepa", "Betreff", "{{#claims}}\nx\n{{/claims}}") + + +def test_a_closing_marker_must_match_the_open_one(monkeypatch) -> None: + two_blocks = MailTemplateSpec( + key="two-blocks", + label="Test", + filename="test.txt", + description="", + placeholders=(), + blocks=( + MailTemplateBlock("first", "", (("item.a", ""),), "{{item.a}}"), + MailTemplateBlock("second", "", (("item.b", ""),), "{{item.b}}"), + ), + ) + monkeypatch.setattr("ccma.domain.mail_templates.MAIL_TEMPLATES", (two_blocks,)) + + with pytest.raises(MailTemplateError, match="Erwartet wird \\{\\{/first\\}\\}"): + validate_block_markers("two-blocks", "Betreff", "{{#first}}\nx\n{{/second}}") + + +def test_a_broken_template_is_not_stored(tmp_path) -> None: + repository = MemberRepository(tmp_path) + repository.initialize() + path = repository.mail_templates_root / "willkommen.txt" + before = path.read_text(encoding="utf-8") + + with pytest.raises(RepositoryError, match="wird nicht geschlossen"): + repository.save_mail_template( + "welcome", subject="Willkommen", body="Hallo\n{{#claims}}\n{{claim.title}}" + ) + + assert path.read_text(encoding="utf-8") == before diff --git a/tests/test_options_dialog_ui.py b/tests/test_options_dialog_ui.py index 1ce34c4..c7bea97 100644 --- a/tests/test_options_dialog_ui.py +++ b/tests/test_options_dialog_ui.py @@ -8,7 +8,7 @@ tk = pytest.importorskip("tkinter") from ccma.config import AppConfig # noqa: E402 from ccma.domain.mail_templates import placeholder_entries # noqa: E402 from ccma.services.welcome_mail import generate_and_send_welcome_mail # noqa: E402 -from ccma.storage.repository import MemberRepository # noqa: E402 +from ccma.storage.repository import MemberRepository, RepositoryError # noqa: E402 # One root for the whole session: the icon library binds its images to the first @@ -136,3 +136,42 @@ def test_edited_template_survives_saving_and_produces_real_lines_in_the_mail( ).get_content() assert "Aufnahmegebühr (fällig 17.09.2026): 15.00 Euro" in content assert "…" not in content + + +def test_the_dialog_refuses_to_save_an_unbalanced_block(tk_root, repository): + stored = repository.mail_templates_root / "willkommen.txt" + before = stored.read_text(encoding="utf-8") + + dialog = _open_options(tk_root, repository) + try: + dialog.mail_template_body.delete("1.0", "end") + dialog.mail_template_body.insert("1.0", "Offen sind aktuell:\n{{#claims}}\n{{claim.title}}") + with pytest.raises(RepositoryError, match="wird nicht geschlossen"): + dialog._save_mail_templates() + finally: + dialog.grab_release() + dialog.destroy() + + assert stored.read_text(encoding="utf-8") == before + + +def test_a_broken_template_does_not_half_save_the_others(tk_root, repository): + reminder_file = repository.mail_templates_root / "mahnung.txt" + before = reminder_file.read_text(encoding="utf-8") + + dialog = _open_options(tk_root, repository) + try: + # Edit a valid template first, then break the second one. + dialog.mail_template_var.set("Mahnung") + dialog._select_mail_template() + dialog.mail_template_body.insert("end", "\nP.S. Bitte Mitgliedsnummer angeben.") + dialog.mail_template_var.set("Willkommen & Erstrechnung") + dialog._select_mail_template() + dialog.mail_template_body.insert("end", "\n{{#claims}}\n{{claim.title}}") + with pytest.raises(RepositoryError, match="wird nicht geschlossen"): + dialog._save_mail_templates() + finally: + dialog.grab_release() + dialog.destroy() + + assert reminder_file.read_text(encoding="utf-8") == before diff --git a/tests/test_welcome_mail.py b/tests/test_welcome_mail.py index 4da56f0..268effa 100644 --- a/tests/test_welcome_mail.py +++ b/tests/test_welcome_mail.py @@ -267,3 +267,29 @@ def test_archiving_failure_after_smtp_success_still_logs_the_sent_mail(tmp_path, assert event.event_type == "welcome_email_sent" assert event.data["archive_error"] assert "document" not in event.references + + +def test_a_template_broken_outside_ccma_stops_the_send(tmp_path): + repository, member = _new_member_repository(tmp_path) + # The templates are plain files in the store, so they can be edited (and broken) + # with any text editor -- the send has to notice instead of mailing the marker. + (repository.mail_templates_root / "willkommen.txt").write_text( + "Betreff: Willkommen\n\nHallo,\n{{#claims}}\n{{claim.title}}\n", + encoding="utf-8", + ) + + with pytest.raises(RepositoryError, match="wird nicht geschlossen"): + generate_and_send_welcome_mail( + repository, + member.member_id, + delivery_mode="local", + output_path=tmp_path / "Willkommen.eml", + sender_name="Verwaltung C3MA", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert not (tmp_path / "Willkommen.eml").exists() + archive = repository.members_root / member.member_id / "files" / "documents" / "Willkommen" + assert not archive.exists() + assert repository.get_events(member.member_id)[-1].event_type != "welcome_email_sent"