From 91f3e6a5576782b175bd61da6345e090c341b164 Mon Sep 17 00:00:00 2001 From: Kenneth Kehl <@kkehl@flexion.us> Date: Tue, 1 Apr 2025 08:00:33 -0700 Subject: [PATCH] fix static scan warnings --- app/formatters.py | 2 +- app/main/forms.py | 16 ++++++++++------ app/main/views/invites.py | 4 ++-- app/main/views/jobs.py | 10 +++++----- app/main/views/send.py | 5 ++++- app/main/views/sign_in.py | 7 ++----- app/main/views/sign_out.py | 2 +- app/main/views/templates.py | 8 +++----- app/utils/govuk_frontend_jinja/templates.py | 7 ++++--- 9 files changed, 32 insertions(+), 29 deletions(-) diff --git a/app/formatters.py b/app/formatters.py index c427c2a9a..a739fe437 100644 --- a/app/formatters.py +++ b/app/formatters.py @@ -344,7 +344,7 @@ def nl2br(value): html="escape", ) ).then(utils_nl2br) - ) + ) # nosec return "" diff --git a/app/main/forms.py b/app/main/forms.py index 90a4409fc..2695fcdc4 100644 --- a/app/main/forms.py +++ b/app/main/forms.py @@ -257,7 +257,7 @@ def govuk_text_input_field_widget( return Markup( render_template("components/components/input/template.njk", params=params) - ) + ) # nosec class GovukTextInputField(StringField): @@ -690,7 +690,9 @@ def govuk_checkbox_field_widget(self, field, param_extensions=None, **kwargs): if param_extensions: merge_jsonlike(params, param_extensions) - return Markup(render_template("forms/fields/checkboxes/macro.njk", params=params)) + return Markup( + render_template("forms/fields/checkboxes/macro.njk", params=params) + ) # nosec def govuk_checkboxes_field_widget( @@ -704,7 +706,7 @@ def govuk_checkboxes_field_widget( f' data-field-label="{field_label}">' f" {checkboxes_string}" f"" - ) + ) # nosec return result @@ -757,12 +759,14 @@ def govuk_checkboxes_field_widget( return _wrap_in_collapsible( self.field_label, - Markup(render_template("forms/fields/checkboxes/macro.njk", params=params)), + Markup( + render_template("forms/fields/checkboxes/macro.njk", params=params) + ), # nosec ) else: return Markup( render_template("forms/fields/checkboxes/macro.njk", params=params) - ) + ) # nosec def govuk_radios_field_widget(self, field, param_extensions=None, **kwargs): @@ -805,7 +809,7 @@ def govuk_radios_field_widget(self, field, param_extensions=None, **kwargs): return Markup( render_template("components/components/radios/template.njk", params=params) - ) + ) # nosec class GovukCheckboxField(BooleanField): diff --git a/app/main/views/invites.py b/app/main/views/invites.py index 07f6b3ac8..420630535 100644 --- a/app/main/views/invites.py +++ b/app/main/views/invites.py @@ -23,7 +23,7 @@ def accept_invite(token): Sign out and click the link again to accept this invite. """ - ) + ) # nosec flash(message=message) @@ -98,7 +98,7 @@ def accept_org_invite(token): """.format( current_user.email_address, url_for("main.sign_out") ) - ) + ) # nosec flash(message=message) diff --git a/app/main/views/jobs.py b/app/main/views/jobs.py index 164ab737f..86ce0bce6 100644 --- a/app/main/views/jobs.py +++ b/app/main/views/jobs.py @@ -351,7 +351,7 @@ def _get_job_counts(job): Markup( f"""total {"text message" if job_type == "sms" else job_type}s""" - ), + ), # nosec "", job.notification_count, ], @@ -359,7 +359,7 @@ def _get_job_counts(job): Markup( f"""pending {message_count_noun(job.notifications_sending, job_type)}""" - ), + ), # nosec "pending", job.notifications_sending, ], @@ -367,7 +367,7 @@ def _get_job_counts(job): Markup( f"""delivered {message_count_noun(job.notifications_delivered, job_type)}""" - ), + ), # nosec "delivered", job.notifications_delivered, ], @@ -375,7 +375,7 @@ def _get_job_counts(job): Markup( f"""failed {message_count_noun(job.notifications_failed, job_type)}""" - ), + ), # nosec "failed", job.notifications_failed, ], @@ -465,4 +465,4 @@ def get_preview_of_content(notification): notification["personalisation"], redact_missing_personalisation=True, ).subject - ) + ) # nosec diff --git a/app/main/views/send.py b/app/main/views/send.py index 54f5eb098..e3b3ed0a9 100644 --- a/app/main/views/send.py +++ b/app/main/views/send.py @@ -4,6 +4,7 @@ import uuid from string import ascii_uppercase from zipfile import BadZipFile +import bleach from flask import ( abort, current_app, @@ -170,7 +171,9 @@ def send_messages(service_id, template_id): error_message = '' error_message = f"{error_message}{first_field_errors[0]}" error_message = f"{error_message}" - error_message = Markup(error_message) + # use 'nosec' because we applied bleach.clean to strip dangerous tags + error_message = bleach.clean(error_message) + error_message = Markup(error_message) # nosec flash(error_message) column_headings = get_spreadsheet_column_headings_from_template(template) diff --git a/app/main/views/sign_in.py b/app/main/views/sign_in.py index 1cb163691..24c49d7bd 100644 --- a/app/main/views/sign_in.py +++ b/app/main/views/sign_in.py @@ -62,7 +62,7 @@ def _get_access_token(code): # pragma: no cover code_param = f"code={code}" url = f"{base_url}{cli_assert}&{cli_assert_type}&{code_param}&grant_type=authorization_code" headers = {"Authorization": "Bearer %s" % token} - response = requests.post(url, headers=headers) + response = requests.post(url, headers=headers, timeout=30) response_json = response.json() id_token = get_id_token(response_json) @@ -88,10 +88,7 @@ def _get_access_token(code): # pragma: no cover def _get_user_email_and_uuid(access_token): # pragma: no cover headers = {"Authorization": "Bearer %s" % access_token} user_info_url = os.getenv("LOGIN_DOT_GOV_USER_INFO_URL") - user_attributes = requests.get( - user_info_url, - headers=headers, - ) + user_attributes = requests.get(user_info_url, headers=headers, timeout=30) user_email = user_attributes.json()["email"] user_uuid = user_attributes.json()["sub"] return user_email, user_uuid diff --git a/app/main/views/sign_out.py b/app/main/views/sign_out.py index 82ba5497e..8055e5f50 100644 --- a/app/main/views/sign_out.py +++ b/app/main/views/sign_out.py @@ -17,7 +17,7 @@ def _sign_out_at_login_dot_gov(): url = f"{base_url}{client_id}&{post_logout_redirect_uri}" current_app.logger.info(f"url={url}") - response = requests.post(url) + response = requests.post(url, timeout=30) # response = requests.post(url) current_app.logger.info(f"login.gov response: {response.text}") diff --git a/app/main/views/templates.py b/app/main/views/templates.py index 83b4e9b25..4ac898cb4 100644 --- a/app/main/views/templates.py +++ b/app/main/views/templates.py @@ -698,9 +698,7 @@ def _get_content_count_error_and_message_for_template(template): # Check for blocked characters if contains_blocked_characters(template.content): warning = f"{s1}{s2}" - return False, Markup( - warning - ) # 🚨 ONLY show the warning, hiding "Will be charged..." + return False, Markup(warning) # nosec # If message is too long, return the length error if template.is_message_too_long(): @@ -715,11 +713,11 @@ def _get_content_count_error_and_message_for_template(template): return False, Markup( f"Will be charged as {message_count(template.fragment_count, template.template_type)} " f"(not including personalization)." - ) + ) # nosec return False, Markup( f"Will be charged as {message_count(template.fragment_count, template.template_type)}." - ) + ) # nosec @main.route( diff --git a/app/utils/govuk_frontend_jinja/templates.py b/app/utils/govuk_frontend_jinja/templates.py index 1295fd616..e0101aa5d 100644 --- a/app/utils/govuk_frontend_jinja/templates.py +++ b/app/utils/govuk_frontend_jinja/templates.py @@ -118,9 +118,10 @@ def indent_njk(s, width=4, first=False, blank=False, indentfirst=None): indention = " " * width newline = "\n" + # we know 'indention' and 'newline' are safe so use nosec to bypass static scan warning if isinstance(s, Markup): - indention = Markup(indention) - newline = Markup(newline) + indention = Markup(indention) # nosec + newline = Markup(newline) # nosec s += newline # this quirk is necessary for splitlines method @@ -190,7 +191,7 @@ class NunjucksUndefined(jinja2.runtime.Undefined): def items(self): return self - # Allow escaping with Markup. This is required when + # Allow escaping. This is required when # autoescape is enabled. Debugging this issue was # annoying; the error messages were not clear as to # the cause of the issue (see upstream pull request