diff --git a/prowler/providers/common/arguments.py b/prowler/providers/common/arguments.py index 075ad974c8..9e23274a90 100644 --- a/prowler/providers/common/arguments.py +++ b/prowler/providers/common/arguments.py @@ -16,15 +16,33 @@ def init_providers_parser(self): # We need to call the arguments parser for each provider providers = Provider.get_available_providers() for provider in providers: - try: - getattr( - import_module( - f"{providers_path}.{provider}.{provider_arguments_lib_path}" - ), - init_provider_arguments_function, - )(self) - except ImportError: - # External provider — try init_parser classmethod via entry point + # Discriminate built-in vs external upfront via find_spec, so an + # ImportError from a transitive dependency missing inside a built-in + # arguments module surfaces clearly instead of being silently + # re-routed to the entry-point path (which only has external providers). + if Provider.is_builtin(provider): + try: + getattr( + import_module( + f"{providers_path}.{provider}.{provider_arguments_lib_path}" + ), + init_provider_arguments_function, + )(self) + except ImportError as e: + logger.critical( + f"Failed to load arguments for built-in provider '{provider}'. " + f"Missing dependency: {e}. " + f"Ensure all required dependencies are installed." + ) + logger.debug("Full traceback:", exc_info=True) + sys.exit(1) + except Exception as error: + logger.critical( + f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + ) + sys.exit(1) + else: + # External provider — init_parser classmethod via entry point cls = Provider._load_ep_provider(provider) if cls and hasattr(cls, "init_parser"): try: @@ -33,11 +51,6 @@ def init_providers_parser(self): logger.warning( f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" ) - except Exception as error: - logger.critical( - f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" - ) - sys.exit(1) def validate_provider_arguments(arguments: Namespace) -> tuple[bool, str]: diff --git a/prowler/providers/common/provider.py b/prowler/providers/common/provider.py index bd60063524..e6366fb798 100644 --- a/prowler/providers/common/provider.py +++ b/prowler/providers/common/provider.py @@ -1,5 +1,6 @@ import importlib import importlib.metadata +import importlib.util import os import pkgutil import sys @@ -265,21 +266,38 @@ class Provider(ABC): @staticmethod def init_global_provider(arguments: Namespace) -> None: try: - # Try built-in provider first, fall back to entry point + # Discriminate built-in vs external upfront via find_spec, so an + # ImportError from a transitive dependency missing inside a + # built-in's own import chain surfaces clearly instead of being + # silently re-routed to the entry-point path. provider_class = None - try: + if Provider.is_builtin(arguments.provider): provider_class_path = f"{providers_path}.{arguments.provider}.{arguments.provider}_provider" provider_class_name = f"{arguments.provider.capitalize()}Provider" - provider_class = getattr( - import_module(provider_class_path), provider_class_name - ) - except (ImportError, AttributeError): - # External provider — load via entry point - provider_class = Provider._load_ep_provider(arguments.provider) - if provider_class is None: - raise ImportError( - f"Provider '{arguments.provider}' not found as built-in or entry point" + try: + provider_class = getattr( + import_module(provider_class_path), provider_class_name ) + except ImportError as e: + logger.critical( + f"Failed to load built-in provider '{arguments.provider}'. " + f"Missing dependency: {e}. " + f"Ensure all required dependencies are installed." + ) + logger.debug("Full traceback:", exc_info=True) + sys.exit(1) + except AttributeError: + # Module exists but doesn't define the expected class — + # treat as external and try entry points. + provider_class = Provider._load_ep_provider(arguments.provider) + else: + provider_class = Provider._load_ep_provider(arguments.provider) + + if provider_class is None: + raise ImportError( + f"Provider '{arguments.provider}' not found as built-in or entry point" + ) + provider_class_name = f"{arguments.provider.capitalize()}Provider" fixer_config = load_and_validate_config_file( arguments.provider, arguments.fixer_config @@ -568,6 +586,23 @@ class Provider(ABC): ep_cls = Provider._load_ep_provider(provider) return bool(ep_cls and getattr(ep_cls, "is_external_tool_provider", False)) + @staticmethod + def is_builtin(provider: str) -> bool: + """Return True if the provider's own package is importable as a built-in. + + Uses `importlib.util.find_spec` — Python's canonical API to check module + existence without executing it. Discriminates at call sites between + built-in providers (`prowler.providers.{provider}`) and externals, so we + don't rely on catching `ImportError` after the fact and inspecting + `e.name` — which is fragile when the error comes from a transitive + dependency inside the built-in's own import chain. + """ + try: + spec = importlib.util.find_spec(f"{providers_path}.{provider}") + return spec is not None + except (ImportError, ValueError): + return False + @staticmethod def _load_ep_provider(name: str): """Load an external provider class from entry points, with cache.""" diff --git a/tests/providers/external/test_dynamic_provider_loading.py b/tests/providers/external/test_dynamic_provider_loading.py index 39662d79c4..baa939fbcf 100644 --- a/tests/providers/external/test_dynamic_provider_loading.py +++ b/tests/providers/external/test_dynamic_provider_loading.py @@ -379,6 +379,102 @@ class TestIsToolWrapperProvider: assert Provider.is_tool_wrapper_provider(None) is False + +class TestIsBuiltinProvider: + """Tests for Provider.is_builtin — the helper that discriminates built-in + providers from external ones before attempting the import, so transitive + dependency failures in built-ins don't get silently re-routed to entry points.""" + + def test_returns_true_for_builtin_provider(self): + assert Provider.is_builtin("aws") is True + assert Provider.is_builtin("github") is True + + def test_returns_false_for_unknown_provider(self): + assert Provider.is_builtin("nonexistent_xyz") is False + + @patch("prowler.providers.common.provider.importlib.util.find_spec") + def test_returns_false_when_find_spec_raises(self, mock_find_spec): + # Certain namespace package edge cases raise ValueError/ImportError — + # helper should swallow and return False rather than propagate. + mock_find_spec.side_effect = ValueError("namespace package edge case") + + assert Provider.is_builtin("some_provider") is False + + +class TestInitProvidersParserBuiltinDependencyFailure: + """Tests the critical behavior fix: when a built-in provider's arguments + module exists but its imports fail (e.g. boto3 not installed), we must + fail loudly with a clear message — not silently fall through to entry + points as if the provider were external.""" + + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_builtin_with_missing_transitive_dep_fails_loudly( + self, mock_import, mock_is_builtin + ): + from prowler.providers.common.arguments import init_providers_parser + + mock_is_builtin.return_value = True + mock_import.side_effect = ImportError("No module named 'boto3'") + + parser = MagicMock() + parser._providers = ["aws"] + + with ( + patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws"], + ), + pytest.raises(SystemExit), + ): + init_providers_parser(parser) + + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.Provider._load_ep_provider") + def test_external_provider_does_not_touch_builtin_path( + self, mock_load_ep, mock_is_builtin + ): + from prowler.providers.common.arguments import init_providers_parser + + mock_is_builtin.return_value = False + ext_cls = MagicMock() + ext_cls.init_parser = MagicMock() + mock_load_ep.return_value = ext_cls + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["fakeexternal"], + ): + init_providers_parser(parser) + + ext_cls.init_parser.assert_called_once_with(parser) + + +class TestInitGlobalProviderBuiltinDependencyFailure: + """Same contract as TestInitProvidersParserBuiltinDependencyFailure but + for the provider class import path in init_global_provider.""" + + @patch("prowler.providers.common.provider.Provider.is_builtin") + @patch("prowler.providers.common.provider.import_module") + def test_builtin_with_missing_transitive_dep_fails_loudly( + self, mock_import, mock_is_builtin + ): + mock_is_builtin.return_value = True + mock_import.side_effect = ImportError("No module named 'boto3'") + + args = Namespace( + provider="aws", + fixer_config="config.yaml", + config_file="config.yaml", + ) + + Provider._global = None + with pytest.raises(SystemExit): + Provider.init_global_provider(args) + Provider._global = None + @patch("prowler.providers.common.provider.importlib.metadata.entry_points") def test_load_ep_provider_handles_load_exception(self, mock_ep): """_load_ep_provider returns None when ep.load() raises."""