Lcs 867 review sonarqube vulnerabilities in GitHub pipelines (#459)

# What is the change?

This PR implements several improvements based on SonarQube
recommendations, including security fixes, code reliability
enhancements, and general refactoring for better reusability.

Although the original scope was focused on security, it made sense to
address related quality improvements at the same time.

Please note that this will not resolve all the issues; its more a first
pass at fixing them. More PRs will follow.


<!-- Describe the intended changes. -->

# Why are we making this change?

To ensure the repository can remain public while adhering to SonarQube
best practices, particularly around security, maintainability, and code
quality.

<!-- Why is this change required? What problem does it solve? -->

Author: mrlockstar

.github/actions/lint-terraform/action.yaml

@@ -13,8 +13,9 @@ runs:
         check_only=true scripts/githooks/check-terraform-format.sh
     - name: "Validate Terraform"
       shell: bash
+      env:
+        STACKS: ${{ inputs.root-modules }}
       run: |
-        stacks=${{ inputs.root-modules }}
-        for dir in $(find infrastructure/environments -maxdepth 1 -mindepth 1 -type d; echo ${stacks//,/$'\n'}); do
+        for dir in $(find infrastructure/environments -maxdepth 1 -mindepth 1 -type d; echo ${STACKS//,/$'\n'}); do
           dir=$dir make terraform-validate
         done

infrastructure/bootstrap/core.bicep

@@ -6,10 +6,6 @@ param miPrincipalId string
 @minLength(1)
 param miName string
 
-param userGroupPrincipalID string
-
-param userGroupName string
-
 // See: https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
 var roleID = {
   contributor: 'b24988ac-6180-42a0-ab88-20f7382dd24c'

infrastructure/bootstrap/hub.bicep

@@ -44,6 +44,8 @@ var privateEndpointSubnetName = 'sn-hub-${hubType}-${regionShortName}-private-en
 var storageAccountName = 'sa${appShortName}${hubType}${regionShortName}state'
 var computeGalleryName = '${appShortName}_hub_compute_gallery'
 
+var roleDefinitions = 'Microsoft.Authorization/roleDefinitions'
+
 var miADOtoAZname = 'mi-${appShortName}-${hubType}-adotoaz-${regionShortName}'
 var miGHtoADOname = 'mi-${appShortName}-${hubType}-ghtoado-${regionShortName}'
 
@@ -163,7 +165,7 @@ module managedIdentiyADOtoAZ 'modules/managedIdentity.bicep' = {
 resource networkContributorAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
   name: guid(subscription().subscriptionId, hubType, 'networkContributor')
   properties: {
-    roleDefinitionId: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleID.networkContributor)
+    roleDefinitionId: subscriptionResourceId(roleDefinitions, roleID.networkContributor)
     principalId: managedIdentiyADOtoAZ.outputs.miPrincipalID
     description: '${miADOtoAZname} Network Contributor access to subscription'
   }
@@ -186,7 +188,7 @@ resource userAccessAdministratorAssignment 'Microsoft.Authorization/roleAssignme
 resource CDNContributorAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
   name: guid(subscription().subscriptionId, hubType, 'CDNContributor')
   properties: {
-    roleDefinitionId: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleID.CDNContributor)
+    roleDefinitionId: subscriptionResourceId(roleDefinitions, roleID.CDNContributor)
     principalId: managedIdentiyADOtoAZ.outputs.miPrincipalID
     description: '${miADOtoAZname} CDN Contributor access to subscription'
   }
@@ -196,7 +198,7 @@ resource CDNContributorAssignment 'Microsoft.Authorization/roleAssignments@2022-
 resource TerraformContributorAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
   name: guid(subscription().subscriptionId, hubType, 'TerraformContributor')
   properties: {
-    roleDefinitionId: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleID.contributor)
+    roleDefinitionId: subscriptionResourceId(roleDefinitions, roleID.contributor)
     principalId: managedIdentiyADOtoAZ.outputs.miPrincipalID
     description: '${miADOtoAZname} Terraform Contributor access to subscription'
   }
@@ -206,7 +208,7 @@ resource TerraformContributorAssignment 'Microsoft.Authorization/roleAssignments
 resource StorageAccountBlobContributorAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
   name: guid(subscription().subscriptionId, hubType, 'StorageAccountBlobContributorAssignment')
   properties: {
-    roleDefinitionId: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleID.storageBlobDataContributor)
+    roleDefinitionId: subscriptionResourceId(roleDefinitions, roleID.storageBlobDataContributor)
     principalId: managedIdentiyADOtoAZ.outputs.miPrincipalID
     description: '${miADOtoAZname} Storage Account Blob Contributor access to subscription'
   }
@@ -230,7 +232,7 @@ module managedIdentiyGHtoADO 'modules/managedIdentity.bicep' = {
 resource readerAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
   name: guid(subscription().subscriptionId, hubType, 'reader')
   properties: {
-    roleDefinitionId: subscriptionResourceId('Microsoft.Authorization/roleDefinitions', roleID.reader)
+    roleDefinitionId: subscriptionResourceId(roleDefinitions, roleID.reader)
     principalId: managedIdentiyGHtoADO.outputs.miPrincipalID
     description: '${miGHtoADOname} Reader access to subscription'
   }

infrastructure/bootstrap/modules/computeGallery.bicep

@@ -10,7 +10,7 @@ resource computeGallery 'Microsoft.Compute/galleries@2023-07-03' = {
   name: galleryName
   location: location
   properties: {
-    description: ''
+    description: 'Compute Gallery for sharing images across subscriptions and regions'
     softDeletePolicy: {
       isSoftDeleteEnabled: false
     }

infrastructure/bootstrap/modules/dns.bicep

@@ -19,9 +19,9 @@ resource privateDNSZone 'Microsoft.Network/privateDnsZones@2024-06-01' = {
 }
 
 resource vnetLink 'Microsoft.Network/privateDnsZones/virtualNetworkLinks@2024-06-01' = {
+  location: 'global'
   name: '${last(split(vnetId, '/'))}-link'
   parent: privateDNSZone
-  location: 'global'
   properties: {
     virtualNetwork: {
       id: vnetId

infrastructure/bootstrap/modules/privateEndpoint-spoke.bicep

@@ -16,14 +16,14 @@ var groupID = {
 
 // Retrieve the existing vnet resource group
 resource vnetRG 'Microsoft.Resources/resourceGroups@2024-11-01' existing = {
-  name: RGName
   scope: subscription()
+  name: RGName
 }
 
 // Retrieve the existing vnet
 resource vnet 'Microsoft.Network/virtualNetworks@2024-01-01' existing = {
-  name: vnetName
   scope: vnetRG
+  name: vnetName
 }
 
 // Retrieve the existing Subnet within the vnet

infrastructure/bootstrap/modules/privateEndpoint.bicep

@@ -17,14 +17,14 @@ var groupID = {
 
 // Retrieve the existing vnet resource group
 resource vnetRG 'Microsoft.Resources/resourceGroups@2024-11-01' existing = {
-  name: RGName
   scope: subscription()
+  name: RGName
 }
 
 // Retrieve the existing vnet
 resource vnet 'Microsoft.Network/virtualNetworks@2024-01-01' existing = {
-  name: vnetName
   scope: vnetRG
+  name: vnetName
 }
 
 resource privateEndpointSubnet 'Microsoft.Network/virtualNetworks/subnets@2025-01-01' = {

infrastructure/bootstrap/modules/storage.bicep

@@ -61,11 +61,6 @@ resource blobContainer 'Microsoft.Storage/storageAccounts/blobServices/container
   }
 }
 
-// See: https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
-var roleID = {
-  blobContributor: 'ba92f5b4-2d11-453d-a403-e96b0029c9fe'
-}
-
 // Define role assignments array
 var roleAssignments = [
   {
@@ -75,6 +70,11 @@ var roleAssignments = [
   }
 ]
 
+// See: https://learn.microsoft.com/en-us/azure/role-based-access-control/built-in-roles
+var roleID = {
+  blobContributor: 'ba92f5b4-2d11-453d-a403-e96b0029c9fe'
+}
+
 // Let the managed identity edit the terraform state
 resource blobContributorAssignment 'Microsoft.Authorization/roleAssignments@2022-04-01' = {
   name: guid(subscription().subscriptionId, miPrincipalID, 'blobContributor')

lung_cancer_screening/questions/forms/agree_terms_of_use_form.py

@@ -8,15 +8,16 @@
 class TermsOfUseForm(forms.ModelForm):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
+        terms_of_use = "Agree to the terms of use to continue"
         self.fields["value"] = MultipleChoiceField(
             choices=[(True, 'I agree')],
             widget=forms.CheckboxSelectMultiple,
             label="Accept terms of use",
             label_classes="nhsuk-u-visually-hidden",
             error_messages={
-                "required": "Agree to the terms of use to continue",
-                "invalid_choice": "Agree to the terms of use to continue",
-                "invalid_list": "Agree to the terms of use to continue"
+                "required": terms_of_use,
+                "invalid_choice": terms_of_use,
+                "invalid_list": terms_of_use
             }
         )
     class Meta:

scripts/bash/container_app_smoke_test.sh

@@ -8,9 +8,9 @@ DNS_ZONE_NAME=$3
 PR_NUMBER=${4:-}
 USE_APEX_DOMAIN=${5:-false}
 
-if [ -z "$PR_NUMBER" ]; then
+if [[ -z "$PR_NUMBER" ]]; then
     # Permanent environments
-    if [ "$USE_APEX_DOMAIN" = "true" ]; then
+    if [[ "$USE_APEX_DOMAIN" = "true" ]]; then
         # For production with apex domain
         ENDPOINT="https://${DNS_ZONE_NAME}/sha"
     else
@@ -33,7 +33,7 @@ while true; do
   # It should fail initially until front door presents the right certificate.
   if ACTUAL_SHA=$(curl -fsS "$ENDPOINT" 2>/dev/null); then
     echo "Endpoint responded: $ACTUAL_SHA"
-    if [ "$ACTUAL_SHA" = "$EXPECTED_SHA" ]; then
+    if [[ "$ACTUAL_SHA" = "$EXPECTED_SHA" ]]; then
       echo "✅ SHA matches expected commit: $EXPECTED_SHA"
       exit 0
     else
@@ -43,7 +43,7 @@ while true; do
 
   now=$(date +%s)
   elapsed=$((now - start_time))
-  if [ $elapsed -ge $TIMEOUT ]; then
+  if [[ $elapsed -ge $TIMEOUT ]]; then
     echo "❌ Timeout: Endpoint did not become ready within ${TIMEOUT}s"
     exit 1
   fi