Skip to content

Commit

Permalink
mark decision notes as safe so jinja doesn't escape them
Browse files Browse the repository at this point in the history
  • Loading branch information
eviljeff committed Jul 8, 2024
1 parent 91e213b commit a5fd3d5
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
7 changes: 6 additions & 1 deletion src/olympia/abuse/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def setUp(self):
self.decision = CinderDecision.objects.create(
cinder_id='ab89',
action=DECISION_ACTIONS.AMO_APPROVE,
notes='extra notes',
notes="extra note's",
addon=addon,
)
self.cinder_job = CinderJob.objects.create(
Expand Down Expand Up @@ -167,6 +167,7 @@ def _test_reporter_appeal_approve_email(self, subject):
assert f'[ref:ab89/{self.abuse_report_auth.id}]' in mail.outbox[0].body
assert '"' not in mail.outbox[0].body
assert '<b>' not in mail.outbox[0].body
assert ''' not in mail.outbox[0].body
assert self.decision.notes in mail.outbox[0].body

def _check_owner_email(self, mail_item, subject, snippet):
Expand All @@ -177,6 +178,7 @@ def _check_owner_email(self, mail_item, subject, snippet):
assert '[ref:ab89]' in mail_item.body
assert '"' not in mail_item.body
assert '<b>' not in mail_item.body
assert ''' not in mail_item.body
assert self.decision.notes in mail_item.body

def _test_owner_takedown_email(self, subject, snippet):
Expand All @@ -198,6 +200,7 @@ def _test_owner_takedown_email(self, subject, snippet):
)
assert '"' not in mail_item.body
assert '<b>' not in mail_item.body
assert ''' not in mail_item.body
assert self.decision.notes in mail_item.body

def _test_owner_affirmation_email(self, subject):
Expand All @@ -206,6 +209,7 @@ def _test_owner_affirmation_email(self, subject):
assert 'right to appeal' not in mail_item.body
notes = f'{self.decision.notes}. ' if self.decision.notes else ''
assert f' was correct. {notes}Based on that determination' in (mail_item.body)
assert ''' not in mail_item.body
if isinstance(self.decision.target, Addon):
# Verify we used activity mail for Addon related target emails
log_token = ActivityLogToken.objects.get()
Expand All @@ -216,6 +220,7 @@ def _test_owner_restore_email(self, subject):
assert len(mail.outbox) == 1
self._check_owner_email(mail_item, subject, 'we have restored')
assert 'right to appeal' not in mail_item.body
assert ''' not in mail_item.body
assert self.decision.notes in mail_item.body

def _test_approve_appeal_or_override(CinderActionClass):
Expand Down
11 changes: 9 additions & 2 deletions src/olympia/abuse/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ def notify_owners(self, *, log_entry_id=None, extra_context=None):
reference_id = f'ref:{self.decision.get_reference_id()}'
context_dict = {
'is_third_party_initiated': self.decision.is_third_party_initiated,
'manual_reasoning_text': self.decision.notes or '',
# it's a plain-text email, and text from our own reveiewers, so we're safe
# - and we don't want ' or other charactors rendered with html escaping.
'manual_reasoning_text': mark_safe(self.decision.notes or ''),
# Auto-escaping is already disabled above as we're dealing with an
# email but the target name could have triggered lazy escaping when
# it was generated so it needs special treatment to avoid it.
Expand Down Expand Up @@ -182,7 +184,12 @@ def notify_reporters(self, *, reporter_abuse_reports, is_appeal=False):
'SITE_URL': settings.SITE_URL,
}
if self.decision.cinder_job.is_appeal:
context_dict['manual_reasoning_text'] = self.decision.notes or ''
# it's a plain-text email, and text from our own reveiewers, so
# we're safe - and we don't want ' or other charactors rendered with
# html escaping.
context_dict['manual_reasoning_text'] = mark_safe(
self.decision.notes or ''
)
if self.decision.can_be_appealed(
is_reporter=True, abuse_report=abuse_report
):
Expand Down

0 comments on commit a5fd3d5

Please sign in to comment.