diff --git a/prowler/CHANGELOG.md b/prowler/CHANGELOG.md index 135cced4a7..69f3f743ce 100644 --- a/prowler/CHANGELOG.md +++ b/prowler/CHANGELOG.md @@ -36,6 +36,7 @@ All notable changes to the **Prowler SDK** are documented in this file. ### 🐞 Fixed - GCP `logging_log_metric_filter_and_alert_*` checks now credit org-level aggregated sinks filtered to the Admin Activity audit stream [(#11575)](https://github.com/prowler-cloud/prowler/pull/11575) +- A broken built-in provider no longer aborts the CLI when a different provider was invoked [(#11618)](https://github.com/prowler-cloud/prowler/pull/11618) - GCP organization scans with `--organization-id` no longer silently fall back to the credentials' host project when the Cloud Asset API call fails; the new `GCPGetOrganizationProjectsError` (3011) is raised instead, naming the required `roles/cloudasset.viewer` binding and Cloud Asset API enablement [(#11280)](https://github.com/prowler-cloud/prowler/pull/11280) --- diff --git a/prowler/lib/cli/parser.py b/prowler/lib/cli/parser.py index 8f05fc30cb..2848e9c788 100644 --- a/prowler/lib/cli/parser.py +++ b/prowler/lib/cli/parser.py @@ -15,6 +15,8 @@ from prowler.lib.check.models import Severity from prowler.lib.cli.redact import warn_sensitive_argument_values from prowler.lib.outputs.common import Status from prowler.providers.common.arguments import ( + PROVIDER_ALIASES, + enforce_invoked_provider_loaded, init_providers_parser, validate_asff_usage, validate_provider_arguments, @@ -166,13 +168,13 @@ Detailed documentation at https://docs.prowler.com if sys.argv[1].startswith("-"): sys.argv = self.__set_default_provider__(sys.argv) - # Provider aliases mapping - # Microsoft 365 - elif sys.argv[1] == "microsoft365": - sys.argv[1] = "m365" - # Oracle Cloud Infrastructure - elif sys.argv[1] == "oci": - sys.argv[1] = "oraclecloud" + # Provider aliases mapping (single source: arguments.PROVIDER_ALIASES) + elif sys.argv[1] in PROVIDER_ALIASES: + sys.argv[1] = PROVIDER_ALIASES[sys.argv[1]] + + # Selective fail-loud here (post argv-normalisation, pre parse_args) + # so the invoked-provider check stays correct under parse(args=...). + enforce_invoked_provider_loaded(self) # Warn about sensitive flags passed with explicit values # Snapshot argv before parse_args() which may exit on errors diff --git a/prowler/providers/common/arguments.py b/prowler/providers/common/arguments.py index 9e23274a90..2e4d9c8896 100644 --- a/prowler/providers/common/arguments.py +++ b/prowler/providers/common/arguments.py @@ -10,16 +10,43 @@ provider_arguments_lib_path = "lib.arguments.arguments" validate_provider_arguments_function = "validate_arguments" init_provider_arguments_function = "init_parser" +# Kept in sync with parser.py's argv normalisation; both consumers import this. +PROVIDER_ALIASES = { + "microsoft365": "m365", + "oci": "oraclecloud", +} + + +def _invoked_provider_from_argv(available_providers: Sequence[str]) -> Optional[str]: + """Return the provider name the user invoked, or None. + + Mirrors `ProwlerArgumentParser.parse()` resolution: only inspects + `sys.argv[1]`. Scanning the whole argv would misclassify + `prowler --output-directory stackit` as `stackit`. + """ + available = set(available_providers) + if len(sys.argv) < 2: + return "aws" if "aws" in available else None + first = sys.argv[1] + if first in ("-h", "--help", "-v", "--version"): + return None + if first.startswith("-"): + return "aws" if "aws" in available else None + normalized = PROVIDER_ALIASES.get(first, first) + return normalized if normalized in available else None + def init_providers_parser(self): - """init_providers_parser calls the provider init_parser function to load all the arguments and flags. Receives a ProwlerArgumentParser object""" - # We need to call the arguments parser for each provider + """Build the subparser of each available provider. + + Built-in load failures are captured silently on + `self._builtin_load_failures`; the warn/exit decision is deferred to + `enforce_invoked_provider_loaded()` because `parse(args=...)` can + override `sys.argv` after this function ran. + """ + self._builtin_load_failures = {} providers = Provider.get_available_providers() for provider in providers: - # 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( @@ -28,21 +55,9 @@ def init_providers_parser(self): ), 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) + self._builtin_load_failures[provider] = error else: - # External provider — init_parser classmethod via entry point cls = Provider._load_ep_provider(provider) if cls and hasattr(cls, "init_parser"): try: @@ -53,6 +68,51 @@ def init_providers_parser(self): ) +def enforce_invoked_provider_loaded(self): + """Apply selective fail-loud over the failures captured at init time. + + Called by `ProwlerArgumentParser.parse()` AFTER argv normalisation so + the invoked provider matches what argparse will dispatch to — including + the case where `parse(args=...)` overrode the ambient `sys.argv`. + + Invoked + failed → critical + `sys.exit(1)`. Others → warning. + """ + failures = getattr(self, "_builtin_load_failures", {}) + if not failures: + return + invoked = _invoked_provider_from_argv(Provider.get_available_providers()) + for provider, error in failures.items(): + if provider == invoked: + continue + if isinstance(error, ImportError): + logger.warning( + f"Skipping built-in provider '{provider}' due to missing " + f"dependency: {error}. It will be unavailable in this " + f"invocation, but the CLI continues because you invoked a " + f"different provider." + ) + else: + logger.warning( + f"Skipping built-in provider '{provider}': " + f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + ) + if invoked is None or invoked not in failures: + return + error = failures[invoked] + if isinstance(error, ImportError): + logger.critical( + f"Failed to load arguments for built-in provider '{invoked}'. " + f"Missing dependency: {error}. " + f"Ensure all required dependencies are installed." + ) + logger.debug("Full traceback:", exc_info=True) + else: + logger.critical( + f"{error.__class__.__name__}[{error.__traceback__.tb_lineno}]: {error}" + ) + sys.exit(1) + + def validate_provider_arguments(arguments: Namespace) -> tuple[bool, str]: """validate_provider_arguments returns {True, "} if the provider arguments passed are valid and can be used together""" try: diff --git a/tests/providers/external/test_dynamic_provider_loading.py b/tests/providers/external/test_dynamic_provider_loading.py index eefde19545..6bd1e6a2ce 100644 --- a/tests/providers/external/test_dynamic_provider_loading.py +++ b/tests/providers/external/test_dynamic_provider_loading.py @@ -417,17 +417,19 @@ class TestIsBuiltinProvider: 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.""" + """Selective fail-loud: init captures failures silently, enforce emits + warning for non-invoked and exits for the invoked broken provider.""" + @patch("sys.argv", ["prowler", "aws"]) @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 + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) mock_is_builtin.return_value = True mock_import.side_effect = ImportError("No module named 'boto3'") @@ -435,14 +437,14 @@ class TestInitProvidersParserBuiltinDependencyFailure: parser = MagicMock() parser._providers = ["aws"] - with ( - patch( - "prowler.providers.common.arguments.Provider.get_available_providers", - return_value=["aws"], - ), - pytest.raises(SystemExit), + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws"], ): init_providers_parser(parser) + assert "aws" in parser._builtin_load_failures + with pytest.raises(SystemExit): + enforce_invoked_provider_loaded(parser) @patch("prowler.providers.common.arguments.Provider.is_builtin") @patch("prowler.providers.common.arguments.Provider._load_ep_provider") @@ -466,6 +468,290 @@ class TestInitProvidersParserBuiltinDependencyFailure: ext_cls.init_parser.assert_called_once_with(parser) + @patch("sys.argv", ["prowler", "aws"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_unrelated_builtin_failure_does_not_abort_when_other_provider_invoked( + self, mock_import, mock_is_builtin + ): + """Broken stackit + invoked aws → warning, no abort.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + aws_module = MagicMock() + + def import_side_effect(module_path): + if "stackit" in module_path: + raise ImportError("No module named 'stackit.objectstorage'") + return aws_module + + mock_import.side_effect = import_side_effect + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws", "stackit"], + ): + init_providers_parser(parser) + assert "stackit" in parser._builtin_load_failures + enforce_invoked_provider_loaded(parser) + + aws_module.init_parser.assert_called_once_with(parser) + + @patch("sys.argv", ["prowler", "-h"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_no_provider_invoked_failure_does_not_abort( + self, mock_import, mock_is_builtin + ): + """`prowler -h` + broken built-in → warning, help still renders.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + mock_import.side_effect = ImportError("No module named 'stackit.objectstorage'") + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["stackit"], + ): + init_providers_parser(parser) + enforce_invoked_provider_loaded(parser) + + @patch("sys.argv", ["prowler", "microsoft365"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_invoked_microsoft365_alias_still_triggers_fail_loud( + self, mock_import, mock_is_builtin + ): + """Alias `microsoft365 → m365` must be normalised before matching.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + mock_import.side_effect = ImportError("No module named 'msgraph'") + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["m365"], + ): + init_providers_parser(parser) + with pytest.raises(SystemExit): + enforce_invoked_provider_loaded(parser) + + @patch("sys.argv", ["prowler", "oci"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_invoked_oci_alias_still_triggers_fail_loud( + self, mock_import, mock_is_builtin + ): + """Alias `oci → oraclecloud` must be normalised before matching.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + mock_import.side_effect = ImportError("No module named 'oci'") + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["oraclecloud"], + ): + init_providers_parser(parser) + with pytest.raises(SystemExit): + enforce_invoked_provider_loaded(parser) + + @patch("sys.argv", ["prowler", "--output-directory", "stackit"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_flag_value_matching_provider_name_not_treated_as_invoked( + self, mock_import, mock_is_builtin + ): + """Flag-first invocation → invoked is 'aws' (default), not the flag's value.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + aws_module = MagicMock() + + def import_side_effect(module_path): + if "stackit" in module_path: + raise ImportError("No module named 'stackit.objectstorage'") + return aws_module + + mock_import.side_effect = import_side_effect + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws", "stackit"], + ): + init_providers_parser(parser) + enforce_invoked_provider_loaded(parser) + + aws_module.init_parser.assert_called_once_with(parser) + + @patch("sys.argv", ["prowler", "aws"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_invoked_builtin_non_import_error_fails_loudly( + self, mock_import, mock_is_builtin + ): + """Non-ImportError in invoked provider → still fail-loud.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + mock_import.side_effect = RuntimeError("Unexpected error in aws init_parser") + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws"], + ): + init_providers_parser(parser) + with pytest.raises(SystemExit): + enforce_invoked_provider_loaded(parser) + + @patch("sys.argv", ["prowler", "aws"]) + @patch("prowler.providers.common.arguments.Provider.is_builtin") + @patch("prowler.providers.common.arguments.import_module") + def test_unrelated_builtin_non_import_error_does_not_abort( + self, mock_import, mock_is_builtin + ): + """Non-ImportError in unrelated provider → warning, no abort.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + mock_is_builtin.return_value = True + aws_module = MagicMock() + + def import_side_effect(module_path): + if "stackit" in module_path: + raise RuntimeError("Unexpected error in stackit init_parser") + return aws_module + + mock_import.side_effect = import_side_effect + + parser = MagicMock() + + with patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws", "stackit"], + ): + init_providers_parser(parser) + enforce_invoked_provider_loaded(parser) + + aws_module.init_parser.assert_called_once_with(parser) + + +class TestParseArgsOverrideAlignment: + """Regression: `parse(args=...)` overrides sys.argv AFTER __init__ ran; + the selective fail-loud must read argv at enforce time, not init time.""" + + def test_enforce_reads_current_sys_argv_not_init_time_sys_argv(self): + """Init with argv=['prowler','-h'] (no provider) captures stackit + failure silently. Enforce with argv=['prowler','stackit'] must + fail-loud — proving alignment under parse(args=...).""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + def import_side_effect(path): + if "stackit" in path: + raise ImportError("No module named 'stackit.objectstorage'") + return MagicMock() + + parser = MagicMock() + + with ( + patch( + "prowler.providers.common.arguments.Provider.is_builtin", + return_value=True, + ), + patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws", "stackit"], + ), + patch( + "prowler.providers.common.arguments.import_module", + side_effect=import_side_effect, + ), + ): + # Phase 1: __init__ with ambient argv = ['prowler', '-h'] + with patch("sys.argv", ["prowler", "-h"]): + init_providers_parser(parser) + # Failure captured silently — no SystemExit during init + assert "stackit" in parser._builtin_load_failures + + # Phase 2: parse(args=...) overrode sys.argv → stackit invoked + with patch("sys.argv", ["prowler", "stackit"]): + with pytest.raises(SystemExit): + enforce_invoked_provider_loaded(parser) + + def test_enforce_reads_current_sys_argv_for_no_invocation(self): + """Inverse: init's argv invokes stackit, but parse(args=['prowler', + '-h']) overrides. Enforce must NOT fail-loud.""" + from prowler.providers.common.arguments import ( + enforce_invoked_provider_loaded, + init_providers_parser, + ) + + def import_side_effect(path): + if "stackit" in path: + raise ImportError("No module named 'stackit.objectstorage'") + return MagicMock() + + parser = MagicMock() + + with ( + patch( + "prowler.providers.common.arguments.Provider.is_builtin", + return_value=True, + ), + patch( + "prowler.providers.common.arguments.Provider.get_available_providers", + return_value=["aws", "stackit"], + ), + patch( + "prowler.providers.common.arguments.import_module", + side_effect=import_side_effect, + ), + ): + # Phase 1: __init__ with ambient argv pretending stackit invoked + with patch("sys.argv", ["prowler", "stackit"]): + init_providers_parser(parser) + assert "stackit" in parser._builtin_load_failures + + # Phase 2: parse(args=['prowler', '-h']) overrode sys.argv → + # no provider invoked anymore → enforce must NOT exit + with patch("sys.argv", ["prowler", "-h"]): + enforce_invoked_provider_loaded(parser) + class TestInitGlobalProviderBuiltinDependencyFailure: """Same contract as TestInitProvidersParserBuiltinDependencyFailure but