fix one more place where senders weren't sanitised

make sure everything is using the `nl2br` formatter that properly wraps
it in markdown to keep everything sanitised nicely. Also write a couple
of tests

app/main/views/send.py

@@ -31,6 +31,7 @@ from xlrd.xldate import XLDateError
 from app import (
     current_service,
     job_api_client,
+    nl2br,
     notification_api_client,
     service_api_client,
 )
@@ -272,7 +273,7 @@ def get_sender_context(sender_details, template_type):
         if context['default_id'] == context.get('receives_text_message', None):
             context['default_and_receives'] = context['default_id']
 
-    context['value_and_label'] = [(sender['id'], sender[sender_format]) for sender in sender_details]
+    context['value_and_label'] = [(sender['id'], nl2br(sender[sender_format])) for sender in sender_details]
     return context
 
 

app/main/views/templates.py

@@ -4,14 +4,13 @@ from functools import partial
 from dateutil.parser import parse
 from flask import abort, flash, redirect, render_template, request, url_for
 from flask_login import current_user
-from markupsafe import Markup
 from notifications_python_client.errors import HTTPError
 from notifications_utils import LETTER_MAX_PAGE_COUNT
-from notifications_utils.formatters import nl2br
 from notifications_utils.pdf import is_letter_too_long
 
 from app import (
     current_service,
+    nl2br,
     service_api_client,
     template_folder_api_client,
     template_statistics_client,
@@ -844,7 +843,7 @@ def get_template_sender_form_dict(service_id, template):
     if not service_senders:
         context['no_senders'] = True
 
-    context['value_and_label'] = [(sender['id'], Markup(nl2br(sender[sender_format]))) for sender in service_senders]
+    context['value_and_label'] = [(sender['id'], nl2br(sender[sender_format])) for sender in service_senders]
     context['value_and_label'].insert(0, ('', 'Blank'))  # Add blank option to start of list
 
     context['current_choice'] = template['service_letter_contact'] if template['service_letter_contact'] else ''

app/templates/views/service-settings.html

@@ -136,7 +136,7 @@
         {% call settings_row(if_has_permission='sms') %}
           {{ text_field('Text message senders') }}
           {% call field(status='default' if current_service.default_sms_sender == "None" else '') %}
-            {{ current_service.default_sms_sender | string | nl2br if current_service.default_sms_sender else 'None'}}
+            {{ current_service.default_sms_sender | nl2br if current_service.default_sms_sender else 'None'}}
             {% if current_service.count_sms_senders > 1 %}
               <div class="hint">
                 {{ '…and %d more' | format(current_service.count_sms_senders - 1) }}

tests/app/main/views/test_service_settings.py

@@ -1977,7 +1977,7 @@ def test_api_ids_dont_show_on_option_pages_with_a_single_sender(
             'Blank Make default',
             '1 Example Street (default) Change 1234',
             '2 Example Street Change 5678',
-            '3 Example Street Change 9457',
+            'foo<bar>baz Change 9457',
         ],
     ), (
         'main.service_sms_senders',
@@ -2756,6 +2756,19 @@ def test_default_box_shows_on_non_default_sender_details_while_editing(
         )
 
 
+def test_sender_details_are_escaped(client_request, mocker, fake_uuid):
+    letter_contact_block = create_letter_contact_block(contact_block='foo\n\n<br>\n\nbar')
+    mocker.patch('app.service_api_client.get_letter_contacts', return_value=[letter_contact_block])
+
+    page = client_request.get(
+        'main.service_letter_contact_details',
+        service_id=SERVICE_ONE_ID,
+    )
+
+    # get the second row (first is the default Blank sender)
+    assert 'foo<br>bar' in normalize_spaces(page.select('.user-list-item')[1].text)
+
+
 @pytest.mark.parametrize('sms_sender, expected_link_text, partial_href', [
     (
         create_sms_sender(is_default=False),

tests/app/main/views/test_templates.py

@@ -2207,7 +2207,6 @@ def test_add_sender_link_only_appears_on_services_with_no_senders(
     mocker,
     contact_block_data,
     mock_get_service_letter_template,
-    no_letter_contact_blocks
 ):
     mocker.patch('app.service_api_client.get_letter_contacts', return_value=contact_block_data)
     page = client_request.get(
@@ -2221,3 +2220,24 @@ def test_add_sender_link_only_appears_on_services_with_no_senders(
         service_id=SERVICE_ONE_ID,
         from_template=fake_uuid,
     )
+
+
+def test_set_template_sender_escapes_letter_contact_block_names(
+    client_request,
+    fake_uuid,
+    mocker,
+    mock_get_service_letter_template,
+):
+    letter_contact_block = create_letter_contact_block(contact_block='foo\n\n<script>\n\nbar')
+    mocker.patch('app.service_api_client.get_letter_contacts', return_value=[letter_contact_block])
+    page = client_request.get(
+        'main.set_template_sender',
+        service_id=SERVICE_ONE_ID,
+        template_id=fake_uuid,
+    )
+
+    # use decode_contents, which returns the raw html, rather than text, which sanitises it and makes
+    # testing confusing
+    radio_text = page.select_one('.column-three-quarters label[for="sender-1"]').decode_contents()
+    assert "&lt;script&gt;" in radio_text
+    assert "<script>" not in radio_text

tests/conftest.py

@@ -3648,7 +3648,7 @@ def create_multiple_letter_contact_blocks(service_id='abcd'):
         }, {
             'id': '9457',
             'service_id': service_id,
-            'contact_block': '3 Example Street',
+            'contact_block': 'foo\n\n<bar>\n\nbaz',
             'is_default': False,
             'created_at': datetime.utcnow(),
             'updated_at': None