From 7b0ce7842bb77b1b13722e552f34280b621028a1 Mon Sep 17 00:00:00 2001 From: Apoorv Darshan Date: Fri, 27 Mar 2026 13:53:15 +0530 Subject: [PATCH] fix: remove return statements from finally blocks (#10102) Co-authored-by: Pepe Fagoaga --- prowler/CHANGELOG.md | 4 ++ prowler/lib/check/custom_checks_metadata.py | 34 +++++----- .../providers/aws/services/iam/iam_service.py | 64 ++++++++----------- .../iam_user_two_active_access_key.py | 5 +- .../organizations/organizations_service.py | 9 +-- prowler/providers/gcp/gcp_provider.py | 6 +- 6 files changed, 52 insertions(+), 70 deletions(-) diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 5bba9b2789..066d25a658 100644 --- a/prowler/CHANGELOG.md +++ b/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) diff --git a/prowler/lib/check/custom_checks_metadata.py b/prowler/lib/check/custom_checks_metadata.py index 3c100a720b..29fe64a7b8 100644 --- a/prowler/lib/check/custom_checks_metadata.py +++ b/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( diff --git a/prowler/providers/aws/services/iam/iam_service.py b/prowler/providers/aws/services/iam/iam_service.py index 1ff3dbd07a..a8469d3b9d 100644 --- a/prowler/providers/aws/services/iam/iam_service.py +++ b/prowler/providers/aws/services/iam/iam_service.py @@ -112,8 +112,8 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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 @@ class IAM(AWSService): 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...") diff --git a/prowler/providers/aws/services/iam/iam_user_two_active_access_key/iam_user_two_active_access_key.py b/prowler/providers/aws/services/iam/iam_user_two_active_access_key/iam_user_two_active_access_key.py index 6788c85322..769f9b86fe 100644 --- a/prowler/providers/aws/services/iam/iam_user_two_active_access_key/iam_user_two_active_access_key.py +++ b/prowler/providers/aws/services/iam/iam_user_two_active_access_key/iam_user_two_active_access_key.py @@ -5,8 +5,8 @@ from prowler.providers.aws.services.iam.iam_client import iam_client 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 @@ class iam_user_two_active_access_key(Check): findings.append(report) except Exception as error: logger.error(f"{error.__class__.__name__} -- {error}") - finally: - return findings + return findings diff --git a/prowler/providers/aws/services/organizations/organizations_service.py b/prowler/providers/aws/services/organizations/organizations_service.py index 78f5b134bd..65b5dd8e94 100644 --- a/prowler/providers/aws/services/organizations/organizations_service.py +++ b/prowler/providers/aws/services/organizations/organizations_service.py @@ -80,10 +80,9 @@ class Organizations(AWSService): 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 @@ class Organizations(AWSService): 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 @@ class Organizations(AWSService): f"{self.region} -- {error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - finally: - return self.delegated_administrators + return self.delegated_administrators class Policy(BaseModel): diff --git a/prowler/providers/gcp/gcp_provider.py b/prowler/providers/gcp/gcp_provider.py index c9e7c25501..5017b84c42 100644 --- a/prowler/providers/gcp/gcp_provider.py +++ b/prowler/providers/gcp/gcp_provider.py @@ -626,9 +626,8 @@ class GcpProvider(Provider): 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 @@ class GcpProvider(Provider): logger.critical( f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - finally: - return projects + return projects def update_projects_with_organizations(self): """