fix(sdk): resolve entry-point checks on built-in providers (#12294)

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Daniel Barranquero <danielbo2001@gmail.com>

prowler/changelog.d/external-checks-builtin-provider-resolution.fixed.md

@@ -0,0 +1 @@
+Checks registered through the `prowler.checks.<provider>` entry-point group can now run against built-in providers. The built-in probe in `_resolve_check_module` used a bare `find_spec`, which imports the parent package to search it and so raised `ModuleNotFoundError` for a plug-in check instead of returning `None`, aborting the lookup before the entry points were consulted. Such a check was discovered, listed and selected for execution, then silently produced no findings.

prowler/lib/check/check.py

@@ -21,7 +21,11 @@ from prowler.lib.check.utils import recover_checks_from_provider
 from prowler.lib.logger import logger
 from prowler.lib.outputs.outputs import report
 from prowler.lib.utils.utils import open_file, parse_json_file, print_boxes
-from prowler.providers.common.builtin import is_builtin_provider
+from prowler.providers.common.builtin import (
+    builtin_check_module,
+    is_builtin_check,
+    is_builtin_provider,
+)
 from prowler.providers.common.models import Audit_Metadata
 
 
@@ -401,21 +405,23 @@ def _resolve_check_module(
     when a plug-in tries to override, so the user knows their plug-in
     duplicate is being ignored and can rename it.
 
-    Gates the built-in branch on `is_builtin_provider(provider_type)` —
-    calling `find_spec` on `prowler.providers.{provider_type}.services...`
-    directly would propagate `ModuleNotFoundError` for external providers
-    (their parent package `prowler.providers.{provider_type}` does not
-    exist) instead of returning None. The leaf helper encapsulates the
-    safe lookup, so external providers go straight to entry points. For
-    built-ins we still use `find_spec` to distinguish "check doesn't
-    exist" from "check exists but failed to import" (broken transitive
-    dep, etc.).
+    Both probes are gated on leaf helpers rather than a raw `find_spec`,
+    because `find_spec` imports the parent package in order to search it and
+    so propagates `ModuleNotFoundError` instead of returning None whenever
+    that parent is absent. That happens on both axes: for an external
+    provider (no `prowler.providers.{provider_type}` package) and, on a
+    built-in provider, for an external check (no
+    `prowler.providers.{provider_type}.services.{service}.{check_name}`
+    package). Either one, probed naively, aborts the lookup before the entry
+    points are ever consulted. `is_builtin_check` still distinguishes "check
+    doesn't exist" from "check exists but failed to import" (broken
+    transitive dep, etc.), which a blanket except would flatten.
     """
     # Built-in first — built-in wins on CheckID collision
-    if is_builtin_provider(provider_type):
-        builtin_path = f"prowler.providers.{provider_type}.services.{service}.{check_name}.{check_name}"
-        if importlib.util.find_spec(builtin_path) is not None:
-            return import_check(builtin_path)
+    if is_builtin_provider(provider_type) and is_builtin_check(
+        provider_type, service, check_name
+    ):
+        return import_check(builtin_check_module(provider_type, service, check_name))
 
     # Entry point lookup — only consulted when the built-in truly doesn't exist
     for ep in importlib.metadata.entry_points(group=f"prowler.checks.{provider_type}"):

prowler/providers/common/builtin.py

@@ -27,3 +27,44 @@ def is_builtin_provider(provider: str) -> bool:
         return spec is not None
     except (ImportError, ValueError):
         return False
+
+
+def builtin_check_module(provider: str, service: str, check_name: str) -> str:
+    """Return the module path a built-in check would live at."""
+    return f"prowler.providers.{provider}.services.{service}.{check_name}.{check_name}"
+
+
+def is_builtin_check(provider: str, service: str, check_name: str) -> bool:
+    """Return True if the check's module ships with the SDK.
+
+    Sibling of `is_builtin_provider`, and unsafe for the same reason if probed
+    naively: `find_spec` imports the parent package in order to search it, so
+    asking about a check that lives in a plug-in raises `ModuleNotFoundError`
+    rather than returning `None`. A check registered through
+    `prowler.checks.{provider}` never has a parent under
+    `prowler.providers.{provider}.services.{service}`, so the naive probe makes
+    every external check on a built-in provider unresolvable.
+
+    Unlike its sibling this one narrows the exception instead of swallowing
+    every `ImportError`. A provider either ships with the SDK or it does not,
+    but callers rely on this probe to tell "the check is not built-in" apart
+    from "the check is built-in and its imports are broken". Reporting the
+    second as the first would turn a broken dependency into a silent
+    "check not found".
+    """
+    module = builtin_check_module(provider, service, check_name)
+    try:
+        return importlib.util.find_spec(module) is not None
+    except ModuleNotFoundError as error:
+        # Only absorb "this check is simply not here". `error.name` is the
+        # module that could not be imported; when it is the check's own path
+        # (or a prefix of it) the check does not ship with the SDK. Anything
+        # else — a missing third-party dependency, say — belongs to a built-in
+        # check that does exist and must stay loud.
+        if error.name is None or (
+            error.name != module and not module.startswith(f"{error.name}.")
+        ):
+            raise
+        return False
+    except ValueError:
+        return False

tests/providers/common/builtin_test.py

@@ -0,0 +1,119 @@
+from unittest.mock import patch
+
+import pytest
+
+from prowler.providers.common.builtin import (
+    builtin_check_module,
+    is_builtin_check,
+    is_builtin_provider,
+)
+
+
+class TestBuiltinCheckModule:
+    def test_builds_the_sdk_module_path(self):
+        assert (
+            builtin_check_module("aws", "ec2", "ec2_instance_public_ip")
+            == "prowler.providers.aws.services.ec2.ec2_instance_public_ip.ec2_instance_public_ip"
+        )
+
+
+class TestIsBuiltinProvider:
+    def test_true_for_a_provider_shipped_with_the_sdk(self):
+        assert is_builtin_provider("aws") is True
+
+    def test_false_for_a_provider_that_lives_in_a_plugin(self):
+        # No `prowler.providers.acme` package: find_spec raises on the absent
+        # parent rather than returning None, and the helper absorbs it.
+        assert is_builtin_provider("acme") is False
+
+
+class TestIsBuiltinCheck:
+    def test_true_for_a_check_shipped_with_the_sdk(self):
+        assert is_builtin_check("aws", "ec2", "ec2_instance_public_ip") is True
+
+    def test_false_for_an_external_check_on_a_builtin_provider(self):
+        """The case that made every plug-in check unresolvable.
+
+        `prowler.providers.aws.services.ec2` exists, so the naive probe gets
+        far enough to try importing the check package as a parent — and that
+        package only exists inside the plug-in. find_spec raises instead of
+        returning None.
+        """
+        assert (
+            is_builtin_check("aws", "ec2", "ec2_acme_instance_has_owner_tag") is False
+        )
+
+    def test_false_for_a_service_that_does_not_exist(self):
+        assert (
+            is_builtin_check("aws", "acmeservice", "acmeservice_thing_is_fine") is False
+        )
+
+    def test_false_for_an_external_provider(self):
+        assert (
+            is_builtin_check("acme", "inventory", "inventory_item_has_owner") is False
+        )
+
+    def test_reraises_when_a_builtin_checks_own_dependency_is_missing(self):
+        """A broken import must not read as "the check is not built-in".
+
+        Collapsing the two would turn a missing dependency into a silent
+        "check not found", which is the failure mode this probe exists to
+        avoid.
+        """
+        module = builtin_check_module("aws", "ec2", "ec2_instance_public_ip")
+
+        with patch(
+            "prowler.providers.common.builtin.importlib.util.find_spec",
+            side_effect=ModuleNotFoundError("No module named 'boto3'", name="boto3"),
+        ):
+            with pytest.raises(ModuleNotFoundError):
+                is_builtin_check("aws", "ec2", "ec2_instance_public_ip")
+
+        # Sanity: the same error naming the check's own path is absorbed.
+        with patch(
+            "prowler.providers.common.builtin.importlib.util.find_spec",
+            side_effect=ModuleNotFoundError(f"No module named '{module}'", name=module),
+        ):
+            assert is_builtin_check("aws", "ec2", "ec2_instance_public_ip") is False
+
+    def test_reraises_when_missing_module_name_is_only_a_textual_prefix(self):
+        """A sibling module prefix must not read as the check's missing parent."""
+        sibling_prefix = "prowler.providers.aws.services.ec2.ec2"
+
+        with patch(
+            "prowler.providers.common.builtin.importlib.util.find_spec",
+            side_effect=ModuleNotFoundError(
+                f"No module named '{sibling_prefix}'", name=sibling_prefix
+            ),
+        ):
+            with pytest.raises(ModuleNotFoundError):
+                is_builtin_check("aws", "ec2", "ec2_instance_public_ip")
+
+    @pytest.mark.parametrize(
+        "error",
+        [
+            ValueError("namespace package edge case"),
+        ],
+        ids=["value_error"],
+    )
+    def test_false_when_find_spec_raises_value_error(self, error):
+        """Mirrors the guard `is_builtin_provider` already carries.
+
+        `find_spec` can fail for reasons that are not "the module is absent" —
+        a namespace-package edge case raises ValueError. That does not say the
+        check ships with the SDK, so it falls through to the entry points.
+        """
+        with patch(
+            "prowler.providers.common.builtin.importlib.util.find_spec",
+            side_effect=error,
+        ):
+            assert is_builtin_check("aws", "ec2", "ec2_instance_public_ip") is False
+
+    def test_reraises_plain_import_error(self):
+        """A plain ImportError can indicate a broken built-in check import."""
+        with patch(
+            "prowler.providers.common.builtin.importlib.util.find_spec",
+            side_effect=ImportError("partially initialised"),
+        ):
+            with pytest.raises(ImportError, match="partially initialised"):
+                is_builtin_check("aws", "ec2", "ec2_instance_public_ip")

