fix(sdk): discriminate builtin vs external providers via find_spec for clearer import errors

This commit is contained in:
StylusFrost
2026-04-24 20:33:38 +02:00
parent 0883baad78
commit 907166d88a
3 changed files with 169 additions and 25 deletions
+27 -14
View File
@@ -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]:
+46 -11
View File
@@ -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."""
@@ -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."""