mirror of
https://github.com/prowler-cloud/prowler.git
synced 2026-10-09 21:14:22 +00:00
fix(cli): prevent unrelated built-in provider failures from aborting the CLI (#11618)
This commit is contained in:
@@ -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)
|
||||
|
||||
---
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
+297
-11
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user