tests/providers/external/test_dynamic_provider_loading.py

@@ -1333,15 +1333,17 @@ class TestCheckDiscovery:
 class TestCheckExecution:
     """Tests 15-17: _resolve_check_module."""
 
-    @patch("prowler.lib.check.check.importlib.util.find_spec")
+    @patch("prowler.lib.check.check.is_builtin_check")
     @patch("prowler.lib.check.check.import_check")
-    def test_resolve_check_module_builtin_first(self, mock_import, mock_find_spec):
+    def test_resolve_check_module_builtin_first(
+        self, mock_import, mock_is_builtin_check
+    ):
         """Test 15: _resolve_check_module resolves built-in checks first."""
         from prowler.lib.check.check import _resolve_check_module
 
         mock_module = MagicMock()
         mock_import.return_value = mock_module
-        mock_find_spec.return_value = MagicMock()  # built-in package exists
+        mock_is_builtin_check.return_value = True  # built-in check exists
 
         result = _resolve_check_module("aws", "ec2", "my_check")
 
@@ -1350,15 +1352,15 @@ class TestCheckExecution:
             "prowler.providers.aws.services.ec2.my_check.my_check"
         )
 
-    @patch("prowler.lib.check.check.importlib.util.find_spec")
+    @patch("prowler.lib.check.check.is_builtin_check")
     @patch("prowler.lib.check.check.import_check")
     def test_resolve_check_module_fallback_to_entry_point(
-        self, mock_import_check, mock_find_spec
+        self, mock_import_check, mock_is_builtin_check
     ):
         """Test 16: _resolve_check_module falls back to entry point when built-in is absent."""
         from prowler.lib.check.check import _resolve_check_module
 
