DS-943 Fixing high severity sonarcloud reported vulnerabilities (#1144)

# Task Branch Pull Request

**<https://nhsd-jira.digital.nhs.uk/browse/DS-943>**

## Description of Changes

Fixing issues reported by sonarcloud for DoS-Integration repo

## Type of change

Delete not appropriate

- Bug fix (non-breaking change which fixes sonarcloud vulnerabilities
issues)

## Development Checklist

- [x] I have performed a self-review of my own code
- [x] Tests have added that prove my fix is effective or that my feature
works (Integration tests)
- [x] I have updated Dependabot to include my changes (if applicable)

## Code Reviewer Checklist

- [x] I can confirm the changes have been tested or approved by a tester

Author: ajmu1

application/common/dos.py

@@ -254,7 +254,7 @@ def db_rows_to_spec_open_times(db_rows: Iterable[dict]) -> list[SpecifiedOpening
     for date, rows in groupby(date_sorted_rows, lambda row: row["date"]):
         is_open = True
         open_periods = []
-        for row in list(rows):
+        for row in rows:
             if row["isclosed"] is True:
                 is_open = False
             else:

application/common/errors.py

@@ -4,3 +4,7 @@ class ValidationError(Exception):
 
 class DynamoDBError(Exception):
     """Exception raised for all DynamoDB errors."""
+
+
+class SecretsManagerError(Exception):
+    """Exception raised for AWS Secrets Manager errors."""

application/common/nhs.py

@@ -165,7 +165,7 @@ def _get_specified_opening_times(self: Self) -> list[SpecifiedOpeningTime]:
             date = datetime.strptime(date_str, "%b  %d  %Y").date()
             is_open = True
 
-            for item in list(op_dict_list):
+            for item in op_dict_list:
                 if item["IsOpen"]:
                     open_periods.append(OpenPeriod.from_string_times(item["OpeningTime"], item["ClosingTime"]))
                 else:

application/common/secretsmanager.py

@@ -4,6 +4,8 @@
 from boto3 import client
 from botocore.exceptions import ClientError
 
+from .errors import SecretsManagerError
+
 logger = Logger()
 
 secrets_manager = client(service_name="secretsmanager")
@@ -16,7 +18,7 @@ def get_secret(secret_name: str) -> dict[str, str]:
         secret_name (str): Secret name to get
 
     Raises:
-        e: ClientError caused by secrets manager
+        SecretsManagerError: When unable to retrieve secret from AWS Secrets Manager
 
     Returns:
         Dict[str, str]: Secrets as a dictionary
@@ -25,6 +27,6 @@ def get_secret(secret_name: str) -> dict[str, str]:
         secret_value_response = secrets_manager.get_secret_value(SecretId=secret_name)
     except ClientError as err:
         msg = f"Failed getting secret '{secret_name}' from secrets manager"
-        raise Exception(msg) from err  # noqa: TRY002
+        raise SecretsManagerError(msg) from err
     secrets_json_str = secret_value_response["SecretString"]
     return loads(secrets_json_str)

application/common/tests/test_secretsmanager.py

@@ -4,6 +4,8 @@
 import pytest
 from moto import mock_aws
 
+from application.common.errors import SecretsManagerError
+
 FILE_PATH = "application.common.secretsmanager"
 
 
@@ -26,5 +28,5 @@ def test_get_secret() -> None:
 def test_get_secret_resource_not_found() -> None:
     from application.common.secretsmanager import get_secret
 
-    with pytest.raises(Exception, match="Failed getting secret 'fake_secret_name' from secrets manager"):
+    with pytest.raises(SecretsManagerError, match="Failed getting secret 'fake_secret_name' from secrets manager"):
         get_secret("fake_secret_name")

application/event_replay/event_replay.py

@@ -8,10 +8,19 @@
 from aws_lambda_powertools.utilities.typing import LambdaContext
 from boto3 import client
 from boto3.dynamodb.types import TypeDeserializer
+from botocore.config import Config
 from simplejson import dumps
 
 from common.middlewares import unhandled_exception_logging
 
+# Configure boto3 client with explicit timeout to prevent hanging in Lambda
+# Timeouts tuned for typical DynamoDB/SQS operations while preventing indefinite hangs
+boto_config = Config(connect_timeout=10, read_timeout=15)
+
+# Create clients at module level for connection reuse across Lambda invocations
+dynamodb_client = client("dynamodb", config=boto_config)
+sqs_client = client("sqs", config=boto_config)
+
 tracer = Tracer()
 logger = Logger()
 
@@ -75,7 +84,7 @@ def get_change_event(odscode: str, sequence_number: Decimal) -> dict[str, Any]:
     Returns:
         dict[str, Any]: The change event
     """
-    response = client("dynamodb").query(
+    response = dynamodb_client.query(
         TableName=getenv("CHANGE_EVENTS_TABLE_NAME"),
         IndexName="gsi_ods_sequence",
         ProjectionExpression="Event",
@@ -112,11 +121,10 @@ def send_change_event(change_event: dict[str, Any], odscode: str, sequence_numbe
         sequence_number (int): The sequence number of the change event
         correlation_id (str): The correlation id of the event replay
     """
-    sqs = client("sqs")
     queue_url = getenv("CHANGE_EVENT_SQS_URL")
     logger.info("Sending change event to SQS", queue_url=queue_url)
     change_event_str = dumps(change_event)
-    response = sqs.send_message(
+    response = sqs_client.send_message(
         QueueUrl=queue_url,
         MessageBody=change_event_str,
         MessageGroupId=odscode,

application/event_replay/tests/test_event_replay.py

@@ -102,21 +102,20 @@ def test_build_correlation_id(mock_time_ns: MagicMock) -> None:
     assert response == f"{time}-local-replayed-event"
 
 
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.dynamodb_client")
 def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]) -> None:
     # Arrange
     table_name = "my-table"
     environ["CHANGE_EVENTS_TABLE_NAME"] = table_name
     environ["AWS_REGION"] = "eu-west-1"
     serializer = TypeSerializer()
 
-    mock_client.return_value.query.return_value = {"Items": [{"Event": serializer.serialize(change_event)}]}
+    mock_client.query.return_value = {"Items": [{"Event": serializer.serialize(change_event)}]}
     # Act
     response = get_change_event(event["odscode"], Decimal(event["sequence_number"]))
     # Assert
     assert response == change_event
-    mock_client.assert_called_with("dynamodb")
-    mock_client().query.assert_called_with(
+    mock_client.query.assert_called_with(
         TableName=table_name,
         IndexName="gsi_ods_sequence",
         ProjectionExpression="Event",
@@ -130,21 +129,20 @@ def test_get_change_event(mock_client: MagicMock, change_event: dict[str, str],
     del environ["AWS_REGION"]
 
 
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.dynamodb_client")
 def test_get_change_event_no_change_event_in_dynamodb(
     mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]
 ) -> None:
     # Arrange
     table_name = "my-table"
     environ["CHANGE_EVENTS_TABLE_NAME"] = table_name
     environ["AWS_REGION"] = "eu-west-1"
-    mock_client.return_value.query.return_value = {"Items": []}
+    mock_client.query.return_value = {"Items": []}
     # Act
     with pytest.raises(ValueError, match="No change event found for ods code FXXX1 and sequence number 1"):
         get_change_event(event["odscode"], Decimal(event["sequence_number"]))
     # Assert
-    mock_client.assert_called_with("dynamodb")
-    mock_client().query.assert_called_with(
+    mock_client.query.assert_called_with(
         TableName=table_name,
         IndexName="gsi_ods_sequence",
         ProjectionExpression="Event",
@@ -158,16 +156,15 @@ def test_get_change_event_no_change_event_in_dynamodb(
     del environ["AWS_REGION"]
 
 
-@patch(f"{FILE_PATH}.client")
+@patch(f"{FILE_PATH}.sqs_client")
 def test_send_change_event(mock_client: MagicMock, change_event: dict[str, str], event: dict[str, str]) -> None:
     # Arrange
     correlation_id = "CORRELATION_ID"
     environ["CHANGE_EVENT_SQS_URL"] = queue_url = "https://sqs.eu-west-1.amazonaws.com/123456789/my-queue"
     # Act
     send_change_event(change_event, event["odscode"], int(event["sequence_number"]), correlation_id)
     # Assert
-    mock_client.assert_called_with("sqs")
-    mock_client().send_message.assert_called_with(
+    mock_client.send_message.assert_called_with(
         QueueUrl=queue_url,
         MessageBody=dumps(change_event),
         MessageGroupId=event["odscode"],

application/ingest_change_event/ingest_change_event.py

@@ -7,16 +7,22 @@
 from aws_lambda_powertools.utilities.data_classes import SQSEvent, event_source
 from aws_lambda_powertools.utilities.typing.lambda_context import LambdaContext
 from boto3 import client
+from botocore.config import Config
+from botocore.exceptions import ClientError
 
 from .change_event_validation import validate_change_event
 from common.dynamodb import add_change_event_to_dynamodb, get_latest_sequence_id_for_a_given_odscode_from_dynamodb
 from common.middlewares import redact_staff_key_from_event, unhandled_exception_logging
 from common.types import HoldingQueueChangeEventItem
 from common.utilities import extract_body, get_sequence_number
 
+# Configure boto3 client with explicit timeout to prevent hanging in Lambda
+boto_config = Config(connect_timeout=10, read_timeout=15)
+
 logger = Logger()
 tracer = Tracer()
-sqs = client("sqs")
+# Create SQS client at module level for connection reuse across Lambda invocations
+sqs_client = client("sqs", config=boto_config)
 
 
 @redact_staff_key_from_event()
@@ -82,8 +88,16 @@ def lambda_handler(event: SQSEvent, context: LambdaContext) -> None:  # noqa: AR
         correlation_id=logger.get_correlation_id(),
     )
     logger.debug("Change event validated", holding_queue_change_event_item=holding_queue_change_event_item)
-    sqs.send_message(
-        QueueUrl=getenv("HOLDING_QUEUE_URL"),
-        MessageBody=dumps(holding_queue_change_event_item),
-        MessageGroupId=ods_code,
-    )
+    try:
+        sqs_client.send_message(
+            QueueUrl=getenv("HOLDING_QUEUE_URL"),
+            MessageBody=dumps(holding_queue_change_event_item),
+            MessageGroupId=ods_code,
+        )
+    except ClientError as err:
+        logger.exception(
+            "Failed to send message to holding queue",
+            error_code=err.response["Error"]["Code"],
+            queue_url=getenv("HOLDING_QUEUE_URL"),
+        )
+        raise

application/ingest_change_event/tests/test_ingest_change_event.py

@@ -12,7 +12,7 @@
 FILE_PATH = "application.ingest_change_event.ingest_change_event"
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -72,7 +72,7 @@ def test_lambda_handler(
     del environ["HOLDING_QUEUE_URL"]
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -130,7 +130,7 @@ def test_lambda_handler_with_sensitive_staff_key(
 
 
 @patch.object(Logger, "error")
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -183,7 +183,7 @@ def test_lambda_handler_no_sequence_number(
 
 
 @patch.object(Logger, "error")
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")
@@ -239,7 +239,7 @@ def test_lambda_handler_less_than_latest_sequence_number(
     del environ["HOLDING_QUEUE_URL"]
 
 
-@patch(f"{FILE_PATH}.sqs")
+@patch(f"{FILE_PATH}.sqs_client")
 @patch(f"{FILE_PATH}.HoldingQueueChangeEventItem")
 @patch(f"{FILE_PATH}.add_change_event_to_dynamodb")
 @patch(f"{FILE_PATH}.get_latest_sequence_id_for_a_given_odscode_from_dynamodb")

application/service_matcher/requirements.txt

@@ -1,3 +1,2 @@
 aws-lambda-powertools[tracer] ~= 3.20.0
 psycopg[binary]
-pytz