fix: remove return statements from finally blocks (#10102)

Co-authored-by: Pepe Fagoaga <[email protected]>

Author: apoorvdarshan

prowler/CHANGELOG.md

@@ -17,6 +17,10 @@ All notable changes to the **Prowler SDK** are documented in this file.
 
 - Minimum Python version from 3.9 to 3.10 and updated classifiers to reflect supported versions (3.10, 3.11, 3.12) [(#10464)](https://github.com/prowler-cloud/prowler/pull/10464)
 
+### 🐞 Fixed
+
+- `return` statements in `finally` blocks replaced across IAM, Organizations, GCP provider, and custom checks metadata to stop silently swallowing exceptions [(#10102)](https://github.com/prowler-cloud/prowler/pull/10102)
+
 ---
 
 ## [5.22.1] (Prowler UNRELEASED)

prowler/lib/check/custom_checks_metadata.py

@@ -112,24 +112,22 @@ def update_checks_metadata(bulk_checks_metadata, custom_checks_metadata):
 
 def update_check_metadata(check_metadata, custom_metadata):
     """update_check_metadata updates the check_metadata fields present in the custom_metadata and returns the updated version of the check_metadata. If some field is not present or valid the check_metadata is returned with the original fields."""
-    try:
-        if custom_metadata:
-            for attribute in custom_metadata:
-                if attribute == "Remediation":
-                    for remediation_attribute in custom_metadata[attribute]:
-                        update_check_metadata_remediation(
-                            check_metadata,
-                            custom_metadata,
-                            attribute,
-                            remediation_attribute,
-                        )
-                else:
-                    try:
-                        setattr(check_metadata, attribute, custom_metadata[attribute])
-                    except ValueError:
-                        pass
-    finally:
-        return check_metadata
+    if custom_metadata:
+        for attribute in custom_metadata:
+            if attribute == "Remediation":
+                for remediation_attribute in custom_metadata[attribute]:
+                    update_check_metadata_remediation(
+                        check_metadata,
+                        custom_metadata,
+                        attribute,
+                        remediation_attribute,
+                    )
+            else:
+                try:
+                    setattr(check_metadata, attribute, custom_metadata[attribute])
+                except ValueError:
+                    pass
+    return check_metadata
 
 
 def update_check_metadata_remediation(

prowler/providers/aws/services/iam/iam_service.py

@@ -112,8 +112,8 @@ def _get_client(self):
 
     def _get_roles(self):
         logger.info("IAM - List Roles...")
+        roles = []
         try:
-            roles = []
             get_roles_paginator = self.client.get_paginator("list_roles")
             for page in get_roles_paginator.paginate():
                 for role in page["Roles"]:
@@ -142,8 +142,7 @@ def _get_roles(self):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return roles
+        return roles
 
     def _get_credential_report(self):
         logger.info("IAM - Get Credential Report...")
@@ -175,13 +174,12 @@ def _get_credential_report(self):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return credential_list
+        return credential_list
 
     def _get_groups(self):
         logger.info("IAM - Get Groups...")
+        groups = []
         try:
-            groups = []
             get_groups_paginator = self.client.get_paginator("list_groups")
             for page in get_groups_paginator.paginate():
                 for group in page["Groups"]:
@@ -194,25 +192,23 @@ def _get_groups(self):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return groups
+        return groups
 
     def _get_account_summary(self):
         logger.info("IAM - Get Account Summary...")
+        account_summary = None
         try:
             account_summary = self.client.get_account_summary()
         except Exception as error:
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-            account_summary = None
-        finally:
-            return account_summary
+        return account_summary
 
     def _get_password_policy(self):
         logger.info("IAM - Get Password Policy...")
+        stored_password_policy = None
         try:
-            stored_password_policy = None
             password_policy = self.client.get_account_password_policy()[
                 "PasswordPolicy"
             ]
@@ -274,14 +270,13 @@ def _get_password_policy(self):
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
 
-        finally:
-            return stored_password_policy
+        return stored_password_policy
 
     def _get_users(self):
         logger.info("IAM - Get Users...")
+        users = []
         try:
             get_users_paginator = self.client.get_paginator("list_users")
-            users = []
             for page in get_users_paginator.paginate():
                 for user in page["Users"]:
                     if not self.audit_resources or (
@@ -311,13 +306,12 @@ def _get_users(self):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return users
+        return users
 
     def _list_virtual_mfa_devices(self):
         logger.info("IAM - List Virtual MFA Devices...")
+        mfa_devices = []
         try:
-            mfa_devices = []
             list_virtual_mfa_devices_paginator = self.client.get_paginator(
                 "list_virtual_mfa_devices"
             )
@@ -329,8 +323,7 @@ def _list_virtual_mfa_devices(self):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return mfa_devices
+        return mfa_devices
 
     def _list_attached_group_policies(self):
         logger.info("IAM - List Attached Group Policies...")
@@ -677,12 +670,11 @@ def _list_inline_role_policies(self):
 
     def _list_entities_role_for_policy(self, policy_arn):
         logger.info("IAM - List Entities Role For Policy...")
+        roles = []
         try:
-            roles = []
             roles = self.client.list_entities_for_policy(
                 PolicyArn=policy_arn, EntityFilter="Role"
             )["PolicyRoles"]
-            return roles
         except ClientError as error:
             if error.response["Error"]["Code"] == "AccessDenied":
                 logger.error(
@@ -697,18 +689,16 @@ def _list_entities_role_for_policy(self, policy_arn):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return roles
+        return roles
 
     def _list_entities_for_policy(self, policy_arn):
         logger.info("IAM - List Entities For Policy...")
+        entities = {
+            "Users": [],
+            "Groups": [],
+            "Roles": [],
+        }
         try:
-            entities = {
-                "Users": [],
-                "Groups": [],
-                "Roles": [],
-            }
-
             paginator = self.client.get_paginator("list_entities_for_policy")
             for response in paginator.paginate(PolicyArn=policy_arn):
                 entities["Users"].extend(
@@ -720,7 +710,6 @@ def _list_entities_for_policy(self, policy_arn):
                 entities["Roles"].extend(
                     role["RoleName"] for role in response.get("PolicyRoles", [])
                 )
-            return entities
         except ClientError as error:
             if error.response["Error"]["Code"] == "AccessDenied":
                 logger.error(
@@ -735,13 +724,12 @@ def _list_entities_for_policy(self, policy_arn):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return entities
+        return entities
 
     def _list_policies(self, scope):
         logger.info("IAM - List Policies...")
+        policies = {}
         try:
-            policies = {}
             list_policies_paginator = self.client.get_paginator("list_policies")
             for page in list_policies_paginator.paginate(
                 Scope=scope, OnlyAttached=False if scope == "Local" else True
@@ -762,8 +750,7 @@ def _list_policies(self, scope):
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return policies
+        return policies
 
     def _list_policies_version(self, policies):
         logger.info("IAM - List Policies Version...")
@@ -817,8 +804,8 @@ def _list_saml_providers(self):
 
     def _list_server_certificates(self) -> list:
         logger.info("IAM - List Server Certificates...")
+        server_certificates = []
         try:
-            server_certificates = []
             for certificate in self.client.list_server_certificates()[
                 "ServerCertificateMetadataList"
             ]:
@@ -837,8 +824,7 @@ def _list_server_certificates(self) -> list:
             logger.error(
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return server_certificates
+        return server_certificates
 
     def _list_tags(self, resource: any):
         logger.info("IAM - List Tags...")

prowler/providers/aws/services/iam/iam_user_two_active_access_key/iam_user_two_active_access_key.py

@@ -5,8 +5,8 @@
 
 class iam_user_two_active_access_key(Check):
     def execute(self) -> Check_Report_AWS:
+        findings = []
         try:
-            findings = []
             response = iam_client.credential_report
             for user in response:
                 report = Check_Report_AWS(metadata=self.metadata(), resource=user)
@@ -34,5 +34,4 @@ def execute(self) -> Check_Report_AWS:
                 findings.append(report)
         except Exception as error:
             logger.error(f"{error.__class__.__name__} -- {error}")
-        finally:
-            return findings
+        return findings

prowler/providers/aws/services/organizations/organizations_service.py

@@ -80,10 +80,9 @@ def _describe_organization(self):
 
     def _list_policies(self):
         logger.info("Organizations - List policies...")
-
+        policies = {}
         try:
             list_policies_paginator = self.client.get_paginator("list_policies")
-            policies = {}
             for policy_type in AVAILABLE_ORGANIZATIONS_POLICIES:
                 logger.info(
                     "Organizations - List policies... - Type: %s",
@@ -122,8 +121,7 @@ def _list_policies(self):
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
 
-        finally:
-            return policies
+        return policies
 
     def _describe_policy(self, policy_id) -> dict:
         logger.info("Organizations - Describe policy: %s ...", policy_id)
@@ -192,8 +190,7 @@ def _list_delegated_administrators(self):
                 f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
 
-        finally:
-            return self.delegated_administrators
+        return self.delegated_administrators
 
 
 class Policy(BaseModel):

prowler/providers/gcp/gcp_provider.py

@@ -626,9 +626,8 @@ def get_projects(
         Usage:
             >>> GcpProvider.get_projects(credentials=credentials, organization_id=organization_id)
         """
+        projects = {}
         try:
-            projects = {}
-
             if organization_id:
                 try:
                     # Initialize Cloud Asset Inventory API for recursive project retrieval
@@ -803,8 +802,7 @@ def get_projects(
             logger.critical(
                 f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}"
             )
-        finally:
-            return projects
+        return projects
 
     def update_projects_with_organizations(self):
         """