-        mock_find_spec.return_value = None  # built-in does not exist
+        mock_is_builtin_check.return_value = False  # built-in does not exist
 
         mock_ext_module = MagicMock()
         ep = _make_entry_point(
@@ -1375,10 +1377,10 @@ class TestCheckExecution:
         mock_imp.assert_called_with("ext_pkg.checks.my_check")
         mock_import_check.assert_not_called()
 
-    @patch("prowler.lib.check.check.importlib.util.find_spec")
+    @patch("prowler.lib.check.check.is_builtin_check")
     @patch("prowler.lib.check.check.import_check")
     def test_resolve_check_module_builtin_wins_over_entry_point(
-        self, mock_import_check, mock_find_spec
+        self, mock_import_check, mock_is_builtin_check
     ):
         """Regression guard: when both a built-in and an entry-point check
         exist with the same CheckID, the BUILT-IN wins. Plug-ins extend
@@ -1389,7 +1391,7 @@ class TestCheckExecution:
         review (HugoPBrito)."""
         from prowler.lib.check.check import _resolve_check_module
 
-        mock_find_spec.return_value = MagicMock()  # built-in exists
+        mock_is_builtin_check.return_value = True  # built-in exists
         builtin_module = MagicMock()
         mock_import_check.return_value = builtin_module
 
@@ -1414,21 +1416,23 @@ class TestCheckExecution:
         mock_imp.assert_not_called()
 
     @patch("prowler.lib.check.check.importlib.metadata.entry_points")
-    @patch("prowler.lib.check.check.importlib.util.find_spec")
-    def test_resolve_check_module_raises_when_not_found(self, mock_find_spec, mock_ep):
+    @patch("prowler.lib.check.check.is_builtin_check")
+    def test_resolve_check_module_raises_when_not_found(
+        self, mock_is_builtin_check, mock_ep
+    ):
         """Test 17: _resolve_check_module raises ModuleNotFoundError when both fail."""
         from prowler.lib.check.check import _resolve_check_module
 
-        mock_find_spec.return_value = None
+        mock_is_builtin_check.return_value = False
         mock_ep.return_value = []
 
         with pytest.raises(ModuleNotFoundError, match="not found"):
             _resolve_check_module("fake", "svc", "nonexistent_check")
 
-    @patch("prowler.lib.check.check.importlib.util.find_spec")
+    @patch("prowler.lib.check.check.is_builtin_check")
     @patch("prowler.lib.check.check.import_check")
     def test_resolve_check_module_surfaces_error_when_builtin_import_fails(
-        self, mock_import_check, mock_find_spec
+        self, mock_import_check, mock_is_builtin_check
     ):
         """Regression guard: when no plug-in entry-point overrides the
         check, a built-in whose module exists but fails to import (e.g.
@@ -1437,7 +1441,7 @@ class TestCheckExecution:
         (HugoPBrito)."""
         from prowler.lib.check.check import _resolve_check_module
 
-        mock_find_spec.return_value = MagicMock()  # built-in module exists
+        mock_is_builtin_check.return_value = True  # built-in module exists
         mock_import_check.side_effect = ImportError("missing transitive dep: foo")
 
         # No plug-in override — the built-in's import failure must propagate
@@ -1445,6 +1449,40 @@ class TestCheckExecution:
             with pytest.raises(ImportError, match="missing transitive dep"):
                 _resolve_check_module("aws", "ec2", "ec2_instance_public_ip")
 
+    def test_resolve_check_module_entry_point_check_on_builtin_provider(self):
+        """Regression guard: a plug-in check attached to a BUILT-IN provider.
+
+        Deliberately does not mock the built-in probe. The bug this guards
+        against was invisible to every other test here precisely because they
+        mock `find_spec` and hand it `None`, while the real call raises: it
+        imports `prowler.providers.aws.services.ec2.<check>` as the parent it
+        must search, and that package only exists inside the plug-in. The raw
+        exception escaped `_resolve_check_module` before the entry points were
+        ever consulted, so no external check could run against aws, azure, gcp
+        or any other built-in provider.
+        """
+        from prowler.lib.check.check import _resolve_check_module
+
+        mock_module = MagicMock()
+        ep = _make_entry_point(
+            "ec2_acme_instance_has_owner_tag",
+            "acme_checks.services.ec2.ec2_acme_instance_has_owner_tag.ec2_acme_instance_has_owner_tag",
+            "prowler.checks.aws",
+        )
+
+        with (
+            patch("importlib.metadata.entry_points", return_value=[ep]),
+            patch("importlib.import_module", return_value=mock_module) as mock_imp,
+        ):
+            result = _resolve_check_module(
+                "aws", "ec2", "ec2_acme_instance_has_owner_tag"
+            )
+
+        assert result is mock_module
+        mock_imp.assert_called_with(
+            "acme_checks.services.ec2.ec2_acme_instance_has_owner_tag.ec2_acme_instance_has_owner_tag"
+        )
+
 
 # ===========================================================================
 # 5. CLI Arguments