Skip to content

Commit

Permalink
Don't escape content for plaintext version
Browse files Browse the repository at this point in the history
  • Loading branch information
TheByronHimes committed Jul 22, 2024
1 parent e1bc432 commit 1b2009f
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 17 deletions.
15 changes: 8 additions & 7 deletions src/ns/core/notifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,21 @@ def _construct_email(
message["Subject"] = notification.subject
message["From"] = self._config.from_address

# Escape values exposed to the email in case they've been maliciously crafted
payload_as_dict = {}
for k, v in notification.model_dump().items():
if isinstance(v, list):
payload_as_dict[k] = ", ".join([html.escape(_) for _ in v])
else:
payload_as_dict[k] = html.escape(v)
payload_as_dict = {**notification.model_dump()}

# create plaintext html with template
plaintext_email = self._build_email_subtype(
template_type=EmailTemplateType.PLAINTEXT, values_dict=payload_as_dict
)
message.set_content(plaintext_email)

# Escape values exposed to the email in case they've been maliciously crafted
for k, v in payload_as_dict.items():
if isinstance(v, list):
payload_as_dict[k] = ", ".join([html.escape(_) for _ in v])
else:
payload_as_dict[k] = html.escape(v)

# create html version of email, replacing variables of $var format
html_email = self._build_email_subtype(
template_type=EmailTemplateType.HTML, values_dict=payload_as_dict
Expand Down
23 changes: 13 additions & 10 deletions tests/test_basic.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,21 +241,23 @@ async def test_idempotence_and_transmission(joint_fixture: JointFixture):


async def test_html_escaping(joint_fixture: JointFixture):
"""Make sure dirty args are escaped properly."""
"""Make sure dirty args are escaped properly in the HTML email and unchanged in the
plaintext email.
"""
# Cast notifier type
joint_fixture.notifier = cast(Notifier, joint_fixture.notifier)

injected_body = "<script>alert('danger');</script>"
injected_name = f"<p>{sample_notification["recipient_name"]}</p>"
original_body = "<script>alert('danger');</script>"
original_name = f"<p>{sample_notification["recipient_name"]}</p>"
injected_notification = {**sample_notification}
injected_notification["plaintext_body"] = injected_body
injected_notification["recipient_name"] = injected_name
injected_notification["plaintext_body"] = original_body
injected_notification["recipient_name"] = original_name

# Precompute the escaped values and make sure they're not the same as the original
escaped_name = html.escape(injected_name)
escaped_body = html.escape(injected_body)
assert injected_name != escaped_name
assert injected_body != escaped_body
escaped_name = html.escape(original_name)
escaped_body = html.escape(original_body)
assert original_name != escaped_name
assert original_body != escaped_body

notification = make_notification(injected_notification)

Expand All @@ -267,7 +269,8 @@ async def test_html_escaping(joint_fixture: JointFixture):

plaintext_content = plaintext_body.get_content() # type: ignore
expected_plaintext = (
f"Dear {escaped_name},\n\n{escaped_body}\n" + "\nWarm regards,\n\nThe GHGA Team"
f"Dear {original_name},\n\n{original_body}\n"
+ "\nWarm regards,\n\nThe GHGA Team"
)
assert plaintext_content.strip() == expected_plaintext

Expand Down

0 comments on commit 1b2009f

Please sign in to comment.